Re: [PRE-REVIEW PATCH 00/16] Modernize the tasklet API

Romain Perier
2019-11-07T07:30:13Z

Le mer. 30 oct. 2019 à 09:21, Allen <allen.lkml@gmail.com> a écrit :
>
> Romain,
> >
>
> First of all Romain, nice work. I started working on this
> set a few months back, but could only carve out limited time.
>
> I sent out RFC for this sometime in May[1]. And my approach
> was a little different when compared to what you have sent on the
> list.
>
> Well, I have pushed my work to github[2], only thing I could
> think of as an improvement in your patch set it to break it down
> into smaller chunk so that it's easier to review. I have made each
> occurrence of tasklet_init() into a commit[3] which I thought would
> make it easier to review. I'll leave that decision to you and kees.
>
> Let me know if I could help in any way.
>
> [1] https://www.openwall.com/lists/kernel-hardening/2019/05/06/1
> [2] https://github.com/allenpais/tasklet
> [3] Sample list of patches:
> 5d0b728649b6 atm/solos-pci: Convert tasklets to use new tasklet_init API
> e5144c3c16d8 atm: Convert tasklets to use new tasklet_init API
> 71028976d3ed arch/um: Convert tasklets to use new tasklet_init API
> c9a39c23b78c xfrm: Convert tasklets to use new tasklet_init API
> 91d93fe12bbc mac80211: Convert tasklets to use new tasklet_init API
> d68f1e9e4531 ipv4: Convert tasklets to use new tasklet_init API
> 4f9379dcd8ad sound/timer: Convert tasklets to use new tasklet_init API
> b4519111b75e drivers/usb: Convert tasklets to use new tasklet_init API
> 52f04bf54a5a drivers:vt/keyboard: Convert tasklets to use new tasklet_init API
> 295de7c9812c dma/virt-dma: Convert tasklets to use new tasklet_init API
> 6c713c83b58f dma/dw: Convert tasklets to use new tasklet_init API
> eaaaaba8a4a7 debug:Convert tasklets to use new tasklet_init API
> b23f4ff5021b tasklet: prepare to change tasklet API

From experience, this is better to group bunch of commits like we
currently do with Kees on this series, instead to have one commit per
change (I mean for huge patchset)
Mainly because you have too much replacements with this API change,
and it will be really complicated to merge.

Last time I have proposed an API change for removing "pci_pool" , it
was a patchset of 20 commits (something like this), it tooks 6 months
to be merged :) (with a fine grain granularity on each commit)

This is better to be the more atomic as possible. If we split the "one
massive tasklet_init replacement" commit into many commit, I am sure
that we find old tasklet API for months in the kernel... it is not
something we want , imho. + treewide commits are common in the kernel
tree, for important API changes :)

@Kees: agreed ?

I think that the timer_list approach is good. You can help by
providing feedbacks and by testing if you want.

Regards,
Romain

>
> Thanks,
> - Allen