On Sun, Nov 12, 2017 at 11:23:31PM +0100, Gregor Best wrote: > Hi Martin, > > On Sun, Nov 12, 2017 at 03:40:59PM +0100, Martin Pieuchot wrote: > > [...] > > It does, some comments below. > > [...] > > Wonderful. > > > [...] > > 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. > > [...] > > I had not considered that the second pool_get might not block at all. On > the other hand `netstat -m` says "requests for memory delayed" for that > counter, so returning immediately on the second try not counting as a > delay is OK, I think. > > > [...] > > Please do not expand the splx(). Only the counter need it. Simply put > > the NULL check after the splx(). > > [...] > > I'm not sure I understand. If I move the NULL check after the splx(), > counters_leave() will already have been called so increasing the counter > value has no effect anymore. The only additional things running under > splnet will be a few assignments, so I think moving the splx() a bit > further down does not hurt too much. > > Alternatively, I've attached a slightly different suggestion which > doesn't expand the scope of the splx() but duplicates a bit of code. > Does that make more sense?
pools maintain count of how many times they failed to provide an allocation. you can watch this with vmstat -m or systat pool. however, we could use that to populate mb_drops too. the diff below moves populating the mbstat from kern_sysctl to uipc_mbuf, and adds copying of pr_nfails in. if we want to count the number of "delays", i could easily add that to pools too and copy it out in sysctl_mbstat. Index: sys/sysctl.h =================================================================== RCS file: /cvs/src/sys/sys/sysctl.h,v retrieving revision 1.175 diff -u -p -r1.175 sysctl.h --- sys/sysctl.h 12 Oct 2017 09:14:16 -0000 1.175 +++ sys/sysctl.h 13 Nov 2017 03:42:32 -0000 @@ -936,6 +936,7 @@ int sysctl_rdstruct(void *, size_t *, vo int sysctl_struct(void *, size_t *, void *, size_t, void *, size_t); int sysctl_file(int *, u_int, char *, size_t *, struct proc *); int sysctl_doproc(int *, u_int, char *, size_t *); +int sysctl_mbstat(int *, u_int, void *, size_t *, void *, size_t); struct mbuf_queue; int sysctl_mq(int *, u_int, void *, size_t *, void *, size_t, struct mbuf_queue *); Index: kern/kern_sysctl.c =================================================================== RCS file: /cvs/src/sys/kern/kern_sysctl.c,v retrieving revision 1.330 diff -u -p -r1.330 kern_sysctl.c --- kern/kern_sysctl.c 11 Aug 2017 21:24:19 -0000 1.330 +++ kern/kern_sysctl.c 13 Nov 2017 03:42:32 -0000 @@ -392,24 +392,9 @@ kern_sysctl(int *name, u_int namelen, vo case KERN_FILE: return (sysctl_file(name + 1, namelen - 1, oldp, oldlenp, p)); #endif - case KERN_MBSTAT: { - extern struct cpumem *mbstat; - uint64_t counters[MBSTAT_COUNT]; - struct mbstat mbs; - unsigned int i; - - memset(&mbs, 0, sizeof(mbs)); - counters_read(mbstat, counters, MBSTAT_COUNT); - for (i = 0; i < MBSTAT_TYPES; i++) - mbs.m_mtypes[i] = counters[i]; - - mbs.m_drops = counters[MBSTAT_DROPS]; - mbs.m_wait = counters[MBSTAT_WAIT]; - mbs.m_drain = counters[MBSTAT_DRAIN]; - - return (sysctl_rdstruct(oldp, oldlenp, newp, - &mbs, sizeof(mbs))); - } + case KERN_MBSTAT: + return (sysctl_mbstat(name + 1, namelen -1, oldp, oldlenp, + newp, newlen)); #if defined(GPROF) || defined(DDBPROF) case KERN_PROF: return (sysctl_doprof(name + 1, namelen - 1, oldp, oldlenp, Index: kern/uipc_mbuf.c =================================================================== RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v retrieving revision 1.250 diff -u -p -r1.250 uipc_mbuf.c --- kern/uipc_mbuf.c 12 Oct 2017 09:14:16 -0000 1.250 +++ kern/uipc_mbuf.c 13 Nov 2017 03:42:32 -0000 @@ -1421,6 +1421,30 @@ m_pool_init(struct pool *pp, u_int size, pool_set_constraints(pp, &kp_dma_contig); } +int +sysctl_mbstat(int *name, u_int namelen, void *oldp, size_t *oldlenp, + void *newp, size_t newlen) +{ + uint64_t counters[MBSTAT_COUNT]; + struct mbstat mbs; + unsigned int i; + + if (namelen != 0) + return (ENOTDIR); + + memset(&mbs, 0, sizeof(mbs)); + + counters_read(mbstat, counters, MBSTAT_COUNT); + for (i = 0; i < MBSTAT_TYPES; i++) + mbs.m_mtypes[i] = counters[i]; + + mbs.m_drops = counters[MBSTAT_DROPS] + mbpool.pr_nfail; + mbs.m_wait = counters[MBSTAT_WAIT]; + mbs.m_drain = counters[MBSTAT_DRAIN]; /* pool gcs? */ + + return (sysctl_rdstruct(oldp, oldlenp, newp, &mbs, sizeof(mbs))); +} + #ifdef DDB void m_print(void *v,