Re: [PATCH 3/3] zram: remove swap_slot_free_notify

From: Alexey Romanov <AVRomanov@sberdevices.ru>
To: Minchan Kim <minchan@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm <linux-mm@kvack.org>, Jens Axboe <axboe@kernel.dk>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	Seth Jennings <sjenning@redhat.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>
Subject: Re: [PATCH 3/3] zram: remove swap_slot_free_notify
Date: Tue, 11 Jul 2023 10:09:58 +0000	[thread overview]
Message-ID: <20230711101000.mqby77xjewnhkx2t@cab-wsm-0029881.sigma.sbrf.ru> (raw)
In-Reply-To: <20230710221659.2473460-4-minchan@kernel.org>

Hello,

On Mon, Jul 10, 2023 at 03:16:59PM -0700, Minchan Kim wrote:
> Frontswap replaces the need for swap_slot_free_notify in block_device.
> Therefore, we can remove it.
> 
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  Documentation/filesystems/locking.rst |  5 -----
>  drivers/block/zram/zram_drv.c         | 20 +-------------------
>  include/linux/blkdev.h                |  2 --
>  mm/swapfile.c                         |  8 --------
>  4 files changed, 1 insertion(+), 34 deletions(-)
> 
> diff --git a/Documentation/filesystems/locking.rst b/Documentation/filesystems/locking.rst
> index aa1a233b0fa8..c94ef7d9fc6e 100644
> --- a/Documentation/filesystems/locking.rst
> +++ b/Documentation/filesystems/locking.rst
> @@ -477,7 +477,6 @@ block_device_operations
>  				unsigned long *);
>  	void (*unlock_native_capacity) (struct gendisk *);
>  	int (*getgeo)(struct block_device *, struct hd_geometry *);
> -	void (*swap_slot_free_notify) (struct block_device *, unsigned long);
>  
>  locking rules:
>  
> @@ -491,12 +490,8 @@ compat_ioctl:		no
>  direct_access:		no
>  unlock_native_capacity:	no
>  getgeo:			no
> -swap_slot_free_notify:	no	(see below)
>  ======================= ===================
>  
> -swap_slot_free_notify is called with swap_lock and sometimes the page lock
> -held.
> -
>  
>  file_operations
>  ===============
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 5e973c982235..ec040ab3ab91 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -1947,23 +1947,6 @@ static void zram_submit_bio(struct bio *bio)
>  	}
>  }
>  
> -static void zram_slot_free_notify(struct block_device *bdev,
> -				unsigned long index)
> -{
> -	struct zram *zram;
> -
> -	zram = bdev->bd_disk->private_data;
> -
> -	atomic64_inc(&zram->stats.notify_free);
> -	if (!zram_slot_trylock(zram, index)) {
> -		atomic64_inc(&zram->stats.miss_free);
> -		return;
> -	}
> -
> -	zram_free_page(zram, index);
> -	zram_slot_unlock(zram, index);
> -}
> -
>  static void zram_destroy_comps(struct zram *zram)
>  {
>  	u32 prio;
> @@ -2117,7 +2100,6 @@ static int zram_open(struct block_device *bdev, fmode_t mode)
>  static const struct block_device_operations zram_devops = {
>  	.open = zram_open,
>  	.submit_bio = zram_submit_bio,
> -	.swap_slot_free_notify = zram_slot_free_notify,
>  	.owner = THIS_MODULE
>  };
>  
> @@ -2206,7 +2188,7 @@ static int zram_frontswap_load(unsigned int type, pgoff_t index,
>  
>  	err = zram_read_from_zspool(zram, page, index);
>  	if (!err)
> -		zram_accessed(zram, index);
> +		zram_free_page(zram, index);

Probably it should be in the previous commit.

>  	zram_slot_unlock(zram, index);
>  
>  	return err;
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index c0ffe203a602..49e1843e44ea 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1390,8 +1390,6 @@ struct block_device_operations {
>  	int (*getgeo)(struct block_device *, struct hd_geometry *);
>  	int (*set_read_only)(struct block_device *bdev, bool ro);
>  	void (*free_disk)(struct gendisk *disk);
> -	/* this callback is with swap_lock and sometimes page table lock held */
> -	void (*swap_slot_free_notify) (struct block_device *, unsigned long);
>  	int (*report_zones)(struct gendisk *, sector_t sector,
>  			unsigned int nr_zones, report_zones_cb cb, void *data);
>  	char *(*devnode)(struct gendisk *disk, umode_t *mode);
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index a9424fd226bc..adc16d8883c8 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -726,7 +726,6 @@ static void swap_range_free(struct swap_info_struct *si, unsigned long offset,
>  {
>  	unsigned long begin = offset;
>  	unsigned long end = offset + nr_entries - 1;
> -	void (*swap_slot_free_notify)(struct block_device *, unsigned long);
>  
>  	if (offset < si->lowest_bit)
>  		si->lowest_bit = offset;
> @@ -739,16 +738,9 @@ static void swap_range_free(struct swap_info_struct *si, unsigned long offset,
>  	}
>  	atomic_long_add(nr_entries, &nr_swap_pages);
>  	WRITE_ONCE(si->inuse_pages, si->inuse_pages - nr_entries);
> -	if (si->flags & SWP_BLKDEV)
> -		swap_slot_free_notify =
> -			si->bdev->bd_disk->fops->swap_slot_free_notify;
> -	else
> -		swap_slot_free_notify = NULL;
>  	while (offset <= end) {
>  		arch_swap_invalidate_page(si->type, offset);
>  		frontswap_invalidate_page(si->type, offset);
> -		if (swap_slot_free_notify)
> -			swap_slot_free_notify(si->bdev, offset);
>  		offset++;
>  	}
>  	clear_shadow_from_swap_cache(si->type, begin, end);
> -- 
> 2.41.0.255.g8b1d071c50-goog
> 
> 

-- 
Thank you,
Alexey