On Mon, Dec 11, 2023 at 02:03:05PM -0800, Darrick J. Wong wrote:
> On Sat, Dec 09, 2023 at 08:21:07PM +0800, Long Li wrote:
> > During growfs, if new ag in memory has been initialized, however
> > sb_agcount has not been updated, if an error occurs at this time it
> > will cause perag leaks as follows, these new AGs will not been freed
> > during umount , because of these new AGs are not visible(that is
> > included in mp->m_sb.sb_agcount).
> >
> > unreferenced object 0xffff88810be40200 (size 512):
> > comm "xfs_growfs", pid 857, jiffies 4294909093
> > hex dump (first 32 bytes):
> > 00 c0 c1 05 81 88 ff ff 04 00 00 00 00 00 00 00 ................
> > 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > backtrace (crc 381741e2):
> > [<ffffffff8191aef6>] __kmalloc+0x386/0x4f0
> > [<ffffffff82553e65>] kmem_alloc+0xb5/0x2f0
> > [<ffffffff8238dac5>] xfs_initialize_perag+0xc5/0x810
> > [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
> > [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
> > [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
> > [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
> > [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a
> > unreferenced object 0xffff88810be40800 (size 512):
> > comm "xfs_growfs", pid 857, jiffies 4294909093
> > hex dump (first 32 bytes):
> > 20 00 00 00 00 00 00 00 57 ef be dc 00 00 00 00 .......W.......
> > 10 08 e4 0b 81 88 ff ff 10 08 e4 0b 81 88 ff ff ................
> > backtrace (crc bde50e2d):
> > [<ffffffff8191b43a>] __kmalloc_node+0x3da/0x540
> > [<ffffffff81814489>] kvmalloc_node+0x99/0x160
> > [<ffffffff8286acff>] bucket_table_alloc.isra.0+0x5f/0x400
> > [<ffffffff8286bdc5>] rhashtable_init+0x405/0x760
> > [<ffffffff8238dda3>] xfs_initialize_perag+0x3a3/0x810
> > [<ffffffff824f679c>] xfs_growfs_data+0x9bc/0xbc0
> > [<ffffffff8250b90e>] xfs_file_ioctl+0x5fe/0x14d0
> > [<ffffffff81aa5194>] __x64_sys_ioctl+0x144/0x1c0
> > [<ffffffff83c3d81f>] do_syscall_64+0x3f/0xe0
> > [<ffffffff83e00087>] entry_SYSCALL_64_after_hwframe+0x62/0x6a
> >
> > Now, the logic for freeing perag in xfs_free_perag() and
> > xfs_initialize_perag() error handling is essentially the same. Factor
> > out xfs_free_perag_range() from xfs_free_perag(), used for freeing
> > unused perag within a specified range, inclued when growfs fails.
> >
> > Signed-off-by: Long Li <leo.lilong@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_ag.c | 35 ++++++++++++++++++++---------------
> > fs/xfs/libxfs/xfs_ag.h | 2 ++
> > fs/xfs/xfs_fsops.c | 5 ++++-
> > 3 files changed, 26 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> > index 11ed048c350c..edec03ab09aa 100644
> > --- a/fs/xfs/libxfs/xfs_ag.c
> > +++ b/fs/xfs/libxfs/xfs_ag.c
> > @@ -245,16 +245,20 @@ __xfs_free_perag(
> > }
> >
> > /*
> > - * Free up the per-ag resources associated with the mount structure.
> > + * Free per-ag within the specified range, if agno is not found in the
> > + * radix tree, then it means that agno and subsequent AGs have not been
> > + * initialized.
> > */
> > void
> > -xfs_free_perag(
> > - struct xfs_mount *mp)
> > +xfs_free_perag_range(
> > + xfs_mount_t *mp,
>
> Please stop reverting the codebase's use of typedefs for struct
> pointers.
>
> > + xfs_agnumber_t agstart,
> > + xfs_agnumber_t agend)
>
> This is also ^^ unnecessary indentation.
>
> > {
> > - struct xfs_perag *pag;
> > xfs_agnumber_t agno;
> > + struct xfs_perag *pag;
>
> ...and unnecessary rearranging of variables...
I ignored these minor issues, thanks for pointing them out, and it will be changed
in the next version.
Thanks,
Long Li
