On Tue, 05/12 13:52, Kevin Wolf wrote: > Am 12.05.2015 um 08:09 hat Fam Zheng geschrieben: > > For zero write, callers pass in NULL qiov (qemu-io "write -z" or > > scsi-disk "write same"). > > > > Commit fc3959e466 fixed bdrv_co_write_zeroes which is the common case > > for this bug, but it still exists in bdrv_aio_write_zeroes. A simpler > > fix would be in bdrv_co_do_pwritev which is the NULL dereference point > > and covers both cases. > > > > So don't access it in bdrv_co_do_pwritev in this case, use three aligned > > writes. > > > > Signed-off-by: Fam Zheng <f...@redhat.com> > > --- > > block/io.c | 95 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 95 insertions(+) > > > > diff --git a/block/io.c b/block/io.c > > index 4e5a92e..d766220 100644 > > --- a/block/io.c > > +++ b/block/io.c > > @@ -1174,6 +1174,97 @@ static int coroutine_fn > > bdrv_aligned_pwritev(BlockDriverState *bs, > > return ret; > > } > > > > +static int coroutine_fn bdrv_co_do_zero_pwritev(BlockDriverState *bs, > > + int64_t offset, > > + unsigned int bytes, > > + BdrvRequestFlags flags) > > +{ > > + BdrvTrackedRequest req; > > + uint8_t *buf = NULL; > > + QEMUIOVector local_qiov; > > + struct iovec iov; > > + uint64_t align = MAX(BDRV_SECTOR_SIZE, bs->request_alignment); > > + unsigned int head_padding_bytes, tail_padding_bytes; > > + int ret; > > + > > + head_padding_bytes = offset & (align - 1); > > + tail_padding_bytes = align - ((offset + bytes) & (align - 1)); > > Don't we have macros for these calculations?
No, I don't see any. > > + tracked_request_begin(&req, bs, offset, bytes, true); > > Why duplicate this when it would already be the next line in > bdrv_co_do_pwritev()? I'll remove the duplication. > > > + mark_request_serialising(&req, align); > > + wait_serialising_requests(&req); > > So this patch serialises all zero writes, even if they are perfectly > aligned? Why? > > Actually, even for misaligned requests, I think the part in the middle > doesn't require any serialisation, only the RMW parts do. I'll move to branches. Thanks, Fam