Re: [PATCH] dcache: separate slab for directory dentries

Dave Kleikamp
2005-10-07T13:00:02Z

On Fri, 2005-10-07 at 13:54 +1000, Greg Banks wrote:
> On Thu, 2005-10-06 at 22:28, Dave Kleikamp wrote:
> > On Thu, 2005-10-06 at 16:27 +1000, David Chinner wrote:
> > > +static struct dentry * d_realloc_for_inode(struct dentry * dentry,
> > > + struct inode *inode)
> > > +{
> > > + int flags = 0;
> > > + struct dentry *new;
> > > [...]
> > > + new = __d_alloc(parent, &dentry->d_name, dentry->d_flags | flags);
> > > +
> > > + spin_lock(&dcache_lock);
> > > +
> > > + BUG_ON(new == NULL); /* TODO */
> > > + if (new) {
> > > +// new->d_op = dentry->d_op;
> > > +// new->d_fsdata = dentry->d_fsdata;
> > > + }
> > > +
> > > + return new;
> > > +}
> >
> > Isn't this leaking the original dentry? Shouldn't it be doing a dput or
> > at least a d_free here?
>
> While it has been some months since I wrote this, and it was not
> intended to be a production patch, I don't believe it leaks dentries.
>
> The function is called on the path real_lookup() to i_op->lookup() to
> d_splice_alias(), and the original dentry allocated in real_lookup()
> is d_put() there if a non-NULL new dentry pointer is passed back up
> via d_splice_alias() and i_op->lookup().

Yes, you are right. I've always found the dcache code a little
confusing. Your patch looks good to me.
--
David Kleikamp
IBM Linux Technology Center