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 :( And also, you are missing few of the cleanups I sent + ext4 aop conversion. Do you want me to send it again ? 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