On Tue, 2007-04-03 at 01:58 +0200, Nick Piggin wrote:
> On Mon, Apr 02, 2007 at 01:44: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*
> > > 
> > > Contains numerous fixes from Mark and myself -- I'd say the core code is
> > > getting reasonably stable at this point.
> > 
> > 
> > ext3_write_failure() conversion is NOT quite correct.
> > Old code was returing failure of do_journal_get_write_access(),
> > where as your changes will return journal_stop(). 
> 
> Good catch. This still requires a journal_stop, however?

Yes.
> 
> How's this look?

Looks good. I am little worried about the page-lock vs journal
start/stop order. I remember running into deadlock while trying
to do writepages() for ordered mode. I think we are okay here.
I will take a closer look.

Thanks,
Badari

> 
> --
> Index: linux-2.6/fs/ext3/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/ext3/inode.c
> +++ linux-2.6/fs/ext3/inode.c
> @@ -1167,11 +1167,13 @@ static int ext3_write_failure(struct fil
>       handle_t *handle = ext3_journal_current_handle();
>  
>       if (ext3_should_writeback_data(mapping->host)) {
> +skip_and_stop:
>               /* optimization: no constraints about data */
> +             ret = ext3_journal_stop(handle);
>  skip:
>               unlock_page(page);
>               page_cache_release(page);
> -             return ext3_journal_stop(handle);
> +             return ret;
>       }
>  
>       from = pos & (PAGE_CACHE_SIZE - 1);
> @@ -1196,8 +1198,10 @@ skip:
>                       break;
>               if (ext3_should_journal_data(mapping->host)) {
>                       ret = do_journal_get_write_access(handle, bh);
> -                     if (ret)
> +                     if (ret) {
> +                             ext3_journal_stop(handle);
>                               goto skip;
> +                     }
>               }
>       /*
>        * block_start here becomes the first block where the current iteration
> @@ -1205,7 +1209,7 @@ skip:
>        */
>       }
>       if (block_start <= from)
> -             goto skip;
> +             goto skip_and_stop;
>  
>       /* commit allocated and zeroed buffers */
>       return mapping->a_ops->write_end(file, mapping, pos, len,

-
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