Here's the second of the fs/buffer.c patches I'm working on. My posting
on the free_async_buffers SMP problem didn't get any comments, so I'll
offer this patch for comment.
The patch reworks free_async_buffers by first linking the buffer heads
into a chain, adding the current reuse_list chain, and then swapping in
the new head and possibly getting a chain in return. (It's very
unlikely, but possible.) If it gets a chain, it finds the tail and
repeats the process.
One nice side effect is that free_async_buffers no longer needs to be
called with interrupts disabled, so this should marginally reduce
latency in some cases.
The SMP race is probably too small to observe in testing, but it seems
it should be fixed anyway.
Regards,
Bill
--------------DF30D3318BF60FEDBB588231
Content-Type: text/plain; charset=us-ascii; name="buffer_fab47-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="buffer_fab47-patch"
--- fs/buffer.c.old Sat Jul 19 08:17:10 1997
+++ fs/buffer.c Fri Jul 25 21:39:04 1997
@@ -1154,24 +1223,49 @@
/*
* Free all temporary buffers belonging to a page.
- * This needs to be called with interrupts disabled.
+ * WSH 07/23/97: Rewritten to be SMP-safe. No longer needs to have
+ * interrupts disabled.
*/
static inline void free_async_buffers (struct buffer_head * bh)
{
- struct buffer_head * tmp;
+ struct buffer_head * tail, * next;
- tmp = bh;
- do {
- if (!test_bit(BH_FreeOnIO, &tmp->b_state)) {
- printk ("Whoops: unlock_buffer: "
- "async IO mismatch on page.\n");
- return;
- }
- tmp->b_next_free = xchg(&reuse_list, NULL);
- reuse_list = tmp;
- clear_bit(BH_FreeOnIO, &tmp->b_state);
- tmp = tmp->b_this_page;
- } while (tmp != bh);
+ /*
+ * The following ugly loop generates near-perfect assembly code.
+ */
+ tail = bh;
+ goto enter;
+linkit:
+ tail->b_next_free = next;
+ tail = next;
+enter:
+ if (!test_bit(BH_FreeOnIO, &tail->b_state))
+ goto mismatch;
+ next = tail->b_this_page;
+ if (next != bh)
+ goto linkit;
+
+ /*
+ * Put the chain onto the reuse list, and be prepared to handle
+ * another chain in return.
+ */
+repeat:
+ tail->b_next_free = xchg(&reuse_list, NULL);
+ bh = xchg(&reuse_list, bh);
+ if (!bh)
+ return;
+
+ printk("free_async_buffers: memory chain returned\n");
+ /*
+ * We got a buffer chain back ... find the tail and requeue it.
+ */
+ tail = bh;
+ while (tail->b_next_free)
+ tail = tail->b_next_free;
+ goto repeat;
+
+mismatch:
+ printk("Whoops: unlock_buffer: async IO mismatch on page.\n");
}
/*
@@ -1254,15 +1353,16 @@
/* The rest of the work is done in mark_buffer_uptodate()
* and unlock_buffer(). */
} else {
- unsigned long flags;
clear_bit(PG_locked, &page->flags);
set_bit(PG_uptodate, &page->flags);
wake_up(&page->wait);
- save_flags(flags);
- cli();
+ /*
+ * WSH: no longer need to disable interrupts.
+ */
free_async_buffers(bh);
- restore_flags(flags);
after_unlock_page(page);
+ if (waitqueue_active(&buffer_wait))
+ wake_up(&buffer_wait);
}
++current->maj_flt;
return 0;
@@ -1336,12 +1436,16 @@
} while (tmp != bh);
/* OK, the async IO on this page is complete. */
- free_async_buffers(bh);
restore_flags(flags);
+ /*
+ * No longer need to disable interrupts for free_async_buffers.
+ */
+ free_async_buffers(bh);
clear_bit(PG_locked, &page->flags);
wake_up(&page->wait);
after_unlock_page(page);
- wake_up(&buffer_wait);
+ if (waitqueue_active(&buffer_wait))
+ wake_up(&buffer_wait);
return;
still_busy:
--------------DF30D3318BF60FEDBB588231--