On Fri, Apr 13, 2007 at 09:42:04AM -0700, Badari Pulavarty wrote:
> On Fri, 2007-04-13 at 01:25 +0200, Nick Piggin wrote:
> > On Thu, Apr 12, 2007 at 09:37:36AM -0700, Badari Pulavarty wrote:
> > > On Thu, 2007-04-12 at 06:48 +0200, Nick Piggin wrote:
> > > > http://www.kernel.org/pub/linux/kernel/people/npiggin/patches/new-aops/
> > > > 
> > > > 2.6.21-rc6-new-aops*
> > > > 
> > > > New aops patchset against 2.6.21-rc6.
> 
> Bah !! ext3_ordered_write_end() is still not quite right :(
> 
> 
> ext3_ordered_write_end()
> {
>        ret = walk_page_buffers(handle, page_buffers(page),
>                 from, to, NULL, ext3_journal_dirty_data);
> 
>         if (ret == 0) {
>               ...
>               
>               generic_write_end();
>       }
>       ret2 = ext3_journal_stop(handle);
>       ...
> }
> 
> If walk_page_buffers() fails, we leave the page locked and refed :(

Thanks for the patch.


> And also, you are missing few of the cleanups I sent + ext4 aop
> conversion. Do you want me to send it again ?

If you could, that would be great.

Thanks,
Nick

> 
> Thanks,
> Badari
> 
> ext3_ordered_write_end() should unlock and release the page if 
> walk_buffers() fail.
> 
> 
> Signed-off-by: Badari Pulavarty <[EMAIL PROTECTED]>
> ---
>  fs/ext3/inode.c |   11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> Index: linux-2.6.21-rc6.aop/fs/ext3/inode.c
> ===================================================================
> --- linux-2.6.21-rc6.aop.orig/fs/ext3/inode.c 2007-04-12 09:21:17.000000000 
> -0700
> +++ linux-2.6.21-rc6.aop/fs/ext3/inode.c      2007-04-13 09:32:58.000000000 
> -0700
> @@ -1251,12 +1251,13 @@ static int ext3_ordered_write_end(struct
>               new_i_size = pos + copied;
>               if (new_i_size > EXT3_I(inode)->i_disksize)
>                       EXT3_I(inode)->i_disksize = new_i_size;
> -             ret2 = generic_write_end(file, mapping, pos, len, copied,
> +             copied = generic_write_end(file, mapping, pos, len, copied,
>                                                       page, fsdata);
> -             if (ret2 >= 0)
> -                     copied = ret2;
> -             else
> -                     ret = ret2;
> +             if (copied < 0)
> +                     ret = copied;
> +     } else {
> +             unlock_page(page);
> +             page_cache_release(page);
>       }
>       ret2 = ext3_journal_stop(handle);
>       if (!ret)
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to