Re: buffer/swapping in pre-patch-2.0.31-2 + the 17th July patch

Bill Hawes (whawes@star.net)
Mon, 21 Jul 1997 22:29:41 -0400


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;
>