Re: [PATCH net-next v9 10/13] mm: page_frag: introduce prepare/probe/commit API

[Yunsheng Lin <yunshenglin0825@gmail.com>]

On 6/30/2024 1:37 AM, Alexander Duyck wrote:
> On Sat, Jun 29, 2024 at 4:15 AM Yunsheng Lin <linyunsheng@huawei.com> wrote:

...

>>>
>>> Why is this a macro instead of just being an inline? Are you trying to
>>> avoid having to include a header due to the virt_to_page?
>>
>> Yes, you are right.
>> I tried including different headers for virt_to_page(), and it did not
>> work for arch/x86/kernel/asm-offsets.s, which has included linux/sched.h,
>> and linux/sched.h need 'struct page_frag_cache' for 'struct task_struct'
>> after this patchset, including page_frag_cache.h for sched.h causes the
>> below compiler error:
>>
>>    CC      arch/x86/kernel/asm-offsets.s
>> In file included from ./arch/x86/include/asm/page.h:89,
>>                   from ./arch/x86/include/asm/thread_info.h:12,
>>                   from ./include/linux/thread_info.h:60,
>>                   from ./include/linux/spinlock.h:60,
>>                   from ./include/linux/swait.h:7,
>>                   from ./include/linux/completion.h:12,
>>                   from ./include/linux/crypto.h:15,
>>                   from arch/x86/kernel/asm-offsets.c:9:
>> ./include/linux/page_frag_cache.h: In function ‘page_frag_alloc_align’:
>> ./include/asm-generic/memory_model.h:37:34: error: ‘vmemmap’ undeclared (first use in this function); did you mean ‘mem_map’?
>>     37 | #define __pfn_to_page(pfn)      (vmemmap + (pfn))
>>        |                                  ^~~~~~~
>> ./include/asm-generic/memory_model.h:65:21: note: in expansion of macro ‘__pfn_to_page’
>>     65 | #define pfn_to_page __pfn_to_page
>>        |                     ^~~~~~~~~~~~~
>> ./arch/x86/include/asm/page.h:68:33: note: in expansion of macro ‘pfn_to_page’
>>     68 | #define virt_to_page(kaddr)     pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
>>        |                                 ^~~~~~~~~~~
>> ./include/linux/page_frag_cache.h:151:16: note: in expansion of macro ‘virt_to_page’
>>    151 |         return virt_to_page(va);
>>        |                ^~~~~~~~~~~~
>> ./include/asm-generic/memory_model.h:37:34: note: each undeclared identifier is reported only once for each function it appears in
>>     37 | #define __pfn_to_page(pfn)      (vmemmap + (pfn))
>>        |                                  ^~~~~~~
>> ./include/asm-generic/memory_model.h:65:21: note: in expansion of macro ‘__pfn_to_page’
>>     65 | #define pfn_to_page __pfn_to_page
>>        |                     ^~~~~~~~~~~~~
>> ./arch/x86/include/asm/page.h:68:33: note: in expansion of macro ‘pfn_to_page’
>>     68 | #define virt_to_page(kaddr)     pfn_to_page(__pa(kaddr) >> PAGE_SHIFT)
>>        |                                 ^~~~~~~~~~~
>> ./include/linux/page_frag_cache.h:151:16: note: in expansion of macro ‘virt_to_page’
>>    151 |         return virt_to_page(va);
>>
>>
> 
> I am pretty sure you just need to add:
> #include <asm/page.h>

I am supposing you mean adding the above to page_frag_cache.h, right?

It seems thing is more complicated for SPARSEMEM_VMEMMAP case, as it 
needs the declaration of 'vmemmap'(some arch defines it as a pointer 
variable while some arch defines it as a macro) and the definition of 
'struct page' for '(vmemmap + (pfn))' operation.

Adding below for 'vmemmap' and 'struct page' seems to have some compiler 
error caused by interdependence between linux/mm_types.h and asm/pgtable.h:
#include <asm/pgtable.h>
#include <linux/mm_types.h>

As below, asm/pgtable.h obviously need the definition of 'struct 
mm_struct' from linux/mm_types.h, and linux/mm_types.h has some
a long dependency of asm/pgtable.h starting from linux/uprobes.h
if we add '#include <asm/pgtable.h>' in linux/page_frag_cache.h:

In file included from ./include/linux/page_frag_cache.h:8,
                  from ./include/linux/sched.h:49,
                  from ./include/linux/percpu.h:13,
                  from ./arch/x86/include/asm/msr.h:15,
                  from ./arch/x86/include/asm/tsc.h:10,
                  from ./arch/x86/include/asm/timex.h:6,
                  from ./include/linux/timex.h:67,
                  from ./include/linux/time32.h:13,
                  from ./include/linux/time.h:60,
                  from ./include/linux/jiffies.h:10,
                  from ./include/linux/ktime.h:25,
                  from ./include/linux/timer.h:6,
                  from ./include/linux/workqueue.h:9,
                  from ./include/linux/srcu.h:21,
                  from ./include/linux/notifier.h:16,
                  from ./arch/x86/include/asm/uprobes.h:13,
                  from ./include/linux/uprobes.h:49,
                  from ./include/linux/mm_types.h:16,
                  from ./include/linux/mmzone.h:22,
                  from ./include/linux/gfp.h:7,
                  from ./include/linux/slab.h:16,
                  from ./include/linux/crypto.h:17,
                  from arch/x86/kernel/asm-offsets.c:9:
./arch/x86/include/asm/pgtable.h: In function ‘pte_accessible’:
./arch/x86/include/asm/pgtable.h:970:40: error: invalid use of undefined 
type ‘struct mm_struct’
   970 |                         atomic_read(&mm->tlb_flush_pending))
       |                                        ^~
./arch/x86/include/asm/pgtable.h: In function ‘pmdp_establish’:
./arch/x86/include/asm/pgtable.h:1370:37: error: invalid use of 
undefined type ‘struct vm_area_struct’
  1370 |         page_table_check_pmd_set(vma->vm_mm, pmdp, pmd);
       |                                     ^~
./arch/x86/include/asm/pgtable.h: At top level:
./arch/x86/include/asm/pgtable.h:1495:50: error: ‘struct vm_fault’ 
declared inside parameter list will not be visible outside of this 
definition or declaration [-Werror]
  1495 | static inline void update_mmu_cache_range(struct vm_fault *vmf,
       |                                                  ^~~~~~~~
In file included from ./arch/x86/include/asm/page.h:89,
                  from ./arch/x86/include/asm/thread_info.h:12,
                  from ./include/linux/thread_info.h:60,
                  from ./include/linux/spinlock.h:60,
                  from ./include/linux/swait.h:7,
                  from ./include/linux/completion.h:12,
                  from ./include/linux/crypto.h:15,
                  from arch/x86/kernel/asm-offsets.c:9:
./include/linux/page_frag_cache.h: In function ‘page_frag_alloc_probe’:
./include/asm-generic/memory_model.h:37:42: error: invalid use of 
undefined type ‘struct page’
    37 | #define __pfn_to_page(pfn)      (vmemmap + (pfn))
       |                                          ^
./include/asm-generic/memory_model.h:65:21: note: in expansion of macro 
‘__pfn_to_page’
    65 | #define pfn_to_page __pfn_to_page
       |                     ^~~~~~~~~~~~~
./arch/x86/include/asm/page.h:68:33: note: in expansion of macro 
‘pfn_to_page’
    68 | #define virt_to_page(kaddr)     pfn_to_page(__pa(kaddr) >> 
PAGE_SHIFT)
       |                                 ^~~~~~~~~~~
./include/linux/page_frag_cache.h:225:16: note: in expansion of macro 
‘virt_to_page’
   225 |         return virt_to_page(encoded_va);
       |                ^~~~~~~~~~~~
cc1: all warnings being treated as errors

>