linux-kernel - Re: [PATCH 4/5] scsi: libsas: Drop SAS

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]

Date:   Wed, 14 Nov 2018 15:12:06 +0000
From:   John Garry <john.garry@...wei.com>
To:     Joe Perches <joe@...ches.com>, <jejb@...ux.vnet.ibm.com>,
        <martin.petersen@...cle.com>
CC:     <hare@...e.com>, <yanaijie@...wei.com>, <linuxarm@...wei.com>,
        <linux-kernel@...r.kernel.org>, <linux-scsi@...r.kernel.org>
Subject: Re: [PATCH 4/5] scsi: libsas: Drop SAS_DPRINTK() and revise logs
 levels

On 14/11/2018 14:53, Joe Perches wrote:
> On Wed, 2018-11-14 at 21:47 +0800, John Garry wrote:
>> Like sas_printk() did previously, SAS_DPRINTK() offers little value now
>> that libsas logs already have the "sas" prefix through pr_fmt(fmt). So it
>> can be dropped.
>>
>> However, after reviewing some logs in libsas, it is noticed that debug
>> level is too low in many instances.
>>
>> So this change drops SAS_DPRINTK() and revises some logs to a more
>> appropriate level. However many stay at debug level, although some
>> are significantly promoted.
> []
>> All the pre-existing checkpatch errors for spanning messages across
>> multiple lines are untouched.
>
> I think coalescing would be useful.

Sorry, I missed that. Do you mean that we stop spanning strings over 
multiple lines?

If yes, I tend to agree. It means we can grep for full strings vs just a 
different checkpatch issue (>80 lines or spanning multiple lines)

>
> Where there are already embedded "sas: "
> prefixes, those should be removed too.

Ah, I missed that guy. File sas_ata.c may also have some.

>
> You could verify this by using
>
> 	$ strings drivers/scsi/libsas/*.o | grep "^[0-7]"
>
>> @@ -186,10 +186,10 @@ int sas_notify_lldd_dev_found(struct domain_device *dev)
>>
>>  	res = i->dft->lldd_dev_found(dev);
>>  	if (res) {
>> -		printk("sas: driver on pcidev %s cannot handle "
>> -		       "device %llx, error:%d\n",
>> -		       dev_name(sas_ha->dev),
>> -		       SAS_ADDR(dev->sas_addr), res);
>> +		pr_warn("sas: driver on pcidev %s cannot handle "
>> +			"device %llx, error:%d\n",
>> +			dev_name(sas_ha->dev),
>> +			SAS_ADDR(dev->sas_addr), res);
>
> e.g.: this now emits "sas: sas: driver etc..."
>
>

Cheers,
John

>
> .
>