Re: dax_lock_mapping_entry was never safe
On Mon, Nov 26, 2018 at 12:36:26PM -0800, Dan Williams wrote:
On Mon, Nov 26, 2018 at 9:11 AM Jan Kara <jack(a)suse.cz> wrote:
> The code looks good. Maybe can we call this wait_entry_unlocked() to stress
> that entry is not really usable after this function returns? And comment
> before the function that this is safe to call even if we don't have a
> reference keeping mapping alive?
Yes, maybe even something more ambiguous like "wait_entry_event()",
because there's no guarantee the entry is unlocked just that now is a
good time to try to interrogate the entry again.
It _became_ unlocked ... it might be locked again, or have disappeared
by the time we get to it, but somebody called dax_wake_entry() for this
exact entry. I mean, we could call it wait_entry_wake(), but I think
that's a little generic. I'm going with Jan's version ;-)
> The continue here actually is not safe either because if the
mapping got
> freed, page->mapping will be NULL and we oops at the beginning of the loop.
> So that !dax_mapping() check should also check for mapping != NULL.
Yes.
Sigh. I'll make that a separate patch so it applies cleanly to 4.19.