On Tue, 1 Dec 2009, Sage Weil wrote: > On Tue, 1 Dec 2009, Andrew Morton wrote: > > Quite a VFS patch backlog here, some held over from 2.6.32: > > > > vfs-fix-vfs_rename_dir-for-fs_rename_does_d_move-filesystems.patch > > This problem will bite ceph without this patch. I could work around it, > but it would be nice to fix the real bug. Al was worried about nfs: > > > It's _probably_ OK now, but I'd really like to think about NFS > > behaviour. There are subtle traps in that area. > > http://marc.info/?l=linux-kernel&m=121666695629006&w=2 I had a look at the NFS rename code again, and it does have the look of of some legacy, especially the comments. I'll will submit a short series of cleanup patches in that area. But otherwise it looks perfectly OK, I could not see any traps, see below: - Filesystems without FS_RENAME_DOES_D_MOVE o target has external references dentry_unhash() doesn't unhash it, so it doesn't need to be rehashed o target doesn't have external references - rename succeeded the d_rehash() is basically a NOP, the following d_move() unhash it again - rename failed the d_rehash() is an optimization, saves a ->lookup() next time the target is accessed. This is unnecessary, we don't need to optimize this failure case - Filesystems with FS_RENAME_DOES_D_MOVE o target has external references - rename succeeded dentry_unhash() doesn't unhash the target, but d_move() will, it will also exchange names and parents of the source/target dentries. In this situation d_rehash will resurrect the target dentry under the old name, /proc/PID/fd/N symlink won't show it as "(deleted)" for example. Lookup of the old name will result in ESTALE, since d_revalidate() fails but the dentry can't be invalidated because of the external refs. - rename failed dentry_unhash() doens't unhash it, filesystem doesn't do d_move(), so everything is fine, no need to rehash o target doesn't have external refereces - rename succeeded again the target dentry is resurrected in the cache, but the next lookup will successfully invalidate it. But there's no need to rehash, it just slows down lookup - rename failed the d_rehash() is an optimization, saves a ->lookup() next time the target is accessed. This is unnecessary Does anyone see a fault in my analysis? Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
