Hi Al, Thank you for the quick review. On 15-01-18 23:36, Al Viro wrote:
* what hch said about mount options. Get rid of that before the thing goes into mainline.
Ack.
* all "inode = GET_F_DENTRY(f)->d_inode;" are complete BS; it's called file_inode(f). Kill GET_F_DENTRY() off and be done with that.
Ok, will fix.
* your ->rename() can race with ->get_link(). Look at the place where the former reassigns ->path and frees the old value and think what happens if the latter is called just prior to that kfree(). * the same goes for sf_inode_revalidate() vs. rename(). * just what happens to ->path of inode when e.g. its grandparent directory is renamed?
Can I summarize all 3 above with: Caching the path is a bad idea and instead the code should always look up the name from e.g. file->f_path.dentry ?
* AFAICS, you consider all negative dentries invalid. Why do you even hash them, then?
I'm afraid I'm not entirely following you. Note I've no experience with fs code prior to this. Also I'm not the original author of this code, this code started as part of the out-of-tree kernel modules used by the VirtualBox guest-additions. I've been working on cleaning these modules up and then mainlinging them (there are 3 of them, this is the last). Note I believe that this code is based on the fs/hostfs code. Can you reword your question keeping my lack of experience wrt fs code in mind ? Regards, Hans
