Re: [PATCH] vfs: add d_prune dentry operation

[Sage Weil]

On Mon, 10 Oct 2011, Dave Chinner wrote:
> On Thu, Oct 06, 2011 at 03:20:37PM -0700, Sage Weil wrote:
> > On Thu, 6 Oct 2011, Christoph Hellwig wrote:
> > > On Wed, Oct 05, 2011 at 09:26:10PM -0700, Sage Weil wrote:
> > > > This adds a d_prune dentry operation that is called by the VFS prior to
> > > > pruning (i.e. unhashing and killing) a hashed dentry from the dcache.  This
> > > > will be used by Ceph to maintain a flag indicating whether the complete
> > > > contents of a directory are contained in the dcache, allowing it to satisfy
> > > > lookups and readdir without addition server communication.
> > > 
> > > What tree is the patch against?  I seems to fail to apply against Linus'
> > > latests.  
> > 
> > Whoops, I rebased against v3.0 instead of latest master.
> > 
> > > It also seem like it basically should not be be opencoded but in a
> > > wrapper around dentry_lru_del for all cases but the lazy removal from
> > > LRU in case that is referenced, with some comments explaining the whole
> > > thing.
> > 
> > Yeah, that's a bit better.  There are four dentry_lru_del() callers, two 
> > where we there is a reference, and two where we want to ->d_prune too.
> 
> The code in the patch doesn't explain to me why you'd need to call
> dentry_lru_prune() rather than dentry_lru_del()? It's something to
> do with the difference between active and inactive LRU removal, but
> I can't really tell. My patchset removes the LRU abuse from
> select_parent, so I'm kind of wondering what the correct thing is to
> do there. 
> 
> Hence, can you add a bit of documentation to those functions
> explaining why and when you should use one or the other?  i.e
> document the situations where the FS needs to be notified of
> pruning, rather than leaving anyone who is reading the code
> guessing.

Let me know if the below is clearer.  The requirement is that ->d_prune be 
called prior to unhashing (and then destroying) a victim dentry.  I'm a 
little worried about mixing that in with the lru helpers because it is 
only indirectly related to whether the dentry is on the LRU, and that may 
confuse people.  A cleaned up opencoded patch is here:

	https://github.com/NewDreamNetwork/ceph-client/commit/784a6aa6dc7baf2069c40988d79130dba17c7068

and the updated dentry_lru_prune() wrapper version is below.

Thanks-
sage