Re: [PATCH 10/26] dm: Use the block layer zone append emulation

[Mike Snitzer <snitzer@kernel.org>]

On Mon, Feb 05 2024 at  6:40P -0500,
Damien Le Moal <dlemoal@kernel.org> wrote:

> On 2/6/24 05:33, Mike Snitzer wrote:
> > On Mon, Feb 05 2024 at 12:38P -0500,
> > Damien Le Moal <dlemoal@kernel.org> wrote:
> > 
> >> On 2/4/24 02:58, Mike Snitzer wrote:
> >>> Love the overall improvement to the DM core code and the broader block
> >>> layer by switching to this bio-based ZWP approach.
> >>>
> >>> Reviewed-by: Mike Snitzer <snitzer@kernel.org>
> >>
> >> Thanks Mike !
> >>
> >>> But one incremental suggestion inlined below.
> >>
> >> I made this change, but in a lightly different form as I noticed that I was
> >> getting compile errors when CONFIG_BLK_DEV_ZONED is disabled.
> >> The change look like this now:
> >>
> >> static void dm_split_and_process_bio(struct mapped_device *md,
> >> 				     struct dm_table *map, struct bio *bio)
> >> {
> >> 	...
> >> 	need_split = is_abnormal = is_abnormal_io(bio);
> >> 	if (static_branch_unlikely(&zoned_enabled))
> >> 		need_split = is_abnormal || dm_zone_bio_needs_split(md, bio);
> >>
> >> 	...
> >>
> >> 	/*
> >> 	 * Use the block layer zone write plugging for mapped devices that
> >> 	 * need zone append emulation (e.g. dm-crypt).
> >> 	 */
> >> 	if (static_branch_unlikely(&zoned_enabled) &&
> >> 	    dm_zone_write_plug_bio(md, bio))
> >> 		return;
> >>
> >> 	...
> >>
> >> with these added to dm-core.h:
> >>
> >> static inline bool dm_zone_bio_needs_split(struct mapped_device *md,
> >> 					   struct bio *bio)
> >> {
> >> 	return md->emulate_zone_append && bio_straddle_zones(bio);
> >> }
> >> static inline bool dm_zone_write_plug_bio(struct mapped_device *md,
> >> 					  struct bio *bio)
> >> {
> >> 	return md->emulate_zone_append && blk_zone_write_plug_bio(bio, 0);
> >> }
> >>
> >> These 2 helpers define to "return false" for !CONFIG_BLK_DEV_ZONED.
> >> I hope this works for you. Otherwise, I will drop your review tag when posting V2.
> > 
> > Why expose them in dm-core.h ?
> > 
> > Just have what you put in dm-core.h above dm_split_and_process_bio in dm.c ?
> 
> I wanted to avoid "#ifdef CONFIG_BLK_DEV_ZONED" in the .c files. But if you are
> OK with that, I can move these inline functions in dm.c.

I'm OK with it, dm.c already does something like this for
dm_queue_destroy_crypto_profile() by checking if
CONFIG_BLK_INLINE_ENCRYPTION defined.

Mike