From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3D8CEC6FD18 for ; Thu, 20 Apr 2023 00:50:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 671B5280001; Wed, 19 Apr 2023 20:50:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 5FB4B6B0074; Wed, 19 Apr 2023 20:50:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 47428280001; Wed, 19 Apr 2023 20:50:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 326E56B0072 for ; Wed, 19 Apr 2023 20:50:00 -0400 (EDT) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id F08F4120490 for ; Thu, 20 Apr 2023 00:49:59 +0000 (UTC) X-FDA: 80699937318.25.DF3966B Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) by imf25.hostedemail.com (Postfix) with ESMTP id 34A21A0011 for ; Thu, 20 Apr 2023 00:49:57 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=0KQvDXHL; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of 3NYxAZAYKCHgoaWjfYckkcha.Ykihejqt-iigrWYg.knc@flex--seanjc.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=3NYxAZAYKCHgoaWjfYckkcha.Ykihejqt-iigrWYg.knc@flex--seanjc.bounces.google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681951798; h=from:from:sender: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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=WElRL1S1A/l6yYWkPjKb60TbLP43vB7Va3vAhnMHy4o=; b=OhkpaW0qiKb1P4bF5JGYgVX015C+ZpnuyCIjE5t21AGhq4Gf5Xf64iAK4vm+AqsysRLbYW 8L+1v41xwDBt/EQjtTNPZAAwlZaBVRrATwA/6BekAQndEhcPgaxyp0hmZ1jcXcMNOGhdeM eiuYhgqzbDyKQP844Rd0Vpom1DoePI4= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=0KQvDXHL; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf25.hostedemail.com: domain of 3NYxAZAYKCHgoaWjfYckkcha.Ykihejqt-iigrWYg.knc@flex--seanjc.bounces.google.com designates 209.85.128.201 as permitted sender) smtp.mailfrom=3NYxAZAYKCHgoaWjfYckkcha.Ykihejqt-iigrWYg.knc@flex--seanjc.bounces.google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681951798; a=rsa-sha256; cv=none; b=O3I+C1LZGfm9oAfMBl3ps0fUlzGdCovFeUbMfsg/C/ej9wZwVNtFL9UN75y/cnyKX/AYNC n5dw77ZVCjYJDp4bA5uPXC9ltmK42ntvrOCLSzSvaEL8gjh2KUpN4ir1TOjd7S8YtbCNUY DUqtxgtMOpFzKrNWZbMoS3cqBEUq6tk= Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-54fba72c1adso11018527b3.18 for ; Wed, 19 Apr 2023 17:49:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681951797; x=1684543797; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=WElRL1S1A/l6yYWkPjKb60TbLP43vB7Va3vAhnMHy4o=; b=0KQvDXHLR89BVxU2oI9O1ZusFid2YKHOcipULiVP6+PSDHwriZYqd2A06zVvzb+5zg +1pAM1zXlSNPZvINL7u8jaguPJCIxfMpOP+rF/bo3e7YipW5A5h9Z1uQ5Ne2aQDNcW6c EcQXFII0XVCoUfr19ADWbM/w1kzMojqRNFL4dpL0bA9tYUhTVMdjBXnH0ldjTdoS4rfS B4qULmS58/9Ub/zlvy0EPJx+IEG/RXiTWy5ry2WStJSaH5TR16lXIzvnkE2V+5Qqr+ic KCotEARF8eci5Ax95+I+ZNzK3NrfAcOPUHTnTLXEbEOQev6PRHk+rxhdkpcXFMAn0sKn YqLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681951797; x=1684543797; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=WElRL1S1A/l6yYWkPjKb60TbLP43vB7Va3vAhnMHy4o=; b=JMyuhMvXh8FuxtHyiuYJAb7X9Z1Yov2w2YWCE6NSOWsIJxpfNH86Of42CJoHZijJFY 7soXQQd/1oQFjGQFKvxfb192D4jP8slfAnac/+mkfmFfAWeSn5LGEZLVjHA0hPthcNcm 9lD0LFQqPrwfOtmC9lPA1BMbDBSEGpXXQyKIzh9MIM6MR7sR61SE1t+stwunFrRnNVjW ycvGTRxRf6Te4k69+20xSM3WxRn6zrnku0Cu2zj4CsxrkiZZB2kqqrpujCz2KYWuf1do gjnW7cpQYZj7n/SPnxjRbseXAdMMGk8GZnSU6CRkutF7qhzj8CPtspoh1TOe2zzpN2kj F9SA== X-Gm-Message-State: AAQBX9fpHqFzNfzPJ+35+C61GyPgc8UmFzsWtaGoJPpWVguKask05/39 l8gWbD7nnqIpUYO8EiPzywM/xgby8w0= X-Google-Smtp-Source: AKy350ZW3QSDcu1b4uay718+0UKdCoR5698g0B4gA8e9kbzHBPJ0H7w91zco9M9KgDo6hSqXOcwRuk93omk= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:d24c:0:b0:b95:460c:1776 with SMTP id j73-20020a25d24c000000b00b95460c1776mr766347ybg.13.1681951797267; Wed, 19 Apr 2023 17:49:57 -0700 (PDT) Date: Wed, 19 Apr 2023 17:49:55 -0700 In-Reply-To: <20230418-anfallen-irdisch-6993a61be10b@brauner> Mime-Version: 1.0 References: <20220818132421.6xmjqduempmxnnu2@box> <20221202061347.1070246-2-chao.p.peng@linux.intel.com> <20230413-anlegen-ergibt-cbefffe0b3de@brauner> <20230418-anfallen-irdisch-6993a61be10b@brauner> Message-ID: Subject: Re: [PATCH v7 00/14] KVM: mm: fd-based approach for supporting KVM guest private memory From: Sean Christopherson To: Christian Brauner Cc: "Kirill A . Shutemov" , Ackerley Tng , Chao Peng , Hugh Dickins , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org, linux-doc@vger.kernel.org, qemu-devel@nongnu.org, linux-kselftest@vger.kernel.org, Paolo Bonzini , Jonathan Corbet , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , Thomas Gleixner , Ingo Molnar , Borislav Petkov , x86@kernel.org, "H . Peter Anvin" , Jeff Layton , "J . Bruce Fields" , Andrew Morton , Shuah Khan , Mike Rapoport , Steven Price , "Maciej S . Szmigiero" , Vlastimil Babka , Vishal Annapurve , Yu Zhang , luto@kernel.org, jun.nakajima@intel.com, dave.hansen@intel.com, ak@linux.intel.com, david@redhat.com, aarcange@redhat.com, ddutile@redhat.com, dhildenb@redhat.com, Quentin Perret , Michael Roth , mhocko@suse.com, Muchun Song , Pankaj Gupta , linux-arch@vger.kernel.org, arnd@arndb.de, linmiaohe@huawei.com, naoya.horiguchi@nec.com, tabba@google.com, wei.w.wang@intel.com Content-Type: text/plain; charset="us-ascii" X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 34A21A0011 X-Stat-Signature: q5w7cfw5e6fqq6ufmfowfnbdxrmtaos1 X-HE-Tag: 1681951797-940131 X-HE-Meta: U2FsdGVkX1/Hhs+mUSKz5yQdqJ3lvVCNGUcqGhP8FzPEcDQ42ERRMciGX2PDkTyU8/vZmVEdyf1BRaUkzWWfEz/0WxDjLh0yU9FjP4a87atX2IiVtg7okEIXqCPp1UdKPiCqy2jlUXdaEpWQgyx/DzOAXgX7c3uhHADLADCM6NcR1zu3LF0QjpY0z16Na/grzLSgeSfh4u6tjDnjR81UdbK/smop/phdhIgLDCsMlsJ3EDUIklB6WF9/7HojBhquO1TXsjIs4AzA4FXw72z7iyYrhohDrZduhIFBNt8nF89VeH8KpygOYRMA3vrGto0pgFSBBBw09vqPaRueoO25k0cCQgB1oGBe9RvmSxus8wgGWVUNOL8GqOi5vqFAnw8VuMDrsNT6Sqx/aNQUQ2y4DqLHxF0f7nasc8TvHVjLal3vSMs2l1wH3cHTaO0qK7P1mfGGRXJEN+kJUGGvMoRKguQ1l18ARbXz1yn3NMsu6SFvRQGYLTfzy+JLaSO9DpG6m/gdeTX+ZXwyhAg+6AJNp9LRHH6w04m/h0+7+EfxzncUJBAuI9kjCK7losmzWe3mddqvDlpLFSVTFOgccSMeww8+50dVhKEQByYIlIB0JFj2sxH6k5mR7rlfXcNZwi/LKc3WUf7iGYmryTFSuVehJoRo28xUxHbpupjYSAfWevhH+Ii3PTnXI4BZBNu4jkaH3+GGy82tLCyljWHLRBHUg2+2v7xyc/R0SR7hJoDN7m/vdi0ijLbDSID29NZ+g+docebgRMiKR7Go6u7BnRBZFRgXOgoDIjPU13PanzV4+Vx1D6V2Er2LOgMlNoruefWfOsV3bo0j/iHJTyQvhVvw6n+u5BI/4HdpFciUADpa2QHqjzTl5ATyGTNv+ghXKY0LNcZSBiZZW9/tt5YNF0ySwSxo5EA8Ja+TyzRjnLy05AqZPjDAnTWkMDXgFrpZZEqj0B4LSvWJf6uVbBPmaFJ rU0OYP6j ZhyNRcdi71oXclxAao+bDSzL1+Ok4ZKigVN4U6Dp+7V6yfGQeJjXG8gcG2TRKcxrAgZ+Ybiwp5UnmAjnpDrpDSE3LH9VSV+yvCqvq2x+IlXEL9fQID9VcYOksE3y+57ZHUriwjM2mxQ1iAj98ccW0a47t3t6OKig7omq7SNK3YVolrE8xfjs3484fbttTdcw6ho8w13cp0/qbqpwykCzYQwT/BXkk9zmqkowSRoTXohWLasBCjMmh838C9/yE8QiJTlkOk2tITUWxWpaZlENvUeARcaB/lKzQfA+RsPRKQcD8cBPJXCaWVTgHROZDJI8tpdCeKw2zM8t7WMDjxrOjAjW/usLDAjTaQG5NUFmAKqn7r2VzFtGRA0j108LjzOFeCYhG6Zr6EO5JoGKOkjqiIOrn6RxeVR/yTk7CckytWl+q1xuAaz4SkkPnAoC3mSpWsekYx995hnbVqf8tVwd8WzMymb9fv8NWPULDXiL/SPbXa8HGWGNOK43NYjaotfJ+ymbs1/9S3Z7KTEGvNbeX5tryUR3MM5jVakLTvTTRcOxtDGvAKc6+xWV/EO8V65NpegpLj1uyVLPKtY8DcHiQevROG6sLU+/wEl5L X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Wed, Apr 19, 2023, Christian Brauner wrote: > On Thu, Apr 13, 2023 at 03:28:43PM -0700, Sean Christopherson wrote: > > > But if you want to preserve the inode number and device number of the > > > relevant tmpfs instance but still report memfd restricted as your > > > filesystem type > > > > Unless I missed something along the way, reporting memfd_restricted as a distinct > > filesystem is very much a non-goal. AFAIK it's purely a side effect of the > > proposed implementation. > > In the current implementation you would have to put in effort to fake > this. For example, you would need to also implement ->statfs > super_operation where you'd need to fill in the details of the tmpfs > instance. At that point all that memfd_restricted fs code that you've > written is nothing but deadweight, I would reckon. After digging a bit, I suspect the main reason Kirill implemented an overlay to inode_operations was to prevent modifying the file size via ->setattr(). Relying on shmem_setattr() to unmap entries in KVM's MMU wouldn't work because, by design, the memory can't be mmap()'d into host userspace. if (attr->ia_valid & ATTR_SIZE) { if (memfd->f_inode->i_size) return -EPERM; if (!PAGE_ALIGNED(attr->ia_size)) return -EINVAL; } But I think we can solve this particular problem by using F_SEAL_{GROW,SHRINK} or SHMEM_LONGPIN. For a variety of reasons, I'm leaning more and more toward making this a KVM ioctl() instead of a dedicated syscall, at which point we can be both more flexible and more draconian, e.g. let userspace provide the file size at the time of creation, but make the size immutable, at least by default. > > After giving myself a bit of a crash course in file systems, would something like > > the below have any chance of (a) working, (b) getting merged, and (c) being > > maintainable? > > > > The idea is similar to a stacking filesystem, but instead of stacking, restrictedmem > > hijacks a f_ops and a_ops to create a lightweight shim around tmpfs. There are > > undoubtedly issues and edge cases, I'm just looking for a quick "yes, this might > > be doable" or a "no, that's absolutely bonkers, don't try it". > > Maybe, but I think it's weird. Yeah, agreed. > _Replacing_ f_ops isn't something that's unprecedented. It happens everytime > a character device is opened (see fs/char_dev.c:chrdev_open()). And debugfs > does a similar (much more involved) thing where it replaces it's proxy f_ops > with the relevant subsystem's f_ops. The difference is that in both cases the > replace happens at ->open() time; and the replace is done once. Afterwards > only the newly added f_ops are relevant. > > In your case you'd be keeping two sets of {f,a}_ops; one usable by > userspace and another only usable by in-kernel consumers. And there are > some concerns (non-exhaustive list), I think: > > * {f,a}_ops weren't designed for this. IOW, one set of {f,a}_ops is > authoritative per @file and it is left to the individual subsystems to > maintain driver specific ops (see the sunrpc stuff or sockets). > * lifetime management for the two sets of {f,a}_ops: If the ops belong > to a module then you need to make sure that the module can't get > unloaded while you're using the fops. Might not be a concern in this > case. Ah, whereas I assume the owner of inode_operations is pinned by ??? (dentry?) holding a reference to the inode? > * brittleness: Not all f_ops for example deal with userspace > functionality some deal with cleanup when the file is closed like > ->release(). So it's delicate to override that functionality with > custom f_ops. Restricted memfds could easily forget to cleanup > resources. > * Potential for confusion why there's two sets of {f,a}_ops. > * f_ops specifically are generic across a vast amount of consumers and > are subject to change. If memfd_restricted() has specific requirements > because of this weird double-use they won't be taken into account. > > I find this hard to navigate tbh and it feels like taking a shortcut to > avoid building a proper api. Agreed. At the very least, it would be better to take an explicit dependency on whatever APIs are being used instead of somewhat blindly bouncing through ->fallocate(). I think that gives us a clearer path to getting something merged too, as we'll need Acks on making specific functions visible, i.e. will give MM maintainers something concrete to react too. > If you only care about a specific set of operations specific to memfd > restricte that needs to be available to in-kernel consumers, I wonder if you > shouldn't just go one step further then your proposal below and build a > dedicated minimal ops api. This is actually very doable for shmem. Unless I'm missing something, because our use case doesn't allow mmap(), swap, or migration, a good chunk of shmem_fallocate() is simply irrelevant. The result is only ~100 lines of code, and quite straightforward. My biggest concern, outside of missing a detail in shmem, is adding support for HugeTLBFS, which is likely going to be requested/needed sooner than later. At a glance, hugetlbfs_fallocate() is quite a bit more complex, i.e. not something I'm keen to duplicate. But that's also a future problem to some extent, as it's purely kernel internals; the uAPI side of things doesn't seem like it'll be messy at all. Thanks again!