Date: Tue, 15 Sep 2020 09:20:27 -0600 From: Warner Losh <imp@bsdimp.com> To: Mark Johnston <markj@freebsd.org> Cc: Konstantin Belousov <kostikbel@gmail.com>, Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>, Peter Holm <peter@holm.cc> Subject: Re: svn commit: r364944 - head/sys/kern Message-ID: <CANCZdfo2bxKSJrFxa6zPU9kSuRbYmr02nbqD63vuqOSntKAJnQ@mail.gmail.com> In-Reply-To: <20200915143730.GB4145@raichu> References: <202008290429.07T4TrbH007764@repo.freebsd.org> <20200915141530.GM94807@kib.kiev.ua> <20200915142209.GA4145@raichu> <20200915143730.GB4145@raichu>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Sep 15, 2020 at 8:37 AM Mark Johnston <markj@freebsd.org> wrote: > 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); > I've come to a similar conclusion... You can't queue w/o allocating... and that's where we're supposed to pop off the oldest, free it so we can allocate. So this isn't the right place for this fix, and the 'queued' name may be a bad name since it's from before. It's now supposed to be closer to 'allocated'. Warner
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfo2bxKSJrFxa6zPU9kSuRbYmr02nbqD63vuqOSntKAJnQ>