All of lore.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 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.