From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758952AbZBTBor (ORCPT ); Thu, 19 Feb 2009 20:44:47 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754846AbZBTBoi (ORCPT ); Thu, 19 Feb 2009 20:44:38 -0500 Received: from hrndva-omtalb.mail.rr.com ([71.74.56.124]:64852 "EHLO hrndva-omtalb.mail.rr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754573AbZBTBoi (ORCPT ); Thu, 19 Feb 2009 20:44:38 -0500 Date: Thu, 19 Feb 2009 20:44:36 -0500 (EST) From: Steven Rostedt X-X-Sender: rostedt@gandalf.stny.rr.com To: Andrew Morton cc: linux-kernel@vger.kernel.org, mingo@elte.hu, tglx@linutronix.de, peterz@infradead.org, fweisbec@gmail.com, torvalds@linux-foundation.org, arjan@infradead.org, rusty@rustcorp.com.au, mathieu.desnoyers@polymtl.ca, hpa@zytor.com, srostedt@redhat.com Subject: Re: [PATCH 4/6] ftrace, x86: make kernel text writable only for conversions In-Reply-To: <20090219173210.73318ebc.akpm@linux-foundation.org> Message-ID: References: <20090220011316.379904625@goodmis.org> <20090220011521.003556651@goodmis.org> <20090219173210.73318ebc.akpm@linux-foundation.org> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 19 Feb 2009, Andrew Morton wrote: > On Thu, 19 Feb 2009 20:13:20 -0500 > Steven Rostedt wrote: > > > +int ftrace_arch_modify_prepare(void) > > +{ > > + /* at boot up, we are still writable */ > > + if (system_state != SYSTEM_RUNNING) > > + return 0; > > + > > + set_kernel_text_rw(); > > + return 0; > > +} > > + > > +int ftrace_arch_modify_post_process(void) > > +{ > > + /* at boot up, we are still writable */ > > + if (system_state != SYSTEM_RUNNING) > > + return 0; > > + > > + set_kernel_text_ro(); > > + return 0; > > +} > > It would be prudent to avoid using system_state. People can change the > point at which it transitions and can unexpectedly insert (or move) > code to sites where system_state has new values, etc. It was a bad > idea. Good to know. > > It would be clearer and more robust to create your own little flag for > this purpose and set to it true and false at the places where that is > appropriate for this application. It's just one more byte... I just did not want to set it to read-only before the main text decided to do this. I could probably move those checks into the set_kernel_text_* functions in init_32/64.c files. We should not convert until after the DEBUG_RODATA did its first conversion. Yeah, that's a better place for it. I'll write up another patch. Thanks, -- Steve