From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hin-Tak Leung Subject: Re: [PATCH 1/2] hfs/hfsplus: Convert dprint to hfs_dbg Date: Mon, 15 Apr 2013 04:46:59 +0100 (BST) Message-ID: <1365997619.11055.YahooMailClassic@web172306.mail.ir2.yahoo.com> References: <1365991579.1878.26.camel@joe-AO722> Reply-To: htl10@users.sourceforge.net Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Vyacheslav Dubeyko , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Joe Perches Return-path: Received: from nm10.bullet.mail.ird.yahoo.com ([77.238.189.39]:48373 "HELO nm10.bullet.mail.ird.yahoo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1753398Ab3DODrB convert rfc822-to-8bit (ORCPT ); Sun, 14 Apr 2013 23:47:01 -0400 In-Reply-To: <1365991579.1878.26.camel@joe-AO722> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: --- On Mon, 15/4/13, Joe Perches wrote: > On Mon, 2013-04-15 at 02:56 +0100, > Hin-Tak Leung wrote: > > --- On Mon, 15/4/13, Joe Perches > wrote: > > > On Mon, 2013-04-15 at 01:53 +0100, > > > Hin-Tak Leung wrote: > > > > --- On Mon, 8/4/13, Joe Perches > wrote: > > > > > Use a more current logging style. > > > [] > > > > I have been sitting on a patch which changes > this part > > > of the code to dynamic debugging, and it is much > simplier. > [] > > > This change wouldn't work well as it would make a > mess > > > of output that uses no prefix (ie: emits at > KERN_DEFAULT) > > > with output that uses KERN_DEBUG > > >=20 > > > That's the reason for _dbg and _dbg_cont. > >=20 > > Hmm, I don't get it. Is there any *existing* use of > dprint > > in the hfplus code which is affected by your comment? >=20 > Code like this prints out currently on a single line at > KERN_DEFAULT. >=20 > @@ -138,16 +138,16 @@ void hfs_bnode_dump(struct hfs_bnode > *node) > [] > =A0 =A0 =A0 =A0 for (i =3D > be16_to_cpu(desc.num_recs); i >=3D 0; off -=3D 2, i--) { > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 > key_off =3D hfs_bnode_read_u16(node, off); > -=A0 =A0 =A0 =A0 =A0 =A0 > =A0=A0=A0dprint(DBG_BNODE_MOD, " %d", key_off); > +=A0 =A0 =A0 =A0 =A0 =A0 > =A0=A0=A0hfs_dbg_cont(BNODE_MOD, " %d", key_off); >=20 > By converting this dprint() to pr_debug(), it would > print out on a multiple lines, one for each read. >=20 > That's why it should use a mechanism like dbg_cont. >=20 > btw: there is no current pr_debug_cont mechanism. That's rubbish. dprint() are compiled in/out debug printing statements,= and are entirely suppressed in unmodified kernel source (the value of = DBG_MASK being zero). So your rather large and invasive change - which = is still conditional on DBG_MASK - is just substituting one form of pri= nt nothing to another form of print nothing. I am not saying what I have in private is correct - otherwise I would h= ave submitted it a long time ago. What I am saying is that the code sni= pplet I posted is functional: it is not conditional on DBG_MASK, but co= nditional on the meaning of pr_debug (and only on it), which is either = printing indiscriminantly, or on/off switchable at runtime for dynamica= lly enabled kernel. And it is a small and non-invasive change in any ca= se, which I can hang on to indefinitely. I think the current *implementation* of dprint is bad - it depending on= a modification of kernel source and re-compilation to make debug state= ment visible instead of the default "print nothing". But your patch, wh= ich modifies a lot of "print nothing" to another style of "print nothin= g", has no functional consequence at all. There is no user-visible chan= ge. It changes a few hundred lines of print nothing to another few hund= red lines of print nothing. =20 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html