1. 18 Jul, 2020 2 commits
    • Boris Burkov's avatar
      blk-cgroup: show global disk stats in root cgroup io.stat · ef45fe47
      Boris Burkov authored
      In order to improve consistency and usability in cgroup stat accounting,
      we would like to support the root cgroup's io.stat.
      
      Since the root cgroup has processes doing io even if the system has no
      explicitly created cgroups, we need to be careful to avoid overhead in
      that case.  For that reason, the rstat algorithms don't handle the root
      cgroup, so just turning the file on wouldn't give correct statistics.
      
      To get around this, we simulate flushing the iostat struct by filling it
      out directly from global disk stats. The result is a root cgroup io.stat
      file consistent with both /proc/diskstats and io.stat.
      
      Note that in order to collect the disk stats, we needed to iterate over
      devices. To facilitate that, we had to change the linkage of a disk_type
      to external so that it can be used from blk-cgroup.c to iterate over
      disks.
      Suggested-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarBoris Burkov <boris@bur.io>
      Acked-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      ef45fe47
    • Boris Burkov's avatar
      blk-cgroup: make iostat functions visible to stat printing · cd1fc4b9
      Boris Burkov authored
      Previously, the code which printed io.stat only needed access to the
      generic rstat flushing code, but since we plan to write some more
      specific code for preparing root cgroup stats, we need to manipulate
      iostat structs directly. Since declaring static functions ahead does not
      seem like common practice in this file, simply move the iostat functions
      up. We only plan to use blkg_iostat_set, but it seems better to keep them
      all together.
      Signed-off-by: default avatarBoris Burkov <boris@bur.io>
      Acked-by: default avatarTejun Heo <tj@kernel.org>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      cd1fc4b9
  2. 17 Jul, 2020 4 commits
    • Coly Li's avatar
      block: improve discard bio alignment in __blkdev_issue_discard() · 9b15d109
      Coly Li authored
      This patch improves discard bio split for address and size alignment in
      __blkdev_issue_discard(). The aligned discard bio may help underlying
      device controller to perform better discard and internal garbage
      collection, and avoid unnecessary internal fragment.
      
      Current discard bio split algorithm in __blkdev_issue_discard() may have
      non-discarded fregment on device even the discard bio LBA and size are
      both aligned to device's discard granularity size.
      
      Here is the example steps on how to reproduce the above problem.
      - On a VMWare ESXi 6.5 update3 installation, create a 51GB virtual disk
        with thin mode and give it to a Linux virtual machine.
      - Inside the Linux virtual machine, if the 50GB virtual disk shows up as
        /dev/sdb, fill data into the first 50GB by,
              # dd if=/dev/zero of=/dev/sdb bs=4096 count=13107200
      - Discard the 50GB range from offset 0 on /dev/sdb,
              # blkdiscard /dev/sdb -o 0 -l 53687091200
      - Observe the underlying mapping status of the device
              # sg_get_lba_status /dev/sdb -m 1048 --lba=0
        descriptor LBA: 0x0000000000000000  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000000000800  blocks: 16773120  deallocated
        descriptor LBA: 0x0000000000fff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000001000000  blocks: 8386560  deallocated
        descriptor LBA: 0x00000000017ff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000001800000  blocks: 8386560  deallocated
        descriptor LBA: 0x0000000001fff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000002000000  blocks: 8386560  deallocated
        descriptor LBA: 0x00000000027ff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000002800000  blocks: 8386560  deallocated
        descriptor LBA: 0x0000000002fff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000003000000  blocks: 8386560  deallocated
        descriptor LBA: 0x00000000037ff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000003800000  blocks: 8386560  deallocated
        descriptor LBA: 0x0000000003fff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000004000000  blocks: 8386560  deallocated
        descriptor LBA: 0x00000000047ff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000004800000  blocks: 8386560  deallocated
        descriptor LBA: 0x0000000004fff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000005000000  blocks: 8386560  deallocated
        descriptor LBA: 0x00000000057ff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000005800000  blocks: 8386560  deallocated
        descriptor LBA: 0x0000000005fff800  blocks: 2048  mapped (or unknown)
        descriptor LBA: 0x0000000006000000  blocks: 6291456  deallocated
        descriptor LBA: 0x0000000006600000  blocks: 0  deallocated
      
      Although the discard bio starts at LBA 0 and has 50<<30 bytes size which
      are perfect aligned to the discard granularity, from the above list
      these are many 1MB (2048 sectors) internal fragments exist unexpectedly.
      
      The problem is in __blkdev_issue_discard(), an improper algorithm causes
      an improper bio size which is not aligned.
      
       25 int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
       26                 sector_t nr_sects, gfp_t gfp_mask, int flags,
       27                 struct bio **biop)
       28 {
       29         struct request_queue *q = bdev_get_queue(bdev);
         [snipped]
       56
       57         while (nr_sects) {
       58                 sector_t req_sects = min_t(sector_t, nr_sects,
       59                                 bio_allowed_max_sectors(q));
       60
       61                 WARN_ON_ONCE((req_sects << 9) > UINT_MAX);
       62
       63                 bio = blk_next_bio(bio, 0, gfp_mask);
       64                 bio->bi_iter.bi_sector = sector;
       65                 bio_set_dev(bio, bdev);
       66                 bio_set_op_attrs(bio, op, 0);
       67
       68                 bio->bi_iter.bi_size = req_sects << 9;
       69                 sector += req_sects;
       70                 nr_sects -= req_sects;
         [snipped]
       79         }
       80
       81         *biop = bio;
       82         return 0;
       83 }
       84 EXPORT_SYMBOL(__blkdev_issue_discard);
      
      At line 58-59, to discard a 50GB range, req_sects is set as return value
      of bio_allowed_max_sectors(q), which is 8388607 sectors. In the above
      case, the discard granularity is 2048 sectors, although the start LBA
      and discard length are aligned to discard granularity, req_sects never
      has chance to be aligned to discard granularity. This is why there are
      some still-mapped 2048 sectors fragment in every 4 or 8 GB range.
      
      If req_sects at line 58 is set to a value aligned to discard_granularity
      and close to UNIT_MAX, then all consequent split bios inside device
      driver are (almostly) aligned to discard_granularity of the device
      queue. The 2048 sectors still-mapped fragment will disappear.
      
      This patch introduces bio_aligned_discard_max_sectors() to return the
      the value which is aligned to q->limits.discard_granularity and closest
      to UINT_MAX. Then this patch replaces bio_allowed_max_sectors() with
      this new routine to decide a more proper split bio length.
      
      But we still need to handle the situation when discard start LBA is not
      aligned to q->limits.discard_granularity, otherwise even the length is
      aligned, current code may still leave 2048 fragment around every 4GB
      range. Therefore, to calculate req_sects, firstly the start LBA of
      discard range is checked (including partition offset), if it is not
      aligned to discard granularity, the first split location should make
      sure following bio has bi_sector aligned to discard granularity. Then
      there won't be still-mapped fragment in the middle of the discard range.
      
      The above is how this patch improves discard bio alignment in
      __blkdev_issue_discard(). Now with this patch, after discard with same
      command line mentiond previously, sg_get_lba_status returns,
      descriptor LBA: 0x0000000000000000  blocks: 106954752  deallocated
      descriptor LBA: 0x0000000006600000  blocks: 0  deallocated
      
      We an see there is no 2048 sectors segment anymore, everything is clean.
      Reported-and-tested-by: default avatarAcshai Manoj <acshai.manoj@microfocus.com>
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarHannes Reinecke <hare@suse.com>
      Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
      Reviewed-by: default avatarXiao Ni <xni@redhat.com>
      Cc: Bart Van Assche <bvanassche@acm.org>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Enzo Matsumiya <ematsumiya@suse.com>
      Cc: Jens Axboe <axboe@kernel.dk>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      9b15d109
    • Coly Li's avatar
      block: change REQ_OP_ZONE_RESET and REQ_OP_ZONE_RESET_ALL to be odd numbers · ecdef9f4
      Coly Li authored
      Currently REQ_OP_ZONE_RESET and REQ_OP_ZONE_RESET_ALL are defined as
      even numbers 6 and 8, such zone reset bios are treated as READ bios by
      bio_data_dir(), which is obviously misleading.
      
      The macro bio_data_dir() is defined in include/linux/bio.h as,
       55 #define bio_data_dir(bio) \
       56         (op_is_write(bio_op(bio)) ? WRITE : READ)
      
      And op_is_write() is defined in include/linux/blk_types.h as,
      397 static inline bool op_is_write(unsigned int op)
      398 {
      399         return (op & 1);
      400 }
      
      The convention of op_is_write() is when there is data transfer then the
      op code should be odd number, and treat as a write op. bio_data_dir()
      treats all bio direction as READ if op_is_write() reports false, and
      WRITE if op_is_write() reports true.
      
      Because REQ_OP_ZONE_RESET and REQ_OP_ZONE_RESET_ALL are even numbers,
      although they don't transfer data but reporting them as READ bio by
      bio_data_dir() is misleading and might be wrong. Because these two
      commands will reset the writer pointers of the resetting zones, and all
      content after the reset write pointer will be invalid and unaccessible,
      obviously they are not READ bios in any means.
      
      This patch changes REQ_OP_ZONE_RESET from 6 to 15, and changes
      REQ_OP_ZONE_RESET_ALL from 8 to 17. Now bios with these two op code
      can be treated as WRITE by bio_data_dir(). Although they don't transfer
      data, now we keep them consistent with REQ_OP_DISCARD and
      REQ_OP_WRITE_ZEROES with the ituition that they change on-media content
      and should be WRITE request.
      Signed-off-by: default avatarColy Li <colyli@suse.de>
      Reviewed-by: default avatarDamien Le Moal <damien.lemoal@wdc.com>
      Reviewed-by: default avatarChaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
      Cc: Christoph Hellwig <hch@lst.de>
      Cc: Hannes Reinecke <hare@suse.de>
      Cc: Jens Axboe <axboe@fb.com>
      Cc: Johannes Thumshirn <johannes.thumshirn@wdc.com>
      Cc: Keith Busch <kbusch@kernel.org>
      Cc: Shaun Tancheff <shaun.tancheff@seagate.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      ecdef9f4
    • Yufen Yu's avatar
      block: defer flush request no matter whether we have elevator · b5718d6c
      Yufen Yu authored
      Commit 7520872c ("block: don't defer flushes on blk-mq + scheduling")
      tried to fix deadlock for cycled wait between flush requests and data
      request into flush_data_in_flight. The former holded all driver tags
      and wait for data request completion, but the latter can not complete
      for waiting free driver tags.
      
      After commit 923218f6 ("blk-mq: don't allocate driver tag upfront
      for flush rq"), flush requests will not get driver tag before queuing
      into flush queue.
      
      * With elevator, flush request just get sched_tags before inserting
        flush queue. It will not get driver tag until issue them to driver.
        data request on list fq->flush_data_in_flight will complete in
        the end.
      
      * Without elevator, each flush request will get a driver tag when
        allocate request. Then data request on fq->flush_data_in_flight
        don't worry about lacking driver tag.
      
      In both of these cases, cycled wait cannot be true. So we may allow
      to defer flush request.
      Signed-off-by: default avatarYufen Yu <yuyufen@huawei.com>
      Reviewed-by: default avatarMing Lei <ming.lei@redhat.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      b5718d6c
    • Wei Yongjun's avatar
      block: make blk_timeout_init() static · 943c4d90
      Wei Yongjun authored
      The sparse tool complains as follows:
      
      block/blk-timeout.c:93:12: warning:
       symbol 'blk_timeout_init' was not declared. Should it be static?
      
      Function blk_timeout_init() is not used outside of blk-timeout.c, so
      mark it static.
      
      Fixes: 9054650f ("block: relax jiffies rounding for timeouts")
      Reported-by: default avatarHulk Robot <hulkci@huawei.com>
      Signed-off-by: default avatarWei Yongjun <weiyongjun1@huawei.com>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      943c4d90
  3. 16 Jul, 2020 6 commits
  4. 15 Jul, 2020 3 commits
    • Jens Axboe's avatar
      Revert "blk-rq-qos: remove redundant finish_wait to rq_qos_wait." · e791ee68
      Jens Axboe authored
      This reverts commit 826f2f48.
      
      Qian Cai reports that this commit causes stalls with swap. Revert until
      the reason can be figured out.
      Reported-by: default avatarQian Cai <cai@lca.pw>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      e791ee68
    • Ming Lei's avatar
      block: always remove partitions from blk_drop_partitions() · d0f0f1b4
      Ming Lei authored
      In theory, when GENHD_FL_NO_PART_SCAN is set, no partitions can be created
      on one disk. However, ioctl(BLKPG, BLKPG_ADD_PARTITION) doesn't check
      GENHD_FL_NO_PART_SCAN, so partitions still can be added even though
      GENHD_FL_NO_PART_SCAN is set.
      
      So far blk_drop_partitions() only removes partitions when disk_part_scan_enabled()
      return true. This way can make ghost partition on loop device after changing/clearing
      FD in case that PARTSCAN is disabled, such as partitions can be added
      via 'parted' on loop disk even though GENHD_FL_NO_PART_SCAN is set.
      
      Fix this issue by always removing partitions in blk_drop_partitions(), and
      this way is correct because the current code supposes that no partitions
      can be added in case of GENHD_FL_NO_PART_SCAN.
      Signed-off-by: default avatarMing Lei <ming.lei@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Cc: Christoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      d0f0f1b4
    • Jens Axboe's avatar
      block: relax jiffies rounding for timeouts · 9054650f
      Jens Axboe authored
      In doing high IOPS testing, blk-mq is generally pretty well optimized.
      There are a few things that stuck out as using more CPU than what is
      really warranted, and one thing is the round_jiffies_up() that we do
      twice for each request. That accounts for about 0.8% of the CPU in
      my testing.
      
      We can make this cheaper by avoiding an integer division, by just adding
      a rough HZ mask that we can AND with instead. The timeouts are only on a
      second granularity already, we don't have to be that accurate here and
      this patch barely changes that. All we care about is nice grouping.
      Signed-off-by: default avatarJens Axboe <axboe@kernel.dk>
      9054650f
  5. 10 Jul, 2020 2 commits
  6. 08 Jul, 2020 12 commits
  7. 07 Jul, 2020 1 commit
  8. 02 Jul, 2020 2 commits
  9. 01 Jul, 2020 8 commits