[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <15623dac-9358-4597-b3ee-3694a5956920@gmail.com>
Date: Sun, 30 Jun 2024 23:05:34 +0800
From: Yunsheng Lin <yunshenglin0825@...il.com>
To: Alexander Duyck <alexander.duyck@...il.com>
Cc: Yunsheng Lin <linyunsheng@...wei.com>, davem@...emloft.net,
kuba@...nel.org, pabeni@...hat.com, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>,
linux-mm@...ck.org
Subject: Re: [PATCH net-next v9 10/13] mm: page_frag: introduce
prepare/probe/commit API
On 6/30/2024 10:35 PM, Alexander Duyck wrote:
> On Sun, Jun 30, 2024 at 7:05 AM Yunsheng Lin <yunshenglin0825@...il.com> wrote:
>>
>> On 6/30/2024 1:37 AM, Alexander Duyck wrote:
>>> On Sat, Jun 29, 2024 at 4:15 AM Yunsheng Lin <linyunsheng@...wei.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 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.
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);
| ^~
> 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.
Powered by blists - more mailing lists