RE: [RFC] weird stuff in exfat_lookup()

["Sungjong Seo" <sj1557.seo@samsung.com>]

Hello,
> Hello? This is Sungjong. Currently, I am unable to reply using my
> samsung.com email, so I am responding with my other Gmail account.
> 
> On 2/28/25 14:44, Namjae Jeon wrote:
> > On Fri, Feb 28, 2025 at 7:48 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >>
> >>         There's a really odd comment in that thing:
> >>                 /*
> >>                  * Unhashed alias is able to exist because of revalidate()
> >>                  * called by lookup_fast. You can easily make this status
> >>                  * by calling create and lookup concurrently
> >>                  * In such case, we reuse an alias instead of new dentry
> >>                  */
> >> and AFAICS it had been there since the original merge.  What I don't
> >> understand is how the hell could revalidate result in that -
> >> exfat_d_revalidate() always returns 1 on any positive dentry and alias
> is
> >> obviously positive (it has the same inode as the one we are about to
> use).
> >>
> >> It mentions a way to reproduce that, but I don't understand what does
> >> that refer to; could you give details?

I tested it on an arm64 device running on Android with kernel v5.15, v6.6.
And I can see unhashed-positive dentry with the following simple TC,
even if it is not an Android stacked fs environment.

* TestCase
- thread1: while(1) { mkdir(A) and rmdir(A) }
- thread2: while(1) { stat(A) }

This is due to the characteristics of exfat allowing negative dentry and
considering CI in d_revalidate. As mentioned in the comment,
unhashed-positive dentry can exist in a situation where mkdir
and stat are competing, and it can be dropped, but exfat_lookup has
been implemented to reuse(rehash) this dentry.

I hope the following callstack will help you understand.
Thank you.

     <CPU 0>                      <CPU 1>
do_mkdirat
  user_path_create
   *filename_create
      inode_lock(I_MUTEX_PARENT)
      __lookup_hash(LOOKUP_CREATE | LOOKUP_EXCL)
        dentry = d_alloc
        exfat_lookup
          lock(sbi->s_lock)
            no_exist_file
          unlock(sbi->s_lock)
          d_version_set
          d_splice_alias
            __d_add()   //hashed-negative dentry here
   *vfs_mkdir
      exfat_mkdir
        lock(sbi->s_lock)
        inc_iversion(dir)//iversion diff occured
			      vfs_statx
                                  filename_lookup
                                    path_lookupat
                                      lookup_last
                                        work_component
                                          lookup_fast
                                            dentry = __d_lookup
                                            d_revalidate(dentry) //because of iversion diff
                                            d_invalidate(dentry)
                                              __d_drop(dentry)
                                          lookup_slow
                                            inode_lock_shared(dir)
        inc_nlink(dir)
        inode = build_inode
        inc_iversion(inode)
        d_instantiate(dentry, inode) //* unhashed-positive dentry here
        unlock(sbi->s_lock)
   *done_path_create
       inode_unlock(I_MUTEX_PARENT)
	                                   __lookup_slow
                                              exfat_lookup
                                                lock(sbi->s_lock)
                                                inode = exfat_build_inode
                                                alias = d_find_alias(inode)
                                                d_unhashed(alias)? // * found unhashed positive dentry *
                                                d_rehash(alias)
...