Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 21 Jul 1997 22:29:41 -0400 | From | Bill Hawes <> | Subject | Re: buffer/swapping in pre-patch-2.0.31-2 + the 17th July patch |
| |
Dr. Werner Fink wrote: > Hmmm ... interesting. Please try the appended patch. It contains a few > changes: > > * run buffer wait queue only if recover_reusable_buffer_heads() > is called in brw_page() due get_more_buffer_heads()
Werner,
From a quick scan I don't think your changes to conditionally call wake_up() will work. You want to call wake_up whenever there's a task actually waiting, but in your code the wakeup_on_io flag could get cleared even if there are tasks in the wait queue.
The correct test is to see whether anyone is waiting using waitqueue_active(), as in
restore_flags(flags); after_unlock_page(page); + if (waitqueue_active(&buffer_wait)) + wake_up(&buffer_wait);
waitqueue_active is an inline function and will be marginally faster than just calling wake_up(). But as the reason for the patch in the first place is to avoid a potential deadlock, please make sure the wake_up() gets called if there's a task waiting!
Also, your amended patch leaves out a much more important change pointed out by Mark Hemment -- after the call to try_to_free_buffer in find_candidate, there needs to be a test
if (!bh) break;
This code is in the 2.1.xx tree (and I thought it was already in 2.0.30, but overlooked it.) The absence of the above test is one of the causes of oopses in 2.0.30 ... please make sure it gets in!
Regards, Bill
> } > ++current->maj_flt; > return 0; >
| ![\](/images/icornerr.gif) |