On Sat, Nov 11, 2023 at 8:50 PM Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > > On Sat, Nov 11, 2023 at 08:31:11PM +0200, Amir Goldstein wrote: > > > in ovl_lookup(), and in case we have d_splice_alias() return a non-NULL > > > dentry we can simply copy it there. Sure, somebody might race with > > > us, pick dentry from hash and call ->d_revalidate() before we notice that > > > DCACHE_OP_REVALIDATE could be cleaned. So what? That call of ->d_revalidate() > > > will find nothing to do and return 1. Which is the effect of having > > > DCACHE_OP_REVALIDATE cleared, except for pointless method call. Anyone > > > who finds that dentry after the flag is cleared will skip the call. > > > IOW, that race is harmless. > > > > Just a minute. > > Do you know that ovl_obtain_alias() is *only* used to obtain a disconnected > > non-dir overlayfs dentry? > > D'oh... > > > I think that makes all the analysis regarding race with d_splice_alias() > > moot. Right? > > Right you are. > > > Do DCACHE_OP_*REVALIDATE even matter for a disconnected > > non-dir dentry? > > As long as nothing picks it via d_find_any_alias() and moves it somewhere > manually. The former might happen, the latter, AFAICS, doesn't - nothing > like d_move() anywhere in sight... > > > You are missing that the OVL_E_UPPER_ALIAS flag is a property of > > the overlay dentry, not a property of the inode. > > > > N lower hardlinks, the first copy up created an upper inode > > all the rest of the N upper aliases to that upper inode are > > created lazily. > > > > However, for obvious reasons, OVL_E_UPPER_ALIAS is not > > well defined for a disconnected overlay dentry. > > There should not be any code (I hope) that cares about > > OVL_E_UPPER_ALIAS for a disconnected overlay dentry, > > so I *think* ovl_dentry_set_upper_alias() in this code is moot. > > > > I need to look closer to verify, but please confirm my assumption > > regarding the irrelevance of DCACHE_OP_*REVALIDATE for a > > disconnected non-dir dentry. > > Correct; we only care if it gets reconnected to the main tree. > The fact that it's only for non-directories simplifies life a lot > there. Sorry, got confused by the work you do with ->d_flags > and hadn't stopped to ask whether it's needed in the first place > in there. > > OK, so... are there any reasons why simply calling d_obtain_alias() > wouldn't do the right thing these days? None that I can think of. I will try it out and run the tests to see if I have missed something. Thanks, Amir.
