On Wed, 2 Dec 2009, Miklos Szeredi wrote: > 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 Yes. > - 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. - Assuming d_revalidate() fails, yes. - If the name is stort (<32 chars or 40 chars, depending on arch), the name is copied, not swapped, so the target will get rehashed at the same name. > - 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 Again, only assuming d_revalidate() explicitly fails on renamed-over dentries. > - 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? Otherwise, this looks right to me. sage -- 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
