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

2020-11-12 Thread Mateusz Guzik
On 11/12/20, Oliver Pinter  wrote:
> On Thursday, November 12, 2020, Mateusz Guzik  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 
>>  #include 
>>
>> +/*
>> + * 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-h...@freebsd.org mailing list
>> https://lists.freebsd.org/mailman/listinfo/svn-src-head
>> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
>>
>


-- 
Mateusz Guzik 
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


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

2020-11-12 Thread Oliver Pinter
On Thursday, November 12, 2020, Mateusz Guzik  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 
>  #include 
>
> +/*
> + * 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.


>
> 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-h...@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
>
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


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

2020-11-12 Thread Mateusz Guzik
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 
 #include 
 
+/*
+ * 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);
+}
+
 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);
 
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-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"