On Wed, Mar 11, 2020 at 05:06:16PM +0000, Rantala, Tommi T. (Nokia - FI/Espoo) wrote: > On Wed, 2020-03-11 at 09:14 +1100, Dave Chinner wrote: > > On Tue, Mar 10, 2020 at 08:45:58AM +0000, Rantala, Tommi T. (Nokia - > > FI/Espoo) wrote: > > > Hello, > > > > > > One of my GitLab CI jobs stopped working after upgrading server > > > 5.4.18- > > > 100.fc30.x86_64 -> 5.5.7-100.fc30.x86_64. > > > (tested 5.5.8-100.fc30.x86_64 too, no change) > > > The server is fedora30 with XFS rootfs. > > > The problem reproduces always, and takes only couple minutes to run. > > > > > > The CI job fails in the beginning when doing "git clean" in docker > > > container, and failing to rmdir some directory: > > > "warning: failed to remove > > > .vendor/pkg/mod/golang.org/x/net@v0.0.0-20200114155413-6afb5195e5aa/in > > > tern > > > al/socket: Directory not empty" > > > > > > Quick google search finds some other people reporting similar problems > > > with 5.5.0: > > > https://gitlab.com/gitlab-org/gitlab-runner/issues/3185 > > > > Which appears to be caused by multiple gitlab processes modifying > > the directory at the same time. i.e. something is adding an entry to > > the directory at the same time something is trying to rm -rf it. > > That's a race condition, and would lead to the exact symptoms you > > see here, depending on where in the directory the new entry is > > added. > > OK traced "execve" with strace too, and it shows that it's "git clean > -ffdx" command (single process) that is being executed in the container, > which is doing the cleanup. > > Tested with 5.6-rc5, it's failing the same way. > > Spent some time to bisect this, and the problem is introduced by this: > > commit 263dde869bd09b1a709fd92118c7fff832773689 > Author: Christoph Hellwig <hch@xxxxxx> > Date: Fri Nov 8 15:05:32 2019 -0800 > > xfs: cleanup xfs_dir2_block_getdents > > Use an offset as the main means for iteration, and only do pointer > arithmetics to find the data/unused entries. > > Signed-off-by: Christoph Hellwig <hch@xxxxxx> > Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > Hmmmmm, looking at that commit, I think it slighty changed how the > "offset" is used compared to how the pointers were used. > > This cures the issue for me, tested (briefly) on top of 5.6-rc5. > Does it make sense...? > (Email client probably damages white-space, sorry, I'll send this properly > signed-off with git-send-email if it's OK) Thanks, this looks good. Although I wonder if the slightly different version below might be a little more elegant? diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c index 0d3b640cf1cc..871ec22c9aee 100644 --- a/fs/xfs/xfs_dir2_readdir.c +++ b/fs/xfs/xfs_dir2_readdir.c @@ -147,7 +147,7 @@ xfs_dir2_block_getdents( xfs_off_t cook; struct xfs_da_geometry *geo = args->geo; int lock_mode; - unsigned int offset; + unsigned int offset, next_offset; unsigned int end; /* @@ -173,9 +173,10 @@ xfs_dir2_block_getdents( * Loop over the data portion of the block. * Each object is a real entry (dep) or an unused one (dup). */ - offset = geo->data_entry_offset; end = xfs_dir3_data_end_offset(geo, bp->b_addr); - while (offset < end) { + for (offset = geo->data_entry_offset; + offset < end; + offset = next_offset) { struct xfs_dir2_data_unused *dup = bp->b_addr + offset; struct xfs_dir2_data_entry *dep = bp->b_addr + offset; uint8_t filetype; @@ -184,14 +185,15 @@ xfs_dir2_block_getdents( * Unused, skip it. */ if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) { - offset += be16_to_cpu(dup->length); + next_offset = offset + be16_to_cpu(dup->length); continue; } /* * Bump pointer for the next iteration. */ - offset += xfs_dir2_data_entsize(dp->i_mount, dep->namelen); + next_offset = offset + + xfs_dir2_data_entsize(dp->i_mount, dep->namelen); /* * The entry is before the desired starting point, skip it.
