From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.3451.1610726306756259259 for ; Fri, 15 Jan 2021 07:58:26 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=gPggKgEd; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: philmd@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610726306; 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=zDRDbxY5OG3+bSymMbDBtQ9peC1sswiX8DHL+UKmW+c=; b=gPggKgEdrEYjTVelbPROUFe0pO/Y14+VFDIa8UcfzfQRo/wO1er0F3PWzz58x+R6PXlToP SyWz38Rtz4Mbt/sUgEj/YFPkEBr47t8svMOOg99gnPYnyjudGATPKcSYCRa5lpp2QPh5ol Vdj/xaltPrTpXyDgMEYEZGYpE83FlBg= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-327-870I29TlOWif5sB4goD6hg-1; Fri, 15 Jan 2021 10:58:22 -0500 X-MC-Unique: 870I29TlOWif5sB4goD6hg-1 Received: by mail-ed1-f71.google.com with SMTP id n8so2071103edo.19 for ; Fri, 15 Jan 2021 07:58:22 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=zDRDbxY5OG3+bSymMbDBtQ9peC1sswiX8DHL+UKmW+c=; b=HPQD/EgPCNc9IZ3gIGw78QphyW+vSF6+uaTVCiMkwVyCtCf/nTwoniu5bZUisDSf7C L+Wg8y9kjcDkbH81EhrOhUMksd8QI+sMUuQgBWOJEQx2stylu5p2jnmk+/cN3qLF9Bk6 UZmaHNd5c+QOjdWCVn6l98LrhTjqQuS8sKJNFlRwWanTOVz53b3LR6DnyQdwUDWAjluq hUkjhhPNdALQrzxK9lTffxCgvAEQJCsdE5tM2EjIwieYcTlhhEhjvOEkozZuwqzDgC6W YGQdOCOcB58D9geomgWXQs39ObYrzaI6TvHXOpc2N7RHmez8LSU6L8Tha3uUfZ1s+k6l p+zQ== X-Gm-Message-State: AOAM531GeADCnljmLRVsOw8oRrZWDhIWMjso1sc1hW4RW4e4Yrvt3Z45 0qKA+n+sFRYWWcnX9nUd9PlTtw2ZJuTOHcWGSkQKhif6MD5Kp9JHaDkdQqILV9qJGUWaZvBismK uMLltXUksdjpu9Q== X-Received: by 2002:a05:6402:34c5:: with SMTP id w5mr10162277edc.65.1610726301572; Fri, 15 Jan 2021 07:58:21 -0800 (PST) X-Google-Smtp-Source: ABdhPJww1KmLWw8wW9Yq4vXo3/+2v8LuC1Hvw3WzJH+mUxHxCTaqHysIWXmJRAqhv+sytBcrX+bQuA== X-Received: by 2002:a05:6402:34c5:: with SMTP id w5mr10162263edc.65.1610726301364; Fri, 15 Jan 2021 07:58:21 -0800 (PST) Return-Path: Received: from [192.168.1.36] (13.red-83-57-169.dynamicip.rima-tde.net. [83.57.169.13]) by smtp.gmail.com with ESMTPSA id rh2sm3738440ejb.68.2021.01.15.07.58.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 15 Jan 2021 07:58:20 -0800 (PST) Subject: Re: [PATCH v2 02/10] OvmfPkg: raise PcdShellFileOperationSize to 128KB To: Laszlo Ersek , 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> <9ecb1a31-001f-f57e-c97c-afd527383fa9@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Message-ID: Date: Fri, 15 Jan 2021 16:58:19 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <9ecb1a31-001f-f57e-c97c-afd527383fa9@redhat.com> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=philmd@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 1/15/21 11:09 AM, Laszlo Ersek wrote: > On 01/15/21 10:34, Philippe Mathieu-Daudé wrote: >> On 1/13/21 9:54 AM, Laszlo Ersek wrote: >>> >>> 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 a lot for the full explanation! I'm glad I asked this early enough so this was fresh in your memory :) At least we got it archived on the list. Shouldn't QEMU announce this via a better mechanism? I.e. a fw_cfg entry? This is so convoluted (as you said) than I'm worried libfuse change a parameter and we end having a debug nightmare figuring the root cause... Anyhow this is not blocking this patch, so: Reviewed-by: Philippe Mathieu-Daude Regards, Phil.