linux-kernel - Re: [PATCH v1 03/29] mm/zsmalloc: drop PageIsolated() related VM_BUG

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: <411b94bb-4662-4357-86a3-05478f2f8c8f@redhat.com>
Date: Tue, 1 Jul 2025 10:03:57 +0200
From: David Hildenbrand <david@...hat.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 linux-doc@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
 virtualization@...ts.linux.dev, linux-fsdevel@...r.kernel.org,
 Andrew Morton <akpm@...ux-foundation.org>, Jonathan Corbet <corbet@....net>,
 Madhavan Srinivasan <maddy@...ux.ibm.com>,
 Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>,
 Christophe Leroy <christophe.leroy@...roup.eu>,
 Jerrin Shaji George <jerrin.shaji-george@...adcom.com>,
 Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Michael S. Tsirkin" <mst@...hat.com>, Jason Wang <jasowang@...hat.com>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez
 <eperezma@...hat.com>, Alexander Viro <viro@...iv.linux.org.uk>,
 Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>,
 Zi Yan <ziy@...dia.com>, Matthew Brost <matthew.brost@...el.com>,
 Joshua Hahn <joshua.hahnjy@...il.com>, Rakie Kim <rakie.kim@...com>,
 Byungchul Park <byungchul@...com>, Gregory Price <gourry@...rry.net>,
 Ying Huang <ying.huang@...ux.alibaba.com>,
 Alistair Popple <apopple@...dia.com>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>, Vlastimil Babka
 <vbabka@...e.cz>, Mike Rapoport <rppt@...nel.org>,
 Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
 "Matthew Wilcox (Oracle)" <willy@...radead.org>,
 Minchan Kim <minchan@...nel.org>,
 Sergey Senozhatsky <senozhatsky@...omium.org>,
 Brendan Jackman <jackmanb@...gle.com>, Johannes Weiner <hannes@...xchg.org>,
 Jason Gunthorpe <jgg@...pe.ca>, John Hubbard <jhubbard@...dia.com>,
 Peter Xu <peterx@...hat.com>, Xu Xin <xu.xin16@....com.cn>,
 Chengming Zhou <chengming.zhou@...ux.dev>, Miaohe Lin
 <linmiaohe@...wei.com>, Naoya Horiguchi <nao.horiguchi@...il.com>,
 Oscar Salvador <osalvador@...e.de>, Rik van Riel <riel@...riel.com>,
 Harry Yoo <harry.yoo@...cle.com>, Qi Zheng <zhengqi.arch@...edance.com>,
 Shakeel Butt <shakeel.butt@...ux.dev>
Subject: Re: [PATCH v1 03/29] mm/zsmalloc: drop PageIsolated() related
 VM_BUG_ONs

On 30.06.25 17:17, Lorenzo Stoakes wrote:
> On Mon, Jun 30, 2025 at 02:59:44PM +0200, David Hildenbrand wrote:
>> Let's drop these checks; these are conditions the core migration code
>> must make sure will hold either way, no need to double check.
>>
>> Acked-by: Zi Yan <ziy@...dia.com>
>> Reviewed-by: Sergey Senozhatsky <senozhatsky@...omium.org>
>> Acked-by: Harry Yoo <harry.yoo@...cle.com>
>> Signed-off-by: David Hildenbrand <david@...hat.com>
> 
> LGTM, one comment below.
> 
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
> 

Thanks!

>> ---
>>   mm/zpdesc.h   | 5 -----
>>   mm/zsmalloc.c | 5 -----
>>   2 files changed, 10 deletions(-)
>>
>> diff --git a/mm/zpdesc.h b/mm/zpdesc.h
>> index d3df316e5bb7b..5cb7e3de43952 100644
>> --- a/mm/zpdesc.h
>> +++ b/mm/zpdesc.h
>> @@ -168,11 +168,6 @@ static inline void __zpdesc_clear_zsmalloc(struct zpdesc *zpdesc)
>>   	__ClearPageZsmalloc(zpdesc_page(zpdesc));
>>   }
>>
>> -static inline bool zpdesc_is_isolated(struct zpdesc *zpdesc)
>> -{
>> -	return PageIsolated(zpdesc_page(zpdesc));
>> -}
>> -
>>   static inline struct zone *zpdesc_zone(struct zpdesc *zpdesc)
>>   {
>>   	return page_zone(zpdesc_page(zpdesc));
>> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
>> index 999b513c7fdff..7f1431f2be98f 100644
>> --- a/mm/zsmalloc.c
>> +++ b/mm/zsmalloc.c
>> @@ -1719,8 +1719,6 @@ static bool zs_page_isolate(struct page *page, isolate_mode_t mode)
>>   	 * Page is locked so zspage couldn't be destroyed. For detail, look at
>>   	 * lock_zspage in free_zspage.
>>   	 */
>> -	VM_BUG_ON_PAGE(PageIsolated(page), page);
>> -
>>   	return true;
>>   }
>>
>> @@ -1739,8 +1737,6 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
>>   	unsigned long old_obj, new_obj;
>>   	unsigned int obj_idx;
>>
>> -	VM_BUG_ON_PAGE(!zpdesc_is_isolated(zpdesc), zpdesc_page(zpdesc));
>> -
>>   	/* The page is locked, so this pointer must remain valid */
>>   	zspage = get_zspage(zpdesc);
>>   	pool = zspage->pool;
>> @@ -1811,7 +1807,6 @@ static int zs_page_migrate(struct page *newpage, struct page *page,
>>
>>   static void zs_page_putback(struct page *page)
>>   {
>> -	VM_BUG_ON_PAGE(!PageIsolated(page), page);
>>   }
> 
> Can we just drop zs_page_putback from movable_operations() now this is empty?

Common code expects there to be a callback, and I don't want to change 
that. Long-term I assume it will rather indicate a BUG if there is no 
putback handler, not something we want to encourage.

Likely, once we rework that isolated pages cannot get freed here, we'd 
have to handle stuff on the putback path (realize that the page can be 
freed and free it) -- TODO for that is added in #12.

-- 
Cheers,

David / dhildenb