Hi.
At 07:40 05/04/06, Stephen C. Tweedie wrote: >Sorry, was offline for a week last week; I'll try to look at this more >closely tomorrow. Checking the buffer_uptodate() without either a >refcount or a lock certainly looks unsafe at first glance. > >There are lots of ways to pin the bh in that particular bit of the >code. The important thing will be to do so without causing leaks if >we're truly finished with the buffer after this flush. >
I have measured the bh refcount before the buffer_uptodate() for a few days. I found out that the bh refcount sometimes reached to 0 . So, I think following modifications are effective.
diff -Nru 2.4.30-rc3/fs/jbd/commit.c 2.4.30-rc3_patch/fs/jbd/commit.c
--- 2.4.30-rc3/fs/jbd/commit.c 2005-04-06 17:14:47.000000000 +0900
+++ 2.4.30-rc3_patch/fs/jbd/commit.c 2005-04-06 17:18:49.000000000 +0900
@@ -295,6 +295,7 @@
struct buffer_head *bh;
jh = jh->b_tprev; /* Wait on the last written */
bh = jh2bh(jh);
+ get_bh(bh);
if (buffer_locked(bh)) {
spin_unlock(&journal_datalist_lock);
unlock_journal(journal);
@@ -302,11 +303,14 @@
if (unlikely(!buffer_uptodate(bh)))
err = -EIO;
/* the journal_head may have been removed now */
+ put_bh(bh);
lock_journal(journal);
goto write_out_data;
} else if (buffer_dirty(bh)) {
+ put_bh(bh);
goto write_out_data_locked;
}
+ put_bh(bh);
} while (jh != commit_transaction->t_sync_datalist);
goto write_out_data_locked;> >> > If some of the write succeeded and some failed, then I believe the >> > correct behaviour is to return the number of bytes that succeeded. >> > However this change to the return status (remember the above patch is >> > a reversal) causes any failure to over-ride any success. This, I >> > think, is wrong. >> >> Yeap, that part also looks wrong. > >Certainly it's normal for a short read/write to imply either error or >EOF, without the error necessarily needing to be returned explicitly. >I'm not convinced that the Singleunix language actually requires that, >but it seems the most obvious and consistent behaviour. > >--Stephen
When an O_SYNC flag is set , if commit_write() succeed but generic_osync_inode() return
error due to I/O failure, write() must fail .
I think that following error handling code is rational in do_generic_file_write() .
if (file->f_flags & O_SYNC)
err = (status < 0) ? status : written;
else
err = written ? written : status;
out:return err;
Thanks.
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
