While stress-testing zswap a memory corruption was happening when writing back pages. __frontswap_store used to check for duplicate entries before attempting to store a page in zswap, this was because if the store fails the old entry isn't removed from the tree. This change removes duplicate entries in zswap_store before the actual attempt. [cerasuolodomenico@gmail.com: add a warning and a comment, per Johannes] Link: https://lkml.kernel.org/r/20230925130002.1929369-1-cerasuolodomenico@gmail.com Link: https://lkml.kernel.org/r/20230922172211.1704917-1-cerasuolodomenico@gmail.com Fixes: 42c06a0e8ebe ("mm: kill frontswap") Signed-off-by: Domenico Cerasuolo <cerasuolodomenico@gmail.com> Acked-by: Johannes Weiner <hannes@cmpxchg.org> Acked-by: Nhat Pham <nphamcs@gmail.com> Cc: Dan Streetman <ddstreet@ieee.org> Cc: Domenico Cerasuolo <cerasuolodomenico@gmail.com> Cc: Seth Jennings <sjenning@redhat.com> Cc: Vitaly Wool <vitaly.wool@konsulko.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Unified · Split
| @@ -1218,6 +1218,19 @@ bool zswap_store(struct folio *folio) · 1217 unmodified lines | |||
| 1218 | if (!zswap_enabled || !tree) | 1218 | if (!zswap_enabled || !tree) |
| 1219 | return false; | 1219 | return false; |
| 1220 | 1220 | ||
| 1221 | /* | ||
| 1222 | * If this is a duplicate, it must be removed before attempting to store | ||
| 1223 | * it, otherwise, if the store fails the old page won't be removed from | ||
| 1224 | * the tree, and it might be written back overriding the new data. | ||
| 1225 | */ | ||
| 1226 | spin_lock(&tree->lock); | ||
| 1227 | dupentry = zswap_rb_search(&tree->rbroot, offset); | ||
| 1228 | if (dupentry) { | ||
| 1229 | zswap_duplicate_entry++; | ||
| 1230 | zswap_invalidate_entry(tree, dupentry); | ||
| 1231 | } | ||
| 1232 | spin_unlock(&tree->lock); | ||
| 1233 | |||
| 1221 | /* | 1234 | /* |
| 1222 | * XXX: zswap reclaim does not work with cgroups yet. Without a | 1235 | * XXX: zswap reclaim does not work with cgroups yet. Without a |
| 1223 | * cgroup-aware entry LRU, we will push out entries system-wide based on | 1236 | * cgroup-aware entry LRU, we will push out entries system-wide based on |
| @@ -1333,7 +1346,14 @@ insert_entry: · 109 unmodified lines | |||
| 1333 | 1346 | ||
| 1334 | /* map */ | 1347 | /* map */ |
| 1335 | spin_lock(&tree->lock); | 1348 | spin_lock(&tree->lock); |
| 1349 | /* | ||
| 1350 | * A duplicate entry should have been removed at the beginning of this | ||
| 1351 | * function. Since the swap entry should be pinned, if a duplicate is | ||
| 1352 | * found again here it means that something went wrong in the swap | ||
| 1353 | * cache. | ||
| 1354 | */ | ||
| 1336 | while (zswap_rb_insert(&tree->rbroot, entry, &dupentry) == -EEXIST) { | 1355 | while (zswap_rb_insert(&tree->rbroot, entry, &dupentry) == -EEXIST) { |
| 1356 | WARN_ON(1); | ||
| 1337 | zswap_duplicate_entry++; | 1357 | zswap_duplicate_entry++; |
| 1338 | zswap_invalidate_entry(tree, dupentry); | 1358 | zswap_invalidate_entry(tree, dupentry); |
| 1339 | } | 1359 | } |