On Wed 03-07-19 20:27:28, Matthew Wilcox wrote:
On Wed, Jul 03, 2019 at 02:28:41PM -0700, Dan Williams wrote:
On Wed, Jul 3, 2019 at 12:53 PM Matthew Wilcox willy@infradead.org wrote:
@@ -211,7 +215,8 @@ static void *get_unlocked_entry(struct xa_state *xas) for (;;) { entry = xas_find_conflict(xas); if (!entry || WARN_ON_ONCE(!xa_is_value(entry)) ||
!dax_is_locked(entry))
!dax_is_locked(entry) ||dax_entry_order(entry) < xas_get_order(xas))Doesn't this potentially allow a locked entry to be returned for a caller that expects all value entries are unlocked?
It only allows locked entries to be returned for callers which pass in an xas which refers to a PMD entry. This is fine for grab_mapping_entry() because it checks size_flag & is_pte_entry.
dax_layout_busy_page() only uses 0-order. __dax_invalidate_entry() only uses 0-order. dax_writeback_one() needs an extra fix:
/* Did a PMD entry get split? */ if (dax_is_locked(entry)) goto put_unlocked;dax_insert_pfn_mkwrite() checks for a mismatch of pte vs pmd.
So I think we're good for all current users.
Agreed but it is an ugly trap. As I already said, I'd rather pay the unnecessary cost of waiting for pte entry and have an easy to understand interface. If we ever have a real world use case that would care for this optimization, we will need to refactor functions to make this possible and still keep the interfaces sane. For example get_unlocked_entry() could return special "error code" indicating that there's no entry with matching order in xarray but there's a conflict with it. That would be much less error-prone interface.
Honza