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

Dr. Werner Fink (werner@suse.de)
Tue, 22 Jul 1997 11:59:00 +0200

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

OK ... I'm want to speed up the I/O initated by brw_page() ... without
a condition for the `wake_up' of the buffer wait queue slows down this I/O
is really slow for the case of nr of buffer heads equals zero.

This can be tested very easily: run a `time e2fsck -f' on a big partition
in a loop and count the long check cycles. With a wake_up() call you
have to run e2fsck very often to get a speed up due to the same sized buffers.

[...]

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

Hmmm ... this null pointer detection is already included in
pre-patch-2.0.31-2. My patches are always against pre-patch-2.0.31-2 because
of the fixes in the net and isdn layer.

Werner