From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f179.google.com (mail-wi0-f179.google.com [209.85.212.179]) by kanga.kvack.org (Postfix) with ESMTP id 9AD7B6B00B6 for ; Mon, 18 May 2015 09:23:30 -0400 (EDT) Received: by wicnf17 with SMTP id nf17so69509796wic.1 for ; Mon, 18 May 2015 06:23:30 -0700 (PDT) Received: from mail-wg0-f51.google.com (mail-wg0-f51.google.com. [74.125.82.51]) by mx.google.com with ESMTPS id r6si12722264wiv.0.2015.05.18.06.23.28 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 18 May 2015 06:23:28 -0700 (PDT) Received: by wgjc11 with SMTP id c11so26890054wgj.0 for ; Mon, 18 May 2015 06:23:28 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <7254.1431945085@warthog.procyon.org.uk> References: <7254.1431945085@warthog.procyon.org.uk> From: Leon Romanovsky Date: Mon, 18 May 2015 16:23:07 +0300 Message-ID: Subject: Re: [RFC] Refactor kenter/kleave/kdebug macros Content-Type: multipart/alternative; boundary=f46d041828086d28b005165b196e Sender: owner-linux-mm@kvack.org List-ID: To: David Howells Cc: Linux-MM , "linux-kernel@vger.kernel.org" , linux-cachefs , linux-afs --f46d041828086d28b005165b196e Content-Type: text/plain; charset=UTF-8 On Mon, May 18, 2015 at 1:31 PM, David Howells wrote: > I can turn on all the macros in a file just be #defining __KDEBUG at the > top. > When I first did this, pr_xxx() didn't exist. > > Note that the macros in afs, cachefiles, fscache and rxrpc are more complex > than a grep tells you. There are _enter(), _leave() and _debug() macros > which > are conditional via a module parameter. These are trivially individually > enableable during debugging by changing the initial underscore to a 'k'. > They > are otherwise enableable by module parameter (macros are individually > selectable) or enableably by file __KDEBUG. These are well used. Note > that > just turning them all into pr_devel() would represent a loss of useful > function. > > The ones in the keys directory are also very well used, though they aren't > externally selectable. I've added functionality to the debugging, but > haven't > necessarily needed to backport it to earlier variants. > > For the mn10300 macros, I would just recommend leaving them as is. > > For the nommu macros, you could convert them to pr_devel() - but putting > all > the information in the kenter/kleave/kdebug macro into each pr_devel macro > would be more intrusive in the code since you'd have to move the stuff out > of > there macro definition into each caller. You could also reexpress the > macros > in terms of pr_devel and get rid of the conditional. OTOH, there's not > that > much in the nommu code, so you could probably slim down a lot of what's > printed. > > For the cred macro, just convert to pr_devel() or pr_debug() and make > pr_fmt > insert current->comm and current->pid. > > > 2. Move it to general include file (for example linux/printk.h) and > > commonize the output to be consistent between different kdebug users. > > I would quite like to see kenter() and kleave() be moved to printk.h, > expressed in a similar way to pr_devel() or pr_debug() (and perhaps renamed > pr_enter() and pr_leave()) but separately so they can be enabled > separately. > OTOH, possibly they should be enableable by compilation block rather than > by > macro set. > > The main thing I like out of the ones in afs, cachefiles, fscache and > rxrpc is > the ability to just turn on a few across a bunch of files so as not to get > overwhelmed by data. > Blind conversion to pr_debug will blow the code because it will be always compiled in. In current implementation, it replaced by empty functions which is thrown by compiler. Additionally, It looks like the output of these macros can be viewed by ftrace mechanism. Maybe we should delete them from mm/nommu.c as was pointed by Joe? > > David > -- Leon Romanovsky | Independent Linux Consultant www.leon.nu | leon@leon.nu --f46d041828086d28b005165b196e Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

= On Mon, May 18, 2015 at 1:31 PM, David Howells <dhowells@redhat.com&= gt; wrote:
I can turn on all the m= acros in a file just be #defining __KDEBUG at the top.
When I first did this, pr_xxx() didn't exist.

Note that the macros in afs, cachefiles, fscache and rxrpc are more complex=
than a grep tells you.=C2=A0 There are _enter(), _leave() and _debug() macr= os which
are conditional via a module parameter.=C2=A0 These are trivially individua= lly
enableable during debugging by changing the initial underscore to a 'k&= #39;.=C2=A0 They
are otherwise enableable by module parameter (macros are individually
selectable) or enableably by file __KDEBUG.=C2=A0 These are well used.=C2= =A0 Note that
just turning them all into pr_devel() would represent a loss of useful
function.

The ones in the keys directory are also very well used, though they aren= 9;t
externally selectable.=C2=A0 I've added functionality to the debugging,= but haven't
necessarily needed to backport it to earlier variants.

For the mn10300 macros, I would just recommend leaving them as is.

For the nommu macros, you could convert them to pr_devel() - but putting al= l
the information in the kenter/kleave/kdebug macro into each pr_devel macro<= br> would be more intrusive in the code since you'd have to move the stuff = out of
there macro definition into each caller.=C2=A0 You could also reexpress the= macros
in terms of pr_devel and get rid of the conditional.=C2=A0 OTOH, there'= s not that
much in the nommu code, so you could probably slim down a lot of what's=
printed.

For the cred macro, just convert to pr_devel() or pr_debug() and make pr_fm= t
insert current->comm and current->pid.

> 2. Move it to general include file (for example linux/printk.h) and > commonize the output to be consistent between different kdebug users.<= br>
I would quite like to see kenter() and kleave() be moved to printk.h= ,
expressed in a similar way to pr_devel() or pr_debug() (and perhaps renamed=
pr_enter() and pr_leave()) but separately so they can be enabled separately= .
OTOH, possibly they should be enableable by compilation block rather than b= y
macro set.

The main thing I like out of the ones in afs, cachefiles, fscache and rxrpc= is
the ability to just turn on a few across a bunch of files so as not to get<= br> overwhelmed by data.
Blind conversion to pr_debug will= blow the code because it will be always compiled in. In current implementa= tion, it replaced by empty functions which is thrown by compiler.

Additionally, It looks like the output of these macros can be view= ed by ftrace mechanism.

Maybe we should delete them from = mm/nommu.c as was pointed by Joe?

=C2=A0

David



--
Leon Romanovsky | Independent L= inux Consultant
=C2=A0 =C2=A0 =C2=A0 =C2=A0=C2=A0www.leon.nu=C2=A0| leon@leon.nu
--f46d041828086d28b005165b196e-- -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org