From owner-svn-src-head@freebsd.org Thu Sep 17 17:29:34 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id AC5C53E7F33; Thu, 17 Sep 2020 17:29:34 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BskVQ49Mhz4Kjg; Thu, 17 Sep 2020 17:29:34 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 6E214DB1C; Thu, 17 Sep 2020 17:29:34 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 08HHTYEF066184; Thu, 17 Sep 2020 17:29:34 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 08HHTYlU066183; Thu, 17 Sep 2020 17:29:34 GMT (envelope-from imp@FreeBSD.org) Message-Id: <202009171729.08HHTYlU066183@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Thu, 17 Sep 2020 17:29:34 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r365843 - head/sys/kern X-SVN-Group: head X-SVN-Commit-Author: imp X-SVN-Commit-Paths: head/sys/kern X-SVN-Commit-Revision: 365843 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 17 Sep 2020 17:29:34 -0000 Author: imp Date: Thu Sep 17 17:29:33 2020 New Revision: 365843 URL: https://svnweb.freebsd.org/changeset/base/365843 Log: Move to a more robust and conservative alloation scheme for devctl messages Change the zone setup: - Allow slabs to be returned to the OS - Set the number of slots to the max devctl will queue before discarding - Reserve 2% of the max (capped at 100) for low memory allocations - Disable per-cpu caching since we don't need it and we avoid some pathologies Change the alloation strategiy a bit: - If a normal allocation fails, try to get the reserve - If a reserve allocation fails, re-use the oldest-queued entry for storage - If there's a weird race/failure and nothing on the queue to steal, return NULL This addresses two main issues in the old code: - If devd had died, and we're generating a lot of messages, we have an unbounded leak. This new scheme avoids the issue that lead to this. - The MPASS that was 'sure' the allocation couldn't have failed turned out to be wrong in some rare cases. The new code doesn't make this assumption. Since we reserve only 2% of the space, we go from about 1MB of allocation all the time to more like 50kB for the reserve. Reviewed by: markj@ Differential Revision: https://reviews.freebsd.org/D26448 Modified: head/sys/kern/subr_bus.c Modified: head/sys/kern/subr_bus.c ============================================================================== --- head/sys/kern/subr_bus.c Thu Sep 17 17:07:04 2020 (r365842) +++ head/sys/kern/subr_bus.c Thu Sep 17 17:29:33 2020 (r365843) @@ -426,6 +426,9 @@ static struct cdev *devctl_dev; static void devinit(void) { + int reserve; + uma_zone_t z; + devctl_dev = make_dev_credf(MAKEDEV_ETERNAL, &dev_cdevsw, 0, NULL, UID_ROOT, GID_WHEEL, 0600, "devctl"); mtx_init(&devsoftc.mtx, "dev mtx", "devd", MTX_DEF); @@ -433,9 +436,21 @@ devinit(void) STAILQ_INIT(&devsoftc.devq); knlist_init_mtx(&devsoftc.sel.si_note, &devsoftc.mtx); if (devctl_queue_length > 0) { - devsoftc.zone = uma_zcreate("DEVCTL", sizeof(struct dev_event_info), - NULL, NULL, NULL, NULL, UMA_ALIGN_PTR, UMA_ZONE_NOFREE); - uma_prealloc(devsoftc.zone, devctl_queue_length); + /* + * Allocate a zone for the messages. Preallocate 2% of these for + * a reserve. Allow only devctl_queue_length slabs to cap memory + * usage. The reserve usually allows coverage of surges of + * events during memory shortages. Normally we won't have to + * re-use events from the queue, but will in extreme shortages. + */ + z = devsoftc.zone = uma_zcreate("DEVCTL", + sizeof(struct dev_event_info), NULL, NULL, NULL, NULL, + UMA_ALIGN_PTR, 0); + reserve = max(devctl_queue_length / 50, 100); /* 2% reserve */ + uma_zone_set_max(z, devctl_queue_length); + uma_zone_set_maxcache(z, 0); + uma_zone_reserve(z, reserve); + uma_prealloc(z, reserve); } devctl2_init(); } @@ -598,15 +613,25 @@ devctl_alloc_dei(void) mtx_lock(&devsoftc.mtx); if (devctl_queue_length == 0) goto out; - if (devctl_queue_length == devsoftc.queued) { + dei = uma_zalloc(devsoftc.zone, M_NOWAIT); + if (dei == NULL) + dei = uma_zalloc(devsoftc.zone, M_NOWAIT | M_USE_RESERVE); + if (dei == NULL) { + /* + * Guard against no items in the queue. Normally, this won't + * happen, but if lots of events happen all at once and there's + * a chance we're out of allocated space but none have yet been + * queued when we get here, leaving nothing to steal. This can + * also happen with error injection. Fail safe by returning + * NULL in that case.. + */ + if (devsoftc.queued == 0) + goto out; dei = STAILQ_FIRST(&devsoftc.devq); STAILQ_REMOVE_HEAD(&devsoftc.devq, dei_link); devsoftc.queued--; - } 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); } + MPASS(dei != NULL); *dei->dei_data = '\0'; out: mtx_unlock(&devsoftc.mtx);