From patchwork Fri Feb 26 20:16:46 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yang Shi X-Patchwork-Id: 63109 Delivered-To: patch@linaro.org Received: by 10.112.199.169 with SMTP id jl9csp65600lbc; Fri, 26 Feb 2016 12:16:52 -0800 (PST) X-Received: by 10.66.63.67 with SMTP id e3mr4636315pas.141.1456517812291; Fri, 26 Feb 2016 12:16:52 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id tr4si3069001pac.222.2016.02.26.12.16.51; Fri, 26 Feb 2016 12:16:52 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dkim=pass header.i=@linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423070AbcBZUQv (ORCPT + 30 others); Fri, 26 Feb 2016 15:16:51 -0500 Received: from mail-pf0-f178.google.com ([209.85.192.178]:36583 "EHLO mail-pf0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752141AbcBZUQt (ORCPT ); Fri, 26 Feb 2016 15:16:49 -0500 Received: by mail-pf0-f178.google.com with SMTP id e127so57614606pfe.3 for ; Fri, 26 Feb 2016 12:16:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=wdS8k/YxiLBEc/j0srbpyBTv6cth0EBDdN8qe5x3vog=; b=QtATOXOJswjBexxXPsBqHEob8PAwUpzQonqLms2qraVaE8/KT3J1VPDqFhRSzpqtXf 6w11ECcFkJVhvn89SQ4QtvQHs+x051KFM9vxj6apLIcfh/AuMlLhi8SvFO72mXvadO4z iLRaXv/FB0+Yh7AFvEUr2KtbGbIGlgOQltDqc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=wdS8k/YxiLBEc/j0srbpyBTv6cth0EBDdN8qe5x3vog=; b=V4CWUC4ZTF8TSY1nwfpafH1V1Yoh1LzNRqMMX3G+djF4q8LbQfMskiQadO2kk2bw22 eEYtbpT25eZ91JSPW1sL+JaOAGbOMwFmgizEEOyo1w35yxdsZC1913IHyj099VLKcWlp pIqZo4Zw2xbGJtDKSurgfqGOiVXUotoaRHPyk8XdC8tOHTvaAm1yAL5xf708chhVmFiO X/GRpgghR7Tlxwws44HqbwF2B8fuyKEUBC8KlPl2gDXOkkcJqiTQhvjCj0ypJ8J4wkkq Ep0r3MTpvcNQ2o155cQoBM64aGMjnH/hBTYFH5xRERvEtVC+B5QdHLdFZ6AhJlg5wt/2 Dqdg== X-Gm-Message-State: AD7BkJKrBOqLmgI30zt5BOwxMWu1Cqo+r7STJOChsJHKojB1UCuTCUEh9Gy0nUnjZYXIfi/Q X-Received: by 10.98.13.216 with SMTP id 85mr4686950pfn.143.1456517808350; Fri, 26 Feb 2016 12:16:48 -0800 (PST) Received: from [147.11.216.196] (unknown-216-196.windriver.com. [147.11.216.196]) by smtp.googlemail.com with ESMTPSA id yh5sm21266559pab.13.2016.02.26.12.16.47 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 26 Feb 2016 12:16:47 -0800 (PST) Subject: Re: [RFC PATCH] kernfs: create raw version kernfs_path_len and kernfs_path To: Steven Rostedt References: <1456510505-6620-1-git-send-email-yang.shi@linaro.org> <20160226135628.09ce727c@gandalf.local.home> <56D0A97E.90706@linaro.org> <56D0AEAC.3070002@linaro.org> Cc: gregkh@linuxfoundation.org, tj@kernel.org, lizefan@huawei.com, tglx@linutronix.de, bigeasy@linutronix.de, linux-kernel@vger.kernel.org, linux-rt-users@vger.kernel.org, linaro-kernel@lists.linaro.org From: "Shi, Yang" Message-ID: <56D0B2AE.7020104@linaro.org> Date: Fri, 26 Feb 2016 12:16:46 -0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <56D0AEAC.3070002@linaro.org> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/26/2016 11:59 AM, Shi, Yang wrote: > On 2/26/2016 11:37 AM, Shi, Yang wrote: >> On 2/26/2016 10:56 AM, Steven Rostedt wrote: >>> On Fri, 26 Feb 2016 10:15:05 -0800 >>> Yang Shi wrote: >>> >>>> commit 5634cc2aa9aebc77bc862992e7805469dcf83dac ("writeback: update >>>> writeback >>>> tracepoints to report cgroup") made writeback tracepoints report cgroup >>>> writeback, but it may trigger the below bug on -rt kernel since >>>> kernfs_path >>>> and kernfs_path_len are called by tracepoints, which acquire sleeping >>>> lock. >>>> >>>> BUG: sleeping function called from invalid context at >>>> kernel/locking/rtmutex.c:930 >>>> in_atomic(): 1, irqs_disabled(): 0, pid: 625, name: kworker/u16:3 >>>> INFO: lockdep is turned off. >>>> Preemption disabled at:[] wb_writeback+0xec/0x830 >>>> >>>> CPU: 7 PID: 625 Comm: kworker/u16:3 Not tainted 4.4.1-rt5 #20 >>>> Hardware name: Freescale Layerscape 2085a RDB Board (DT) >>>> Workqueue: writeback wb_workfn (flush-7:0) >>>> Call trace: >>>> [] dump_backtrace+0x0/0x200 >>>> [] show_stack+0x24/0x30 >>>> [] dump_stack+0x88/0xa8 >>>> [] ___might_sleep+0x2ec/0x300 >>>> [] rt_spin_lock+0x38/0xb8 >>>> [] kernfs_path_len+0x30/0x90 >>>> [] >>>> trace_event_raw_event_writeback_work_class+0xe8/0x2e8 >>>> [] wb_writeback+0x620/0x830 >>>> [] wb_workfn+0x61c/0x950 >>>> [] process_one_work+0x3ac/0xb30 >>>> [] worker_thread+0x9c/0x7a8 >>>> [] kthread+0x190/0x1b0 >>>> [] ret_from_fork+0x10/0x30 >>>> >>>> Since kernfs_* functions are heavily used by cgroup, so it sounds not >>>> reasonable to convert kernfs_rename_lock to raw lock. >>>> >>>> Create raw version kernfs_path, kernfs_path_len and cgroup_path, >>>> which don't >>>> acquire lock and are used by tracepoints only. >>>> >>> >>> And what prevents name from being freed while the tracepoint is reading >>> it? >>> >>> Perhaps we need this change as well: >> >> Thanks for pointing this out. Yes, it looks we need this since the >> tracepoints are protected by rcu_read_lock_sched. >> >> Will add this in V2. > > BTW, it sounds this is not the only point where kernfs_node could be > updated, __kernfs_remove should need synchronize_sched too. Should be synchronize_sched moved into kernfs_put itself, just before the kfree call since it is called at some places other than kernfs_remove and kernfs_rename_ns. But, kernfs_put might be called recursively, is it fine? Thanks, Yang > > Thanks, > Yang > >> >> Yang >> >>> >>> -- Steve >>> >>> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c >>> index 996b7742c90b..d2ef153145c0 100644 >>> --- a/fs/kernfs/dir.c >>> +++ b/fs/kernfs/dir.c >>> @@ -1397,6 +1397,12 @@ int kernfs_rename_ns(struct kernfs_node *kn, >>> struct kernfs_node *new_parent, >>> kn->hash = kernfs_name_hash(kn->name, kn->ns); >>> kernfs_link_sibling(kn); >>> >>> + /* >>> + * Tracepoints may be reading the old name. They are protected >>> + * by rcu_read_lock_sched(). >>> + */ >>> + synchronize_sched(); >>> + >>> kernfs_put(old_parent); >>> kfree_const(old_name); >>> >>> >> > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index a4b1db1..89c0c16 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -456,6 +456,12 @@ void kernfs_put(struct kernfs_node *kn) if (kernfs_type(kn) == KERNFS_LINK) kernfs_put(kn->symlink.target_kn); + /* + * Tracepoints may be reading the old name. They are protected + * by rcu_read_lock_sched(). + */ + synchronize_sched(); + kfree_const(kn->name); if (kn->iattr) {