[RFC][fat] use mpage_readpage when cluster size is page-alignment

[RFC][fat] use mpage_readpage when cluster size is page-alignment

[junjie cai]

hi,

it seems that mpage_read is faster then block_read_full_page
when performing block-adjacent I/O.
though not tested strictly, in a flash-based system,
copying a 600k file reduced to 17ms from 30ms

thanks.
                                             junjie

diff --git a/fs/fat/inode.c b/fs/fat/inode.c
index a0f9b9f..3d25a2b 100644
--- a/fs/fat/inode.c
+++ b/fs/fat/inode.c
@@ -22,6 +22,7 @@
 #include <linux/mount.h>
 #include <linux/vfs.h>
 #include <linux/parser.h>
+#include <linux/mpage.h>
 #include <asm/unaligned.h>

 #ifndef CONFIG_FAT_DEFAULT_IOCHARSET
@@ -95,6 +96,11 @@ static int fat_readpage(struct file *fil
 	return block_read_full_page(page, fat_get_block);
 }

+static int fat_mpage_readpage(struct file *file, struct page *page)
+{
+	return  mpage_readpage(page, fat_get_block);
+}
+
 static int fat_prepare_write(struct file *file, struct page *page,
 			     unsigned from, unsigned to)
 {
@@ -130,6 +136,18 @@ static struct address_space_operations f
 };

 /*
+ * for page-alignemnt cluster-size
+ */
+static struct address_space_operations fat_mpage_aops = {
+	.readpage	= fat_mpage_readpage,
+	.writepage	= fat_writepage,
+	.sync_page	= block_sync_page,
+	.prepare_write	= fat_prepare_write,
+	.commit_write	= fat_commit_write,
+	.bmap		= _fat_bmap
+};
+
+/*
  * New FAT inode stuff. We do the following:
  *	a) i_ino is constant and has nothing with on-disk location.
  *	b) FAT manages its own cache of directory entries.
@@ -288,7 +306,12 @@ static int fat_fill_inode(struct inode *
 		inode->i_size = le32_to_cpu(de->size);
 		inode->i_op = &fat_file_inode_operations;
 		inode->i_fop = &fat_file_operations;
-		inode->i_mapping->a_ops = &fat_aops;
+
+		if (sbi->cluster_size & ~PAGE_MASK)
+			inode->i_mapping->a_ops = &fat_aops;
+		else
+			inode->i_mapping->a_ops = &fat_mpage_aops;
+
 		MSDOS_I(inode)->mmu_private = inode->i_size;
 	}
 	if (de->attr & ATTR_SYS) {

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[junjie cai]

hi,

> > +static struct address_space_operations fat_mpage_aops = {
> > +       .readpage       = fat_mpage_readpage,
>
> Should use mpage_readpage directly?
>

no, it is used only when the cluster size is page-alignment

thanks.
                                              junjie

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[OGAWA Hirofumi]

junjie cai <junjiec@gmail.com> writes:

> it seems that mpage_read is faster then block_read_full_page
> when performing block-adjacent I/O.
> though not tested strictly, in a flash-based system,
> copying a 600k file reduced to 17ms from 30ms

Looks like good to me. Thanks for doing this.

I changed it recently, and it's waiting to open 2.6.16 in -mm tree.
The patch (fat-add-the-read-writepages.patch) is the following, but it
is using mpage_readpage() always. (also use mpage_xxxpages().)

Can't we use mpage_readpage() always? IIRC, that should work fine
without disadvantage.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>



From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 fs/fat/inode.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletion(-)

diff -puN fs/fat/inode.c~fat-add-the-read-writepages fs/fat/inode.c
--- 25/fs/fat/inode.c~fat-add-the-read-writepages	Mon Nov  7 17:02:07 2005
+++ 25-akpm/fs/fat/inode.c	Mon Nov  7 17:02:07 2005
@@ -18,6 +18,7 @@
 #include <linux/seq_file.h>
 #include <linux/msdos_fs.h>
 #include <linux/pagemap.h>
+#include <linux/mpage.h>
 #include <linux/buffer_head.h>
 #include <linux/mount.h>
 #include <linux/vfs.h>
@@ -90,9 +91,21 @@ static int fat_writepage(struct page *pa
 	return block_write_full_page(page, fat_get_block, wbc);
 }
 
+static int fat_writepages(struct address_space *mapping,
+			  struct writeback_control *wbc)
+{
+	return mpage_writepages(mapping, wbc, fat_get_block);
+}
+
 static int fat_readpage(struct file *file, struct page *page)
 {
-	return block_read_full_page(page, fat_get_block);
+	return mpage_readpage(page, fat_get_block);
+}
+
+static int fat_readpages(struct file *file, struct address_space *mapping,
+			 struct list_head *pages, unsigned nr_pages)
+{
+	return mpage_readpages(mapping, pages, nr_pages, fat_get_block);
 }
 
 static int fat_prepare_write(struct file *file, struct page *page,
@@ -122,7 +135,9 @@ static sector_t _fat_bmap(struct address
 
 static struct address_space_operations fat_aops = {
 	.readpage	= fat_readpage,
+	.readpages	= fat_readpages,
 	.writepage	= fat_writepage,
+	.writepages	= fat_writepages,
 	.sync_page	= block_sync_page,
 	.prepare_write	= fat_prepare_write,
 	.commit_write	= fat_commit_write,
_

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[cai]

hi,

>Can't we use mpage_readpage() always? IIRC, that should work fine
>without disadvantage.
>
>Thanks.
>  
>
it should work, but maybe some performance loses if the
cluster size is not page-alignment, for example, 4 sector/cluster
in a 4KB/page system.
because it will fall back to the block_read_full_page when
non-adjacent block found in do_mpage_readpage, i think.
the same applies to mpage_readpages too.

thanks.
                                      junjie

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[Pekka Enberg]

Hi,

On 12/29/05, cai <junjiec@gmail.com> wrote:
> it should work, but maybe some performance loses if the
> cluster size is not page-alignment, for example, 4 sector/cluster
> in a 4KB/page system.
> because it will fall back to the block_read_full_page when
> non-adjacent block found in do_mpage_readpage, i think.
> the same applies to mpage_readpages too.

I am not sure I am following you. Shouldn't do_mpage_readpage work for
all adjacent blocks regardless of whether block size is page-aligned
or not? What's is the performance problem you're thinking of?

                            Pekka

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[cai]

hi,

>I am not sure I am following you. Shouldn't do_mpage_readpage work for
>all adjacent blocks regardless of whether block size is page-aligned
>or not? What's is the performance problem you're thinking of?
>
>                            Pekka
>
>  
>
no, not block size but cluster size
as you know, in FAT, file is organized in clusters
and one cluster could have N blocks(sectors).
so if cluster size is not page-aligned,
a page may live in non-adjacent blocks, and
do_mpage_readpage has to fall back to block_read_full_page
in this case.

thanks.
                                   junjie

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[Pekka J Enberg]

On Thu, 29 Dec 2005, cai wrote:
> > I am not sure I am following you. Shouldn't do_mpage_readpage work for
> > all adjacent blocks regardless of whether block size is page-aligned
> > or not? What's is the performance problem you're thinking of?
>  
> no, not block size but cluster size
> as you know, in FAT, file is organized in clusters
> and one cluster could have N blocks(sectors).
> so if cluster size is not page-aligned,
> a page may live in non-adjacent blocks, and
> do_mpage_readpage has to fall back to block_read_full_page
> in this case.

But the non-page-aligned clusters can be adjacent on disk, no? Besides, I 
don't think there's enough overhead in do_mpage_readpage for the 
non-adjacent case to justify keeping the non-mpage version around.

				Pekka

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[OGAWA Hirofumi]

cai <junjiec@gmail.com> writes:

>>Can't we use mpage_readpage() always? IIRC, that should work fine
>>without disadvantage.
>>
> it should work, but maybe some performance loses if the
> cluster size is not page-alignment, for example, 4 sector/cluster
> in a 4KB/page system.
> because it will fall back to the block_read_full_page when
> non-adjacent block found in do_mpage_readpage, i think.
> the same applies to mpage_readpages too.

Ah, yes.

But if cluster is not fragmented it shouldn't fall back, and rather it
will get advantage.  And I guess, even if it fall back to
block_read_full_page(), it would be very trivial.

What do you think?   We may need benchmark...
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [RFC][fat] use mpage_readpage when cluster size is page-alignment

[cai]

hi,

>Ah, yes.
>
>But if cluster is not fragmented it shouldn't fall back, and rather it
>will get advantage.  And I guess, even if it fall back to
>block_read_full_page(), it would be very trivial.
>
>What do you think?   We may need benchmark...
>  
>
i think you are right, i didn't think of adjacent-cluster
so maybe just calling mpage_readpage{s} is enough.

thanks.
                         junjie