On 2024/6/30 23:05, Yunsheng Lin wrote:

On 6/30/2024 10:35 PM, Alexander Duyck wrote:

quoted

On Sun, Jun 30, 2024 at 7:05 AM Yunsheng Lin [off-list ref] wrote:

quoted

On 6/30/2024 1:37 AM, Alexander Duyck wrote:

quoted

On Sat, Jun 29, 2024 at 4:15 AM Yunsheng Lin [off-list ref] wrote:

...

quoted

quoted

quoted

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.

...

quoted

quoted

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>

Maybe you should just include linux/mm.h as that should have all the
necessary includes to handle these cases. In any case though it

Including linux/mm.h seems to have similar compiler error, just the
interdependence is between linux/mm_types.h and linux/mm.h now.

How about splitting page_frag_cache.h into page_frag_types.h and
page_frag_cache.h mirroring the above linux/mm_types.h and linux/mm.h
to fix the compiler error?

As below, linux/mmap_lock.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 linux/mm.h starting from
linux/uprobes.h if we add '#include <linux/mm.h>' in linux/page_frag_cache.h:

In file included from ./include/linux/mm.h:16,
                 from ./include/linux/page_frag_cache.h:6,
                 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:
./include/linux/mmap_lock.h: In function ‘mmap_assert_locked’:
./include/linux/mmap_lock.h:65:30: error: invalid use of undefined type ‘const struct mm_struct’
   65 |         rwsem_assert_held(&mm->mmap_lock);
      |                              ^~

quoted

doesn't make any sense to have a define in one include that expects
the user to then figure out what other headers to include in order to
make the define work they should be included in the header itself to
avoid any sort of weird dependencies.

Perhaps there are some season why there are two headers for the mm subsystem, linux/mm_types.h and linux/mm.h?
And .h file is supposed to include the linux/mm_types.h while .c file
is supposed to include the linux/mm.h?
If the above is correct, it seems the above rule is broked by including linux/mm.h in linux/page_frag_cache.h.
.