From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail01-md.ns.itscom.net ([175.177.155.111]:57107 "EHLO mail01-md.ns.itscom.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751300AbcFSFYs (ORCPT ); Sun, 19 Jun 2016 01:24:48 -0400 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: <20160617221614.GE14480@ZenIV.linux.org.uk> References: <13136.1466196630@jrobl> <20160617221614.GE14480@ZenIV.linux.org.uk> Date: Sun, 19 Jun 2016 14:24:44 +0900 Message-ID: <2123.1466313884@jrobl> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Al Viro: > Huh? If you have two processes reaching that insertion into the in-lookup > hash, whoever gets the hlist_bl_lock() first wins; the loser will find > the instance added by the winner and bugger off with it. RCU is completely > unrelated to that. It's about the search in *primary* hash. Ok, forget about rcu_barrier. ---------------------------------------- > look for match in in-lookup hash > if found > unlock the chain > wait for the match to cease being in-lookup > drop the match > goto retry [see below] > insert new into in-lookup hash The actual matching test which corresponds to above pseudo-code (if found) is this (from v4.7-rc3). dentry->d_name.hash != hash dentry->d_parent != parent d_unhashed(dentry) name (length and string) I am afraid this d_unhashed() test is racy. Here is what I am guessing. - two processes try opening the same file - the both enter the hlist_bl_lock protected loop in d_alloc_parallel() - the winner puts the new dentry into in-lookup hash + here d_unhashed(dentry) would still return true. - then the winner process will call ->atomic_open or ->lookup. finally d_add() and rehash will be called and the dentry will be moved to the primary hash. + here d_unhashed(dentry) would return false. As soon as the winner calls hlist_bl_unlock(), the looser starts d_in_lookup_hash loop and find the dentry which the winner added. - the looser (or we should call processB) do the tests dentry->d_name.hash != hash dentry->d_parent != parent d_unhashed(dentry) - if processA has already called d_add and rehash, then this d_unhashed() test would return false, and processB will throw away his own 'new' dentry and return the found one. - if processA has NOT called d_add and rehash yet (due to the schedule timing), then this d_unhashed() test would return true, and processB will simply skip the found dentry. in this case, processB will add his own 'new' dentry into in-lookup hash and return it. Finally this race between these two - d_add and rehash via ->atomic_open or ->lookup - d_unhashed test in d_alloc_parallel leads to the duplicated dentries (same named dentry under the same parent). Do you think it can happen? J. R. Okajima