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

Greg Banks
2005-10-07T03:54:37Z

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().

>From the 2.6.5-based tree this patch was developed in:

static struct dentry * real_lookup(struct dentry * parent, struct qstr *
name, struct nameidata *nd)
{
...
struct dentry * dentry = d_alloc(parent, name);
result = ERR_PTR(-ENOMEM);
if (dentry) {
result = dir->i_op->lookup(dir, dentry, nd);
if (result)
dput(dentry);
else
result = dentry;
}
...
}

STATIC struct dentry *
linvfs_lookup(
struct inode *dir,
struct dentry *dentry,
struct nameidata *nd)
{
...
VOP_LOOKUP(vp, dentry, &cvp, 0, NULL, NULL, error);
...
return d_splice_alias(LINVFS_GET_IP(cvp), dentry);
}

Had this been a production patch I might have mentioned this
subtlety in the comments ;-)

Greg.
--
Greg Banks, R&D Software Engineer, SGI Australian Software Group.
I don't speak for SGI.