Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 11:01, Hifumi Hisashi wrote: > 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.c2005-04-06 17:14:47.0 +0900 > +++ 2.4.30-rc3_patch/fs/jbd/commit.c 2005-04-06 17:18:49.0 +0900 > @@ -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; ... In further testing, this chunk is causing some problems. It is entirely legal for a buffer to be !uptodate here, although the path is somewhat convoluted. The trouble is, once the IO has completed, journal_dirty_data() can steal the buffer from the committing transaction. And once that happens, journal_unmap_buffer() can then release the buffer and clear the uptodate bit. I've run this under buffer tracing and can show you this exact path on a trace. It only takes a few seconds to reproduce under fsx. For this to have happened, though, journal_dirty_data needs to have waited on the data, so it has an opportunity to see the !uptodate state at that point. I think it's safe enough to sidestep this by double-checking to see whether the bh is still on the committing transaction after we've waited for the buffer and retaken journaling locks, but before testing buffer_uptodate(). --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 11:01, Hifumi Hisashi wrote: 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.c2005-04-06 17:14:47.0 +0900 +++ 2.4.30-rc3_patch/fs/jbd/commit.c 2005-04-06 17:18:49.0 +0900 @@ -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; ... In further testing, this chunk is causing some problems. It is entirely legal for a buffer to be !uptodate here, although the path is somewhat convoluted. The trouble is, once the IO has completed, journal_dirty_data() can steal the buffer from the committing transaction. And once that happens, journal_unmap_buffer() can then release the buffer and clear the uptodate bit. I've run this under buffer tracing and can show you this exact path on a trace. It only takes a few seconds to reproduce under fsx. For this to have happened, though, journal_dirty_data needs to have waited on the data, so it has an opportunity to see the !uptodate state at that point. I think it's safe enough to sidestep this by double-checking to see whether the bh is still on the committing transaction after we've waited for the buffer and retaken journaling locks, but before testing buffer_uptodate(). --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Mon, 2005-04-11 at 21:46, Andrew Morton wrote: > "Stephen C. Tweedie" <[EMAIL PROTECTED]> wrote: > > > > Andrew, what was the exact illegal state of the pages you were seeing > > when fixing that recent leak? It looks like it's nothing more complex > > than dirty buffers on an anon page. > > Correct. Good --- I think I've got the debugging solid against 2.4 for that case, patching against 2.6 now. > > I think that simply calling > > try_to_release_page() for all the remaining buffers at umount time will > > Presumably these pages have no ->mapping, so try_to_release_page() will > call try_to_free_buffers(). Exactly. > > The only thing that would be required on > > top of that would be a check that the page is also on the VM LRU lists. > > Why do we have dirty buffers left over at umount time? In the leak case we're worried about, the buffers are dirty but the page is anon so there's nothing to clean them up. The buffers _will_ be destroyed by unmount, sure; invalidate_bdev() should see to that. But I'm doing the bh leak check before we get to the final invalidate_bdev(), so they should still be available for testing at that point. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
"Stephen C. Tweedie" <[EMAIL PROTECTED]> wrote: > > Andrew, what was the exact illegal state of the pages you were seeing > when fixing that recent leak? It looks like it's nothing more complex > than dirty buffers on an anon page. Correct. > I think that simply calling > try_to_release_page() for all the remaining buffers at umount time will Presumably these pages have no ->mapping, so try_to_release_page() will call try_to_free_buffers(). > be enough to catch these; if that function fails, it tells us that the > VM can't reclaim these pages. Yes, if the buffers are dirty then 2.4's try_to_free_buffers() won't free them. > The only thing that would be required on > top of that would be a check that the page is also on the VM LRU lists. Why do we have dirty buffers left over at umount time? - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Thu, 2005-04-07 at 16:51, Stephen C. Tweedie wrote: > I'm currently running with the buffer-trace debug patch, on 2.4, with a > custom patch to put every buffer jbd ever sees onto a per-superblock > list, and remove it only when the bh is destroyed in > put_unused_buffer_head(). At unmount time, we can walk that list to > find stale buffers attached to data pages (invalidate_buffers() already > does such a walk for metadata.) After a 50-process dbench run, unmount is showing ~300 buffers unclaimed; that seems to be OK, it's a large stress test doing _lots_ of create/unlink and we expect to see a few buffers around at the end where they were truncated during commit and couldn't be instantly reclaimed. The main thing now is to test these buffers to make 100% sure that they are in a state where the VM can reclaim them. That looks fine: the buffers I see are unjournaled, have no jh, and are clean and on the BUF_CLEAN lru. Andrew, what was the exact illegal state of the pages you were seeing when fixing that recent leak? It looks like it's nothing more complex than dirty buffers on an anon page. I think that simply calling try_to_release_page() for all the remaining buffers at umount time will be enough to catch these; if that function fails, it tells us that the VM can't reclaim these pages. The only thing that would be required on top of that would be a check that the page is also on the VM LRU lists. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Thu, 2005-04-07 at 16:51, Stephen C. Tweedie wrote: I'm currently running with the buffer-trace debug patch, on 2.4, with a custom patch to put every buffer jbd ever sees onto a per-superblock list, and remove it only when the bh is destroyed in put_unused_buffer_head(). At unmount time, we can walk that list to find stale buffers attached to data pages (invalidate_buffers() already does such a walk for metadata.) After a 50-process dbench run, unmount is showing ~300 buffers unclaimed; that seems to be OK, it's a large stress test doing _lots_ of create/unlink and we expect to see a few buffers around at the end where they were truncated during commit and couldn't be instantly reclaimed. The main thing now is to test these buffers to make 100% sure that they are in a state where the VM can reclaim them. That looks fine: the buffers I see are unjournaled, have no jh, and are clean and on the BUF_CLEAN lru. Andrew, what was the exact illegal state of the pages you were seeing when fixing that recent leak? It looks like it's nothing more complex than dirty buffers on an anon page. I think that simply calling try_to_release_page() for all the remaining buffers at umount time will be enough to catch these; if that function fails, it tells us that the VM can't reclaim these pages. The only thing that would be required on top of that would be a check that the page is also on the VM LRU lists. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Stephen C. Tweedie [EMAIL PROTECTED] wrote: Andrew, what was the exact illegal state of the pages you were seeing when fixing that recent leak? It looks like it's nothing more complex than dirty buffers on an anon page. Correct. I think that simply calling try_to_release_page() for all the remaining buffers at umount time will Presumably these pages have no -mapping, so try_to_release_page() will call try_to_free_buffers(). be enough to catch these; if that function fails, it tells us that the VM can't reclaim these pages. Yes, if the buffers are dirty then 2.4's try_to_free_buffers() won't free them. The only thing that would be required on top of that would be a check that the page is also on the VM LRU lists. Why do we have dirty buffers left over at umount time? - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Mon, 2005-04-11 at 21:46, Andrew Morton wrote: Stephen C. Tweedie [EMAIL PROTECTED] wrote: Andrew, what was the exact illegal state of the pages you were seeing when fixing that recent leak? It looks like it's nothing more complex than dirty buffers on an anon page. Correct. Good --- I think I've got the debugging solid against 2.4 for that case, patching against 2.6 now. I think that simply calling try_to_release_page() for all the remaining buffers at umount time will Presumably these pages have no -mapping, so try_to_release_page() will call try_to_free_buffers(). Exactly. The only thing that would be required on top of that would be a check that the page is also on the VM LRU lists. Why do we have dirty buffers left over at umount time? In the leak case we're worried about, the buffers are dirty but the page is anon so there's nothing to clean them up. The buffers _will_ be destroyed by unmount, sure; invalidate_bdev() should see to that. But I'm doing the bh leak check before we get to the final invalidate_bdev(), so they should still be available for testing at that point. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 21:10, Stephen C. Tweedie wrote: > However, 2.6 is suspected of still having leaks in ext3. To be certain > that we're not just backporting one of those to 2.4, we need to > understand who exactly is going to clean up these bh's if they are in > fact unused once we complete this code and do the final put_bh(). > > I'll give this patch a spin with Andrew's fsx-based leak stress and see > if anything unpleasant appears. I'm currently running with the buffer-trace debug patch, on 2.4, with a custom patch to put every buffer jbd ever sees onto a per-superblock list, and remove it only when the bh is destroyed in put_unused_buffer_head(). At unmount time, we can walk that list to find stale buffers attached to data pages (invalidate_buffers() already does such a walk for metadata.) I just ran it with a quick test and it found 300,000 buffers still present at unmount. Whoops, I guess I need to move the check to _after_ the final invalidate_inodes() call. :-) But this method ought to allow us to test for this sort of leak a lot more reliably, and to report via the usual buffer history tracing the most recent activity on any bh that does leak. Andrew, I'll give this a try on 2.6 once I've got this 2.4 patch tested for Marcelo. I've got uptodate buffer_trace for 2.6 anyway, so it shouldn't be too hard. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 21:10, Stephen C. Tweedie wrote: However, 2.6 is suspected of still having leaks in ext3. To be certain that we're not just backporting one of those to 2.4, we need to understand who exactly is going to clean up these bh's if they are in fact unused once we complete this code and do the final put_bh(). I'll give this patch a spin with Andrew's fsx-based leak stress and see if anything unpleasant appears. I'm currently running with the buffer-trace debug patch, on 2.4, with a custom patch to put every buffer jbd ever sees onto a per-superblock list, and remove it only when the bh is destroyed in put_unused_buffer_head(). At unmount time, we can walk that list to find stale buffers attached to data pages (invalidate_buffers() already does such a walk for metadata.) I just ran it with a quick test and it found 300,000 buffers still present at unmount. Whoops, I guess I need to move the check to _after_ the final invalidate_inodes() call. :-) But this method ought to allow us to test for this sort of leak a lot more reliably, and to report via the usual buffer history tracing the most recent activity on any bh that does leak. Andrew, I'll give this a try on 2.6 once I've got this 2.4 patch tested for Marcelo. I've got uptodate buffer_trace for 2.6 anyway, so it shouldn't be too hard. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, At 23:20 05/04/06, Stephen C. Tweedie wrote: >Yes. But it is conventional to interpret a short write as being a >failure. Returning less bytes than were requested in the write >indicates that the rest failed. It just doesn't give the exact nature >of the failure (EIO vs ENOSPC etc.) For regular files, a short write is >never permitted unless there are errors of some description. When commit_write() FULLY succeed (requested bytes == returned bytes) but generic_osync_inode() return error due to I/O failure, current do_generic_file_write() cannot return error. I encountered above situation a lot under an I/O trouble condition . In ver 2.6.11, the return value of generic_osync_inode() is returned directry to user when I/O failure occur. 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 11:01, Hifumi Hisashi wrote: > 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. Thanks --- it certainly looks like this should fix the dereferencing of invalid bh's, and this code mirrors what 2.6 does around that area. However, 2.6 is suspected of still having leaks in ext3. To be certain that we're not just backporting one of those to 2.4, we need to understand who exactly is going to clean up these bh's if they are in fact unused once we complete this code and do the final put_bh(). I'll give this patch a spin with Andrew's fsx-based leak stress and see if anything unpleasant appears. Cheers, Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 11:01, Hifumi Hisashi wrote: > >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. > 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 . Yes. But it is conventional to interpret a short write as being a failure. Returning less bytes than were requested in the write indicates that the rest failed. It just doesn't give the exact nature of the failure (EIO vs ENOSPC etc.) For regular files, a short write is never permitted unless there are errors of some description. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
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.0 +0900
+++ 2.4.30-rc3_patch/fs/jbd/commit.c2005-04-06 17:18:49.0 +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(_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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
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.0 +0900
+++ 2.4.30-rc3_patch/fs/jbd/commit.c2005-04-06 17:18:49.0 +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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 11:01, Hifumi Hisashi wrote: 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. 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 . Yes. But it is conventional to interpret a short write as being a failure. Returning less bytes than were requested in the write indicates that the rest failed. It just doesn't give the exact nature of the failure (EIO vs ENOSPC etc.) For regular files, a short write is never permitted unless there are errors of some description. --Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-04-06 at 11:01, Hifumi Hisashi wrote: 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. Thanks --- it certainly looks like this should fix the dereferencing of invalid bh's, and this code mirrors what 2.6 does around that area. However, 2.6 is suspected of still having leaks in ext3. To be certain that we're not just backporting one of those to 2.4, we need to understand who exactly is going to clean up these bh's if they are in fact unused once we complete this code and do the final put_bh(). I'll give this patch a spin with Andrew's fsx-based leak stress and see if anything unpleasant appears. Cheers, Stephen - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, At 23:20 05/04/06, Stephen C. Tweedie wrote: Yes. But it is conventional to interpret a short write as being a failure. Returning less bytes than were requested in the write indicates that the rest failed. It just doesn't give the exact nature of the failure (EIO vs ENOSPC etc.) For regular files, a short write is never permitted unless there are errors of some description. When commit_write() FULLY succeed (requested bytes == returned bytes) but generic_osync_inode() return error due to I/O failure, current do_generic_file_write() cannot return error. I encountered above situation a lot under an I/O trouble condition . In ver 2.6.11, the return value of generic_osync_inode() is returned directry to user when I/O failure occur. 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-03-30 at 12:59, Marcelo Tosatti wrote: > > I'm not certain that this is right, but it seems possible and would > > explain the symptoms. Maybe Stephen or Andrew could comments? > > Andrew, Stephen? 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. > > 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 - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
Hi, On Wed, 2005-03-30 at 12:59, Marcelo Tosatti wrote: I'm not certain that this is right, but it seems possible and would explain the symptoms. Maybe Stephen or Andrew could comments? Andrew, Stephen? 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. 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 - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
On Wed, Mar 30, 2005 at 02:06:39PM +1000, Neil Brown wrote: > On Tuesday March 29, [EMAIL PROTECTED] wrote: > > > > Attached is the backout patch, for convenience. > > Thanks. I had another look, and think I may be able to see the > problem. If I'm right, it is a problem with this patch. > > > diff -Nru a/fs/jbd/commit.c b/fs/jbd/commit.c > > --- a/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 > > +++ b/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 > > @@ -92,7 +92,7 @@ > > struct buffer_head *wbuf[64]; > > int bufs; > > int flags; > > - int err = 0; > > + int err; > > unsigned long blocknr; > > char *tagp = NULL; > > journal_header_t *header; > > @@ -299,8 +299,6 @@ > > spin_unlock(_datalist_lock); > > unlock_journal(journal); > > wait_on_buffer(bh); > > - if (unlikely(!buffer_uptodate(bh))) > > - err = -EIO; > > /* the journal_head may have been removed now */ > > lock_journal(journal); > > goto write_out_data; > > > I think the "!buffer_update(bh)" test is not safe at this point as, > after the wait_on_buffer which could cause a schedule, the bh may > no longer exist, or be for the same block. There doesn't seem to be > any locking or refcounting that would keep it valid. > > Note the comment "the journal_head may have been removed now". > If the journal_head is gone, the associated buffer_head is likely gone > as well. Seems to be possible, yes. > I'm not certain that this is right, but it seems possible and would > explain the symptoms. Maybe Stephen or Andrew could comments? Andrew, Stephen? > > --- a/mm/filemap.c 2005-03-29 18:50:55 -03:00 > > +++ b/mm/filemap.c 2005-03-29 18:50:55 -03:00 > > @@ -3261,12 +3261,7 @@ > > status = generic_osync_inode(inode, > > OSYNC_METADATA|OSYNC_DATA); > > } > > > > - /* > > -* generic_osync_inode always returns 0 or negative value. > > -* So 'status < written' is always true, and written should > > -* be returned if status >= 0. > > -*/ > > - err = (status < 0) ? status : written; > > + err = written ? written : status; > > out: > > > > return err; > > As an aside, this looks extremely dubious to me. > > There is a loop earlier in this routine (do_generic_file_write) that > passes a piece-at-a-time of the write request to prepare_write / > commit_write. > Successes are counted in 'written'. A failure causes the loop to > abort with a status in 'status'. > > 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. - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
On Wed, Mar 30, 2005 at 02:06:39PM +1000, Neil Brown wrote: On Tuesday March 29, [EMAIL PROTECTED] wrote: Attached is the backout patch, for convenience. Thanks. I had another look, and think I may be able to see the problem. If I'm right, it is a problem with this patch. diff -Nru a/fs/jbd/commit.c b/fs/jbd/commit.c --- a/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 +++ b/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 @@ -92,7 +92,7 @@ struct buffer_head *wbuf[64]; int bufs; int flags; - int err = 0; + int err; unsigned long blocknr; char *tagp = NULL; journal_header_t *header; @@ -299,8 +299,6 @@ spin_unlock(journal_datalist_lock); unlock_journal(journal); wait_on_buffer(bh); - if (unlikely(!buffer_uptodate(bh))) - err = -EIO; /* the journal_head may have been removed now */ lock_journal(journal); goto write_out_data; I think the !buffer_update(bh) test is not safe at this point as, after the wait_on_buffer which could cause a schedule, the bh may no longer exist, or be for the same block. There doesn't seem to be any locking or refcounting that would keep it valid. Note the comment the journal_head may have been removed now. If the journal_head is gone, the associated buffer_head is likely gone as well. Seems to be possible, yes. I'm not certain that this is right, but it seems possible and would explain the symptoms. Maybe Stephen or Andrew could comments? Andrew, Stephen? --- a/mm/filemap.c 2005-03-29 18:50:55 -03:00 +++ b/mm/filemap.c 2005-03-29 18:50:55 -03:00 @@ -3261,12 +3261,7 @@ status = generic_osync_inode(inode, OSYNC_METADATA|OSYNC_DATA); } - /* -* generic_osync_inode always returns 0 or negative value. -* So 'status written' is always true, and written should -* be returned if status = 0. -*/ - err = (status 0) ? status : written; + err = written ? written : status; out: return err; As an aside, this looks extremely dubious to me. There is a loop earlier in this routine (do_generic_file_write) that passes a piece-at-a-time of the write request to prepare_write / commit_write. Successes are counted in 'written'. A failure causes the loop to abort with a status in 'status'. 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. - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
On Tuesday March 29, [EMAIL PROTECTED] wrote: > > Attached is the backout patch, for convenience. Thanks. I had another look, and think I may be able to see the problem. If I'm right, it is a problem with this patch. > diff -Nru a/fs/jbd/commit.c b/fs/jbd/commit.c > --- a/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 > +++ b/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 > @@ -92,7 +92,7 @@ > struct buffer_head *wbuf[64]; > int bufs; > int flags; > - int err = 0; > + int err; > unsigned long blocknr; > char *tagp = NULL; > journal_header_t *header; > @@ -299,8 +299,6 @@ > spin_unlock(_datalist_lock); > unlock_journal(journal); > wait_on_buffer(bh); > - if (unlikely(!buffer_uptodate(bh))) > - err = -EIO; > /* the journal_head may have been removed now */ > lock_journal(journal); > goto write_out_data; I think the "!buffer_update(bh)" test is not safe at this point as, after the wait_on_buffer which could cause a schedule, the bh may no longer exist, or be for the same block. There doesn't seem to be any locking or refcounting that would keep it valid. Note the comment "the journal_head may have been removed now". If the journal_head is gone, the associated buffer_head is likely gone as well. I'm not certain that this is right, but it seems possible and would explain the symptoms. Maybe Stephen or Andrew could comments? > --- a/mm/filemap.c2005-03-29 18:50:55 -03:00 > +++ b/mm/filemap.c2005-03-29 18:50:55 -03:00 > @@ -3261,12 +3261,7 @@ > status = generic_osync_inode(inode, > OSYNC_METADATA|OSYNC_DATA); > } > > - /* > - * generic_osync_inode always returns 0 or negative value. > - * So 'status < written' is always true, and written should > - * be returned if status >= 0. > - */ > - err = (status < 0) ? status : written; > + err = written ? written : status; > out: > > return err; As an aside, this looks extremely dubious to me. There is a loop earlier in this routine (do_generic_file_write) that passes a piece-at-a-time of the write request to prepare_write / commit_write. Successes are counted in 'written'. A failure causes the loop to abort with a status in 'status'. 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. NeilBrown - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems
On Tue, Mar 29, 2005 at 10:10:34AM +1000, Neil Brown wrote:
> On Monday March 28, [EMAIL PROTECTED] wrote:
> > On Mon, Mar 28, 2005 at 10:34:05AM +0300, [Ville Herva] wrote:
> > >
> > > I just upgraded from linux-2.4.21 + vserser 0.17 to 2.4.30rc3 + vserver
> > > 1.2.10. The box has been running stable with 2.4.21 + vserver 0.17/0.16
> > > for
> > > a few years (uptime before reboot was nearly 400 days.)
> > >
> > > The boot went fine, but after few hours I got
> > > Message from [EMAIL PROTECTED] at Sun Mar 27 22:07:00 2005 ...
> > > kernel: journal commit I/O error
>
> I got that error on 2.4.30-rc1 a couple of times, and now cannot
> reproduce it :-(
> But if you got it too, then it wasn't just bad luck.
>
> The ext3 code in 2.4.30-rc does have a few more checks for IO errors
> which will cause the journal to be aborted and produce this error, so
> I suspect that change which caused the problem is a change in ext3.
> However that doesn't mean the bug is there.
>
> The extra code in ext3 seems to just check if buffer_uptodate is false
> after it has waited on a locked buffer, and triggers a journal abort
> if it isn't. This should be perfectly safe, and I cannot find any
> logic error near by. But nor can I find any errors that would cause a
> buffer returned from raid1 to not be uptodate (unless there really was
> an IO error).
Attached is the backout patch, for convenience.
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/03/29 18:49:25-03:00 [EMAIL PROTECTED]
# Cset exclude: [EMAIL PROTECTED]|ChangeSet|20050226095914|25750
#
# mm/filemap.c
# 2005/03/29 18:49:22-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# include/linux/jbd.h
# 2005/03/29 18:49:22-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/jbd/transaction.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/jbd/journal.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/jbd/commit.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/ext3/super.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/ext3/fsync.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
diff -Nru a/fs/ext3/fsync.c b/fs/ext3/fsync.c
--- a/fs/ext3/fsync.c 2005-03-29 18:50:56 -03:00
+++ b/fs/ext3/fsync.c 2005-03-29 18:50:56 -03:00
@@ -69,7 +69,7 @@
if (test_opt(inode->i_sb, DATA_FLAGS) == EXT3_MOUNT_WRITEBACK_DATA)
ret |= fsync_inode_data_buffers(inode);
- ret |= ext3_force_commit(inode->i_sb);
+ ext3_force_commit(inode->i_sb);
return ret;
}
diff -Nru a/fs/ext3/super.c b/fs/ext3/super.c
--- a/fs/ext3/super.c 2005-03-29 18:50:56 -03:00
+++ b/fs/ext3/super.c 2005-03-29 18:50:56 -03:00
@@ -1608,13 +1608,12 @@
static int ext3_sync_fs(struct super_block *sb)
{
- int err;
tid_t target;
sb->s_dirt = 0;
target = log_start_commit(EXT3_SB(sb)->s_journal, NULL);
- err = log_wait_commit(EXT3_SB(sb)->s_journal, target);
- return err;
+ log_wait_commit(EXT3_SB(sb)->s_journal, target);
+ return 0;
}
/*
diff -Nru a/fs/jbd/commit.c b/fs/jbd/commit.c
--- a/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00
+++ b/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00
@@ -92,7 +92,7 @@
struct buffer_head *wbuf[64];
int bufs;
int flags;
- int err = 0;
+ int err;
unsigned long blocknr;
char *tagp = NULL;
journal_header_t *header;
@@ -299,8 +299,6 @@
spin_unlock(_datalist_lock);
unlock_journal(journal);
wait_on_buffer(bh);
- if (unlikely(!buffer_uptodate(bh)))
- err = -EIO;
/* the journal_head may have been removed now */
lock_journal(journal);
goto write_out_data;
@@ -328,8 +326,6 @@
spin_unlock(_datalist_lock);
unlock_journal(journal);
wait_on_buffer(bh);
- if (unlikely(!buffer_uptodate(bh)))
- err = -EIO;
lock_journal(journal);
spin_lock(_datalist_lock);
continue; /* List may have changed */
@@ -355,9 +351,6 @@
}
spin_unlock(_datalist_lock);
- if (err)
- __journal_abort_hard(journal);
-
/*
* If we found any dirty or locked buffers, then we should have
* looped back up to the write_out_data label. If there weren't
@@ -548,8 +541,6 @@
if (buffer_locked(bh)) {
unlock_journal(journal);
wait_on_buffer(bh);
- if (unlikely(!buffer_uptodate(bh)))
- err = -EIO;
lock_journal(journal);
Re: Linux 2.4.30-rc3 md/ext3 problems
On Tue, Mar 29, 2005 at 10:10:34AM +1000, Neil Brown wrote:
On Monday March 28, [EMAIL PROTECTED] wrote:
On Mon, Mar 28, 2005 at 10:34:05AM +0300, [Ville Herva] wrote:
I just upgraded from linux-2.4.21 + vserser 0.17 to 2.4.30rc3 + vserver
1.2.10. The box has been running stable with 2.4.21 + vserver 0.17/0.16
for
a few years (uptime before reboot was nearly 400 days.)
The boot went fine, but after few hours I got
Message from [EMAIL PROTECTED] at Sun Mar 27 22:07:00 2005 ...
kernel: journal commit I/O error
I got that error on 2.4.30-rc1 a couple of times, and now cannot
reproduce it :-(
But if you got it too, then it wasn't just bad luck.
The ext3 code in 2.4.30-rc does have a few more checks for IO errors
which will cause the journal to be aborted and produce this error, so
I suspect that change which caused the problem is a change in ext3.
However that doesn't mean the bug is there.
The extra code in ext3 seems to just check if buffer_uptodate is false
after it has waited on a locked buffer, and triggers a journal abort
if it isn't. This should be perfectly safe, and I cannot find any
logic error near by. But nor can I find any errors that would cause a
buffer returned from raid1 to not be uptodate (unless there really was
an IO error).
Attached is the backout patch, for convenience.
# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
# 2005/03/29 18:49:25-03:00 [EMAIL PROTECTED]
# Cset exclude: [EMAIL PROTECTED]|ChangeSet|20050226095914|25750
#
# mm/filemap.c
# 2005/03/29 18:49:22-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# include/linux/jbd.h
# 2005/03/29 18:49:22-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/jbd/transaction.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/jbd/journal.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/jbd/commit.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/ext3/super.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
# fs/ext3/fsync.c
# 2005/03/29 18:49:21-03:00 [EMAIL PROTECTED] +0 -0
# Exclude
#
diff -Nru a/fs/ext3/fsync.c b/fs/ext3/fsync.c
--- a/fs/ext3/fsync.c 2005-03-29 18:50:56 -03:00
+++ b/fs/ext3/fsync.c 2005-03-29 18:50:56 -03:00
@@ -69,7 +69,7 @@
if (test_opt(inode-i_sb, DATA_FLAGS) == EXT3_MOUNT_WRITEBACK_DATA)
ret |= fsync_inode_data_buffers(inode);
- ret |= ext3_force_commit(inode-i_sb);
+ ext3_force_commit(inode-i_sb);
return ret;
}
diff -Nru a/fs/ext3/super.c b/fs/ext3/super.c
--- a/fs/ext3/super.c 2005-03-29 18:50:56 -03:00
+++ b/fs/ext3/super.c 2005-03-29 18:50:56 -03:00
@@ -1608,13 +1608,12 @@
static int ext3_sync_fs(struct super_block *sb)
{
- int err;
tid_t target;
sb-s_dirt = 0;
target = log_start_commit(EXT3_SB(sb)-s_journal, NULL);
- err = log_wait_commit(EXT3_SB(sb)-s_journal, target);
- return err;
+ log_wait_commit(EXT3_SB(sb)-s_journal, target);
+ return 0;
}
/*
diff -Nru a/fs/jbd/commit.c b/fs/jbd/commit.c
--- a/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00
+++ b/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00
@@ -92,7 +92,7 @@
struct buffer_head *wbuf[64];
int bufs;
int flags;
- int err = 0;
+ int err;
unsigned long blocknr;
char *tagp = NULL;
journal_header_t *header;
@@ -299,8 +299,6 @@
spin_unlock(journal_datalist_lock);
unlock_journal(journal);
wait_on_buffer(bh);
- if (unlikely(!buffer_uptodate(bh)))
- err = -EIO;
/* the journal_head may have been removed now */
lock_journal(journal);
goto write_out_data;
@@ -328,8 +326,6 @@
spin_unlock(journal_datalist_lock);
unlock_journal(journal);
wait_on_buffer(bh);
- if (unlikely(!buffer_uptodate(bh)))
- err = -EIO;
lock_journal(journal);
spin_lock(journal_datalist_lock);
continue; /* List may have changed */
@@ -355,9 +351,6 @@
}
spin_unlock(journal_datalist_lock);
- if (err)
- __journal_abort_hard(journal);
-
/*
* If we found any dirty or locked buffers, then we should have
* looped back up to the write_out_data label. If there weren't
@@ -548,8 +541,6 @@
if (buffer_locked(bh)) {
unlock_journal(journal);
wait_on_buffer(bh);
- if (unlikely(!buffer_uptodate(bh)))
- err = -EIO;
lock_journal(journal);
goto
Re: Linux 2.4.30-rc3 md/ext3 problems (ext3 gurus : please check)
On Tuesday March 29, [EMAIL PROTECTED] wrote: Attached is the backout patch, for convenience. Thanks. I had another look, and think I may be able to see the problem. If I'm right, it is a problem with this patch. diff -Nru a/fs/jbd/commit.c b/fs/jbd/commit.c --- a/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 +++ b/fs/jbd/commit.c 2005-03-29 18:50:55 -03:00 @@ -92,7 +92,7 @@ struct buffer_head *wbuf[64]; int bufs; int flags; - int err = 0; + int err; unsigned long blocknr; char *tagp = NULL; journal_header_t *header; @@ -299,8 +299,6 @@ spin_unlock(journal_datalist_lock); unlock_journal(journal); wait_on_buffer(bh); - if (unlikely(!buffer_uptodate(bh))) - err = -EIO; /* the journal_head may have been removed now */ lock_journal(journal); goto write_out_data; I think the !buffer_update(bh) test is not safe at this point as, after the wait_on_buffer which could cause a schedule, the bh may no longer exist, or be for the same block. There doesn't seem to be any locking or refcounting that would keep it valid. Note the comment the journal_head may have been removed now. If the journal_head is gone, the associated buffer_head is likely gone as well. I'm not certain that this is right, but it seems possible and would explain the symptoms. Maybe Stephen or Andrew could comments? --- a/mm/filemap.c2005-03-29 18:50:55 -03:00 +++ b/mm/filemap.c2005-03-29 18:50:55 -03:00 @@ -3261,12 +3261,7 @@ status = generic_osync_inode(inode, OSYNC_METADATA|OSYNC_DATA); } - /* - * generic_osync_inode always returns 0 or negative value. - * So 'status written' is always true, and written should - * be returned if status = 0. - */ - err = (status 0) ? status : written; + err = written ? written : status; out: return err; As an aside, this looks extremely dubious to me. There is a loop earlier in this routine (do_generic_file_write) that passes a piece-at-a-time of the write request to prepare_write / commit_write. Successes are counted in 'written'. A failure causes the loop to abort with a status in 'status'. 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. NeilBrown - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems
On Monday March 28, [EMAIL PROTECTED] wrote: > On Mon, Mar 28, 2005 at 10:34:05AM +0300, [Ville Herva] wrote: > > > > I just upgraded from linux-2.4.21 + vserser 0.17 to 2.4.30rc3 + vserver > > 1.2.10. The box has been running stable with 2.4.21 + vserver 0.17/0.16 for > > a few years (uptime before reboot was nearly 400 days.) > > > > The boot went fine, but after few hours I got > > Message from [EMAIL PROTECTED] at Sun Mar 27 22:07:00 2005 ... > > kernel: journal commit I/O error I got that error on 2.4.30-rc1 a couple of times, and now cannot reproduce it :-( But if you got it too, then it wasn't just bad luck. The ext3 code in 2.4.30-rc does have a few more checks for IO errors which will cause the journal to be aborted and produce this error, so I suspect that change which caused the problem is a change in ext3. However that doesn't mean the bug is there. The extra code in ext3 seems to just check if buffer_uptodate is false after it has waited on a locked buffer, and triggers a journal abort if it isn't. This should be perfectly safe, and I cannot find any logic error near by. But nor can I find any errors that would cause a buffer returned from raid1 to not be uptodate (unless there really was an IO error). NeilBrown - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems
On Mon, Mar 28, 2005 at 07:25:58PM +0200, you [Willy Tarreau] wrote: > > Since you don't seem to be willing to remove vserver, I guess you really > need it on this machine, and to be honnest, Yes, the machine is in production, and for that it, it needs vserver. The fact it is in production also makes it a tad hankala awkward to test different options, at least the most experimental ones. > I too don't see what trouble it could cause in this area. Neither do I. Of course it could be memory corruption caused by vserver in different part of kernel, but the fact that the symptoms are so consistent, make me think that is unlikely. But not impossible. > However, could you try removing the journal, or simply mount the FS as > ext2 ? It would help to narrow the problem down. That is a good idea, if only I could boot the box at will and reliably reproduce the problem. While it took less than ten minutes to trigger it the first time, it took more than five hours the first time. I will try to reproduce the problem on different setup first, and if I can do that, I'll try your suggestion. > To resume, you have your root on ext3 on top of soft raid1 consisting in > two IDE disks, which works in 2.4.21 but not on 2.4.30-rc3, that's > correct ? Correct. This is PII 266MHz, no SMP. > There was a fix last week by Neil Brown about RAID1 rebuild process > (degraded array of 3 disks, etc...), unless it obviously does not come > from there, you might want to try reverting it first ? Sounds sane, although the raid array was not in degraded state at any stage and no raid rebuild aver triggered. > The next one is from Doug Ledford on 2004/09/18 and should only affect > SMP. Ok, as said, this is UP. > My different raid machines run either reiserfs or xfs on soft raid5 on > top of scsi and with kernel 2.4.27, so there's not much to compare... > Perhaps someone on the list has a setup similar to yours and could test > the kernel ? I will try to contruct a similar setup on another machine. thanks for your insights, -- v -- [EMAIL PROTECTED] - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems
Hi Ville,
On Mon, Mar 28, 2005 at 07:55:01PM +0300, Ville Herva wrote:
(...)
> I rebooted (fsck took the fs errors away, no big offenders), and after a few
> minutes, I got the same error ("journal commit I/O error"). So it doesn't
> appear all that random memory corruption. The error happened right when I
> logged out, but that might have been a coincidence. No ide nor md errors
> this time either.
>
> I don't know what to suspect. What I gather from changelogs, there haven't
> been any critical looking ext3 changes in 2.4 lately, but then again,
> vserver doesn't mess with block layer / ext3 journalling either.
>
> Any ideas?
Since you don't seem to be willing to remove vserver, I guess you really
need it on this machine, and to be honnest, I too don't see what trouble
it could cause in this area. However, could you try removing the journal,
or simply mount the FS as ext2 ? It would help to narrow the problem down.
To resume, you have your root on ext3 on top of soft raid1 consisting in
two IDE disks, which works in 2.4.21 but not on 2.4.30-rc3, that's
correct ? There was a fix last week by Neil Brown about RAID1 rebuild
process (degraded array of 3 disks, etc...), unless it obviously does
not come from there, you might want to try reverting it first ? The
next one is from Doug Ledford on 2004/09/18 and should only affect SMP.
My different raid machines run either reiserfs or xfs on soft raid5 on
top of scsi and with kernel 2.4.27, so there's not much to compare...
Perhaps someone on the list has a setup similar to yours and could test
the kernel ?
Cheers,
Willy
-
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/
Re: Linux 2.4.30-rc3 md/ext3 problems
Hi Ville, On Mon, Mar 28, 2005 at 07:55:01PM +0300, Ville Herva wrote: (...) I rebooted (fsck took the fs errors away, no big offenders), and after a few minutes, I got the same error (journal commit I/O error). So it doesn't appear all that random memory corruption. The error happened right when I logged out, but that might have been a coincidence. No ide nor md errors this time either. I don't know what to suspect. What I gather from changelogs, there haven't been any critical looking ext3 changes in 2.4 lately, but then again, vserver doesn't mess with block layer / ext3 journalling either. Any ideas? Since you don't seem to be willing to remove vserver, I guess you really need it on this machine, and to be honnest, I too don't see what trouble it could cause in this area. However, could you try removing the journal, or simply mount the FS as ext2 ? It would help to narrow the problem down. To resume, you have your root on ext3 on top of soft raid1 consisting in two IDE disks, which works in 2.4.21 but not on 2.4.30-rc3, that's correct ? There was a fix last week by Neil Brown about RAID1 rebuild process (degraded array of 3 disks, etc...), unless it obviously does not come from there, you might want to try reverting it first ? The next one is from Doug Ledford on 2004/09/18 and should only affect SMP. My different raid machines run either reiserfs or xfs on soft raid5 on top of scsi and with kernel 2.4.27, so there's not much to compare... Perhaps someone on the list has a setup similar to yours and could test the kernel ? Cheers, Willy - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems
On Mon, Mar 28, 2005 at 07:25:58PM +0200, you [Willy Tarreau] wrote: Since you don't seem to be willing to remove vserver, I guess you really need it on this machine, and to be honnest, Yes, the machine is in production, and for that it, it needs vserver. The fact it is in production also makes it a tad hankala awkward to test different options, at least the most experimental ones. I too don't see what trouble it could cause in this area. Neither do I. Of course it could be memory corruption caused by vserver in different part of kernel, but the fact that the symptoms are so consistent, make me think that is unlikely. But not impossible. However, could you try removing the journal, or simply mount the FS as ext2 ? It would help to narrow the problem down. That is a good idea, if only I could boot the box at will and reliably reproduce the problem. While it took less than ten minutes to trigger it the first time, it took more than five hours the first time. I will try to reproduce the problem on different setup first, and if I can do that, I'll try your suggestion. To resume, you have your root on ext3 on top of soft raid1 consisting in two IDE disks, which works in 2.4.21 but not on 2.4.30-rc3, that's correct ? Correct. This is PII 266MHz, no SMP. There was a fix last week by Neil Brown about RAID1 rebuild process (degraded array of 3 disks, etc...), unless it obviously does not come from there, you might want to try reverting it first ? Sounds sane, although the raid array was not in degraded state at any stage and no raid rebuild aver triggered. The next one is from Doug Ledford on 2004/09/18 and should only affect SMP. Ok, as said, this is UP. My different raid machines run either reiserfs or xfs on soft raid5 on top of scsi and with kernel 2.4.27, so there's not much to compare... Perhaps someone on the list has a setup similar to yours and could test the kernel ? I will try to contruct a similar setup on another machine. thanks for your insights, -- v -- [EMAIL PROTECTED] - 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/
Re: Linux 2.4.30-rc3 md/ext3 problems
On Monday March 28, [EMAIL PROTECTED] wrote: On Mon, Mar 28, 2005 at 10:34:05AM +0300, [Ville Herva] wrote: I just upgraded from linux-2.4.21 + vserser 0.17 to 2.4.30rc3 + vserver 1.2.10. The box has been running stable with 2.4.21 + vserver 0.17/0.16 for a few years (uptime before reboot was nearly 400 days.) The boot went fine, but after few hours I got Message from [EMAIL PROTECTED] at Sun Mar 27 22:07:00 2005 ... kernel: journal commit I/O error I got that error on 2.4.30-rc1 a couple of times, and now cannot reproduce it :-( But if you got it too, then it wasn't just bad luck. The ext3 code in 2.4.30-rc does have a few more checks for IO errors which will cause the journal to be aborted and produce this error, so I suspect that change which caused the problem is a change in ext3. However that doesn't mean the bug is there. The extra code in ext3 seems to just check if buffer_uptodate is false after it has waited on a locked buffer, and triggers a journal abort if it isn't. This should be perfectly safe, and I cannot find any logic error near by. But nor can I find any errors that would cause a buffer returned from raid1 to not be uptodate (unless there really was an IO error). NeilBrown - 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/
Re: Linux 2.4.30-rc3
On Sat, Mar 26, 2005 at 01:28:01PM -0300, you [Marcelo Tosatti] wrote: > > Hi, > > Here goes -rc3. > > A nasty typo happened while merging v2.6 load_elf_library() DoS fix, > which could leap to oopses. > > Summary of changes from v2.4.30-rc2 to v2.4.30-rc3 > > > Marcelo Tosatti: > o Andreas Arens: Fix deadly mismerge of binfmt_elf DoS fix > o Change VERSION to 2.4.30-rc3 I just upgraded from linux-2.4.21 + vserser 0.17 to 2.4.30rc3 + vserver 1.2.10. The box has been running stable with 2.4.21 + vserver 0.17/0.16 for a few years (uptime before reboot was nearly 400 days.) The boot went fine, but after few hours I got Message from [EMAIL PROTECTED] at Sun Mar 27 22:07:00 2005 ... turing kernel: journal commit I/O error and dmesg is filled with --8<--- EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted --8<--- This is roofs, on top software raid1 and two ide disks. mdstat claims it's healthy: --8<--- md3 : active raid1 hdc3[1] hda3[0] 37955648 blocks [2/2] [UU] --8<--- While dmesg has filled up and /var/log/messages is read-only - I can't see all the kernel messages - there appears to be no IO errors from the underlying devices (md, ide). smartctl -a does not report errors for hda nor hdc. During reboot, fsck was run for md3, and it was clean. Now I get --8<--- Block bitmap differences: -(7800660--7801060) -(7801934--7802030) -(7802370--7802602) -(7802604--7802613) -(7802681--7802700) -(7802715--7802716) -(7802726--7802732) -(7802744--7802750)-(7802914--7802927) -(7802934--7802937) -(7802946--7802964) -(7803392--7803417) -(7805060--7808825) -(7808976--7809608) Fix? no Inode bitmap differences: -3899400 Fix? no --8<--- No errors from the badblocks part of the fsck, though. Running fsck triggers the "journal commit I/O error" messages again, and still no IO errors from either md or ide. This _could_ have something to do with the vserver patch but it doesn't appear so. Also, it doesn't immediately look like hardware problem. Any ideas? -- v -- [EMAIL PROTECTED] - 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/
Re: Linux 2.4.30-rc3
On Sat, Mar 26, 2005 at 01:28:01PM -0300, you [Marcelo Tosatti] wrote: Hi, Here goes -rc3. A nasty typo happened while merging v2.6 load_elf_library() DoS fix, which could leap to oopses. Summary of changes from v2.4.30-rc2 to v2.4.30-rc3 Marcelo Tosatti: o Andreas Arens: Fix deadly mismerge of binfmt_elf DoS fix o Change VERSION to 2.4.30-rc3 I just upgraded from linux-2.4.21 + vserser 0.17 to 2.4.30rc3 + vserver 1.2.10. The box has been running stable with 2.4.21 + vserver 0.17/0.16 for a few years (uptime before reboot was nearly 400 days.) The boot went fine, but after few hours I got Message from [EMAIL PROTECTED] at Sun Mar 27 22:07:00 2005 ... turing kernel: journal commit I/O error and dmesg is filled with --8--- EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted EXT3-fs error (device md(9,3)) in start_transaction: Journal has aborted --8--- This is roofs, on top software raid1 and two ide disks. mdstat claims it's healthy: --8--- md3 : active raid1 hdc3[1] hda3[0] 37955648 blocks [2/2] [UU] --8--- While dmesg has filled up and /var/log/messages is read-only - I can't see all the kernel messages - there appears to be no IO errors from the underlying devices (md, ide). smartctl -a does not report errors for hda nor hdc. During reboot, fsck was run for md3, and it was clean. Now I get --8--- Block bitmap differences: -(7800660--7801060) -(7801934--7802030) -(7802370--7802602) -(7802604--7802613) -(7802681--7802700) -(7802715--7802716) -(7802726--7802732) -(7802744--7802750)-(7802914--7802927) -(7802934--7802937) -(7802946--7802964) -(7803392--7803417) -(7805060--7808825) -(7808976--7809608) Fix? no Inode bitmap differences: -3899400 Fix? no --8--- No errors from the badblocks part of the fsck, though. Running fsck triggers the journal commit I/O error messages again, and still no IO errors from either md or ide. This _could_ have something to do with the vserver patch but it doesn't appear so. Also, it doesn't immediately look like hardware problem. Any ideas? -- v -- [EMAIL PROTECTED] - 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/
