linux-kernel - Re: [PATCH 0/10] Add yaffs2 file system: Fifth patchset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4D619C70.6010508@bluewatersys.com>
Date:	Mon, 21 Feb 2011 11:57:52 +1300
From:	Ryan Mallon <ryan@...ewatersys.com>
To:	Greg KH <greg@...ah.com>
CC:	Charles Manning <manningc2@...rix.gen.nz>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Christoph Hellwig <hch@...radead.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	akpm@...ux-foundation.org
Subject: Re: [PATCH 0/10] Add yaffs2 file system:  Fifth patchset

On 02/21/2011 11:29 AM, Greg KH wrote:
> On Mon, Feb 21, 2011 at 09:52:00AM +1300, Ryan Mallon wrote:
>> On 02/21/2011 09:07 AM, Greg KH wrote:
>>> On Mon, Feb 21, 2011 at 06:25:08AM +1300, Charles Manning wrote:
>>>> On Friday 18 February 2011 13:58:52 Greg KH wrote:
>>>> I still intend to keep the tracing printk-based tracing:
>>>>
>>>> #define yaffs_trace(msk, fmt, ...) do { \
>>>> 	if (yaffs_trace_mask & (msk)) \
>>>> 		printk(KERN_DEBUG "yaffs: " fmt "\n", ##__VA_ARGS__); \
>>>> } while (0)
>>>
>>> No, please don't invent your own stuff like this, again, use the
>>> in-kernel functionality provided for this.
>>
>> Do you mean using pr_debug? Other filesystems (see for example
>> fs/ubifs/debug.h:dbg_do_msg) and drivers have similar approaches to this
>> to allow printk debugging with multiple message levels.
> 
> Yes, other ones do have this, and it's a pain, and not consistant across
> the kernel, and usually just not worth it at all.
> 
> Please use the standardized functions for this, you do not need to roll
> your own at all.

What standardised functions are there for managing debug printk's with
different log types? For example, yaffs currently allows printk
debugging to be enabled individually for garbage collection, check
pointing, bad blocks, etc. I am at least in favour of switching to
pr_debug and using pr_fmt for the "yaffs: " prefix, but what should the
yaffs_trace_mask be replaced with?

My understanding is that replacing the proc interface which controls the
yaffs_trace_mask with a more sane sysfs one (ie exposing
yaffs_trace_mask directly), and keeping the yaffs_trace macro but
replacing the printk with pr_debug so that it can be compiled out
completely should be sufficient. i.e.:

#ifdef CONFIG_YAFFS_DEBUG
#define DEBUG
#endif

#define pr_fmt "yaffs: " fmt
#define yaffs_trace(msk, fmt, ...) do {		\
	if (yaffs_trace_mask & (msk))		\
		pr_debug(fmt, ##__VA_ARGS__);	\
	} while (0)

> 
> Especially for trace stuff.

I think we are conflating two things here. Tracepoints have been
suggested as a replacement for some parts of the printk debugging, such
as the mtd debugging. However, it is likely that much of the printk
debugging remains useful and is not ideally replaced by tracepoints.

~Ryan

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan@...ewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/