| Message ID | eec1797734e3d080662aa732c565ed4a3c261799.1616506559.git.matti.vaittinen@fi.rohmeurope.com |
|---|---|
| State | Accepted |
| Commit | 2077ca682169afb212d8a887c70057a660290df9 |
| Headers | show |
| Series | Add managed version of delayed work init | expand |
On Tue, 2021-03-23 at 15:19 +0100, Greg KH wrote: > On Tue, Mar 23, 2021 at 02:58:28PM +0100, Hans de Goede wrote: > > Hi, > > > > On 3/23/21 2:56 PM, Matti Vaittinen wrote: > > > Devm helper header containing small inline helpers was added. > > > Hans promised to maintain it. > > > > > > Add Hans as maintainer and myself as designated reviewer. > > > > > Ultimately this is up to Greg though, so lets wait and see what > > Greg has to say about this. > > Can we move some of the devm_* calls in include/device.h into here as > well so that you all can be in charge of them instead of me? Seems like this was left w/o answer. I guess the question was pointed to Hans - but what comes to my (not always so humble) opinion - most of the devm functions in device.h are tightly related to the device interface or devres. Thus the device.h feels like appropriate place for most of those. OTOH, the kmalloc/kfree related functions, strdub and kmemdub might be candidates for move - those are not really "device things". But this is really not my call :) Best Regards Matti Vaittinen
Hi, On 4/21/21 9:51 AM, Matti Vaittinen wrote: > > On Tue, 2021-03-23 at 15:19 +0100, Greg KH wrote: >> On Tue, Mar 23, 2021 at 02:58:28PM +0100, Hans de Goede wrote: >>> Hi, >>> >>> On 3/23/21 2:56 PM, Matti Vaittinen wrote: >>>> Devm helper header containing small inline helpers was added. >>>> Hans promised to maintain it. >>>> >>>> Add Hans as maintainer and myself as designated reviewer. >>>> >>> Ultimately this is up to Greg though, so lets wait and see what >>> Greg has to say about this. >> >> Can we move some of the devm_* calls in include/device.h into here as >> well so that you all can be in charge of them instead of me? > > Seems like this was left w/o answer. I guess the question was pointed > to Hans I believe that Greg was (mostly) joking here. At least that is how I interpreted Greg's reply,which is why I did not answer. Also note that Greg merged this series, but not this patch, so the new devm-helpers.h file will presumably be maintained by Greg. Regards, Hans
On Wed, Apr 21, 2021 at 01:58:29PM +0200, Hans de Goede wrote: > Hi, > > On 4/21/21 9:51 AM, Matti Vaittinen wrote: > > > > On Tue, 2021-03-23 at 15:19 +0100, Greg KH wrote: > >> On Tue, Mar 23, 2021 at 02:58:28PM +0100, Hans de Goede wrote: > >>> Hi, > >>> > >>> On 3/23/21 2:56 PM, Matti Vaittinen wrote: > >>>> Devm helper header containing small inline helpers was added. > >>>> Hans promised to maintain it. > >>>> > >>>> Add Hans as maintainer and myself as designated reviewer. > >>>> > >>> Ultimately this is up to Greg though, so lets wait and see what > >>> Greg has to say about this. > >> > >> Can we move some of the devm_* calls in include/device.h into here as > >> well so that you all can be in charge of them instead of me? > > > > Seems like this was left w/o answer. I guess the question was pointed > > to Hans > > I believe that Greg was (mostly) joking here. At least that is how > I interpreted Greg's reply,which is why I did not answer. I have no idea what this thread was about anymore, sorry :) > Also note that Greg merged this series, but not this patch, > so the new devm-helpers.h file will presumably be maintained by Greg. What's one more file... thanks, greg k-h
On Wed, 2021-04-21 at 13:58 +0200, Hans de Goede wrote: > Hi, > > On 4/21/21 9:51 AM, Matti Vaittinen wrote: > > On Tue, 2021-03-23 at 15:19 +0100, Greg KH wrote: > > > On Tue, Mar 23, 2021 at 02:58:28PM +0100, Hans de Goede wrote: > > > > Hi, > > > > > > > > On 3/23/21 2:56 PM, Matti Vaittinen wrote: > > > > > Devm helper header containing small inline helpers was added. > > > > > Hans promised to maintain it. > > > > > > > > > > Add Hans as maintainer and myself as designated reviewer. > > > > > > > > > Ultimately this is up to Greg though, so lets wait and see what > > > > Greg has to say about this. > > > > > > Can we move some of the devm_* calls in include/device.h into > > > here as > > > well so that you all can be in charge of them instead of me? > > > > Seems like this was left w/o answer. I guess the question was > > pointed > > to Hans > > I believe that Greg was (mostly) joking here. At least that is how > I interpreted Greg's reply,which is why I did not answer. Ah. I missed the sarcastic tone of typing. I should've noted that by the font :] > Also note that Greg merged this series, but not this patch, > so the new devm-helpers.h file will presumably be maintained by Greg. Hmm. Are you sure? https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-next&id=2077ca682169afb212d8a887c70057a660290df9 Best Regards Matti Vaittinen
Hi, On 4/21/21 2:17 PM, Vaittinen, Matti wrote: > > On Wed, 2021-04-21 at 13:58 +0200, Hans de Goede wrote: >> Hi, >> >> On 4/21/21 9:51 AM, Matti Vaittinen wrote: >>> On Tue, 2021-03-23 at 15:19 +0100, Greg KH wrote: >>>> On Tue, Mar 23, 2021 at 02:58:28PM +0100, Hans de Goede wrote: >>>>> Hi, >>>>> >>>>> On 3/23/21 2:56 PM, Matti Vaittinen wrote: >>>>>> Devm helper header containing small inline helpers was added. >>>>>> Hans promised to maintain it. >>>>>> >>>>>> Add Hans as maintainer and myself as designated reviewer. >>>>>> >>>>> Ultimately this is up to Greg though, so lets wait and see what >>>>> Greg has to say about this. >>>> >>>> Can we move some of the devm_* calls in include/device.h into >>>> here as >>>> well so that you all can be in charge of them instead of me? >>> >>> Seems like this was left w/o answer. I guess the question was >>> pointed >>> to Hans >> >> I believe that Greg was (mostly) joking here. At least that is how >> I interpreted Greg's reply,which is why I did not answer. > > Ah. I missed the sarcastic tone of typing. I should've noted that by > the font :] > >> Also note that Greg merged this series, but not this patch, >> so the new devm-helpers.h file will presumably be maintained by Greg. > > Hmm. Are you sure? > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=driver-core-next&id=2077ca682169afb212d8a887c70057a660290df9 Ah, you're right. I was looking at the wrong branch, sorry about the confusion. Ok, so I guess I do maintain the new devm-helpers.h file, that is fine. Which makes your email from earlier today more relevant: > but what comes to my (not always so humble) opinion - most of > the devm functions in device.h are tightly related to the device > interface or devres. Thus the device.h feels like appropriate place for > most of those. I agree with you that most devm_ functions in device.h are probably left there. Moving them will also mean modifying all the drivers which use them to include the new devm-helpers.h include file which seems like needless churn. > OTOH, the kmalloc/kfree related functions, strdub and > kmemdub might be candidates for move - those are not really "device > things". I'm certainly open to moving some functions to devm-helpers.h, but also see above about needless churn. Regards, Hans
On Wed, 2021-04-21 at 14:09 +0200, Greg KH wrote: > On Wed, Apr 21, 2021 at 01:58:29PM +0200, Hans de Goede wrote: > > Hi, > > > > On 4/21/21 9:51 AM, Matti Vaittinen wrote: > > > On Tue, 2021-03-23 at 15:19 +0100, Greg KH wrote: > > > > On Tue, Mar 23, 2021 at 02:58:28PM +0100, Hans de Goede wrote: > > > > > Hi, > > > > > > > > > > On 3/23/21 2:56 PM, Matti Vaittinen wrote: > > > > > > Devm helper header containing small inline helpers was > > > > > > added. > > > > > > Hans promised to maintain it. > > > > > > > > > > > > Add Hans as maintainer and myself as designated reviewer. > > > > > > > > > > > Ultimately this is up to Greg though, so lets wait and see > > > > > what > > > > > Greg has to say about this. > > > > > > > > Can we move some of the devm_* calls in include/device.h into > > > > here as > > > > well so that you all can be in charge of them instead of me? > > > > > > Seems like this was left w/o answer. I guess the question was > > > pointed > > > to Hans > > > > I believe that Greg was (mostly) joking here. At least that is how > > I interpreted Greg's reply,which is why I did not answer. > > I have no idea what this thread was about anymore, sorry :) Need more B12 ;) Can't remember a minor discussion just a few thousand other patch mails ago? Huh, that's what the age does, right :p Well, this is not urgent in any way but here's some context: > This series implements delayed wq cancellation on top of devm and > replaces > the obvious cases where only thing remove call-back in a driver does > is > cancelling the work. There might be other cases where we could switch > more than just work cancellation to use managed version and thus get > rid > of remove or mixed (manual and devm) resource management. > > The series introduces include/linux/devm-helpers.h file which > hopefully works as a place where this kind of helpers can be inlined. and your reply: > Can we move some of the devm_* calls in include/device.h into here > as well ... I thought you meant that literally, Hans assumed you were joking - and you don't remember how it was :] So - if no further suggestions, then we keep the devm stuff which is currently in device.h still in device.h. Although the malloc/free stuff is not really strictly device thing. Best Regards Matti Vaittinen
On Wed, 2021-04-21 at 14:26 +0200, Hans de Goede wrote: > Hi, > > On 4/21/21 2:17 PM, Vaittinen, Matti wrote: > > On Wed, 2021-04-21 at 13:58 +0200, Hans de Goede wrote: > > > Hi, > > > > > > On 4/21/21 9:51 AM, Matti Vaittinen wrote: > > > > On Tue, 2021-03-23 at 15:19 +0100, Greg KH wrote: > > > > > On Tue, Mar 23, 2021 at 02:58:28PM +0100, Hans de Goede > > > > > wrote: > > > > > > Hi, > > > > > > > > > > > > On 3/23/21 2:56 PM, Matti Vaittinen wrote: > > > > > > > Devm helper header containing small inline helpers was > > > > > > > added. > > > > > > > Hans promised to maintain it. > > > > > > > > > > > > > > Add Hans as maintainer and myself as designated reviewer. > > > > > > > > > > > > > Ultimately this is up to Greg though, so lets wait and see > > > > > > what > > > > > > Greg has to say about this. > > > > > > > > > > Can we move some of the devm_* calls in include/device.h into > > > > > here as > > > > > well so that you all can be in charge of them instead of me? ... > > but what comes to my (not always so humble) opinion - most of > > the devm functions in device.h are tightly related to the device > > interface or devres. Thus the device.h feels like appropriate place > > for > > most of those. > > I agree with you that most devm_ functions in device.h are probably > left there. Moving them will also mean modifying all the drivers > which use them to include the new devm-helpers.h include file > which seems like needless churn. > > > OTOH, the kmalloc/kfree related functions, strdub and > > kmemdub might be candidates for move - those are not really "device > > things". > > I'm certainly open to moving some functions to devm-helpers.h, but > also see above about needless churn. I agree. Whether the move is worth all the hassle depends on how much Greg _really_ wishes to get rid of those devm functions. Especially the allocs are used _a lot_. Best Regards --Matti
diff --git a/MAINTAINERS b/MAINTAINERS index 9e876927c60d..fa5ac3164678 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5169,6 +5169,12 @@ M: Torben Mathiasen <device@lanana.org> S: Maintained W: http://lanana.org/docs/device-list/index.html +DEVICE RESOURCE MANAGEMENT HELPERS +M: Hans de Goede <hdegoede@redhat.com> +R: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> +S: Maintained +F: include/linux/devm-helpers.h + DEVICE-MAPPER (LVM) M: Alasdair Kergon <agk@redhat.com> M: Mike Snitzer <snitzer@redhat.com>
Devm helper header containing small inline helpers was added. Hans promised to maintain it. Add Hans as maintainer and myself as designated reviewer. Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> --- Changelog from RFCv2: - RFC dropped. No functional changes. MAINTAINERS | 6 ++++++ 1 file changed, 6 insertions(+)