public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
* [bug report] proc: rewrite stable_page_flags()
@ 2024-04-03  9:01 Dan Carpenter
  2024-04-05 19:23 ` [FIX] " Matthew Wilcox
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2024-04-03  9:01 UTC (permalink / raw)
  To: willy; +Cc: linux-fsdevel

Hello Matthew Wilcox (Oracle),

Commit ea1be2228bb6 ("proc: rewrite stable_page_flags()") from Mar
26, 2024 (linux-next), leads to the following Smatch static checker
warning:

fs/proc/page.c:156 stable_page_flags() warn: bit shifter 'PG_lru' used for logical '&'
fs/proc/page.c:207 stable_page_flags() warn: bit shifter 'KPF_HUGE' used for logical '&'

fs/proc/page.c
    110 u64 stable_page_flags(const struct page *page)
    111 {
    112         const struct folio *folio;
    113         unsigned long k;
    114         unsigned long mapping;
    115         bool is_anon;
    116         u64 u = 0;
    117 
    118         /*
    119          * pseudo flag: KPF_NOPAGE
    120          * it differentiates a memory hole from a page with no flags
    121          */
    122         if (!page)
    123                 return 1 << KPF_NOPAGE;
    124         folio = page_folio(page);
    125 
    126         k = folio->flags;
    127         mapping = (unsigned long)folio->mapping;
    128         is_anon = mapping & PAGE_MAPPING_ANON;
    129 
    130         /*
    131          * pseudo flags for the well known (anonymous) memory mapped pages
    132          */
    133         if (page_mapped(page))
    134                 u |= 1 << KPF_MMAP;
    135         if (is_anon) {
    136                 u |= 1 << KPF_ANON;
    137                 if (mapping & PAGE_MAPPING_KSM)
    138                         u |= 1 << KPF_KSM;
    139         }
    140 
    141         /*
    142          * compound pages: export both head/tail info
    143          * they together define a compound page's start/end pos and order
    144          */
    145         if (page == &folio->page)
    146                 u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
    147         else
    148                 u |= 1 << KPF_COMPOUND_TAIL;
    149         if (folio_test_hugetlb(folio))
    150                 u |= 1 << KPF_HUGE;
                             ^^^^^^^^^^^^^
Here KPF_HUGE is a shifter

    151         /*
    152          * We need to check PageLRU/PageAnon
    153          * to make sure a given page is a thp, not a non-huge compound page.
    154          */
    155         else if (folio_test_large(folio)) {
--> 156                 if ((k & PG_lru) || is_anon)
                             ^^^^^^^^^^
The PG_lru enum isn't used consistently.  Should this be?:

	if ((k & (1 << PG_lru)) || ...


    157                         u |= 1 << KPF_THP;
    158                 else if (is_huge_zero_folio(folio)) {
    159                         u |= 1 << KPF_ZERO_PAGE;
    160                         u |= 1 << KPF_THP;
    161                 }
    162         } else if (is_zero_pfn(page_to_pfn(page)))
    163                 u |= 1 << KPF_ZERO_PAGE;
    164 
    165         /*
    166          * Caveats on high order pages: PG_buddy and PG_slab will only be set
    167          * on the head page.
    168          */
    169         if (PageBuddy(page))
    170                 u |= 1 << KPF_BUDDY;
    171         else if (page_count(page) == 0 && is_free_buddy_page(page))
    172                 u |= 1 << KPF_BUDDY;
    173 
    174         if (PageOffline(page))
    175                 u |= 1 << KPF_OFFLINE;
    176         if (PageTable(page))
    177                 u |= 1 << KPF_PGTABLE;
    178 
    179 #if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
    180         u |= kpf_copy_bit(k, KPF_IDLE,          PG_idle);
    181 #else
    182         if (folio_test_idle(folio))
    183                 u |= 1 << KPF_IDLE;
    184 #endif
    185 
    186         u |= kpf_copy_bit(k, KPF_LOCKED,        PG_locked);
    187         u |= kpf_copy_bit(k, KPF_SLAB,                PG_slab);
    188         u |= kpf_copy_bit(k, KPF_ERROR,                PG_error);
    189         u |= kpf_copy_bit(k, KPF_DIRTY,                PG_dirty);
    190         u |= kpf_copy_bit(k, KPF_UPTODATE,        PG_uptodate);
    191         u |= kpf_copy_bit(k, KPF_WRITEBACK,        PG_writeback);
    192 
    193         u |= kpf_copy_bit(k, KPF_LRU,                PG_lru);
    194         u |= kpf_copy_bit(k, KPF_REFERENCED,        PG_referenced);
    195         u |= kpf_copy_bit(k, KPF_ACTIVE,        PG_active);
    196         u |= kpf_copy_bit(k, KPF_RECLAIM,        PG_reclaim);
    197 
    198 #define SWAPCACHE ((1 << PG_swapbacked) | (1 << PG_swapcache))
    199         if ((k & SWAPCACHE) == SWAPCACHE)
    200                 u |= 1 << KPF_SWAPCACHE;
    201         u |= kpf_copy_bit(k, KPF_SWAPBACKED,        PG_swapbacked);
    202 
    203         u |= kpf_copy_bit(k, KPF_UNEVICTABLE,        PG_unevictable);
    204         u |= kpf_copy_bit(k, KPF_MLOCKED,        PG_mlocked);
    205 
    206 #ifdef CONFIG_MEMORY_FAILURE
    207         if (u & KPF_HUGE)
                    ^^^^^^^^^^^^
Here it is a mask.

    208                 u |= kpf_copy_bit(k, KPF_HWPOISON,        PG_hwpoison);
    209         else
    210                 u |= kpf_copy_bit(page->flags, KPF_HWPOISON,        PG_hwpoison);
    211 #endif
    212 
    213 #ifdef CONFIG_ARCH_USES_PG_UNCACHED
    214         u |= kpf_copy_bit(k, KPF_UNCACHED,        PG_uncached);
    215 #endif
    216 
    217         u |= kpf_copy_bit(k, KPF_RESERVED,        PG_reserved);
    218         u |= kpf_copy_bit(k, KPF_MAPPEDTODISK,        PG_mappedtodisk);
    219         u |= kpf_copy_bit(k, KPF_PRIVATE,        PG_private);
    220         u |= kpf_copy_bit(k, KPF_PRIVATE_2,        PG_private_2);
    221         u |= kpf_copy_bit(k, KPF_OWNER_PRIVATE,        PG_owner_priv_1);
    222         u |= kpf_copy_bit(k, KPF_ARCH,                PG_arch_1);
    223 #ifdef CONFIG_ARCH_USES_PG_ARCH_X
    224         u |= kpf_copy_bit(k, KPF_ARCH_2,        PG_arch_2);
    225         u |= kpf_copy_bit(k, KPF_ARCH_3,        PG_arch_3);
    226 #endif
    227 
    228         return u;
    229 };

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [FIX] proc: rewrite stable_page_flags()
  2024-04-03  9:01 [bug report] proc: rewrite stable_page_flags() Dan Carpenter
@ 2024-04-05 19:23 ` Matthew Wilcox
  0 siblings, 0 replies; 2+ messages in thread
From: Matthew Wilcox @ 2024-04-05 19:23 UTC (permalink / raw)
  To: Andrew Morton, Dan Carpenter; +Cc: linux-fsdevel

On Wed, Apr 03, 2024 at 12:01:35PM +0300, Dan Carpenter wrote:
> Hello Matthew Wilcox (Oracle),
> 
> Commit ea1be2228bb6 ("proc: rewrite stable_page_flags()") from Mar
> 26, 2024 (linux-next), leads to the following Smatch static checker
> warning:
> 
> fs/proc/page.c:156 stable_page_flags() warn: bit shifter 'PG_lru' used for logical '&'
> fs/proc/page.c:207 stable_page_flags() warn: bit shifter 'KPF_HUGE' used for logical '&'

Thanks.  I thought I sent this out on Tuesday, but it doesn't seem to
have left my machine.  Andrew, can you add this -fix patch?

diff --git a/fs/proc/page.c b/fs/proc/page.c
index 5bc82828c6aa..55b01535eb22 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -175,6 +175,8 @@ u64 stable_page_flags(const struct page *page)
 		u |= 1 << KPF_OFFLINE;
 	if (PageTable(page))
 		u |= 1 << KPF_PGTABLE;
+	if (folio_test_slab(folio))
+		u |= 1 << KPF_SLAB;
 
 #if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
 	u |= kpf_copy_bit(k, KPF_IDLE,          PG_idle);
@@ -184,7 +186,6 @@ u64 stable_page_flags(const struct page *page)
 #endif
 
 	u |= kpf_copy_bit(k, KPF_LOCKED,	PG_locked);
-	u |= kpf_copy_bit(k, KPF_SLAB,		PG_slab);
 	u |= kpf_copy_bit(k, KPF_ERROR,		PG_error);
 	u |= kpf_copy_bit(k, KPF_DIRTY,		PG_dirty);
 	u |= kpf_copy_bit(k, KPF_UPTODATE,	PG_uptodate);
diff --git a/tools/cgroup/memcg_slabinfo.py b/tools/cgroup/memcg_slabinfo.py
index 1d3a90d93fe2..270c28a0d098 100644
--- a/tools/cgroup/memcg_slabinfo.py
+++ b/tools/cgroup/memcg_slabinfo.py
@@ -146,12 +146,11 @@ def detect_kernel_config():
 
 
 def for_each_slab(prog):
-    PGSlab = 1 << prog.constant('PG_slab')
-    PGHead = 1 << prog.constant('PG_head')
+    PGSlab = ~prog.constant('PG_slab')
 
     for page in for_each_page(prog):
         try:
-            if page.flags.value_() & PGSlab:
+            if page.page_type.value_() == PGSlab:
                 yield cast('struct slab *', page)
         except FaultError:
             pass

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-04-05 19:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-03  9:01 [bug report] proc: rewrite stable_page_flags() Dan Carpenter
2024-04-05 19:23 ` [FIX] " Matthew Wilcox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox