From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49301) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWLsh-0007K5-32 for qemu-devel@nongnu.org; Tue, 23 Sep 2014 04:55:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWLsU-00088b-GW for qemu-devel@nongnu.org; Tue, 23 Sep 2014 04:55:31 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:58644 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWLsU-00087J-6d for qemu-devel@nongnu.org; Tue, 23 Sep 2014 04:55:18 -0400 Message-ID: <5421356C.3030108@kamp.de> Date: Tue, 23 Sep 2014 10:55:08 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1409935888-18552-1-git-send-email-pl@kamp.de> <1409935888-18552-3-git-send-email-pl@kamp.de> <20140908134434.GB22582@irqsave.net> <540DB3E2.6010905@redhat.com> <540DB583.4030101@kamp.de> <540DB5EB.2070705@redhat.com> <540DBEBD.9040701@kamp.de> <540DC059.4000907@redhat.com> <540DC30B.2040408@kamp.de> <20140923084758.GA3871@noname.str.redhat.com> In-Reply-To: <20140923084758.GA3871@noname.str.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/4] block: immediately cancel oversized read/write requests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: =?windows-1252?Q?Beno=EEt_Cane?= =?windows-1252?Q?t?= , stefanha@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, ronniesahlberg@gmail.com, Paolo Bonzini 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