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
>> > 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
>> exception" to "deleted" to "struct page".
>> So it is absolutely certain that a thread cannot go to sleep after
>> finding a "locked exception" and wake up to find a "struct
> 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()
>> 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" - yes. But it doesn't wake everybody does it? It just
+ __wake_up(wq, TASK_NORMAL, 1, &key);
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?
Jan Kara <jack(a)suse.com>
SUSE Labs, CR