Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Sat, 9 Dec 2000, Mikulas Patocka wrote: > > I did a test. I disabled readahead except for reading all 4 buffers in > map_4sectors. > > I observed 14% slowdown on walking directories with find and 4% slowdown > on grepping one my working directory (10M, 281 files). > > If you can't make it

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Mikulas Patocka
On Sat, 9 Dec 2000, Alexander Viro wrote: > On Sat, 9 Dec 2000, Andries Brouwer wrote: > > > On Sat, Dec 09, 2000 at 05:40:47AM -0500, Alexander Viro wrote: > > > > > > @@ -1210,7 +1204,6 @@ > > > [breada()] > > > Umm... why do we keep it, in the first place? AFAICS the only > > > in-tree

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Andi Kleen
Linus Torvalds <[EMAIL PROTECTED]> writes: > > Modulo the comments above - fine with me. However, there is stuff in > > drivers/md that I don't understand. Ingo, could you comment on the use of > > ->b_end_io there? > > I already sent him mail about it for the same reason. How about

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Sat, 9 Dec 2000, Alexander Viro wrote: > Fine > > - atomic_inc(>b_count); > > Why? It's cleaner the old way - why bother postponing that until we > lock the thing? I had a rule: we always do the "lock_buffer()" and "b_count increment" together with setting "b_end_io =

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread David Woodhouse
On Fri, 8 Dec 2000, Alexander Viro wrote: > BTW, what do you think about the following: > * add_to_page_cache() is not exported and never used. Kill? I have my eye on that for execute-in-place of romfs from real ROM chips - making up struct pages for parts of the ROM chips and inserting

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread David S. Miller
Date:Sat, 9 Dec 2000 00:45:51 -0800 (PST) From: Linus Torvalds <[EMAIL PROTECTED]> out: -if (nr) { -ll_rw_block(WRITE, nr, arr); -} else { -UnlockPage(page); -} +UnlockPage(page); ClearPageUptodate(page);

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Alexander Viro
On Sat, 9 Dec 2000, Andries Brouwer wrote: > On Sat, Dec 09, 2000 at 05:40:47AM -0500, Alexander Viro wrote: > > > > @@ -1210,7 +1204,6 @@ > > [breada()] > > Umm... why do we keep it, in the first place? AFAICS the only > > in-tree user is hpfs_map_sector() and it doesn't look like we

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Andries Brouwer
On Sat, Dec 09, 2000 at 05:40:47AM -0500, Alexander Viro wrote: > > @@ -1210,7 +1204,6 @@ > [breada()] > Umm... why do we keep it, in the first place? AFAICS the only > in-tree user is hpfs_map_sector() and it doesn't look like we really > need it there. OTOH, trimming the buffer.c down is

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Alexander Viro
On Sat, 9 Dec 2000, Linus Torvalds wrote: > This is a preliminary patch that I have not compiled and probably breaks, > but you get the idea. I'm going to sleep, to survive another night with > three small kids. > > If somebody wants to run with this, please try it out, but realize that >

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Sat, 9 Dec 2000, Linus Torvalds wrote: > > This is a preliminary patch that I have not compiled and probably breaks, > but you get the idea. I'm going to sleep, to survive another night with > three small kids. Call me stupid [ Chorus: "You're stupid, Linus" ], but I actually compiled and

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: > On Fri, 8 Dec 2000, Linus Torvalds wrote: > > > Looking more at this issue, I suspect that the easiest pretty solution > > that everybody can probably agree is reasonable is to either pass down the > > end-of-io callback to ll_rw_block as you

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: On Fri, 8 Dec 2000, Linus Torvalds wrote: Looking more at this issue, I suspect that the easiest pretty solution that everybody can probably agree is reasonable is to either pass down the end-of-io callback to ll_rw_block as you suggested, or,

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Sat, 9 Dec 2000, Linus Torvalds wrote: This is a preliminary patch that I have not compiled and probably breaks, but you get the idea. I'm going to sleep, to survive another night with three small kids. Call me stupid [ Chorus: "You're stupid, Linus" ], but I actually compiled and

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Alexander Viro
On Sat, 9 Dec 2000, Linus Torvalds wrote: This is a preliminary patch that I have not compiled and probably breaks, but you get the idea. I'm going to sleep, to survive another night with three small kids. If somebody wants to run with this, please try it out, but realize that it's a

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Andries Brouwer
On Sat, Dec 09, 2000 at 05:40:47AM -0500, Alexander Viro wrote: @@ -1210,7 +1204,6 @@ [breada()] Umm... why do we keep it, in the first place? AFAICS the only in-tree user is hpfs_map_sector() and it doesn't look like we really need it there. OTOH, trimming the buffer.c down is

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Alexander Viro
On Sat, 9 Dec 2000, Andries Brouwer wrote: On Sat, Dec 09, 2000 at 05:40:47AM -0500, Alexander Viro wrote: @@ -1210,7 +1204,6 @@ [breada()] Umm... why do we keep it, in the first place? AFAICS the only in-tree user is hpfs_map_sector() and it doesn't look like we really need

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread David S. Miller
Date:Sat, 9 Dec 2000 00:45:51 -0800 (PST) From: Linus Torvalds [EMAIL PROTECTED] out: -if (nr) { -ll_rw_block(WRITE, nr, arr); -} else { -UnlockPage(page); -} +UnlockPage(page); ClearPageUptodate(page);

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread David Woodhouse
On Fri, 8 Dec 2000, Alexander Viro wrote: BTW, what do you think about the following: * add_to_page_cache() is not exported and never used. Kill? I have my eye on that for execute-in-place of romfs from real ROM chips - making up struct pages for parts of the ROM chips and inserting

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Sat, 9 Dec 2000, Alexander Viro wrote: Fine - atomic_inc(bh-b_count); Why? It's cleaner the old way - why bother postponing that until we lock the thing? I had a rule: we always do the "lock_buffer()" and "b_count increment" together with setting "b_end_io =

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Andi Kleen
Linus Torvalds [EMAIL PROTECTED] writes: Modulo the comments above - fine with me. However, there is stuff in drivers/md that I don't understand. Ingo, could you comment on the use of -b_end_io there? I already sent him mail about it for the same reason. How about sending mail to

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Mikulas Patocka
On Sat, 9 Dec 2000, Alexander Viro wrote: On Sat, 9 Dec 2000, Andries Brouwer wrote: On Sat, Dec 09, 2000 at 05:40:47AM -0500, Alexander Viro wrote: @@ -1210,7 +1204,6 @@ [breada()] Umm... why do we keep it, in the first place? AFAICS the only in-tree user is

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-09 Thread Linus Torvalds
On Sat, 9 Dec 2000, Mikulas Patocka wrote: I did a test. I disabled readahead except for reading all 4 buffers in map_4sectors. I observed 14% slowdown on walking directories with find and 4% slowdown on grepping one my working directory (10M, 281 files). If you can't make it

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Alexander Viro
On Fri, 8 Dec 2000, Linus Torvalds wrote: > Looking more at this issue, I suspect that the easiest pretty solution > that everybody can probably agree is reasonable is to either pass down the > end-of-io callback to ll_rw_block as you suggested, or, preferably by just > forcing the _caller_ to

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: > > I'm quite aware of that fact ;-) However, you said > >On the other hand, I have this suspicion that there is an even simpler >solution: stop using the end_buffer_io_sync version for writes >altogether. > > If that happens (i.e. if

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Alexander Viro
On Fri, 8 Dec 2000, Linus Torvalds wrote: > > > On Fri, 8 Dec 2000, Alexander Viro wrote: > > > > Erm... So you want to make ->commit_write() page-unlocking? Fine with me, > > but that will make for somewhat bigger patch. Hey, _you_ are in position > > to change the locking rules, freeze or

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Rajagopal Ananthanarayanan
Linus Torvalds wrote: > > On Fri, 8 Dec 2000, Daniel Phillips wrote: > > > > [ flush-buffers taking the page lock ] > > > > This is great when you have buffersize==pagesize. When there are > > multiple buffers per page it means that some of the buffers might have > > to wait for flushing just

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Daniel Phillips wrote: > > [ flush-buffers taking the page lock ] > > This is great when you have buffersize==pagesize. When there are > multiple buffers per page it means that some of the buffers might have > to wait for flushing just because bdflush started IO on some

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Daniel Phillips
On Fri, 08 Dec 2000, Linus Torvalds wrote: > Btw, I also think that the dirty buffer flushing should get the page lock. > Right now it touches the buffer list without holding the lock on the page > that the buffer is on, which means that there is really nothign that > prevents it from racing with

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: > > Erm... So you want to make ->commit_write() page-unlocking? Fine with me, > but that will make for somewhat bigger patch. Hey, _you_ are in position > to change the locking rules, freeze or not, so if it's OK with you... No. Read the code a bit

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: > > Fix: postpone changing ->b_end_io until the call of ll_rw_block(); if by > the time of ll_rw_block() some fragments will still have IO in progress - > wait on them. > > Comments? Yes. On the other hand, I have this suspicion that there is an

[PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Alexander Viro
Folks, see if the following patch helps. AFAICS it closes a pretty real race - we could call block_write_full_page() for a page that has sync IO in progress and blindly change ->b_end_io callbacks on the bh with pending requests. With a little bit of bad luck they would complete before we got to

[PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Alexander Viro
Folks, see if the following patch helps. AFAICS it closes a pretty real race - we could call block_write_full_page() for a page that has sync IO in progress and blindly change -b_end_io callbacks on the bh with pending requests. With a little bit of bad luck they would complete before we got to

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: Fix: postpone changing -b_end_io until the call of ll_rw_block(); if by the time of ll_rw_block() some fragments will still have IO in progress - wait on them. Comments? Yes. On the other hand, I have this suspicion that there is an even

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: Erm... So you want to make -commit_write() page-unlocking? Fine with me, but that will make for somewhat bigger patch. Hey, _you_ are in position to change the locking rules, freeze or not, so if it's OK with you... No. Read the code a bit more.

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Daniel Phillips
On Fri, 08 Dec 2000, Linus Torvalds wrote: Btw, I also think that the dirty buffer flushing should get the page lock. Right now it touches the buffer list without holding the lock on the page that the buffer is on, which means that there is really nothign that prevents it from racing with the

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Daniel Phillips wrote: [ flush-buffers taking the page lock ] This is great when you have buffersize==pagesize. When there are multiple buffers per page it means that some of the buffers might have to wait for flushing just because bdflush started IO on some other

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Alexander Viro
On Fri, 8 Dec 2000, Linus Torvalds wrote: On Fri, 8 Dec 2000, Alexander Viro wrote: Erm... So you want to make -commit_write() page-unlocking? Fine with me, but that will make for somewhat bigger patch. Hey, _you_ are in position to change the locking rules, freeze or not, so if

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Linus Torvalds
On Fri, 8 Dec 2000, Alexander Viro wrote: I'm quite aware of that fact ;-) However, you said On the other hand, I have this suspicion that there is an even simpler solution: stop using the end_buffer_io_sync version for writes altogether. If that happens (i.e. if write

Re: [PATCH] Re: kernel BUG at buffer.c:827 in test12-pre6 and 7

2000-12-08 Thread Alexander Viro
On Fri, 8 Dec 2000, Linus Torvalds wrote: Looking more at this issue, I suspect that the easiest pretty solution that everybody can probably agree is reasonable is to either pass down the end-of-io callback to ll_rw_block as you suggested, or, preferably by just forcing the _caller_ to do