Re: [PATCH v2 02/18] scsi: mptfusion: Simplify the alloc*_workqueue() invocations

[Damien Le Moal <dlemoal@kernel.org>]

On 8/20/24 02:08, Bart Van Assche wrote:
> On 8/18/24 4:51 PM, Damien Le Moal wrote:
>> On 8/17/24 06:55, Bart Van Assche wrote:
>>> Let alloc*_workqueue() format the workqueue names instead of calling
>>> snprintf() explicitly.
>>>
>>> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
>>
>> In patch 1, you have all the changes for removing the use of
>> create_singlethread_workqueue() in a single patch, touching different drivers.
>> But the series has 17 more patches to further cleanup the workqueue API use in
>> various drivers. So why not have the changes in patch 1 split into these
>> different driver patches with a title like "Cleanup and simplify workqueue API
>> use" ? That would make reviewing easier I think and avoid having the patch 2-17
>> changing again code that was changed in patch 1...
> 
> Hi Damien,
> 
> Thanks for having taken a look at this patch series. Would splitting
> patch 01/18 really help? Splitting that patch would make the description
> of the split patches longer than the actual code changes. That might
> annoy other reviewers. Additionally, isn't typical that Coccinelle
> patches are applied tree-wide instead of one driver at a time? A few
> examples:
> * 795f90c6f13c ("sysctl: treewide: constify argument
>                   ctl_table_root::permissions(table)").
> * e8058a49e67f ("netlink: introduce type-checking attribute iteration").

I know about script-based patches. But in this case, the script generated patch
changes lines of code that following patches change again (not all of them
though). So I thought splitting patch 1 may be a good idea as that would as well
isolate driver changes in their own patches, which definitely should facilitate
reviewing by the driver maintainers.

But no strong feelings about all this. If you do not want to do that, fine.

> 
> Thanks,
> 
> Bart.
> 

-- 
Damien Le Moal
Western Digital Research