On Mon, Feb 05 2024 at 6:40P -0500,
Damien Le Moal <dlemoal@xxxxxxxxxx> wrote:
> On 2/6/24 05:33, Mike Snitzer wrote:
> > On Mon, Feb 05 2024 at 12:38P -0500,
> > Damien Le Moal <dlemoal@xxxxxxxxxx> 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@xxxxxxxxxx>
> >>
> >> 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
