| Message ID | 1663126621-26926-1-git-send-email-zhaoyang.huang@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] mm: fix logic error of page_expected_state | expand |
On Wed, Sep 14, 2022 at 11:37 AM zhaoyang.huang <zhaoyang.huang@unisoc.com> wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > The page with special page type will be deemed as bad page wrongly since > type share the same address with mapcount. > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > --- > include/linux/page-flags.h | 4 ++-- > mm/page_alloc.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index e66f7aa..5d3274b 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -946,9 +946,9 @@ static inline bool is_page_hwpoison(struct page *page) > #define PageType(page, flag) \ > ((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE) > > -static inline int page_has_type(struct page *page) > +static inline bool page_has_type(struct page *page) > { > - return (int)page->page_type < PAGE_MAPCOUNT_RESERVE; > + return (int)page->page_type < PAGE_MAPCOUNT_RESERVE; amend the type conversion > } > > #define PAGE_TYPE_OPS(uname, lname) \ > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index e008a3d..3714680 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1164,7 +1164,7 @@ int split_free_page(struct page *free_page, > static inline bool page_expected_state(struct page *page, > unsigned long check_flags) > { > - if (unlikely(atomic_read(&page->_mapcount) != -1)) > + if (unlikely(!page_has_type(page) && atomic_read(&page->_mapcount) != -1)) > return false; > > if (unlikely((unsigned long)page->mapping | > -- > 1.9.1 >
On Wed, Sep 14, 2022 at 11:37:00AM +0800, zhaoyang.huang wrote: > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > The page with special page type will be deemed as bad page wrongly since > type share the same address with mapcount. That's not wrongly. You didn't clear the bit. I told you you would need to do that in the first version of the patch you sent.
On Wed, Sep 14, 2022 at 3:35 PM Matthew Wilcox <willy@infradead.org> wrote: > > On Wed, Sep 14, 2022 at 11:37:00AM +0800, zhaoyang.huang wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > > > The page with special page type will be deemed as bad page wrongly since > > type share the same address with mapcount. > > That's not wrongly. You didn't clear the bit. I told you you would > need to do that in the first version of the patch you sent. Yes, I have cleared the PG_trackleak since v2 as you suggested. However, IMHO, the page which has page type should be skipped for mapcount check. The present problem is there is no invoking of free_pages_prepare within the chain of drain_pages_zone->free_pcppages_bulk, special pages with page->type(PG_guard and PG_trackleak etc) will be leaked as bulkfree_pcp_prepare will deem it as bad by checking the mapcount.
Nack here and will be updated with trackleak if necessary. On Wed, Sep 14, 2022 at 11:37 AM zhaoyang.huang <zhaoyang.huang@unisoc.com> wrote: > > From: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > > The page with special page type will be deemed as bad page wrongly since > type share the same address with mapcount. > > Signed-off-by: Zhaoyang Huang <zhaoyang.huang@unisoc.com> > --- > include/linux/page-flags.h | 4 ++-- > mm/page_alloc.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h > index e66f7aa..5d3274b 100644 > --- a/include/linux/page-flags.h > +++ b/include/linux/page-flags.h > @@ -946,9 +946,9 @@ static inline bool is_page_hwpoison(struct page *page) > #define PageType(page, flag) \ > ((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE) > > -static inline int page_has_type(struct page *page) > +static inline bool page_has_type(struct page *page) > { > - return (int)page->page_type < PAGE_MAPCOUNT_RESERVE; > + return page->page_type < PAGE_MAPCOUNT_RESERVE; > } > > #define PAGE_TYPE_OPS(uname, lname) \ > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index e008a3d..3714680 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -1164,7 +1164,7 @@ int split_free_page(struct page *free_page, > static inline bool page_expected_state(struct page *page, > unsigned long check_flags) > { > - if (unlikely(atomic_read(&page->_mapcount) != -1)) > + if (unlikely(!page_has_type(page) && atomic_read(&page->_mapcount) != -1)) > return false; > > if (unlikely((unsigned long)page->mapping | > -- > 1.9.1 >
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index e66f7aa..5d3274b 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -946,9 +946,9 @@ static inline bool is_page_hwpoison(struct page *page) #define PageType(page, flag) \ ((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE) -static inline int page_has_type(struct page *page) +static inline bool page_has_type(struct page *page) { - return (int)page->page_type < PAGE_MAPCOUNT_RESERVE; + return page->page_type < PAGE_MAPCOUNT_RESERVE; } #define PAGE_TYPE_OPS(uname, lname) \ diff --git a/mm/page_alloc.c b/mm/page_alloc.c index e008a3d..3714680 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -1164,7 +1164,7 @@ int split_free_page(struct page *free_page, static inline bool page_expected_state(struct page *page, unsigned long check_flags) { - if (unlikely(atomic_read(&page->_mapcount) != -1)) + if (unlikely(!page_has_type(page) && atomic_read(&page->_mapcount) != -1)) return false; if (unlikely((unsigned long)page->mapping |