On Fri, Nov 30, 2018 at 7:49 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > On Thu, Nov 29, 2018 at 04:13:46PM -0800, Dan Williams wrote: > > Internal to dax_unlock_mapping_entry(), dax_unlock_entry() is used to > > store a replacement entry in the Xarray at the given xas-index with the > > DAX_LOCKED bit clear. When called, dax_unlock_entry() expects the unlocked > > value of the entry relative to the current Xarray state to be specified. > > > > In most contexts dax_unlock_entry() is operating in the same scope as > > the matched dax_lock_entry(). However, in the dax_unlock_mapping_entry() > > case the implementation needs to recall the original entry. In the case > > where the original entry is a 'pmd' entry it is possible that the pfn > > performed to do the lookup is misaligned to the value retrieved in the > > Xarray. > > So far, dax_unlock_mapping_entry only has the one caller. I'd rather we > returned the 'entry' to the caller, then had them pass it back to the > unlock function. That matches the flow in the rest of DAX and doesn't > pose an undue burden to the caller. > > I plan to reclaim the DAX_LOCK bit (and the DAX_EMPTY bit for that > matter), instead using a special DAX_LOCK value. DAX is almost free of > assumptions about the other bits in a locked entry, and this will remove > the assuption that there's a PMD bit in the entry. > > How does this look? > Looks good to me, although can we make that cookie an actual type? I think it's mostly ok to pass around (void *) for 'entry' inside of fs/dax.c, but once an entry leaves that file I'd like it to have an explicit type to catch people that might accidentally pass a (struct page *) to the unlock routine.