mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-01 00:58:39 +02:00 
			
		
		
		
	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: b15cd80068 ("dax: Convert page fault handlers to XArray")
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Tested-by: Dan Williams <dan.j.williams@intel.com>
Reported-by: Robert Barror <robert.barror@intel.com>
Reported-by: Seema Pandit <seema.pandit@intel.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
			
			
This commit is contained in:
		
							parent
							
								
									40cdc60ac1
								
							
						
					
					
						commit
						23c84eb783
					
				
					 1 changed files with 33 additions and 20 deletions
				
			
		
							
								
								
									
										53
									
								
								fs/dax.c
									
									
									
									
									
								
							
							
						
						
									
										53
									
								
								fs/dax.c
									
									
									
									
									
								
							|  | @ -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) | ||||||
|  * 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) | ||||||
| 
 | 
 | ||||||
| 	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) | ||||||
| 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) | ||||||
|  * 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, | ||||||
| 
 | 
 | ||||||
| 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, | ||||||
| 	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) | ||||||
| 		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, | ||||||
| 	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, | ||||||
| 	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, | ||||||
| 	 * 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) | ||||||
| 	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, | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue
	
	 Matthew Wilcox (Oracle)
						Matthew Wilcox (Oracle)