On 4/9/22 02:12, Ming Lei wrote:
> dm_zone_map_bio() is only called from __map_bio in which the io's
> reference is grabbed already, and the reference won't be released
> until the bio is submitted, so no necessary to do it dm_zone_map_bio
> any more.
I do not think that this patch is correct. Removing the extra reference on
the io can lead to problems if the io is completed in the target
->map(ti, clone) call or before dm_zone_map_bio_end() is called for the
DM_MAPIO_SUBMITTED or DM_MAPIO_REMAPPED cases. dm_zone_map_bio_end() needs
to reference the original bio, so we must ensure that it is still around,
which means keeping an extra IO reference to avoid dm_io_dec_pending()
calling bio_endio() on the orgi bio before dm_zone_map_bio_end() has a
chance to run.
>
> Cc: Damien Le Moal <damien.lemoal@xxxxxxx>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> drivers/md/dm-core.h | 7 -------
> drivers/md/dm-zone.c | 10 ----------
> drivers/md/dm.c | 7 ++++++-
> 3 files changed, 6 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index 4277853c7535..13136bd47cb3 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -277,13 +277,6 @@ static inline void dm_io_set_flag(struct dm_io *io, unsigned int bit)
> io->flags |= (1U << bit);
> }
>
> -static inline void dm_io_inc_pending(struct dm_io *io)
> -{
> - atomic_inc(&io->io_count);
> -}
> -
> -void dm_io_dec_pending(struct dm_io *io, blk_status_t error);
> -
> static inline struct completion *dm_get_completion_from_kobject(struct kobject *kobj)
> {
> return &container_of(kobj, struct dm_kobject_holder, kobj)->completion;
> diff --git a/drivers/md/dm-zone.c b/drivers/md/dm-zone.c
> index c1ca9be4b79e..85d3c158719f 100644
> --- a/drivers/md/dm-zone.c
> +++ b/drivers/md/dm-zone.c
> @@ -545,13 +545,6 @@ int dm_zone_map_bio(struct dm_target_io *tio)
> return DM_MAPIO_KILL;
> }
>
> - /*
> - * The target map function may issue and complete the IO quickly.
> - * Take an extra reference on the IO to make sure it does disappear
> - * until we run dm_zone_map_bio_end().
> - */
> - dm_io_inc_pending(io);
> -
> /* Let the target do its work */
> r = ti->type->map(ti, clone);
> switch (r) {
> @@ -580,9 +573,6 @@ int dm_zone_map_bio(struct dm_target_io *tio)
> break;
> }
>
> - /* Drop the extra reference on the IO */
> - dm_io_dec_pending(io, sts);
> -
> if (sts != BLK_STS_OK)
> return DM_MAPIO_KILL;
>
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 3c5fad7c4ee6..b8424a4b4725 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -929,11 +929,16 @@ static inline bool dm_tio_is_normal(struct dm_target_io *tio)
> !dm_tio_flagged(tio, DM_TIO_IS_DUPLICATE_BIO));
> }
>
> +static void dm_io_inc_pending(struct dm_io *io)
> +{
> + atomic_inc(&io->io_count);
> +}
> +
> /*
> * Decrements the number of outstanding ios that a bio has been
> * cloned into, completing the original io if necc.
> */
> -void dm_io_dec_pending(struct dm_io *io, blk_status_t error)
> +static void dm_io_dec_pending(struct dm_io *io, blk_status_t error)
> {
> /* Push-back supersedes any I/O errors */
> if (unlikely(error)) {
--
Damien Le Moal
Western Digital Research
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
