mbox series

[v7,0/9] discard blockstats

Message ID 20190514121015.32190-1-anton.nefedov@virtuozzo.com (mailing list archive)
Headers show
Series discard blockstats | expand

Message

Anton Nefedov May 14, 2019, 12:10 p.m. UTC
hi,

yet another take for this patch series; please kindly consider these for 4.1

Just a few cosmetic comments were received for v6 so this is mostly
a rebase+ping.

new in v7:
    - general rebase
    - since clauses -> 4.1
    - patch 8: not completely trivial rebase: raw_account_discard moved to
      common raw_do_pdiscard()
    - patch 9: comment wording fixed

v6: http://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg06633.html
v5: http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg06828.html
v4: http://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg04308.html
v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg03688.html

----

qmp query-blockstats provides stats info for write/read/flush ops.

Patches 1-7 implement the similar for discard (unmap) command for scsi
and ide disks.
Discard stat "unmap_ops / unmap_bytes" is supposed to account the ops that
have completed without an error.

However, discard operation is advisory. Specifically,
 - common block layer ignores ENOTSUP error code.
   That might be returned if the block driver does not support discard,
   or discard has been configured to be ignored.
 - format drivers such as qcow2 may ignore discard if they were configured
   to ignore that, or if the corresponding area is already marked unused
   (unallocated / zero clusters).

And what is actually useful is the number of bytes actually discarded
down on the host filesystem.
To achieve that, driver-specific statistics has been added to blockstats
(patch 9).
With patch 8, file-posix driver accounts discard operations on its level too.

query-blockstat result:

(note the difference between blockdevice unmap and file discard stats. qcow2
sends fewer ops down to the file as the clusters are actually unallocated
on qcow2 level)

    {
      "device": "drive-scsi0-0-0-0",
      "node-name": "#block159",
      "stats": {
>       "invalid_unmap_operations": 0,
>       "failed_unmap_operations": 0,
        "wr_highest_offset": 13411688448,
        "rd_total_time_ns": 2859566315,
        "rd_bytes": 103182336,
        "rd_merged": 0,
        "flush_operations": 19,
        "invalid_wr_operations": 0,
        "flush_total_time_ns": 23111608,
        "failed_rd_operations": 0,
        "failed_flush_operations": 0,
        "invalid_flush_operations": 0,
        "timed_stats": [
          
        ],
        "wr_merged": 0,
        "wr_bytes": 1702912,
>       "unmap_bytes": 11954954240,
>       "unmap_operations": 865,
        "idle_time_ns": 2669508623,
        "account_invalid": true,
>       "unmap_total_time_ns": 19698002,
        "wr_operations": 143,
        "failed_wr_operations": 0,
        "rd_operations": 4816,
        "account_failed": true,
>       "unmap_merged": 0,
        "wr_total_time_ns": 1262686124,
        "invalid_rd_operations": 0
      },
      "parent": {
>       "driver-specific": {
>         "discard-nb-failed": 0,
>         "discard-bytes-ok": 720896,
>         "driver": "file",
>         "discard-nb-ok": 8
>       },
        "node-name": "#block009",
        "stats": {
        [..]
        }
      }
    },
    {
      "device": "floppy0",

Anton Nefedov (9):
  qapi: group BlockDeviceStats fields
  qapi: add unmap to BlockDeviceStats
  block: add empty account cookie type
  ide: account UNMAP (TRIM) operations
  scsi: store unmap offset and nb_sectors in request struct
  scsi: move unmap error checking to the complete callback
  scsi: account unmap operations
  file-posix: account discard operations
  qapi: query-blockstat: add driver specific file-posix stats

 qapi/block-core.json       | 81 ++++++++++++++++++++++++++++++++------
 include/block/accounting.h |  2 +
 include/block/block.h      |  1 +
 include/block/block_int.h  |  1 +
 block.c                    |  9 +++++
 block/accounting.c         |  6 +++
 block/file-posix.c         | 54 ++++++++++++++++++++++++-
 block/qapi.c               | 11 ++++++
 hw/ide/core.c              | 12 ++++++
 hw/scsi/scsi-disk.c        | 32 +++++++++------
 tests/qemu-iotests/227.out | 18 +++++++++
 11 files changed, 204 insertions(+), 23 deletions(-)

Comments

Stefano Garzarella June 3, 2019, 10:09 a.m. UTC | #1
On Tue, May 14, 2019 at 12:10:40PM +0000, Anton Nefedov wrote:
> hi,
> 
> yet another take for this patch series; please kindly consider these for 4.1
> 
> Just a few cosmetic comments were received for v6 so this is mostly
> a rebase+ping.
> 
> new in v7:
>     - general rebase
>     - since clauses -> 4.1
>     - patch 8: not completely trivial rebase: raw_account_discard moved to
>       common raw_do_pdiscard()
>     - patch 9: comment wording fixed
> 
> v6: http://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg06633.html
> v5: http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg06828.html
> v4: http://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg04308.html
> v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg03688.html
> 

Hi Anton,
since qemu 4.0 added the discard support on virtio-blk, should we consider it
in this series?

If you prefer I can work on it and send you the patch.

Cheers,
Stefano
Anton Nefedov June 4, 2019, 4:53 p.m. UTC | #2
On 3/6/2019 1:09 PM, Stefano Garzarella wrote:
> On Tue, May 14, 2019 at 12:10:40PM +0000, Anton Nefedov wrote:
>> hi,
>>
>> yet another take for this patch series; please kindly consider these for 4.1
>>
>> Just a few cosmetic comments were received for v6 so this is mostly
>> a rebase+ping.
>>
>> new in v7:
>>      - general rebase
>>      - since clauses -> 4.1
>>      - patch 8: not completely trivial rebase: raw_account_discard moved to
>>        common raw_do_pdiscard()
>>      - patch 9: comment wording fixed
>>
>> v6: http://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg06633.html
>> v5: http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg06828.html
>> v4: http://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg04308.html
>> v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg03688.html
>>
> 
> Hi Anton,
> since qemu 4.0 added the discard support on virtio-blk, should we consider it
> in this series?
> 
> If you prefer I can work on it and send you the patch.
> 

hi Stefano,

if this series is finally getting in, it is really nice if you can add
virtio-blk support too.

thanks!
Stefano Garzarella June 6, 2019, 3:35 p.m. UTC | #3
On Tue, Jun 04, 2019 at 04:53:23PM +0000, Anton Nefedov wrote:
> On 3/6/2019 1:09 PM, Stefano Garzarella wrote:
> > On Tue, May 14, 2019 at 12:10:40PM +0000, Anton Nefedov wrote:
> >> hi,
> >>
> >> yet another take for this patch series; please kindly consider these for 4.1
> >>
> >> Just a few cosmetic comments were received for v6 so this is mostly
> >> a rebase+ping.
> >>
> >> new in v7:
> >>      - general rebase
> >>      - since clauses -> 4.1
> >>      - patch 8: not completely trivial rebase: raw_account_discard moved to
> >>        common raw_do_pdiscard()
> >>      - patch 9: comment wording fixed
> >>
> >> v6: http://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg06633.html
> >> v5: http://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg06828.html
> >> v4: http://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg04308.html
> >> v3: http://lists.nongnu.org/archive/html/qemu-devel/2018-06/msg03688.html
> >>
> > 
> > Hi Anton,
> > since qemu 4.0 added the discard support on virtio-blk, should we consider it
> > in this series?
> > 
> > If you prefer I can work on it and send you the patch.
> > 
> 
> hi Stefano,
> 
> if this series is finally getting in, it is really nice if you can add
> virtio-blk support too.
> 

Sure, I'll do it!

Thanks,
Stefano