Hi,
The following patch makes lock_buffer() use the exclusive wakeup scheme
added in 2.3.
Against 2.4.2pre4.
--- linux/fs/buffer.c.orig Mon Feb 19 23:13:08 2001
+++ linux/fs/buffer.c Mon Feb 19 23:16:26 2001
@@ -142,13 +142,18 @@
* if 'b_wait' is set before calling this, so that the queues aren't set
* up unnecessarily.
*/
-void __wait_on_buffer(struct buffer_head * bh)
+void __wait_on_buffer(struct buffer_head * bh, int exclusive)
{
struct task_struct *tsk = current;
DECLARE_WAITQUEUE(wait, tsk);
atomic_inc(&bh->b_count);
- add_wait_queue(&bh->b_wait, &wait);
+
+ if (exclusive)
+ add_wait_queue_exclusive(&bh->b_wait, &wait);
+ else
+ add_wait_queue(&bh->b_wait, &wait);
+
do {
run_task_queue(&tq_disk);
set_task_state(tsk, TASK_UNINTERRUPTIBLE);
@@ -2332,7 +2337,7 @@
tmp = tmp->b_this_page;
if (buffer_locked(p)) {
if (wait > 1)
- __wait_on_buffer(p);
+ __wait_on_buffer(p, 0);
} else if (buffer_dirty(p))
ll_rw_block(WRITE, 1, &p);
} while (tmp != bh);
--- linux/include/linux/locks.h.orig Mon Feb 19 23:21:10 2001
+++ linux/include/linux/locks.h Mon Feb 19 23:17:42 2001
@@ -12,18 +12,18 @@
* Buffer cache locking - note that interrupts may only unlock, not
* lock buffers.
*/
-extern void __wait_on_buffer(struct buffer_head *);
+extern void __wait_on_buffer(struct buffer_head *, int);
extern inline void wait_on_buffer(struct buffer_head * bh)
{
if (test_bit(BH_Lock, &bh->b_state))
- __wait_on_buffer(bh);
+ __wait_on_buffer(bh, 0);
}
extern inline void lock_buffer(struct buffer_head * bh)
{
while (test_and_set_bit(BH_Lock, &bh->b_state))
- __wait_on_buffer(bh);
+ __wait_on_buffer(bh, 1);
}
extern inline void unlock_buffer(struct buffer_head *bh)
On Mon, 19 Feb 2001, Marcelo Tosatti wrote:
>
> The following patch makes lock_buffer() use the exclusive wakeup scheme
> added in 2.3.
Ugh, This is horrible.
You should NOT have one function that does two completely different things
depending on a flag. That way lies madness and bad coding habits.
Just do two different functions - make one be "__wait_on_buffer()", and
the other be "__lock_buffer()". See how the page functions work.
Linus
On Mon, 19 Feb 2001, Linus Torvalds wrote:
>
>
> On Mon, 19 Feb 2001, Marcelo Tosatti wrote:
> >
> > The following patch makes lock_buffer() use the exclusive wakeup scheme
> > added in 2.3.
>
> Ugh, This is horrible.
>
> You should NOT have one function that does two completely different things
> depending on a flag. That way lies madness and bad coding habits.
>
> Just do two different functions - make one be "__wait_on_buffer()", and
> the other be "__lock_buffer()". See how the page functions work.
>
> Linus
Ok.
--- linux/include/linux/locks.h.orig Mon Feb 19 23:16:50 2001
+++ linux/include/linux/locks.h Mon Feb 19 23:21:48 2001
@@ -13,6 +13,7 @@
* lock buffers.
*/
extern void __wait_on_buffer(struct buffer_head *);
+extern void __lock_buffer(struct buffer_head *);
extern inline void wait_on_buffer(struct buffer_head * bh)
{
@@ -22,8 +23,8 @@
extern inline void lock_buffer(struct buffer_head * bh)
{
- while (test_and_set_bit(BH_Lock, &bh->b_state))
- __wait_on_buffer(bh);
+ if (test_and_set_bit(BH_Lock, &bh->b_state))
+ __lock_on_buffer(bh);
}
extern inline void unlock_buffer(struct buffer_head *bh)
--- linux/fs/buffer.c.orig Mon Feb 19 23:09:31 2001
+++ linux/fs/buffer.c Mon Feb 19 23:31:25 2001
@@ -161,6 +161,30 @@
atomic_dec(&bh->b_count);
}
+void __lock_on_buffer(struct buffer_head * bh)
+{
+ struct task_struct *tsk = current;
+ DECLARE_WAITQUEUE(wait, tsk);
+
+ atomic_inc(&bh->b_count);
+ add_wait_queue_exclusive(&bh->b_wait, &wait);
+ for(;;) {
+ set_task_state(tsk, TASK_UNINTERRUPTIBLE);
+ if (test_bit(BH_Lock, &bh->b_state)) {
+ run_task_queue(&tq_disk);
+ schedule();
+ continue;
+ }
+
+ if (!test_and_set_bit(BH_Lock, &bh->b_state))
+ break;
+ }
+ tsk->state = TASK_RUNNING;
+ remove_wait_queue(&bh->b_wait, &wait);
+ atomic_dec(&bh->b_count);
+}
+
+
/* Call sync_buffers with wait!=0 to ensure that the call does not
* return until all buffer writes have completed. Sync() may return
* before the writes have finished; fsync() may not.
> --- linux/include/linux/locks.h.orig Mon Feb 19 23:16:50 2001
> +++ linux/include/linux/locks.h Mon Feb 19 23:21:48 2001
> @@ -13,6 +13,7 @@
> * lock buffers.
> */
> extern void __wait_on_buffer(struct buffer_head *);
> +extern void __lock_buffer(struct buffer_head *);
So are we starting 2.5 now ?
On Tue, 20 Feb 2001, Alan Cox wrote:
> Date: Tue, 20 Feb 2001 03:12:15 +0000 (GMT)
> From: Alan Cox <[email protected]>
> To: Marcelo Tosatti <[email protected]>
> Cc: Linus Torvalds <[email protected]>,
> lkml <[email protected]>
> Subject: Re: [PATCH] exclusive wakeup for lock_buffer
>
> > --- linux/include/linux/locks.h.orig Mon Feb 19 23:16:50 2001
> > +++ linux/include/linux/locks.h Mon Feb 19 23:21:48 2001
> > @@ -13,6 +13,7 @@
> > * lock buffers.
> > */
> > extern void __wait_on_buffer(struct buffer_head *);
> > +extern void __lock_buffer(struct buffer_head *);
>
> So are we starting 2.5 now ?
If per chance it's so, what's actually in plan for 2.5 so far?
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
-----------------------------------------------------------------
John Heil
South Coast Software
Custom systems software for UNIX and IBM MVS mainframes
1-714-774-6952
[email protected]
http://www.sc-software.com
-----------------------------------------------------------------
On Tue, 20 Feb 2001, Alan Cox wrote:
> > --- linux/include/linux/locks.h.orig Mon Feb 19 23:16:50 2001
> > +++ linux/include/linux/locks.h Mon Feb 19 23:21:48 2001
> > @@ -13,6 +13,7 @@
> > * lock buffers.
> > */
> > extern void __wait_on_buffer(struct buffer_head *);
> > +extern void __lock_buffer(struct buffer_head *);
>
> So are we starting 2.5 now ?
Alan,
This patch only avoids unecessary wakeups. It doesn't add any new
functionality.
> > > extern void __wait_on_buffer(struct buffer_head *);
> > > +extern void __lock_buffer(struct buffer_head *);
> >
> > So are we starting 2.5 now ?
>
> Alan,
>
> This patch only avoids unecessary wakeups. It doesn't add any new
> functionality.
I think making potentially very hard to debug changes to core code for
minor performance reasons alone isnt 2.4.x work. At least not yet
> --- linux/include/linux/locks.h.orig Mon Feb 19 23:16:50 2001
> +++ linux/include/linux/locks.h Mon Feb 19 23:21:48 2001
> @@ -13,6 +13,7 @@
> * lock buffers.
> */
> extern void __wait_on_buffer(struct buffer_head *);
> +extern void __lock_buffer(struct buffer_head *);
This doesn't match the function definition either.
Ulf