linux-kernel - Re: [RFC PATCH 10/10] scsi/trace: Use scsi_print

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: <5404144F.2070509@hitachi.com>
Date:	Mon, 01 Sep 2014 15:38:07 +0900
From:	Yoshihiro YUNOMAE <yoshihiro.yunomae.ez@...achi.com>
To:	Hannes Reinecke <hare@...e.de>
Cc:	linux-scsi@...r.kernel.org,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	yrl.pp-manager.tt@...achi.com, linux-kernel@...r.kernel.org,
	"James E.J. Bottomley" <JBottomley@...allels.com>,
	Hidehiro Kawai <hidehiro.kawai.ez@...achi.com>,
	Doug Gilbert <dgilbert@...erlog.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Christoph Hellwig <hch@....de>
Subject: Re: [RFC PATCH 10/10] scsi/trace: Use scsi_print_command trace point
 instead of printk

Hi Hannes,

Sorry for the late reply.

(2014/08/28 21:15), Hannes Reinecke wrote:
> On 08/28/2014 08:19 AM, Yoshihiro YUNOMAE wrote:
>> Hi Hannes,
>>
>> I tried to remove duplicated decoder of SCSI command, but the
>> output format of it in constants.c is different from it in traceevents.
>> I have two questions for it.
>>
>> (Ex1)
>> traceevents: TEST_UNIT_READY
>> constants: Test Unit Ready
>>
>> => Which of "XXX_YYY_ZZZ" and "Xxx Yyy Zzz" should the kernel output
>> strings? This difference comes from difference of implementation.
>> The decoder in traceevents are using macro. So, it cannot define
>> separated words. On the other hand, the decoder in constants are using
>> constant string array table. So, it can define separated words.
>>
> I would go with the wording in constants.c, but set in double quotes
> like "Test Unit Ready" to avoid parsing errors later on.

OK, we use constants' format and add double quotes for each sense code.

>> (Ex2)
>> traceevents: (nothing)
>> constants: Set limits(12)
>>
>> => Should we merge those decoder?
>>
> Yes, I would prefer this. Again, we should be setting the strings in
> double quotes irrespective if there are spaces in the resulting string
> or not.

Sure.

>> I understand we use the decoder of constants, but we need to solve
>> these problems. Would you give me your comments?
>>
> As mentioned previously, my aim is to convert the logging system in
> several steps:
>
> a) convert all lone 'printk' statements into dev_printk() variants
>     and ensure everything is printed in one statement to avoid
>     linebreaks in the resulting logging message.
>     This will eliminate the immediate problem we're having, namely
>     that debugging is near to impossible under high load.
>     The patchset is nearing completion, and I will be posting it
>     later this week.
> b) Convert scsi_trace to use the functions from constants.c to
>     avoid code duplication and update scsi_trace to log sense codes.
> c) Remove all logging functions in the hot path (ie
>     SCSI_LOG_MLQUEUE and SCSI_LOG_MLCOMPLETE) and replace them
>     with trace events.
>
> b) is what you're working on, right?

Right.

> For 'c)' the problem is that we should retain the existing functionality
> with scsi_logging_level here, at least initially.
> So we cannot just replace it, as this would leave SCSI_LOG_ML
> a dummy without any real functionality.

I tried to change printk messages to new tracevents, but I'll stop it
until we solve the problem.
I'll submit the patch set removing duplicated decoder first.

Thanks,
Yoshihiro YUNOMAE

-- 
Yoshihiro YUNOMAE
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: yoshihiro.yunomae.ez@...achi.com


--
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/