[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230607205310.GA4122@monkey>
Date: Wed, 7 Jun 2023 13:53:10 -0700
From: Mike Kravetz <mike.kravetz@...cle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: Gerd Hoffmann <kraxel@...hat.com>,
Junxiao Chang <junxiao.chang@...el.com>,
kirill.shutemov@...ux.intel.com, mhocko@...e.com,
jmarchan@...hat.com, linux-mm@...ck.org,
linux-kernel@...r.kernel.org, muchun.song@...ux.dev,
Vivek Kasireddy <vivek.kasireddy@...el.com>,
Dongwon Kim <dongwon.kim@...el.com>,
James Houghton <jthoughton@...gle.com>,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] mm: fix hugetlb page unmap count balance issue
On 06/07/23 12:03, Andrew Morton wrote:
> On Tue, 16 May 2023 15:34:40 -0700 Mike Kravetz <mike.kravetz@...cle.com> wrote:
>
> > On 05/15/23 10:04, Mike Kravetz wrote:
> > > On 05/12/23 16:29, Mike Kravetz wrote:
> > > > On 05/12/23 14:26, James Houghton wrote:
> > > > > On Fri, May 12, 2023 at 12:20 AM Junxiao Chang <junxiao.chang@...el.com> wrote:
> > > > >
> > > > > This alone doesn't fix mapcounting for PTE-mapped HugeTLB pages. You
> > > > > need something like [1]. I can resend it if that's what we should be
> > > > > doing, but this mapcounting scheme doesn't work when the page structs
> > > > > have been freed.
> > > > >
> > > > > It seems like it was a mistake to include support for hugetlb memfds in udmabuf.
> > > >
> > > > IIUC, it was added with commit 16c243e99d33 udmabuf: Add support for mapping
> > > > hugepages (v4). Looks like it was never sent to linux-mm? That is unfortunate
> > > > as hugetlb vmemmap freeing went in at about the same time. And, as you have
> > > > noted udmabuf will not work if hugetlb vmemmap freeing is enabled.
> > > >
> > > > Sigh!
> > > >
> > > > Trying to think of a way forward.
> > > > --
> > > > Mike Kravetz
> > > >
> > > > >
> > > > > [1]: https://lore.kernel.org/linux-mm/20230306230004.1387007-2-jthoughton@google.com/
> > > > >
> > > > > - James
> > >
> > > Adding people and list on Cc: involved with commit 16c243e99d33.
> > >
> > > There are several issues with trying to map tail pages of hugetllb pages
> > > not taken into account with udmabuf. James spent quite a bit of time trying
> > > to understand and address all the issues with the HGM code. While using
> > > the scheme proposed by James, may be an approach to the mapcount issue there
> > > are also other issues that need attention. For example, I do not see how
> > > the fault code checks the state of the hugetlb page (such as poison) as none
> > > of that state is carried in tail pages.
> > >
> > > The more I think about it, the more I think udmabuf should treat hugetlb
> > > pages as hugetlb pages. They should be mapped at the appropriate level
> > > in the page table. Of course, this would impose new restrictions on the
> > > API (mmap and ioctl) that may break existing users. I have no idea how
> > > extensively udmabuf is being used with hugetlb mappings.
> >
> > Verified that using udmabug on a hugetlb mapping with vmemmap optimization will
> > BUG as:
>
> BUGs aren't good. Can we please find a way to push this along?
>
> Have we heard anything from any udmabuf people?
>
I have not heard anything. When this issue popped up, it took me by surprise.
udmabuf maintainer (Gerd Hoffmann), the people who added hugetlb support and
the list where udmabuf was developed (dri-devel@...ts.freedesktop.org) have
been on cc.
My 'gut reaction' would be to remove hugetlb support from udmabuf. From a
quick look, if we really want this support then there will need to be some
API changes. For example UDMABUF_CREATE should be hugetlb page aligned
and a multiple of hugetlb page size if using a hugetlb mapping.
It would be good to know about users of the driver.
--
Mike Kravetz
Powered by blists - more mailing lists