[PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

We have to lock the buffer before we can delete the dquot log item from
the buffer's log item list.

Cc: <stable@vger.kernel.org> # v6.13-rc3
Fixes: acc8f8628c3737 ("xfs: attach dquot buffer to dquot log item buffer")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 fs/xfs/xfs_dquot.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/xfs_dquot.c b/fs/xfs/xfs_dquot.c
index 576b7755b1f1fc..84b69f686ba82e 100644
--- a/fs/xfs/xfs_dquot.c
+++ b/fs/xfs/xfs_dquot.c
@@ -87,8 +87,9 @@ xfs_dquot_detach_buf(
 	}
 	spin_unlock(&qlip->qli_lock);
 	if (bp) {
+		xfs_buf_lock(bp);
 		list_del_init(&qlip->qli_item.li_bio_list);
-		xfs_buf_rele(bp);
+		xfs_buf_relse(bp);
 	}
 }
 

Re: [PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Christoph Hellwig]

On Wed, Jan 08, 2025 at 04:54:02PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> We have to lock the buffer before we can delete the dquot log item from
> the buffer's log item list.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

I did look a bit over how the inode items handles the equivalent
functionality, and AFAICS there is no direct one.  xfs_qm_dquot_isolate
is for shrinking the dquot LRU, which is handled through the VFS
for inodes.  xfs_qm_dqpurge tries to write back dirty dquots, which
I thought is dead code as all dirty dquots should have log
items and thus be handled through the log and AIL, but it seems like
xfs_qm_quotacheck_dqadjust dirties dquots without logging them.
So we'll need that for now, but I wonder if we should convert this
last bit of meatada to also go through our normal log mechanism
eventually?

Re: [PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Darrick J. Wong]

On Wed, Jan 08, 2025 at 10:08:05PM -0800, Christoph Hellwig wrote:
> On Wed, Jan 08, 2025 at 04:54:02PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > We have to lock the buffer before we can delete the dquot log item from
> > the buffer's log item list.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> I did look a bit over how the inode items handles the equivalent
> functionality, and AFAICS there is no direct one.  xfs_qm_dquot_isolate
> is for shrinking the dquot LRU, which is handled through the VFS
> for inodes.  xfs_qm_dqpurge tries to write back dirty dquots, which
> I thought is dead code as all dirty dquots should have log
> items and thus be handled through the log and AIL, but it seems like
> xfs_qm_quotacheck_dqadjust dirties dquots without logging them.
> So we'll need that for now, but I wonder if we should convert this
> last bit of meatada to also go through our normal log mechanism
> eventually?

What if we replace it with the one in scrub/repair_quotacheck.c? :)

--D

Re: [PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Christoph Hellwig]

On Wed, Jan 08, 2025 at 11:17:18PM -0800, Darrick J. Wong wrote:
> > I did look a bit over how the inode items handles the equivalent
> > functionality, and AFAICS there is no direct one.  xfs_qm_dquot_isolate
> > is for shrinking the dquot LRU, which is handled through the VFS
> > for inodes.  xfs_qm_dqpurge tries to write back dirty dquots, which
> > I thought is dead code as all dirty dquots should have log
> > items and thus be handled through the log and AIL, but it seems like
> > xfs_qm_quotacheck_dqadjust dirties dquots without logging them.
> > So we'll need that for now, but I wonder if we should convert this
> > last bit of meatada to also go through our normal log mechanism
> > eventually?
> 
> What if we replace it with the one in scrub/repair_quotacheck.c? :)

Yes, that's a much better implementation.  It will probably be a while
until we can get everyone convinced to actually build the online repair
code into their kernels, though and I'm not sure if it's feasibable to
have a subset just for quotacheck.

Re: [PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Carlos Maiolino]

On Wed, 08 Jan 2025 16:54:02 -0800, Darrick J. Wong wrote:
> We have to lock the buffer before we can delete the dquot log item from
> the buffer's log item list.
> 
> 

Applied to next-rc, thanks!

[1/1] xfs: lock dquot buffer before detaching dquot from b_li_list
      commit: 4e7dfb45fe08b2b54d7fe2499fab0eeaa42004ad

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>

Re: [PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Darrick J. Wong]

On Mon, Jan 13, 2025 at 01:33:07PM +0100, Carlos Maiolino wrote:
> On Wed, 08 Jan 2025 16:54:02 -0800, Darrick J. Wong wrote:
> > We have to lock the buffer before we can delete the dquot log item from
> > the buffer's log item list.
> > 
> > 
> 
> Applied to next-rc, thanks!
> 
> [1/1] xfs: lock dquot buffer before detaching dquot from b_li_list
>       commit: 4e7dfb45fe08b2b54d7fe2499fab0eeaa42004ad

Um... you already pushed this to Linus, why is it queued again?
(albeit with the same commit id so I don't think it matters)

--D

> Best regards,
> -- 
> Carlos Maiolino <cem@kernel.org>
> 
> 

Re: [PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Carlos Maiolino]

On Mon, Jan 13, 2025 at 08:21:34AM -0800, Darrick J. Wong wrote:
> On Mon, Jan 13, 2025 at 01:33:07PM +0100, Carlos Maiolino wrote:
> > On Wed, 08 Jan 2025 16:54:02 -0800, Darrick J. Wong wrote:
> > > We have to lock the buffer before we can delete the dquot log item from
> > > the buffer's log item list.
> > > 
> > > 
> > 
> > Applied to next-rc, thanks!
> > 
> > [1/1] xfs: lock dquot buffer before detaching dquot from b_li_list
> >       commit: 4e7dfb45fe08b2b54d7fe2499fab0eeaa42004ad
> 
> Um... you already pushed this to Linus, why is it queued again?
> (albeit with the same commit id so I don't think it matters)

I rushed to push this patch to -rc7 so I didn't end up sending a TY message to
it. It was still on my b4 stack when I sent the last TY batch.

Which reminds me I should update the message, as the TY is going after I push to
for-next, no to next-rc :).


> 
> --D
> 
> > Best regards,
> > -- 
> > Carlos Maiolino <cem@kernel.org>
> > 
> > 
> 

Re: [PATCH] xfs: lock dquot buffer before detaching dquot from b_li_list

[Darrick J. Wong]

On Tue, Jan 14, 2025 at 11:45:48AM +0100, Carlos Maiolino wrote:
> On Mon, Jan 13, 2025 at 08:21:34AM -0800, Darrick J. Wong wrote:
> > On Mon, Jan 13, 2025 at 01:33:07PM +0100, Carlos Maiolino wrote:
> > > On Wed, 08 Jan 2025 16:54:02 -0800, Darrick J. Wong wrote:
> > > > We have to lock the buffer before we can delete the dquot log item from
> > > > the buffer's log item list.
> > > > 
> > > > 
> > > 
> > > Applied to next-rc, thanks!
> > > 
> > > [1/1] xfs: lock dquot buffer before detaching dquot from b_li_list
> > >       commit: 4e7dfb45fe08b2b54d7fe2499fab0eeaa42004ad
> > 
> > Um... you already pushed this to Linus, why is it queued again?
> > (albeit with the same commit id so I don't think it matters)
> 
> I rushed to push this patch to -rc7 so I didn't end up sending a TY message to
> it. It was still on my b4 stack when I sent the last TY batch.
> 
> Which reminds me I should update the message, as the TY is going after I push to
> for-next, no to next-rc :).

Ah, ok.  Carry on! :)

--D

> 
> > 
> > --D
> > 
> > > Best regards,
> > > -- 
> > > Carlos Maiolino <cem@kernel.org>
> > > 
> > > 
> > 
>