2021-12-17 23:01:02

by Christian Dietrich

[permalink] [raw]
Subject: vm_insert_pages, missing #define pte_index

Hello Everyone!

I tried to understand the mm/memory.c and the function
`vm_insert_pages', and perhaps I found a CPP-related bug there.

Roughly, the function looks like this:

int vm_insert_pages(...)
{
#ifdef pte_index
[.....]
[// insert pages amortizes spinlock acquisitions]
return insert_pages(vma, addr, pages, num, vma->vm_page_prot);
#else
for (; idx < pgcount; ++idx) {
[[ // Takes one lock per PTE ]]
err = vm_insert_page(vma, addr + (PAGE_SIZE * idx), pages[idx]);
[....]
}
#endif /* ifdef pte_index */
}

So, you surely want to have the upper variant as it performs less
spinlock acquisitions. Assume you would use the lower one, everything
would be fine and you would not notice it right from the start.

With 974b9b2c68f3d35a65e80af9657fe378d2439b60 (Jun 2020), Mike Rapoport
moved all arch-specific pte_index defintions from arch/ to
include/linux/pgtable.h and transformed it to an inline function. Which
is a good thing, but the preprocessor does not know about inline
functions.... They do not define CPP macros.

Therefore, I think, we always use the lower variant. When compiling an
x86 Linux and adding an #error to the upper CPP block, I don't get a
preprocessor error and my source code (I'm looking at next-20211015)
does not contain any define for pte_index.

This is probably not as intented.

chris
--
Prof. Dr.-Ing. Christian Dietrich
Operating System Group (E-EXK4)
Technische Universität Hamburg
Am Schwarzenberg-Campus 3 (E), 4.092
21073 Hamburg

eMail: [email protected]
Tel: +49 40 42878 2188
WWW: https://osg.tuhh.de/