svn commit: r364944 - head/sys/kern

[Mark Johnston]

On Tue, Sep 15, 2020 at 10:22:09AM -0400, Mark Johnston wrote:
> On Tue, Sep 15, 2020 at 05:15:30PM +0300, Konstantin Belousov wrote:
> > On Sat, Aug 29, 2020 at 04:29:53AM +0000, Warner Losh wrote:
> > > Author: imp
> > > Date: Sat Aug 29 04:29:53 2020
> > > New Revision: 364944
> > > URL: https://svnweb.freebsd.org/changeset/base/364944
> > > 
> > > Log:
> > >   devctl: move to using a uma zone
> > >   
> > >   Convert the memory management of devctl.  Rewrite if to make better
> > >   use of memory. This eliminates several mallocs (5? worse case) needed
> > >   to send a message. It's now possible to always send a message, though
> > >   if things are really backed up the oldest message will be dropped to
> > >   free up space for the newest.
> > >   
> > >   Add a static bus_child_{location,pnpinfo}_sb to start migrating to
> > >   sbuf instead of buffer + length. Use it in the new code.  Other code
> > >   will be converted later (bus_child_*_str is only used inside of
> > >   subr_bus.c, though implemented in ~100 places in the tree).
> > >   
> > >   Reviewed by: markj@
> > >   Differential Revision: https://reviews.freebsd.org/D26140
> > > 
> > > Modified:
> > >   head/sys/kern/subr_bus.c
> > > 
> > 
> > > +	} else {
> > > +		/* dei can't be NULL -- we know we have at least one in the zone */
> > > +		dei = uma_zalloc(devsoftc.zone, M_NOWAIT);
> > > +		MPASS(dei != NULL);
> > This does not work.  I believe you need to disable per-cpu cache for the
> > zone, at least.  But I am not sure it is enough.
> 
> From the report we have:
> 
> db:0:pho>  show uma
>               Zone   Size    Used    Free    Requests  Sleeps  Bucket  Total Mem    XFree
>             DEVCTL   1024 6415164       0     6416203       0      16 6569127936        0
> 
> so it looks like the primary problem is a leak.
> 
> > https://people.freebsd.org/~pho/stress/log/kostik1314.txt

devctl_queue() does not maintain the queue length bound, so if devd goes
away (due to an OOM kill in this case), we can end up with a large
backlog.  This hack might be sufficient.

diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 19c056ab9974..91c57cdfae5b 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -635,7 +635,14 @@ devctl_queue(struct dev_event_info *dei)
 {
 	mtx_lock(&devsoftc.mtx);
 	STAILQ_INSERT_TAIL(&devsoftc.devq, dei, dei_link);
-	devsoftc.queued++;
+	if (devctl_queue_length != 0 &&
+	    devctl_queue_length == devsoftc.queued) {
+		dei = STAILQ_FIRST(&devsoftc.devq);
+		STAILQ_REMOVE_HEAD(&devsoftc.devq, dei_link);
+		uma_zfree(devsoftc.zone, dei);
+	} else {
+		devsoftc.queued++;
+	}
 	cv_broadcast(&devsoftc.cv);
 	KNOTE_LOCKED(&devsoftc.sel.si_note, 0);
 	mtx_unlock(&devsoftc.mtx);