Re: [PATCH 12/12] dax: New fault locking

From: NeilBrown <neilb@suse.com>
To: Jan Kara <jack@suse.cz>
Cc: linux-fsdevel@vger.kernel.org, "Wilcox,
	Matthew R" <matthew.r.wilcox@intel.com>,
	Ross Zwisler <ross.zwisler@linux.intel.com>,
	Dan Williams <dan.j.williams@intel.com>,
	linux-nvdimm@lists.01.org
Subject: Re: [PATCH 12/12] dax: New fault locking
Date: Fri, 01 Apr 2016 11:34:56 +1100	[thread overview]
Message-ID: <87oa9uqi8f.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <20160331085449.GB11041@quack.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 1260 bytes --]

On Thu, Mar 31 2016, Jan Kara wrote:

> On Thu 31-03-16 15:20:00, NeilBrown wrote:
>> On Wed, Mar 23 2016, Jan Kara wrote:
>> 

>> > 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);

"0" meaning "MAX_INT+1" (or something).  I didn't realize you could do
that, but it makes perfect sense.

>
> Are you OK with the code now?

Certainly less unhappy.  Assume that I am OK but I'll try to have
another look when you post again and make sure any lingering doubts are
gone.

Thanks,
NeilBrown

>
> 								Honza
> -- 
> Jan Kara <jack@suse.com>
> SUSE Labs, CR

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]