On Wed, Mar 23 2016, Jan Kara wrote: > On Wed 23-03-16 08:10:42, NeilBrown wrote: >> On Sat, Mar 19 2016, Jan Kara wrote: >> > >> > Actually, after some thought I don't think the wakeup is needed except for >> > dax_pfn_mkwrite(). In the other cases we know there is no radix tree >> > exceptional entry and thus there can be no waiters for its lock... >> > >> >> I think that is fragile logic - though it may be correct at present. >> >> A radix tree slot can transition from "Locked exception" to "unlocked >> exception" to "deleted" to "struct page". > > Yes. > >> So it is absolutely certain that a thread cannot go to sleep after >> finding a "locked exception" and wake up to find a "struct page" ?? > > With current implementation this should not happen but I agree entry > locking code should not rely on this. > >> How about a much simpler change. >> - new local variable "slept" in lookup_unlocked_mapping_entry() which >> is set if prepare_to_wait_exclusive() gets called. >> - if after __radix_tree_lookup() returns: >> (ret==NULL || !radix_tree_exceptional_entry(ret)) && slept >> then it calls wakeup immediately - because if it was waiting, >> something else might be to. >> >> That would cover all vaguely possible cases except dax_pfn_mkwrite() > > But how does this really help? If lookup_unlocked_mapping_entry() finds > there is no entry (and it was there before), the process deleting the entry > (or replacing it with something else) is responsible for waking up > everybody. "everybody" - yes. But it doesn't wake everybody does it? It just wakes one. + __wake_up(wq, TASK_NORMAL, 1, &key); ^one! Or am I misunderstanding how exclusive waiting works?