Hello,
On Mon, Jul 10, 2023 at 03:16:58PM -0700, Minchan Kim wrote:
> With frontswap, zram can perform swapout 13% faster, which restores
> its original performance with rw_page.
>
> 200M swapout using MADV_PAGEOUT:
> Before: 4355ms After: 3786ms
>
> Signed-off-by: Minchan Kim <minchan@xxxxxxxxxx>
> ---
> drivers/block/zram/Kconfig | 1 +
> drivers/block/zram/zram_drv.c | 98 +++++++++++++++++++++++++++++++++++
> drivers/block/zram/zram_drv.h | 1 +
> 3 files changed, 100 insertions(+)
>
> diff --git a/drivers/block/zram/Kconfig b/drivers/block/zram/Kconfig
> index 0386b7da02aa..a841d6d14c74 100644
> --- a/drivers/block/zram/Kconfig
> +++ b/drivers/block/zram/Kconfig
> @@ -4,6 +4,7 @@ config ZRAM
> depends on BLOCK && SYSFS && MMU
> depends on CRYPTO_LZO || CRYPTO_ZSTD || CRYPTO_LZ4 || CRYPTO_LZ4HC || CRYPTO_842
> select ZSMALLOC
> + select FRONTSWAP
> help
> Creates virtual block devices called /dev/zramX (X = 0, 1, ...).
> Pages written to these disks are compressed and stored in memory
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 5d258a28ec43..5e973c982235 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -33,6 +33,8 @@
> #include <linux/debugfs.h>
> #include <linux/cpuhotplug.h>
> #include <linux/part_stat.h>
> +#include <linux/swap.h>
> +#include <linux/frontswap.h>
>
> #include "zram_drv.h"
>
> @@ -2170,6 +2172,90 @@ static struct attribute *zram_disk_attrs[] = {
>
> ATTRIBUTE_GROUPS(zram_disk);
>
> +static struct zram *zram_swaps[MAX_SWAPFILES];
> +static LIST_HEAD(zram_list);
> +static DEFINE_SPINLOCK(zram_list_lock);
> +
> +static int zram_frontswap_store(unsigned int type, pgoff_t index,
> + struct page *page)
> +{
> + int err;
> + struct zram *zram = zram_swaps[type];
> +
> + err = zram_write_page(zram, page, index);
> + if (!err) {
> + zram_slot_lock(zram, index);
> + zram_accessed(zram, index);
> + zram_slot_unlock(zram, index);
> + }
> +
> + return err;
> +}
> +
> +static int zram_frontswap_load(unsigned int type, pgoff_t index,
> + struct page *page, bool *exclusive)
> +{
> + int err;
> + struct zram *zram = zram_swaps[type];
> +
> + zram_slot_lock(zram, index);
> + if (zram_test_flag(zram, index, ZRAM_WB)) {
> + zram_slot_lock(zram, index);
Looks like zram_slot_unlock should be here instead of zram_slot_lock.
> + return -1;
> + }
> +
> + err = zram_read_from_zspool(zram, page, index);
> + if (!err)
> + zram_accessed(zram, index);
> + zram_slot_unlock(zram, index);
> +
> + return err;
> +}
> +
> +static void zram_frontswap_invalidate_area(unsigned int type)
> +{
> + struct zram *zram = zram_swaps[type];
> +
> + if (!zram)
> + return;
> +}
It makes sense? Maybe we just leave this function empty?
> +
> +static void zram_frontswap_init(unsigned int type, struct block_device *bdev)
> +{
> + struct zram *zram;
> +
> + spin_lock(&zram_list_lock);
> + list_for_each_entry(zram, &zram_list, list) {
> + if (&zram_devops == bdev->bd_disk->fops) {
> + zram_swaps[type] = zram;
> + break;
> + }
> + }
> + spin_unlock(&zram_list_lock);
> +}
> +
> +static void zram_frontswap_invalidate_page(unsigned int type, pgoff_t offset)
> +{
> + struct zram *zram = zram_swaps[type];
> +
> + if (!zram_slot_trylock(zram, offset)) {
> + atomic64_inc(&zram->stats.miss_free);
> + return;
> + }
> +
> + zram_free_page(zram, offset);
> + zram_slot_unlock(zram, offset);
> +}
> +
> +static const struct frontswap_ops zram_frontswap_ops = {
> + .store = zram_frontswap_store,
> + .load = zram_frontswap_load,
> + .init = zram_frontswap_init,
> + .invalidate_area = zram_frontswap_invalidate_area,
> + .invalidate_page = zram_frontswap_invalidate_page,
> + .init = zram_frontswap_init
> +};
> +
> /*
> * Allocate and initialize new zram device. the function returns
> * '>= 0' device_id upon success, and negative value otherwise.
> @@ -2246,6 +2332,10 @@ static int zram_add(void)
>
> comp_algorithm_set(zram, ZRAM_PRIMARY_COMP, default_compressor);
>
> + spin_lock(&zram_list_lock);
> + list_add(&zram->list, &zram_list);
> + spin_unlock(&zram_list_lock);
> +
> zram_debugfs_register(zram);
> pr_info("Added device: %s\n", zram->disk->disk_name);
> return device_id;
> @@ -2303,6 +2393,11 @@ static int zram_remove(struct zram *zram)
> zram_reset_device(zram);
>
> put_disk(zram->disk);
> +
> + spin_lock(&zram_list_lock);
> + list_del(&zram->list);
> + spin_unlock(&zram_list_lock);
> +
> kfree(zram);
> return 0;
> }
> @@ -2428,6 +2523,9 @@ static int __init zram_init(void)
> num_devices--;
> }
>
> + if (frontswap_register_ops(&zram_frontswap_ops))
> + pr_info("Frontswap is not used, which is suboptimal for zram swap.\n");
> +
> return 0;
>
> out_error:
> diff --git a/drivers/block/zram/zram_drv.h b/drivers/block/zram/zram_drv.h
> index ca7a15bd4845..0f52d2da8512 100644
> --- a/drivers/block/zram/zram_drv.h
> +++ b/drivers/block/zram/zram_drv.h
> @@ -139,5 +139,6 @@ struct zram {
> #ifdef CONFIG_ZRAM_MEMORY_TRACKING
> struct dentry *debugfs_dir;
> #endif
> + struct list_head list;
> };
> #endif
> --
> 2.41.0.255.g8b1d071c50-goog
>
>
--
Thank you,
Alexey
