Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests

[Peter Lieven]

On 23.09.2014 10:47, Kevin Wolf wrote:
> Am 08.09.2014 um 16:54 hat Peter Lieven geschrieben:
> > On 08.09.2014 16:42, Paolo Bonzini wrote:
> > > Il 08/09/2014 16:35, Peter Lieven ha scritto:
> > > > Whats your opinion changed the max_xfer_len to 0xffff regardsless
> > > > of use_16_for_rw in iSCSI?
> > > If you implemented request splitting in the block layer, it would be
> > > okay to force max_xfer_len to 0xffff.
> > Unfortunately, I currently have no time for that. It will include some 
> > shuffling with
> > qiovs that has to be properly tested.
> >
> > Regarding iSCSI: In fact currently the limit is 0xffff for all iSCSI Targets < 
> > 2TB.
> > So I thought that its not obvious at all why a > 2TB target can handle bigger 
> > requests.
> >
> > To the root cause of this patch multiwrite_merge I still have some thoughts:
> >   - why are we merging requests for raw (especially host devices and/or iSCSI?)
> >     The original patch from Kevin was to mitigate a QCOW2 performance 
> > regression.
> The problem wasn't in qcow2, though, it just became more apparent there
> because lots of small requests are deadly to performance during cluster
> allocation. Installation simply took ages compared to IDE. If you do a
> real benchmark, you'll probably see (smaller) differences with raw, too.
>
> The core problem is virtio-blk getting much smaller requests. IIRC, I
> got an average of 128k-512k per request for IDE and something as poor as
> 4k-32k for virtio-blk. If I read this thread correctly, you're saying
> that this is still the case. I suspect there is something wrong with the
> guest driver, but somebody would have to dig into that.

This seems to be the case. I will check if this disappears with most
recent kernels virtio-blk versions.

> >     For iSCSI the qiov concats are destroying all the zerocopy efforts we made.
> If this is true, it just means that the iscsi driver sucks for vectored
> I/O and needs to be fixed.

It works quite well, I misunderstood the iovec_concat routine at first. I 
thought
it was copying payload data around. The overhead for bilding the qiov structure
only should be neglectible.


> >   - should we only merge requests within the same cluster?
> Does it hurt to merge everything we can? The block driver needs to be
> able to take things apart anyway, the large request could come from
> somewhere else (guest, block job, builtin NBD server, etc.)

I was just thinking. It was unclear what the original intention was.

My concern was that merging too much will increase latency for
the specific I/O even if it increases throughput.

> >   - why is there no multiread_merge?
> Because nobody implemented it. :-)

Maybe its worth looking at this. Taking the multiwrite_merge stuff as
a basis it shouldn't be too hard.

> As I said above, writes hurt a lot because of qcow2 cluster allocation.
> Reads are probably losing some performance as well (someone would need
> to check this), but processing them separately isn't quite as painful.

I will try to sort that out.

Peter