Re: [ANNOUNCE] new new aops patchset

[Nick Piggin <npiggin@suse.de>]

On Mon, Apr 02, 2007 at 04:14:59PM -0700, Badari Pulavarty wrote:
> On Mon, 2007-04-02 at 14:09 +0200, Nick Piggin wrote:
> > Updated aops patchset against 2.6.21-rc5.
> > 
> > http://www.kernel.org/pub/linux/kernel/people/npiggin/patches/new-aops/
> > 
> > Files/dirs are 2.6.21-rc5-new-aops*
> 
> Baaah !! You took away ext3 -nobh option :(

Ahh, just the person I wanted to ask! ;) How useful is it, out of curiosity?
What sort of users use it, and what sort of improvements do they get?


> Do you have plans to support nobh versions of block_write_begin/end ?

At the moment I'm looking at doing it another way. Having the seperate
nobh path is quite annoying -- there are still bugs in it and it is
simply less tested (not that the bh path is bug-free either, but it
is better to be able to concentrate on one). So I hope to merge them
at some point to restore that functionality. 


> BTW, I don't see how block_write_end() can ever return < 0.
> If so, here is the cleanup fix for ext3 (no unnecessay checks).

Shouldn't we allow for the possibility?


> ---
>  fs/ext3/inode.c |   11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> Index: linux-2.6.21-rc5/fs/ext3/inode.c
> ===================================================================
> --- linux-2.6.21-rc5.orig/fs/ext3/inode.c	2007-04-02 16:01:27.000000000 -0700
> +++ linux-2.6.21-rc5/fs/ext3/inode.c	2007-04-02 16:06:39.000000000 -0700
> @@ -1328,8 +1328,6 @@ static int ext3_ordered_write_end(struct
>  		if (new_i_size > EXT3_I(inode)->i_disksize)
>  			EXT3_I(inode)->i_disksize = new_i_size;
>  		copied = block_write_end(file, mapping, pos, len, copied, page, fsdata);
> -		if (copied < 0)
> -			ret = copied;
>  	}
>  	ret2 = ext3_journal_stop(handle);
>  	if (!ret)
> @@ -1344,7 +1342,7 @@ static int ext3_writeback_write_end(stru
>  {
>  	handle_t *handle = ext3_journal_current_handle();
>  	struct inode *inode = file->f_mapping->host;
> -	int ret = 0, ret2;
> +	int ret;
>  	loff_t new_i_size;
>  
>  	new_i_size = pos + copied;
> @@ -1352,12 +1350,9 @@ static int ext3_writeback_write_end(stru
>  		EXT3_I(inode)->i_disksize = new_i_size;
>  
>  	copied = block_write_end(file, mapping, pos, len, copied, page, fsdata);
> -	if (copied < 0)
> -		ret = copied;
>  
> -	ret2 = ext3_journal_stop(handle);
> -	if (!ret)
> -		ret = ret2;
> +	ret = ext3_journal_stop(handle);
> +
>  	return ret ? ret : copied;
>  }
>  
>