Re: [PATCH 1/1] fat: improve sync performance by grouping writes in fat_mirror_bhs [really unmangled]

From: Holden Karau
Date: Fri Oct 27 2006 - 14:56:45 EST


Hi Jörn,

Thanks for your time, I'll make those changes [along with a few other
things I noticed while benchmarking it]. Before I put together a
patch, does anyone else see any obvious stuff I should clean up?

Cheers,

Holden :-)

On 10/26/06, Jörn Engel <joern@xxxxxxxxxxxxxxxxxxxx> wrote:
I didn't pay too much attention, but found some low hanging fruits.

On Thu, 26 October 2006 07:59:42 -0400, Holden Karau wrote:
>
> -/* FIXME: We can write the blocks as more big chunk. */
> static int fat_mirror_bhs(struct super_block *sb, struct buffer_head **bhs,
> - int nr_bhs)
> + int nr_bhs ) {
> + return fat_mirror_bhs_optw(sb , bhs , nr_bhs, 0);
> +}
> +
> +static int fat_mirror_bhs_optw(struct super_block *sb, struct buffer_head **bhs,
> + int nr_bhs , int wait)

Does this compile without warnings? Looks as if you should reverse
the order of the two functions.

For some reason it compiles without warnings for me, but I'll switch the order.
> {
> struct msdos_sb_info *sbi = MSDOS_SB(sb);
> - struct buffer_head *c_bh;
> + struct buffer_head *c_bh[nr_bhs];
> int err, n, copy;
>
> + /* Always wait if mounted -o sync */
> + if (sb->s_flags & MS_SYNCHRONOUS ) {
> + wait = 1;
> + }

Coding style. Use a tab for indentation and don't use braces for
single-line conditional statements.

Sorry about that. A lot of the places where I used braces are because
I had some debugging output in there while I was hacking on it. I'll
change it.
> +
> err = 0;
> + err = fat_sync_bhs_optw( bhs , nr_bhs , wait);

The err=0; is superfluous now, isn't it?

.... no comment :-)
> + if (err)
> + goto error;

Indentation.

oops :-) I'll fix that.
Jörn

--
Fantasy is more important than knowledge. Knowledge is limited,
while fantasy embraces the whole world.
-- Albert Einstein



--
Cell: 613-276-1645
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/