From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1946055AbbEVX5R (ORCPT ); Fri, 22 May 2015 19:57:17 -0400 Received: from smtprelay0237.hostedemail.com ([216.40.44.237]:38875 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1945987AbbEVX5O (ORCPT ); Fri, 22 May 2015 19:57:14 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::::::::,RULES_HIT:41:355:379:541:599:973:988:989:1260:1277:1311:1313:1314:1345:1359:1373:1437:1515:1516:1518:1534:1543:1593:1594:1605:1711:1730:1747:1777:1792:2198:2199:2393:2553:2559:2562:2692:2828:3138:3139:3140:3141:3142:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4321:5007:6261:7903:9036:10004:10400:10450:10455:10848:11026:11232:11658:11914:12296:12517:12519:12740:13095:13161:13229:14096:14097:19904:19999:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: range01_48d8d060d43e X-Filterd-Recvd-Size: 4737 Message-ID: <1432339030.29657.20.camel@perches.com> Subject: Re: [PATCH v4 10/13] staging: lustre: lnet: lnet: checkpatch.pl fixes From: Joe Perches To: "Drokin, Oleg" Cc: Julia Lawall , Michael Shuey , "" , "" , "" , "" , "" , "" Date: Fri, 22 May 2015 16:57:10 -0700 In-Reply-To: <05DE4AF3-20A6-40F6-BAC6-79C140E490AF@intel.com> References: <1432237849-53947-1-git-send-email-shuey@purdue.edu> <1432237849-53947-11-git-send-email-shuey@purdue.edu> <1432242004.20840.68.camel@perches.com> <15C0AFDB-CA69-40E5-B65E-C559A5B5CE47@intel.com> <1432309337.29657.16.camel@perches.com> <05DE4AF3-20A6-40F6-BAC6-79C140E490AF@intel.com> Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.12.11-0ubuntu3 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2015-05-22 at 21:16 +0000, Drokin, Oleg wrote: > On May 22, 2015, at 11:42 AM, Joe Perches wrote: > > > On Fri, 2015-05-22 at 08:08 +0000, Drokin, Oleg wrote: > >> On May 22, 2015, at 1:06 AM, Julia Lawall wrote: > >> > >>> On Thu, 21 May 2015, Michael Shuey wrote: > >>> > >>>> That's a task (of many) I've been putting on the back burner until the code > >>>> is cleaner. It's also a HUGE change, since there are debug macros > >>>> everywhere, and they all check a #define'd mask to see if they should fire, > >>>> and the behavior is likely governed by parts of the lustre user land tools > >>>> as well. > >>>> > >>>> Suggestions are welcome. Do other parts of the linux kernel define complex > >>>> debugging macros like these, or is this a lustre-ism? Any suggestions on > >>>> how to handle this more in line with existing drivers? > >>> > >>> Once you decide what to do, you can use Coccinelle to make the changes for > >>> you. So you shouldn't be put off by the number of code sites to change. > >>> > >>> The normal functions are pr_err, pr_warn, etc. Perhaps you can follow > >>> Joe's suggestions if you really need something more complicated. > >> > >> Ideally leaving CERROR/CDEBUG in Lustre would be desirable from my perspective. > > > > My issue with CERROR is the name is little misleading. > > It's actually a debugging message. > > #define CERROR(format, ...) CDEBUG_LIMIT(D_ERROR, format, ## __VA_ARGS__) > > Except it's not a debugging message. > There is a clear distinction. Not really. If the first reading sjows that the mechanism it goes through is called CDEBUG, a reasonable expectation should be that it's a debugging message. > CERROR is something that get's printed on the console, because it's believed > to be serious error (At least that's how the theory for it's usage goes). > It also gets rate-limited so that the console does not get overflown. > (but the debug buffer gets the full version). > (there's also LCONSOLE that always get's printed, but it does not get the > prefixes like line numbers and stuff). > > CDEBUG on the other hand is a debugging message (of which ERROR messages are > sort of a subset (D_ERROR mask)). You can fine-tune those to be noops or > to go into console or to debug buffer only. Most of those are doing nothing > because they are off in the default debug mask, until actually enabled. > > That CERROR usees CDEBUG underneath is just to share some common infrastructure. > > > I think it'd be clearer as > > lustre_debug(ERROR, ... > > even if the name and use style is a little longer. > > I wonder what is more clear about that in your opinion ve > lustre_error/lustre_debug? The fact that you have to explain this shows that it's at least misleading unless you completely understand the code. It'd be more intelligible if this CERROR became lustre_err and the actual debugging uses were lustre_dbg Perhaps it needs a better explanation somewhere not in the code but in some external documentation. I haven't looked.