Re: [PATCH] [GIT PULL] tracing: allow to change permissions for text with dynamic ftrace enabled

[Suresh Siddha <suresh.b.siddha@intel.com>]

On Tue, 2009-10-27 at 16:31 -0700, Steven Rostedt wrote: 
> On Tue, 2009-10-27 at 15:23 -0800, Suresh Siddha wrote:
> 
> > > 
> > > I'll test it out, and if it does work, you can write up a formal patch
> > > and remove the !define that I added.
> > 
> > I just saw one more place calling do_ftrace_mod_code(). So moved this
> > check inside the do_ftrace_mod_code(). Does this cover all the cases?
> > Thanks.
> 
> For ftrace, probably. But I just realized this may have other
> consequences. The CPA_DEBUG tests setting a bit in the page table at
> random locations.

CPA_DEBUG is setting/clearing unused bit 9. It has no interaction with
RW bits. Though the random page and random length selections may end up
splitting the large page mappings for kernel text. This is a different
issue that needs to be addressed (perhaps I can ignore this bit 9 as
well for the kernel text mapping. In general it sounds like a bad idea
to break any large page mappings to small page mappings for cpa debug
tests, especially when we don't restore the large page mapping when we
clear those unused bits once the test is done. Will look into this).

> The code you added makes any change to kernel text
> page attributes remove the RW bit.

Only for CONFIG_DEBUG_RODATA.

> On boot up, it is considered that
> kernel text is writable.

For CONFIG_DEBUG_RODATA, during the boot we change it to RO.

> Perhaps you need to add a check in your if statement to only prevent
> that bit when kernel_set_to_readonly is not true.

Are you worried that kernel_set_to_readonly might not be true but we
might have the kernel text mapping read-only?

As ftrace is now using kernel identity mapping on 64-bit, we can change
the semantics of kernel_set_to_readonly to reflect the status of kernel
identity mapping for 64-bit. I can update the comments.

thanks,
suresh