Messages in this thread | | | Date | Fri, 26 Nov 1999 03:55:08 +0100 (CET) | From | Andrea Arcangeli <> | Subject | Re: [patch] FS (ext2) corruption generated by BLKFLSBUF/invalidate_buffers (2.2.x and 2.3.x) |
| |
On Fri, 26 Nov 1999, Guest section DW wrote:
>On Thu, Nov 25, 1999 at 07:14:05PM +0100, Andrea Arcangeli wrote: > >> Jason and Samuel told me they could reliable reproduce fs corruption by >> writing to a filesystem while the ioctl(BLKFLSBUF) is running in parallel >> on the blockdevice where the fs is mounted. > >Don't do that, then.
Well this can be an option and infact I considered it. The feedback I had so far doesn't agree with you though. Anyway my patch won't hurt you if you assume you can't do that and you don't do that.
>I hope I misunderstand your "no need to wait I/O completion". >We certainly need that when BLKFLSBUF returns (i) the I/O that >was initiated before it was called has completed, and (ii) the >buffers that were involved in that I/O are now invalid.
Of course that still happens. What I meant is that the sync_buffers() doesn't need to wait for I/O completation as the invalidate_buffers() will wait for I/O completation later. BLKFLSBUF can be seen as:
sync (no wait) invalidate (wait I/O compleatation)
>So, as long as this is not fixed, applications have to resort >to sync() and BLKFLSBUF to try to avoid such problems.
No need to do that as far as the BLKFLSBUF is implemented correctly as I pointed out above.
>A different reason for BLKFLSBUF is the desire to change media, >especially in cases where the kernel does not recognize the media >as removable.
This is still possible of course.
Basically my patch make no downside at all except that you'll avoid to trash an fs by running BLKFLSBUF on a rw mounted fs.
>I have an aesthetic comment: instead of giving invalidate_buffers() >a second argument, and changing the source in lots of places, why >don't you introduce a second routine invalidate_all_buffers() >that is called two places or so and does the invalidate-also-dirty.
Because my way more efficient (you can't better preserve the CPU cache) and more maintainable (that is the real point). The kind of loops in invalidate_buffers() are just duplicated in set_blocksize and I don't want to duplicated them a third time. Such loops are race prone and infact my patch fixes races in such loops as well. The less and the simpler loops there are the better (IMHO).
Andrea
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |