All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Dietrich <stettberger@dokucode.de>
To: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Cc: Mike Rapoport <rppt@kernel.org>,
	Yannick Loeck <yannick.loeck@tuhh.de>,
	kernel@maxfragg.de
Subject: vm_insert_pages, missing #define pte_index
Date: Fri, 17 Dec 2021 23:32:51 +0100	[thread overview]
Message-ID: <s7b7dc2ooa4.fsf@dokucode.de> (raw)

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:  christian.dietrich@tuhh.de
Tel:    +49 40 42878 2188
WWW:    https://osg.tuhh.de/


                 reply	other threads:[~2021-12-17 23:01 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s7b7dc2ooa4.fsf@dokucode.de \
    --to=stettberger@dokucode.de \
    --cc=akpm@linux-foundation.org \
    --cc=kernel@maxfragg.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rppt@kernel.org \
    --cc=yannick.loeck@tuhh.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.