All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Damien Le Moal <damien.lemoal@wdc.com>
Cc: linux-block@vger.kernel.org, dm-devel@redhat.com
Subject: Re: dm: Fix report zone remapping
Date: Fri, 5 Oct 2018 14:29:49 -0400	[thread overview]
Message-ID: <20181005182948.GA5628@redhat.com> (raw)
In-Reply-To: <20181005103452.7134-1-damien.lemoal@wdc.com>

On Fri, Oct 05 2018 at  6:34am -0400,
Damien Le Moal <damien.lemoal@wdc.com> wrote:

> If dm-linear or dm-flakey have targets on top of a partition of a zoned
> block device, remapping of the start sector and write pointer position
> of the zones reported by a report zones BIO must be modified to not
> only account for the target table entry mapping, but also to account
> for the partition first sector. This start sector must be substracted
> to the start sector of all zones reported. The write pointer position
> of sequential zones must also be reduced by this offset.
> 
> Since there is no easy way to access the underlying bdev of the target
> table entry from the dm_target or dm_target_io pointers, modify the
> interface of the function dm_remap_zone_report() to allow a target to
> pass the partition block device starting sector (offset).
> 
> Fixes: 10999307c14e ("dm: introduce dm_remap_zone_report()")
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> Cc: <stable@vger.kernel.org>

[dropping the actual stable cc from the mail, please don't cc stable in
the mail header, could be that git-send-email just pulled it in but...]

This needs a different fix.  There should be absolutely no need to pass
in a start offset for a device that is already supposed to be described
within the 'struct bio' passed to dm_remap_zone_report().

Why can't you just use the bio->bi_bdev ?  Err, since commit
74d46992e0d9d, I mean bio->bi_disk to get the start sector?

You should be able to get it from:

struct hd_struct *part = disk_get_part(bio->bi_disk, bio->bi_partno);
part->start_sect;
disk_put_part(part);

or:

struct block_device *bdev = bdget_disk(bio->bi_disk, bio->bi_partno);
get_start_sect(fc->dev->bdev);
bdput(bdev);

Mike


> ---
>  drivers/md/dm-flakey.c        |  3 ++-
>  drivers/md/dm-linear.c        |  3 ++-
>  drivers/md/dm.c               | 16 ++++++++++++----
>  include/linux/device-mapper.h |  2 +-
>  4 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
> index 21d126a5078c..c68c2e9cde6b 100644
> --- a/drivers/md/dm-flakey.c
> +++ b/drivers/md/dm-flakey.c
> @@ -381,7 +381,8 @@ static int flakey_end_io(struct dm_target *ti, struct bio *bio,
>  		return DM_ENDIO_DONE;
>  
>  	if (bio_op(bio) == REQ_OP_ZONE_REPORT) {
> -		dm_remap_zone_report(ti, bio, fc->start);
> +		dm_remap_zone_report(ti, bio, get_start_sect(fc->dev->bdev),
> +				     fc->start);
>  		return DM_ENDIO_DONE;
>  	}
>  
> diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
> index d10964d41fd7..f8cd367abbf6 100644
> --- a/drivers/md/dm-linear.c
> +++ b/drivers/md/dm-linear.c
> @@ -108,7 +108,8 @@ static int linear_end_io(struct dm_target *ti, struct bio *bio,
>  	struct linear_c *lc = ti->private;
>  
>  	if (!*error && bio_op(bio) == REQ_OP_ZONE_REPORT)
> -		dm_remap_zone_report(ti, bio, lc->start);
> +		dm_remap_zone_report(ti, bio, get_start_sect(lc->dev->bdev),
> +				     lc->start);
>  
>  	return DM_ENDIO_DONE;
>  }
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 20f7e4ef5342..ffd8544eedd8 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1162,7 +1162,8 @@ EXPORT_SYMBOL_GPL(dm_accept_partial_bio);
>   * REQ_OP_ZONE_REPORT bio to remap the zone descriptors obtained
>   * from the target device mapping to the dm device.
>   */
> -void dm_remap_zone_report(struct dm_target *ti, struct bio *bio, sector_t start)
> +void dm_remap_zone_report(struct dm_target *ti, struct bio *bio,
> +			  sector_t dev_offset, sector_t start)
>  {
>  #ifdef CONFIG_BLK_DEV_ZONED
>  	struct dm_target_io *tio = container_of(bio, struct dm_target_io, clone);
> @@ -1195,18 +1196,25 @@ void dm_remap_zone_report(struct dm_target *ti, struct bio *bio, sector_t start)
>  		/* Set zones start sector */
>  		while (hdr->nr_zones && ofst < bvec.bv_len) {
>  			zone = addr + ofst;
> +			zone->start -= dev_offset;
>  			if (zone->start >= start + ti->len) {
>  				hdr->nr_zones = 0;
>  				break;
>  			}
>  			zone->start = zone->start + ti->begin - start;
>  			if (zone->type != BLK_ZONE_TYPE_CONVENTIONAL) {
> -				if (zone->cond == BLK_ZONE_COND_FULL)
> +				switch (zone->cond) {
> +				case BLK_ZONE_COND_FULL:
>  					zone->wp = zone->start + zone->len;
> -				else if (zone->cond == BLK_ZONE_COND_EMPTY)
> +					break;
> +				case BLK_ZONE_COND_EMPTY:
>  					zone->wp = zone->start;
> -				else
> +					break;
> +				default:
> +					zone->wp -= dev_offset;
>  					zone->wp = zone->wp + ti->begin - start;
> +					break;
> +				}
>  			}
>  			ofst += sizeof(struct blk_zone);
>  			hdr->nr_zones--;
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index 6fb0808e87c8..22c1924c0ee8 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -421,7 +421,7 @@ int dm_suspended(struct dm_target *ti);
>  int dm_noflush_suspending(struct dm_target *ti);
>  void dm_accept_partial_bio(struct bio *bio, unsigned n_sectors);
>  void dm_remap_zone_report(struct dm_target *ti, struct bio *bio,
> -			  sector_t start);
> +			  sector_t dev_offset, sector_t start);
>  union map_info *dm_get_rq_mapinfo(struct request *rq);
>  
>  struct queue_limits *dm_get_queue_limits(struct mapped_device *md);
> -- 
> 2.17.1
> 

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Damien Le Moal <damien.lemoal@wdc.com>
Cc: dm-devel@redhat.com, linux-block@vger.kernel.org
Subject: Re: dm: Fix report zone remapping
Date: Fri, 5 Oct 2018 14:29:49 -0400	[thread overview]
Message-ID: <20181005182948.GA5628@redhat.com> (raw)
In-Reply-To: <20181005103452.7134-1-damien.lemoal@wdc.com>

On Fri, Oct 05 2018 at  6:34am -0400,
Damien Le Moal <damien.lemoal@wdc.com> wrote:

> If dm-linear or dm-flakey have targets on top of a partition of a zoned
> block device, remapping of the start sector and write pointer position
> of the zones reported by a report zones BIO must be modified to not
> only account for the target table entry mapping, but also to account
> for the partition first sector. This start sector must be substracted
> to the start sector of all zones reported. The write pointer position
> of sequential zones must also be reduced by this offset.
> 
> Since there is no easy way to access the underlying bdev of the target
> table entry from the dm_target or dm_target_io pointers, modify the
> interface of the function dm_remap_zone_report() to allow a target to
> pass the partition block device starting sector (offset).
> 
> Fixes: 10999307c14e ("dm: introduce dm_remap_zone_report()")
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> Cc: <stable@vger.kernel.org>

[dropping the actual stable cc from the mail, please don't cc stable in
the mail header, could be that git-send-email just pulled it in but...]

This needs a different fix.  There should be absolutely no need to pass
in a start offset for a device that is already supposed to be described
within the 'struct bio' passed to dm_remap_zone_report().

Why can't you just use the bio->bi_bdev ?  Err, since commit
74d46992e0d9d, I mean bio->bi_disk to get the start sector?

You should be able to get it from:

struct hd_struct *part = disk_get_part(bio->bi_disk, bio->bi_partno);
part->start_sect;
disk_put_part(part);

or:

struct block_device *bdev = bdget_disk(bio->bi_disk, bio->bi_partno);
get_start_sect(fc->dev->bdev);
bdput(bdev);

Mike


> ---
>  drivers/md/dm-flakey.c        |  3 ++-
>  drivers/md/dm-linear.c        |  3 ++-
>  drivers/md/dm.c               | 16 ++++++++++++----
>  include/linux/device-mapper.h |  2 +-
>  4 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/md/dm-flakey.c b/drivers/md/dm-flakey.c
> index 21d126a5078c..c68c2e9cde6b 100644
> --- a/drivers/md/dm-flakey.c
> +++ b/drivers/md/dm-flakey.c
> @@ -381,7 +381,8 @@ static int flakey_end_io(struct dm_target *ti, struct bio *bio,
>  		return DM_ENDIO_DONE;
>  
>  	if (bio_op(bio) == REQ_OP_ZONE_REPORT) {
> -		dm_remap_zone_report(ti, bio, fc->start);
> +		dm_remap_zone_report(ti, bio, get_start_sect(fc->dev->bdev),
> +				     fc->start);
>  		return DM_ENDIO_DONE;
>  	}
>  
> diff --git a/drivers/md/dm-linear.c b/drivers/md/dm-linear.c
> index d10964d41fd7..f8cd367abbf6 100644
> --- a/drivers/md/dm-linear.c
> +++ b/drivers/md/dm-linear.c
> @@ -108,7 +108,8 @@ static int linear_end_io(struct dm_target *ti, struct bio *bio,
>  	struct linear_c *lc = ti->private;
>  
>  	if (!*error && bio_op(bio) == REQ_OP_ZONE_REPORT)
> -		dm_remap_zone_report(ti, bio, lc->start);
> +		dm_remap_zone_report(ti, bio, get_start_sect(lc->dev->bdev),
> +				     lc->start);
>  
>  	return DM_ENDIO_DONE;
>  }
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index 20f7e4ef5342..ffd8544eedd8 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1162,7 +1162,8 @@ EXPORT_SYMBOL_GPL(dm_accept_partial_bio);
>   * REQ_OP_ZONE_REPORT bio to remap the zone descriptors obtained
>   * from the target device mapping to the dm device.
>   */
> -void dm_remap_zone_report(struct dm_target *ti, struct bio *bio, sector_t start)
> +void dm_remap_zone_report(struct dm_target *ti, struct bio *bio,
> +			  sector_t dev_offset, sector_t start)
>  {
>  #ifdef CONFIG_BLK_DEV_ZONED
>  	struct dm_target_io *tio = container_of(bio, struct dm_target_io, clone);
> @@ -1195,18 +1196,25 @@ void dm_remap_zone_report(struct dm_target *ti, struct bio *bio, sector_t start)
>  		/* Set zones start sector */
>  		while (hdr->nr_zones && ofst < bvec.bv_len) {
>  			zone = addr + ofst;
> +			zone->start -= dev_offset;
>  			if (zone->start >= start + ti->len) {
>  				hdr->nr_zones = 0;
>  				break;
>  			}
>  			zone->start = zone->start + ti->begin - start;
>  			if (zone->type != BLK_ZONE_TYPE_CONVENTIONAL) {
> -				if (zone->cond == BLK_ZONE_COND_FULL)
> +				switch (zone->cond) {
> +				case BLK_ZONE_COND_FULL:
>  					zone->wp = zone->start + zone->len;
> -				else if (zone->cond == BLK_ZONE_COND_EMPTY)
> +					break;
> +				case BLK_ZONE_COND_EMPTY:
>  					zone->wp = zone->start;
> -				else
> +					break;
> +				default:
> +					zone->wp -= dev_offset;
>  					zone->wp = zone->wp + ti->begin - start;
> +					break;
> +				}
>  			}
>  			ofst += sizeof(struct blk_zone);
>  			hdr->nr_zones--;
> diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
> index 6fb0808e87c8..22c1924c0ee8 100644
> --- a/include/linux/device-mapper.h
> +++ b/include/linux/device-mapper.h
> @@ -421,7 +421,7 @@ int dm_suspended(struct dm_target *ti);
>  int dm_noflush_suspending(struct dm_target *ti);
>  void dm_accept_partial_bio(struct bio *bio, unsigned n_sectors);
>  void dm_remap_zone_report(struct dm_target *ti, struct bio *bio,
> -			  sector_t start);
> +			  sector_t dev_offset, sector_t start);
>  union map_info *dm_get_rq_mapinfo(struct request *rq);
>  
>  struct queue_limits *dm_get_queue_limits(struct mapped_device *md);
> -- 
> 2.17.1
> 

  reply	other threads:[~2018-10-05 18:29 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-05 10:34 [PATCH] dm: Fix report zone remapping Damien Le Moal
2018-10-05 18:29 ` Mike Snitzer [this message]
2018-10-05 18:29   ` Mike Snitzer
2018-10-06  1:24   ` Damien Le Moal
2018-10-06  1:24     ` Damien Le Moal
2018-10-06  3:49     ` Mike Snitzer
2018-10-06  3:49       ` Mike Snitzer
2018-10-06  4:03       ` Damien Le Moal
2018-10-06  4:03         ` Damien Le Moal
2018-10-06  5:32         ` Mike Snitzer
2018-10-06  5:32           ` Mike Snitzer
2018-10-06  7:37           ` Damien Le Moal
2018-10-06  7:37             ` Damien Le Moal
2018-10-08 16:25             ` Mike Snitzer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181005182948.GA5628@redhat.com \
    --to=snitzer@redhat.com \
    --cc=damien.lemoal@wdc.com \
    --cc=dm-devel@redhat.com \
    --cc=linux-block@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.