RE: [PATCH v8] virtio_blk: add discard and write zeroes support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@xxxxxxxxxxxxx]
> Sent: Monday, October 15, 2018 5:28 PM
> To: Daniel Verkamp <dverkamp@xxxxxxxxxxxx>
> Cc: virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx; linux-block@xxxxxxxxxxxxxxx;
> Michael S. Tsirkin <mst@xxxxxxxxxx>; Jason Wang <jasowang@xxxxxxxxxx>;
> Jens Axboe <axboe@xxxxxxxxx>; Stefan Hajnoczi <stefanha@xxxxxxxxxx>; Liu,
> Changpeng <changpeng.liu@xxxxxxxxx>
> Subject: Re: [PATCH v8] virtio_blk: add discard and write zeroes support
> 
> On Fri, Oct 12, 2018 at 02:06:28PM -0700, Daniel Verkamp wrote:
> > From: Changpeng Liu <changpeng.liu@xxxxxxxxx>
> >
> > In commit 88c85538, "virtio-blk: add discard and write zeroes features
> > to specification" (https://github.com/oasis-tcs/virtio-spec), the virtio
> 
> There is some issues in this spec.  For one using the multiple ranges
> also for write zeroes is rather inefficient.  Write zeroes really should
> use the same format as read and write.
Because there is no length parameter for virtio block specification, adding the
two extra commands will not break the existing specification and driver implementation. 
Also existing Linux implementation for write zeroes will not use multiple segment
at all so there is always one range in practice.
> 
> Second the unmap flag isn't properly specified at all, as nothing
> says the device may not unmap without the unmap flag.  Please take
> a look at the SCSI or NVMe ѕpec for some guidance.
The unmap flag is only used for write zeroes command, as discard command will not 
guarantee the spaces will be zeroed, so adding this flag means (Discard + Write Zeroes),
so this definitely is backend related, the backend implementation can use same code
to implement discard and write zeroes commands.
> 
> > +static inline int virtblk_setup_discard_write_zeroes(struct request *req,
> > +						bool unmap)
> 
> Why is this an inline function?




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux