On Wed, Oct 10 2018 at 10:15am -0400,
Mike Snitzer <snit...@redhat.com> wrote:

> On Tue, Oct 09 2018 at  9:52pm -0400,
> Damien Le Moal <damien.lem...@wdc.com> wrote:
> 
> > From: Christoph Hellwig <h...@lst.de>
> > 
> > Dispatching a report zones command through the request queue is a major
> > pain due to the command reply payload rewriting necessary. Given that
> > blkdev_report_zones() is executing everything synchronously, implement
> > report zones as a block device file operation instead, allowing major
> > simplification of the code in many places.
> > 
> > sd, null-blk, dm-linear and dm-flakey being the only block device
> > drivers supporting exposing zoned block devices, these drivers are
> > modified to provide the device side implementation of the
> > report_zones() block device file operation.
> > 
> > For dm-linear and dm-flakey, a new report_zones() target type operation
> > is defined so that the upper block layer call can be propagated down to
> > the underlying devices of the dm targets.
> > 
> > Signed-off-by: Christoph Hellwig <h...@lst.de>
> > [Damien]
> > * Changed method block_device argument to gendisk
> > * Various bug fixes and improvements
> > * Added support for null_blk, dm-linear and dm-flakey.
> > Signed-off-by: Damien Le Moal <damien.lem...@wdc.com>
> 
> I like that this patch removes the need for endio from dm-linear, etc.
> Very welcomed.
> 
> > ---
> >  block/blk-core.c               |   1 -
> >  block/blk-mq-debugfs.c         |   1 -
> >  block/blk-zoned.c              | 164 ++++++++++-----------------------
> >  drivers/block/null_blk.h       |  11 ++-
> >  drivers/block/null_blk_main.c  |  23 +----
> >  drivers/block/null_blk_zoned.c |  57 +++---------
> >  drivers/md/dm-flakey.c         |  28 ++++--
> >  drivers/md/dm-linear.c         |  31 ++++---
> >  drivers/md/dm.c                | 150 ++++++++++++++++--------------
> 
> <snip>
> 
> > diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> > index 20f7e4ef5342..e9fb3c706ef6 100644
> > --- a/drivers/md/dm.c
> > +++ b/drivers/md/dm.c
> > @@ -1156,79 +1203,48 @@ EXPORT_SYMBOL_GPL(dm_accept_partial_bio);
> >  
> >  /*
> >   * The zone descriptors obtained with a zone report indicate
> > - * zone positions within the target device. The zone descriptors
> > - * must be remapped to match their position within the dm device.
> > - * A target may call dm_remap_zone_report after completion of a
> > - * REQ_OP_ZONE_REPORT bio to remap the zone descriptors obtained
> > - * from the target device mapping to the dm device.
> > + * zone positions within the underlying device of the target. The zone
> > + * descriptors must be remapped to match their position within the dm 
> > device.
> > + * The caller target should obtain the zones information using
> > + * blkdev_report_zones() to ensure that remapping for partition offset is
> > + * already handled.
> >   */
> > -void dm_remap_zone_report(struct dm_target *ti, struct bio *bio, sector_t 
> > start)
> > +void dm_remap_zone_report(struct dm_target *ti, sector_t start,
> > +                     struct blk_zone *zones, unsigned int *nr_zones)
> >  {
> >  #ifdef CONFIG_BLK_DEV_ZONED
> > -   struct dm_target_io *tio = container_of(bio, struct dm_target_io, 
> > clone);
> > -   struct bio *report_bio = tio->io->orig_bio;
> > -   struct blk_zone_report_hdr *hdr = NULL;
> >     struct blk_zone *zone;
> > -   unsigned int nr_rep = 0;
> > -   unsigned int ofst;
> > -   struct bio_vec bvec;
> > -   struct bvec_iter iter;
> > -   void *addr;
> > -
> > -   if (bio->bi_status)
> > -           return;
> > +   unsigned int nrz = *nr_zones;
> > +   int i;
> >  
> >     /*
> > -    * Remap the start sector of the reported zones. For sequential zones,
> > -    * also remap the write pointer position.
> > +    * Remap the start sector and write pointer position of the zones in
> > +    * the array. Since we may have obtained from the target underlying
> > +    * device more zones that the target size, also adjust the number
> > +    * of zones.
> >      */
> > -   bio_for_each_segment(bvec, report_bio, iter) {
> > -           addr = kmap_atomic(bvec.bv_page);
> > -
> > -           /* Remember the report header in the first page */
> > -           if (!hdr) {
> > -                   hdr = addr;
> > -                   ofst = sizeof(struct blk_zone_report_hdr);
> > -           } else
> > -                   ofst = 0;
> > -
> > -           /* Set zones start sector */
> > -           while (hdr->nr_zones && ofst < bvec.bv_len) {
> > -                   zone = addr + ofst;
> > -                   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)
> > -                                   zone->wp = zone->start + zone->len;
> > -                           else if (zone->cond == BLK_ZONE_COND_EMPTY)
> > -                                   zone->wp = zone->start;
> > -                           else
> > -                                   zone->wp = zone->wp + ti->begin - start;
> > -                   }
> > -                   ofst += sizeof(struct blk_zone);
> > -                   hdr->nr_zones--;
> > -                   nr_rep++;
> > +   for (i = 0; i < nrz; i++) {
> > +           zone = zones + i;
> > +           if (zone->start >= start + ti->len) {
> > +                   memset(zone, 0, sizeof(struct blk_zone) * (nrz - i));
> > +                   break;
> >             }
> >  
> > -           if (addr != hdr)
> > -                   kunmap_atomic(addr);
> > -
> > -           if (!hdr->nr_zones)
> > -                   break;
> > -   }
> > +           zone->start = zone->start + ti->begin - start;
> > +           if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL)
> > +                   continue;
> >  
> > -   if (hdr) {
> > -           hdr->nr_zones = nr_rep;
> > -           kunmap_atomic(hdr);
> > +           if (zone->cond == BLK_ZONE_COND_FULL)
> > +                   zone->wp = zone->start + zone->len;
> > +           else if (zone->cond == BLK_ZONE_COND_EMPTY)
> > +                   zone->wp = zone->start;
> > +           else
> > +                   zone->wp = zone->wp + ti->begin - start;
> >     }
> >  
> > -   bio_advance(report_bio, report_bio->bi_iter.bi_size);
> > -
> > +   *nr_zones = i;
> >  #else /* !CONFIG_BLK_DEV_ZONED */
> > -   bio->bi_status = BLK_STS_NOTSUPP;
> > +   *nr_zones = 0;
> >  #endif
> >  }
> >  EXPORT_SYMBOL_GPL(dm_remap_zone_report);
> 
> Doesn't this hunk need to get rebased given your latest <= 4.19 stable@
> fix?  I've also tweaked that fix slightly to avoid line wrapping, I'm
> more relaxed about that aspect of code-style if it helps readability.
> Please rebase ontop of:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.19&id=9864cd5dc54cade89fd4b0954c2e522841aa247c
> 
> I'll be sending this to gregkh in the last dm pull for 4.19 final today
> or tomorrow.

FYI, I also just staged this performance fix that I'll also be sending
to gregkh (and it'll require you to rebase):

https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.19&id=beb9caac211c1be1bc118bb62d5cf09c4107e6a5

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to