svn commit: r367626 - head/sys/geom/bde

[Mateusz Guzik]

On 11/12/20, Oliver Pinter <oliver.pntr at gmail.com> wrote:
> On Thursday, November 12, 2020, Mateusz Guzik <mjg at freebsd.org> wrote:
>
>> Author: mjg
>> Date: Thu Nov 12 20:20:57 2020
>> New Revision: 367626
>> URL: https://svnweb.freebsd.org/changeset/base/367626
>>
>> Log:
>>   gbde: replace malloc_last_fail with a kludge
>>
>>   This facilitates removal of malloc_last_fail without really impacting
>>   anything.
>>
>> Modified:
>>   head/sys/geom/bde/g_bde_work.c
>>
>> Modified: head/sys/geom/bde/g_bde_work.c
>> ============================================================
>> ==================
>> --- head/sys/geom/bde/g_bde_work.c      Thu Nov 12 20:20:43 2020
>> (r367625)
>> +++ head/sys/geom/bde/g_bde_work.c      Thu Nov 12 20:20:57 2020
>> (r367626)
>> @@ -77,6 +77,20 @@
>>  #include <geom/geom.h>
>>  #include <geom/bde/g_bde.h>
>>
>> +/*
>> + * FIXME: This used to call malloc_last_fail which in practice was
>> almost
>> + * guaranteed to return time_uptime even in face of severe memory
>> shortage.
>> + * As GBDE is the only consumer the kludge below was added to facilitate
>> the
>> + * removal with minimial changes. The code should be fixed to respond to
>> memory
>> + * pressure (e.g., by using lowmem eventhandler) instead.
>> + */
>> +static int
>> +g_bde_malloc_last_fail(void)
>> +{
>> +
>> +       return (time_uptime);
>> +}
>> +
>
>
> Previously malloc_last_fail returned a relatively small number - if i read
> the code correctly:
>
> -int
> -malloc_last_fail(void)
> -{
> -
> -       return (time_uptime - t_malloc_fail);
> -}
> -
>
>
>>  static void g_bde_delete_sector(struct g_bde_softc *wp, struct
>> g_bde_sector *sp);
>>  static struct g_bde_sector * g_bde_new_sector(struct g_bde_work *wp,
>> u_int len);
>>  static void g_bde_release_keysector(struct g_bde_work *wp);
>> @@ -210,7 +224,7 @@ g_bde_get_keysector(struct g_bde_work *wp)
>>         g_trace(G_T_TOPOLOGY, "g_bde_get_keysector(%p, %jd)", wp,
>> (intmax_t)offset);
>>         sc = wp->softc;
>>
>> -       if (malloc_last_fail() < g_bde_ncache)
>> +       if (g_bde_malloc_last_fail() < g_bde_ncache)
>>                 g_bde_purge_sector(sc, -1);
>
>
> And in this case, the semantic change renders all of these calls from alway
> true to always false expression.
>

t_malloc_fail value was almost guaranteed to always be 0, so there is
no actual change.

The hack was put in place so that gbde does not stall work on malloc.
gbde itself definitely needs love.

>
>>
>>         sp = TAILQ_FIRST(&sc->freelist);
>> @@ -228,7 +242,7 @@ g_bde_get_keysector(struct g_bde_work *wp)
>>                 if (sp->ref == 1)
>>                         sp->owner = wp;
>>         } else {
>> -               if (malloc_last_fail() < g_bde_ncache) {
>> +               if (g_bde_malloc_last_fail() < g_bde_ncache) {
>>                         TAILQ_FOREACH(sp, &sc->freelist, list)
>>                                 if (sp->ref == 0)
>>                                         break;
>> @@ -311,7 +325,7 @@ g_bde_purge_sector(struct g_bde_softc *sc, int
>> fractio
>>         if (fraction > 0)
>>                 n = sc->ncache / fraction + 1;
>>         else
>> -               n = g_bde_ncache - malloc_last_fail();
>> +               n = g_bde_ncache - g_bde_malloc_last_fail();
>>         if (n < 0)
>>                 return;
>>         if (n > sc->ncache)
>> _______________________________________________
>> svn-src-head at freebsd.org mailing list
>> https://lists.freebsd.org/mailman/listinfo/svn-src-head
>> To unsubscribe, send any mail to "svn-src-head-unsubscribe at freebsd.org"
>>
>


-- 
Mateusz Guzik <mjguzik gmail.com>