| Message ID | 20240109021734.GB722975@frogsfrogsfrogs (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: fix backwards logic in xfs_bmap_alloc_account | expand |
On Mon, Jan 08, 2024 at 06:17:34PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > We're only allocating from the realtime device if the inode is marked > for realtime and we're /not/ allocating into the attr fork. Hmm, interesting how this survived all my rtalloc tests. How did you find this? Reviewed-by: Christoph Hellwig <hch@lst.de>
On Mon, Jan 08, 2024 at 08:36:19PM -0800, Christoph Hellwig wrote: > On Mon, Jan 08, 2024 at 06:17:34PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > We're only allocating from the realtime device if the inode is marked > > for realtime and we're /not/ allocating into the attr fork. > > Hmm, interesting how this survived all my rtalloc tests. How did you > find this? I actually found it while reabasing the rt reflink patchset atop for-next, because you unified the bmap allocator accounting functions instead of copy-pasting them like I did. Then I tried generic/476 and it blew up the first time it encountered a reflink file. > Reviewed-by: Christoph Hellwig <hch@lst.de> Thanks! Chandan, can we get this merged? --D
On Mon, Jan 08, 2024 at 09:08:48 PM -0800, Darrick J. Wong wrote: > On Mon, Jan 08, 2024 at 08:36:19PM -0800, Christoph Hellwig wrote: >> On Mon, Jan 08, 2024 at 06:17:34PM -0800, Darrick J. Wong wrote: >> > From: Darrick J. Wong <djwong@kernel.org> >> > >> > We're only allocating from the realtime device if the inode is marked >> > for realtime and we're /not/ allocating into the attr fork. >> >> Hmm, interesting how this survived all my rtalloc tests. How did you >> find this? > > I actually found it while reabasing the rt reflink patchset atop > for-next, because you unified the bmap allocator accounting functions > instead of copy-pasting them like I did. Then I tried generic/476 and > it blew up the first time it encountered a reflink file. > >> Reviewed-by: Christoph Hellwig <hch@lst.de> > > Thanks! Chandan, can we get this merged? I have picked up the patch and will update for-next soon. I will send the second pull request for v6.8-rc1 on coming Monday if everything goes well.
On Mon, Jan 08, 2024 at 06:17:34 PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@kernel.org> > > We're only allocating from the realtime device if the inode is marked > for realtime and we're /not/ allocating into the attr fork. > > Fixes: 8a3cf489410dd ("xfs: also use xfs_bmap_btalloc_accounting for RT allocations") The commit ID should be 58643460546d (https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?id=58643460546da1dc61593fc6fd78762798b4534f) right? If yes, I will fix it before pushing it for-next. > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > --- > fs/xfs/libxfs/xfs_bmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > index ed7e11697249e..e1f2e61cb308e 100644 > --- a/fs/xfs/libxfs/xfs_bmap.c > +++ b/fs/xfs/libxfs/xfs_bmap.c > @@ -3320,7 +3320,7 @@ xfs_bmap_alloc_account( > struct xfs_bmalloca *ap) > { > bool isrt = XFS_IS_REALTIME_INODE(ap->ip) && > - (ap->flags & XFS_BMAPI_ATTRFORK); > + !(ap->flags & XFS_BMAPI_ATTRFORK); > uint fld; > > if (ap->flags & XFS_BMAPI_COWFORK) {
On Wed, Jan 10, 2024 at 03:41:32PM +0530, Chandan Babu R wrote: > On Mon, Jan 08, 2024 at 06:17:34 PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <djwong@kernel.org> > > > > We're only allocating from the realtime device if the inode is marked > > for realtime and we're /not/ allocating into the attr fork. > > > > Fixes: 8a3cf489410dd ("xfs: also use xfs_bmap_btalloc_accounting for RT allocations") > > The commit ID should be 58643460546d > (https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/commit/?id=58643460546da1dc61593fc6fd78762798b4534f) > right? > > If yes, I will fix it before pushing it for-next. Yes. Apparently I ran git blame on the wrong branch. :( --D > > Signed-off-by: Darrick J. Wong <djwong@kernel.org> > > --- > > fs/xfs/libxfs/xfs_bmap.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > > index ed7e11697249e..e1f2e61cb308e 100644 > > --- a/fs/xfs/libxfs/xfs_bmap.c > > +++ b/fs/xfs/libxfs/xfs_bmap.c > > @@ -3320,7 +3320,7 @@ xfs_bmap_alloc_account( > > struct xfs_bmalloca *ap) > > { > > bool isrt = XFS_IS_REALTIME_INODE(ap->ip) && > > - (ap->flags & XFS_BMAPI_ATTRFORK); > > + !(ap->flags & XFS_BMAPI_ATTRFORK); > > uint fld; > > > > if (ap->flags & XFS_BMAPI_COWFORK) { > > -- > Chandan >
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c index ed7e11697249e..e1f2e61cb308e 100644 --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -3320,7 +3320,7 @@ xfs_bmap_alloc_account( struct xfs_bmalloca *ap) { bool isrt = XFS_IS_REALTIME_INODE(ap->ip) && - (ap->flags & XFS_BMAPI_ATTRFORK); + !(ap->flags & XFS_BMAPI_ATTRFORK); uint fld; if (ap->flags & XFS_BMAPI_COWFORK) {