2005-12-28 07:56:57

by junjie cai

[permalink] [raw]
Subject: [RFC][fat] use mpage_readpage when cluster size is page-alignment

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) {


2005-12-28 09:36:18

by junjie cai

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

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

2005-12-28 12:58:03

by OGAWA Hirofumi

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

junjie cai <[email protected]> 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 <[email protected]>



From: OGAWA Hirofumi <[email protected]>

Signed-off-by: OGAWA Hirofumi <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

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,
_

2005-12-29 06:34:45

by junjie cai

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

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

2005-12-29 08:06:18

by Pekka Enberg

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

Hi,

On 12/29/05, cai <[email protected]> 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

2005-12-29 08:49:23

by junjie cai

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

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

2005-12-29 09:13:14

by Pekka Enberg

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

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

2005-12-29 09:19:15

by OGAWA Hirofumi

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

cai <[email protected]> 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 <[email protected]>

2005-12-29 10:20:55

by junjie cai

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

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