From: Joerg Roedel <joro@8bytes.org>
To: Alex Shi <alex.shi@linux.alibaba.com>,
Robin Murphy <robin.murphy@arm.com>
Cc: "Hugh Dickins" <hughd@google.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Matthew Wilcox" <willy@infradead.org>,
Linux-MM <linux-mm@kvack.org>,
iommu@lists.linux-foundation.org,
"Mika Penttilä" <mika.penttila@nextfour.com>,
"Johannes Weiner" <hannes@cmpxchg.org>,
"Kirill A. Shutemov" <kirill@shutemov.name>
Subject: Re: a question of split_huge_page
Date: Fri, 10 Jul 2020 14:56:23 +0200 [thread overview]
Message-ID: <20200710125623.GH27672@8bytes.org> (raw)
In-Reply-To: <50113530-fae5-bb36-56c2-5b5c4f90426d@linux.alibaba.com>
Adding Robin.
On Fri, Jul 10, 2020 at 05:34:52PM +0800, Alex Shi wrote:
> 在 2020/7/10 下午1:28, Mika Penttilä 写道:
> >
> >
> > On 10.7.2020 7.51, Alex Shi wrote:
> >>
> >> 在 2020/7/10 上午12:07, Kirill A. Shutemov 写道:
> >>> On Thu, Jul 09, 2020 at 04:50:02PM +0100, Matthew Wilcox wrote:
> >>>> On Thu, Jul 09, 2020 at 11:11:11PM +0800, Alex Shi wrote:
> >>>>> Hi Kirill & Matthew,
> >>>>>
> >>>>> In the func call chain, from split_huge_page() to lru_add_page_tail(),
> >>>>> Seems tail pages are added to lru list at line 963, but in this scenario
> >>>>> the head page has no lru bit and isn't set the bit later. Why we do this?
> >>>>> or do I miss sth?
> >>>> I don't understand how we get to split_huge_page() with a page that's
> >>>> not on an LRU list. Both anonymous and page cache pages should be on
> >>>> an LRU list. What am I missing?>
> >>
> >> Thanks a lot for quick reply!
> >> What I am confusing is the call chain: __iommu_dma_alloc_pages()
> >> to split_huge_page(), in the func, splited page,
> >> page = alloc_pages_node(nid, alloc_flags, order);
> >> And if the pages were added into lru, they maybe reclaimed and lost,
> >> that would be a panic bug. But in fact, this never happened for long time.
> >> Also I put a BUG() at the line, it's nevre triggered in ltp, and run_vmtests
> >
> >
> > In __iommu_dma_alloc_pages, after split_huge_page(), who is taking a
> > reference on tail pages? Seems tail pages are freed and the function
> > errornously returns them in pages[] array for use?
> >
>
> CC Joerg and iommu list,
>
> That's a good question. seems the split_huge_page was never triggered here,
> since the func would check the PageLock first. and have page->mapping and PageAnon
> check, any of them couldn't be matched for the alloced page.
>
> Hi Joerg,
> would you like look into this? do we still need the split_huge_page() here?
>
> Thanks
> Alex
>
> int split_huge_page_to_list(struct page *page, struct list_head *list)
> {
> struct page *head = compound_head(page);
> struct deferred_split *ds_queue = get_deferred_split_queue(head);
> struct anon_vma *anon_vma = NULL;
> struct address_space *mapping = NULL;
> int count, mapcount, extra_pins, ret;
> pgoff_t end;
>
> VM_BUG_ON_PAGE(is_huge_zero_page(head), head);
> VM_BUG_ON_PAGE(!PageLocked(head), head); <==
> >
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
WARNING: multiple messages have this Message-ID (diff)
From: Joerg Roedel <joro@8bytes.org>
To: Alex Shi <alex.shi@linux.alibaba.com>,
Robin Murphy <robin.murphy@arm.com>
Cc: "Mika Penttilä" <mika.penttila@nextfour.com>,
"Kirill A. Shutemov" <kirill@shutemov.name>,
"Matthew Wilcox" <willy@infradead.org>,
"Johannes Weiner" <hannes@cmpxchg.org>,
Linux-MM <linux-mm@kvack.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"Hugh Dickins" <hughd@google.com>,
iommu@lists.linux-foundation.org
Subject: Re: a question of split_huge_page
Date: Fri, 10 Jul 2020 14:56:23 +0200 [thread overview]
Message-ID: <20200710125623.GH27672@8bytes.org> (raw)
In-Reply-To: <50113530-fae5-bb36-56c2-5b5c4f90426d@linux.alibaba.com>
Adding Robin.
On Fri, Jul 10, 2020 at 05:34:52PM +0800, Alex Shi wrote:
> 在 2020/7/10 下午1:28, Mika Penttilä 写道:
> >
> >
> > On 10.7.2020 7.51, Alex Shi wrote:
> >>
> >> 在 2020/7/10 上午12:07, Kirill A. Shutemov 写道:
> >>> On Thu, Jul 09, 2020 at 04:50:02PM +0100, Matthew Wilcox wrote:
> >>>> On Thu, Jul 09, 2020 at 11:11:11PM +0800, Alex Shi wrote:
> >>>>> Hi Kirill & Matthew,
> >>>>>
> >>>>> In the func call chain, from split_huge_page() to lru_add_page_tail(),
> >>>>> Seems tail pages are added to lru list at line 963, but in this scenario
> >>>>> the head page has no lru bit and isn't set the bit later. Why we do this?
> >>>>> or do I miss sth?
> >>>> I don't understand how we get to split_huge_page() with a page that's
> >>>> not on an LRU list. Both anonymous and page cache pages should be on
> >>>> an LRU list. What am I missing?>
> >>
> >> Thanks a lot for quick reply!
> >> What I am confusing is the call chain: __iommu_dma_alloc_pages()
> >> to split_huge_page(), in the func, splited page,
> >> page = alloc_pages_node(nid, alloc_flags, order);
> >> And if the pages were added into lru, they maybe reclaimed and lost,
> >> that would be a panic bug. But in fact, this never happened for long time.
> >> Also I put a BUG() at the line, it's nevre triggered in ltp, and run_vmtests
> >
> >
> > In __iommu_dma_alloc_pages, after split_huge_page(), who is taking a
> > reference on tail pages? Seems tail pages are freed and the function
> > errornously returns them in pages[] array for use?
> >
>
> CC Joerg and iommu list,
>
> That's a good question. seems the split_huge_page was never triggered here,
> since the func would check the PageLock first. and have page->mapping and PageAnon
> check, any of them couldn't be matched for the alloced page.
>
> Hi Joerg,
> would you like look into this? do we still need the split_huge_page() here?
>
> Thanks
> Alex
>
> int split_huge_page_to_list(struct page *page, struct list_head *list)
> {
> struct page *head = compound_head(page);
> struct deferred_split *ds_queue = get_deferred_split_queue(head);
> struct anon_vma *anon_vma = NULL;
> struct address_space *mapping = NULL;
> int count, mapcount, extra_pins, ret;
> pgoff_t end;
>
> VM_BUG_ON_PAGE(is_huge_zero_page(head), head);
> VM_BUG_ON_PAGE(!PageLocked(head), head); <==
> >
next prev parent reply other threads:[~2020-07-10 12:56 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-09 15:11 a question of split_huge_page Alex Shi
2020-07-09 15:50 ` Matthew Wilcox
2020-07-09 16:07 ` Kirill A. Shutemov
2020-07-10 4:51 ` Alex Shi
2020-07-10 5:28 ` Mika Penttilä
2020-07-10 7:00 ` Alex Shi
2020-07-10 7:22 ` Mika Penttilä
2020-07-10 9:34 ` Alex Shi
2020-07-10 9:34 ` Alex Shi
2020-07-10 12:56 ` Joerg Roedel [this message]
2020-07-10 12:56 ` Joerg Roedel
2020-07-10 17:29 ` Yang Shi
2020-07-10 17:29 ` Yang Shi
2020-07-10 10:33 ` Kirill A. Shutemov
2020-07-10 14:23 ` Alex Shi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200710125623.GH27672@8bytes.org \
--to=joro@8bytes.org \
--cc=alex.shi@linux.alibaba.com \
--cc=hannes@cmpxchg.org \
--cc=hughd@google.com \
--cc=iommu@lists.linux-foundation.org \
--cc=kirill@shutemov.name \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mika.penttila@nextfour.com \
--cc=robin.murphy@arm.com \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.