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 X-Spam-Level: X-Spam-Status: No, score=-18.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC4CEC47082 for ; Thu, 27 May 2021 02:48:14 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2AE05613C5 for ; Thu, 27 May 2021 02:48:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2AE05613C5 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9046E6B0036; Wed, 26 May 2021 22:48:13 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8B3516B006E; Wed, 26 May 2021 22:48:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 77C6F6B0070; Wed, 26 May 2021 22:48:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0118.hostedemail.com [216.40.44.118]) by kanga.kvack.org (Postfix) with ESMTP id 409DC6B0036 for ; Wed, 26 May 2021 22:48:13 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id B1D948249980 for ; Thu, 27 May 2021 02:48:12 +0000 (UTC) X-FDA: 78185476824.01.FCE6E08 Received: from mail-pg1-f180.google.com (mail-pg1-f180.google.com [209.85.215.180]) by imf09.hostedemail.com (Postfix) with ESMTP id D99B7600024C for ; Thu, 27 May 2021 02:48:07 +0000 (UTC) Received: by mail-pg1-f180.google.com with SMTP id m124so2578050pgm.13 for ; Wed, 26 May 2021 19:48:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=eWx+OpCJ4rmuZE/Novcm4YkSd9BZdaQy71xHN9zFsao=; b=gkNTUeHU0Oz/4A7/7lrXStSVLbfy0HORatcJ+/A9JgrmvUrbc9wUpDRb27/1TURNAy K1wGYk73NBabCOl59C0WW1oiXYdkQsxgMuCX6QFfNTGX2zm8ZsXPT7c8SF1Vjki6Sd77 Z+gKcWdyyx4ppllxMBHnyDJ8oEONoTArBXOzX3zheJJQ6Vb/yGbO76eqxoOuS4xNKgBN 3maBY1dnui5HZJoW+H/tMtj8RWimCa3xDzbwYPlktY4gwtrQkgAs0iepa07YuYalFeYx I/yIp2ntbScDhNR+L41ET6/Q5bvuUHCzdt4OdtoWfFDWdez/jNk2n1/71Nh0CHuO4rw5 XZgw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=eWx+OpCJ4rmuZE/Novcm4YkSd9BZdaQy71xHN9zFsao=; b=l3aYE9JnZCFBabHniB89QhMrC5Iv/CJdEnsToskjIrnaKsx/qFbWhbcEk8IkQ0TF3N s813AaKJ414+mdAxEgYW6if+iQUKr2HtzE+TBfn7NmxiJmeIGLz/4wRjFcmgGxbyBhCM aws0iQ6XTA8ecwn7XYxO35FPNkPZMxGoAVB5Iem9CQaf+ef4qmv1+YMmrtuz1JTTheNH ma9RMq2JEEvILqIioa2ji8hJznsErQt/bDy/yzjwy+0BpruNpk2diplpRg5iKfLvPSdd QrjUQPyeaIGRVc3YmMwzsPU4ylLMegXUp9CIfyoSwUG/1g+osSohugp1IoJyJxQMQG0r 45qA== X-Gm-Message-State: AOAM532MMnhQJlNvi21s/KDpfuvT+oyQ68YQKKdb1ee5HDHJsKxXHrQT H+mGzPe519Yu5ZmUrE0VdZJPJhZS02Fqr3M8gDS/Nw== X-Google-Smtp-Source: ABdhPJxghayE1dn+evykauwejO/vJ/Kzt5OpRkvBbNztO1rZe/JIDryuivbzRx17L/OETeoyUArQUXTt1qzoXweexpI= X-Received: by 2002:a05:6a00:c86:b029:2e9:3041:162f with SMTP id a6-20020a056a000c86b02902e93041162fmr1533796pfv.78.1622083691247; Wed, 26 May 2021 19:48:11 -0700 (PDT) MIME-Version: 1.0 References: <78359cf0-6e28-2aaa-d17e-6519b117b3db@oracle.com> <20210525233134.246444-1-mike.kravetz@oracle.com> <1485e64d-e794-c24e-9688-51b0c1bc1340@oracle.com> In-Reply-To: From: Mina Almasry Date: Wed, 26 May 2021 19:48:00 -0700 Message-ID: Subject: Re: [External] [PATCH 0/2] Track reserve map changes to restore on error To: Mike Kravetz Cc: Muchun Song , Linux-MM , open list , Axel Rasmussen , Peter Xu , Andrew Morton Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf09.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=gkNTUeHU; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf09.hostedemail.com: domain of almasrymina@google.com designates 209.85.215.180 as permitted sender) smtp.mailfrom=almasrymina@google.com X-Rspamd-Server: rspam05 X-Rspamd-Queue-Id: D99B7600024C X-Stat-Signature: ger3bwy8t4b3gqrau6e4w74bph88m4aq X-HE-Tag: 1622083687-476723 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, May 26, 2021 at 4:19 PM Mina Almasry wrote: > > On Wed, May 26, 2021 at 10:17 AM Mike Kravetz wrote: > > > > On 5/25/21 8:19 PM, Muchun Song wrote: > > > On Wed, May 26, 2021 at 7:31 AM Mike Kravetz wrote: > > >> > > >> Here is a modification to the reservation tracking for fixup on errors. > > >> It is a more general change, but should work for the hugetlb_mcopy_pte_atomic > > >> case as well. > > >> > > >> Perhaps use this as a prerequisite for your fix(es)? Pretty sure this > > >> will eliminate the need for the call to hugetlb_unreserve_pages. > > > > > > Hi Mike, > > > > > > It seems like someone is fixing a bug, right? Maybe a link should be > > > placed in the cover letter so that someone can know what issue > > > we are facing. > > > > > > > Thanks Muchun, > > > > I wanted to first see if these patches would work in the code Mina is > > modifying. If this works for Mina, then a more formal patch and request > > for inclusion will be sent. > > > > So a quick test: I apply my patche and yours on top of linus/master, > and I remove the hugetlb_unreserve_pages() call that triggered this > conversation, and run the userfaultfd test, resv_huge_pages underflows > again, so it seems on the surface this doesn't quite work as is. > > Not quite sure what to do off the top of my head. I think I will try > to debug why the 3 patches don't work together and I will fix either > your patch or mine. I haven't taken a deep look yet; I just ran a > quick test. > Ok found the issue. With the setup I described above, the hugetlb_shared test case passes: ./tools/testing/selftests/vm/userfaultfd hugetlb_shared 10 2 /tmp/kokonut_test/huge/userfaultfd_test && echo test success The non-shared test case is the one that underflows: ./tools/testing/selftests/vm/userfaultfd hugetlb 10 2 /tmp/kokonut_test/huge/userfaultfd_test && echo test success I've debugged a bit, and this messy hunk 'fixes' the underflow with the non-shared case. (Sorry for the messiness). @@ -2329,17 +2340,14 @@ void restore_reserve_on_error(struct hstate *h, struct vm_area_struct *vma, */ SetHPageRestoreRsvCnt(page); } else { - rc = vma_needs_reservation(h, vma, address); - if (rc < 0) - /* - * See above comment about rare out of - * memory condition. - */ - SetHPageRestoreRsvCnt(page); - else if (rc) - vma_add_reservation(h, vma, address); - else - vma_end_reservation(h, vma, address); + resv = inode_resv_map(vma->vm_file->f_mapping->host); + if (resv) { + int chg = region_del(resv, idx, idx+1); + VM_BUG_ON(chg); + } The reason being is that on page allocation we region_add() an entry into the resv_map regardless of whether this is a shared mapping or not (vma_needs_reservation() + vma_commit_reservation(), which amounts to region_add() at the end of the day). To unroll back this change on error, we need to region_del() the region_add(). The code removed above doesn't end up calling region_del(), because vma_needs_reservation() returns 0, because region_chg() sees there is an entry in the resv_map, and returns 0. The VM_BUG_ON() is just because I'm not sure how to handle that error. > > I believe this issue has existed since the introduction of hugetlb > > reservations in v2.6.18. Since the bug only shows up when we take error > > paths, the issue may not have been observed. Mina found a similar issue > > in an error path which could also expose this issue. > > -- > > Mike Kravetz