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 <[email protected]>
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;
}
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 <[email protected]>
>
> 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
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 <[email protected]>
>
> 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.
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
On Fri, 22 Sep 2006 12:03:30 +1000
Timothy Shimmin <[email protected]> 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 <[email protected]>
> >
> > 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? ;)
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
On Fri, 22 Sep 2006 18:19:18 -0500
Eric Sandeen <[email protected]> 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>
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