From patchwork Fri Sep 6 19:29:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bruce Fields X-Patchwork-Id: 2855071 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id C68C7BF43F for ; Fri, 6 Sep 2013 19:30:06 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 56BE720328 for ; Fri, 6 Sep 2013 19:30:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0D828202E8 for ; Fri, 6 Sep 2013 19:30:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751553Ab3IFT37 (ORCPT ); Fri, 6 Sep 2013 15:29:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:5493 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750910Ab3IFT36 (ORCPT ); Fri, 6 Sep 2013 15:29:58 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r86JTZsR032608 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 6 Sep 2013 15:29:35 -0400 Received: from pad.fieldses.org (vpn-63-165.rdu2.redhat.com [10.10.63.165]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r86JTY2B030836; Fri, 6 Sep 2013 15:29:34 -0400 Received: by pad.fieldses.org (Postfix, from userid 2815) id 557CC1A66E4; Fri, 6 Sep 2013 15:29:34 -0400 (EDT) Date: Fri, 6 Sep 2013 15:29:34 -0400 From: "J. Bruce Fields" To: Christoph Hellwig Cc: Al Viro , linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, Nick Piggin Subject: Re: [PATCH 2/3] dcache: Don't set DISCONNECTED on "pseudo filesystem" dentries Message-ID: <20130906192933.GC23157@pad.fieldses.org> References: <1378482230-16312-1-git-send-email-bfields@redhat.com> <1378482230-16312-2-git-send-email-bfields@redhat.com> <20130906170339.GB6460@infradead.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130906170339.GB6460@infradead.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-9.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Sep 06, 2013 at 10:03:39AM -0700, Christoph Hellwig wrote: > On Fri, Sep 06, 2013 at 11:43:49AM -0400, J. Bruce Fields wrote: > > From: "J. Bruce Fields" > > > > I can't for the life of me see any reason why anyone would care whether > > a dentry that is never hooked into the dentry cache would need > > DCACHE_DISCONNECTED set. > > __d_shrink did before your PATCH 1/3. Well, d_alloc_pseudo dentries aren't even hashed, so even d_shrink didn't actually reach that DCACHE_DISCONNECTED check. Uh, maybe change that "would" above to a "should". > > With that fixed your patch looks fine. > > > +/* > > + * For filesystems that do not actually use the dentry cache at all, and > > + * only ever deal in IS_ROOT() dentries: > > + */ > > If you document the function it really should be a kerneldoc comment. > > Also the description, while technically correct, isn't all that useful. > > It need an explanation why the filesystem is fine with unhashed > dentries, and the reason is that it never performs any lookups as it > pins the dentries in memory. Makes sense--result follows. Thanks for the review. --b. commit 089d891965374b4262973e51839fe361e2ca3cf0 Author: J. Bruce Fields Date: Fri Jun 29 16:20:47 2012 -0400 dcache: Don't set DISCONNECTED on "pseudo filesystem" dentries I can't for the life of me see any reason why anyone should care whether a dentry that is never hooked into the dentry cache would need DCACHE_DISCONNECTED set. This originates from 4b936885ab04dc6e0bb0ef35e0e23c1a7364d9e5 "fs: improve scalability of pseudo filesystems", which probably just made the false assumption the DCACHE_DISCONNECTED was meant to be set on anything not connected to a parent somehow. So this is just confusing. Ideally the only uses of DCACHE_DISCONNECTED would be in the filehandle-lookup code, which needs it to ensure dentries are connected into the dentry tree before use. I left d_alloc_pseudo there even though it's now equivalent to __d_alloc(), just on the theory the name is better documentation of its intended use outside dcache.c. Cc: Nick Piggin Signed-off-by: J. Bruce Fields --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/dcache.c b/fs/dcache.c index 7208b38..2255359 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1313,12 +1313,17 @@ struct dentry *d_alloc(struct dentry * parent, const struct qstr *name) } EXPORT_SYMBOL(d_alloc); +/** + * d_alloc_pseudo - allocate a dentry (for lookup-less filesystems) + * @sb: the superblock + * @name: qstr of the name + * + * For a filesystem that just pins its dentries in memory and never + * performs lookups at all, return an unhashed IS_ROOT dentry. + */ struct dentry *d_alloc_pseudo(struct super_block *sb, const struct qstr *name) { - struct dentry *dentry = __d_alloc(sb, name); - if (dentry) - dentry->d_flags |= DCACHE_DISCONNECTED; - return dentry; + return __d_alloc(sb, name); } EXPORT_SYMBOL(d_alloc_pseudo);