On Thu, 28 Jul 2022, Trond Myklebust wrote: > On Tue, 2022-07-26 at 14:40 +1000, NeilBrown wrote: > > > > Hi Trond, > > did you have a change to look at this patch? > > I did take a look, and the patch does seem acceptable to me as it > stands. Thanks. > > However without the full context of the other patches you're writing, > I'm not able to ascertain whether or not a better approach than > overloading the meaning of DCACHE_DONTCACHE might exist. I'm wondering > if perhaps co-opting struct nfs_unlinkdata and/or creating something > similar for the non-sillyrename case might not work just as well? The significant part of the context is that i_rwsem will no longer provide exclusion between non-cached lookup and unlink, so the d_unhash() trick won't work any more. Using d_fsdata somehow would be a better solution providing we can make it work. I've looked again through the use of d_fsdata and I see now that an nfs_unlinkdata is only stored there when DCACHE_NFS_RENAMED is set, and in that case we won't be called to unlink. It is sometimes set to a "devname", but that is only for directories (I think) and only relevant while the dentry IS_ROOT() - if it is being unlinked, it cannot be IS_ROOT(). So d_fsdata can only be NULL at the time of interest. So I can set it to some other value ((void*)1 ??) to block d_revalidate while the unlink is progressing. I also noticed that I need to make the same change in nfs_rename() when the target exists. I'll let you know when I have something suitable along those lines. Thanks, NeilBrown
