[2/2] kcompressd: Add Kcompressd for accelerated zram compression

Commit Message

Qun-wei Lin (林群崴) March 7, 2025, 12:01 p.m. UTC

Introduced Kcompressd to offload zram page compression, improving
system efficiency by handling compression separately from memory
reclaiming. Added necessary configurations and dependencies.

Signed-off-by: Qun-Wei Lin <[e-mail]>
---
 drivers/block/zram/Kconfig      |  11 ++
 drivers/block/zram/Makefile     |   3 +-
 drivers/block/zram/kcompressd.c | 340 ++++++++++++++++++++++++++++++++
 drivers/block/zram/kcompressd.h |  25 +++
 drivers/block/zram/zram_drv.c   |  22 ++-
 5 files changed, 397 insertions(+), 4 deletions(-)
 create mode 100644 drivers/block/zram/kcompressd.c
 create mode 100644 drivers/block/zram/kcompressd.h

Comments

Barry Song March 7, 2025, 7:41 p.m. UTC | #1

On Sat, Mar 8, 2025 at 1:02 AM Qun-Wei Lin <[e-mail]> wrote:
>
> Introduced Kcompressd to offload zram page compression, improving
> system efficiency by handling compression separately from memory
> reclaiming. Added necessary configurations and dependencies.
>
> Signed-off-by: Qun-Wei Lin <[e-mail]>
> ---
>  drivers/block/zram/Kconfig      |  11 ++
>  drivers/block/zram/Makefile     |   3 +-
>  drivers/block/zram/kcompressd.c | 340 ++++++++++++++++++++++++++++++++
>  drivers/block/zram/kcompressd.h |  25 +++
>  drivers/block/zram/zram_drv.c   |  22 ++-
>  5 files changed, 397 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/block/zram/kcompressd.c
>  create mode 100644 drivers/block/zram/kcompressd.h
>
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index 402b7b175863..f0a1b574f770 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -145,3 +145,14 @@ config ZRAM_MULTI_COMP
>           re-compress pages using a potentially slower but more effective
>           compression algorithm. Note, that IDLE page recompression
>           requires ZRAM_TRACK_ENTRY_ACTIME.
> +
> +config KCOMPRESSD
> +       tristate "Kcompressd: Accelerated zram compression"
> +       depends on ZRAM
> +       help
> +         Kcompressd creates multiple daemons to accelerate the compression of pages
> +         in zram, offloading this time-consuming task from the zram driver.
> +
> +         This approach improves system efficiency by handling page compression separately,
> +         which was originally done by kswapd or direct reclaim.

For direct reclaim, we were previously able to compress using multiple CPUs
with multi-threading.
After your patch, it seems that only a single thread/CPU is used for compression
so it won't necessarily improve direct reclaim performance?

Even for kswapd, we used to have multiple threads like [kswapd0], [kswapd1],
and [kswapd2] for different nodes. Now, are we also limited to just one thread?
I also wonder if this could be handled at the vmscan level instead of the zram
level. then it might potentially help other sync devices or even zswap later.

But I agree that for phones, modifying zram seems like an easier starting
point. However, relying on a single thread isn't always the best approach.


> +
> diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
> index 0fdefd576691..23baa5dfceb9 100644
> --- a/drivers/block/zram/Makefile
> +++ b/drivers/block/zram/Makefile
> @@ -9,4 +9,5 @@ zram-$(CONFIG_ZRAM_BACKEND_ZSTD)        += backend_zstd.o
>  zram-$(CONFIG_ZRAM_BACKEND_DEFLATE)    += backend_deflate.o
>  zram-$(CONFIG_ZRAM_BACKEND_842)                += backend_842.o
>
> -obj-$(CONFIG_ZRAM)     +=      zram.o
> +obj-$(CONFIG_ZRAM)             += zram.o
> +obj-$(CONFIG_KCOMPRESSD)       += kcompressd.o
> diff --git a/drivers/block/zram/kcompressd.c b/drivers/block/zram/kcompressd.c
> new file mode 100644
> index 000000000000..195b7e386869
> --- /dev/null
> +++ b/drivers/block/zram/kcompressd.c
> @@ -0,0 +1,340 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 MediaTek Inc.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/bio.h>
> +#include <linux/bitops.h>
> +#include <linux/freezer.h>
> +#include <linux/kernel.h>
> +#include <linux/psi.h>
> +#include <linux/kfifo.h>
> +#include <linux/swap.h>
> +#include <linux/delay.h>
> +
> +#include "kcompressd.h"
> +
> +#define INIT_QUEUE_SIZE                4096
> +#define DEFAULT_NR_KCOMPRESSD  4
> +
> +static atomic_t enable_kcompressd;
> +static unsigned int nr_kcompressd;
> +static unsigned int queue_size_per_kcompressd;
> +static struct kcompress *kcompress;
> +
> +enum run_state {
> +       KCOMPRESSD_NOT_STARTED = 0,
> +       KCOMPRESSD_RUNNING,
> +       KCOMPRESSD_SLEEPING,
> +};
> +
> +struct kcompressd_para {
> +       wait_queue_head_t *kcompressd_wait;
> +       struct kfifo *write_fifo;
> +       atomic_t *running;
> +};
> +
> +static struct kcompressd_para *kcompressd_para;
> +static BLOCKING_NOTIFIER_HEAD(kcompressd_notifier_list);
> +
> +struct write_work {
> +       void *mem;
> +       struct bio *bio;
> +       compress_callback cb;
> +};
> +
> +int kcompressd_enabled(void)
> +{
> +       return likely(atomic_read(&enable_kcompressd));
> +}
> +EXPORT_SYMBOL(kcompressd_enabled);
> +
> +static void kcompressd_try_to_sleep(struct kcompressd_para *p)
> +{
> +       DEFINE_WAIT(wait);
> +
> +       if (!kfifo_is_empty(p->write_fifo))
> +               return;
> +
> +       if (freezing(current) || kthread_should_stop())
> +               return;
> +
> +       atomic_set(p->running, KCOMPRESSD_SLEEPING);
> +       prepare_to_wait(p->kcompressd_wait, &wait, TASK_INTERRUPTIBLE);
> +
> +       /*
> +        * After a short sleep, check if it was a premature sleep. If not, then
> +        * go fully to sleep until explicitly woken up.
> +        */
> +       if (!kthread_should_stop() && kfifo_is_empty(p->write_fifo))
> +               schedule();
> +
> +       finish_wait(p->kcompressd_wait, &wait);
> +       atomic_set(p->running, KCOMPRESSD_RUNNING);
> +}
> +
> +static int kcompressd(void *para)
> +{
> +       struct task_struct *tsk = current;
> +       struct kcompressd_para *p = (struct kcompressd_para *)para;
> +
> +       tsk->flags |= PF_MEMALLOC | PF_KSWAPD;
> +       set_freezable();
> +
> +       while (!kthread_should_stop()) {
> +               bool ret;
> +
> +               kcompressd_try_to_sleep(p);
> +               ret = try_to_freeze();
> +               if (kthread_should_stop())
> +                       break;
> +
> +               if (ret)
> +                       continue;
> +
> +               while (!kfifo_is_empty(p->write_fifo)) {
> +                       struct write_work entry;
> +
> +                       if (sizeof(struct write_work) == kfifo_out(p->write_fifo,
> +                                               &entry, sizeof(struct write_work))) {
> +                               entry.cb(entry.mem, entry.bio);
> +                               bio_put(entry.bio);
> +                       }
> +               }
> +
> +       }
> +
> +       tsk->flags &= ~(PF_MEMALLOC | PF_KSWAPD);
> +       atomic_set(p->running, KCOMPRESSD_NOT_STARTED);
> +       return 0;
> +}
> +
> +static int init_write_queue(void)
> +{
> +       int i;
> +       unsigned int queue_len = queue_size_per_kcompressd * sizeof(struct write_work);
> +
> +       for (i = 0; i < nr_kcompressd; i++) {
> +               if (kfifo_alloc(&kcompress[i].write_fifo,
> +                                       queue_len, GFP_KERNEL)) {
> +                       pr_err("Failed to alloc kfifo %d\n", i);
> +                       return -ENOMEM;
> +               }
> +       }
> +       return 0;
> +}
> +
> +static void clean_bio_queue(int idx)
> +{
> +       struct write_work entry;
> +
> +       while (sizeof(struct write_work) == kfifo_out(&kcompress[idx].write_fifo,
> +                               &entry, sizeof(struct write_work))) {
> +               bio_put(entry.bio);
> +               entry.cb(entry.mem, entry.bio);
> +       }
> +       kfifo_free(&kcompress[idx].write_fifo);
> +}
> +
> +static int kcompress_update(void)
> +{
> +       int i;
> +       int ret;
> +
> +       kcompress = kvmalloc_array(nr_kcompressd, sizeof(struct kcompress), GFP_KERNEL);
> +       if (!kcompress)
> +               return -ENOMEM;
> +
> +       kcompressd_para = kvmalloc_array(nr_kcompressd, sizeof(struct kcompressd_para), GFP_KERNEL);
> +       if (!kcompressd_para)
> +               return -ENOMEM;
> +
> +       ret = init_write_queue();
> +       if (ret) {
> +               pr_err("Initialization of writing to FIFOs failed!!\n");
> +               return ret;
> +       }
> +
> +       for (i = 0; i < nr_kcompressd; i++) {
> +               init_waitqueue_head(&kcompress[i].kcompressd_wait);
> +               kcompressd_para[i].kcompressd_wait = &kcompress[i].kcompressd_wait;
> +               kcompressd_para[i].write_fifo = &kcompress[i].write_fifo;
> +               kcompressd_para[i].running = &kcompress[i].running;
> +       }
> +
> +       return 0;
> +}
> +
> +static void stop_all_kcompressd_thread(void)
> +{
> +       int i;
> +
> +       for (i = 0; i < nr_kcompressd; i++) {
> +               kthread_stop(kcompress[i].kcompressd);
> +               kcompress[i].kcompressd = NULL;
> +               clean_bio_queue(i);
> +       }
> +}
> +
> +static int do_nr_kcompressd_handler(const char *val,
> +               const struct kernel_param *kp)
> +{
> +       int ret;
> +
> +       atomic_set(&enable_kcompressd, false);
> +
> +       stop_all_kcompressd_thread();
> +
> +       ret = param_set_int(val, kp);
> +       if (!ret) {
> +               pr_err("Invalid number of kcompressd.\n");
> +               return -EINVAL;
> +       }
> +
> +       ret = init_write_queue();
> +       if (ret) {
> +               pr_err("Initialization of writing to FIFOs failed!!\n");
> +               return ret;
> +       }
> +
> +       atomic_set(&enable_kcompressd, true);
> +
> +       return 0;
> +}
> +
> +static const struct kernel_param_ops param_ops_change_nr_kcompressd = {
> +       .set = &do_nr_kcompressd_handler,
> +       .get = &param_get_uint,
> +       .free = NULL,
> +};
> +
> +module_param_cb(nr_kcompressd, &param_ops_change_nr_kcompressd,
> +               &nr_kcompressd, 0644);
> +MODULE_PARM_DESC(nr_kcompressd, "Number of pre-created daemon for page compression");
> +
> +static int do_queue_size_per_kcompressd_handler(const char *val,
> +               const struct kernel_param *kp)
> +{
> +       int ret;
> +
> +       atomic_set(&enable_kcompressd, false);
> +
> +       stop_all_kcompressd_thread();
> +
> +       ret = param_set_int(val, kp);
> +       if (!ret) {
> +               pr_err("Invalid queue size for kcompressd.\n");
> +               return -EINVAL;
> +       }
> +
> +       ret = init_write_queue();
> +       if (ret) {
> +               pr_err("Initialization of writing to FIFOs failed!!\n");
> +               return ret;
> +       }
> +
> +       pr_info("Queue size for kcompressd was changed: %d\n", queue_size_per_kcompressd);
> +
> +       atomic_set(&enable_kcompressd, true);
> +       return 0;
> +}
> +
> +static const struct kernel_param_ops param_ops_change_queue_size_per_kcompressd = {
> +       .set = &do_queue_size_per_kcompressd_handler,
> +       .get = &param_get_uint,
> +       .free = NULL,
> +};
> +
> +module_param_cb(queue_size_per_kcompressd, &param_ops_change_queue_size_per_kcompressd,
> +               &queue_size_per_kcompressd, 0644);
> +MODULE_PARM_DESC(queue_size_per_kcompressd,
> +               "Size of queue for kcompressd");
> +
> +int schedule_bio_write(void *mem, struct bio *bio, compress_callback cb)
> +{
> +       int i;
> +       bool submit_success = false;
> +       size_t sz_work = sizeof(struct write_work);
> +
> +       struct write_work entry = {
> +               .mem = mem,
> +               .bio = bio,
> +               .cb = cb
> +       };
> +
> +       if (unlikely(!atomic_read(&enable_kcompressd)))
> +               return -EBUSY;
> +
> +       if (!nr_kcompressd || !current_is_kswapd())
> +               return -EBUSY;
> +
> +       bio_get(bio);
> +
> +       for (i = 0; i < nr_kcompressd; i++) {
> +               submit_success =
> +                       (kfifo_avail(&kcompress[i].write_fifo) >= sz_work) &&
> +                       (sz_work == kfifo_in(&kcompress[i].write_fifo, &entry, sz_work));
> +
> +               if (submit_success) {
> +                       switch (atomic_read(&kcompress[i].running)) {
> +                       case KCOMPRESSD_NOT_STARTED:
> +                               atomic_set(&kcompress[i].running, KCOMPRESSD_RUNNING);
> +                               kcompress[i].kcompressd = kthread_run(kcompressd,
> +                                               &kcompressd_para[i], "kcompressd:%d", i);
> +                               if (IS_ERR(kcompress[i].kcompressd)) {
> +                                       atomic_set(&kcompress[i].running, KCOMPRESSD_NOT_STARTED);
> +                                       pr_warn("Failed to start kcompressd:%d\n", i);
> +                                       clean_bio_queue(i);
> +                               }
> +                               break;
> +                       case KCOMPRESSD_RUNNING:
> +                               break;
> +                       case KCOMPRESSD_SLEEPING:
> +                               wake_up_interruptible(&kcompress[i].kcompressd_wait);
> +                               break;
> +                       }
> +                       return 0;
> +               }
> +       }
> +
> +       bio_put(bio);
> +       return -EBUSY;
> +}
> +EXPORT_SYMBOL(schedule_bio_write);
> +
> +static int __init kcompressd_init(void)
> +{
> +       int ret;
> +
> +       nr_kcompressd = DEFAULT_NR_KCOMPRESSD;
> +       queue_size_per_kcompressd = INIT_QUEUE_SIZE;
> +
> +       ret = kcompress_update();
> +       if (ret) {
> +               pr_err("Init kcompressd failed!\n");
> +               return ret;
> +       }
> +
> +       atomic_set(&enable_kcompressd, true);
> +       blocking_notifier_call_chain(&kcompressd_notifier_list, 0, NULL);
> +       return 0;
> +}
> +
> +static void __exit kcompressd_exit(void)
> +{
> +       atomic_set(&enable_kcompressd, false);
> +       stop_all_kcompressd_thread();
> +
> +       kvfree(kcompress);
> +       kvfree(kcompressd_para);
> +}
> +
> +module_init(kcompressd_init);
> +module_exit(kcompressd_exit);
> +
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_AUTHOR("Qun-Wei Lin <[e-mail]>");
> +MODULE_DESCRIPTION("Separate the page compression from the memory reclaiming");
> +
> diff --git a/drivers/block/zram/kcompressd.h b/drivers/block/zram/kcompressd.h
> new file mode 100644
> index 000000000000..2fe0b424a7af
> --- /dev/null
> +++ b/drivers/block/zram/kcompressd.h
> @@ -0,0 +1,25 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2024 MediaTek Inc.
> + */
> +
> +#ifndef _KCOMPRESSD_H_
> +#define _KCOMPRESSD_H_
> +
> +#include <linux/rwsem.h>
> +#include <linux/kfifo.h>
> +#include <linux/atomic.h>
> +
> +typedef void (*compress_callback)(void *mem, struct bio *bio);
> +
> +struct kcompress {
> +       struct task_struct *kcompressd;
> +       wait_queue_head_t kcompressd_wait;
> +       struct kfifo write_fifo;
> +       atomic_t running;
> +};
> +
> +int kcompressd_enabled(void);
> +int schedule_bio_write(void *mem, struct bio *bio, compress_callback cb);
> +#endif
> +
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 2e1a70f2f4bd..bcd63ecb6ff2 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -35,6 +35,7 @@
>  #include <linux/part_stat.h>
>  #include <linux/kernel_read_file.h>
>
> +#include "kcompressd.h"
>  #include "zram_drv.h"
>
>  static DEFINE_IDR(zram_index_idr);
> @@ -2240,6 +2241,15 @@ static void zram_bio_write(struct zram *zram, struct bio *bio)
>         bio_endio(bio);
>  }
>
> +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> +static void zram_bio_write_callback(void *mem, struct bio *bio)
> +{
> +       struct zram *zram = (struct zram *)mem;
> +
> +       zram_bio_write(zram, bio);
> +}
> +#endif
> +
>  /*
>   * Handler function for all zram I/O requests.
>   */
> @@ -2252,6 +2262,10 @@ static void zram_submit_bio(struct bio *bio)
>                 zram_bio_read(zram, bio);
>                 break;
>         case REQ_OP_WRITE:
> +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> +               if (kcompressd_enabled() && !schedule_bio_write(zram, bio, zram_bio_write_callback))
> +                       break;
> +#endif
>                 zram_bio_write(zram, bio);
>                 break;
>         case REQ_OP_DISCARD:
> @@ -2535,9 +2549,11 @@ static int zram_add(void)
>  #if ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE
>                 .max_write_zeroes_sectors       = UINT_MAX,
>  #endif
> -               .features                       = BLK_FEAT_STABLE_WRITES        |
> -                                                 BLK_FEAT_READ_SYNCHRONOUS     |
> -                                                 BLK_FEAT_WRITE_SYNCHRONOUS,
> +               .features                       = BLK_FEAT_STABLE_WRITES
> +                                                 | BLK_FEAT_READ_SYNCHRONOUS
> +#if !IS_ENABLED(CONFIG_KCOMPRESSD)
> +                                                 | BLK_FEAT_WRITE_SYNCHRONOUS,
> +#endif
>         };
>         struct zram *zram;
>         int ret, device_id;
> --
> 2.45.2
>

Thanks
Barry

Nhat Pham March 7, 2025, 11:13 p.m. UTC | #2

On Fri, Mar 7, 2025 at 11:41 AM Barry Song <[e-mail]> wrote:
>
> On Sat, Mar 8, 2025 at 1:02 AM Qun-Wei Lin <[e-mail]> wrote:
> >
> > Introduced Kcompressd to offload zram page compression, improving
> > system efficiency by handling compression separately from memory
> > reclaiming. Added necessary configurations and dependencies.
> >
> > Signed-off-by: Qun-Wei Lin <[e-mail]>
> > ---
> >  drivers/block/zram/Kconfig      |  11 ++
> >  drivers/block/zram/Makefile     |   3 +-
> >  drivers/block/zram/kcompressd.c | 340 ++++++++++++++++++++++++++++++++
> >  drivers/block/zram/kcompressd.h |  25 +++
> >  drivers/block/zram/zram_drv.c   |  22 ++-
> >  5 files changed, 397 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/block/zram/kcompressd.c
> >  create mode 100644 drivers/block/zram/kcompressd.h
> >
> > diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> > index 402b7b175863..f0a1b574f770 100644
> > --- a/drivers/block/zram/Kconfig
> > +++ b/drivers/block/zram/Kconfig
> > @@ -145,3 +145,14 @@ config ZRAM_MULTI_COMP
> >           re-compress pages using a potentially slower but more effective
> >           compression algorithm. Note, that IDLE page recompression
> >           requires ZRAM_TRACK_ENTRY_ACTIME.
> > +
> > +config KCOMPRESSD
> > +       tristate "Kcompressd: Accelerated zram compression"
> > +       depends on ZRAM
> > +       help
> > +         Kcompressd creates multiple daemons to accelerate the compression of pages
> > +         in zram, offloading this time-consuming task from the zram driver.
> > +
> > +         This approach improves system efficiency by handling page compression separately,
> > +         which was originally done by kswapd or direct reclaim.
>
> For direct reclaim, we were previously able to compress using multiple CPUs
> with multi-threading.
> After your patch, it seems that only a single thread/CPU is used for compression
> so it won't necessarily improve direct reclaim performance?
>
> Even for kswapd, we used to have multiple threads like [kswapd0], [kswapd1],
> and [kswapd2] for different nodes. Now, are we also limited to just one thread?
> I also wonder if this could be handled at the vmscan level instead of the zram
> level. then it might potentially help other sync devices or even zswap later.

Agree. A shared solution would be much appreciated. We can keep the
kcompressd idea, but have it accept IO work from multiple sources
(zram, zswap, whatever) through a shared API.

Otherwise we would need to reinvent the wheel multiple times :)

Nhat Pham March 7, 2025, 11:14 p.m. UTC | #3

On Fri, Mar 7, 2025 at 3:13 PM Nhat Pham <[e-mail]> wrote:
>
>
> Agree. A shared solution would be much appreciated. We can keep the
> kcompressd idea, but have it accept IO work from multiple sources
> (zram, zswap, whatever) through a shared API.

by IO I meant compress work (should be double quoted "IO"). But you
get the idea :)

Nhat Pham March 9, 2025, 7:56 p.m. UTC | #4

On Sat, Mar 8, 2025 at 5:05 PM Hillf Danton <[e-mail]> wrote:
>
> Could you explain what nr_kcompressd means, Qun-Wei, to quiesce barking lads?

Who's the "barking lads" you are referring to? Please mind your language.

Barry Song March 9, 2025, 8:44 p.m. UTC | #5

On Mon, Mar 10, 2025 at 8:56 AM Nhat Pham <[e-mail]> wrote:
>
> On Sat, Mar 8, 2025 at 5:05 PM Hillf Danton <[e-mail]> wrote:
> >
> > Could you explain what nr_kcompressd means, Qun-Wei, to quiesce barking lads?
>
> Who's the "barking lads" you are referring to? Please mind your language.

I also feel extremely uncomfortable. In Eastern culture, this is an extremely
vulgar word, more offensive than any others.

I strongly feel that this violates the mutual respect within the Linux
community. This is a serious case of verbal abuse.

Regardless of the existence of nr_kcompressd, it is still unacceptable to
invent an interface that requires users to figure out how to set it up, while
kswapd can launch threads based on NUMA nodes.
This should be transparent to users, just as kswapd does.

void __meminit kswapd_run(int nid)

{
        ...
        if (!pgdat->kswapd) {
                pgdat->kswapd = kthread_create_on_node(kswapd, pgdat,
nid, "kswapd%d", nid);
                ...
        }
        pgdat_kswapd_unlock(pgdat);
}

On the other hand, no one will know how to set up the proper number of
threads, while direct reclaim can utilize each CPU.

Therefore, listing nr_kcompressd does not change the essence of my
question.

Thanks
Barry

Nhat Pham March 9, 2025, 10:20 p.m. UTC | #6

On Sun, Mar 9, 2025 at 1:44 PM Barry Song <[e-mail]> wrote:
>
> On Mon, Mar 10, 2025 at 8:56 AM Nhat Pham <[e-mail]> wrote:
> >
> > On Sat, Mar 8, 2025 at 5:05 PM Hillf Danton <[e-mail]> wrote:
> > >
> > > Could you explain what nr_kcompressd means, Qun-Wei, to quiesce barking lads?
> >
> > Who's the "barking lads" you are referring to? Please mind your language.
>
> I also feel extremely uncomfortable. In Eastern culture, this is an extremely
> vulgar word, more offensive than any others.
>
> I strongly feel that this violates the mutual respect within the Linux
> community. This is a serious case of verbal abuse.
>
> Regardless of the existence of nr_kcompressd, it is still unacceptable to
> invent an interface that requires users to figure out how to set it up, while
> kswapd can launch threads based on NUMA nodes.
> This should be transparent to users, just as kswapd does.
>
> void __meminit kswapd_run(int nid)
>
> {
>         ...
>         if (!pgdat->kswapd) {
>                 pgdat->kswapd = kthread_create_on_node(kswapd, pgdat,
> nid, "kswapd%d", nid);
>                 ...
>         }
>         pgdat_kswapd_unlock(pgdat);
> }
>
> On the other hand, no one will know how to set up the proper number of
> threads, while direct reclaim can utilize each CPU.

Agree - how are users supposed to set this? The default puzzles me
too. Why 4? Does it work across architectures? Across workloads?

This makes no sense to me. Can we scale the number of threads in
proportion to the number of CPUs? Per-cpu kcompressd?

Qun-wei Lin (林群崴) March 10, 2025, 1:23 p.m. UTC | #7

On Sun, 2025-03-09 at 15:20 -0700, Nhat Pham wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Sun, Mar 9, 2025 at 1:44 PM Barry Song <[e-mail]> wrote:
> > 
> > On Mon, Mar 10, 2025 at 8:56 AM Nhat Pham <[e-mail]>
> > wrote:
> > > 
> > > On Sat, Mar 8, 2025 at 5:05 PM Hillf Danton <[e-mail]>
> > > wrote:
> > > > 
> > > > Could you explain what nr_kcompressd means, Qun-Wei, to quiesce
> > > > barking lads?
> > > 
> > > Who's the "barking lads" you are referring to? Please mind your
> > > language.
> > 
> > I also feel extremely uncomfortable. In Eastern culture, this is an
> > extremely
> > vulgar word, more offensive than any others.
> > 
> > I strongly feel that this violates the mutual respect within the
> > Linux
> > community. This is a serious case of verbal abuse.
> > 
> > Regardless of the existence of nr_kcompressd, it is still
> > unacceptable to
> > invent an interface that requires users to figure out how to set it
> > up, while
> > kswapd can launch threads based on NUMA nodes.
> > This should be transparent to users, just as kswapd does.
> > 
> > void __meminit kswapd_run(int nid)
> > 
> > {
> >         ...
> >         if (!pgdat->kswapd) {
> >                 pgdat->kswapd = kthread_create_on_node(kswapd,
> > pgdat,
> > nid, "kswapd%d", nid);
> >                 ...
> >         }
> >         pgdat_kswapd_unlock(pgdat);
> > }
> > 
> > On the other hand, no one will know how to set up the proper number
> > of
> > threads, while direct reclaim can utilize each CPU.
> 
> Agree - how are users supposed to set this? The default puzzles me
> too. Why 4? Does it work across architectures? Across workloads?
> 
> This makes no sense to me. Can we scale the number of threads in
> proportion to the number of CPUs? Per-cpu kcompressd?

The default value is actually designed to be the maximum number of
kcompressd that can run simultaneously. The current design is to create
the next kcompressd thread when all kfifos are full, and the number 4
is just a temporary setting. 

You are right, changing it to match the number of CPUs might be better
to avoid confusion on how to set it up.


Best Regards,
Qun-wei

Qun-wei Lin (林群崴) March 10, 2025, 1:26 p.m. UTC | #8

On Fri, 2025-03-07 at 15:14 -0800, Nhat Pham wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Fri, Mar 7, 2025 at 3:13 PM Nhat Pham <[e-mail]> wrote:
> > 
> > 
> > Agree. A shared solution would be much appreciated. We can keep the
> > kcompressd idea, but have it accept IO work from multiple sources
> > (zram, zswap, whatever) through a shared API.
> 
> by IO I meant compress work (should be double quoted "IO"). But you
> get the idea :)

I also agree that we can evolve into a more general solution.
So this is also why we designed kcompressd to do writeback using
callbacks rather than hard coding it directly into zram.

But currently we only extend Zram. If anyone has good suggestions, we
would greatly appreciate your help.

Thanks!
Qun-wei

Qun-wei Lin (林群崴) March 10, 2025, 1:26 p.m. UTC | #9

On Sat, 2025-03-08 at 08:41 +1300, Barry Song wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Sat, Mar 8, 2025 at 1:02 AM Qun-Wei Lin <[e-mail]>
> wrote:
> > 
> > Introduced Kcompressd to offload zram page compression, improving
> > system efficiency by handling compression separately from memory
> > reclaiming. Added necessary configurations and dependencies.
> > 
> > Signed-off-by: Qun-Wei Lin <[e-mail]>
> > ---
> >  drivers/block/zram/Kconfig      |  11 ++
> >  drivers/block/zram/Makefile     |   3 +-
> >  drivers/block/zram/kcompressd.c | 340
> > ++++++++++++++++++++++++++++++++
> >  drivers/block/zram/kcompressd.h |  25 +++
> >  drivers/block/zram/zram_drv.c   |  22 ++-
> >  5 files changed, 397 insertions(+), 4 deletions(-)
> >  create mode 100644 drivers/block/zram/kcompressd.c
> >  create mode 100644 drivers/block/zram/kcompressd.h
> > 
> > diff --git a/drivers/block/zram/Kconfig
> > b/drivers/block/zram/Kconfig
> > index 402b7b175863..f0a1b574f770 100644
> > --- a/drivers/block/zram/Kconfig
> > +++ b/drivers/block/zram/Kconfig
> > @@ -145,3 +145,14 @@ config ZRAM_MULTI_COMP
> >           re-compress pages using a potentially slower but more
> > effective
> >           compression algorithm. Note, that IDLE page recompression
> >           requires ZRAM_TRACK_ENTRY_ACTIME.
> > +
> > +config KCOMPRESSD
> > +       tristate "Kcompressd: Accelerated zram compression"
> > +       depends on ZRAM
> > +       help
> > +         Kcompressd creates multiple daemons to accelerate the
> > compression of pages
> > +         in zram, offloading this time-consuming task from the
> > zram driver.
> > +
> > +         This approach improves system efficiency by handling page
> > compression separately,
> > +         which was originally done by kswapd or direct reclaim.
> 
> For direct reclaim, we were previously able to compress using
> multiple CPUs
> with multi-threading.
> After your patch, it seems that only a single thread/CPU is used for
> compression
> so it won't necessarily improve direct reclaim performance?
> 

Our patch only splits the context of kswapd. When direct reclaim is
occurred, it is bypassed, so direct reclaim remains unchanged, with
each thread that needs memory directly reclaiming it.

> Even for kswapd, we used to have multiple threads like [kswapd0],
> [kswapd1],
> and [kswapd2] for different nodes. Now, are we also limited to just
> one thread?

We only considered a single kswapd here and didn't account for multiple
instances. Since I use kfifo to collect the bios, if there are multiple
kswapds, we need to add a lock to protect the bio queue. I can revise
this in the 2nd version, or do you have any other suggested approaches?

> I also wonder if this could be handled at the vmscan level instead of
> the zram
> level. then it might potentially help other sync devices or even
> zswap later.
> 
> But I agree that for phones, modifying zram seems like an easier
> starting
> point. However, relying on a single thread isn't always the best
> approach.
> 
> 
> > +
> > diff --git a/drivers/block/zram/Makefile
> > b/drivers/block/zram/Makefile
> > index 0fdefd576691..23baa5dfceb9 100644
> > --- a/drivers/block/zram/Makefile
> > +++ b/drivers/block/zram/Makefile
> > @@ -9,4 +9,5 @@ zram-$(CONFIG_ZRAM_BACKEND_ZSTD)        +=
> > backend_zstd.o
> >  zram-$(CONFIG_ZRAM_BACKEND_DEFLATE)    += backend_deflate.o
> >  zram-$(CONFIG_ZRAM_BACKEND_842)                += backend_842.o
> > 
> > -obj-$(CONFIG_ZRAM)     +=      zram.o
> > +obj-$(CONFIG_ZRAM)             += zram.o
> > +obj-$(CONFIG_KCOMPRESSD)       += kcompressd.o
> > diff --git a/drivers/block/zram/kcompressd.c
> > b/drivers/block/zram/kcompressd.c
> > new file mode 100644
> > index 000000000000..195b7e386869
> > --- /dev/null
> > +++ b/drivers/block/zram/kcompressd.c
> > @@ -0,0 +1,340 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2024 MediaTek Inc.
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/kernel.h>
> > +#include <linux/bio.h>
> > +#include <linux/bitops.h>
> > +#include <linux/freezer.h>
> > +#include <linux/kernel.h>
> > +#include <linux/psi.h>
> > +#include <linux/kfifo.h>
> > +#include <linux/swap.h>
> > +#include <linux/delay.h>
> > +
> > +#include "kcompressd.h"
> > +
> > +#define INIT_QUEUE_SIZE                4096
> > +#define DEFAULT_NR_KCOMPRESSD  4
> > +
> > +static atomic_t enable_kcompressd;
> > +static unsigned int nr_kcompressd;
> > +static unsigned int queue_size_per_kcompressd;
> > +static struct kcompress *kcompress;
> > +
> > +enum run_state {
> > +       KCOMPRESSD_NOT_STARTED = 0,
> > +       KCOMPRESSD_RUNNING,
> > +       KCOMPRESSD_SLEEPING,
> > +};
> > +
> > +struct kcompressd_para {
> > +       wait_queue_head_t *kcompressd_wait;
> > +       struct kfifo *write_fifo;
> > +       atomic_t *running;
> > +};
> > +
> > +static struct kcompressd_para *kcompressd_para;
> > +static BLOCKING_NOTIFIER_HEAD(kcompressd_notifier_list);
> > +
> > +struct write_work {
> > +       void *mem;
> > +       struct bio *bio;
> > +       compress_callback cb;
> > +};
> > +
> > +int kcompressd_enabled(void)
> > +{
> > +       return likely(atomic_read(&enable_kcompressd));
> > +}
> > +EXPORT_SYMBOL(kcompressd_enabled);
> > +
> > +static void kcompressd_try_to_sleep(struct kcompressd_para *p)
> > +{
> > +       DEFINE_WAIT(wait);
> > +
> > +       if (!kfifo_is_empty(p->write_fifo))
> > +               return;
> > +
> > +       if (freezing(current) || kthread_should_stop())
> > +               return;
> > +
> > +       atomic_set(p->running, KCOMPRESSD_SLEEPING);
> > +       prepare_to_wait(p->kcompressd_wait, &wait,
> > TASK_INTERRUPTIBLE);
> > +
> > +       /*
> > +        * After a short sleep, check if it was a premature sleep.
> > If not, then
> > +        * go fully to sleep until explicitly woken up.
> > +        */
> > +       if (!kthread_should_stop() && kfifo_is_empty(p-
> > >write_fifo))
> > +               schedule();
> > +
> > +       finish_wait(p->kcompressd_wait, &wait);
> > +       atomic_set(p->running, KCOMPRESSD_RUNNING);
> > +}
> > +
> > +static int kcompressd(void *para)
> > +{
> > +       struct task_struct *tsk = current;
> > +       struct kcompressd_para *p = (struct kcompressd_para *)para;
> > +
> > +       tsk->flags |= PF_MEMALLOC | PF_KSWAPD;
> > +       set_freezable();
> > +
> > +       while (!kthread_should_stop()) {
> > +               bool ret;
> > +
> > +               kcompressd_try_to_sleep(p);
> > +               ret = try_to_freeze();
> > +               if (kthread_should_stop())
> > +                       break;
> > +
> > +               if (ret)
> > +                       continue;
> > +
> > +               while (!kfifo_is_empty(p->write_fifo)) {
> > +                       struct write_work entry;
> > +
> > +                       if (sizeof(struct write_work) ==
> > kfifo_out(p->write_fifo,
> > +                                               &entry,
> > sizeof(struct write_work))) {
> > +                               entry.cb(entry.mem, entry.bio);
> > +                               bio_put(entry.bio);
> > +                       }
> > +               }
> > +
> > +       }
> > +
> > +       tsk->flags &= ~(PF_MEMALLOC | PF_KSWAPD);
> > +       atomic_set(p->running, KCOMPRESSD_NOT_STARTED);
> > +       return 0;
> > +}
> > +
> > +static int init_write_queue(void)
> > +{
> > +       int i;
> > +       unsigned int queue_len = queue_size_per_kcompressd *
> > sizeof(struct write_work);
> > +
> > +       for (i = 0; i < nr_kcompressd; i++) {
> > +               if (kfifo_alloc(&kcompress[i].write_fifo,
> > +                                       queue_len, GFP_KERNEL)) {
> > +                       pr_err("Failed to alloc kfifo %d\n", i);
> > +                       return -ENOMEM;
> > +               }
> > +       }
> > +       return 0;
> > +}
> > +
> > +static void clean_bio_queue(int idx)
> > +{
> > +       struct write_work entry;
> > +
> > +       while (sizeof(struct write_work) ==
> > kfifo_out(&kcompress[idx].write_fifo,
> > +                               &entry, sizeof(struct write_work)))
> > {
> > +               bio_put(entry.bio);
> > +               entry.cb(entry.mem, entry.bio);
> > +       }
> > +       kfifo_free(&kcompress[idx].write_fifo);
> > +}
> > +
> > +static int kcompress_update(void)
> > +{
> > +       int i;
> > +       int ret;
> > +
> > +       kcompress = kvmalloc_array(nr_kcompressd, sizeof(struct
> > kcompress), GFP_KERNEL);
> > +       if (!kcompress)
> > +               return -ENOMEM;
> > +
> > +       kcompressd_para = kvmalloc_array(nr_kcompressd,
> > sizeof(struct kcompressd_para), GFP_KERNEL);
> > +       if (!kcompressd_para)
> > +               return -ENOMEM;
> > +
> > +       ret = init_write_queue();
> > +       if (ret) {
> > +               pr_err("Initialization of writing to FIFOs
> > failed!!\n");
> > +               return ret;
> > +       }
> > +
> > +       for (i = 0; i < nr_kcompressd; i++) {
> > +               init_waitqueue_head(&kcompress[i].kcompressd_wait);
> > +               kcompressd_para[i].kcompressd_wait =
> > &kcompress[i].kcompressd_wait;
> > +               kcompressd_para[i].write_fifo =
> > &kcompress[i].write_fifo;
> > +               kcompressd_para[i].running = &kcompress[i].running;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +static void stop_all_kcompressd_thread(void)
> > +{
> > +       int i;
> > +
> > +       for (i = 0; i < nr_kcompressd; i++) {
> > +               kthread_stop(kcompress[i].kcompressd);
> > +               kcompress[i].kcompressd = NULL;
> > +               clean_bio_queue(i);
> > +       }
> > +}
> > +
> > +static int do_nr_kcompressd_handler(const char *val,
> > +               const struct kernel_param *kp)
> > +{
> > +       int ret;
> > +
> > +       atomic_set(&enable_kcompressd, false);
> > +
> > +       stop_all_kcompressd_thread();
> > +
> > +       ret = param_set_int(val, kp);
> > +       if (!ret) {
> > +               pr_err("Invalid number of kcompressd.\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = init_write_queue();
> > +       if (ret) {
> > +               pr_err("Initialization of writing to FIFOs
> > failed!!\n");
> > +               return ret;
> > +       }
> > +
> > +       atomic_set(&enable_kcompressd, true);
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct kernel_param_ops
> > param_ops_change_nr_kcompressd = {
> > +       .set = &do_nr_kcompressd_handler,
> > +       .get = &param_get_uint,
> > +       .free = NULL,
> > +};
> > +
> > +module_param_cb(nr_kcompressd, &param_ops_change_nr_kcompressd,
> > +               &nr_kcompressd, 0644);
> > +MODULE_PARM_DESC(nr_kcompressd, "Number of pre-created daemon for
> > page compression");
> > +
> > +static int do_queue_size_per_kcompressd_handler(const char *val,
> > +               const struct kernel_param *kp)
> > +{
> > +       int ret;
> > +
> > +       atomic_set(&enable_kcompressd, false);
> > +
> > +       stop_all_kcompressd_thread();
> > +
> > +       ret = param_set_int(val, kp);
> > +       if (!ret) {
> > +               pr_err("Invalid queue size for kcompressd.\n");
> > +               return -EINVAL;
> > +       }
> > +
> > +       ret = init_write_queue();
> > +       if (ret) {
> > +               pr_err("Initialization of writing to FIFOs
> > failed!!\n");
> > +               return ret;
> > +       }
> > +
> > +       pr_info("Queue size for kcompressd was changed: %d\n",
> > queue_size_per_kcompressd);
> > +
> > +       atomic_set(&enable_kcompressd, true);
> > +       return 0;
> > +}
> > +
> > +static const struct kernel_param_ops
> > param_ops_change_queue_size_per_kcompressd = {
> > +       .set = &do_queue_size_per_kcompressd_handler,
> > +       .get = &param_get_uint,
> > +       .free = NULL,
> > +};
> > +
> > +module_param_cb(queue_size_per_kcompressd,
> > &param_ops_change_queue_size_per_kcompressd,
> > +               &queue_size_per_kcompressd, 0644);
> > +MODULE_PARM_DESC(queue_size_per_kcompressd,
> > +               "Size of queue for kcompressd");
> > +
> > +int schedule_bio_write(void *mem, struct bio *bio,
> > compress_callback cb)
> > +{
> > +       int i;
> > +       bool submit_success = false;
> > +       size_t sz_work = sizeof(struct write_work);
> > +
> > +       struct write_work entry = {
> > +               .mem = mem,
> > +               .bio = bio,
> > +               .cb = cb
> > +       };
> > +
> > +       if (unlikely(!atomic_read(&enable_kcompressd)))
> > +               return -EBUSY;
> > +
> > +       if (!nr_kcompressd || !current_is_kswapd())
> > +               return -EBUSY;
> > +
> > +       bio_get(bio);
> > +
> > +       for (i = 0; i < nr_kcompressd; i++) {
> > +               submit_success =
> > +                       (kfifo_avail(&kcompress[i].write_fifo) >=
> > sz_work) &&
> > +                       (sz_work ==
> > kfifo_in(&kcompress[i].write_fifo, &entry, sz_work));
> > +
> > +               if (submit_success) {
> > +                       switch (atomic_read(&kcompress[i].running))
> > {
> > +                       case KCOMPRESSD_NOT_STARTED:
> > +                               atomic_set(&kcompress[i].running,
> > KCOMPRESSD_RUNNING);
> > +                               kcompress[i].kcompressd =
> > kthread_run(kcompressd,
> > +                                              
> > &kcompressd_para[i], "kcompressd:%d", i);
> > +                               if
> > (IS_ERR(kcompress[i].kcompressd)) {
> > +                                      
> > atomic_set(&kcompress[i].running, KCOMPRESSD_NOT_STARTED);
> > +                                       pr_warn("Failed to start
> > kcompressd:%d\n", i);
> > +                                       clean_bio_queue(i);
> > +                               }
> > +                               break;
> > +                       case KCOMPRESSD_RUNNING:
> > +                               break;
> > +                       case KCOMPRESSD_SLEEPING:
> > +                              
> > wake_up_interruptible(&kcompress[i].kcompressd_wait);
> > +                               break;
> > +                       }
> > +                       return 0;
> > +               }
> > +       }
> > +
> > +       bio_put(bio);
> > +       return -EBUSY;
> > +}
> > +EXPORT_SYMBOL(schedule_bio_write);
> > +
> > +static int __init kcompressd_init(void)
> > +{
> > +       int ret;
> > +
> > +       nr_kcompressd = DEFAULT_NR_KCOMPRESSD;
> > +       queue_size_per_kcompressd = INIT_QUEUE_SIZE;
> > +
> > +       ret = kcompress_update();
> > +       if (ret) {
> > +               pr_err("Init kcompressd failed!\n");
> > +               return ret;
> > +       }
> > +
> > +       atomic_set(&enable_kcompressd, true);
> > +       blocking_notifier_call_chain(&kcompressd_notifier_list, 0,
> > NULL);
> > +       return 0;
> > +}
> > +
> > +static void __exit kcompressd_exit(void)
> > +{
> > +       atomic_set(&enable_kcompressd, false);
> > +       stop_all_kcompressd_thread();
> > +
> > +       kvfree(kcompress);
> > +       kvfree(kcompressd_para);
> > +}
> > +
> > +module_init(kcompressd_init);
> > +module_exit(kcompressd_exit);
> > +
> > +MODULE_LICENSE("Dual BSD/GPL");
> > +MODULE_AUTHOR("Qun-Wei Lin <[e-mail]>");
> > +MODULE_DESCRIPTION("Separate the page compression from the memory
> > reclaiming");
> > +
> > diff --git a/drivers/block/zram/kcompressd.h
> > b/drivers/block/zram/kcompressd.h
> > new file mode 100644
> > index 000000000000..2fe0b424a7af
> > --- /dev/null
> > +++ b/drivers/block/zram/kcompressd.h
> > @@ -0,0 +1,25 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Copyright (C) 2024 MediaTek Inc.
> > + */
> > +
> > +#ifndef _KCOMPRESSD_H_
> > +#define _KCOMPRESSD_H_
> > +
> > +#include <linux/rwsem.h>
> > +#include <linux/kfifo.h>
> > +#include <linux/atomic.h>
> > +
> > +typedef void (*compress_callback)(void *mem, struct bio *bio);
> > +
> > +struct kcompress {
> > +       struct task_struct *kcompressd;
> > +       wait_queue_head_t kcompressd_wait;
> > +       struct kfifo write_fifo;
> > +       atomic_t running;
> > +};
> > +
> > +int kcompressd_enabled(void);
> > +int schedule_bio_write(void *mem, struct bio *bio,
> > compress_callback cb);
> > +#endif
> > +
> > diff --git a/drivers/block/zram/zram_drv.c
> > b/drivers/block/zram/zram_drv.c
> > index 2e1a70f2f4bd..bcd63ecb6ff2 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -35,6 +35,7 @@
> >  #include <linux/part_stat.h>
> >  #include <linux/kernel_read_file.h>
> > 
> > +#include "kcompressd.h"
> >  #include "zram_drv.h"
> > 
> >  static DEFINE_IDR(zram_index_idr);
> > @@ -2240,6 +2241,15 @@ static void zram_bio_write(struct zram
> > *zram, struct bio *bio)
> >         bio_endio(bio);
> >  }
> > 
> > +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> > +static void zram_bio_write_callback(void *mem, struct bio *bio)
> > +{
> > +       struct zram *zram = (struct zram *)mem;
> > +
> > +       zram_bio_write(zram, bio);
> > +}
> > +#endif
> > +
> >  /*
> >   * Handler function for all zram I/O requests.
> >   */
> > @@ -2252,6 +2262,10 @@ static void zram_submit_bio(struct bio *bio)
> >                 zram_bio_read(zram, bio);
> >                 break;
> >         case REQ_OP_WRITE:
> > +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> > +               if (kcompressd_enabled() &&
> > !schedule_bio_write(zram, bio, zram_bio_write_callback))
> > +                       break;
> > +#endif
> >                 zram_bio_write(zram, bio);
> >                 break;
> >         case REQ_OP_DISCARD:
> > @@ -2535,9 +2549,11 @@ static int zram_add(void)
> >  #if ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE
> >                 .max_write_zeroes_sectors       = UINT_MAX,
> >  #endif
> > -               .features                       =
> > BLK_FEAT_STABLE_WRITES        |
> > -                                                
> > BLK_FEAT_READ_SYNCHRONOUS     |
> > -                                                
> > BLK_FEAT_WRITE_SYNCHRONOUS,
> > +               .features                       =
> > BLK_FEAT_STABLE_WRITES
> > +                                                 |
> > BLK_FEAT_READ_SYNCHRONOUS
> > +#if !IS_ENABLED(CONFIG_KCOMPRESSD)
> > +                                                 |
> > BLK_FEAT_WRITE_SYNCHRONOUS,
> > +#endif
> >         };
> >         struct zram *zram;
> >         int ret, device_id;
> > --
> > 2.45.2
> > 
> 
> Thanks
> Barry

Barry Song March 10, 2025, 5:44 p.m. UTC | #10

On Mon, Mar 10, 2025 at 6:34 PM Hillf Danton <[e-mail]> wrote:
>
> On Mon, 10 Mar 2025 09:44:24 +1300 Barry Song <[e-mail]>
> > I also feel extremely uncomfortable. In Eastern culture, this is an extremely
> > vulgar word, more offensive than any others.
> >
> If culture is not abused, feel free to show us how it is defined to be
> more offensive than any others in Eastern culture.
>

Having no manners is not your fault. The ignorant fear nothing.

> > I strongly feel that this violates the mutual respect within the Linux
> > community. This is a serious case of verbal abuse.
> >
> You will feel better I think after proving that your patch/comment is
> able to escape standing ovation, given respect is not free, lad.

It won't happen. A snake won't stop biting just because humans have
fed it.

I believe replying to you will only fuel your excitement, so this will be
the last email I ever respond to. Kindly do not reply.

Thanks
Barry

Barry Song March 11, 2025, 3:57 a.m. UTC | #11

On Tue, Mar 11, 2025 at 12:09 PM Hillf Danton <[e-mail]> wrote:
>
> On Tue, 11 Mar 2025 01:44:27 +0800 Barry Song <[e-mail]>
> > On Mon, Mar 10, 2025 at 6:34 PM Hillf Danton <[e-mail]> wrote:
> > > On Mon, 10 Mar 2025 09:44:24 +1300 Barry Song <[e-mail]>
> > > > I also feel extremely uncomfortable. In Eastern culture, this is an extremely
> > > > vulgar word, more offensive than any others.
> > > >
> > > If culture is not abused, feel free to show us how it is defined to be
> > > more offensive than any others in Eastern culture.
> > >
> > Having no manners is not your fault. The ignorant fear nothing.
> >
> Oh lad, just tell us anything true. It is not difficult.

+Code of conduct.

I know there is already a ban for this person in another thread [1],
but I am also
officially requesting an additional ban. Having contributed to the
Linux kernel for
over 15+ years, both professionally and out of personal interest, I have never
encountered a developer who engages in random and irrational personal
attacks[2] and persistently did this despite clear requests to stop. This is
utterly bizarre.

Please give serious consideration to extending this guy’s ban on a
case-by-case basis.

[1] https://lore.kernel.org/all/[e-mail]/
[2] https://lore.kernel.org/all/[e-mail]/

Thanks
Barry

Sergey Senozhatsky March 11, 2025, 5:02 a.m. UTC | #12

On (25/03/07 15:13), Nhat Pham wrote:
> > > +config KCOMPRESSD
> > > +       tristate "Kcompressd: Accelerated zram compression"
> > > +       depends on ZRAM
> > > +       help
> > > +         Kcompressd creates multiple daemons to accelerate the compression of pages
> > > +         in zram, offloading this time-consuming task from the zram driver.
> > > +
> > > +         This approach improves system efficiency by handling page compression separately,
> > > +         which was originally done by kswapd or direct reclaim.
> >
> > For direct reclaim, we were previously able to compress using multiple CPUs
> > with multi-threading.
> > After your patch, it seems that only a single thread/CPU is used for compression
> > so it won't necessarily improve direct reclaim performance?
> >
> > Even for kswapd, we used to have multiple threads like [kswapd0], [kswapd1],
> > and [kswapd2] for different nodes. Now, are we also limited to just one thread?
> > I also wonder if this could be handled at the vmscan level instead of the zram
> > level. then it might potentially help other sync devices or even zswap later.
> 
> Agree. A shared solution would be much appreciated. We can keep the
> kcompressd idea, but have it accept IO work from multiple sources
> (zram, zswap, whatever) through a shared API.

I guess it also need to take swapoff into consideration (especially
if it takes I/O from multiple sources)?

Greg KH March 11, 2025, 6:36 a.m. UTC | #13

On Tue, Mar 11, 2025 at 04:57:19PM +1300, Barry Song wrote:
> On Tue, Mar 11, 2025 at 12:09 PM Hillf Danton <[e-mail]> wrote:
> >
> > On Tue, 11 Mar 2025 01:44:27 +0800 Barry Song <[e-mail]>
> > > On Mon, Mar 10, 2025 at 6:34 PM Hillf Danton <[e-mail]> wrote:
> > > > On Mon, 10 Mar 2025 09:44:24 +1300 Barry Song <[e-mail]>
> > > > > I also feel extremely uncomfortable. In Eastern culture, this is an extremely
> > > > > vulgar word, more offensive than any others.
> > > > >
> > > > If culture is not abused, feel free to show us how it is defined to be
> > > > more offensive than any others in Eastern culture.
> > > >
> > > Having no manners is not your fault. The ignorant fear nothing.
> > >
> > Oh lad, just tell us anything true. It is not difficult.
> 
> +Code of conduct.
> 
> I know there is already a ban for this person in another thread [1],
> but I am also
> officially requesting an additional ban. Having contributed to the
> Linux kernel for
> over 15+ years, both professionally and out of personal interest, I have never
> encountered a developer who engages in random and irrational personal
> attacks[2] and persistently did this despite clear requests to stop. This is
> utterly bizarre.
> 
> Please give serious consideration to extending this guy’s ban on a
> case-by-case basis.
> 
> [1] https://lore.kernel.org/all/[e-mail]/
> [2] https://lore.kernel.org/all/[e-mail]/

We will consider it, thank you for letting us know.

greg k-h

Barry Song March 11, 2025, 7:05 a.m. UTC | #14

On Tue, Mar 11, 2025 at 2:26 AM Qun-wei Lin (林群崴)
<[e-mail]> wrote:
>
> On Sat, 2025-03-08 at 08:41 +1300, Barry Song wrote:
> >
> > External email : Please do not click links or open attachments until
> > you have verified the sender or the content.
> >
> >
> > On Sat, Mar 8, 2025 at 1:02 AM Qun-Wei Lin <[e-mail]>
> > wrote:
> > >
> > > Introduced Kcompressd to offload zram page compression, improving
> > > system efficiency by handling compression separately from memory
> > > reclaiming. Added necessary configurations and dependencies.
> > >
> > > Signed-off-by: Qun-Wei Lin <[e-mail]>
> > > ---
> > >  drivers/block/zram/Kconfig      |  11 ++
> > >  drivers/block/zram/Makefile     |   3 +-
> > >  drivers/block/zram/kcompressd.c | 340
> > > ++++++++++++++++++++++++++++++++
> > >  drivers/block/zram/kcompressd.h |  25 +++
> > >  drivers/block/zram/zram_drv.c   |  22 ++-
> > >  5 files changed, 397 insertions(+), 4 deletions(-)
> > >  create mode 100644 drivers/block/zram/kcompressd.c
> > >  create mode 100644 drivers/block/zram/kcompressd.h
> > >
> > > diff --git a/drivers/block/zram/Kconfig
> > > b/drivers/block/zram/Kconfig
> > > index 402b7b175863..f0a1b574f770 100644
> > > --- a/drivers/block/zram/Kconfig
> > > +++ b/drivers/block/zram/Kconfig
> > > @@ -145,3 +145,14 @@ config ZRAM_MULTI_COMP
> > >           re-compress pages using a potentially slower but more
> > > effective
> > >           compression algorithm. Note, that IDLE page recompression
> > >           requires ZRAM_TRACK_ENTRY_ACTIME.
> > > +
> > > +config KCOMPRESSD
> > > +       tristate "Kcompressd: Accelerated zram compression"
> > > +       depends on ZRAM
> > > +       help
> > > +         Kcompressd creates multiple daemons to accelerate the
> > > compression of pages
> > > +         in zram, offloading this time-consuming task from the
> > > zram driver.
> > > +
> > > +         This approach improves system efficiency by handling page
> > > compression separately,
> > > +         which was originally done by kswapd or direct reclaim.
> >
> > For direct reclaim, we were previously able to compress using
> > multiple CPUs
> > with multi-threading.
> > After your patch, it seems that only a single thread/CPU is used for
> > compression
> > so it won't necessarily improve direct reclaim performance?
> >
>
> Our patch only splits the context of kswapd. When direct reclaim is
> occurred, it is bypassed, so direct reclaim remains unchanged, with
> each thread that needs memory directly reclaiming it.

Qun-wei, I’m getting a bit confused. Looking at the code in page_io.c,
you always call swap_writepage_bdev_async() no matter if it is kswapd
or direct reclaim:

- else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
+ else if (data_race(sis->flags & SWP_WRITE_SYNCHRONOUS_IO))
           swap_writepage_bdev_sync(folio, wbc, sis);
  else
            swap_writepage_bdev_async(folio, wbc, sis);

In zram, I notice you are bypassing kcompressd by:

+ if (!nr_kcompressd || !current_is_kswapd())
+        return -EBUSY;

How will this work if no one is calling __end_swap_bio_write(&bio),
which is present in swap_writepage_bdev_sync()?
Am I missing something? Or is it done by zram_bio_write() ?

On the other hand, zram is a generic block device, and coupling its
code with kswapd/direct reclaim somehow violates layering
principles :-)

>
> > Even for kswapd, we used to have multiple threads like [kswapd0],
> > [kswapd1],
> > and [kswapd2] for different nodes. Now, are we also limited to just
> > one thread?
>
> We only considered a single kswapd here and didn't account for multiple
> instances. Since I use kfifo to collect the bios, if there are multiple
> kswapds, we need to add a lock to protect the bio queue. I can revise
> this in the 2nd version, or do you have any other suggested approaches?

I'm wondering if we can move the code to vmscan/page_io instead
of zram. If we're using a sync I/O swap device or have enabled zswap,
we could run reclamation in this separate thread, which should also be
NUMA-aware.

I would definitely be interested in prototyping it when I have the time.

>
> > I also wonder if this could be handled at the vmscan level instead of
> > the zram
> > level. then it might potentially help other sync devices or even
> > zswap later.
> >
> > But I agree that for phones, modifying zram seems like an easier
> > starting
> > point. However, relying on a single thread isn't always the best
> > approach.
> >
> >
> > > +
> > > diff --git a/drivers/block/zram/Makefile
> > > b/drivers/block/zram/Makefile
> > > index 0fdefd576691..23baa5dfceb9 100644
> > > --- a/drivers/block/zram/Makefile
> > > +++ b/drivers/block/zram/Makefile
> > > @@ -9,4 +9,5 @@ zram-$(CONFIG_ZRAM_BACKEND_ZSTD)        +=
> > > backend_zstd.o
> > >  zram-$(CONFIG_ZRAM_BACKEND_DEFLATE)    += backend_deflate.o
> > >  zram-$(CONFIG_ZRAM_BACKEND_842)                += backend_842.o
> > >
> > > -obj-$(CONFIG_ZRAM)     +=      zram.o
> > > +obj-$(CONFIG_ZRAM)             += zram.o
> > > +obj-$(CONFIG_KCOMPRESSD)       += kcompressd.o
> > > diff --git a/drivers/block/zram/kcompressd.c
> > > b/drivers/block/zram/kcompressd.c
> > > new file mode 100644
> > > index 000000000000..195b7e386869
> > > --- /dev/null
> > > +++ b/drivers/block/zram/kcompressd.c
> > > @@ -0,0 +1,340 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Copyright (C) 2024 MediaTek Inc.
> > > + */
> > > +
> > > +#include <linux/module.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/bio.h>
> > > +#include <linux/bitops.h>
> > > +#include <linux/freezer.h>
> > > +#include <linux/kernel.h>
> > > +#include <linux/psi.h>
> > > +#include <linux/kfifo.h>
> > > +#include <linux/swap.h>
> > > +#include <linux/delay.h>
> > > +
> > > +#include "kcompressd.h"
> > > +
> > > +#define INIT_QUEUE_SIZE                4096
> > > +#define DEFAULT_NR_KCOMPRESSD  4
> > > +
> > > +static atomic_t enable_kcompressd;
> > > +static unsigned int nr_kcompressd;
> > > +static unsigned int queue_size_per_kcompressd;
> > > +static struct kcompress *kcompress;
> > > +
> > > +enum run_state {
> > > +       KCOMPRESSD_NOT_STARTED = 0,
> > > +       KCOMPRESSD_RUNNING,
> > > +       KCOMPRESSD_SLEEPING,
> > > +};
> > > +
> > > +struct kcompressd_para {
> > > +       wait_queue_head_t *kcompressd_wait;
> > > +       struct kfifo *write_fifo;
> > > +       atomic_t *running;
> > > +};
> > > +
> > > +static struct kcompressd_para *kcompressd_para;
> > > +static BLOCKING_NOTIFIER_HEAD(kcompressd_notifier_list);
> > > +
> > > +struct write_work {
> > > +       void *mem;
> > > +       struct bio *bio;
> > > +       compress_callback cb;
> > > +};
> > > +
> > > +int kcompressd_enabled(void)
> > > +{
> > > +       return likely(atomic_read(&enable_kcompressd));
> > > +}
> > > +EXPORT_SYMBOL(kcompressd_enabled);
> > > +
> > > +static void kcompressd_try_to_sleep(struct kcompressd_para *p)
> > > +{
> > > +       DEFINE_WAIT(wait);
> > > +
> > > +       if (!kfifo_is_empty(p->write_fifo))
> > > +               return;
> > > +
> > > +       if (freezing(current) || kthread_should_stop())
> > > +               return;
> > > +
> > > +       atomic_set(p->running, KCOMPRESSD_SLEEPING);
> > > +       prepare_to_wait(p->kcompressd_wait, &wait,
> > > TASK_INTERRUPTIBLE);
> > > +
> > > +       /*
> > > +        * After a short sleep, check if it was a premature sleep.
> > > If not, then
> > > +        * go fully to sleep until explicitly woken up.
> > > +        */
> > > +       if (!kthread_should_stop() && kfifo_is_empty(p-
> > > >write_fifo))
> > > +               schedule();
> > > +
> > > +       finish_wait(p->kcompressd_wait, &wait);
> > > +       atomic_set(p->running, KCOMPRESSD_RUNNING);
> > > +}
> > > +
> > > +static int kcompressd(void *para)
> > > +{
> > > +       struct task_struct *tsk = current;
> > > +       struct kcompressd_para *p = (struct kcompressd_para *)para;
> > > +
> > > +       tsk->flags |= PF_MEMALLOC | PF_KSWAPD;
> > > +       set_freezable();
> > > +
> > > +       while (!kthread_should_stop()) {
> > > +               bool ret;
> > > +
> > > +               kcompressd_try_to_sleep(p);
> > > +               ret = try_to_freeze();
> > > +               if (kthread_should_stop())
> > > +                       break;
> > > +
> > > +               if (ret)
> > > +                       continue;
> > > +
> > > +               while (!kfifo_is_empty(p->write_fifo)) {
> > > +                       struct write_work entry;
> > > +
> > > +                       if (sizeof(struct write_work) ==
> > > kfifo_out(p->write_fifo,
> > > +                                               &entry,
> > > sizeof(struct write_work))) {
> > > +                               entry.cb(entry.mem, entry.bio);
> > > +                               bio_put(entry.bio);
> > > +                       }
> > > +               }
> > > +
> > > +       }
> > > +
> > > +       tsk->flags &= ~(PF_MEMALLOC | PF_KSWAPD);
> > > +       atomic_set(p->running, KCOMPRESSD_NOT_STARTED);
> > > +       return 0;
> > > +}
> > > +
> > > +static int init_write_queue(void)
> > > +{
> > > +       int i;
> > > +       unsigned int queue_len = queue_size_per_kcompressd *
> > > sizeof(struct write_work);
> > > +
> > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > +               if (kfifo_alloc(&kcompress[i].write_fifo,
> > > +                                       queue_len, GFP_KERNEL)) {
> > > +                       pr_err("Failed to alloc kfifo %d\n", i);
> > > +                       return -ENOMEM;
> > > +               }
> > > +       }
> > > +       return 0;
> > > +}
> > > +
> > > +static void clean_bio_queue(int idx)
> > > +{
> > > +       struct write_work entry;
> > > +
> > > +       while (sizeof(struct write_work) ==
> > > kfifo_out(&kcompress[idx].write_fifo,
> > > +                               &entry, sizeof(struct write_work)))
> > > {
> > > +               bio_put(entry.bio);
> > > +               entry.cb(entry.mem, entry.bio);
> > > +       }
> > > +       kfifo_free(&kcompress[idx].write_fifo);
> > > +}
> > > +
> > > +static int kcompress_update(void)
> > > +{
> > > +       int i;
> > > +       int ret;
> > > +
> > > +       kcompress = kvmalloc_array(nr_kcompressd, sizeof(struct
> > > kcompress), GFP_KERNEL);
> > > +       if (!kcompress)
> > > +               return -ENOMEM;
> > > +
> > > +       kcompressd_para = kvmalloc_array(nr_kcompressd,
> > > sizeof(struct kcompressd_para), GFP_KERNEL);
> > > +       if (!kcompressd_para)
> > > +               return -ENOMEM;
> > > +
> > > +       ret = init_write_queue();
> > > +       if (ret) {
> > > +               pr_err("Initialization of writing to FIFOs
> > > failed!!\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > +               init_waitqueue_head(&kcompress[i].kcompressd_wait);
> > > +               kcompressd_para[i].kcompressd_wait =
> > > &kcompress[i].kcompressd_wait;
> > > +               kcompressd_para[i].write_fifo =
> > > &kcompress[i].write_fifo;
> > > +               kcompressd_para[i].running = &kcompress[i].running;
> > > +       }
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static void stop_all_kcompressd_thread(void)
> > > +{
> > > +       int i;
> > > +
> > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > +               kthread_stop(kcompress[i].kcompressd);
> > > +               kcompress[i].kcompressd = NULL;
> > > +               clean_bio_queue(i);
> > > +       }
> > > +}
> > > +
> > > +static int do_nr_kcompressd_handler(const char *val,
> > > +               const struct kernel_param *kp)
> > > +{
> > > +       int ret;
> > > +
> > > +       atomic_set(&enable_kcompressd, false);
> > > +
> > > +       stop_all_kcompressd_thread();
> > > +
> > > +       ret = param_set_int(val, kp);
> > > +       if (!ret) {
> > > +               pr_err("Invalid number of kcompressd.\n");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       ret = init_write_queue();
> > > +       if (ret) {
> > > +               pr_err("Initialization of writing to FIFOs
> > > failed!!\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       atomic_set(&enable_kcompressd, true);
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static const struct kernel_param_ops
> > > param_ops_change_nr_kcompressd = {
> > > +       .set = &do_nr_kcompressd_handler,
> > > +       .get = &param_get_uint,
> > > +       .free = NULL,
> > > +};
> > > +
> > > +module_param_cb(nr_kcompressd, &param_ops_change_nr_kcompressd,
> > > +               &nr_kcompressd, 0644);
> > > +MODULE_PARM_DESC(nr_kcompressd, "Number of pre-created daemon for
> > > page compression");
> > > +
> > > +static int do_queue_size_per_kcompressd_handler(const char *val,
> > > +               const struct kernel_param *kp)
> > > +{
> > > +       int ret;
> > > +
> > > +       atomic_set(&enable_kcompressd, false);
> > > +
> > > +       stop_all_kcompressd_thread();
> > > +
> > > +       ret = param_set_int(val, kp);
> > > +       if (!ret) {
> > > +               pr_err("Invalid queue size for kcompressd.\n");
> > > +               return -EINVAL;
> > > +       }
> > > +
> > > +       ret = init_write_queue();
> > > +       if (ret) {
> > > +               pr_err("Initialization of writing to FIFOs
> > > failed!!\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       pr_info("Queue size for kcompressd was changed: %d\n",
> > > queue_size_per_kcompressd);
> > > +
> > > +       atomic_set(&enable_kcompressd, true);
> > > +       return 0;
> > > +}
> > > +
> > > +static const struct kernel_param_ops
> > > param_ops_change_queue_size_per_kcompressd = {
> > > +       .set = &do_queue_size_per_kcompressd_handler,
> > > +       .get = &param_get_uint,
> > > +       .free = NULL,
> > > +};
> > > +
> > > +module_param_cb(queue_size_per_kcompressd,
> > > &param_ops_change_queue_size_per_kcompressd,
> > > +               &queue_size_per_kcompressd, 0644);
> > > +MODULE_PARM_DESC(queue_size_per_kcompressd,
> > > +               "Size of queue for kcompressd");
> > > +
> > > +int schedule_bio_write(void *mem, struct bio *bio,
> > > compress_callback cb)
> > > +{
> > > +       int i;
> > > +       bool submit_success = false;
> > > +       size_t sz_work = sizeof(struct write_work);
> > > +
> > > +       struct write_work entry = {
> > > +               .mem = mem,
> > > +               .bio = bio,
> > > +               .cb = cb
> > > +       };
> > > +
> > > +       if (unlikely(!atomic_read(&enable_kcompressd)))
> > > +               return -EBUSY;
> > > +
> > > +       if (!nr_kcompressd || !current_is_kswapd())
> > > +               return -EBUSY;
> > > +
> > > +       bio_get(bio);
> > > +
> > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > +               submit_success =
> > > +                       (kfifo_avail(&kcompress[i].write_fifo) >=
> > > sz_work) &&
> > > +                       (sz_work ==
> > > kfifo_in(&kcompress[i].write_fifo, &entry, sz_work));
> > > +
> > > +               if (submit_success) {
> > > +                       switch (atomic_read(&kcompress[i].running))
> > > {
> > > +                       case KCOMPRESSD_NOT_STARTED:
> > > +                               atomic_set(&kcompress[i].running,
> > > KCOMPRESSD_RUNNING);
> > > +                               kcompress[i].kcompressd =
> > > kthread_run(kcompressd,
> > > +
> > > &kcompressd_para[i], "kcompressd:%d", i);
> > > +                               if
> > > (IS_ERR(kcompress[i].kcompressd)) {
> > > +
> > > atomic_set(&kcompress[i].running, KCOMPRESSD_NOT_STARTED);
> > > +                                       pr_warn("Failed to start
> > > kcompressd:%d\n", i);
> > > +                                       clean_bio_queue(i);
> > > +                               }
> > > +                               break;
> > > +                       case KCOMPRESSD_RUNNING:
> > > +                               break;
> > > +                       case KCOMPRESSD_SLEEPING:
> > > +
> > > wake_up_interruptible(&kcompress[i].kcompressd_wait);
> > > +                               break;
> > > +                       }
> > > +                       return 0;
> > > +               }
> > > +       }
> > > +
> > > +       bio_put(bio);
> > > +       return -EBUSY;
> > > +}
> > > +EXPORT_SYMBOL(schedule_bio_write);
> > > +
> > > +static int __init kcompressd_init(void)
> > > +{
> > > +       int ret;
> > > +
> > > +       nr_kcompressd = DEFAULT_NR_KCOMPRESSD;
> > > +       queue_size_per_kcompressd = INIT_QUEUE_SIZE;
> > > +
> > > +       ret = kcompress_update();
> > > +       if (ret) {
> > > +               pr_err("Init kcompressd failed!\n");
> > > +               return ret;
> > > +       }
> > > +
> > > +       atomic_set(&enable_kcompressd, true);
> > > +       blocking_notifier_call_chain(&kcompressd_notifier_list, 0,
> > > NULL);
> > > +       return 0;
> > > +}
> > > +
> > > +static void __exit kcompressd_exit(void)
> > > +{
> > > +       atomic_set(&enable_kcompressd, false);
> > > +       stop_all_kcompressd_thread();
> > > +
> > > +       kvfree(kcompress);
> > > +       kvfree(kcompressd_para);
> > > +}
> > > +
> > > +module_init(kcompressd_init);
> > > +module_exit(kcompressd_exit);
> > > +
> > > +MODULE_LICENSE("Dual BSD/GPL");
> > > +MODULE_AUTHOR("Qun-Wei Lin <[e-mail]>");
> > > +MODULE_DESCRIPTION("Separate the page compression from the memory
> > > reclaiming");
> > > +
> > > diff --git a/drivers/block/zram/kcompressd.h
> > > b/drivers/block/zram/kcompressd.h
> > > new file mode 100644
> > > index 000000000000..2fe0b424a7af
> > > --- /dev/null
> > > +++ b/drivers/block/zram/kcompressd.h
> > > @@ -0,0 +1,25 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Copyright (C) 2024 MediaTek Inc.
> > > + */
> > > +
> > > +#ifndef _KCOMPRESSD_H_
> > > +#define _KCOMPRESSD_H_
> > > +
> > > +#include <linux/rwsem.h>
> > > +#include <linux/kfifo.h>
> > > +#include <linux/atomic.h>
> > > +
> > > +typedef void (*compress_callback)(void *mem, struct bio *bio);
> > > +
> > > +struct kcompress {
> > > +       struct task_struct *kcompressd;
> > > +       wait_queue_head_t kcompressd_wait;
> > > +       struct kfifo write_fifo;
> > > +       atomic_t running;
> > > +};
> > > +
> > > +int kcompressd_enabled(void);
> > > +int schedule_bio_write(void *mem, struct bio *bio,
> > > compress_callback cb);
> > > +#endif
> > > +
> > > diff --git a/drivers/block/zram/zram_drv.c
> > > b/drivers/block/zram/zram_drv.c
> > > index 2e1a70f2f4bd..bcd63ecb6ff2 100644
> > > --- a/drivers/block/zram/zram_drv.c
> > > +++ b/drivers/block/zram/zram_drv.c
> > > @@ -35,6 +35,7 @@
> > >  #include <linux/part_stat.h>
> > >  #include <linux/kernel_read_file.h>
> > >
> > > +#include "kcompressd.h"
> > >  #include "zram_drv.h"
> > >
> > >  static DEFINE_IDR(zram_index_idr);
> > > @@ -2240,6 +2241,15 @@ static void zram_bio_write(struct zram
> > > *zram, struct bio *bio)
> > >         bio_endio(bio);
> > >  }
> > >
> > > +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> > > +static void zram_bio_write_callback(void *mem, struct bio *bio)
> > > +{
> > > +       struct zram *zram = (struct zram *)mem;
> > > +
> > > +       zram_bio_write(zram, bio);
> > > +}
> > > +#endif
> > > +
> > >  /*
> > >   * Handler function for all zram I/O requests.
> > >   */
> > > @@ -2252,6 +2262,10 @@ static void zram_submit_bio(struct bio *bio)
> > >                 zram_bio_read(zram, bio);
> > >                 break;
> > >         case REQ_OP_WRITE:
> > > +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> > > +               if (kcompressd_enabled() &&
> > > !schedule_bio_write(zram, bio, zram_bio_write_callback))
> > > +                       break;
> > > +#endif
> > >                 zram_bio_write(zram, bio);
> > >                 break;
> > >         case REQ_OP_DISCARD:
> > > @@ -2535,9 +2549,11 @@ static int zram_add(void)
> > >  #if ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE
> > >                 .max_write_zeroes_sectors       = UINT_MAX,
> > >  #endif
> > > -               .features                       =
> > > BLK_FEAT_STABLE_WRITES        |
> > > -
> > > BLK_FEAT_READ_SYNCHRONOUS     |
> > > -
> > > BLK_FEAT_WRITE_SYNCHRONOUS,
> > > +               .features                       =
> > > BLK_FEAT_STABLE_WRITES
> > > +                                                 |
> > > BLK_FEAT_READ_SYNCHRONOUS
> > > +#if !IS_ENABLED(CONFIG_KCOMPRESSD)
> > > +                                                 |
> > > BLK_FEAT_WRITE_SYNCHRONOUS,
> > > +#endif
> > >         };
> > >         struct zram *zram;
> > >         int ret, device_id;
> > > --
> > > 2.45.2
> > >
> >

Thanks
Barry

Barry Song March 11, 2025, 7:25 a.m. UTC | #15

On Tue, Mar 11, 2025 at 8:05 PM Barry Song <[e-mail]> wrote:
>
> On Tue, Mar 11, 2025 at 2:26 AM Qun-wei Lin (林群崴)
> <[e-mail]> wrote:
> >
> > On Sat, 2025-03-08 at 08:41 +1300, Barry Song wrote:
> > >
> > > External email : Please do not click links or open attachments until
> > > you have verified the sender or the content.
> > >
> > >
> > > On Sat, Mar 8, 2025 at 1:02 AM Qun-Wei Lin <[e-mail]>
> > > wrote:
> > > >
> > > > Introduced Kcompressd to offload zram page compression, improving
> > > > system efficiency by handling compression separately from memory
> > > > reclaiming. Added necessary configurations and dependencies.
> > > >
> > > > Signed-off-by: Qun-Wei Lin <[e-mail]>
> > > > ---
> > > >  drivers/block/zram/Kconfig      |  11 ++
> > > >  drivers/block/zram/Makefile     |   3 +-
> > > >  drivers/block/zram/kcompressd.c | 340
> > > > ++++++++++++++++++++++++++++++++
> > > >  drivers/block/zram/kcompressd.h |  25 +++
> > > >  drivers/block/zram/zram_drv.c   |  22 ++-
> > > >  5 files changed, 397 insertions(+), 4 deletions(-)
> > > >  create mode 100644 drivers/block/zram/kcompressd.c
> > > >  create mode 100644 drivers/block/zram/kcompressd.h
> > > >
> > > > diff --git a/drivers/block/zram/Kconfig
> > > > b/drivers/block/zram/Kconfig
> > > > index 402b7b175863..f0a1b574f770 100644
> > > > --- a/drivers/block/zram/Kconfig
> > > > +++ b/drivers/block/zram/Kconfig
> > > > @@ -145,3 +145,14 @@ config ZRAM_MULTI_COMP
> > > >           re-compress pages using a potentially slower but more
> > > > effective
> > > >           compression algorithm. Note, that IDLE page recompression
> > > >           requires ZRAM_TRACK_ENTRY_ACTIME.
> > > > +
> > > > +config KCOMPRESSD
> > > > +       tristate "Kcompressd: Accelerated zram compression"
> > > > +       depends on ZRAM
> > > > +       help
> > > > +         Kcompressd creates multiple daemons to accelerate the
> > > > compression of pages
> > > > +         in zram, offloading this time-consuming task from the
> > > > zram driver.
> > > > +
> > > > +         This approach improves system efficiency by handling page
> > > > compression separately,
> > > > +         which was originally done by kswapd or direct reclaim.
> > >
> > > For direct reclaim, we were previously able to compress using
> > > multiple CPUs
> > > with multi-threading.
> > > After your patch, it seems that only a single thread/CPU is used for
> > > compression
> > > so it won't necessarily improve direct reclaim performance?
> > >
> >
> > Our patch only splits the context of kswapd. When direct reclaim is
> > occurred, it is bypassed, so direct reclaim remains unchanged, with
> > each thread that needs memory directly reclaiming it.
>
> Qun-wei, I’m getting a bit confused. Looking at the code in page_io.c,
> you always call swap_writepage_bdev_async() no matter if it is kswapd
> or direct reclaim:
>
> - else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
> + else if (data_race(sis->flags & SWP_WRITE_SYNCHRONOUS_IO))
>            swap_writepage_bdev_sync(folio, wbc, sis);
>   else
>             swap_writepage_bdev_async(folio, wbc, sis);
>
> In zram, I notice you are bypassing kcompressd by:
>
> + if (!nr_kcompressd || !current_is_kswapd())
> +        return -EBUSY;
>
> How will this work if no one is calling __end_swap_bio_write(&bio),
> which is present in swap_writepage_bdev_sync()?
> Am I missing something? Or is it done by zram_bio_write() ?
>
> On the other hand, zram is a generic block device, and coupling its
> code with kswapd/direct reclaim somehow violates layering
> principles :-)
>
> >
> > > Even for kswapd, we used to have multiple threads like [kswapd0],
> > > [kswapd1],
> > > and [kswapd2] for different nodes. Now, are we also limited to just
> > > one thread?
> >
> > We only considered a single kswapd here and didn't account for multiple
> > instances. Since I use kfifo to collect the bios, if there are multiple
> > kswapds, we need to add a lock to protect the bio queue. I can revise
> > this in the 2nd version, or do you have any other suggested approaches?
>
> I'm wondering if we can move the code to vmscan/page_io instead
> of zram. If we're using a sync I/O swap device or have enabled zswap,
> we could run reclamation in this separate thread, which should also be

Sorry for the typo:
s/reclamation/__swap_writepage/g

> NUMA-aware.
>
> I would definitely be interested in prototyping it when I have the time.
>

Thanks
Barry

Qun-wei Lin (林群崴) March 11, 2025, 2:33 p.m. UTC | #16

On Tue, 2025-03-11 at 20:05 +1300, Barry Song wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> On Tue, Mar 11, 2025 at 2:26 AM Qun-wei Lin (林群崴)
> <[e-mail]> wrote:
> > 
> > On Sat, 2025-03-08 at 08:41 +1300, Barry Song wrote:
> > > 
> > > External email : Please do not click links or open attachments
> > > until
> > > you have verified the sender or the content.
> > > 
> > > 
> > > On Sat, Mar 8, 2025 at 1:02 AM Qun-Wei Lin
> > > <[e-mail]>
> > > wrote:
> > > > 
> > > > Introduced Kcompressd to offload zram page compression,
> > > > improving
> > > > system efficiency by handling compression separately from
> > > > memory
> > > > reclaiming. Added necessary configurations and dependencies.
> > > > 
> > > > Signed-off-by: Qun-Wei Lin <[e-mail]>
> > > > ---
> > > >  drivers/block/zram/Kconfig      |  11 ++
> > > >  drivers/block/zram/Makefile     |   3 +-
> > > >  drivers/block/zram/kcompressd.c | 340
> > > > ++++++++++++++++++++++++++++++++
> > > >  drivers/block/zram/kcompressd.h |  25 +++
> > > >  drivers/block/zram/zram_drv.c   |  22 ++-
> > > >  5 files changed, 397 insertions(+), 4 deletions(-)
> > > >  create mode 100644 drivers/block/zram/kcompressd.c
> > > >  create mode 100644 drivers/block/zram/kcompressd.h
> > > > 
> > > > diff --git a/drivers/block/zram/Kconfig
> > > > b/drivers/block/zram/Kconfig
> > > > index 402b7b175863..f0a1b574f770 100644
> > > > --- a/drivers/block/zram/Kconfig
> > > > +++ b/drivers/block/zram/Kconfig
> > > > @@ -145,3 +145,14 @@ config ZRAM_MULTI_COMP
> > > >           re-compress pages using a potentially slower but more
> > > > effective
> > > >           compression algorithm. Note, that IDLE page
> > > > recompression
> > > >           requires ZRAM_TRACK_ENTRY_ACTIME.
> > > > +
> > > > +config KCOMPRESSD
> > > > +       tristate "Kcompressd: Accelerated zram compression"
> > > > +       depends on ZRAM
> > > > +       help
> > > > +         Kcompressd creates multiple daemons to accelerate the
> > > > compression of pages
> > > > +         in zram, offloading this time-consuming task from the
> > > > zram driver.
> > > > +
> > > > +         This approach improves system efficiency by handling
> > > > page
> > > > compression separately,
> > > > +         which was originally done by kswapd or direct
> > > > reclaim.
> > > 
> > > For direct reclaim, we were previously able to compress using
> > > multiple CPUs
> > > with multi-threading.
> > > After your patch, it seems that only a single thread/CPU is used
> > > for
> > > compression
> > > so it won't necessarily improve direct reclaim performance?
> > > 
> > 
> > Our patch only splits the context of kswapd. When direct reclaim is
> > occurred, it is bypassed, so direct reclaim remains unchanged, with
> > each thread that needs memory directly reclaiming it.
> 
> Qun-wei, I’m getting a bit confused. Looking at the code in
> page_io.c,
> you always call swap_writepage_bdev_async() no matter if it is kswapd
> or direct reclaim:
> 
> - else if (data_race(sis->flags & SWP_SYNCHRONOUS_IO))
> + else if (data_race(sis->flags & SWP_WRITE_SYNCHRONOUS_IO))
>            swap_writepage_bdev_sync(folio, wbc, sis);
>   else
>             swap_writepage_bdev_async(folio, wbc, sis);
> 
> In zram, I notice you are bypassing kcompressd by:
> 
> + if (!nr_kcompressd || !current_is_kswapd())
> +        return -EBUSY;
> 
> How will this work if no one is calling __end_swap_bio_write(&bio),
> which is present in swap_writepage_bdev_sync()?
> Am I missing something? Or is it done by zram_bio_write() ?
> 

In swap_writepage_bdev_async(), 'bio->bi_end_io = end_swap_bio_write'
is set. So yes, the bio_endio(bio) in zram_bio_write() will handle
this.

> On the other hand, zram is a generic block device, and coupling its
> code with kswapd/direct reclaim somehow violates layering
> principles :-)

OK, so it seems that moving kcompressd to the vmscan level might
resolve this issue, right?


> > 
> > > Even for kswapd, we used to have multiple threads like [kswapd0],
> > > [kswapd1],
> > > and [kswapd2] for different nodes. Now, are we also limited to
> > > just
> > > one thread?
> > 
> > We only considered a single kswapd here and didn't account for
> > multiple
> > instances. Since I use kfifo to collect the bios, if there are
> > multiple
> > kswapds, we need to add a lock to protect the bio queue. I can
> > revise
> > this in the 2nd version, or do you have any other suggested
> > approaches?
> 
> I'm wondering if we can move the code to vmscan/page_io instead
> of zram. If we're using a sync I/O swap device or have enabled zswap,
> we could run reclamation in this separate thread, which should also
> be
> NUMA-aware.
> 
> I would definitely be interested in prototyping it when I have the
> time.

Thank you so much!

> 
> > 
> > > I also wonder if this could be handled at the vmscan level
> > > instead of
> > > the zram
> > > level. then it might potentially help other sync devices or even
> > > zswap later.
> > > 
> > > But I agree that for phones, modifying zram seems like an easier
> > > starting
> > > point. However, relying on a single thread isn't always the best
> > > approach.
> > > 
> > > 
> > > > +
> > > > diff --git a/drivers/block/zram/Makefile
> > > > b/drivers/block/zram/Makefile
> > > > index 0fdefd576691..23baa5dfceb9 100644
> > > > --- a/drivers/block/zram/Makefile
> > > > +++ b/drivers/block/zram/Makefile
> > > > @@ -9,4 +9,5 @@ zram-$(CONFIG_ZRAM_BACKEND_ZSTD)        +=
> > > > backend_zstd.o
> > > >  zram-$(CONFIG_ZRAM_BACKEND_DEFLATE)    += backend_deflate.o
> > > >  zram-$(CONFIG_ZRAM_BACKEND_842)                +=
> > > > backend_842.o
> > > > 
> > > > -obj-$(CONFIG_ZRAM)     +=      zram.o
> > > > +obj-$(CONFIG_ZRAM)             += zram.o
> > > > +obj-$(CONFIG_KCOMPRESSD)       += kcompressd.o
> > > > diff --git a/drivers/block/zram/kcompressd.c
> > > > b/drivers/block/zram/kcompressd.c
> > > > new file mode 100644
> > > > index 000000000000..195b7e386869
> > > > --- /dev/null
> > > > +++ b/drivers/block/zram/kcompressd.c
> > > > @@ -0,0 +1,340 @@
> > > > +// SPDX-License-Identifier: GPL-2.0
> > > > +/*
> > > > + * Copyright (C) 2024 MediaTek Inc.
> > > > + */
> > > > +
> > > > +#include <linux/module.h>
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/bio.h>
> > > > +#include <linux/bitops.h>
> > > > +#include <linux/freezer.h>
> > > > +#include <linux/kernel.h>
> > > > +#include <linux/psi.h>
> > > > +#include <linux/kfifo.h>
> > > > +#include <linux/swap.h>
> > > > +#include <linux/delay.h>
> > > > +
> > > > +#include "kcompressd.h"
> > > > +
> > > > +#define INIT_QUEUE_SIZE                4096
> > > > +#define DEFAULT_NR_KCOMPRESSD  4
> > > > +
> > > > +static atomic_t enable_kcompressd;
> > > > +static unsigned int nr_kcompressd;
> > > > +static unsigned int queue_size_per_kcompressd;
> > > > +static struct kcompress *kcompress;
> > > > +
> > > > +enum run_state {
> > > > +       KCOMPRESSD_NOT_STARTED = 0,
> > > > +       KCOMPRESSD_RUNNING,
> > > > +       KCOMPRESSD_SLEEPING,
> > > > +};
> > > > +
> > > > +struct kcompressd_para {
> > > > +       wait_queue_head_t *kcompressd_wait;
> > > > +       struct kfifo *write_fifo;
> > > > +       atomic_t *running;
> > > > +};
> > > > +
> > > > +static struct kcompressd_para *kcompressd_para;
> > > > +static BLOCKING_NOTIFIER_HEAD(kcompressd_notifier_list);
> > > > +
> > > > +struct write_work {
> > > > +       void *mem;
> > > > +       struct bio *bio;
> > > > +       compress_callback cb;
> > > > +};
> > > > +
> > > > +int kcompressd_enabled(void)
> > > > +{
> > > > +       return likely(atomic_read(&enable_kcompressd));
> > > > +}
> > > > +EXPORT_SYMBOL(kcompressd_enabled);
> > > > +
> > > > +static void kcompressd_try_to_sleep(struct kcompressd_para *p)
> > > > +{
> > > > +       DEFINE_WAIT(wait);
> > > > +
> > > > +       if (!kfifo_is_empty(p->write_fifo))
> > > > +               return;
> > > > +
> > > > +       if (freezing(current) || kthread_should_stop())
> > > > +               return;
> > > > +
> > > > +       atomic_set(p->running, KCOMPRESSD_SLEEPING);
> > > > +       prepare_to_wait(p->kcompressd_wait, &wait,
> > > > TASK_INTERRUPTIBLE);
> > > > +
> > > > +       /*
> > > > +        * After a short sleep, check if it was a premature
> > > > sleep.
> > > > If not, then
> > > > +        * go fully to sleep until explicitly woken up.
> > > > +        */
> > > > +       if (!kthread_should_stop() && kfifo_is_empty(p-
> > > > > write_fifo))
> > > > +               schedule();
> > > > +
> > > > +       finish_wait(p->kcompressd_wait, &wait);
> > > > +       atomic_set(p->running, KCOMPRESSD_RUNNING);
> > > > +}
> > > > +
> > > > +static int kcompressd(void *para)
> > > > +{
> > > > +       struct task_struct *tsk = current;
> > > > +       struct kcompressd_para *p = (struct kcompressd_para
> > > > *)para;
> > > > +
> > > > +       tsk->flags |= PF_MEMALLOC | PF_KSWAPD;
> > > > +       set_freezable();
> > > > +
> > > > +       while (!kthread_should_stop()) {
> > > > +               bool ret;
> > > > +
> > > > +               kcompressd_try_to_sleep(p);
> > > > +               ret = try_to_freeze();
> > > > +               if (kthread_should_stop())
> > > > +                       break;
> > > > +
> > > > +               if (ret)
> > > > +                       continue;
> > > > +
> > > > +               while (!kfifo_is_empty(p->write_fifo)) {
> > > > +                       struct write_work entry;
> > > > +
> > > > +                       if (sizeof(struct write_work) ==
> > > > kfifo_out(p->write_fifo,
> > > > +                                               &entry,
> > > > sizeof(struct write_work))) {
> > > > +                               entry.cb(entry.mem, entry.bio);
> > > > +                               bio_put(entry.bio);
> > > > +                       }
> > > > +               }
> > > > +
> > > > +       }
> > > > +
> > > > +       tsk->flags &= ~(PF_MEMALLOC | PF_KSWAPD);
> > > > +       atomic_set(p->running, KCOMPRESSD_NOT_STARTED);
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static int init_write_queue(void)
> > > > +{
> > > > +       int i;
> > > > +       unsigned int queue_len = queue_size_per_kcompressd *
> > > > sizeof(struct write_work);
> > > > +
> > > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > > +               if (kfifo_alloc(&kcompress[i].write_fifo,
> > > > +                                       queue_len, GFP_KERNEL))
> > > > {
> > > > +                       pr_err("Failed to alloc kfifo %d\n",
> > > > i);
> > > > +                       return -ENOMEM;
> > > > +               }
> > > > +       }
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static void clean_bio_queue(int idx)
> > > > +{
> > > > +       struct write_work entry;
> > > > +
> > > > +       while (sizeof(struct write_work) ==
> > > > kfifo_out(&kcompress[idx].write_fifo,
> > > > +                               &entry, sizeof(struct
> > > > write_work)))
> > > > {
> > > > +               bio_put(entry.bio);
> > > > +               entry.cb(entry.mem, entry.bio);
> > > > +       }
> > > > +       kfifo_free(&kcompress[idx].write_fifo);
> > > > +}
> > > > +
> > > > +static int kcompress_update(void)
> > > > +{
> > > > +       int i;
> > > > +       int ret;
> > > > +
> > > > +       kcompress = kvmalloc_array(nr_kcompressd, sizeof(struct
> > > > kcompress), GFP_KERNEL);
> > > > +       if (!kcompress)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       kcompressd_para = kvmalloc_array(nr_kcompressd,
> > > > sizeof(struct kcompressd_para), GFP_KERNEL);
> > > > +       if (!kcompressd_para)
> > > > +               return -ENOMEM;
> > > > +
> > > > +       ret = init_write_queue();
> > > > +       if (ret) {
> > > > +               pr_err("Initialization of writing to FIFOs
> > > > failed!!\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > > +              
> > > > init_waitqueue_head(&kcompress[i].kcompressd_wait);
> > > > +               kcompressd_para[i].kcompressd_wait =
> > > > &kcompress[i].kcompressd_wait;
> > > > +               kcompressd_para[i].write_fifo =
> > > > &kcompress[i].write_fifo;
> > > > +               kcompressd_para[i].running =
> > > > &kcompress[i].running;
> > > > +       }
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static void stop_all_kcompressd_thread(void)
> > > > +{
> > > > +       int i;
> > > > +
> > > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > > +               kthread_stop(kcompress[i].kcompressd);
> > > > +               kcompress[i].kcompressd = NULL;
> > > > +               clean_bio_queue(i);
> > > > +       }
> > > > +}
> > > > +
> > > > +static int do_nr_kcompressd_handler(const char *val,
> > > > +               const struct kernel_param *kp)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       atomic_set(&enable_kcompressd, false);
> > > > +
> > > > +       stop_all_kcompressd_thread();
> > > > +
> > > > +       ret = param_set_int(val, kp);
> > > > +       if (!ret) {
> > > > +               pr_err("Invalid number of kcompressd.\n");
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       ret = init_write_queue();
> > > > +       if (ret) {
> > > > +               pr_err("Initialization of writing to FIFOs
> > > > failed!!\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       atomic_set(&enable_kcompressd, true);
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static const struct kernel_param_ops
> > > > param_ops_change_nr_kcompressd = {
> > > > +       .set = &do_nr_kcompressd_handler,
> > > > +       .get = &param_get_uint,
> > > > +       .free = NULL,
> > > > +};
> > > > +
> > > > +module_param_cb(nr_kcompressd,
> > > > &param_ops_change_nr_kcompressd,
> > > > +               &nr_kcompressd, 0644);
> > > > +MODULE_PARM_DESC(nr_kcompressd, "Number of pre-created daemon
> > > > for
> > > > page compression");
> > > > +
> > > > +static int do_queue_size_per_kcompressd_handler(const char
> > > > *val,
> > > > +               const struct kernel_param *kp)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       atomic_set(&enable_kcompressd, false);
> > > > +
> > > > +       stop_all_kcompressd_thread();
> > > > +
> > > > +       ret = param_set_int(val, kp);
> > > > +       if (!ret) {
> > > > +               pr_err("Invalid queue size for kcompressd.\n");
> > > > +               return -EINVAL;
> > > > +       }
> > > > +
> > > > +       ret = init_write_queue();
> > > > +       if (ret) {
> > > > +               pr_err("Initialization of writing to FIFOs
> > > > failed!!\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       pr_info("Queue size for kcompressd was changed: %d\n",
> > > > queue_size_per_kcompressd);
> > > > +
> > > > +       atomic_set(&enable_kcompressd, true);
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static const struct kernel_param_ops
> > > > param_ops_change_queue_size_per_kcompressd = {
> > > > +       .set = &do_queue_size_per_kcompressd_handler,
> > > > +       .get = &param_get_uint,
> > > > +       .free = NULL,
> > > > +};
> > > > +
> > > > +module_param_cb(queue_size_per_kcompressd,
> > > > &param_ops_change_queue_size_per_kcompressd,
> > > > +               &queue_size_per_kcompressd, 0644);
> > > > +MODULE_PARM_DESC(queue_size_per_kcompressd,
> > > > +               "Size of queue for kcompressd");
> > > > +
> > > > +int schedule_bio_write(void *mem, struct bio *bio,
> > > > compress_callback cb)
> > > > +{
> > > > +       int i;
> > > > +       bool submit_success = false;
> > > > +       size_t sz_work = sizeof(struct write_work);
> > > > +
> > > > +       struct write_work entry = {
> > > > +               .mem = mem,
> > > > +               .bio = bio,
> > > > +               .cb = cb
> > > > +       };
> > > > +
> > > > +       if (unlikely(!atomic_read(&enable_kcompressd)))
> > > > +               return -EBUSY;
> > > > +
> > > > +       if (!nr_kcompressd || !current_is_kswapd())
> > > > +               return -EBUSY;
> > > > +
> > > > +       bio_get(bio);
> > > > +
> > > > +       for (i = 0; i < nr_kcompressd; i++) {
> > > > +               submit_success =
> > > > +                       (kfifo_avail(&kcompress[i].write_fifo)
> > > > >=
> > > > sz_work) &&
> > > > +                       (sz_work ==
> > > > kfifo_in(&kcompress[i].write_fifo, &entry, sz_work));
> > > > +
> > > > +               if (submit_success) {
> > > > +                       switch
> > > > (atomic_read(&kcompress[i].running))
> > > > {
> > > > +                       case KCOMPRESSD_NOT_STARTED:
> > > > +                              
> > > > atomic_set(&kcompress[i].running,
> > > > KCOMPRESSD_RUNNING);
> > > > +                               kcompress[i].kcompressd =
> > > > kthread_run(kcompressd,
> > > > +
> > > > &kcompressd_para[i], "kcompressd:%d", i);
> > > > +                               if
> > > > (IS_ERR(kcompress[i].kcompressd)) {
> > > > +
> > > > atomic_set(&kcompress[i].running, KCOMPRESSD_NOT_STARTED);
> > > > +                                       pr_warn("Failed to
> > > > start
> > > > kcompressd:%d\n", i);
> > > > +                                       clean_bio_queue(i);
> > > > +                               }
> > > > +                               break;
> > > > +                       case KCOMPRESSD_RUNNING:
> > > > +                               break;
> > > > +                       case KCOMPRESSD_SLEEPING:
> > > > +
> > > > wake_up_interruptible(&kcompress[i].kcompressd_wait);
> > > > +                               break;
> > > > +                       }
> > > > +                       return 0;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       bio_put(bio);
> > > > +       return -EBUSY;
> > > > +}
> > > > +EXPORT_SYMBOL(schedule_bio_write);
> > > > +
> > > > +static int __init kcompressd_init(void)
> > > > +{
> > > > +       int ret;
> > > > +
> > > > +       nr_kcompressd = DEFAULT_NR_KCOMPRESSD;
> > > > +       queue_size_per_kcompressd = INIT_QUEUE_SIZE;
> > > > +
> > > > +       ret = kcompress_update();
> > > > +       if (ret) {
> > > > +               pr_err("Init kcompressd failed!\n");
> > > > +               return ret;
> > > > +       }
> > > > +
> > > > +       atomic_set(&enable_kcompressd, true);
> > > > +       blocking_notifier_call_chain(&kcompressd_notifier_list,
> > > > 0,
> > > > NULL);
> > > > +       return 0;
> > > > +}
> > > > +
> > > > +static void __exit kcompressd_exit(void)
> > > > +{
> > > > +       atomic_set(&enable_kcompressd, false);
> > > > +       stop_all_kcompressd_thread();
> > > > +
> > > > +       kvfree(kcompress);
> > > > +       kvfree(kcompressd_para);
> > > > +}
> > > > +
> > > > +module_init(kcompressd_init);
> > > > +module_exit(kcompressd_exit);
> > > > +
> > > > +MODULE_LICENSE("Dual BSD/GPL");
> > > > +MODULE_AUTHOR("Qun-Wei Lin <[e-mail]>");
> > > > +MODULE_DESCRIPTION("Separate the page compression from the
> > > > memory
> > > > reclaiming");
> > > > +
> > > > diff --git a/drivers/block/zram/kcompressd.h
> > > > b/drivers/block/zram/kcompressd.h
> > > > new file mode 100644
> > > > index 000000000000..2fe0b424a7af
> > > > --- /dev/null
> > > > +++ b/drivers/block/zram/kcompressd.h
> > > > @@ -0,0 +1,25 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > > +/*
> > > > + * Copyright (C) 2024 MediaTek Inc.
> > > > + */
> > > > +
> > > > +#ifndef _KCOMPRESSD_H_
> > > > +#define _KCOMPRESSD_H_
> > > > +
> > > > +#include <linux/rwsem.h>
> > > > +#include <linux/kfifo.h>
> > > > +#include <linux/atomic.h>
> > > > +
> > > > +typedef void (*compress_callback)(void *mem, struct bio *bio);
> > > > +
> > > > +struct kcompress {
> > > > +       struct task_struct *kcompressd;
> > > > +       wait_queue_head_t kcompressd_wait;
> > > > +       struct kfifo write_fifo;
> > > > +       atomic_t running;
> > > > +};
> > > > +
> > > > +int kcompressd_enabled(void);
> > > > +int schedule_bio_write(void *mem, struct bio *bio,
> > > > compress_callback cb);
> > > > +#endif
> > > > +
> > > > diff --git a/drivers/block/zram/zram_drv.c
> > > > b/drivers/block/zram/zram_drv.c
> > > > index 2e1a70f2f4bd..bcd63ecb6ff2 100644
> > > > --- a/drivers/block/zram/zram_drv.c
> > > > +++ b/drivers/block/zram/zram_drv.c
> > > > @@ -35,6 +35,7 @@
> > > >  #include <linux/part_stat.h>
> > > >  #include <linux/kernel_read_file.h>
> > > > 
> > > > +#include "kcompressd.h"
> > > >  #include "zram_drv.h"
> > > > 
> > > >  static DEFINE_IDR(zram_index_idr);
> > > > @@ -2240,6 +2241,15 @@ static void zram_bio_write(struct zram
> > > > *zram, struct bio *bio)
> > > >         bio_endio(bio);
> > > >  }
> > > > 
> > > > +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> > > > +static void zram_bio_write_callback(void *mem, struct bio
> > > > *bio)
> > > > +{
> > > > +       struct zram *zram = (struct zram *)mem;
> > > > +
> > > > +       zram_bio_write(zram, bio);
> > > > +}
> > > > +#endif
> > > > +
> > > >  /*
> > > >   * Handler function for all zram I/O requests.
> > > >   */
> > > > @@ -2252,6 +2262,10 @@ static void zram_submit_bio(struct bio
> > > > *bio)
> > > >                 zram_bio_read(zram, bio);
> > > >                 break;
> > > >         case REQ_OP_WRITE:
> > > > +#if IS_ENABLED(CONFIG_KCOMPRESSD)
> > > > +               if (kcompressd_enabled() &&
> > > > !schedule_bio_write(zram, bio, zram_bio_write_callback))
> > > > +                       break;
> > > > +#endif
> > > >                 zram_bio_write(zram, bio);
> > > >                 break;
> > > >         case REQ_OP_DISCARD:
> > > > @@ -2535,9 +2549,11 @@ static int zram_add(void)
> > > >  #if ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE
> > > >                 .max_write_zeroes_sectors       = UINT_MAX,
> > > >  #endif
> > > > -               .features                       =
> > > > BLK_FEAT_STABLE_WRITES        |
> > > > -
> > > > BLK_FEAT_READ_SYNCHRONOUS     |
> > > > -
> > > > BLK_FEAT_WRITE_SYNCHRONOUS,
> > > > +               .features                       =
> > > > BLK_FEAT_STABLE_WRITES
> > > > +                                                 |
> > > > BLK_FEAT_READ_SYNCHRONOUS
> > > > +#if !IS_ENABLED(CONFIG_KCOMPRESSD)
> > > > +                                                 |
> > > > BLK_FEAT_WRITE_SYNCHRONOUS,
> > > > +#endif
> > > >         };
> > > >         struct zram *zram;
> > > >         int ret, device_id;
> > > > --
> > > > 2.45.2
> > > > 
> > > 
> 
> Thanks
> Barry

Best Regards,
Qun-wei

Patch

diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
index 402b7b175863..f0a1b574f770 100644
--- a/drivers/block/zram/Kconfig
+++ b/drivers/block/zram/Kconfig
@@ -145,3 +145,14 @@  config ZRAM_MULTI_COMP
 	  re-compress pages using a potentially slower but more effective
 	  compression algorithm. Note, that IDLE page recompression
 	  requires ZRAM_TRACK_ENTRY_ACTIME.
+
+config KCOMPRESSD
+	tristate "Kcompressd: Accelerated zram compression"
+	depends on ZRAM
+	help
+	  Kcompressd creates multiple daemons to accelerate the compression of pages
+	  in zram, offloading this time-consuming task from the zram driver.
+
+	  This approach improves system efficiency by handling page compression separately,
+	  which was originally done by kswapd or direct reclaim.
+
diff --git a/drivers/block/zram/Makefile b/drivers/block/zram/Makefile
index 0fdefd576691..23baa5dfceb9 100644
--- a/drivers/block/zram/Makefile
+++ b/drivers/block/zram/Makefile
@@ -9,4 +9,5 @@  zram-$(CONFIG_ZRAM_BACKEND_ZSTD)	+= backend_zstd.o
 zram-$(CONFIG_ZRAM_BACKEND_DEFLATE)	+= backend_deflate.o
 zram-$(CONFIG_ZRAM_BACKEND_842)		+= backend_842.o
 
-obj-$(CONFIG_ZRAM)	+=	zram.o
+obj-$(CONFIG_ZRAM)		+= zram.o
+obj-$(CONFIG_KCOMPRESSD)	+= kcompressd.o
diff --git a/drivers/block/zram/kcompressd.c b/drivers/block/zram/kcompressd.c
new file mode 100644
index 000000000000..195b7e386869
--- /dev/null
+++ b/drivers/block/zram/kcompressd.c
@@ -0,0 +1,340 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024 MediaTek Inc.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/bio.h>
+#include <linux/bitops.h>
+#include <linux/freezer.h>
+#include <linux/kernel.h>
+#include <linux/psi.h>
+#include <linux/kfifo.h>
+#include <linux/swap.h>
+#include <linux/delay.h>
+
+#include "kcompressd.h"
+
+#define INIT_QUEUE_SIZE		4096
+#define DEFAULT_NR_KCOMPRESSD	4
+
+static atomic_t enable_kcompressd;
+static unsigned int nr_kcompressd;
+static unsigned int queue_size_per_kcompressd;
+static struct kcompress *kcompress;
+
+enum run_state {
+	KCOMPRESSD_NOT_STARTED = 0,
+	KCOMPRESSD_RUNNING,
+	KCOMPRESSD_SLEEPING,
+};
+
+struct kcompressd_para {
+	wait_queue_head_t *kcompressd_wait;
+	struct kfifo *write_fifo;
+	atomic_t *running;
+};
+
+static struct kcompressd_para *kcompressd_para;
+static BLOCKING_NOTIFIER_HEAD(kcompressd_notifier_list);
+
+struct write_work {
+	void *mem;
+	struct bio *bio;
+	compress_callback cb;
+};
+
+int kcompressd_enabled(void)
+{
+	return likely(atomic_read(&enable_kcompressd));
+}
+EXPORT_SYMBOL(kcompressd_enabled);
+
+static void kcompressd_try_to_sleep(struct kcompressd_para *p)
+{
+	DEFINE_WAIT(wait);
+
+	if (!kfifo_is_empty(p->write_fifo))
+		return;
+
+	if (freezing(current) || kthread_should_stop())
+		return;
+
+	atomic_set(p->running, KCOMPRESSD_SLEEPING);
+	prepare_to_wait(p->kcompressd_wait, &wait, TASK_INTERRUPTIBLE);
+
+	/*
+	 * After a short sleep, check if it was a premature sleep. If not, then
+	 * go fully to sleep until explicitly woken up.
+	 */
+	if (!kthread_should_stop() && kfifo_is_empty(p->write_fifo))
+		schedule();
+
+	finish_wait(p->kcompressd_wait, &wait);
+	atomic_set(p->running, KCOMPRESSD_RUNNING);
+}
+
+static int kcompressd(void *para)
+{
+	struct task_struct *tsk = current;
+	struct kcompressd_para *p = (struct kcompressd_para *)para;
+
+	tsk->flags |= PF_MEMALLOC | PF_KSWAPD;
+	set_freezable();
+
+	while (!kthread_should_stop()) {
+		bool ret;
+
+		kcompressd_try_to_sleep(p);
+		ret = try_to_freeze();
+		if (kthread_should_stop())
+			break;
+
+		if (ret)
+			continue;
+
+		while (!kfifo_is_empty(p->write_fifo)) {
+			struct write_work entry;
+
+			if (sizeof(struct write_work) == kfifo_out(p->write_fifo,
+						&entry, sizeof(struct write_work))) {
+				entry.cb(entry.mem, entry.bio);
+				bio_put(entry.bio);
+			}
+		}
+
+	}
+
+	tsk->flags &= ~(PF_MEMALLOC | PF_KSWAPD);
+	atomic_set(p->running, KCOMPRESSD_NOT_STARTED);
+	return 0;
+}
+
+static int init_write_queue(void)
+{
+	int i;
+	unsigned int queue_len = queue_size_per_kcompressd * sizeof(struct write_work);
+
+	for (i = 0; i < nr_kcompressd; i++) {
+		if (kfifo_alloc(&kcompress[i].write_fifo,
+					queue_len, GFP_KERNEL)) {
+			pr_err("Failed to alloc kfifo %d\n", i);
+			return -ENOMEM;
+		}
+	}
+	return 0;
+}
+
+static void clean_bio_queue(int idx)
+{
+	struct write_work entry;
+
+	while (sizeof(struct write_work) == kfifo_out(&kcompress[idx].write_fifo,
+				&entry, sizeof(struct write_work))) {
+		bio_put(entry.bio);
+		entry.cb(entry.mem, entry.bio);
+	}
+	kfifo_free(&kcompress[idx].write_fifo);
+}
+
+static int kcompress_update(void)
+{
+	int i;
+	int ret;
+
+	kcompress = kvmalloc_array(nr_kcompressd, sizeof(struct kcompress), GFP_KERNEL);
+	if (!kcompress)
+		return -ENOMEM;
+
+	kcompressd_para = kvmalloc_array(nr_kcompressd, sizeof(struct kcompressd_para), GFP_KERNEL);
+	if (!kcompressd_para)
+		return -ENOMEM;
+
+	ret = init_write_queue();
+	if (ret) {
+		pr_err("Initialization of writing to FIFOs failed!!\n");
+		return ret;
+	}
+
+	for (i = 0; i < nr_kcompressd; i++) {
+		init_waitqueue_head(&kcompress[i].kcompressd_wait);
+		kcompressd_para[i].kcompressd_wait = &kcompress[i].kcompressd_wait;
+		kcompressd_para[i].write_fifo = &kcompress[i].write_fifo;
+		kcompressd_para[i].running = &kcompress[i].running;
+	}
+
+	return 0;
+}
+
+static void stop_all_kcompressd_thread(void)
+{
+	int i;
+
+	for (i = 0; i < nr_kcompressd; i++) {
+		kthread_stop(kcompress[i].kcompressd);
+		kcompress[i].kcompressd = NULL;
+		clean_bio_queue(i);
+	}
+}
+
+static int do_nr_kcompressd_handler(const char *val,
+		const struct kernel_param *kp)
+{
+	int ret;
+
+	atomic_set(&enable_kcompressd, false);
+
+	stop_all_kcompressd_thread();
+
+	ret = param_set_int(val, kp);
+	if (!ret) {
+		pr_err("Invalid number of kcompressd.\n");
+		return -EINVAL;
+	}
+
+	ret = init_write_queue();
+	if (ret) {
+		pr_err("Initialization of writing to FIFOs failed!!\n");
+		return ret;
+	}
+
+	atomic_set(&enable_kcompressd, true);
+
+	return 0;
+}
+
+static const struct kernel_param_ops param_ops_change_nr_kcompressd = {
+	.set = &do_nr_kcompressd_handler,
+	.get = &param_get_uint,
+	.free = NULL,
+};
+
+module_param_cb(nr_kcompressd, &param_ops_change_nr_kcompressd,
+		&nr_kcompressd, 0644);
+MODULE_PARM_DESC(nr_kcompressd, "Number of pre-created daemon for page compression");
+
+static int do_queue_size_per_kcompressd_handler(const char *val,
+		const struct kernel_param *kp)
+{
+	int ret;
+
+	atomic_set(&enable_kcompressd, false);
+
+	stop_all_kcompressd_thread();
+
+	ret = param_set_int(val, kp);
+	if (!ret) {
+		pr_err("Invalid queue size for kcompressd.\n");
+		return -EINVAL;
+	}
+
+	ret = init_write_queue();
+	if (ret) {
+		pr_err("Initialization of writing to FIFOs failed!!\n");
+		return ret;
+	}
+
+	pr_info("Queue size for kcompressd was changed: %d\n", queue_size_per_kcompressd);
+
+	atomic_set(&enable_kcompressd, true);
+	return 0;
+}
+
+static const struct kernel_param_ops param_ops_change_queue_size_per_kcompressd = {
+	.set = &do_queue_size_per_kcompressd_handler,
+	.get = &param_get_uint,
+	.free = NULL,
+};
+
+module_param_cb(queue_size_per_kcompressd, &param_ops_change_queue_size_per_kcompressd,
+		&queue_size_per_kcompressd, 0644);
+MODULE_PARM_DESC(queue_size_per_kcompressd,
+		"Size of queue for kcompressd");
+
+int schedule_bio_write(void *mem, struct bio *bio, compress_callback cb)
+{
+	int i;
+	bool submit_success = false;
+	size_t sz_work = sizeof(struct write_work);
+
+	struct write_work entry = {
+		.mem = mem,
+		.bio = bio,
+		.cb = cb
+	};
+
+	if (unlikely(!atomic_read(&enable_kcompressd)))
+		return -EBUSY;
+
+	if (!nr_kcompressd || !current_is_kswapd())
+		return -EBUSY;
+
+	bio_get(bio);
+
+	for (i = 0; i < nr_kcompressd; i++) {
+		submit_success =
+			(kfifo_avail(&kcompress[i].write_fifo) >= sz_work) &&
+			(sz_work == kfifo_in(&kcompress[i].write_fifo, &entry, sz_work));
+
+		if (submit_success) {
+			switch (atomic_read(&kcompress[i].running)) {
+			case KCOMPRESSD_NOT_STARTED:
+				atomic_set(&kcompress[i].running, KCOMPRESSD_RUNNING);
+				kcompress[i].kcompressd = kthread_run(kcompressd,
+						&kcompressd_para[i], "kcompressd:%d", i);
+				if (IS_ERR(kcompress[i].kcompressd)) {
+					atomic_set(&kcompress[i].running, KCOMPRESSD_NOT_STARTED);
+					pr_warn("Failed to start kcompressd:%d\n", i);
+					clean_bio_queue(i);
+				}
+				break;
+			case KCOMPRESSD_RUNNING:
+				break;
+			case KCOMPRESSD_SLEEPING:
+				wake_up_interruptible(&kcompress[i].kcompressd_wait);
+				break;
+			}
+			return 0;
+		}
+	}
+
+	bio_put(bio);
+	return -EBUSY;
+}
+EXPORT_SYMBOL(schedule_bio_write);
+
+static int __init kcompressd_init(void)
+{
+	int ret;
+
+	nr_kcompressd = DEFAULT_NR_KCOMPRESSD;
+	queue_size_per_kcompressd = INIT_QUEUE_SIZE;
+
+	ret = kcompress_update();
+	if (ret) {
+		pr_err("Init kcompressd failed!\n");
+		return ret;
+	}
+
+	atomic_set(&enable_kcompressd, true);
+	blocking_notifier_call_chain(&kcompressd_notifier_list, 0, NULL);
+	return 0;
+}
+
+static void __exit kcompressd_exit(void)
+{
+	atomic_set(&enable_kcompressd, false);
+	stop_all_kcompressd_thread();
+
+	kvfree(kcompress);
+	kvfree(kcompressd_para);
+}
+
+module_init(kcompressd_init);
+module_exit(kcompressd_exit);
+
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("Qun-Wei Lin <[e-mail]>");
+MODULE_DESCRIPTION("Separate the page compression from the memory reclaiming");
+
diff --git a/drivers/block/zram/kcompressd.h b/drivers/block/zram/kcompressd.h
new file mode 100644
index 000000000000..2fe0b424a7af
--- /dev/null
+++ b/drivers/block/zram/kcompressd.h
@@ -0,0 +1,25 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2024 MediaTek Inc.
+ */
+
+#ifndef _KCOMPRESSD_H_
+#define _KCOMPRESSD_H_
+
+#include <linux/rwsem.h>
+#include <linux/kfifo.h>
+#include <linux/atomic.h>
+
+typedef void (*compress_callback)(void *mem, struct bio *bio);
+
+struct kcompress {
+	struct task_struct *kcompressd;
+	wait_queue_head_t kcompressd_wait;
+	struct kfifo write_fifo;
+	atomic_t running;
+};
+
+int kcompressd_enabled(void);
+int schedule_bio_write(void *mem, struct bio *bio, compress_callback cb);
+#endif
+
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 2e1a70f2f4bd..bcd63ecb6ff2 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -35,6 +35,7 @@ 
 #include <linux/part_stat.h>
 #include <linux/kernel_read_file.h>
 
+#include "kcompressd.h"
 #include "zram_drv.h"
 
 static DEFINE_IDR(zram_index_idr);
@@ -2240,6 +2241,15 @@  static void zram_bio_write(struct zram *zram, struct bio *bio)
 	bio_endio(bio);
 }
 
+#if IS_ENABLED(CONFIG_KCOMPRESSD)
+static void zram_bio_write_callback(void *mem, struct bio *bio)
+{
+	struct zram *zram = (struct zram *)mem;
+
+	zram_bio_write(zram, bio);
+}
+#endif
+
 /*
  * Handler function for all zram I/O requests.
  */
@@ -2252,6 +2262,10 @@  static void zram_submit_bio(struct bio *bio)
 		zram_bio_read(zram, bio);
 		break;
 	case REQ_OP_WRITE:
+#if IS_ENABLED(CONFIG_KCOMPRESSD)
+		if (kcompressd_enabled() && !schedule_bio_write(zram, bio, zram_bio_write_callback))
+			break;
+#endif
 		zram_bio_write(zram, bio);
 		break;
 	case REQ_OP_DISCARD:
@@ -2535,9 +2549,11 @@  static int zram_add(void)
 #if ZRAM_LOGICAL_BLOCK_SIZE == PAGE_SIZE
 		.max_write_zeroes_sectors	= UINT_MAX,
 #endif
-		.features			= BLK_FEAT_STABLE_WRITES	|
-						  BLK_FEAT_READ_SYNCHRONOUS	|
-						  BLK_FEAT_WRITE_SYNCHRONOUS,
+		.features			= BLK_FEAT_STABLE_WRITES
+						  | BLK_FEAT_READ_SYNCHRONOUS
+#if !IS_ENABLED(CONFIG_KCOMPRESSD)
+						  | BLK_FEAT_WRITE_SYNCHRONOUS,
+#endif
 	};
 	struct zram *zram;
 	int ret, device_id;