From owner-svn-src-all@freebsd.org Thu Nov 30 18:37:10 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 2365EE6856A; Thu, 30 Nov 2017 18:37:10 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-it0-f50.google.com (mail-it0-f50.google.com [209.85.214.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id DCF486F8F9; Thu, 30 Nov 2017 18:37:09 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-it0-f50.google.com with SMTP id r6so9557489itr.3; Thu, 30 Nov 2017 10:37:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to:cc; bh=4EXEh7mSYO83pPNoEdccDZBFcLJ/DOHfZodKvKJFf5Q=; b=hTtD6BKlzxn0qtl5Qaq3UU0C9UtR/HegWRkIw/49nqW51IBaOpXWVLiWCn6MrizGtj 8ucg1nNJIX9j0PdTAkgQ0YqdzLr2FKqSLwm103/do7JH/jsLaRpXNp1HsayJLA+utT77 wWSLLl82+h0p2TdU8x48CHSONmWqQALmFS8WEDEy/Qmfxd17iXtt72oQjRb/qhJ+M4hb Wied0NK1XEDHZhpEQIpC7kSwH+zLkA8wOlKshvUrNGZIAz0inBpDqN44QzWwaW7lo47k zJEUU8+FgSZh2n7YO56prBovFARibGkw1A/D/wj+/x8JtkWqDFUvHyKSkyvZE42CzT71 Rlfw== X-Gm-Message-State: AJaThX4h3fSfwKcuvAkNqIJ05rlz7rIJte0WdQxDcgJpNDcZRD+KFqqC sOxmSvE1Rff6ox93EQ9DP4nMqJWm X-Google-Smtp-Source: AGs4zMauSbM5hwqQN+Yw08GYbE4Xq6B1IVI8z0mpSFH42lv4R4e+1pNTkgTK9IeCDvjLkWBaDP23+g== X-Received: by 10.36.3.65 with SMTP id e62mr4045170ite.99.1512066710451; Thu, 30 Nov 2017 10:31:50 -0800 (PST) Received: from mail-it0-f48.google.com (mail-it0-f48.google.com. [209.85.214.48]) by smtp.gmail.com with ESMTPSA id f10sm2553346itb.13.2017.11.30.10.31.49 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 30 Nov 2017 10:31:49 -0800 (PST) Received: by mail-it0-f48.google.com with SMTP id t1so9436369ite.5; Thu, 30 Nov 2017 10:31:49 -0800 (PST) X-Received: by 10.36.192.2 with SMTP id u2mr4348910itf.119.1512066709485; Thu, 30 Nov 2017 10:31:49 -0800 (PST) MIME-Version: 1.0 Reply-To: cem@freebsd.org Received: by 10.2.165.150 with HTTP; Thu, 30 Nov 2017 10:31:49 -0800 (PST) In-Reply-To: <201711301238.vAUCcgrC091396@repo.freebsd.org> References: <201711301238.vAUCcgrC091396@repo.freebsd.org> From: Conrad Meyer Date: Thu, 30 Nov 2017 10:31:49 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r326394 - head/sys/fs/devfs To: Emmanuel Vadot Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Nov 2017 18:37:10 -0000 This moves the M_WAITOK (sleepable) memory allocation under dirlist_mtx, which is an ordinary mutex. If you run this under WITNESS, it will (rightfully) complain. Please revert this change. One possible way to accomplish the goal you have in mind is a fallback path if the devfs_dir_findent_locked misses. E.g.: =====================================8<================================== --- a/sys/fs/devfs/devfs_dir.c +++ b/sys/fs/devfs/devfs_dir.c @@ -96,19 +96,27 @@ devfs_dir_ref(const char *dir) if (*dir == '\0') return; - dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK); - dle_new->dir = strdup(dir, M_DEVFS4); - dle_new->refcnt = 1; + dle_new = NULL; +again: mtx_lock(&dirlist_mtx); dle = devfs_dir_findent_locked(dir); if (dle != NULL) { dle->refcnt++; mtx_unlock(&dirlist_mtx); - free(dle_new->dir, M_DEVFS4); - free(dle_new, M_DEVFS4); + if (dle_new != NULL) { + free(dle_new->dir, M_DEVFS4); + free(dle_new, M_DEVFS4); + } return; } + if (dle_new == NULL) { + mtx_unlock(&dirlist_mtx); + dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK); + dle_new->dir = strdup(dir, M_DEVFS4); + dle_new->refcnt = 1; + goto again; + } LIST_INSERT_HEAD(&devfs_dirlist, dle_new, link); mtx_unlock(&dirlist_mtx); } =====================================8<================================== The downside of this method is we potentially have to take the dirlist mutex twice. An even more "optimized" approach would be to M_NOWAIT the allocations inside the lock and fallback if those fail. But that introduces even more code complexity. Best, Conrad On Thu, Nov 30, 2017 at 4:38 AM, Emmanuel Vadot wrote: > Author: manu > Date: Thu Nov 30 12:38:42 2017 > New Revision: 326394 > URL: https://svnweb.freebsd.org/changeset/base/326394 > > Log: > devfs: Avoid a malloc/free if we just need to increment the refcount > > MFC after: 1 week > Sponsored by: Gandi.net > > Modified: > head/sys/fs/devfs/devfs_dir.c > > Modified: head/sys/fs/devfs/devfs_dir.c > ============================================================================== > --- head/sys/fs/devfs/devfs_dir.c Thu Nov 30 12:22:15 2017 (r326393) > +++ head/sys/fs/devfs/devfs_dir.c Thu Nov 30 12:38:42 2017 (r326394) > @@ -98,19 +98,18 @@ devfs_dir_ref(const char *dir) > if (*dir == '\0') > return; > > - dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK); > - dle_new->dir = strdup(dir, M_DEVFS4); > - dle_new->refcnt = 1; > - > mtx_lock(&dirlist_mtx); > dle = devfs_dir_findent_locked(dir); > if (dle != NULL) { > dle->refcnt++; > mtx_unlock(&dirlist_mtx); > - free(dle_new->dir, M_DEVFS4); > - free(dle_new, M_DEVFS4); > return; > } > + > + dle_new = malloc(sizeof(*dle), M_DEVFS4, M_WAITOK); > + dle_new->dir = strdup(dir, M_DEVFS4); > + dle_new->refcnt = 1; > + > LIST_INSERT_HEAD(&devfs_dirlist, dle_new, link); > mtx_unlock(&dirlist_mtx); > } >