From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: [RFC PATCH] ocfs2: don't depend on DCACHE_DISCONNECTED Date: Thu, 2 Aug 2012 08:59:10 -0400 Message-ID: <20120802125910.GC9849@fieldses.org> References: <20120731223323.GA32253@fieldses.org> <20120802075743.GA3368@dhcp-172-17-9-228.mtv.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii To: Mark Fasheh , ocfs2-devel@oss.oracle.com, linux-fsdevel@vger.kernel.org Return-path: Received: from fieldses.org ([174.143.236.118]:47483 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751214Ab2HBM7M (ORCPT ); Thu, 2 Aug 2012 08:59:12 -0400 Content-Disposition: inline In-Reply-To: <20120802075743.GA3368@dhcp-172-17-9-228.mtv.corp.google.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Aug 02, 2012 at 12:57:44AM -0700, Joel Becker wrote: > On Tue, Jul 31, 2012 at 06:33:23PM -0400, J. Bruce Fields wrote: > > From: "J. Bruce Fields" > > > > XXX: I don't understand this code, but I also can't see how it can be > > right as is: a dentry marked DCACHE_DISCONNECTED can in fact be a > > fully-connected member of the dcache. Is IS_ROOT() the right check > > instead? > > > > Signed-off-by: J. Bruce Fields > > NAK. DISCONNECTED is cleared when the dentry is materialized. Are you sure? ocfs2 uses d_splice_alias in its lookup method, which doesn't clear DISCONNECTED. (d_materialise_unique does something similar to d_splice_alias and also clears DISCONNECTED. However, I'm almost certain that's a bug in d_materialise_unique. The export code connects a looked-up-by-filehandle directory by doing lookups in parents one step at a time up to the root, only clearing DISCONNECTED once we know that the dentry is connected all the way up to the root. Clearing DISCONNECTED as soon as a single dentry is connected to parents will lead to bugs.) > Here's the context. When an ocfs2 dentry is discoverable via the tree > (lookup or splicing an alias), we hold a cluster lock (the "dentry > lock"). This is why we override d_move(), to make sure that state is > kept sane. That way, other nodes can communicate unlink to this node. Alas, I'm not following you. I'll stare at some of the code and try to understand.... > They notify our node via the locking system, which does a > d_delete()+dput(), which sets DISCONNECTED. When the dentry gets its > final put, we can properly accept an empty lock. So you also depend on d_kill setting DISCONNECTED, huh. --b.