dget_parent() misuse in xfs_filestream_get_parent()

	dget_parent() never returns NULL.  So this

        parent = dget_parent(dentry);
        if (!parent)
                goto out_dput;

        dir = igrab(d_inode(parent));
        dput(parent);

out_dput:

is obviously fishy. What is that code trying to do? Is that "dentry might be a root of disconnected tree, in which case we want xfs_filestream_get_parent() to return NULL"? If so, that should be

        parent = dget_parent(dentry);
        if (parent != dentry)
		dir = igrab(d_inode(parent));
        dput(parent);

If we want dentry itself, that dir = igrab(...) should be unconditional. I'm not familiar enough with that code, and I'd rather not go into YAMassiveSideRTFS, so...

We want the parent inode of the current file inode if it is in memory. We don't care about the parent dentry that is returned as such, it's just the mechanism for finding the directory inode. The directory inode is what holds the allocation policy for all files in that directory, and that's what we need here.

If there is no parent directory inode in memory, then we'll just use the default allocator behaviour rather than the context specific one we get from the directory inode...

Seems reasonable. This code is largely legacy functionality, the allocator was really a specific workload policy that was never widely deployed and so the combination of disconnected dentries and this allocator have probably never been seen in real life...

Cheers,

Dave.

Dave Chinner
[email protected]