[ANNOUNCE] xfsprogs for-next updated

[ANNOUNCE] xfsprogs for-next updated

[Carlos Maiolino]

[-- Attachment #1: Type: text/plain, Size: 2495 bytes --]

Hi folks,

The for-next branch of the xfsprogs repository at:

        git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git

has just been updated.

This update contains the initial libxfs sync to Linux 6.0 and should be turned
into -rc0 once it (hopefully) gets some testing (and no complains) for more people.

Please, if any questions, let me know.


The new head of the for-next branch is commit:

d3e53ab7c xfs: fix inode reservation space for removing transaction


New Commits:

Andrey Strachuk (1):
      [798d43495] xfs: removed useless condition in function xfs_attr_node_get

Dan Carpenter (1):
      [17df7eb7e] xfs: delete unnecessary NULL checks

Darrick J. Wong (6):
      [722e81c12] xfs: convert XFS_IFORK_PTR to a static inline helper
      [7ff5f1edf] xfs: make inode attribute forks a permanent part of struct xfs_inode
      [d4292c669] xfs: use XFS_IFORK_Q to determine the presence of an xattr fork
      [4f8415858] xfs: replace XFS_IFORK_Q with a proper predicate function
      [eae3e30d4] xfs: replace inode fork size macros with functions
      [e373f06a3] xfs: don't leak memory when attr fork loading fails

Dave Chinner (17):
      [ef78f876e] xfs: make last AG grow/shrink perag centric
      [37dc5890e] xfs: kill xfs_ialloc_pagi_init()
      [4330a9e00] xfs: pass perag to xfs_ialloc_read_agi()
      [87db57baf] xfs: kill xfs_alloc_pagf_init()
      [f9084bd95] xfs: pass perag to xfs_alloc_read_agf()
      [bc87af992] xfs: pass perag to xfs_read_agi
      [c1030eda4] xfs: pass perag to xfs_read_agf
      [1d202c10b] xfs: pass perag to xfs_alloc_get_freelist
      [9a73333d9] xfs: pass perag to xfs_alloc_put_freelist
      [75c01cccf] xfs: pass perag to xfs_alloc_read_agfl
      [83af0d13a] xfs: Pre-calculate per-AG agbno geometry
      [8aa34dc9b] xfs: Pre-calculate per-AG agino geometry
      [cee2d89ae] xfs: replace xfs_ag_block_count() with perag accesses
      [54f6b9e5e] xfs: make is_log_ag() a first class helper
      [0b2f4162b] xfs: rework xfs_buf_incore() API
      [69535dadf] xfs: track the iunlink list pointer in the xfs_inode
      [b9846dc9e] xfs: double link the unlinked inode list

Slark Xiao (1):
      [e4a32219d] xfs: Fix typo 'the the' in comment

Xiaole He (1):
      [ec36ecd2d] xfs: fix comment for start time value of inode with bigtime enabled

hexiaole (1):
      [d3e53ab7c] xfs: fix inode reservation space for removing transaction

-- 
Carlos Maiolino

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [ANNOUNCE] xfsprogs for-next updated

[Darrick J. Wong]

On Tue, Aug 30, 2022 at 01:52:20PM +0200, Carlos Maiolino wrote:
> Hi folks,
> 
> The for-next branch of the xfsprogs repository at:
> 
>         git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
> 
> has just been updated.
> 
> This update contains the initial libxfs sync to Linux 6.0 and should be turned
> into -rc0 once it (hopefully) gets some testing (and no complains) for more people.

Wooo, welcome, new maintainer! :)

> Please, if any questions, let me know.

For the repair deadlock fix[1], do you want me to pin the primary
superblock buffer to the xfs_mount like Dave suggested in [2]?

[1] https://lore.kernel.org/linux-xfs/166007921743.3294543.7334567013352169774.stgit@magnolia/
[2] https://lore.kernel.org/linux-xfs/20220811221541.GQ3600936@dread.disaster.area/

--D

> 
> The new head of the for-next branch is commit:
> 
> d3e53ab7c xfs: fix inode reservation space for removing transaction
> 
> 
> New Commits:
> 
> Andrey Strachuk (1):
>       [798d43495] xfs: removed useless condition in function xfs_attr_node_get
> 
> Dan Carpenter (1):
>       [17df7eb7e] xfs: delete unnecessary NULL checks
> 
> Darrick J. Wong (6):
>       [722e81c12] xfs: convert XFS_IFORK_PTR to a static inline helper
>       [7ff5f1edf] xfs: make inode attribute forks a permanent part of struct xfs_inode
>       [d4292c669] xfs: use XFS_IFORK_Q to determine the presence of an xattr fork
>       [4f8415858] xfs: replace XFS_IFORK_Q with a proper predicate function
>       [eae3e30d4] xfs: replace inode fork size macros with functions
>       [e373f06a3] xfs: don't leak memory when attr fork loading fails
> 
> Dave Chinner (17):
>       [ef78f876e] xfs: make last AG grow/shrink perag centric
>       [37dc5890e] xfs: kill xfs_ialloc_pagi_init()
>       [4330a9e00] xfs: pass perag to xfs_ialloc_read_agi()
>       [87db57baf] xfs: kill xfs_alloc_pagf_init()
>       [f9084bd95] xfs: pass perag to xfs_alloc_read_agf()
>       [bc87af992] xfs: pass perag to xfs_read_agi
>       [c1030eda4] xfs: pass perag to xfs_read_agf
>       [1d202c10b] xfs: pass perag to xfs_alloc_get_freelist
>       [9a73333d9] xfs: pass perag to xfs_alloc_put_freelist
>       [75c01cccf] xfs: pass perag to xfs_alloc_read_agfl
>       [83af0d13a] xfs: Pre-calculate per-AG agbno geometry
>       [8aa34dc9b] xfs: Pre-calculate per-AG agino geometry
>       [cee2d89ae] xfs: replace xfs_ag_block_count() with perag accesses
>       [54f6b9e5e] xfs: make is_log_ag() a first class helper
>       [0b2f4162b] xfs: rework xfs_buf_incore() API
>       [69535dadf] xfs: track the iunlink list pointer in the xfs_inode
>       [b9846dc9e] xfs: double link the unlinked inode list
> 
> Slark Xiao (1):
>       [e4a32219d] xfs: Fix typo 'the the' in comment
> 
> Xiaole He (1):
>       [ec36ecd2d] xfs: fix comment for start time value of inode with bigtime enabled
> 
> hexiaole (1):
>       [d3e53ab7c] xfs: fix inode reservation space for removing transaction
> 
> -- 
> Carlos Maiolino

Re: [ANNOUNCE] xfsprogs for-next updated

[Carlos Maiolino]

On Tue, Aug 30, 2022 at 08:12:17AM -0700, Darrick J. Wong wrote:
> On Tue, Aug 30, 2022 at 01:52:20PM +0200, Carlos Maiolino wrote:
> > Hi folks,
> >
> > The for-next branch of the xfsprogs repository at:
> >
> >         git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
> >
> > has just been updated.
> >
> > This update contains the initial libxfs sync to Linux 6.0 and should be turned
> > into -rc0 once it (hopefully) gets some testing (and no complains) for more people.
> 
> Wooo, welcome, new maintainer! :)

\o/

> 
> > Please, if any questions, let me know.
> 
> For the repair deadlock fix[1], do you want me to pin the primary
> superblock buffer to the xfs_mount like Dave suggested in [2]?

I'd rather have it pinned to the xfs_mount as it's often accessed, do you think
it is doable (you mentioned you've ran into many problems with that)?
I didn't have time to try to reproduce those deadlocks yet though.

> 
> [1] https://lore.kernel.org/linux-xfs/166007921743.3294543.7334567013352169774.stgit@magnolia/
> [2] https://lore.kernel.org/linux-xfs/20220811221541.GQ3600936@dread.disaster.area/

-- 
Carlos Maiolino

Re: [ANNOUNCE] xfsprogs for-next updated

[Darrick J. Wong]

On Wed, Aug 31, 2022 at 11:43:25AM +0200, Carlos Maiolino wrote:
> On Tue, Aug 30, 2022 at 08:12:17AM -0700, Darrick J. Wong wrote:
> > On Tue, Aug 30, 2022 at 01:52:20PM +0200, Carlos Maiolino wrote:
> > > Hi folks,
> > >
> > > The for-next branch of the xfsprogs repository at:
> > >
> > >         git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
> > >
> > > has just been updated.
> > >
> > > This update contains the initial libxfs sync to Linux 6.0 and should be turned
> > > into -rc0 once it (hopefully) gets some testing (and no complains) for more people.
> > 
> > Wooo, welcome, new maintainer! :)
> 
> \o/
> 
> > 
> > > Please, if any questions, let me know.
> > 
> > For the repair deadlock fix[1], do you want me to pin the primary
> > superblock buffer to the xfs_mount like Dave suggested in [2]?
> 
> I'd rather have it pinned to the xfs_mount as it's often accessed, do you think
> it is doable (you mentioned you've ran into many problems with that)?

Oh, the usual problems of adding a new interface...

1. Who is responsible for setting m_sb_bp?

Should libxfs_mount attach m_sb_bp?  Should individual programs decide
to do that if they require the functionality?  Should we instead have a
xfs_getsb function that returns m_sb_bp if set, or libxfs_getbufr's a
new buffer and tries to cmpxchg it with the pointer?

What about mkfs, which needs to libxfs_mount before it's even written
anything to disk?

2. Should it be a cached buffer so that any other program (e.g. xfs_db)
doing open-coded accesses of the superblock will get the same cached
buffer, or should it be uncached like the kernel?

If we decide on uncached, this will necessitate a full audit of xfsprogs
to catch open-coded calls to libxfs_getbuf for the primary super, or
else coherency problems will result.

If we decide on using a cached buffer and setting it in libxfs_mount,
then the part of xfs_repair that tears down the buffer cache and
reinitializes it with a different hash size will also have to learn to
brelse m_sb_bp before destroying the cache and re-assign it afterwards.
Alternately, I suppose it could learn to rehash itself.

This is a /lot/ to think about to solve one problem in one program.

> I didn't have time to try to reproduce those deadlocks yet though.

If you modify cache_node_get like this to make reclaim more aggressive:

diff --git a/libxfs/cache.c b/libxfs/cache.c
index 139c7c1b..b5e1bcf8 100644
--- a/libxfs/cache.c
+++ b/libxfs/cache.c
@@ -448,10 +448,10 @@ cache_node_get(
 		/*
 		 * not found, allocate a new entry
 		 */
+		priority = cache_shake(cache, priority, false);
 		node = cache_node_allocate(cache, key);
 		if (node)
 			break;
-		priority = cache_shake(cache, priority, false);
 		/*
 		 * We start at 0; if we free CACHE_SHAKE_COUNT we get
 		 * back the same priority, if not we get back priority+1.

It's trivially reproducible with xfs_repair (do not specify -n).

--D

> > 
> > [1] https://lore.kernel.org/linux-xfs/166007921743.3294543.7334567013352169774.stgit@magnolia/
> > [2] https://lore.kernel.org/linux-xfs/20220811221541.GQ3600936@dread.disaster.area/
> 
> -- 
> Carlos Maiolino

Re: [ANNOUNCE] xfsprogs for-next updated

[Carlos Maiolino]

> 
> Oh, the usual problems of adding a new interface...
> 
> 1. Who is responsible for setting m_sb_bp?
> 
> Should libxfs_mount attach m_sb_bp?  Should individual programs decide
> to do that if they require the functionality?  Should we instead have a
> xfs_getsb function that returns m_sb_bp if set, or libxfs_getbufr's a
> new buffer and tries to cmpxchg it with the pointer?
> 
> What about mkfs, which needs to libxfs_mount before it's even written
> anything to disk?
> 
> 2. Should it be a cached buffer so that any other program (e.g. xfs_db)
> doing open-coded accesses of the superblock will get the same cached
> buffer, or should it be uncached like the kernel?
> 
> If we decide on uncached, this will necessitate a full audit of xfsprogs
> to catch open-coded calls to libxfs_getbuf for the primary super, or
> else coherency problems will result.
> 
> If we decide on using a cached buffer and setting it in libxfs_mount,
> then the part of xfs_repair that tears down the buffer cache and
> reinitializes it with a different hash size will also have to learn to
> brelse m_sb_bp before destroying the cache and re-assign it afterwards.
> Alternately, I suppose it could learn to rehash itself.
> 
> This is a /lot/ to think about to solve one problem in one program.

Yeah, I do agree. Maybe it's better to go with your initial idea for 6.0, and we
postpone the SB buffer pinning to a later release avoiding people to hit this
issue ASAP?!


> 
> > I didn't have time to try to reproduce those deadlocks yet though.
> 
> If you modify cache_node_get like this to make reclaim more aggressive:
> 
> diff --git a/libxfs/cache.c b/libxfs/cache.c
> index 139c7c1b..b5e1bcf8 100644
> --- a/libxfs/cache.c
> +++ b/libxfs/cache.c
> @@ -448,10 +448,10 @@ cache_node_get(
>  		/*
>  		 * not found, allocate a new entry
>  		 */
> +		priority = cache_shake(cache, priority, false);
>  		node = cache_node_allocate(cache, key);
>  		if (node)
>  			break;
> -		priority = cache_shake(cache, priority, false);
>  		/*
>  		 * We start at 0; if we free CACHE_SHAKE_COUNT we get
>  		 * back the same priority, if not we get back priority+1.
> 
> It's trivially reproducible with xfs_repair (do not specify -n).

Thanks, I'm gonna try it :)

> 
> --D
> 
> > >
> > > [1] https://lore.kernel.org/linux-xfs/166007921743.3294543.7334567013352169774.stgit@magnolia/
> > > [2] https://lore.kernel.org/linux-xfs/20220811221541.GQ3600936@dread.disaster.area/
> >
> > --
> > Carlos Maiolino

-- 
Carlos Maiolino

[ANNOUNCE] xfsprogs for-next updated

[Carlos Maiolino]

Hi,

the xfsprogs for-next branch, located at:

https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/log/?h=for-next

has just been updated to match the master branch. This is just a follow-up sync
for the xfsprogs-6.0.0 release from yesterday, and all staging for the next
release will be done on top of that.

Cheers.

-- 
Carlos Maiolino