Received: by 2002:ac0:a594:0:0:0:0:0 with SMTP id m20-v6csp4967389imm; Mon, 14 May 2018 16:57:43 -0700 (PDT) X-Google-Smtp-Source: AB8JxZqX69tlYyz7i2sHKvpjt7Pl4EkPrygg365ul3dIKbPpdj6hQHnAbBwXJLowV75/N7mc0kBt X-Received: by 2002:a65:64d9:: with SMTP id t25-v6mr9664537pgv.283.1526342263808; Mon, 14 May 2018 16:57:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1526342263; cv=none; d=google.com; s=arc-20160816; b=wS/1GXHQhccurYT0YPSYdjCF08UJLBXP4DwzUaiM3JcLFfDyaKqrDprAhIaYPhZift 9BvSSSHc2jFDxShMkMmb/DfHxWt/AtDce/ULWXzA6SFIVhNZf9kViEN2FNRLgW55dFB2 XcojdGhFuIBGK/oK7lOrrJRKI+DoNatgnmlFa76oiZG56IVVbYlmLDLfxc8HUJQ4sOYU IonCslajgYU9xI9pO+78pIiUFxgXlghuZrW6xlrHb4SOwPiQY7WGrCekIvLxmzVzZuhg WVx8O50XmqCdMLjDnjp9VKH/wAtqvfk5yYQdRJh35P70pwnKxQPFYxK+F9fAE90Qoq9P 0cMA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:user-agent:in-reply-to :content-disposition:mime-version:references:reply-to:subject:cc:to :from:date:arc-authentication-results; bh=ITvRF0XDNu0bzBFQeqgr6qpJVxXm6zxqp+5O16zlMBQ=; b=Cq4RO77jitxyWWEHoePS9k96gak4SStwN0zhMVjf70F3DjORozbbx1vSFhLf51MaDr 8wpXe2O5K+RZGdFYRUe+mBCxK/Hfm88axqqhyFgngbZGoPT6iAzDOfo07/kzGwiulGA3 CleH9zrTP4k3JoeAhnCBZsJLTxUZ8vxjLOljY7e9KfQipbmTOlqASfp0UgvKaD284zCj I1cls7/XytggjD2lF09zKk+ORFA1lHan+ZCuhTqJSJ+lcFRZa78GL2FZRRUUR2d8x7LY FnK1LH/X2FRbvS7KVZ9XGkYXjiPSvGvBPSpb9L8+xt9jtBzgfsBxRAfCxGalQkFeKSUy aqiQ== ARC-Authentication-Results: i=1; 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=ibm.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id b10-v6si10412302pls.81.2018.05.14.16.57.19; Mon, 14 May 2018 16:57:43 -0700 (PDT) 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; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=ibm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752465AbeENX5I (ORCPT + 99 others); Mon, 14 May 2018 19:57:08 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:56598 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752196AbeENX5G (ORCPT ); Mon, 14 May 2018 19:57:06 -0400 Received: from pps.filterd (m0098394.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w4ENncwt035872 for ; Mon, 14 May 2018 19:57:06 -0400 Received: from e12.ny.us.ibm.com (e12.ny.us.ibm.com [129.33.205.202]) by mx0a-001b2d01.pphosted.com with ESMTP id 2hykjv9tn9-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 14 May 2018 19:57:06 -0400 Received: from localhost by e12.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 14 May 2018 19:57:05 -0400 Received: from b01cxnp22034.gho.pok.ibm.com (9.57.198.24) by e12.ny.us.ibm.com (146.89.104.199) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 14 May 2018 19:57:01 -0400 Received: from b01ledav003.gho.pok.ibm.com (b01ledav003.gho.pok.ibm.com [9.57.199.108]) by b01cxnp22034.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w4ENv1x853674226; Mon, 14 May 2018 23:57:01 GMT Received: from b01ledav003.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 31523B204D; Mon, 14 May 2018 20:58:59 -0400 (EDT) Received: from paulmck-ThinkPad-W541 (unknown [9.70.82.108]) by b01ledav003.gho.pok.ibm.com (Postfix) with ESMTP id DC302B2046; Mon, 14 May 2018 20:58:58 -0400 (EDT) Received: by paulmck-ThinkPad-W541 (Postfix, from userid 1000) id A34FB16C3BCB; Mon, 14 May 2018 16:58:32 -0700 (PDT) Date: Mon, 14 May 2018 16:58:32 -0700 From: "Paul E. McKenney" To: Joe Perches Cc: Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , Davidlohr Bueso , linux-kernel@vger.kernel.org Subject: Re: [PATCH 18/18] rcu: Use pr_fmt to prefix "rcu: " to logging output Reply-To: paulmck@linux.vnet.ibm.com References: <41d9686471d67f6f98d160e5891bf61061515b6d.1525964386.git.joe@perches.com> <20180514202910.GI26088@linux.vnet.ibm.com> <32216d8f154b29400d4c11bae91850591114f7f8.camel@perches.com> <20180514222456.GO26088@linux.vnet.ibm.com> <3c7770a062355650fb61e48a5fbeb9653d98d85a.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3c7770a062355650fb61e48a5fbeb9653d98d85a.camel@perches.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 18051423-0048-0000-0000-0000026CF447 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00009026; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000260; SDB=6.01032391; UDB=6.00527789; IPR=6.00811523; MB=3.00021114; MTD=3.00000008; XFM=3.00000015; UTC=2018-05-14 23:57:03 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18051423-0049-0000-0000-0000451F5318 Message-Id: <20180514235832.GR26088@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-05-14_06:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1709140000 definitions=main-1805140234 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, May 14, 2018 at 03:54:13PM -0700, Joe Perches wrote: > On Mon, 2018-05-14 at 15:24 -0700, Paul E. McKenney wrote: > > On Mon, May 14, 2018 at 02:41:59PM -0700, Joe Perches wrote: > > > On Mon, 2018-05-14 at 13:29 -0700, Paul E. McKenney wrote: > > > > On Thu, May 10, 2018 at 08:45:44AM -0700, Joe Perches wrote: > > > > > Use a consistent logging prefix for all rcu related output. > > > > > > > > > > Signed-off-by: Joe Perches > > > > > > > > I took parts of this (thank you!) but have concerns about other parts. > > > > > > [] > > > > > diff --git a/kernel/rcu/rcuperf.c b/kernel/rcu/rcuperf.c > > > > > index 076a50fb22ad..ebdd77b45470 100644 > > > > > --- a/kernel/rcu/rcuperf.c > > > > > +++ b/kernel/rcu/rcuperf.c > > > > > @@ -19,6 +19,9 @@ > > > > > * > > > > > * Authors: Paul E. McKenney > > > > > */ > > > > > + > > > > > +#define pr_fmt(fmt) "rcu: " fmt > > > > > > > > This is going to get us messages of the form "rcu: rcu-perf:", not? > > > > (And other odd combinations, depending on the flavor of RCU under test.) > > > > If so, this does not seem to be an improvement. > > > > > > That depends on the existing embedded content of the format. > > > This will prefix just "rcu: " to pr_ output. > > > > OK, so to make this work, I need to create special-purpose pr_fmt() > > definitions for torture, rcutorture, locktorture, and rcuperf. Most > > of the rest don't care. > > Yes, or allow the new generic #define for pr_fmt > to set those prefixes for you > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > will do the appropriate thing for rcutorture, OK. For rcutorture, there are special prefixes. More on this at the end. > > Or am I missing something basic here? > > > > > > > diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c > > > > > > [] > > > > > @@ -908,7 +909,7 @@ rcu_torture_writer(void *arg) > > > > > VERBOSE_TOROUT_STRING("rcu_torture_writer task started"); > > > > > if (!can_expedite) > > > > > pr_alert("%s" TORTURE_FLAG > > > > > - " GP expediting controlled from boot/sysfs for %s.\n", > > > > > + " GP expediting controlled from boot/sysfs for %s\n", > > > > > torture_type, cur_ops->name); > > > > > > As there is _no_ pr_fmt #defined in this file, > > > output will/could be prefixed with KBUILD_MODNAME ": " > > > (in this case "rcutorture: ") if/when the generic > > > pr_fmt conversion patch is applied. > > > > Not a fan, NACK. > > > > > > > } else if (gp_sync && !cur_ops->sync) { > > > > > - pr_alert("%s: gp_sync without primitives.\n", __func__); > > > > > + pr_alert("%s: gp_sync without primitives\n", __func__); > > > > > > > > I used a CDC Cyber 73 in the 1970s. > > > > > > Fancy. I used a CDC 6400 and an IBM 1620, but > > > those were pretty old when I started. > > > > ;-) > > > > > > It had tiny memory by today's > > > > standards, but even it had periods in its error messages. We can easily > > > > afford them today, especially given that rcutorture is not included in > > > > small-memory Linux configurations. > > > > > > OK, but I like consistency too. > > > > > > ~90 percent of linux logging does not use terminating periods. > > > For instance, on my laptop: > > > > > > $ uptime -p > > > up 1 week, 1 day, 13 hours, 37 minute > > > $ dmesg | wc -l > > > 4240 > > > $ dmesg | grep -P "\w\.$"| wc -l > > > 381 > > > > Why is this a problem worth fixing? From where I sit, it is not. > > It's not a _problem_. It's just an inconsistency. > I modify them passively when I see them. > If you don't like it, don't take it. OK, fair enough. I will pass. > > Even assuming that this is somehow worth solving, why is it buried > > in an unrelated patch? > > Touches each line fewer times. > > > > > > @@ -1711,11 +1712,11 @@ static void rcu_test_debug_objects(void) > > > > > > > > > > /* Wait for them all to get done so we can safely return. */ > > > > > rcu_barrier(); > > > > > - pr_alert("%s: WARN: Duplicate call_rcu() test complete.\n", KBUILD_MODNAME); > > > > > + pr_alert("WARN: Duplicate call_rcu() test complete\n"); > > > > > > > > I would like to keep these, as they mark the region of console output where > > > > splats are expected. > > > > > > The prefixes are still there... > > > > > > > > if (cur_ops->fqs == NULL && fqs_duration != 0) { > > > > > - pr_alert("rcu-torture: ->fqs NULL and non-zero fqs_duration, fqs disabled.\n"); > > > > > + pr_alert("->fqs NULL and non-zero fqs_duration, fqs disabled\n"); > > > > > > > > This I would like to keep. Easier to find. > > > > > > One thing that you could use to validate the > > > output string format is after compilation: > > > > > > $ strings kernel/rcu/rcutorture.o | grep -P "^[0-6]\w+:" > > > > > > With your change, you will see duplicated prefixes. > > > > Except that right now there are not duplicated prefixes. Those > > apparently only show up after some earlier patch in/before your set is > > applied, correct? > > yes. > [PATCH 03/18] printk: Convert pr_fmt from blank define to KBUILD_MODNAME > > > Is there some C-preprocessor macro indicating whether or not your changes > > have been applied? > > ? pr_fmt would either be blank or something else. OK, so if I define pr_fmt as follows, I get the old behavior? #define pr_fmt(fmt) fmt If so, I can add this to the torture files, and then define pr_fmt more sensibly for the torture files once your changes hit mainline. Or if we can agree on the defintion, perhaps we get the sensible definition into your patch series. But I would much prefer the former given the high rate of change in RCU at the moment. Note that my concerns apply only to kernel/torture.c, kernel/rcu/rcuperf.c, kernel/rcu/rcutorture.c, and kernel/locking/locktorture.c (despite my earlier ack). The other RCU files, including kernel/rcu/rcu_segcblist.c, should be just fine with "#define pr_fmt()" added. And I just queued a commit to be squashed into my version of your patch 18/18 that adds this to kernel/rcu/rcu_segcblist.c. This joins the ones that your patch added to kernel/rcu/srcutree.c and kernel/rcu/tree.c. Should I also add "#define pr_fmt(fmt) "rcu: " fmt" to these files? kernel/rcu/srcutiny.c kernel/rcu/sync.c kernel/rcu/tiny.c Thanx, Paul