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().