[PATCH 7.1 262/271] mm/filemap: __filemap_add_folio() restore index before retrying
Greg Kroah-Hartman <[email protected]>
| Newsgroups | dev.linux.lists.patches,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
7.1-stable review patch. If anyone has any objections, please let me know. ------------------ From: Hugh Dickins <[email protected]> commit 86da3f7e1e609e1e8bfbab198af68467c5a015a5 upstream. In __filemap_add_folio()'s split-a-conflict loop, xas_set_order() is applied repeatedly: each application modifies xas.xa_index, rounding it down according to the split_order attempted at that stage: and if all goes as intended, it eventually (or immediately) converges on an xas_try_split() to the required folio_order, with xas.xa_index now the same as index: then xas_store() puts the new folio into the xarray there. But if a new node was needed, and GFP_NOWAIT allocation did not get one, the lock is dropped, xas_nomem() used to allocate, and sequence retried. If (that part of) the xarray is unchanged when the lock is reacquired, no problem. But what if the conflict was meanwhile resolved by another thread (perhaps even doing the same thing, inserting a folio at that same index)? Isn't there a danger of now putting our folio into the xarray at an intermediate rounded-down index? With !folio_contains() bug to follow, when CONFIG_DEBUG_VM=y is checking for that. Fix this with an xas_set_order() to restore the original xas.xa_index at the bottom of the loop, so the retry does a full re-evaluation after reacquiring the lock, and cannot reach xas_store() with the wrong index. Production was suffering from rare SIGILLs and SIGSEGVs, executable text found a page away from where it belonged, !folio_contains() bug hit when debug enabled: symptoms not seen since this patch went in. Link: https://lore.kernel.org/[email protected] Fixes: 200a89c159a7 ("mm/filemap: use xas_try_split() in __filemap_add_folio()") Signed-off-by: Hugh Dickins <[email protected]> Acked-by: Kiryl Shutsemau (Meta) <[email protected]> Reviewed-by: Matthew Wilcox (Oracle) <[email protected]> Reviewed-by: Zi Yan <[email protected]> Cc: Chris J Arges <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Jan Kara <[email protected]> Cc: Kairui Song <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]> --- mm/filemap.c | 6 ++++++ 1 file changed, 6 insertions(+) --- a/mm/filemap.c +++ b/mm/filemap.c @@ -932,6 +932,12 @@ unlock: if (!xas_nomem(&xas, gfp)) break; + + /* + * Lock has been dropped: start again with the original index + * and order (but now with the memory reserved by xas_nomem()). + */ + xas_set_order(&xas, index, forder); } if (xas_error(&xas))