public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@redhat.com>
To: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Ingo Molnar <mingo@elte.hu>,
	Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	LKML <linux-kernel@vger.kernel.org>,
	systemtap-ml <systemtap@sources.redhat.com>
Subject: Re: [BUGFIX][PATCH -tip] x86: fix text_poke to handle highmem pages
Date: Mon, 06 Apr 2009 16:23:18 -0400	[thread overview]
Message-ID: <49DA64B6.2050809@redhat.com> (raw)
In-Reply-To: <20090406175807.GD31867@Krystal>



Mathieu Desnoyers wrote:
> * Masami Hiramatsu (mhiramat@redhat.com) wrote:
>> Mathieu Desnoyers wrote:
>>> * Masami Hiramatsu (mhiramat@redhat.com) wrote:
>>>> Fix a bug in text_poke to handle highmem pages, because module
>>>> text pages are possible to be highmem pages on x86-32.
>>>> In that case, since fixmap can't handle those pages, text_poke
>>>> uses kmap_atomic.
>>>>
>>> Hrm, can you remind me what would be the downside of using kmap_atomic
>>> in every scenarios (highmem and non-highmem) then ?
>> kmap_atomic can handle only highmem pages. If you passes lowmem pages,
>> it returns just original vaddr of it. (because kmap is only for
>> highmem support)
>>
> 
> OK, and if we are doing a second kmap_atomic() of a module text page
> which is already mapped, does the second kmap_atomic return the vaddr of
> the page originally mapped or is it creating a second mapping ?

Good point, kmap_atomic creates a second mapping if the page is highmem.

in arch/x86/mm/highmem_32.c:

void *kmap_atomic_prot(struct page *page, enum km_type type, pgprot_t prot)
{
	...
        if (!PageHighMem(page))
                return page_address(page);
	...
}

> Because if we ever decide to enforce read-only page mapping for module
> text pages, touching highmem pages too, we will run into real trouble if
> those happen to be the same page.

So, as long as the page is in highmem, we can create a second mapping by
using kmap_atomic.

Thank you,

> 
> Mathieu
> 
>>> I would try to avoid "special cases" as much as possible, because they
>>> just make problems harder to reproduce.
>> Actually, this bug is a special case because it happens only on PAE kernel.
>>
>>> The idea would be to either add fixmap highmem support, or to simply use
>>> kmap_atomic() for all cases until we add fixmap highmem support.
>>>
>>
>> Thank you,
>>
>> -- 
>> Masami Hiramatsu
>>
>> Software Engineer
>> Hitachi Computer Products (America) Inc.
>> Software Solutions Division
>>
>> e-mail: mhiramat@redhat.com
>>
> 

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com


  reply	other threads:[~2009-04-06 20:22 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-04 14:34 [BUG][-tip] kprobes on module functions hits kernel BUG in text_poke on x86-32 Masami Hiramatsu
2009-04-04 15:42 ` Mathieu Desnoyers
2009-04-04 18:28   ` Masami Hiramatsu
2009-04-04 19:04     ` Masami Hiramatsu
2009-04-05  3:46       ` Masami Hiramatsu
2009-04-05  3:49         ` Masami Hiramatsu
2009-04-06 17:11         ` [BUGFIX][PATCH -tip] x86: fix text_poke to handle highmem pages Masami Hiramatsu
2009-04-06 17:32           ` Mathieu Desnoyers
2009-04-06 17:44             ` Masami Hiramatsu
2009-04-06 17:58               ` Mathieu Desnoyers
2009-04-06 20:23                 ` Masami Hiramatsu [this message]
2009-04-08 12:31           ` Ingo Molnar
2009-04-08 14:57             ` Masami Hiramatsu
2009-04-08 14:59               ` Ingo Molnar
2009-04-09 17:55                 ` [BUGFIX][PATCH] x86: fix set_fixmap to use phys_addr_t Masami Hiramatsu
2009-04-09 18:46                   ` Mathieu Desnoyers
2009-04-09 21:52                     ` Masami Hiramatsu
2009-04-10 14:06                   ` [tip:x86/urgent] " Masami Hiramatsu
2009-04-10 15:20                     ` Masami Hiramatsu
2009-04-10 16:05                       ` Mathieu Desnoyers
2009-04-10 17:48                         ` Masami Hiramatsu
2009-04-10 18:30                   ` Masami Hiramatsu

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=49DA64B6.2050809@redhat.com \
    --to=mhiramat@redhat.com \
    --cc=ananth@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@polymtl.ca \
    --cc=mingo@elte.hu \
    --cc=systemtap@sources.redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox