From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.9 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, NICE_REPLY_A,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2EFDDC433E3 for ; Tue, 28 Jul 2020 07:34:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id DF8B421775 for ; Tue, 28 Jul 2020 07:34:08 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="Ai+28Ik6" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DF8B421775 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 520EB6B0008; Tue, 28 Jul 2020 03:34:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 4A9D26B000A; Tue, 28 Jul 2020 03:34:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 371776B000C; Tue, 28 Jul 2020 03:34:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0148.hostedemail.com [216.40.44.148]) by kanga.kvack.org (Postfix) with ESMTP id 224836B0008 for ; Tue, 28 Jul 2020 03:34:08 -0400 (EDT) Received: from smtpin01.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with ESMTP id CAC061EE6 for ; Tue, 28 Jul 2020 07:34:07 +0000 (UTC) X-FDA: 77086670934.01.silk56_5116fd826f68 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin01.hostedemail.com (Postfix) with ESMTP id A464710046493 for ; Tue, 28 Jul 2020 07:34:07 +0000 (UTC) X-HE-Tag: silk56_5116fd826f68 X-Filterd-Recvd-Size: 5843 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf20.hostedemail.com (Postfix) with ESMTP for ; Tue, 28 Jul 2020 07:34:06 +0000 (UTC) Received: from devnote2 (NE2965lan1.rev.em-net.ne.jp [210.141.244.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 02F342070B; Tue, 28 Jul 2020 07:34:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1595921646; bh=4CctEpqXsw2X+EjiNA/gb/+djSwvwlkPMoJAMblIkKA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Ai+28Ik68vLxiJUscY/lUed8N+B4bpestmgqJb+Y188jeEcl/ULmEZNiUbbLRHcnL FpljcWZVuNziex5ab5DM4RlpXDXdv/GtI/VRstcF2Y6qkkM029uPwxkFnrvf1JBqZA sh3gtZfs7ncSrVqEIBHPyAHtkDoVBw5CtOGSj96w= Date: Tue, 28 Jul 2020 16:34:00 +0900 From: Masami Hiramatsu To: Ingo Molnar Cc: Jarkko Sakkinen , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andi Kleen , Peter Zijlstra , Masami Hiramatsu , Jessica Yu , "Naveen N. Rao" , Anil S Keshavamurthy , "David S. Miller" , Steven Rostedt , Ingo Molnar Subject: Re: [PATCH v5 1/6] kprobes: Remove dependency to the module_mutex Message-Id: <20200728163400.e00b09c594763349f99ce6cb@kernel.org> In-Reply-To: <20200725102110.GA757173@gmail.com> References: <20200724050553.1724168-1-jarkko.sakkinen@linux.intel.com> <20200724050553.1724168-2-jarkko.sakkinen@linux.intel.com> <20200724091711.GB517988@gmail.com> <20200725030107.GF17052@linux.intel.com> <20200725102110.GA757173@gmail.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: A464710046493 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam04 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, 25 Jul 2020 12:21:10 +0200 Ingo Molnar wrote: > > * Jarkko Sakkinen wrote: > > > On Fri, Jul 24, 2020 at 11:17:11AM +0200, Ingo Molnar wrote: > > > > > > * Jarkko Sakkinen wrote: > > > > > > > --- a/kernel/kprobes.c > > > > +++ b/kernel/kprobes.c > > > > @@ -564,7 +564,7 @@ static void kprobe_optimizer(struct work_struct *work) > > > > cpus_read_lock(); > > > > mutex_lock(&text_mutex); > > > > /* Lock modules while optimizing kprobes */ > > > > - mutex_lock(&module_mutex); > > > > + lock_modules(); > > > > > > > > /* > > > > * Step 1: Unoptimize kprobes and collect cleaned (unused and disarmed) > > > > @@ -589,7 +589,7 @@ static void kprobe_optimizer(struct work_struct *work) > > > > /* Step 4: Free cleaned kprobes after quiesence period */ > > > > do_free_cleaned_kprobes(); > > > > > > > > - mutex_unlock(&module_mutex); > > > > + unlock_modules(); > > > > mutex_unlock(&text_mutex); > > > > cpus_read_unlock(); > > > > > > BTW., it would be nice to expand on the comments above - exactly which > > > parts of the modules code is being serialized against and why? > > > > > > We already hold the text_mutex here, which should protect against most > > > kprobes related activities interfering - and it's unclear (to me) > > > which part of the modules code is being serialized with here, and the > > > 'lock modules while optimizing kprobes' comments is unhelpful. :-) > > > > > > Thanks, > > > > > > Ingo > > > > AFAIK, only if you need to call find_module(), you ever need to acquire > > this mutex. 99% of time it is internally taken care by kernel/module.c. > > > > I cannot make up any obvious reason to acquire it here. > > If it's unnecessary, then it needs to be removed. > > If it's necessary, then it needs to be documented better. Good catch! This is not needed anymore. It has been introduced to avoid conflict with text modification, at that point we didn't get text_mutex. But after commit f1c6ece23729 ("kprobes: Fix potential deadlock in kprobe_optimizer()") moved the text_mutex in the current position, we don't need it. (and anyway, keeping kprobe_mutex locked means any module unloading will be stopped inside kprobes_module_callback()) This may help? >From 2355ecd941c3234b12a6de7443592848ed4e2087 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Tue, 28 Jul 2020 16:32:34 +0900 Subject: [PATCH] kprobes: Remove unneeded module_mutex lock from the optimizer Remove unneeded module_mutex locking from the optimizer. Since we already locks both kprobe_mutex and text_mutex in the optimizer, text will not be changed and the module unloading will be stopped inside kprobes_module_callback(). Suggested-by: Ingo Molnar Signed-off-by: Masami Hiramatsu --- kernel/kprobes.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 4a904cc56d68..d1b02e890793 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -563,8 +563,6 @@ static void kprobe_optimizer(struct work_struct *work) mutex_lock(&kprobe_mutex); cpus_read_lock(); mutex_lock(&text_mutex); - /* Lock modules while optimizing kprobes */ - mutex_lock(&module_mutex); /* * Step 1: Unoptimize kprobes and collect cleaned (unused and disarmed) @@ -589,7 +587,6 @@ static void kprobe_optimizer(struct work_struct *work) /* Step 4: Free cleaned kprobes after quiesence period */ do_free_cleaned_kprobes(); - mutex_unlock(&module_mutex); mutex_unlock(&text_mutex); cpus_read_unlock(); -- 2.25.1 -- Masami Hiramatsu