On Wed, Nov 22, 2023 at 10:36:57PM -0800, Christoph Hellwig wrote:
> On Wed, Nov 22, 2023 at 03:06:59PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@xxxxxxxxxx>
> >
> > In the kernel, commit 8ebbf262d4684 ("xfs: don't block in busy flushing
> > when freeing extents") changed the allocator behavior such that AGFL
> > fixing can return -EAGAIN in response to detection of a deadlock with
> > the transaction busy extent list. If this happens, we're supposed to
> > requeue the EFI so that we can roll the transaction and try the item
> > again.
> >
> > If a requeue happens, we should not free the xefi pointer in
> > xfs_extent_free_finish_item or else the retry will walk off a dangling
> > pointer. There is no extent busy list in userspace so this should
> > never happen, but let's fix the logic bomb anyway.
> >
> > We should have ported kernel commit 0853b5de42b47 ("xfs: allow extent
> > free intents to be retried") to userspace, but neither Carlos nor I
> > noticed this fine detail. :(
>
> It might be time to move this code into shared files?
I think Chandan started looking into what it would take to port the log
code from the kernel into userspace. Then xfs_trans_commit in userspace
could actually write transactions to the log and write them back
atomically; and xfs_repair could finally lose the -L switch.
> Btw, I still keep getting annoyed a bit about minor difference
> in some of the libxfs file due to header inclusion. Maybe we also
> need to be able to automate this more and require libxfs to only
> include libxfs headers and xfs.h?
<shrug> Ages ago Dave had a sketch to make xfs_{log,mount,inode}.h pull
in the relevant headers so that all the other source files only had to
#include at most three files.
I wish the kernel had precompiled headers, then it would make sense to
have one xfs.h file that pulled in the world and took advantage of
caching.
--D
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
>
