lkml.org 
[lkml]   [2022]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH/RFC] VFS: support parallel updates in the one directory.
    On Thu, Feb 24, 2022 at 09:31:28AM -0700, Andreas Dilger wrote:
    > On Feb 23, 2022, at 22:57, NeilBrown <neilb@suse.de> wrote:
    > >
    > >
    > > I added this:
    > > --- a/fs/xfs/xfs_icache.c
    > > +++ b/fs/xfs/xfs_icache.c
    > > @@ -87,6 +87,7 @@ xfs_inode_alloc(
    > > /* VFS doesn't initialise i_mode or i_state! */
    > > VFS_I(ip)->i_mode = 0;
    > > VFS_I(ip)->i_state = 0;
    > > + VFS_I(ip)->i_flags |= S_PAR_UPDATE;
    > > mapping_set_large_folios(VFS_I(ip)->i_mapping);
    > >
    > > XFS_STATS_INC(mp, vn_active);
    > >
    > > and ran my highly sophisticated test in an XFS directory:
    > >
    > > for i in {1..70}; do ( for j in {1000..8000}; do touch $j; rm -f $j ; done ) & done

    I think you want something faster here, like ln to hardlink an existing
    file into the directory.

    > > This doesn't crash - which is a good sign.
    > > While that was going I tried
    > > while : ; do ls -l ; done
    > >
    > > it sometimes reports garbage for the stat info:
    > >
    > > total 0
    > > -????????? ? ? ? ? ? 1749
    > > -????????? ? ? ? ? ? 1764
    > > -????????? ? ? ? ? ? 1765
    > > -rw-r--r-- 1 root root 0 Feb 24 16:47 1768
    > > -rw-r--r-- 1 root root 0 Feb 24 16:47 1770
    > > -rw-r--r-- 1 root root 0 Feb 24 16:47 1772
    > > ....
    > >
    > > I *think* that is bad - probably the "garbage" that you referred to?
    > >
    > > Obviously I gets lots of
    > > ls: cannot access '1764': No such file or directory
    > > ls: cannot access '1749': No such file or directory
    > > ls: cannot access '1780': No such file or directory
    > > ls: cannot access '1765': No such file or directory
    > >
    > > but that is normal and expected when you are creating and deleting
    > > files during the ls.
    >
    > The "ls -l" output with "???" is exactly the case where the filename is
    > in readdir() but stat() on a file fails due to an unavoidable userspace
    > race between the two syscalls and the concurrent unlink(). This is
    > probably visible even without the concurrent dirops patch.
    >
    > The list of affected filenames even correlates with the reported errors:
    > 1764, 1765, 1769
    >
    > It looks like everything is working as expected.

    Here, yes.

    A problem that I saw a week or two ago with online fsck is that an evil
    thread repeatedly link()ing and unlink()ing a file into an otherwise
    empty directory while racing a thread calling readdir() in a loop will
    eventually trigger a corruption report on the directory namecheck
    because the loop in xfs_dir2_sf_getdents that uses sfp->count as a loop
    counter will race with the unlink decrementing sfp->count and run off
    the end of the inline directory data buffer.

    https://elixir.bootlin.com/linux/latest/source/fs/xfs/xfs_dir2_readdir.c#L121

    The solution in that case was a forgotten acquisition of the directory
    IOLOCK, but I don't see why the same principle wouldn't apply here.
    It's probably not so hard to fix it (rewrite readdir to take the ILOCK
    once, format the dirents to a buffer until it's full, save cursor, drop
    ILOCK, copy buffer to userspace) but it's not as easy as setting
    PAR_UPDATE.

    (I am also not a fan of "PAR_UPDATE", since 'par' is already an English
    word that doesn't mean 'parallel'.)

    --D

    >
    > Cheers, Andreas
    >

    \
     
     \ /
      Last update: 2022-02-25 00:39    [W:2.564 / U:0.340 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site