From patchwork Thu Jun 23 01:19:16 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "J. R. Okajima" X-Patchwork-Id: 9194307 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A9D486075A for ; Thu, 23 Jun 2016 01:19:32 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9906D28422 for ; Thu, 23 Jun 2016 01:19:32 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8D02928425; Thu, 23 Jun 2016 01:19:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6F48E28422 for ; Thu, 23 Jun 2016 01:19:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751721AbcFWBTV (ORCPT ); Wed, 22 Jun 2016 21:19:21 -0400 Received: from mail04-md.ns.itscom.net ([175.177.155.114]:57371 "EHLO mail04-md.ns.itscom.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751709AbcFWBTU (ORCPT ); Wed, 22 Jun 2016 21:19:20 -0400 Received: from scan02-mds.s.noc.itscom.net (scan02-md.ns.itscom.net [175.177.155.123]) by mail04-md-outgoing.ns.itscom.net (Postfix) with ESMTP id 6E3286204E6; Thu, 23 Jun 2016 10:19:16 +0900 (JST) Received: from unknown (HELO mail05-md-outgoing.ns.itscom.net) ([175.177.155.115]) by scan02-mds.s.noc.itscom.net with ESMTP; 23 Jun 2016 10:19:16 +0900 Received: from jromail.nowhere (h175-177-085-178.catv02.itscom.jp [175.177.85.178]) by mail05-md-outgoing.ns.itscom.net (Postfix) with ESMTP; Thu, 23 Jun 2016 10:19:16 +0900 (JST) Received: from jro by jrobl id 1bFtIa-0005HE-2c ; Thu, 23 Jun 2016 10:19:16 +0900 From: "J. R. Okajima" Subject: Re: Q. hlist_bl_add_head_rcu() in d_alloc_parallel() To: Al Viro Cc: linux-fsdevel@vger.kernel.org In-Reply-To: <20160620053530.GI14480@ZenIV.linux.org.uk> References: <13136.1466196630@jrobl> <20160617221614.GE14480@ZenIV.linux.org.uk> <2123.1466313884@jrobl> <20160619165557.GH14480@ZenIV.linux.org.uk> <28627.1466397254@jrobl> <20160620053530.GI14480@ZenIV.linux.org.uk> Date: Thu, 23 Jun 2016 10:19:16 +0900 Message-ID: <20287.1466644756@jrobl> Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Al Viro: > That check is definitely bogus and I'm completely at loss as to WTF is it > doing there. Thanks for catching that; this kind of idiotic braino can > escape notice when rereading the code again and again, unfortunately ;-/ > > Fixed, will push to Linus tonight or tomorrow. Thank you too for fixing. I've confirmed the patch was merged and it passed my local tests too. Happy. I have another and relatively less important suggestion. Since the two groups tests in the loop are very similar, how about extract them as a new single static function? Do you think it will invite a performance down? J. R. Okajima --- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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 76afffd..3a9ebbc 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -2462,14 +2462,42 @@ static void d_wait_lookup(struct dentry *dentry) } } +/* tests for d_alloc_parallel() */ +static bool dap_test(struct dentry *dentry, const struct qstr *name, + struct dentry *parent, bool do_unhashed_test) +{ + bool ret; + + ret = false; + if (dentry->d_name.hash != name->hash) + goto out; + if (dentry->d_parent != parent) + goto out; + if (do_unhashed_test && d_unhashed(dentry)) + goto out; + if (parent->d_flags & DCACHE_OP_COMPARE) { + int tlen = dentry->d_name.len; + const char *tname = dentry->d_name.name; + if (parent->d_op->d_compare(parent, dentry, tlen, tname, name)) + goto out; + } else { + unsigned int len = name->len; + if (dentry->d_name.len != len) + goto out; + if (dentry_cmp(dentry, name->name, len)) + goto out; + } + ret = true; + +out: + return ret; +} + struct dentry *d_alloc_parallel(struct dentry *parent, const struct qstr *name, wait_queue_head_t *wq) { - unsigned int len = name->len; - unsigned int hash = name->hash; - const unsigned char *str = name->name; - struct hlist_bl_head *b = in_lookup_hash(parent, hash); + struct hlist_bl_head *b = in_lookup_hash(parent, name->hash); struct hlist_bl_node *node; struct dentry *new = d_alloc(parent, name); struct dentry *dentry; @@ -2515,21 +2543,8 @@ retry: * we encounter. */ hlist_bl_for_each_entry(dentry, node, b, d_u.d_in_lookup_hash) { - if (dentry->d_name.hash != hash) - continue; - if (dentry->d_parent != parent) + if (!dap_test(dentry, name, parent, /*do_unhashed_test*/false)) continue; - if (parent->d_flags & DCACHE_OP_COMPARE) { - int tlen = dentry->d_name.len; - const char *tname = dentry->d_name.name; - if (parent->d_op->d_compare(parent, dentry, tlen, tname, name)) - continue; - } else { - if (dentry->d_name.len != len) - continue; - if (dentry_cmp(dentry, str, len)) - continue; - } hlist_bl_unlock(b); /* now we can try to grab a reference */ if (!lockref_get_not_dead(&dentry->d_lockref)) { @@ -2550,23 +2565,9 @@ retry: * d_lookup() would've found anyway. If it is, just return it; * otherwise we really have to repeat the whole thing. */ - if (unlikely(dentry->d_name.hash != hash)) - goto mismatch; - if (unlikely(dentry->d_parent != parent)) + if (unlikely(!dap_test(dentry, name, parent, + /*do_unhashed_test*/true))) goto mismatch; - if (unlikely(d_unhashed(dentry))) - goto mismatch; - if (parent->d_flags & DCACHE_OP_COMPARE) { - int tlen = dentry->d_name.len; - const char *tname = dentry->d_name.name; - if (parent->d_op->d_compare(parent, dentry, tlen, tname, name)) - goto mismatch; - } else { - if (unlikely(dentry->d_name.len != len)) - goto mismatch; - if (unlikely(dentry_cmp(dentry, str, len))) - goto mismatch; - } /* OK, it *is* a hashed match; return it */ spin_unlock(&dentry->d_lock); dput(new);