Re: [PATCH v2 2/3] mm: drop pte_clear_not_present_full()
"David Hildenbrand (Arm)" <[email protected]> Mon, 29 Jun 2026 19:43:53 +0200
| Newsgroups | org.kernel.vger.sparclinux,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On 6/29/26 19:22, Andrew Morton wrote: > On Mon, 29 Jun 2026 15:49:48 +0200 "David Hildenbrand (Arm)" <[email protected]> wrote: > >> In general, there is no good reason to do anything special when clearing >> non-present PTEs. >> >> In theory, HW that does have to invalidate TLBs for non-present PTEs could >> benefit from a "full" parameter, but fortunately >> pte_clear_not_present_full() is not wired up anymore ... and there would >> have to be something very convincing for us to care about that to re-add >> it. >> >> So, let's just use pte_clear() directly now. To avoid the compiler >> complaining on some configs about unused "addr" parameter, silence that >> here. > > Wait, which configs do that? > >> @@ -1022,8 +1007,10 @@ static inline void pte_clear_not_present_full(struct mm_struct *mm, >> static inline void clear_not_present_full_ptes(struct mm_struct *mm, >> unsigned long addr, pte_t *ptep, unsigned int nr, int full) >> { >> + (void)addr; >> + > > We heavily rely on this warning not happening. > > eg, one of thousands: > > static inline bool page_range_contiguous(const struct page *page, > unsigned long nr_pages) > { > return true; > } > > So... what's happening here? "nr_pages" are not modified in the function, so the compile does not complain. See below. A private build bot barked at me after v1 for arm-linux-gnueabi-gcc openrisc-allnoconfig um-allmodconfig For example: https://lore.kernel.org/all/[email protected]/ All errors (new ones prefixed by >>): In file included from include/linux/kasan.h:38, from include/linux/slab.h:264, from lib/test_bitops.c:12: include/linux/pgtable.h: In function 'clear_not_present_full_ptes': >> include/linux/pgtable.h:974:31: error: parameter 'addr' set but not used [-Werror=unused-but-set-parameter=] 974 | unsigned long addr, pte_t *ptep, unsigned int nr, int full) | ~~~~~~~~~~~~~~^~~~ cc1: all warnings being treated as errors The problem is that addr is updated (written) in the function but never read. This becomes visible once pte_clear() is a macro instead of a function. arch/arm/include/asm/pgtable.h:#define pte_clear(mm,addr,ptep) set_pte_ext(ptep, __pte(0), 0) "(void) addr" silences bots. An alternative would be to find all such macros and convert them into (assuming inline function is non-trivial) #define pte_clear(mm,addr,ptep) ((void)addr, set_pte_ext(ptep, __pte(0), 0)) Something I wanted to avoid for this simple patch here that just removes one function indirection. -- Cheers, David