On Fri, Jun 28, 2019 at 07:00:27AM +0100, Al Viro wrote: > 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"? 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... > If so, > that should be > > parent = dget_parent(dentry); > if (parent != dentry) > dir = igrab(d_inode(parent)); > dput(parent); 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 david@xxxxxxxxxxxxx
