[linux-cifs-client] Re: io blksize problem with cifs

[Andrew Morton]

Please don't top-post!

<edit, edit>

On Tue, 19 Sep 2006 11:30:28 -0500
Steven French <sfrench at us.ibm.com> wrote:

> > --- 
> > a/fs/stat.c~inode-diet-eliminate-i_blksize-and-use-a-per-superblock-default
> > > +++ a/fs/stat.c
> > > @@ -14,6 +14,7 @@
> > >  #include <linux/namei.h>
> > >  #include <linux/security.h>
> > >  #include <linux/syscalls.h>
> > > +#include <linux/pagemap.h>
> > > 
> > >  #include <asm/uaccess.h>
> > >  #include <asm/unistd.h>
> > > @@ -32,7 +33,7 @@ void generic_fillattr(struct inode *inod
> > >                stat->ctime = inode->i_ctime;
> > >                stat->size = i_size_read(inode);
> > >                stat->blocks = inode->i_blocks;
> > > -              stat->blksize = inode->i_blksize;
> > > +              stat->blksize = PAGE_CACHE_SIZE;
> > >  }
> > > 
> > >  EXPORT_SYMBOL(generic_fillattr);
> > > 
> > > Ted, how come that's not (1 << inode->i_blkbits)?
> > 
> > I'm not Ted, but it's probably because stat->blksize isn't really tied
> > to the file system's block size.
> > 
> > from STAT(2):
> > 
> >        The st_blksize field gives the "preferred" blocksize for efficient 
> > file
> >        system  I/O.  (Writing to a file in smaller chunks may cause an 
> > ineffi-
> >        cient read-modify-rewrite.)
> > 
> > PAGE_CACHE_SIZE seems a reasonable default.  The file system is free to
> > overwrite this after calling generic_fillattr().

well...  2.6.18 will use inode->i_blksize here, so using (1<<i_blkbits)
should give exactly the same thing, which is desirable in the context of
this patch.


> ...
>
> For cifs the preferred read size would be 16K, the preferred write size is 
> 56K (pointers 14 pages can fit in cifs's iovec) - there are a few older 
> server which does not support large write (where 16K or in the case of 
> very old servers, 2K, would be preferred).
> 

After Ted's change, CIFS went from reporting 16k down to reporting 4k.

It wasn't clear to me from the original report whether this is actually
causing any problems?

So I think my proposed change is correct and should be proceeded with.  The
issue of "what should CIFS do about stat.blksize" is a separate one.  Be
aware that reiserfs got into all sorts of trouble using a 128k hint.  16k
is probably OK.