Messages in this thread | | | Date | Tue, 22 Jul 1997 21:32:33 -0400 | From | Bill Hawes <> | Subject | SMP question/problem |
| |
I'm still a little fuzzy on SMP matters, so if I'm wrong on this hopefully one of the experts here will explain it :-)
The following section of code from fs/buffer.c looks SMP-unsafe to me: 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);
I see two problems: first, since this code (sometimes) executes from an interrupt context, it would be possible for both processors to be handling interrupts and to execute statements in the following sequence: CPU 1 CPU 2 ... = xchg(...) ... = xchg(...) reuse_list = tmp reuse_list = tmp
in which case the later assignment would cause memory to be lost.
The second problem is more subtle. After the assignment reuse_list = tmp another reference is made to the structure that's now on the reuse list. When buffer heads are taken off the reuse list, the memory is set to 0. So the following sequence is possible:
CPU 1 CPU 2 reuse_list = tmp head = xchg(&reuse_list,NULL); (cache miss) . . . memset(bh, 0, ..); tmp = tmp->b_this_page (Oops)
I've changed the code to be SMP safe, as follows: tail = bh; for (;;) { if (!test_bit(BH_FreeOnIO, &tail->b_state)) goto mismatch; if (tail->b_this_page != bh) { tail->b_next_free = tail->b_this_page; tail = tail->b_this_page; continue; } break; }
repeat: tail->b_next_free = xchg(&reuse_list, NULL); bh = xchg(&reuse_list, bh); if (!bh) return;
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"); return;
The double xchg removes the possibility of losing memory by being prepared to get another list back and handling that as well.
Have I missed something in analyzing this, and is the changed code a reasonable way to fix it?
Regards, Bill
| |