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,

Reply via email to