From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web08.1955.1610705381029764509 for ; Fri, 15 Jan 2021 02:09:41 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Gk0Y9tXQ; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610705380; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=RoXfJOJgcFYFHAB3EAUYSfQcHj2Ph+i5QpUkj3KBpCI=; b=Gk0Y9tXQi3TTcVXnIYd94DjFFkJckulI+6UUwGgVIAfAnsum+EE3TLoX0sA0aQY8oKVZ5t kRwJMNCxU2WYWrhcLRlVJY9A9BB4hCKAiQQ0Bt+KNPyKkTFPZXpTNn/Iy4GFxXBicyuLUP ZMFhrpieisRSSYSypbxzPgsM7TZCqdc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-220--fHSg8NdObCO6-EM9G7VDQ-1; Fri, 15 Jan 2021 05:09:36 -0500 X-MC-Unique: -fHSg8NdObCO6-EM9G7VDQ-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5FC7A180A09D; Fri, 15 Jan 2021 10:09:35 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-223.ams2.redhat.com [10.36.112.223]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4CA4E71C87; Fri, 15 Jan 2021 10:09:32 +0000 (UTC) Subject: Re: [PATCH v2 02/10] OvmfPkg: raise PcdShellFileOperationSize to 128KB To: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= , devel@edk2.groups.io Cc: Ard Biesheuvel , Jordan Justen References: <20210113085453.10168-1-lersek@redhat.com> <20210113085453.10168-3-lersek@redhat.com> <6ca294e1-d19a-8d80-94c4-372c06b0df6b@redhat.com> From: "Laszlo Ersek" Message-ID: <9ecb1a31-001f-f57e-c97c-afd527383fa9@redhat.com> Date: Fri, 15 Jan 2021 11:09:31 +0100 MIME-Version: 1.0 In-Reply-To: <6ca294e1-d19a-8d80-94c4-372c06b0df6b@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 01/15/21 10:34, Philippe Mathieu-Daudé wrote: > Hi Laszlo, > > On 1/13/21 9:54 AM, Laszlo Ersek wrote: >> Some UEFI shell commands read and write files in chunks. The chunk size is >> given by "PcdShellFileOperationSize", whose default in >> "ShellPkg/ShellPkg.dec" is 4KB (0x1000). >> >> The virtio-fs daemon of QEMU advertizes a 128KB maximum buffer size by >> default, for the FUSE_WRITE operation. > > I delayed this patch review because I couldn't find where this value > is advertized in QEMU (virtiofsd is very new to me). Can you enlighten > me please? Yes, absolutely. It's really difficult to find. I actually started to capture all that information in what would be commit 6f7bc7196ff2 ("OvmfPkg/VirtioFsDxe: implement the wrapper function for FUSE_WRITE", 2020-12-21), or perhaps in this patch even -- but in the end, it was such a divergent set of commits, originating actually in the "libfuse" project, from which virtiofsd was forked, that I decided to drop it. So, in virtiofsd, you actually have to look for the *page count* that corresponds to 128KB -- FUSE_DEFAULT_MAX_PAGES_PER_REQ (32). In the fuse_session_new() function, we have se->conn.max_write = UINT_MAX; se->bufsize = FUSE_MAX_MAX_PAGES * getpagesize() + FUSE_BUFFER_HEADER_SIZE; In the do_init() function, which handles the FUSE_INIT request, we have size_t bufsize = se->bufsize; then if (!(arg->flags & FUSE_MAX_PAGES)) { size_t max_bufsize = FUSE_DEFAULT_MAX_PAGES_PER_REQ * getpagesize() + FUSE_BUFFER_HEADER_SIZE; if (bufsize > max_bufsize) { bufsize = max_bufsize; } } and later if (se->conn.max_write > bufsize - FUSE_BUFFER_HEADER_SIZE) { se->conn.max_write = bufsize - FUSE_BUFFER_HEADER_SIZE; } and then outarg.max_write = se->conn.max_write; It's super convoluted, because a bunch of flags are handled, and they all control the buffer size one way or another, and so the various limits are converted and enforced back and forth. In practice, the virtio-fs driver in OVMF does not set the FUSE_MAX_PAGES flag in the FUSE_INIT request, and so in virtiofsd, we first reduce "bufsize" from FUSE_MAX_MAX_PAGES * getpagesize() + FUSE_BUFFER_HEADER_SIZE; to FUSE_DEFAULT_MAX_PAGES_PER_REQ * getpagesize() + FUSE_BUFFER_HEADER_SIZE; and then calculate "max_write" by subtracting FUSE_BUFFER_HEADER_SIZE. The result is FUSE_DEFAULT_MAX_PAGES_PER_REQ * getpagesize() which is 128 KB. In the libfuse project , one related commit is: https://github.com/libfuse/libfuse/commit/027d0d17c8a4 and before that, https://github.com/libfuse/libfuse/commit/97f4a9cb4fc69 If you try to peek even before that, you end up with historical commits like https://github.com/libfuse/libfuse/commit/065f222cd5850 So basically the *origin* of max_write=128KB is untraceable :) But, at least the present behavior can be found; it comes from libfuse commit 027d0d17c8a4. Approaching the same topic from the FUSE client in the Linux kernel, commit 5da784cce430 ("fuse: add max_pages to init_out", 2018-10-01) is relevant; it also points to the following discussion on LKML: . Again it's convoluted; the direct answer to your question is, "max_write=128KB comes from FUSE_DEFAULT_MAX_PAGES_PER_REQ, because the FUSE_MAX_PAGES flag is not requested in the FUSE_INT operation". Thanks, Laszlo > >> By raising PcdShellFileOperationSize 32-fold, the number of FUSE write >> requests shrinks proportionately, when writing large files. And when a >> Virtio Filesystem is not used, a 128KB chunk size is still not >> particularly wasteful. >> >> Some ad-hoc measurements on my laptop, using OVMF: >> >> - The time it takes to copy a ~270MB file from a Virtio Filesystem to the >> same Virtio Filesystem improves from ~9 seconds to ~1 second. >> >> - The time it takes to compare two identical ~270MB files on the same >> Virtio Filesystem improves from ~11 seconds to ~3 seconds. >> >> Cc: Ard Biesheuvel >> Cc: Jordan Justen >> Cc: Philippe Mathieu-Daudé >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3125 >> Signed-off-by: Laszlo Ersek >> Acked-by: Ard Biesheuvel >> --- >> >> Notes: >> v2: >> - no changes >> - pick up Ard's A-b >> >> OvmfPkg/OvmfPkgIa32.dsc | 2 ++ >> OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++ >> OvmfPkg/OvmfPkgX64.dsc | 2 ++ >> 3 files changed, 6 insertions(+) >