From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1754597AbbLQGtM (ORCPT <rfc822;w@1wt.eu>);
	Thu, 17 Dec 2015 01:49:12 -0500
Received: from mga04.intel.com ([192.55.52.120]:4211 "EHLO mga04.intel.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751910AbbLQGtK (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
	Thu, 17 Dec 2015 01:49:10 -0500
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.20,440,1444719600"; 
   d="scan'208";a="14703045"
Message-ID: <56725AD4.4060405@linux.intel.com>
Date: Thu, 17 Dec 2015 14:48:52 +0800
From: "Zhang, Yanmin" <yanmin_zhang@linux.intel.com>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0
MIME-Version: 1.0
To: Steven Rostedt <rostedt@goodmis.org>
CC: "Qiu, PeiyangX" <peiyangx.qiu@intel.com>, linux-kernel@vger.kernel.org,
        mingo@redhat.com, Rusty Russell <rusty@rustcorp.com.au>
Subject: Re: [PATCH] ftrace: fix race between ftrace and insmod
References: <S1751824AbbLNCkP/20151214024015Z+230@vger.kernel.org>	<566E3076.5080708@intel.com>	<566E3482.1090008@intel.com>	<20151214105128.780b8bac@gandalf.local.home>	<566F675C.9000904@linux.intel.com>	<566F8871.7070408@linux.intel.com>	<20151215123719.183879fc@gandalf.local.home>	<56713CD3.80006@linux.intel.com> <20151216092812.34711e3b@grimm.local.home>
In-Reply-To: <20151216092812.34711e3b@grimm.local.home>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2015/12/16 22:28, Steven Rostedt wrote:
> On Wed, 16 Dec 2015 18:28:35 +0800
> "Zhang, Yanmin" <yanmin_zhang@linux.intel.com> wrote:
>
>>> +	/*
>>> +	 * If the tracing is enabled, go ahead and enable the record.
>>> +	 *
>>> +	 * The reason not to enable the record immediatelly is the
>>> +	 * inherent check of ftrace_make_nop/ftrace_make_call for
>>> +	 * correct previous instructions.  Making first the NOP
>>> +	 * conversion puts the module to the correct state, thus
>>> +	 * passing the ftrace_make_call check.
>>> +	 *
>>> +	 * We also delay this to after the module code already set the
>>> +	 * text to read-only, as we now need to set it back to read-write
>>> +	 * so that we can modify the text.
>>> +	 */
>>> +	if (ftrace_start_up)
>>> +		ftrace_arch_code_modify_prepare();
>>> +
>>> +	do_for_each_ftrace_rec(pg, rec) {
>>> +		int cnt;
>>> +		/*
>>> +		 * do_for_each_ftrace_rec() is a double loop.
>>> +		 * module text shares the pg. If a record is
>>> +		 * not part of this module, then skip this pg,
>>> +		 * which the "break" will do.
>>> +		 */
>>> +		if (!within_module_core(rec->ip, mod))
>>> +			break;
>>> +
>>> +		cnt = 0;
>>> +
>>> +		/*
>>> +		 * When adding a module, we need to check if tracers are
>>> +		 * currently enabled and if they are, and can trace this record,
>>> +		 * we need to enable the module functions as well as update the
>>> +		 * reference counts for those function records.
>>> +		 */
>>> +		if (ftrace_start_up)
>>> +			cnt += referenced_filters(rec);
>>> +
>>> +		/* This clears FTRACE_FL_DISABLED */
>>> +		rec->flags = cnt;
>>> +
>>> +		if (ftrace_start_up && cnt) {
>>> +			int failed = __ftrace_replace_code(rec, 1);  
>> If we choose to call ftrace_module_enable when receiving module notification
>>   MODULE_STATE_COMING, TEXT section of the module is already changed to RO.
> And that's why we call ftrace_arch_code_modify_prepare(). That should
> change all text to RW.

Thanks for the kind pointer. 
We would add codes into your patch based on notifier and send patch to you by
private email.

Yanmin