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 D43C5ECAAA1 for ; Mon, 24 Oct 2022 18:19:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 48D6F940009; Mon, 24 Oct 2022 14:19:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 43E50940007; Mon, 24 Oct 2022 14:19:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 306E6940009; Mon, 24 Oct 2022 14:19:07 -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 22D7B940007 for ; Mon, 24 Oct 2022 14:19:07 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id A3DF8140827 for ; Mon, 24 Oct 2022 18:19:06 +0000 (UTC) X-FDA: 80056654692.08.3B37E01 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf26.hostedemail.com (Postfix) with ESMTP id C7D66140036 for ; Mon, 24 Oct 2022 18:19:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1666635545; 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=N8Yvakt3MK7rD0y3RReVeclZNu+K0N6huBCXY+hLsjg=; b=i+6doAyuSqx7IlEgCx6kk/RlCcJ3LiBH0EH1VmpXLieVpE//ihRslARIs2HfMsHIM8wodu JtLS91/5rA8+RiJR9fYcoxCVw/FjatzfmK30LmauYWgsFvKoT/aHtLSuuXmvTaVX0/CXn2 6Ydtu6SOwUaAbKq5xNKztHTvEMsJbGo= Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-651-ijHFdD7DOBKIDpTE9eTKAA-1; Mon, 24 Oct 2022 14:19:04 -0400 X-MC-Unique: ijHFdD7DOBKIDpTE9eTKAA-1 Received: by mail-qk1-f199.google.com with SMTP id v1-20020a05620a440100b006eee30cb799so9703608qkp.23 for ; Mon, 24 Oct 2022 11:19:03 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=N8Yvakt3MK7rD0y3RReVeclZNu+K0N6huBCXY+hLsjg=; b=f5afeCQh3W+oV7xXttc/F7OkRWrqWq1oriu9jCYHo+YiJCCBkshaJgfjFg93zWx/S5 XJec8M5EKpJLeN5wzNb68So8i7Z7DGDTc43sbvE7QhlJxByCEVYtPU2QRAnofkO4lNpA RxuQhfg/Q7hY99uyt9PxEacvd9PKBWHsv5Nwo7ao6eMWGO0/bVYFZXvZ+kz0vlZyBdQb Fkoy7D5AOhCY1E0H78hhfFZ0L5p51HnFvgg2qdicBwofo5cIVd/0xyB5Z8xCmzyhS6B8 +Wt9TPz3KWuAJK1Ijr6yvHBkyO3H8Jue4qlnkGbmL7n+pojQzn1fTD66LF7Q/XEK6VTK /NXQ== X-Gm-Message-State: ACrzQf2qwZ+u2cBtiC1kY6/Dn7aMnQjfXpkrjFwku6n15UOdGFbQJwIS MUeWxWpvpHv5BmDvCjhVc7zR2tKXTkAs1svq2QWvfEeC/V0sJuY9k+Ry9NetszFS3HVkEjFFdaq GLfDPsAwSX4c= X-Received: by 2002:ac8:59c2:0:b0:39c:e3b4:1265 with SMTP id f2-20020ac859c2000000b0039ce3b41265mr27651371qtf.55.1666635543549; Mon, 24 Oct 2022 11:19:03 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5sUMaA/yGMGt9tvsXee0XTNi9PnrHPUDHcbhB8nWX8VOHAmdnt88LQsGtRV3/c6Bhui57suA== X-Received: by 2002:ac8:59c2:0:b0:39c:e3b4:1265 with SMTP id f2-20020ac859c2000000b0039ce3b41265mr27651344qtf.55.1666635543265; Mon, 24 Oct 2022 11:19:03 -0700 (PDT) Received: from x1n (bras-base-aurron9127w-grc-46-70-31-27-79.dsl.bell.ca. [70.31.27.79]) by smtp.gmail.com with ESMTPSA id ff12-20020a05622a4d8c00b0039cb5c9dbacsm348931qtb.22.2022.10.24.11.19.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 24 Oct 2022 11:19:02 -0700 (PDT) Date: Mon, 24 Oct 2022 14:19:01 -0400 From: Peter Xu To: Ira Weiny Cc: Andrew Morton , Randy Dunlap , Andrea Arcangeli , Matthew Wilcox , kernel test robot , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH for rc] mm/shmem: Ensure proper fallback if page faults Message-ID: References: <20221024043305.1491403-1-ira.weiny@intel.com> MIME-Version: 1.0 In-Reply-To: X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1666635546; a=rsa-sha256; cv=none; b=JvGxQKkcW8F0rPVBvMnxTnYUFIFgeJhVznqU0LXlyjHq4veCkzFujgxPlRgOSDiOMDizFP 5wpCBsxApdyR0SI1G1WQJ5Dg+1b+84JXKIEVQrqHZuSb+xvncd0ny0G7q896/Ft/HD+9oi 9Bqv0Mj2uqRglrg2Oln15lU4rAhgf34= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=i+6doAyu; spf=pass (imf26.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1666635546; 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=N8Yvakt3MK7rD0y3RReVeclZNu+K0N6huBCXY+hLsjg=; b=P2uXGRbxUa1ouFmdTFDNcu3XNetayiqmPJs1ilFOkCsZzZmxixFdDX9qbgKdbqlnkdDlVM ve14N8NNgvhFvCrNfAgDYmzVK0qBqwIZ0Yxk5ohQl8RoZak2yI+zHlftTt+WNzIOt8A5Mq 2OWEuVe2P6004kuA7tdqGpmyaIIIyEQ= X-Rspamd-Queue-Id: C7D66140036 X-Rspam-User: Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=i+6doAyu; spf=pass (imf26.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com X-Rspamd-Server: rspam04 X-Stat-Signature: 4693yeq4pp75gd1rn4kdznq8aawpwym6 X-HE-Tag: 1666635545-247371 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 Mon, Oct 24, 2022 at 09:54:30AM -0700, Ira Weiny wrote: > On Sun, Oct 23, 2022 at 09:33:05PM -0700, Ira wrote: > > From: Ira Weiny > > > > The kernel test robot flagged a recursive lock as a result of a > > conversion from kmap_atomic() to kmap_local_folio()[Link] > > > > The cause was due to the code depending on the kmap_atomic() side effect > > of disabling page faults. In that case the code expects the fault to > > fail and take the fallback case. > > > > git archaeology implied that the recursion may not be an actual bug.[1] > > However, the mmap_lock needed in the fault may be the one held.[2] > > > > Add an explicit pagefault_disable() and a big comment to explain this > > for future souls looking at this code. > > > > [1] https://lore.kernel.org/all/Y1MymJ%2FINb45AdaY@iweiny-desk3/ > > [2] https://lore.kernel.org/all/Y1M2p9OtBGnKwGUE@x1n/ > > > > Fixes: 7a7256d5f512 ("shmem: convert shmem_mfill_atomic_pte() to use a folio") > > Cc: Andrew Morton > > Cc: Randy Dunlap > > Cc: Peter Xu > > Cc: Andrea Arcangeli > > Reported-by: Matthew Wilcox (Oracle) > > Reported-by: kernel test robot > > Link: https://lore.kernel.org/r/202210211215.9dc6efb5-yujie.liu@intel.com > > Signed-off-by: Ira Weiny > > > > --- > > Thanks to Matt and Andrew for initial diagnosis. > > Thanks to Randy for pointing out C code needs ';' :-D > > Thanks to Andrew for suggesting an elaborate comment > > Thanks to Peter for pointing out that the mm's may be the same. > > --- > > mm/shmem.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/mm/shmem.c b/mm/shmem.c > > index 8280a5cb48df..c1bca31cd485 100644 > > --- a/mm/shmem.c > > +++ b/mm/shmem.c > > @@ -2424,9 +2424,16 @@ int shmem_mfill_atomic_pte(struct mm_struct *dst_mm, > > > > if (!zeropage) { /* COPY */ > > page_kaddr = kmap_local_folio(folio, 0); > > + /* > > + * The mmap_lock is held here. Disable page faults to > > + * prevent deadlock should copy_from_user() fault. The > > + * copy will be retried outside the mmap_lock. > > + */ > > Offline Dave Hansen and I were discussing this and he was concerned that this > comment implies that a deadlock would always occur rather than might occur. Agreed, "prevent deadlock" might be too strong in this context. > > I was not clear on this as I was thinking the read mmap_lock was non-recursive. > > So I think we have 3 cases only 1 of which will actually deadlock and is, as > Dave puts it, currently theoretical. > > 1) Different mm's are in play (no issue) > 2) Readlock implementation is recursive and same mm is in play (no issue) > 3) Readlock implementation is _not_ recursive (issue) > > In both 1 and 2 lockdep is incorrectly flagging the issue but 3 is a problem > and I think this is what Andrea was thinking. > > Is that the case? IMHO it would be good enough to just mention lockdep (as it can definitely trigger) or just quote similarly as Andrea's original comment somehow: If the rwsem starves writers it wasn't strictly a bug but lockdep doesn't like it and this avoids depending on lowlevel implementation details of the lock. IIUC no deadlock could really trigger at that time or Andrea should have written it in some other way. It also has actually summarized the goal that then we won't rely on rwsem impl but just make it always work. Thanks, > > If so the above comment is incorrectly worded and I should update it. > > Ira > > > + pagefault_disable(); > > ret = copy_from_user(page_kaddr, > > (const void __user *)src_addr, > > PAGE_SIZE); > > + pagefault_enable(); > > kunmap_local(page_kaddr); > > > > /* fallback to copy_from_user outside mmap_lock */ > > -- > > 2.37.2 > > > -- Peter Xu