Oleg Nesterov wrote: >> Yeap, I've used the term 'clearing' as more of a logical term - making >> the pointer invalid, but is there any reason why we can't clear the >> pointer itself? > > Because this breaks cancel_work_sync(work), it needs get_wq_data(work) for > wait_on_work(work).
Right. That's the other user of cached work->wq_data pointer. > So, try_to_grab_pending() should check "VALID && pointers equal" atomically. > We can't do "if (VALID && cwq == get_wq_data(work))". We should do something > like this > > (((long)cwq) | VALID | PENDING) == atomic_long_read(&work->data) > > Yes? I need to think more about this. I don't think the test for PENDING should be atomic too. cwq pointer and VALID is one package. PENDING lives its own life as a atomic bit switch. >> I didn't really get the smp_mb__before_spinlock() thing. How are you >> planning to use it? spinlock() is already a read barrier. What do you >> gain by making it also a write barrier? > > As I said, we can shift set_wq_data() from insert_work() to __queue_work(), OIC. > this needs very minor changes. > > BTW, in _theory_, spinlock() is not a read barrier, yes? It actually is. > From Documentation/memory-barriers.txt > > Memory operations that occur before a LOCK operation may appear to happen > after it completes. Which means that spin_lock() isn't a write barrier. lock is read barrier, unlock is write barrier. Otherwise, locking doesn't make much sense. If we're going the barrier way, I think we're better off with explicit smp_wmb(). It's only barrier() on x86/64. Thanks. -- tejun - 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/