On Thu 31-03-16 15:20:00, NeilBrown wrote: > 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? Ah, OK. I have already fixed that in my local version of the patches so that we wake-up everybody after deleting the entry but forgot to tell you. So I have there now: __wake_up(wq, TASK_NORMAL, 0, &key); Are you OK with the code now? Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
