Commit 23c84eb7 authored by Matthew Wilcox (Oracle)'s avatar Matthew Wilcox (Oracle) Committed by Dan Williams

dax: Fix missed wakeup with PMD faults

RocksDB can hang indefinitely when using a DAX file.  This is due to
a bug in the XArray conversion when handling a PMD fault and finding a
PTE entry.  We use the wrong index in the hash and end up waiting on
the wrong waitqueue.

There's actually no need to wait; if we find a PTE entry while looking
for a PMD entry, we can return immediately as we know we should fall
back to a PTE fault (which may not conflict with the lock held).

We reuse the XA_RETRY_ENTRY to signal a conflicting entry was found.
This value can never be found in an XArray while holding its lock, so
it does not create an ambiguity.

Cc: <stable@vger.kernel.org>
Link: http://lkml.kernel.org/r/CAPcyv4hwHpX-MkUEqxwdTj7wCCZCN4RV-L4jsnuwLGyL_UEG4A@mail.gmail.com
Fixes: b15cd800 ("dax: Convert page fault handlers to XArray")
Signed-off-by: default avatarMatthew Wilcox (Oracle) <willy@infradead.org>
Tested-by: default avatarDan Williams <dan.j.williams@intel.com>
Reported-by: default avatarRobert Barror <robert.barror@intel.com>
Reported-by: default avatarSeema Pandit <seema.pandit@intel.com>
Reviewed-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarDan Williams <dan.j.williams@intel.com>
parent 40cdc60a
...@@ -123,6 +123,15 @@ static int dax_is_empty_entry(void *entry) ...@@ -123,6 +123,15 @@ static int dax_is_empty_entry(void *entry)
return xa_to_value(entry) & DAX_EMPTY; return xa_to_value(entry) & DAX_EMPTY;
} }
/*
* true if the entry that was found is of a smaller order than the entry
* we were looking for
*/
static bool dax_is_conflict(void *entry)
{
return entry == XA_RETRY_ENTRY;
}
/* /*
* DAX page cache entry locking * DAX page cache entry locking
*/ */
...@@ -195,11 +204,13 @@ static void dax_wake_entry(struct xa_state *xas, void *entry, bool wake_all) ...@@ -195,11 +204,13 @@ static void dax_wake_entry(struct xa_state *xas, void *entry, bool wake_all)
* Look up entry in page cache, wait for it to become unlocked if it * Look up entry in page cache, wait for it to become unlocked if it
* is a DAX entry and return it. The caller must subsequently call * is a DAX entry and return it. The caller must subsequently call
* put_unlocked_entry() if it did not lock the entry or dax_unlock_entry() * put_unlocked_entry() if it did not lock the entry or dax_unlock_entry()
* if it did. * if it did. The entry returned may have a larger order than @order.
* If @order is larger than the order of the entry found in i_pages, this
* function returns a dax_is_conflict entry.
* *
* Must be called with the i_pages lock held. * Must be called with the i_pages lock held.
*/ */
static void *get_unlocked_entry(struct xa_state *xas) static void *get_unlocked_entry(struct xa_state *xas, unsigned int order)
{ {
void *entry; void *entry;
struct wait_exceptional_entry_queue ewait; struct wait_exceptional_entry_queue ewait;
...@@ -210,6 +221,8 @@ static void *get_unlocked_entry(struct xa_state *xas) ...@@ -210,6 +221,8 @@ static void *get_unlocked_entry(struct xa_state *xas)
for (;;) { for (;;) {
entry = xas_find_conflict(xas); entry = xas_find_conflict(xas);
if (dax_entry_order(entry) < order)
return XA_RETRY_ENTRY;
if (!entry || WARN_ON_ONCE(!xa_is_value(entry)) || if (!entry || WARN_ON_ONCE(!xa_is_value(entry)) ||
!dax_is_locked(entry)) !dax_is_locked(entry))
return entry; return entry;
...@@ -254,7 +267,7 @@ static void wait_entry_unlocked(struct xa_state *xas, void *entry) ...@@ -254,7 +267,7 @@ static void wait_entry_unlocked(struct xa_state *xas, void *entry)
static void put_unlocked_entry(struct xa_state *xas, void *entry) static void put_unlocked_entry(struct xa_state *xas, void *entry)
{ {
/* If we were the only waiter woken, wake the next one */ /* If we were the only waiter woken, wake the next one */
if (entry) if (entry && dax_is_conflict(entry))
dax_wake_entry(xas, entry, false); dax_wake_entry(xas, entry, false);
} }
...@@ -461,7 +474,7 @@ void dax_unlock_page(struct page *page, dax_entry_t cookie) ...@@ -461,7 +474,7 @@ void dax_unlock_page(struct page *page, dax_entry_t cookie)
* overlap with xarray value entries. * overlap with xarray value entries.
*/ */
static void *grab_mapping_entry(struct xa_state *xas, static void *grab_mapping_entry(struct xa_state *xas,
struct address_space *mapping, unsigned long size_flag) struct address_space *mapping, unsigned int order)
{ {
unsigned long index = xas->xa_index; unsigned long index = xas->xa_index;
bool pmd_downgrade = false; /* splitting PMD entry into PTE entries? */ bool pmd_downgrade = false; /* splitting PMD entry into PTE entries? */
...@@ -469,20 +482,17 @@ static void *grab_mapping_entry(struct xa_state *xas, ...@@ -469,20 +482,17 @@ static void *grab_mapping_entry(struct xa_state *xas,
retry: retry:
xas_lock_irq(xas); xas_lock_irq(xas);
entry = get_unlocked_entry(xas); entry = get_unlocked_entry(xas, order);
if (entry) { if (entry) {
if (dax_is_conflict(entry))
goto fallback;
if (!xa_is_value(entry)) { if (!xa_is_value(entry)) {
xas_set_err(xas, EIO); xas_set_err(xas, EIO);
goto out_unlock; goto out_unlock;
} }
if (size_flag & DAX_PMD) { if (order == 0) {
if (dax_is_pte_entry(entry)) {
put_unlocked_entry(xas, entry);
goto fallback;
}
} else { /* trying to grab a PTE entry */
if (dax_is_pmd_entry(entry) && if (dax_is_pmd_entry(entry) &&
(dax_is_zero_entry(entry) || (dax_is_zero_entry(entry) ||
dax_is_empty_entry(entry))) { dax_is_empty_entry(entry))) {
...@@ -523,7 +533,11 @@ static void *grab_mapping_entry(struct xa_state *xas, ...@@ -523,7 +533,11 @@ static void *grab_mapping_entry(struct xa_state *xas,
if (entry) { if (entry) {
dax_lock_entry(xas, entry); dax_lock_entry(xas, entry);
} else { } else {
entry = dax_make_entry(pfn_to_pfn_t(0), size_flag | DAX_EMPTY); unsigned long flags = DAX_EMPTY;
if (order > 0)
flags |= DAX_PMD;
entry = dax_make_entry(pfn_to_pfn_t(0), flags);
dax_lock_entry(xas, entry); dax_lock_entry(xas, entry);
if (xas_error(xas)) if (xas_error(xas))
goto out_unlock; goto out_unlock;
...@@ -594,7 +608,7 @@ struct page *dax_layout_busy_page(struct address_space *mapping) ...@@ -594,7 +608,7 @@ struct page *dax_layout_busy_page(struct address_space *mapping)
if (WARN_ON_ONCE(!xa_is_value(entry))) if (WARN_ON_ONCE(!xa_is_value(entry)))
continue; continue;
if (unlikely(dax_is_locked(entry))) if (unlikely(dax_is_locked(entry)))
entry = get_unlocked_entry(&xas); entry = get_unlocked_entry(&xas, 0);
if (entry) if (entry)
page = dax_busy_page(entry); page = dax_busy_page(entry);
put_unlocked_entry(&xas, entry); put_unlocked_entry(&xas, entry);
...@@ -621,7 +635,7 @@ static int __dax_invalidate_entry(struct address_space *mapping, ...@@ -621,7 +635,7 @@ static int __dax_invalidate_entry(struct address_space *mapping,
void *entry; void *entry;
xas_lock_irq(&xas); xas_lock_irq(&xas);
entry = get_unlocked_entry(&xas); entry = get_unlocked_entry(&xas, 0);
if (!entry || WARN_ON_ONCE(!xa_is_value(entry))) if (!entry || WARN_ON_ONCE(!xa_is_value(entry)))
goto out; goto out;
if (!trunc && if (!trunc &&
...@@ -849,7 +863,7 @@ static int dax_writeback_one(struct xa_state *xas, struct dax_device *dax_dev, ...@@ -849,7 +863,7 @@ static int dax_writeback_one(struct xa_state *xas, struct dax_device *dax_dev,
if (unlikely(dax_is_locked(entry))) { if (unlikely(dax_is_locked(entry))) {
void *old_entry = entry; void *old_entry = entry;
entry = get_unlocked_entry(xas); entry = get_unlocked_entry(xas, 0);
/* Entry got punched out / reallocated? */ /* Entry got punched out / reallocated? */
if (!entry || WARN_ON_ONCE(!xa_is_value(entry))) if (!entry || WARN_ON_ONCE(!xa_is_value(entry)))
...@@ -1510,7 +1524,7 @@ static vm_fault_t dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp, ...@@ -1510,7 +1524,7 @@ static vm_fault_t dax_iomap_pmd_fault(struct vm_fault *vmf, pfn_t *pfnp,
* entry is already in the array, for instance), it will return * entry is already in the array, for instance), it will return
* VM_FAULT_FALLBACK. * VM_FAULT_FALLBACK.
*/ */
entry = grab_mapping_entry(&xas, mapping, DAX_PMD); entry = grab_mapping_entry(&xas, mapping, PMD_ORDER);
if (xa_is_internal(entry)) { if (xa_is_internal(entry)) {
result = xa_to_internal(entry); result = xa_to_internal(entry);
goto fallback; goto fallback;
...@@ -1659,11 +1673,10 @@ dax_insert_pfn_mkwrite(struct vm_fault *vmf, pfn_t pfn, unsigned int order) ...@@ -1659,11 +1673,10 @@ dax_insert_pfn_mkwrite(struct vm_fault *vmf, pfn_t pfn, unsigned int order)
vm_fault_t ret; vm_fault_t ret;
xas_lock_irq(&xas); xas_lock_irq(&xas);
entry = get_unlocked_entry(&xas); entry = get_unlocked_entry(&xas, order);
/* Did we race with someone splitting entry or so? */ /* Did we race with someone splitting entry or so? */
if (!entry || if (!entry || dax_is_conflict(entry) ||
(order == 0 && !dax_is_pte_entry(entry)) || (order == 0 && !dax_is_pte_entry(entry))) {
(order == PMD_ORDER && !dax_is_pmd_entry(entry))) {
put_unlocked_entry(&xas, entry); put_unlocked_entry(&xas, entry);
xas_unlock_irq(&xas); xas_unlock_irq(&xas);
trace_dax_insert_pfn_mkwrite_no_entry(mapping->host, vmf, trace_dax_insert_pfn_mkwrite_no_entry(mapping->host, vmf,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment