From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from zeniv.linux.org.uk ([195.92.253.2]:53233 "EHLO
	ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S932120AbcCJUeb (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Thu, 10 Mar 2016 15:34:31 -0500
Date: Thu, 10 Mar 2016 20:34:28 +0000
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org
Subject: do we need that smp_wmb() in __d_alloc()?
Message-ID: <20160310203428.GM17997@ZenIV.linux.org.uk>
References: <20160308160537.GV17997@ZenIV.linux.org.uk>
 <498D5A19-9E55-48D7-B5CF-34CA5769FF7F@intel.com>
 <20160308211148.GX17997@ZenIV.linux.org.uk>
 <A5CA4682-77B1-4DF4-A784-7C7C2FAFC9AF@intel.com>
 <20160309003416.GY17997@ZenIV.linux.org.uk>
 <7C3EBB6F-54AC-4744-BEC1-33EA82216F85@intel.com>
 <20160309012658.GZ17997@ZenIV.linux.org.uk>
 <34C2B1C3-2B7F-490B-A03A-3BCDDFC8BE48@intel.com>
 <20160310022041.GF17997@ZenIV.linux.org.uk>
 <20160310195951.GL17997@ZenIV.linux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160310195951.GL17997@ZenIV.linux.org.uk>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Thu, Mar 10, 2016 at 07:59:51PM +0000, Al Viro wrote:

> int ll_d_init(struct dentry *de)
> {
> 	struct ll_dentry_data *lld = kzalloc(sizeof(*lld), GFP_NOFS);
> 	if (unlikely(!lld))
> 		return -ENOMEM;
> 	lld->lld_invalid = 1;
> 	smp_wmb();	/* read barrier in whatever will find us */
> 	de->d_fsdata = lld;
>         return 0;
> }
> 
> as its ->d_init() and forget about all that mess.
> 
> Objections, better ideas?

Speaking of barriers - why do we need one there at all?  In __d_alloc(), that
is.  Look: callers of __d_alloc() are:
	* d_alloc() - cycles parent's ->d_lock, inserts into the list of
parent's children.  Anyone observing it on that list of children will be
holding the same parent's ->d_lock.  And anyone finding it in any other way
will have to observe the effect of store done after the write barrier in
spin_unlock.
	* __d_obtain_alias() - same story, only it's ->i_lock of the inode
and ->d_lock of dentry itself.

There's also d_alloc_pseudo() and d_make_root(); I suspect that for
d_make_root() an implicit barrier in unlocking ->s_umount would serve,
but in any case, wouldn't it make more sense to lift that smp_wmb() from
__d_alloc() to d_alloc_pseudo() and d_make_root()?  Linus, do you see
any problems with that?  I don't really trust my taste wrt barriers, so...