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 <pbadari@xxxxxxxxxx> > --- > 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html