Message ID | 20231030134806.24510492@canb.auug.org.au |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:d641:0:b0:403:3b70:6f57 with SMTP id cy1csp1953014vqb; Sun, 29 Oct 2023 19:48:38 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHnP9ncHL9Bk0lKCf4q27ir3QjkJ33cmTgzlMg4t+u9xMnjfRO6/4Md6q/XJbpDBOl8EF8i X-Received: by 2002:a81:a742:0:b0:5a0:ae01:803c with SMTP id e63-20020a81a742000000b005a0ae01803cmr8940598ywh.38.1698634117959; Sun, 29 Oct 2023 19:48:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1698634117; cv=none; d=google.com; s=arc-20160816; b=ZxNNqf4k5d54z3uveDjFiOAU3gnC7nDk+OZS3unirTXshcIlGW10iC964XHlrNKkKw pk33ZV3kr8AeZsr7wfKfB9dxzfnO4he4c0RLhefGsKdGKcXgv48LjZ4khhrtILMIp7zf dOHUDtVYVUUdszIx6QsMAPOm1U01L2bNc839nWvs4WoE6x9joem48PkGo2Dz2QDLotuX HGYgxf8kZi8RIPuIaMYkukIkeMUYctlgv3KM3yjrUoGl9bqxp7uYvtRksjAzT7Jf/TUK t91rQsRZsDTnSe9HfHQ9GiRs9DdMcCS7VMXiBXWRCaEsORoknrZnr4a+2W78RVgsooXO dZ8A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:message-id:subject:cc:to:from:date :dkim-signature; bh=2WPF2meFlfmuU5Z/7Pvyd5wbBkWFy41KCxzeBwEo1tQ=; fh=coqtP/cPw8bCuO6KWMGBBXIkxfFZp3lDPOFtyjPhUXk=; b=qTqaUrPFfU08dl//4O1rC1zH2SmySs+j5vcR2jnr87IfHfOjhnMlNmiz6p1I+wnm1I fxq5m3tdle2I3ss26cdpH9T/+8zhyiuCEOaUQE3NRfp4sPfBalu3NAjenSX0Q+oH/+fS 0YdTnGiugG8d1RuV6DNPyOpUduMdcP53cz21Hs8YhxKxiCAykQIFLU0o5Im+2Sf6uopX bxQrjMIJjququn5kesUlbKnj8t+QiROE3L68lYUZMztlC9bn1uADBwA9nQHNXEWrCagS h9IO677QxufM0WuWKlH6/u1ENuqqH8QSugpD9kpfn/djGpy/XlIdKs5a+vd9VhVTYvd0 otJQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@canb.auug.org.au header.s=201702 header.b=KvTHyXDt; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=canb.auug.org.au Received: from groat.vger.email (groat.vger.email. [23.128.96.35]) by mx.google.com with ESMTPS id d189-20020a6336c6000000b0054fbd904b6dsi4264043pga.500.2023.10.29.19.48.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 29 Oct 2023 19:48:37 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35; Authentication-Results: mx.google.com; dkim=pass header.i=@canb.auug.org.au header.s=201702 header.b=KvTHyXDt; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=canb.auug.org.au Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id 2AA1C8096FE0; Sun, 29 Oct 2023 19:48:35 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231150AbjJ3CsP (ORCPT <rfc822;zxc52fgh@gmail.com> + 31 others); Sun, 29 Oct 2023 22:48:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59904 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229510AbjJ3CsO (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Sun, 29 Oct 2023 22:48:14 -0400 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D5926BD; Sun, 29 Oct 2023 19:48:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canb.auug.org.au; s=201702; t=1698634088; bh=2WPF2meFlfmuU5Z/7Pvyd5wbBkWFy41KCxzeBwEo1tQ=; h=Date:From:To:Cc:Subject:From; b=KvTHyXDt+80+z/tTMTSfjcnv702x1HfYRbMx2blLRG6T+/7nvBtp9eQqEC7NSiDFO u4uZHG2hWK5hESMoR1CIIl9tc69mwQuL+kNi2IQtW+oJjl4rJj+BFPRJTlJ2IJFJW9 rVMAw5mh2iwJ1Czpb7ck+YfyECPPnN9flZv5knGhxuAS2EElkxP6Sl3O5cYh/JySYk LY8LnGYNEIDS3tiY+QRmSkSkBE9dJITOBuxDFe0mcZYG1EchtJUozE0wvsd4+Hpumh hQ9jj3llfAwwZMX404T8c8392CXHF8AxvQpGUefEkVdVckbWuCsVSTuU8BKSycMjQ9 ZebTPLF5CsMrA== Received: from authenticated.ozlabs.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail.ozlabs.org (Postfix) with ESMTPSA id 4SJd474M69z4wd6; Mon, 30 Oct 2023 13:48:07 +1100 (AEDT) Date: Mon, 30 Oct 2023 13:48:06 +1100 From: Stephen Rothwell <sfr@canb.auug.org.au> To: Sean Christopherson <seanjc@google.com>, Christian Brauner <brauner@kernel.org> Cc: Ackerley Tng <ackerleytng@google.com>, Chao Peng <chao.p.peng@linux.intel.com>, Isaku Yamahata <isaku.yamahata@intel.com>, "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, Michael Roth <michael.roth@amd.com>, Paolo Bonzini <pbonzini@redhat.com>, Yu Zhang <yu.c.zhang@linux.intel.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Linux Next Mailing List <linux-next@vger.kernel.org> Subject: linux-next: build failure after merge of the kvm-x86 tree Message-ID: <20231030134806.24510492@canb.auug.org.au> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="Sig_/=izap_lC71SbQQAr.14+Lg7"; protocol="application/pgp-signature"; micalg=pgp-sha256 X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Sun, 29 Oct 2023 19:48:35 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1781146968685571498 X-GMAIL-MSGID: 1781146968685571498 |
Series |
linux-next: build failure after merge of the kvm-x86 tree
|
|
Commit Message
Stephen Rothwell
Oct. 30, 2023, 2:48 a.m. UTC
Hi all, After merging the kvm-x86 tree, today's linux-next build (x86_64 allmodconfig) failed like this: arch/x86/kvm/../../../virt/kvm/guest_memfd.c: In function 'kvm_gmem_get_file': arch/x86/kvm/../../../virt/kvm/guest_memfd.c:280:35: error: passing argument 1 of 'get_file_rcu' from incompatible pointer type [-Werror=incompatible-pointer-types] 280 | if (file && !get_file_rcu(file)) | ^~~~ | | | struct file * In file included from include/linux/backing-dev.h:13, from arch/x86/kvm/../../../virt/kvm/guest_memfd.c:2: include/linux/fs.h:1046:47: note: expected 'struct file **' but argument is of type 'struct file *' 1046 | struct file *get_file_rcu(struct file __rcu **f); | ~~~~~~~~~~~~~~~~~~~~^ Caused by commit fcbef1e5e5d2 ("KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory") interacting with commit 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") from the vfs-brauner tree. I have applied the following merg resolution patch: From: Stephen Rothwell <sfr@canb.auug.org.au> Date: Mon, 30 Oct 2023 13:35:38 +1100 Subject: [PATCH] fix up for "KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory" interacting with "file: convert to SLAB_TYPESAFE_BY_RCU" Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- virt/kvm/guest_memfd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Comments
On Mon, Oct 30, 2023 at 01:48:06PM +1100, Stephen Rothwell wrote: > Hi all, > > After merging the kvm-x86 tree, today's linux-next build (x86_64 > allmodconfig) failed like this: > > arch/x86/kvm/../../../virt/kvm/guest_memfd.c: In function 'kvm_gmem_get_file': > arch/x86/kvm/../../../virt/kvm/guest_memfd.c:280:35: error: passing argument 1 of 'get_file_rcu' from incompatible pointer type [-Werror=incompatible-pointer-types] > 280 | if (file && !get_file_rcu(file)) > | ^~~~ > | | > | struct file * > In file included from include/linux/backing-dev.h:13, > from arch/x86/kvm/../../../virt/kvm/guest_memfd.c:2: > include/linux/fs.h:1046:47: note: expected 'struct file **' but argument is of type 'struct file *' > 1046 | struct file *get_file_rcu(struct file __rcu **f); > | ~~~~~~~~~~~~~~~~~~~~^ > > Caused by commit > > fcbef1e5e5d2 ("KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for guest-specific backing memory") > > interacting with commit > > 0ede61d8589c ("file: convert to SLAB_TYPESAFE_BY_RCU") > > from the vfs-brauner tree. > > I have applied the following merg resolution patch: > > From: Stephen Rothwell <sfr@canb.auug.org.au> > Date: Mon, 30 Oct 2023 13:35:38 +1100 > Subject: [PATCH] fix up for "KVM: Add KVM_CREATE_GUEST_MEMFD ioctl() for > guest-specific backing memory" > > interacting with "file: convert to SLAB_TYPESAFE_BY_RCU" > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> > --- > virt/kvm/guest_memfd.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > index 94bc478c26f3..7f62abe3df9e 100644 > --- a/virt/kvm/guest_memfd.c > +++ b/virt/kvm/guest_memfd.c > @@ -276,9 +276,7 @@ static struct file *kvm_gmem_get_file(struct kvm_memory_slot *slot) > > rcu_read_lock(); > > - file = rcu_dereference(slot->gmem.file); > - if (file && !get_file_rcu(file)) > - file = NULL; > + file = get_file_rcu(&slot->gmem.file); > > rcu_read_unlock(); Stephen, thanks for the suggested fixup. Afaict, the guest_memfds pin the kvm instance. If a guest_memfd is closed and last fput() is called the file pointer remains stashed in all relevant gmem->file slots until guest_memfd->release::kvm_gmem_release() is called. And kvm_gmem_release() sets all relevant gmem->file instances to NULL via rcu_assign_pointer(). So afaict, the gmem->file pointer isn't part of the reference counting but rather it just caches the result of the reference counting. And it's then cleared when that reference goes down to zero. Basically, I think this is akin to commit 61d4fb0b349e ("file, i915: fix file reference for mmap_singleton()") which is in -next and the discussion we had in https://lore.kernel.org/r/20231025-formfrage-watscheln-84526cd3bd7d@brauner So that means we can't to loop here which is what get_file_rcu() would do. Otherwise you might end up spinning. Because last close of guest_memfd fput() puts the last reference. Now all concurrent gmem->file kvm_gmem_get_file() callers will see f_count at zero. And it might well take a while until the kernel calls guest_memfd->release::kvm_gmem_release() and NULLs the pointer. So get_file_rcu() would retry and loop because it thinks that the pointer is part of the reference counting. So iiuc you want get_file_active() here which also takes the rcu_read_lock() for you. @Paolo and @Sean, does that make sense and is the series for v6.7 or just already in -next for v6.8? diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 94bc478c26f3..a4c194b0b22c 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -272,17 +272,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file) static struct file *kvm_gmem_get_file(struct kvm_memory_slot *slot) { - struct file *file; - - rcu_read_lock(); - - file = rcu_dereference(slot->gmem.file); - if (file && !get_file_rcu(file)) - file = NULL; - - rcu_read_unlock(); - - return file; + return get_file_active(&slot->gmem.file); } static struct file_operations kvm_gmem_fops = {
On 10/30/23 11:05, Christian Brauner wrote: > > @Paolo and @Sean, does that make sense and is the series for v6.7 or > just already in -next for v6.8? It's for 6.8. Thanks for the fixup! Paolo > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c > index 94bc478c26f3..a4c194b0b22c 100644 > --- a/virt/kvm/guest_memfd.c > +++ b/virt/kvm/guest_memfd.c > @@ -272,17 +272,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file) > > static struct file *kvm_gmem_get_file(struct kvm_memory_slot *slot) > { > - struct file *file; > - > - rcu_read_lock(); > - > - file = rcu_dereference(slot->gmem.file); > - if (file && !get_file_rcu(file)) > - file = NULL; > - > - rcu_read_unlock(); > - > - return file; > + return get_file_active(&slot->gmem.file); > } > > static struct file_operations kvm_gmem_fops = { >
Hi Paolo, On Mon, 30 Oct 2023 12:27:07 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 10/30/23 11:05, Christian Brauner wrote: > > > > @Paolo and @Sean, does that make sense and is the series for v6.7 or > > just already in -next for v6.8? > > It's for 6.8. Then it should not be in linux-next yet. :-(
On Tue, Oct 31, 2023, Stephen Rothwell wrote: > Hi Paolo, > > On Mon, 30 Oct 2023 12:27:07 +0100 Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 10/30/23 11:05, Christian Brauner wrote: > > > > > > @Paolo and @Sean, does that make sense and is the series for v6.7 or > > > just already in -next for v6.8? > > > > It's for 6.8. > > Then it should not be in linux-next yet. :-( That's my bad, I wanted to get the guest_memfd code exposure asap and jumped the gun. I'll yank the branch out kvm-x86/next. I assume -rc1 is when the floodgates "officially" open again?
Hi Sean, On Mon, 30 Oct 2023 14:05:12 -0700 Sean Christopherson <seanjc@google.com> wrote: > > That's my bad, I wanted to get the guest_memfd code exposure asap and jumped the > gun. I'll yank the branch out kvm-x86/next. Thanks. > I assume -rc1 is when the floodgates "officially" open again? Yes, indeed.
On Mon, Oct 30, 2023 at 10:10 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > Hi Sean, > > On Mon, 30 Oct 2023 14:05:12 -0700 Sean Christopherson <seanjc@google.com> wrote: > > > > That's my bad, I wanted to get the guest_memfd code exposure asap and jumped the > > gun. I'll yank the branch out kvm-x86/next. > > Thanks. In all fairness, it was only recently pushed back from 6.7 to 6.8 so it probably would have made sense to include it _even earlier_. But I agree that at this point it's better if we wait for 6.7-rc1 before it makes it back into linux-next. At that point I'll include the conflict resolution myself. Paolo
diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c index 94bc478c26f3..7f62abe3df9e 100644 --- a/virt/kvm/guest_memfd.c +++ b/virt/kvm/guest_memfd.c @@ -276,9 +276,7 @@ static struct file *kvm_gmem_get_file(struct kvm_memory_slot *slot) rcu_read_lock(); - file = rcu_dereference(slot->gmem.file); - if (file && !get_file_rcu(file)) - file = NULL; + file = get_file_rcu(&slot->gmem.file); rcu_read_unlock();