Re: [Qemu-devel] [PATCH v4 7/8] file-posix: account discard operations

[Anton Nefedov <anton.nefedov@virtuozzo.com>]

On 4/10/2018 6:52 PM, Kevin Wolf wrote:
> Am 21.08.2018 um 11:46 hat Anton Nefedov geschrieben:
>> This will help to identify how many of the user-issued discard operations
>> (accounted on a device level) have actually suceeded down on the host file
>> (even though the numbers will not be exactly the same if non-raw format
>> driver is used (e.g. qcow2 sending metadata discards)).
>>
>> Note that these numbers will not include discards triggered by
>> write-zeroes + MAY_UNMAP calls.
>>
>> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> 
> Why not implement accounting at the BDS level for all drivers? Then we
> can also reuse the existing BlockStats fields instead of duplicating
> them into driver-specific new ones.
> 
> Kevin
> 

I just wonder how useful is that?
Discards are interesting to see on the BDS level as they are optional.
Maybe one can be curious how much data is being read from backing
images. Anything else?

It feels like BDS level is complex enough as it is.
Accounting means we should keep that in mind in all the error paths,
with a risk to not account or to account twice (even more complicated
with bounce buffer fallback paths).

We could probably choose some anchor point to bind to, like

   - account at the very bottom only, i.e. right after drv->bdrv_x()
       (-) missing the cases where earlier sanity checks fired
       (-) easy to double-account fallback scenarios

   - account at tracked_request_begin/end
       (-) missing the cases where earlier sanity checks fired
       (-) accounting blk layer requests, and not the ones actually
           passed to the driver (which can be smaller in size due to
           BlockLimits - and can fail partially in discard case)

Maybe another option would be to keep BlockStats on BDS but still let
the drivers update them?

/Anton