[PATCH 4/4] fix biosize option

[PATCH 4/4] fix biosize option

[Christoph Hellwig]

iosizelog shouldn't be the same as iosize but the logarithm of it.  Then
again the current biosize option doesn't make much sense to me as it
doesn't set the preferred I/O size as mentioned in the comment next to
it but rather the allocation size and thus is identical to the allocsize
option (except for the missing logarithm).  It's also not documented in
Documentation/filesystems/xfs.txt or the mount manpage.


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

Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_super.c	2008-05-25 20:59:04.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c	2008-05-25 20:59:38.000000000 +0200
@@ -252,7 +252,7 @@ xfs_parseargs(
 				return EINVAL;
 			}
 			iosize = simple_strtoul(value, &eov, 10);
-			iosizelog = (uint8_t) iosize;
+			iosizelog = ffs(iosize) - 1;
 		} else if (!strcmp(this_char, MNTOPT_ALLOCSIZE)) {
 			if (!value || !*value) {
 				cmn_err(CE_WARN,

Re: [PATCH 4/4] fix biosize option

[Christoph Hellwig]

ping?

On Sun, May 25, 2008 at 09:07:54PM +0200, Christoph Hellwig wrote:
> iosizelog shouldn't be the same as iosize but the logarithm of it.  Then
> again the current biosize option doesn't make much sense to me as it
> doesn't set the preferred I/O size as mentioned in the comment next to
> it but rather the allocation size and thus is identical to the allocsize
> option (except for the missing logarithm).  It's also not documented in
> Documentation/filesystems/xfs.txt or the mount manpage.
> 
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_super.c	2008-05-25 20:59:04.000000000 +0200
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c	2008-05-25 20:59:38.000000000 +0200
> @@ -252,7 +252,7 @@ xfs_parseargs(
>  				return EINVAL;
>  			}
>  			iosize = simple_strtoul(value, &eov, 10);
> -			iosizelog = (uint8_t) iosize;
> +			iosizelog = ffs(iosize) - 1;
>  		} else if (!strcmp(this_char, MNTOPT_ALLOCSIZE)) {
>  			if (!value || !*value) {
>  				cmn_err(CE_WARN,
---end quoted text---

Re: [PATCH 4/4] fix biosize option

[Dave Chinner]

On Sun, May 25, 2008 at 09:07:54PM +0200, Christoph Hellwig wrote:
> iosizelog shouldn't be the same as iosize but the logarithm of it.  Then
> again the current biosize option doesn't make much sense to me as it
> doesn't set the preferred I/O size as mentioned in the comment next to

That's a hold-over from irix - biosize set the basic chunk size the
cache used for mapping writes, so was effectively the size of write
to use to avoid RMW cycles as well as the unit of delayed
allocation. 

> it but rather the allocation size and thus is identical to the allocsize
> option (except for the missing logarithm).  It's also not documented in
> Documentation/filesystems/xfs.txt or the mount manpage.

So when it was first added (probably for compatibility) the
allocation part was noticed and not the rest. Maybe we should
hook it up to xfs_preferred_iosize() rather than allocation now
that it exists...

Apart from that, yes it should be the logarithm, so ACK for this
minor mod...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH 4/4] fix biosize option

[Christoph Hellwig]

On Thu, Jul 24, 2008 at 05:15:19PM +1000, Dave Chinner wrote:
> On Sun, May 25, 2008 at 09:07:54PM +0200, Christoph Hellwig wrote:
> > iosizelog shouldn't be the same as iosize but the logarithm of it.  Then
> > again the current biosize option doesn't make much sense to me as it
> > doesn't set the preferred I/O size as mentioned in the comment next to
> 
> That's a hold-over from irix - biosize set the basic chunk size the
> cache used for mapping writes, so was effectively the size of write
> to use to avoid RMW cycles as well as the unit of delayed
> allocation. 
> 
> > it but rather the allocation size and thus is identical to the allocsize
> > option (except for the missing logarithm).  It's also not documented in
> > Documentation/filesystems/xfs.txt or the mount manpage.
> 
> So when it was first added (probably for compatibility) the
> allocation part was noticed and not the rest. Maybe we should
> hook it up to xfs_preferred_iosize() rather than allocation now
> that it exists...

Or just stop making the option have any effect and just print a message
that it's ignored..

Re: [PATCH 4/4] fix biosize option

[Christoph Hellwig]

ping^2?

On Wed, Jul 23, 2008 at 10:05:20AM +0200, Christoph Hellwig wrote:
> ping?
> 
> On Sun, May 25, 2008 at 09:07:54PM +0200, Christoph Hellwig wrote:
> > iosizelog shouldn't be the same as iosize but the logarithm of it.  Then
> > again the current biosize option doesn't make much sense to me as it
> > doesn't set the preferred I/O size as mentioned in the comment next to
> > it but rather the allocation size and thus is identical to the allocsize
> > option (except for the missing logarithm).  It's also not documented in
> > Documentation/filesystems/xfs.txt or the mount manpage.
> > 
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > 
> > Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c
> > ===================================================================
> > --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_super.c	2008-05-25 20:59:04.000000000 +0200
> > +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_super.c	2008-05-25 20:59:38.000000000 +0200
> > @@ -252,7 +252,7 @@ xfs_parseargs(
> >  				return EINVAL;
> >  			}
> >  			iosize = simple_strtoul(value, &eov, 10);
> > -			iosizelog = (uint8_t) iosize;
> > +			iosizelog = ffs(iosize) - 1;
> >  		} else if (!strcmp(this_char, MNTOPT_ALLOCSIZE)) {
> >  			if (!value || !*value) {
> >  				cmn_err(CE_WARN,
> ---end quoted text---
---end quoted text---