zram: remove UNDER_WB and simplify writeback

We now have only one active post-processing at any time, so we don't have
same race conditions that we had before.  If slot selected for
post-processing gets freed or freed and reallocated it loses its PP_SLOT
flag and there is no way for such a slot to gain PP_SLOT flag again until
current post-processing terminates.

Link: https://lkml.kernel.org/r/20240917021020.883356-8-senozhatsky@chromium.org
Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Minchan Kim <minchan@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index c59a3e9218a9c9c35a1dcba05f8c8c00562adb1b..263795c4aef709861ed715088d451a56d5e00670 100644 (file)
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -390,10 +390,7 @@ static void mark_idle(struct zram *zram, ktime_t cutoff)
 
        for (index = 0; index < nr_pages; index++) {
                /*
-                * Do not mark ZRAM_UNDER_WB slot as ZRAM_IDLE to close race.
-                * See the comment in writeback_store.
-                *
-                * Also do not mark ZRAM_SAME slots as ZRAM_IDLE, because no
+                * Do not mark ZRAM_SAME slots as ZRAM_IDLE, because no
                 * post-processing (recompress, writeback) happens to the
                 * ZRAM_SAME slot.
                 *
@@ -402,7 +399,6 @@ static void mark_idle(struct zram *zram, ktime_t cutoff)
                zram_slot_lock(zram, index);
                if (!zram_allocated(zram, index) ||
                    zram_test_flag(zram, index, ZRAM_WB) ||
-                   zram_test_flag(zram, index, ZRAM_UNDER_WB) ||
                    zram_test_flag(zram, index, ZRAM_SAME)) {
                        zram_slot_unlock(zram, index);
                        continue;
@@ -821,22 +817,17 @@ static ssize_t writeback_store(struct device *dev,
 
                index = pps->index;
                zram_slot_lock(zram, index);
-               if (!zram_test_flag(zram, index, ZRAM_PP_SLOT))
-                       goto next;
                /*
-                * Clearing ZRAM_UNDER_WB is duty of caller.
-                * IOW, zram_free_page never clear it.
+                * scan_slots() sets ZRAM_PP_SLOT and relases slot lock, so
+                * slots can change in the meantime. If slots are accessed or
+                * freed they lose ZRAM_PP_SLOT flag and hence we don't
+                * post-process them.
                 */
-               zram_set_flag(zram, index, ZRAM_UNDER_WB);
-               /* Need for hugepage writeback racing */
-               zram_set_flag(zram, index, ZRAM_IDLE);
+               if (!zram_test_flag(zram, index, ZRAM_PP_SLOT))
+                       goto next;
                zram_slot_unlock(zram, index);
-               if (zram_read_page(zram, page, index, NULL)) {
-                       zram_slot_lock(zram, index);
-                       zram_clear_flag(zram, index, ZRAM_UNDER_WB);
-                       zram_clear_flag(zram, index, ZRAM_IDLE);
-                       zram_slot_unlock(zram, index);
 
+               if (zram_read_page(zram, page, index, NULL)) {
                        release_pp_slot(zram, pps);
                        continue;
                }
@@ -852,11 +843,6 @@ static ssize_t writeback_store(struct device *dev,
                 */
                err = submit_bio_wait(&bio);
                if (err) {
-                       zram_slot_lock(zram, index);
-                       zram_clear_flag(zram, index, ZRAM_UNDER_WB);
-                       zram_clear_flag(zram, index, ZRAM_IDLE);
-                       zram_slot_unlock(zram, index);
-
                        release_pp_slot(zram, pps);
                        /*
                         * BIO errors are not fatal, we continue and simply
@@ -871,25 +857,19 @@ static ssize_t writeback_store(struct device *dev,
                }
 
                atomic64_inc(&zram->stats.bd_writes);
+               zram_slot_lock(zram, index);
                /*
-                * We released zram_slot_lock so need to check if the slot was
-                * changed. If there is freeing for the slot, we can catch it
-                * easily by zram_allocated.
-                * A subtle case is the slot is freed/reallocated/marked as
-                * ZRAM_IDLE again. To close the race, idle_store doesn't
-                * mark ZRAM_IDLE once it found the slot was ZRAM_UNDER_WB.
-                * Thus, we could close the race by checking ZRAM_IDLE bit.
+                * Same as above, we release slot lock during writeback so
+                * slot can change under us: slot_free() or slot_free() and
+                * reallocation (zram_write_page()). In both cases slot loses
+                * ZRAM_PP_SLOT flag. No concurrent post-processing can set
+                * ZRAM_PP_SLOT on such slots until current post-processing
+                * finishes.
                 */
-               zram_slot_lock(zram, index);
-               if (!zram_allocated(zram, index) ||
-                         !zram_test_flag(zram, index, ZRAM_IDLE)) {
-                       zram_clear_flag(zram, index, ZRAM_UNDER_WB);
-                       zram_clear_flag(zram, index, ZRAM_IDLE);
+               if (!zram_test_flag(zram, index, ZRAM_PP_SLOT))
                        goto next;
-               }
 
                zram_free_page(zram, index);
-               zram_clear_flag(zram, index, ZRAM_UNDER_WB);
                zram_set_flag(zram, index, ZRAM_WB);
                zram_set_element(zram, index, blk_idx);
                blk_idx = 0;
@@ -1538,7 +1518,6 @@ out:
        atomic64_dec(&zram->stats.pages_stored);
        zram_set_handle(zram, index, 0);
        zram_set_obj_size(zram, index, 0);
-       WARN_ON_ONCE(zram->table[index].flags & ~(1UL << ZRAM_UNDER_WB));
 }
 
 /*

diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
index 73a9d47d76bae1edd6c025bf5c30f76136bba83d..134be414e2106bdfe72745db28090849052eeb56 100644 (file)
--- a/drivers/block/zram/zram_drv.h
+++ b/drivers/block/zram/zram_drv.h
@@ -47,7 +47,6 @@
 enum zram_pageflags {
        ZRAM_SAME = ZRAM_FLAG_SHIFT,    /* Page consists the same element */
        ZRAM_WB,        /* page is stored on backing_device */
-       ZRAM_UNDER_WB,  /* page is under writeback */
        ZRAM_PP_SLOT,   /* Selected for post-processing */
        ZRAM_HUGE,      /* Incompressible page */
        ZRAM_IDLE,      /* not accessed page since last idle marking */