| Message ID | 20240226094936.2677493-12-kernel@pankajraghav.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | enable bs > ps in XFS | expand |
On Mon, Feb 26, 2024 at 10:49:34AM +0100, Pankaj Raghav (Samsung) wrote: > From: Dave Chinner <dchinner@redhat.com> > > For block size larger than page size, the unit of efficient IO is > the block size, not the page size. Leaving stat() to report > PAGE_SIZE as the block size causes test programs like fsx to issue > illegal ranges for operations that require block size alignment > (e.g. fallocate() insert range). Hence update the preferred IO size > to reflect the block size in this case. > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > dd2d535e3fb29d ("xfs: cleanup calculating the stat optimal I/O size")] > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> Something screwed up there, and you haven't put your own SOB on this. > --- > fs/xfs/xfs_iops.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index a0d77f5f512e..1b4edfad464f 100644 > --- a/fs/xfs/xfs_iops.c > +++ b/fs/xfs/xfs_iops.c > @@ -543,7 +543,7 @@ xfs_stat_blksize( > return 1U << mp->m_allocsize_log; > } > > - return PAGE_SIZE; > + return max_t(unsigned long, PAGE_SIZE, mp->m_sb.sb_blocksize); > } This function returns a uint32_t, same type as mp->m_sb.sb_blocksize. The comparision should use uint32_t casts, not unsigned long. ALso, this bears no resemblence to the original patch I wrote back in 2018. Please remove my SOB from it - you can state that "this change is based on a patch originally from Dave Chinner" to credit the history of it, but it's certainly not the patch I wrote 6 years ago and so my SOB does not belong on it. -Dave.
On Mon, Feb 26, 2024 at 11:44:16PM +1100, Dave Chinner wrote: > On Mon, Feb 26, 2024 at 10:49:34AM +0100, Pankaj Raghav (Samsung) wrote: > > From: Dave Chinner <dchinner@redhat.com> > > > > For block size larger than page size, the unit of efficient IO is > > the block size, not the page size. Leaving stat() to report > > PAGE_SIZE as the block size causes test programs like fsx to issue > > illegal ranges for operations that require block size alignment > > (e.g. fallocate() insert range). Hence update the preferred IO size > > to reflect the block size in this case. > > > > Signed-off-by: Dave Chinner <dchinner@redhat.com> > > dd2d535e3fb29d ("xfs: cleanup calculating the stat optimal I/O size")] > > Signed-off-by: Luis Chamberlain <mcgrof@kernel.org> > > Something screwed up there, and you haven't put your own SOB on > this. Oops. I will add it. > > > --- > > fs/xfs/xfs_iops.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > > index a0d77f5f512e..1b4edfad464f 100644 > > --- a/fs/xfs/xfs_iops.c > > +++ b/fs/xfs/xfs_iops.c > > @@ -543,7 +543,7 @@ xfs_stat_blksize( > > return 1U << mp->m_allocsize_log; > > } > > > > - return PAGE_SIZE; > > + return max_t(unsigned long, PAGE_SIZE, mp->m_sb.sb_blocksize); > > } > > This function returns a uint32_t, same type as > mp->m_sb.sb_blocksize. The comparision should use uint32_t casts, > not unsigned long. > Yeah. Something like this instead of using unsigned long: return max_t(uint32_t, PAGE_SIZE, mp->m_sb.sb_blocksize); > ALso, this bears no resemblence to the original patch I wrote back in > 2018. Please remove my SOB from it - you can state that "this change > is based on a patch originally from Dave Chinner" to credit the > history of it, but it's certainly not the patch I wrote 6 years ago > and so my SOB does not belong on it. Ok. > > -Dave. > -- > Dave Chinner > david@fromorbit.com
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index a0d77f5f512e..1b4edfad464f 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -543,7 +543,7 @@ xfs_stat_blksize( return 1U << mp->m_allocsize_log; } - return PAGE_SIZE; + return max_t(unsigned long, PAGE_SIZE, mp->m_sb.sb_blocksize); } STATIC int