On 8/20/24 02:17, Bart Van Assche wrote:
> On 8/18/24 4:25 PM, Damien Le Moal wrote:
>> On 8/17/24 06:55, Bart Van Assche wrote:
>>> diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>> index 1078c20c5ef6..f49783b89d04 100644
>>> --- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>> +++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
>>> @@ -2363,8 +2363,8 @@ static int _bnx2fc_create(struct net_device *netdev,
>>> interface->vlan_id = vlan_id;
>>> interface->tm_timeout = BNX2FC_TM_TIMEOUT;
>>>
>>> - interface->timer_work_queue =
>>> - create_singlethread_workqueue("bnx2fc_timer_wq");
>>> + interface->timer_work_queue = alloc_ordered_workqueue(
>>> + "%s", WQ_MEM_RECLAIM, "bnx2fc_timer_wq");
>>
>> Very odd line split. And there are a few more like this one. Maybe your patch
>> needs some manual tuning after running the script ?
>>
>> The patch overall looks good to me, but it would be nice to have consistency in
>> the line splitting. Personnally, I prefer the pattern such as:
>>
>> - kmpath_rdacd = create_singlethread_workqueue("kmpath_rdacd");
>> + kmpath_rdacd =
>> + alloc_ordered_workqueue("%s", WQ_MEM_RECLAIM, "kmpath_rdacd");
>>
>> instead of:
>>
>> - lio_wq = create_singlethread_workqueue("efct_lio_worker");
>> + lio_wq = alloc_ordered_workqueue("%s", WQ_MEM_RECLAIM,
>> + "efct_lio_worker");
>>
>> Though I guess that is a matter of taste :)
>
> (reduced cc-list)
>
> If I run "git clang-format HEAD^" on this patch, no code is changed.
> Does this perhaps mean that the .clang-format style file in the kernel
> tree needs further tuning? The most recent change in that file other
> than adding for-each macro names is from two years ago (see also commit
> 781121a7f6d1 ("clang-format: Fix space after for_each macros")). Or does
> this perhaps mean that there is broad agreement about the coding style
> parameters in the .clang-format file?
As I said, most likely a matter of taste :)
The pattern:
lio_wq = alloc_ordered_workqueue("%s", WQ_MEM_RECLAIM,
"efct_lio_worker");
follows the regular kernel coding style.
I only meant to say that I find the pattern:
lio_wq =
alloc_ordered_workqueue("%s", WQ_MEM_RECLAIM, "efct_lio_worker");
more pleasing visually. But the line may be too long anyway...
>
> Thanks,
>
> Bart.
>
>
--
Damien Le Moal
Western Digital Research
