On 11/11/17(Sat) 13:05, Gregor Best wrote:
> Hi people,
> 
> while reading around in /sys/kern/uipc_mbuf.c to try to track down a
> problem with my iwm(4) that seems to correlate with mbuf allocation
> failures, I noticed that the MBSTAT_DROPS counter and its friends
> MBSTAT_{WAIT,DRAIN} don't seem to get increased anywhere in /sys.
> 
> Does the patch below the signature make sense for counting MBSTAT_DROPS?

It does, some comments below.

> I've got a similar patch for MBSTAT_WAIT, but it's pretty ugly because
> as far as I can see, there's no real way to notice when pool_get sleeps
> except for "Try pool_get with M_NOWAIT first and if that returns NULL,
> try again with M_WAITOK".

This would be an approximation because it might happen that after
returning NULL the second pool_get(9) won't sleep.  However I think
it's the way to go because m_get(9) calls that can wait are generally
not performance critical.

> Index: /sys/kern/uipc_mbuf.c
> ===================================================================
> RCS file: /home/cvs/src/sys/kern/uipc_mbuf.c,v
> retrieving revision 1.250
> diff -u -p -r1.250 uipc_mbuf.c
> --- /sys/kern/uipc_mbuf.c     12 Oct 2017 09:14:16 -0000      1.250
> +++ /sys/kern/uipc_mbuf.c     11 Nov 2017 12:03:39 -0000
> @@ -233,14 +233,14 @@ m_get(int nowait, int type)
>       KDASSERT(type < MT_NTYPES);
>  
>       m = pool_get(&mbpool, nowait == M_WAIT ? PR_WAITOK : PR_NOWAIT);
> -     if (m == NULL)
> -             return (NULL);
>  
>       s = splnet();
>       counters = counters_enter(&cr, mbstat);
> +     if (m == NULL) {
> +             counters[MBSTAT_DROPS]++;
> +             goto out;
> +     }
>       counters[type]++;
> -     counters_leave(&cr, mbstat);
> -     splx(s);

Please do not expand the splx().  Only the counter need it.  Simply put
the NULL check after the splx().

Reply via email to