| Message ID | 20250109005402.GH1387004@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Queued |
| Headers | show |
| Series | xfs: lock dquot buffer before detaching dquot from b_li_list | expand |
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?
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
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.
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); } }