Re: + remove-current-defines-and-uses-of-pr_err-add-pr_emerg.patchadded to -mm tree

[Jan Engelhardt]

On Aug 8 2007 14:36, Joe Perches wrote:
>On Wed, 2007-08-08 at 22:39 +0200, Jan Engelhardt wrote:
>> I fail to see what problem these are trying to fix. 
>
>Any code that does the equivalent of printk(KERN_foo "\n message");
>
>egrep -r "printk[[:space:]]*\([[:space:]]*KERN.*\\\n[A-JL-Za-jl-z[:space:]" --include=*.[ch] *
>
>At least 61 instances right now.

Eww. Well, let's look at some.
arch/i386/kernel/io_apic.c:line 1529

        printk("\n" KERN_DEBUG "printing local APIC contents on CPU#%d/%d:\n",
                smp_processor_id(), hard_smp_processor_id());

The \n seems unneeded, since print_local_APIC() is run on each cpu.
(see caller).


Then, there are also messages that are just a standalone \n
without even a debugging level. Not sure that is useful.

	# io_apic.c again, line 1601
	#
        printk(KERN_DEBUG "... APIC TDCR: %08x\n", v);  
        printk("\n");

So, well, let's pick one with KERN_DEBUG "\n....

fs/freevxfs/vxfs_bmap.c:line 169 (near "case VXFS_TYPED_DATA_DEV4")

                        printk(KERN_INFO "\n\nTYPED_DEV4 detected!\n");
                        printk(KERN_INFO "block: %Lu\tsize: %Ld\tdev: %d\n",
                               (unsigned long long) typ4->vd4_block,
                               (unsigned long long) typ4->vd4_size,
                               typ4->vd4_dev);

Seems normal. So it looks like your regex has some false positives.
(which nothing can be done about - the 61 must be hand-sorted)

Perhaps we're looking for this idiom:

	printk(KERN_WHATEVER "Doing foo...");
	for (as_long_as_there_is_work) {
		do_it();
		printk(KERN_WHATEVER ".");
	}
	printk(KERN_WHATEVER "\n");

Yes, that imo is horrible :) since code running on another cpu can printk in
the middle. Perhaps

	int i;
	for (as_long with ++i) {
		do_it();
		printk("something informative %d\n", i++);
	}

should be done _instead_. This would also address number 8:

>> >8 Minimization/elimination of interleaved log messages
>> 
>> This is a separate, and worthwhile, thing to do, yes.
>> And it can be done without deviating from the regular printk().
>
>I'm not sure how.  Maybe you have an idea you could share?

[My idea is above]

>Maybe an external tool to reassemble complete messages from
>prefixed {{cookie}} message logs would be fine for awhile.
>
>Perhaps something like:
>
>cookie = printk_block_start()
>printk_block[s](cookie)
>printk_block_end(cookie)?
>
>where printk_block emits cookie when multiple cookies
>are active.

How is this supposed to work? Are you suggesting that the printk buffer is
locked? I hope not. Cache output until a block is ended? No, I do not think
that helps either, because the   for(){printk(".");}  loops are meant to
give an indication _THAT_ something is happening (or not - e.g. lockup).
Now, if you hold the output of a printk block until it's _end()
counterpart is executed, the user does not get any output when the
machine locks up during the for loop.

Hence, better do a full printk (e.g. with newline) in each iteration.
And if possible sparingly. Booting in 9600 bps serial mode is going to
take longer the more output the kernel produces ;-)
Or just to not clutter dmesg too much.



And at the end of the day when this would be done -
what would we need pr_foo for?


	Jan
-- 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/