lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.11.2106031923410.12760@eggly.anvils>
Date:   Thu, 3 Jun 2021 19:45:21 -0700 (PDT)
From:   Hugh Dickins <hughd@...gle.com>
To:     Yang Shi <shy828301@...il.com>
cc:     Hugh Dickins <hughd@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Wang Yugui <wangyugui@...-tech.com>,
        Matthew Wilcox <willy@...radead.org>,
        Naoya Horiguchi <naoya.horiguchi@....com>,
        Alistair Popple <apopple@...dia.com>,
        Ralph Campbell <rcampbell@...dia.com>, Zi Yan <ziy@...dia.com>,
        Miaohe Lin <linmiaohe@...wei.com>,
        Minchan Kim <minchan@...nel.org>, Jue Wang <juew@...gle.com>,
        Peter Xu <peterx@...hat.com>, Jan Kara <jack@...e.cz>,
        Linux MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/7] mm/thp: try_to_unmap() use TTU_SYNC for safe DEBUG_VM
 splitting

On Thu, 3 Jun 2021, Yang Shi wrote:
> On Tue, Jun 1, 2021 at 2:07 PM Hugh Dickins <hughd@...gle.com> wrote:
> >
> > Instead of abandoning the unsafe VM_BUG_ON_PAGE(), and the ones that
> > follow, use PVMW_SYNC in try_to_unmap_one() in this case: adding TTU_SYNC
> > to the options, and passing that from unmap_page() when CONFIG_DEBUG_VM=y.
> > It could be passed in the non-debug case too, but that would sometimes add
> > a little overhead, whereas it's rare for this race to result in failure.
> 
> The above statement makes me feel this patch is just to relieve the
> VM_BUG_ON, but my patch already changed it to VM_WARN, the race sounds
> acceptable (at least not fatal) and the splitting code can handle the
> failure case as well. So I'm wondering if we still need this patch or
> not if it is just used to close the race when CONFIG_DEBUG_VM=y.

I do agree that your 1/2 (what I'm calling 6.1/7) BUG->WARN patch
is the most important of them; but it didn't get marked for stable,
and has got placed behind conflicting mods never intended for stable.

And a lot of the descriptions had been written in terms of the prior
situation, with VM BUG there: it was easier to keep describing that way.

Whether your fix makes mine redundant is arguable (Wang Yugui thinks
not).  It's easier to argue that it makes the racy ones (like this)
redundant, than the persistent ones (like vma_address or pvm_walk).

Since I know of at least one customer who wants all these fixes in 5.10
longterm, I'm fighting to get them that far at least.  But the further
back they go, the less effort I'll make to backport them - will fall
back to porting your BUG->WARN only.

Hugh

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ