Re: [PATCH] FAT: Readahead FAT table to reduce the read request of FAT tabl

[Andrew Morton <akpm@linux-foundation.org>]

On Thu, 25 Nov 2010 16:46:37 +0800 ykzhao <yakui.zhao@intel.com> wrote:

> >From badc27e6b5f732191fb3b7e964f68da030144c76 Mon Sep 17 00:00:00 2001
> From: Zhao Yakui <yakui.zhao@intel.com>
> Date: Fri, 19 Nov 2010 10:29:33 +0800
> Subject: [PATCH] FAT: Readahead FAT table to reduce the read request of FAT table
> 
> On FAT filesystem the FAT table will be accessed very frequently when
> reading/writing the file. Now it reads only block sector from FAT table every
> time and it will have to wait for the completion of read-access. In fact
> as the FAT table is located sequentially on the disk, maybe we can readahead
> the following sectors to reduce the read access of FAT table.
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> ---
> The following is the test result on SD card/U-disk.
> a. Read/Write a large file by using direct mode on SD card
>    dd if=sda.bin of=sda.bin2 bs=64k iflag=direct
> 	The average copy time can be reduced from 470 second to 452 seconds(The file
> of sda.bin is about 1.8G).
> 
> a. Read/Write a large file by using direct mode on U-disk
>    dd if=sda.bin of=sda.bin2 bs=64k iflag=direct
> 	The average copy time can be reduced from about 880 to 855 seconds.(The file
> of sda.bin is about 1.8G).

Seems reasonable.

> diff --git a/fs/fat/fatent.c b/fs/fat/fatent.c
> index b47d2c9..1dfd73f 100644
> --- a/fs/fat/fatent.c
> +++ b/fs/fat/fatent.c
> @@ -67,6 +67,26 @@ static void fat32_ent_set_ptr(struct fat_entry *fatent, int offset)
>  	fatent->u.ent32_p = (__le32 *)(fatent->bhs[0]->b_data + offset);
>  }
>  
> +static void fat_table_readahead(struct super_block *sb, sector_t iphys)
> +{
> +	struct msdos_sb_info *sbi = MSDOS_SB(sb);
> +	struct buffer_head *bh;
> +	int sec, sec_count;
> +
> +	sec_count = sbi->fat_readahead_cnt;
> +	/*
> +	 * It will check whether the current block is already obtained from the
> +	 * disk. If not, we will try to readahead more sectors when
> +	 * reading the FAT table
> +	 */
> +	bh = sb_find_get_block(sb, iphys);
> +	if (bh == NULL || !buffer_uptodate(bh)) {
> +		for (sec = 0; sec < sec_count; sec++)
> +			sb_breadahead(sb, iphys + sec);
> +	}
> +	brelse(bh);
> +}

A few things.

a) I'd like to be really really convinced that there's no way in which
   this function can wander off the end of the device.  Or that the called
   functions handle this situation gracefully.

b) Lots of (old) filesystems probably do block-level readahead like this. 
   We really should have a generic vfs-layer library function, rather than
   open-coding it all the time.

c) sb_breadahead() is a truly ancient function.  It would be nice to use
   the more modern (and probably more efficient)
   force_page_cache_readahead().  It should be OK to pass filp==0.