Messages in this thread Patch in this message | | | Date | Sat, 26 Jul 1997 09:25:43 -0400 | From | Bill Hawes <> | Subject | patch for free_async_buffers SMP problem |
| |
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--- 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: | |