[PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[Eric Sandeen]

The inode diet patch in -mm unhooked xfs_preferred_iosize from the stat call:

--- a/fs/xfs/linux-2.6/xfs_vnode.c
+++ b/fs/xfs/linux-2.6/xfs_vnode.c
@@ -122,7 +122,6 @@ vn_revalidate_core(
        inode->i_blocks     = vap->va_nblocks;
        inode->i_mtime      = vap->va_mtime;
        inode->i_ctime      = vap->va_ctime;
-       inode->i_blksize    = vap->va_blocksize;
        if (vap->va_xflags & XFS_XFLAG_IMMUTABLE)

This in turn breaks the largeio mount option for xfs:

  largeio/nolargeio
        If "nolargeio" is specified, the optimal I/O reported in
        st_blksize by stat(2) will be as small as possible to allow user
        applications to avoid inefficient read/modify/write I/O.
        If "largeio" specified, a filesystem that has a "swidth" specified
        will return the "swidth" value (in bytes) in st_blksize. If the
        filesystem does not have a "swidth" specified but does specify
        an "allocsize" then "allocsize" (in bytes) will be returned
        instead.
        If neither of these two options are specified, then filesystem
        will behave as if "nolargeio" was specified.

and the (undocumented?) allocsize mount option as well.

For a filesystem like this with sunit/swidth specified,

meta-data=/dev/sda1              isize=512    agcount=32, agsize=7625840 blks
         =                       sectsz=512   attr=0
data     =                       bsize=4096   blocks=244026880, imaxpct=25
         =                       sunit=16     swidth=16 blks, unwritten=1
naming   =version 2              bsize=4096
log      =internal               bsize=4096   blocks=32768, version=1
         =                       sectsz=512   sunit=0 blks
realtime =none                   extsz=65536  blocks=0, rtextents=0

stat on a stock FC6 kernel w/ the largeio mount option returns only the page size:

[root@link-07]# mount -o largeio /dev/sda1 /mnt/test/
[root@link-07]# stat -c %o /mnt/test/foo
4096

with the following patch, it does what it should:

[root@link-07]# mount -o largeio /dev/sda1 /mnt/test/
[root@link-07]# stat -c %o /mnt/test/foo
65536

same goes for filesystems w/o sunit,swidth but with the allocsize mount option.

stock:
[root@link-07]# mount -o largeio,allocsize=32768 /dev/sda1 /mnt/test/
[root@link-07]# stat -c %o /mnt/test/foo
4096

w/ patch:
[root@link-07# mount -o largeio,allocsize=32768 /dev/sda1 /mnt/test/
[root@link-07]# stat -c %o /mnt/test/foo
32768

Signed-off-by: Eric Sandeen <sandeen@sandeen.net>

XFS guys, does this look ok?

Index: linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- linux-2.6.18.orig/fs/xfs/linux-2.6/xfs_iops.c
+++ linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
@@ -623,12 +623,16 @@ xfs_vn_getattr(
 {
 	struct inode	*inode = dentry->d_inode;
 	bhv_vnode_t	*vp = vn_from_inode(inode);
+	xfs_inode_t	*ip;
 	int		error = 0;
 
 	if (unlikely(vp->v_flag & VMODIFIED))
 		error = vn_revalidate(vp);
-	if (!error)
+	if (!error) {
 		generic_fillattr(inode, stat);
+		ip = xfs_vtoi(vp);
+		stat->blksize = xfs_preferred_iosize(ip->i_mount);
+	}
 	return -error;
 }
 

Re: [PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[David Chinner]

On Thu, Sep 21, 2006 at 05:33:24PM -0500, Eric Sandeen wrote:
> The inode diet patch in -mm unhooked xfs_preferred_iosize from the stat call:
....
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> 
> XFS guys, does this look ok?
>
> Index: linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
> ===================================================================
> --- linux-2.6.18.orig/fs/xfs/linux-2.6/xfs_iops.c
> +++ linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
> @@ -623,12 +623,16 @@ xfs_vn_getattr(
>  {
>  	struct inode	*inode = dentry->d_inode;
>  	bhv_vnode_t	*vp = vn_from_inode(inode);
> +	xfs_inode_t	*ip;
>  	int		error = 0;
>  
>  	if (unlikely(vp->v_flag & VMODIFIED))
>  		error = vn_revalidate(vp);
> -	if (!error)
> +	if (!error) {
>  		generic_fillattr(inode, stat);
> +		ip = xfs_vtoi(vp);
> +		stat->blksize = xfs_preferred_iosize(ip->i_mount);
> +	}
>  	return -error;
>  }

ACK. Looks good, Eric. Good catch.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: [PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[Timothy Shimmin]

Hi Eric,

Eric Sandeen wrote:
> The inode diet patch in -mm unhooked xfs_preferred_iosize from the stat call:
> 
> --- a/fs/xfs/linux-2.6/xfs_vnode.c
> +++ b/fs/xfs/linux-2.6/xfs_vnode.c
> @@ -122,7 +122,6 @@ vn_revalidate_core(
>         inode->i_blocks     = vap->va_nblocks;
>         inode->i_mtime      = vap->va_mtime;
>         inode->i_ctime      = vap->va_ctime;
> -       inode->i_blksize    = vap->va_blocksize;
>         if (vap->va_xflags & XFS_XFLAG_IMMUTABLE)
> 
> This in turn breaks the largeio mount option for xfs:
> 
>   largeio/nolargeio
>         If "nolargeio" is specified, the optimal I/O reported in
>         st_blksize by stat(2) will be as small as possible to allow user
>         applications to avoid inefficient read/modify/write I/O.
>         If "largeio" specified, a filesystem that has a "swidth" specified
>         will return the "swidth" value (in bytes) in st_blksize. If the
>         filesystem does not have a "swidth" specified but does specify
>         an "allocsize" then "allocsize" (in bytes) will be returned
>         instead.
>         If neither of these two options are specified, then filesystem
>         will behave as if "nolargeio" was specified.
> 
> and the (undocumented?) allocsize mount option as well.
> 
> For a filesystem like this with sunit/swidth specified,
> 
> meta-data=/dev/sda1              isize=512    agcount=32, agsize=7625840 blks
>          =                       sectsz=512   attr=0
> data     =                       bsize=4096   blocks=244026880, imaxpct=25
>          =                       sunit=16     swidth=16 blks, unwritten=1
> naming   =version 2              bsize=4096
> log      =internal               bsize=4096   blocks=32768, version=1
>          =                       sectsz=512   sunit=0 blks
> realtime =none                   extsz=65536  blocks=0, rtextents=0
> 
> stat on a stock FC6 kernel w/ the largeio mount option returns only the page size:
> 
> [root@link-07]# mount -o largeio /dev/sda1 /mnt/test/
> [root@link-07]# stat -c %o /mnt/test/foo
> 4096
> 
> with the following patch, it does what it should:
> 
> [root@link-07]# mount -o largeio /dev/sda1 /mnt/test/
> [root@link-07]# stat -c %o /mnt/test/foo
> 65536
> 
> same goes for filesystems w/o sunit,swidth but with the allocsize mount option.
> 
> stock:
> [root@link-07]# mount -o largeio,allocsize=32768 /dev/sda1 /mnt/test/
> [root@link-07]# stat -c %o /mnt/test/foo
> 4096
> 
> w/ patch:
> [root@link-07# mount -o largeio,allocsize=32768 /dev/sda1 /mnt/test/
> [root@link-07]# stat -c %o /mnt/test/foo
> 32768
> 
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> 
> XFS guys, does this look ok?
> 
> Index: linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
> ===================================================================
> --- linux-2.6.18.orig/fs/xfs/linux-2.6/xfs_iops.c
> +++ linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
> @@ -623,12 +623,16 @@ xfs_vn_getattr(
>  {
>  	struct inode	*inode = dentry->d_inode;
>  	bhv_vnode_t	*vp = vn_from_inode(inode);
> +	xfs_inode_t	*ip;
>  	int		error = 0;
>  
>  	if (unlikely(vp->v_flag & VMODIFIED))
>  		error = vn_revalidate(vp);
> -	if (!error)
> +	if (!error) {
>  		generic_fillattr(inode, stat);
> +		ip = xfs_vtoi(vp);
> +		stat->blksize = xfs_preferred_iosize(ip->i_mount);
> +	}
>  	return -error;
>  }
>  

Looked at your patch and then at our xfs code in the tree and
the existing code is different than what yours is based on.
I then noticed in the logs Nathan has actually made changes for this:

----------------------------
revision 1.254
date: 2006/07/17 10:46:05;  author: nathans;  state: Exp;  lines: +20 -5
modid: xfs-linux-melb:xfs-kern:26565a
Update XFS for i_blksize removal from generic inode structure
----------------------------
I even reviewed the change (and I don't remember it - getting old).

I looked at the mods scheduled for 2.6.19 and this is one of them.

So the fix for this is coming soon (and the fix is different from the
one above).

Cheers,
Tim.

Re: [PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[Eric Sandeen]

Timothy Shimmin wrote:

> Looked at your patch and then at our xfs code in the tree and
> the existing code is different than what yours is based on.
> I then noticed in the logs Nathan has actually made changes for this:
> 
> ----------------------------
> revision 1.254
> date: 2006/07/17 10:46:05;  author: nathans;  state: Exp;  lines: +20 -5
> modid: xfs-linux-melb:xfs-kern:26565a
> Update XFS for i_blksize removal from generic inode structure
> ----------------------------
> I even reviewed the change (and I don't remember it - getting old).
> 
> I looked at the mods scheduled for 2.6.19 and this is one of them.
> 
> So the fix for this is coming soon (and the fix is different from the
> one above).


Ah, ok, thanks guys.  Should have checked CVS I guess.

-Eric

Re: [PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[Andrew Morton]

On Fri, 22 Sep 2006 12:03:30 +1000
Timothy Shimmin <tes@sgi.com> wrote:

> Hi Eric,
> 
> Eric Sandeen wrote:
> > The inode diet patch in -mm unhooked xfs_preferred_iosize from the stat call:
> > 
> > --- a/fs/xfs/linux-2.6/xfs_vnode.c
> > +++ b/fs/xfs/linux-2.6/xfs_vnode.c
> > @@ -122,7 +122,6 @@ vn_revalidate_core(
> >         inode->i_blocks     = vap->va_nblocks;
> >         inode->i_mtime      = vap->va_mtime;
> >         inode->i_ctime      = vap->va_ctime;
> > -       inode->i_blksize    = vap->va_blocksize;
> >         if (vap->va_xflags & XFS_XFLAG_IMMUTABLE)
> > 
> > This in turn breaks the largeio mount option for xfs:
> > 
> >   largeio/nolargeio
> >         If "nolargeio" is specified, the optimal I/O reported in
> >         st_blksize by stat(2) will be as small as possible to allow user
> >         applications to avoid inefficient read/modify/write I/O.
> >         If "largeio" specified, a filesystem that has a "swidth" specified
> >         will return the "swidth" value (in bytes) in st_blksize. If the
> >         filesystem does not have a "swidth" specified but does specify
> >         an "allocsize" then "allocsize" (in bytes) will be returned
> >         instead.
> >         If neither of these two options are specified, then filesystem
> >         will behave as if "nolargeio" was specified.
> > 
> > and the (undocumented?) allocsize mount option as well.
> > 
> > For a filesystem like this with sunit/swidth specified,
> > 
> > meta-data=/dev/sda1              isize=512    agcount=32, agsize=7625840 blks
> >          =                       sectsz=512   attr=0
> > data     =                       bsize=4096   blocks=244026880, imaxpct=25
> >          =                       sunit=16     swidth=16 blks, unwritten=1
> > naming   =version 2              bsize=4096
> > log      =internal               bsize=4096   blocks=32768, version=1
> >          =                       sectsz=512   sunit=0 blks
> > realtime =none                   extsz=65536  blocks=0, rtextents=0
> > 
> > stat on a stock FC6 kernel w/ the largeio mount option returns only the page size:
> > 
> > [root@link-07]# mount -o largeio /dev/sda1 /mnt/test/
> > [root@link-07]# stat -c %o /mnt/test/foo
> > 4096
> > 
> > with the following patch, it does what it should:
> > 
> > [root@link-07]# mount -o largeio /dev/sda1 /mnt/test/
> > [root@link-07]# stat -c %o /mnt/test/foo
> > 65536
> > 
> > same goes for filesystems w/o sunit,swidth but with the allocsize mount option.
> > 
> > stock:
> > [root@link-07]# mount -o largeio,allocsize=32768 /dev/sda1 /mnt/test/
> > [root@link-07]# stat -c %o /mnt/test/foo
> > 4096
> > 
> > w/ patch:
> > [root@link-07# mount -o largeio,allocsize=32768 /dev/sda1 /mnt/test/
> > [root@link-07]# stat -c %o /mnt/test/foo
> > 32768
> > 
> > Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
> > 
> > XFS guys, does this look ok?
> > 
> > Index: linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
> > ===================================================================
> > --- linux-2.6.18.orig/fs/xfs/linux-2.6/xfs_iops.c
> > +++ linux-2.6.18/fs/xfs/linux-2.6/xfs_iops.c
> > @@ -623,12 +623,16 @@ xfs_vn_getattr(
> >  {
> >  	struct inode	*inode = dentry->d_inode;
> >  	bhv_vnode_t	*vp = vn_from_inode(inode);
> > +	xfs_inode_t	*ip;
> >  	int		error = 0;
> >  
> >  	if (unlikely(vp->v_flag & VMODIFIED))
> >  		error = vn_revalidate(vp);
> > -	if (!error)
> > +	if (!error) {
> >  		generic_fillattr(inode, stat);
> > +		ip = xfs_vtoi(vp);
> > +		stat->blksize = xfs_preferred_iosize(ip->i_mount);
> > +	}
> >  	return -error;
> >  }
> >  
> 
> Looked at your patch and then at our xfs code in the tree and
> the existing code is different than what yours is based on.
> I then noticed in the logs Nathan has actually made changes for this:
> 
> ----------------------------
> revision 1.254
> date: 2006/07/17 10:46:05;  author: nathans;  state: Exp;  lines: +20 -5
> modid: xfs-linux-melb:xfs-kern:26565a
> Update XFS for i_blksize removal from generic inode structure
> ----------------------------
> I even reviewed the change (and I don't remember it - getting old).
> 
> I looked at the mods scheduled for 2.6.19 and this is one of them.
> 
> So the fix for this is coming soon (and the fix is different from the
> one above).
> 

eh?  Eric's patch is based on -mm, which includes the XFS git tree.  If I
go and merge the inode-diet patches from -mm, XFS gets broken until you
guys merge the above mystery patch.  (I prefer to merge the -mm patches
after all the git trees have gone, but sometimes maintainers dawdle and I
get bored of waiting).

Is git://oss.sgi.com:8090/nathans/xfs-2.6 obsolete, or are you hiding stuff
from me?  ;)

Re: [PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[Eric Sandeen]

Andrew Morton wrote:

>> So the fix for this is coming soon (and the fix is different from the
>> one above).
>>
> 
> eh?  Eric's patch is based on -mm, which includes the XFS git tree.  If I
> go and merge the inode-diet patches from -mm, XFS gets broken until you
> guys merge the above mystery patch.  (I prefer to merge the -mm patches
> after all the git trees have gone, but sometimes maintainers dawdle and I
> get bored of waiting).
> 
> Is git://oss.sgi.com:8090/nathans/xfs-2.6 obsolete, or are you hiding stuff
> from me?  ;)
> 
> 
well it's in cvs:

http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/linux-2.6/xfs_iops.c.diff?r1=text&tr1=1.254&r2=text&tr2=1.253&f=h

but I'm too lazy to check git on a friday evening. :)

Well, sgi-guys, I'll let you sort out which patch you want.  Sorry for not 
checking cvs first!

-Eric

Re: [PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[Andrew Morton]

On Fri, 22 Sep 2006 18:19:18 -0500
Eric Sandeen <sandeen@sandeen.net> wrote:

> Andrew Morton wrote:
> 
> >> So the fix for this is coming soon (and the fix is different from the
> >> one above).
> >>
> > 
> > eh?  Eric's patch is based on -mm, which includes the XFS git tree.  If I
> > go and merge the inode-diet patches from -mm, XFS gets broken until you
> > guys merge the above mystery patch.  (I prefer to merge the -mm patches
> > after all the git trees have gone, but sometimes maintainers dawdle and I
> > get bored of waiting).
> > 
> > Is git://oss.sgi.com:8090/nathans/xfs-2.6 obsolete, or are you hiding stuff
> > from me?  ;)
> > 
> > 
> well it's in cvs:

That's nearly four months old!

> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/linux-2.6/xfs_iops.c.diff?r1=text&tr1=1.254&r2=text&tr2=1.253&f=h

<checks to see if the changelog is in Aramaic too>

Re: [PATCH -mm] rescue large xfs preferred iosize from the inode diet patch

[Timothy Shimmin]

Andrew Morton wrote:
> On Fri, 22 Sep 2006 12:03:30 +1000

>> Looked at your patch and then at our xfs code in the tree and
>> the existing code is different than what yours is based on.
>> I then noticed in the logs Nathan has actually made changes for this:
>>
>> ----------------------------
>> revision 1.254
>> date: 2006/07/17 10:46:05;  author: nathans;  state: Exp;  lines: +20 -5
>> modid: xfs-linux-melb:xfs-kern:26565a
>> Update XFS for i_blksize removal from generic inode structure
>> ----------------------------
>> I even reviewed the change (and I don't remember it - getting old).
>>
>> I looked at the mods scheduled for 2.6.19 and this is one of them.
>>
>> So the fix for this is coming soon (and the fix is different from the
>> one above).
>>
> 
> eh?  Eric's patch is based on -mm, which includes the XFS git tree.  If I
> go and merge the inode-diet patches from -mm, XFS gets broken until you
> guys merge the above mystery patch.  (I prefer to merge the -mm patches
> after all the git trees have gone, but sometimes maintainers dawdle and I
> get bored of waiting).
> 
> Is git://oss.sgi.com:8090/nathans/xfs-2.6 obsolete, or are you hiding stuff
> from me?  ;)
> 
:)
We're still getting our act together since Nathan is no longer here.
Going forward the new git tree is at:
     git://oss.sgi.com:8090/xfs/xfs-2.6

This has some more recent changes than the "nathans" one but
is far from up to date with the internal sgi tree and the external
cvs tree (as you noticed with the nathans one:).

I will get the "xfs" one updated in the next day or so.

(Aside: for some strange reason, the "nathans" one has 3 extra
  mods (commits) and as expected (to me:) the "xfs" one has 10 extra
  mods (commits) and there is about 46 mods (including missing 3)
  pending for the "xfs" tree.
  If we end up moving from our internal SCM to git at some point,
  this could make the updates less of a hassle:).


--Tim