RE: [PATCH 7/7] block: remove ->rw_page

From: Dan Williams <dan.j.williams@intel.com>
To: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
	Minchan Kim <minchan@kernel.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	"Dan Williams" <dan.j.williams@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Ira Weiny <ira.weiny@intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	<linux-block@vger.kernel.org>, <nvdimm@lists.linux.dev>,
	<linux-fsdevel@vger.kernel.org>, <linux-mm@kvack.org>
Subject: RE: [PATCH 7/7] block: remove ->rw_page
Date: Wed, 25 Jan 2023 10:38:34 -0800	[thread overview]
Message-ID: <63d1772ae1bfd_3a36e529479@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <20230125133436.447864-8-hch@lst.de>

Christoph Hellwig wrote:
> The ->rw_page method is a special purpose bypass of the usual bio
> handling path that is limited to single-page reads and writes and
> synchronous which causes a lot of extra code in the drivers, callers
> and the block layer.
> 
> The only remaining user is the MM swap code.  Switch that swap code to
> simply submit a single-vec on-stack bio an synchronously wait on it
> based on a newly added QUEUE_FLAG_SYNCHRONOUS flag set by the drivers
> that currently implement ->rw_page instead.  While this touches one
> extra cache line and executes extra code, it simplifies the block
> layer and drivers and ensures that all feastures are properly supported
> by all drivers, e.g. right now ->rw_page bypassed cgroup writeback
> entirely.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/bdev.c                  | 78 -----------------------------------
>  drivers/block/brd.c           | 15 +------
>  drivers/block/zram/zram_drv.c | 61 +--------------------------
>  drivers/nvdimm/btt.c          | 16 +------
>  drivers/nvdimm/pmem.c         | 24 +----------
>  include/linux/blkdev.h        | 12 +++---
>  mm/page_io.c                  | 53 ++++++++++++++----------
>  mm/swapfile.c                 |  2 +-
>  8 files changed, 44 insertions(+), 217 deletions(-)
> 
[..]
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 89f51d68c68ad6..1bffe8f44ae9a8 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -555,6 +555,7 @@ struct request_queue {
>  #define QUEUE_FLAG_IO_STAT	7	/* do disk/partitions IO accounting */
>  #define QUEUE_FLAG_NOXMERGES	9	/* No extended merges */
>  #define QUEUE_FLAG_ADD_RANDOM	10	/* Contributes to random pool */
> +#define QUEUE_FLAG_SYNCHRONOUS	11	/* always complets in submit context */

s/complets/completes/

>  #define QUEUE_FLAG_SAME_FORCE	12	/* force complete on same CPU */
>  #define QUEUE_FLAG_INIT_DONE	14	/* queue is initialized */
>  #define QUEUE_FLAG_STABLE_WRITES 15	/* don't modify blks until WB is done */
> @@ -1252,6 +1253,12 @@ static inline bool bdev_nonrot(struct block_device *bdev)
>  	return blk_queue_nonrot(bdev_get_queue(bdev));
>  }
>  

Other than that, this looks good and passes regression:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>