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@xxxxxxx> 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.
