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 2B812C5AE59 for ; Thu, 5 Jun 2025 07:11:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B2EE86B00C6; Thu, 5 Jun 2025 03:11:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ADF126B0580; Thu, 5 Jun 2025 03:11:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9CDD36B0581; Thu, 5 Jun 2025 03:11:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 7C4E06B00C6 for ; Thu, 5 Jun 2025 03:11:00 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 22937121C7E for ; Thu, 5 Jun 2025 07:11:00 +0000 (UTC) X-FDA: 83520475080.18.46AB61D Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) by imf20.hostedemail.com (Postfix) with ESMTP id 1341D1C000A for ; Thu, 5 Jun 2025 07:10:57 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=APm2MuAM; spf=pass (imf20.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.47 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1749107458; 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=vbmofZAA3TlXbKhnQqtXMU8CfPLcrFe6uu6tYmuI958=; b=xaNEznbAYTBnQsY6Q7WptF1HQyLrDI513b5Ek6iZ6aNThOjwZHJZqnsu6Ryw19KTs8tBmb 0zBk5jrF+/8v7J9GJQbJ1aV9/yQkBxC1QUD45qzZL+rFRNaGAnpgxhtJRbgRIkxo5mJYNM G+OVu6KeMKS5Vrz0odlS1frYY9Wyp1Y= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=APm2MuAM; spf=pass (imf20.hostedemail.com: domain of mhocko@suse.com designates 209.85.208.47 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1749107458; a=rsa-sha256; cv=none; b=g81zDfOjJ/PkgYK/AzfHtzb0PB/g3zWVyPC6BW4kGhl+DiTzlPj7QmIqNsDzyFMLblZRsZ cy5uKsqn9p5WyvnEWRN/vByoLGVcEfpO69XzH2FYISP4IupKM9i7HsXY0ehvNwWoR9W+Fp kAUYtsBiZGem1J/ust75jn5bVFisURg= Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-60727e46168so814493a12.0 for ; Thu, 05 Jun 2025 00:10:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1749107456; x=1749712256; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=vbmofZAA3TlXbKhnQqtXMU8CfPLcrFe6uu6tYmuI958=; b=APm2MuAMrCLvXib/+l2iTfSkJJ3rrQ4i/Y4Ldi2ywgHWXOaLjqWiGgA+1ZDx2HbQUe u2H2wruSiL3C6s4fw5H9PICq7nbgBKFlQX6syzfr3ktl2nxwjzDCCvk35XZPlJjRJHlI 0FqiOUmT8GPe7qsZg/WlQJJcWVvKnQKDVuAeK2AT0emIrDwtM6bCv6EGUNVpCVXH4ean eqXnJZxry6PLV4uOZOJouHDLk7+dzXvDhUQTpmPgPBId0kRQK4RzxghqX1M6s3eKq5dh XKNlEkNxev6IJecFBGYSugmO90C9Q7OQMXytZJmf1KDuqt40iCMW0tWxQOq5A7b7RBf1 zF3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1749107456; x=1749712256; 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=vbmofZAA3TlXbKhnQqtXMU8CfPLcrFe6uu6tYmuI958=; b=p8h7ckQ11oaNXPw8sPRovl1ZnRpSlGNwZpBZoTx1q9wCgzBfLcdENzdQpBHzYv3NmY zJ1yCn4v3UPa+QzXm5YXw/qTbt3VuZgIjgyoHSBQsp3amWfN4+Vn1RYUXY0kqpIijH+j jX7uCdVmVn0e1hwyP2lnbX7QMGbAnQYyS9prrvJPxSt+nPMxOlwCM9h2FY/Ck3JLytQa AI0bhuZCkF7fUr6goek36TdBA2Nzb+oJ43tlVBWqftUILsVMY1aCcGDmB8J/MzMMEoZF uXeYEAm2xvBrk41lt6nS5/wtUdDfETLj/iqfPy/6jl8S1GExXFniFMVAjH5HSijdl/yq G8PA== X-Forwarded-Encrypted: i=1; AJvYcCW4tuoUf2r/0vQflFWhzrThLR2UKJANFGWIoSntAL+Tj0u/WJvoNVSU+IHmai7COVkdH+DUF2uMHw==@kvack.org X-Gm-Message-State: AOJu0YzLeQmrxJkZaM8dLUSzmv+9OHGyud0oIFRf44m49AJae547h/Hj NgAv58UtKyCi/oV0LaN/yi0g2JCT7TeyACejQ2nJljflWhAc+j8J9azTT9MbBjH97LE= X-Gm-Gg: ASbGncvez4WXZX/TIFhJuMCkAgP4foWu3AgJ1gXHHEfU0yjz3IVD2GivQOqZsuMyELu XByoTDAjVA9ZtUTWv3J566Lct677oSaaha9+633T5pImbeoubFdxOcX/CRde2gZfs/NOB3ASaWc c2Hgi253/bHe7NaL1oie8Xf55Q6TgTXqGia06XWLlUDOhc0m8mRkqdhN7Pdv52lTgZUSLAinyik Xhe9JcdQP9X0LAIdTycPkAzCF+Z6+VJ2arVVDwEudIqUn06lY+oH9ODyS287xODlyRpoOqSnVYh ULL0y5fGyXntsfbhPDEf39DvDfU/HFBGZdI0DcifAKm03IML7n8goa3hHsSuz67cFFARFtaCvM8 = X-Google-Smtp-Source: AGHT+IE8sHHRwQar+3CLp0koW1FVTyH3vEN2IdU9qg5bSpsv++Z5rRrVGAZDYPz5dOS2nyQtefp2bg== X-Received: by 2002:a05:6402:26d0:b0:604:5cae:4031 with SMTP id 4fb4d7f45d1cf-606ea191815mr6085467a12.28.1749107456276; Thu, 05 Jun 2025 00:10:56 -0700 (PDT) Received: from localhost (109-81-89-112.rct.o2.cz. [109.81.89.112]) by smtp.gmail.com with UTF8SMTPSA id 4fb4d7f45d1cf-60566c5a758sm9856845a12.19.2025.06.05.00.10.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Jun 2025 00:10:55 -0700 (PDT) Date: Thu, 5 Jun 2025 09:10:55 +0200 From: Michal Hocko To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andrew Morton , Lorenzo Stoakes , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Jason Gunthorpe , John Hubbard , Peter Xu Subject: Re: [PATCH v1] mm/gup: remove (VM_)BUG_ONs Message-ID: References: <20250604140544.688711-1-david@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250604140544.688711-1-david@redhat.com> X-Rspamd-Queue-Id: 1341D1C000A X-Stat-Signature: gqarekg3a94pknqpzoyfq74es3q5izgr X-Rspam-User: X-Rspamd-Server: rspam07 X-HE-Tag: 1749107457-23669 X-HE-Meta: U2FsdGVkX18zJFNyuZhXRSsKsT3qqTuCzg8vZOh18dFUjfvNC/m7OY2iWSACvR3d+rbJY8r9X4YWPHxwCjob1vBdJk9kO07BGfIdA6vO7HgknPUk8peS4WhiM780kTZlzSaDPnnLbq0snQauo4jSu1/fNkZ+CnZmnLvBPInwjDPaFOLdyr2QJXzXGRyiXRlaQr/LxhCpIgeUzwtyWzmc6MXX0mFXxHaP3ooxe52HFt5DuUTDUeY8mNONmpa3amdcrva4jhSNOPGNJEibvwOcUBiSvV7OQuhceAu61qYb5bdkmBT+Zi4Xz6u7YscSZ0WQCAW2YW4/W82MgSxsras8POMfQLLea3ZJ/IzsQXOgtKTxsfqhcDIKvslfV5T+xJimOAwicWf6uiBOwTtZ/0UXPj8XMBsBppioNHfn8UyczpEVc5xGd2G3SVe/5l+Qgdy7z+iQcQFyaMnv3nuz0bE/P9DGo7wMDOWKW+oAAcW/bu8CLtpL1mOZ1PYfCmn7+0hBR5a0ETnNFg+j6vCDknWBg+IeWt5fI9u5ZQNCaE0kYoonqNn0MiaKHbr3R5FPgAPgKZs7f40lXBfnkC4mb9UAgUAXsBQVvhgMKXz0QIMlUWJBPJj48uJ7OrfgL3gTsuLqwhHa8E4jAd6GCLQwnALXHp0k9srELEEMN9NMwZZNAprqm8Zp+/2+ql4iDSjzTggyNc+3UxVTYbdlGk+4j9NZsSEHJjQ/SYu0wT8zmZxi5rVRKDUgLfgauiMGr1KkdjiaR07cvGjf1QYCR6+/GkxF6RDrWK5d4yFH6OTgjbBAMtcg9gI2ylUeq+LvDcMEZMNK5WKlQUYVrRl759Ld+3FCZdbYThDfmQ9oMfwb4m5p9d9uDAsGPbSfu8Fo4xzoGpD8/dxmU2KN9qEdMc8WsCw90DY8IPomBA0PdMDsW30Gg9vfvl8/GCd3wt3smmMolxMseXDLpEQMHssWvbKSbEM MddpOaKH gt3hbAbOc72o9A6MxPYHYNfSf0bg9qGQEL1Jf7y8ezbfiRETOdRWI2cQ85VD9tzRA1YX9m3UeocFe8RJCc9kWcaNQcnpjsGGp5NgvOOy9ZcsvInE3Sgz7aH9wGxH3Mcj7bIdMknw0APniJGE7NT64Egu2khy8mYmst3e+B1+y7b7XZHfKOLdbe3aJz/+HwWfEugr19ue1cOUqS67SiDpvYAanIFZfXRJSyQboPX4z0MeSFsFvwN/UZvMxPM8bHiuyty1F+lp6vgM6HJzCGsTYNKXYsnr0pEk8gnbvGDjC90k6NvICPyisYiQyIhbftqGuHHnOBmsEaaJMaxG8kyvOY7xwVzm3/0lFCw+5zPeiz6CtrhZI/jSpZ3QdzKLSMR+RfOg7uyd7+yjGZIy9DUfcS+6K1btCW2bZUNGsDWmDGgdtAcL1OPmnmVJuwpbBQKshUKL08k5bTNQwJgVUS13SzlJTrVO5BGlSAXY2G2oZTPMdUNj/lh1Ej42s6fWZV1R8z0C0IQNSovKCTAJg/ftcBxwK+d5EKr7XCdbmpOYzPBlGDRqzvELI2YSXaxT071AgbDCZNI3EcrqRYklNz6soqyYYVjeegHMsjpsU9srMUz0gPpwd9GwBp6oSeXYkRSJAVmgKGrpMRsQGC6Ni0H+lWVT0/g== 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: List-Subscribe: List-Unsubscribe: On Wed 04-06-25 16:05:44, David Hildenbrand wrote: > Especially once we hit one of the assertions in > sanity_check_pinned_pages(), observing follow-up assertions failing > in other code can give good clues about what went wrong, so use > VM_WARN_ON_ONCE instead. > > While at it, let's just convert all VM_BUG_ON to VM_WARN_ON_ONCE as > well. Add one comment for the pfn_valid() check. > > We have to introduce VM_WARN_ON_ONCE_VMA() to make that fly. > > Drop the BUG_ON after mmap_read_lock_killable(), if that ever returns > something > 0 we're in bigger trouble. Convert the other BUG_ON's into > VM_WARN_ON_ONCE as well, they are in a similar domain "should never > happen", but more reasonable to check for during early testing. The patch itself makes sense and I think it is good time to revisit old discussion [1] and finally drop VM_BUG_ON altogether and replace it by VM_WARN_ON which could be still a useful debugging aid. [1] https://lore.kernel.org/all/c9abf109-80f2-88f5-4aae-d6fd4a30bcd3@google.com/T/#u > > Cc: Andrew Morton > Cc: Lorenzo Stoakes > Cc: "Liam R. Howlett" > Cc: Vlastimil Babka > Cc: Mike Rapoport > Cc: Suren Baghdasaryan > Cc: Michal Hocko > Cc: Jason Gunthorpe > Cc: John Hubbard > Cc: Peter Xu > Signed-off-by: David Hildenbrand Acked-by: Michal Hocko > --- > > Wanted to do this for a long time, but my todo list keeps growing ... > > Based on mm/mm-unstable > > --- > include/linux/mmdebug.h | 12 ++++++++++++ > mm/gup.c | 41 +++++++++++++++++++---------------------- > 2 files changed, 31 insertions(+), 22 deletions(-) > > diff --git a/include/linux/mmdebug.h b/include/linux/mmdebug.h > index a0a3894900ed4..14a45979cccc9 100644 > --- a/include/linux/mmdebug.h > +++ b/include/linux/mmdebug.h > @@ -89,6 +89,17 @@ void vma_iter_dump_tree(const struct vma_iterator *vmi); > } \ > unlikely(__ret_warn_once); \ > }) > +#define VM_WARN_ON_ONCE_VMA(cond, vma) ({ \ > + static bool __section(".data..once") __warned; \ > + int __ret_warn_once = !!(cond); \ > + \ > + if (unlikely(__ret_warn_once && !__warned)) { \ > + dump_vma(vma); \ > + __warned = true; \ > + WARN_ON(1); \ > + } \ > + unlikely(__ret_warn_once); \ > +}) > #define VM_WARN_ON_VMG(cond, vmg) ({ \ > int __ret_warn = !!(cond); \ > \ > @@ -115,6 +126,7 @@ void vma_iter_dump_tree(const struct vma_iterator *vmi); > #define VM_WARN_ON_FOLIO(cond, folio) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ON_ONCE_FOLIO(cond, folio) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ON_ONCE_MM(cond, mm) BUILD_BUG_ON_INVALID(cond) > +#define VM_WARN_ON_ONCE_VMA(cond, vma) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ON_VMG(cond, vmg) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN_ONCE(cond, format...) BUILD_BUG_ON_INVALID(cond) > #define VM_WARN(cond, format...) BUILD_BUG_ON_INVALID(cond) > diff --git a/mm/gup.c b/mm/gup.c > index e065a49842a87..3c3931fcdd820 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -64,11 +64,11 @@ static inline void sanity_check_pinned_pages(struct page **pages, > !folio_test_anon(folio)) > continue; > if (!folio_test_large(folio) || folio_test_hugetlb(folio)) > - VM_BUG_ON_PAGE(!PageAnonExclusive(&folio->page), page); > + VM_WARN_ON_ONCE_PAGE(!PageAnonExclusive(&folio->page), page); > else > /* Either a PTE-mapped or a PMD-mapped THP. */ > - VM_BUG_ON_PAGE(!PageAnonExclusive(&folio->page) && > - !PageAnonExclusive(page), page); > + VM_WARN_ON_ONCE_PAGE(!PageAnonExclusive(&folio->page) && > + !PageAnonExclusive(page), page); > } > } > > @@ -760,8 +760,8 @@ static struct page *follow_huge_pmd(struct vm_area_struct *vma, > if (!pmd_write(pmdval) && gup_must_unshare(vma, flags, page)) > return ERR_PTR(-EMLINK); > > - VM_BUG_ON_PAGE((flags & FOLL_PIN) && PageAnon(page) && > - !PageAnonExclusive(page), page); > + VM_WARN_ON_ONCE_PAGE((flags & FOLL_PIN) && PageAnon(page) && > + !PageAnonExclusive(page), page); > > ret = try_grab_folio(page_folio(page), 1, flags); > if (ret) > @@ -899,8 +899,8 @@ static struct page *follow_page_pte(struct vm_area_struct *vma, > goto out; > } > > - VM_BUG_ON_PAGE((flags & FOLL_PIN) && PageAnon(page) && > - !PageAnonExclusive(page), page); > + VM_WARN_ON_ONCE_PAGE((flags & FOLL_PIN) && PageAnon(page) && > + !PageAnonExclusive(page), page); > > /* try_grab_folio() does nothing unless FOLL_GET or FOLL_PIN is set. */ > ret = try_grab_folio(folio, 1, flags); > @@ -1180,7 +1180,7 @@ static int faultin_page(struct vm_area_struct *vma, > if (unshare) { > fault_flags |= FAULT_FLAG_UNSHARE; > /* FAULT_FLAG_WRITE and FAULT_FLAG_UNSHARE are incompatible */ > - VM_BUG_ON(fault_flags & FAULT_FLAG_WRITE); > + VM_WARN_ON_ONCE(fault_flags & FAULT_FLAG_WRITE); > } > > ret = handle_mm_fault(vma, address, fault_flags, NULL); > @@ -1760,10 +1760,7 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm, > } > > /* VM_FAULT_RETRY or VM_FAULT_COMPLETED cannot return errors */ > - if (!*locked) { > - BUG_ON(ret < 0); > - BUG_ON(ret >= nr_pages); > - } > + VM_WARN_ON_ONCE(!*locked && (ret < 0 || ret >= nr_pages)); > > if (ret > 0) { > nr_pages -= ret; > @@ -1808,7 +1805,6 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm, > > ret = mmap_read_lock_killable(mm); > if (ret) { > - BUG_ON(ret > 0); > if (!pages_done) > pages_done = ret; > break; > @@ -1819,11 +1815,11 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm, > pages, locked); > if (!*locked) { > /* Continue to retry until we succeeded */ > - BUG_ON(ret != 0); > + VM_WARN_ON_ONCE(ret != 0); > goto retry; > } > if (ret != 1) { > - BUG_ON(ret > 1); > + VM_WARN_ON_ONCE(ret > 1); > if (!pages_done) > pages_done = ret; > break; > @@ -1885,10 +1881,10 @@ long populate_vma_page_range(struct vm_area_struct *vma, > int gup_flags; > long ret; > > - VM_BUG_ON(!PAGE_ALIGNED(start)); > - VM_BUG_ON(!PAGE_ALIGNED(end)); > - VM_BUG_ON_VMA(start < vma->vm_start, vma); > - VM_BUG_ON_VMA(end > vma->vm_end, vma); > + VM_WARN_ON_ONCE(!PAGE_ALIGNED(start)); > + VM_WARN_ON_ONCE(!PAGE_ALIGNED(end)); > + VM_WARN_ON_ONCE_VMA(start < vma->vm_start, vma); > + VM_WARN_ON_ONCE_VMA(end > vma->vm_end, vma); > mmap_assert_locked(mm); > > /* > @@ -1957,8 +1953,8 @@ long faultin_page_range(struct mm_struct *mm, unsigned long start, > int gup_flags; > long ret; > > - VM_BUG_ON(!PAGE_ALIGNED(start)); > - VM_BUG_ON(!PAGE_ALIGNED(end)); > + VM_WARN_ON_ONCE(!PAGE_ALIGNED(start)); > + VM_WARN_ON_ONCE(!PAGE_ALIGNED(end)); > mmap_assert_locked(mm); > > /* > @@ -2908,7 +2904,8 @@ static int gup_fast_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr, > } else if (pte_special(pte)) > goto pte_unmap; > > - VM_BUG_ON(!pfn_valid(pte_pfn(pte))); > + /* If it's not marked as special it must have a valid memmap. */ > + VM_WARN_ON_ONCE(!pfn_valid(pte_pfn(pte))); > page = pte_page(pte); > > folio = try_grab_folio_fast(page, 1, flags); > > base-commit: 2d0c297637e7d59771c1533847c666cdddc19884 > -- > 2.49.0 -- Michal Hocko SUSE Labs