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 90D4CC282DE for ; Mon, 10 Mar 2025 19:57:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0C5F0280005; Mon, 10 Mar 2025 15:57:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 074E8280003; Mon, 10 Mar 2025 15:57:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E3EA3280005; Mon, 10 Mar 2025 15:57:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id C35DF280003 for ; Mon, 10 Mar 2025 15:57:19 -0400 (EDT) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id CB8E4814E3 for ; Mon, 10 Mar 2025 19:57:20 +0000 (UTC) X-FDA: 83206700640.01.B16708E Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf09.hostedemail.com (Postfix) with ESMTP id 55623140003 for ; Mon, 10 Mar 2025 19:57:18 +0000 (UTC) Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=V7MV2gdN; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf09.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741636638; a=rsa-sha256; cv=none; b=AoIDH0+/wRNvdaPVbcWzPR7SLq/zhC3xxbe/dKVqeJlY9xgmddo0p6bLcum+xJXTSmg46b RE3VBx7u6GC6YSFKp9iGH7jJ/aU3LjPCpSgy41weDn6zfut7AcqQq+FcksZwB7P3A+FUa6 5pY5qrzMDTML/XMGE2zC2yOlcl1Y/rw= ARC-Authentication-Results: i=1; imf09.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=V7MV2gdN; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf09.hostedemail.com: domain of peterx@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=peterx@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741636638; 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=K8xyWG42ZuOIaLO2r4meAYJugvJJCVkcbSBl0v7IQsw=; b=bK+sajvhiYQWu6Vt9NaupeJRcPStA01zfjQ/pItG9YYBekT94WKRG0EZGGl3+rL57shQUA wJOsb8vMvVQ1HXcfG7kQlwFtfHdxZ4KQDEdsYJUPlXTK53cTCPlnd/xnAlI5j9jXtaUrDr 4PcM/vWwPgCOU+nKF8DzSkblup42hr4= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1741636637; 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: in-reply-to:in-reply-to:references:references; bh=K8xyWG42ZuOIaLO2r4meAYJugvJJCVkcbSBl0v7IQsw=; b=V7MV2gdNqKkvywcMZmsK3H/g+a3xqNF+aaFpE1mwEad1sRZXOsfnv3Rzig2LRq+Hpthal1 wXJrn0JNnK+jvjpOt9nI9HqiEtq5k2z9uaubb8hQLRBKQ8T0Fi22q4+U/VKoU3umogE/MW 8MhAYf54qrDHDwizMsZi34hOtqPZlOg= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-70-kZ52osLQNHKOvaJxy_-ERw-1; Mon, 10 Mar 2025 15:57:16 -0400 X-MC-Unique: kZ52osLQNHKOvaJxy_-ERw-1 X-Mimecast-MFC-AGG-ID: kZ52osLQNHKOvaJxy_-ERw_1741636636 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-4768f9fea35so31703801cf.2 for ; Mon, 10 Mar 2025 12:57:16 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741636636; x=1742241436; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=K8xyWG42ZuOIaLO2r4meAYJugvJJCVkcbSBl0v7IQsw=; b=kfekcoC7t7QFTgSqOeB9y7rVo5kcUOZDuYyqYIpzad6sirRtHnyrV77Ho7lxFX0w+v kuYMzoyJgXpOzzS6egSkd+AJH9aQ6IHCmVTZ5XNXIcx9DzbbfK8C+cj8vK5Koe7TO6dW 6j/jWFZ/jzN6z72bfIbvE/Hw8GTOn/yRr2DdcX186f+qPoHY1jiH93XGMzGcfWT0X/sM 0hxLwoAgsWi6oHMPIaY2DFH0T918qUdkr1mXN7Y07cC+xXQEiEysBJF+nzCJRGNZ6Ihv CcXoNR524xSxxOun7ZFi1ybDlHd/BkPJrMQlYwF3BrPlchav502aeEAN//cS9UfKMtA6 cRww== X-Forwarded-Encrypted: i=1; AJvYcCX/3OIPB3xy8MRDe05kI2vd+gtcePZnIg4QZkadyFfSvOXep8qezwSEMluxs20YvpSQO1dyBNxRPg==@kvack.org X-Gm-Message-State: AOJu0Yyq9Fk3nfiCIJIgY3nmplyuDrijvVgChlBfGDbGuF5sukcdna5i kLU6GWcICgCVfw+kn48n4yTZWx9G493Eb70yganaWjb338fe5qZs4Pa1kqieR8rVjJLKlfs/ZwN P3uRqpvBIOMRu0ZuuEHyTiRBisjqCTGcb3sDKJUIzAzBem+Bg X-Gm-Gg: ASbGnctBmKoCrFfnCFAUmQTY4l1CBO+bDfQxJTJCQsdnKshiam8e1PKIlAKYAYYTrN5 a0qUJ9INIMLT/ApeaLv63bwIPvPedsa7OsuDga9gWOo6oqlryuqqBa+57PCaJdxs+INuIpyfbqd gBTXD8PYfNdn5CZ9COvPuNg89LBFAlZag5fQSkhKI2h97V5/o5ZEN6NgtUdi/6HgBYymyFV/xTg Nu/rzLQq0H2M8TVrXAaTeIFbbmZQfa+x9N/2m+yJGxit8bvWDuT0fCdHw86qYWAVfivZ/1sDvDa EKUqT2c= X-Received: by 2002:ac8:5745:0:b0:476:74de:81e2 with SMTP id d75a77b69052e-47674de857emr144524561cf.43.1741636636066; Mon, 10 Mar 2025 12:57:16 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEJaP7gBJHUVmVF8NuJDRjjC2wD/aDoV/tl7RcR7xOw/jV+Wi87S1Vt6HJV54rbfWOqlAyd6g== X-Received: by 2002:ac8:5745:0:b0:476:74de:81e2 with SMTP id d75a77b69052e-47674de857emr144523981cf.43.1741636635560; Mon, 10 Mar 2025 12:57:15 -0700 (PDT) Received: from x1.local ([85.131.185.92]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-476839ea30csm22806571cf.55.2025.03.10.12.57.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 10 Mar 2025 12:57:14 -0700 (PDT) Date: Mon, 10 Mar 2025 15:57:09 -0400 From: Peter Xu To: Nikita Kalyazin Cc: James Houghton , akpm@linux-foundation.org, pbonzini@redhat.com, shuah@kernel.org, kvm@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, lorenzo.stoakes@oracle.com, david@redhat.com, ryan.roberts@arm.com, quic_eberman@quicinc.com, graf@amazon.de, jgowans@amazon.com, roypat@amazon.co.uk, derekmn@amazon.com, nsaenz@amazon.es, xmarcalx@amazon.com Subject: Re: [RFC PATCH 0/5] KVM: guest_memfd: support for uffd missing Message-ID: References: <20250303133011.44095-1-kalyazin@amazon.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: E3tp6bVOY6PRk2tblLxqHxpaogMovdOqsEE0hoX-TgA_1741636636 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspamd-Queue-Id: 55623140003 X-Stat-Signature: jwpyfbqiy1xu4ipo1776z5kbs4wyexii X-Rspamd-Server: rspam02 X-Rspam-User: X-HE-Tag: 1741636638-842144 X-HE-Meta: U2FsdGVkX1+pmpbJjwXjkr30Pb9Sca0LE5fGGUMik3ZuQgVS2KhAe2DFYrGHLFlE0ZSp21W+fCClRhevcFmqcfnbcCHFaXeDzaYJ4q0lQzt54MC1KVQCCv8nJMDktmJCJRjXmsPFcN+kXNLKsoCoPTZmencr28nI73B7BIDna6Jz8YtoZRFSmUsOWggE760S8oObC/scZus75bvPE44YA+fUhwzXHeyojg+2u5U4YgsW+jqauI8141Yk1qp0yG7fvSsdPSvVRMh4SwM6hfpKC3lzZ0Nac4HdLnnMMkELkih5jzzRuwQu9jOR6yLwxmOq9iI7cQ7zhuZ7wZnH+X/OspGFdajNklWJxhX0WbuDYfT2lxOBmnwXOBPS/HQ+yHQ9nVujlYUEbfXcqetrlhG7VhP1RCLl6PoV/swNY1/S8SzypF2ge3X5HJ7UdzVc3xcyRkFkEiIQhh7NxTUyufFw2EzJaEx4nKTqSMky1uUp59XCEXTMZPmFm/bd3fXALrYC/QQ/2sPZG+Qxz2azLS/lHsO4c+18EGwozYrglQgcROVhU34HYKfSl6Pdnsf3+CRAEMfS+Qnlgtjx0dPj2yJcvH5K0jk2NzqV1Uhf8KzslE1CfLcffc1FeOtobb0MIOHglE+P3WXukxqyr70mOFfI05PZFqUuWLrxfqCH3+u3CE2++fxf94UrQumImlRfGstijkhlfIXLKMu4h+ns9UJFCvTpuiO41KN9KAfC8qXUt99I2F6Ki+I5/y2hLkB8zB3LNX7+TaqcaCrVwkWlNslXHlu6Y9e3KmUJyk20/eeFsRpPwwJ/p6L9j3bKkdk28DWoYeb0WtHGBBpA/h02XxmInR2Lc4/wyy/99aion2XhwIY2MkHm/p/Fy7DE8AWrnxaJUOyqHJADcv7jzbwzqdQC0Zhdlr0hayb9KaanOUSbavabMi9sBgEmh4XL/2lAA1m/IztryUezeE8ezobLuZE Wp0fFYis jsInlHmZ7qPXVGQdtPJUSqbhJ8vuUv4pLPnOnovJz/XsuFa3CDWYgjxnM0ejE2OTZQnERhKaOJMg8zaH4InWWCXjlDbEJadF/OsRuQMwZ7Yb20xYeOjnbdBSiozIhr87MB2eWES1cyIHYUO0oqtK8oq4ujPLl0iRwSwSCIARDf9le8ckytUtDH2n2SIP/Xd7YnMMcSa0GAvQfVz6bcL3aIIg52t2S8Oyo1FQSADfZSviSZ+oE+aUI4OJLjNDhF4xppXDu2qW61+4nGMJEXI8G+EhPU4WegeoXm3V6TbzRtwhTo3EnUfmmn1IpL2CW1YPSi/84YlBS04QQe4ZQTUm/UEj78AKychdZA+Exn90Qh1uyf+n5k8g6zEXCqDzyF7O85ggU X-Bogosity: Ham, tests=bogofilter, spamicity=0.000001, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Mar 10, 2025 at 06:12:22PM +0000, Nikita Kalyazin wrote: > > > On 05/03/2025 20:29, Peter Xu wrote: > > On Wed, Mar 05, 2025 at 11:35:27AM -0800, James Houghton wrote: > > > I think it might be useful to implement an fs-generic MINOR mode. The > > > fault handler is already easy enough to do generically (though it > > > would become more difficult to determine if the "MINOR" fault is > > > actually a MISSING fault, but at least for my userspace, the > > > distinction isn't important. :)) So the question becomes: what should > > > UFFDIO_CONTINUE look like? > > > > > > And I think it would be nice if UFFDIO_CONTINUE just called > > > vm_ops->fault() to get the page we want to map and then mapped it, > > > instead of having shmem-specific and hugetlb-specific versions (though > > > maybe we need to keep the hugetlb specialization...). That would avoid > > > putting kvm/gmem/etc. symbols in mm/userfaultfd code. > > > > > > I've actually wanted to do this for a while but haven't had a good > > > reason to pursue it. I wonder if it can be done in a > > > backwards-compatible fashion... > > > > Yes I also thought about that. :) > > Hi Peter, hi James. Thanks for pointing at the race condition! > > I did some experimentation and it indeed looks possible to call > vm_ops->fault() from userfault_continue() to make it generic and decouple > from KVM, at least for non-hugetlb cases. One thing is we'd need to prevent > a recursive handle_userfault() invocation, which I believe can be solved by > adding a new VMF flag to ignore the userfault path when the fault handler is > called from userfault_continue(). I'm open to a more elegant solution > though. It sounds working to me. Adding fault flag can also be seen as part of extension of vm_operations_struct ops. So we could consider reusing fault() API indeed. > > Regarding usage of the MINOR notification, in what case do you recommend > sending it? If following the logic implemented in shmem and hugetlb, ie if > the page is _present_ in the pagecache, I can't see how it is going to work It could be confusing when reading that chunk of code, because it looks like it notifies minor fault when cache hit. But the critical part here is that we rely on the pgtable missing causing the fault() to trigger first. So it's more like "cache hit && pgtable missing" for minor fault. > with the write syscall, as we'd like to know when the page is _missing_ in > order to respond with the population via the write. If going against > shmem/hugetlb logic, and sending the MINOR event when the page is missing > from the pagecache, how would it solve the race condition problem? Should be easier we stick with mmap() rather than write(). E.g. for shmem case of current code base: if (folio && vma && userfaultfd_minor(vma)) { if (!xa_is_value(folio)) folio_put(folio); *fault_type = handle_userfault(vmf, VM_UFFD_MINOR); return 0; } vma is only availble if vmf!=NULL, aka in fault context. With that, in write() to shmem inodes, nothing will generate a message, because minor fault so far is only about pgtable missing. It needs to be mmap()ed first, and has nothing yet to do with write() syscalls. > > Also, where would the check for the folio_test_uptodate() mentioned by James > fit into here? Would it only be used for fortifying the MINOR (present) > against the race? > > > When Axel added minor fault, it's not a major concern as it's the only fs > > that will consume the feature anyway in the do_fault() path - hugetlbfs has > > its own path to take care of.. even until now. > > > > And there's some valid points too if someone would argue to put it there > > especially on folio lock - do that in shmem.c can avoid taking folio lock > > when generating minor fault message. It might make some difference when > > the faults are heavy and when folio lock is frequently taken elsewhere too. > > Peter, could you expand on this? Are you referring to the following > (shmem_get_folio_gfp)? > > if (folio) { > folio_lock(folio); > > /* Has the folio been truncated or swapped out? */ > if (unlikely(folio->mapping != inode->i_mapping)) { > folio_unlock(folio); > folio_put(folio); > goto repeat; > } > if (sgp == SGP_WRITE) > folio_mark_accessed(folio); > if (folio_test_uptodate(folio)) > goto out; > /* fallocated folio */ > if (sgp != SGP_READ) > goto clear; > folio_unlock(folio); > folio_put(folio); > } > > Could you explain in what case the lock can be avoided? AFAIC, the function > is called by both the shmem fault handler and userfault_continue(). I think you meant the UFFDIO_CONTINUE side of things. I agree with you, we always need the folio lock. What I was saying is the trapping side, where the minor fault message can be generated without the folio lock now in case of shmem. It's about whether we could generalize the trapping side, so handle_mm_fault() can generate the minor fault message instead of by shmem.c. If the only concern is "referring to a module symbol from core mm", then indeed the trapping side should be less of a concern anyway, because the trapping side (when in the module codes) should always be able to reference mm functions. Actually.. if we have a fault() flag introduced above, maybe we can generalize the trap side altogether without the folio lock overhead. When the flag set, if we can always return the folio unlocked (as long as refcount held), then in UFFDIO_CONTINUE ioctl we can lock it. > > > It might boil down to how many more FSes would support minor fault, and > > whether we would care about such difference at last to shmem users. If gmem > > is the only one after existing ones, IIUC there's still option we implement > > it in gmem code. After all, I expect the change should be very under > > control (<20 LOCs?).. > > > > -- > > Peter Xu > > > -- Peter Xu