Messages in this thread | | | Date | Tue, 27 Jan 2004 14:43:43 -0500 | From | Joe Korty <> | Subject | Re: [PATCH] volatile may be needed in rwsem |
| |
On Tue, Jan 27, 2004 at 07:19:40PM +0000, David Howells wrote: >> 'flags' should be declared volatile as rwsem_down_failed_common() spins >> waiting for this to change. Untested. > > Is it though? Does this fix an error? > > The thing is, we make a function call inside of the loop: > > /* wait to be given the lock */ > for (;;) { > if (!waiter->flags) > break; > schedule(); > set_task_state(tsk, TASK_UNINTERRUPTIBLE); > } > > Which might preclude that need. I'm not entirely sure, though... it's one of > those compiler black magic things. > > I suppose it can't hurt... > > David
Hi David, I misspoke. The potentially failing spin is in __down_write and __down_read in lib/rwsem-spinlock.c, not in rwsem_down_failed_common.
The problem is is that 'flags' is on the callee's stack and is thus subject to be optimized out of the loop if the compiler is smart enough to discover that it is on the stack. Apparently gcc is not yet smart enough but that doesn't mean it won't be so soon.
Joe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |