* [PATCH v4 0/2] kunit: fix minor error path mistakes
@ 2024-04-19 13:25 Wander Lairson Costa
2024-04-19 13:25 ` [PATCH v4 1/2] kunit: unregister the device on error Wander Lairson Costa
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Wander Lairson Costa @ 2024-04-19 13:25 UTC (permalink / raw
To: Brendan Higgins, David Gow, Rae Moar, Matti Vaittinen,
Greg Kroah-Hartman, Shuah Khan, Maxime Ripard,
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit), open list
Cc: Wander Lairson Costa
Hi,
These two patches fix some minor error path mistakes in the device
module.
Changes
-------
v1->v2
* Add fixes tag
* Add imperative statement in the commit description
v2->v3
* Add a goto exit label kunit_device_register_internal
v3->v4
* Remove some changes requested by Marcus Elfring, as I was alerted he
is a known troll.
Wander Lairson Costa (2):
kunit: unregister the device on error
kunit: avoid memory leak on device register error
lib/kunit/device.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
--
2.44.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/2] kunit: unregister the device on error
2024-04-19 13:25 [PATCH v4 0/2] kunit: fix minor error path mistakes Wander Lairson Costa
@ 2024-04-19 13:25 ` Wander Lairson Costa
2024-04-19 14:04 ` Greg Kroah-Hartman
2024-04-19 13:25 ` [PATCH v4 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
2024-04-19 16:18 ` [PATCH v4 0/2] kunit: fix minor error path mistakes Markus Elfring
2 siblings, 1 reply; 10+ messages in thread
From: Wander Lairson Costa @ 2024-04-19 13:25 UTC (permalink / raw
To: Brendan Higgins, David Gow, Rae Moar, Matti Vaittinen, Shuah Khan,
Greg Kroah-Hartman, Maxime Ripard,
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit), open list
Cc: Wander Lairson Costa
kunit_init_device() should unregister the device on bus register error,
but mistakenly it tries to unregister the bus.
Unregister the device instead of the bus.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Fixes: d03c720e03bd ("kunit: Add APIs for managing devices")
---
lib/kunit/device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/device.c b/lib/kunit/device.c
index abc603730b8e..25c81ed465fb 100644
--- a/lib/kunit/device.c
+++ b/lib/kunit/device.c
@@ -51,7 +51,7 @@ int kunit_bus_init(void)
error = bus_register(&kunit_bus_type);
if (error)
- bus_unregister(&kunit_bus_type);
+ root_device_unregister(kunit_bus_device);
return error;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] kunit: avoid memory leak on device register error
2024-04-19 13:25 [PATCH v4 0/2] kunit: fix minor error path mistakes Wander Lairson Costa
2024-04-19 13:25 ` [PATCH v4 1/2] kunit: unregister the device on error Wander Lairson Costa
@ 2024-04-19 13:25 ` Wander Lairson Costa
2024-04-19 14:03 ` Greg Kroah-Hartman
2024-04-19 16:18 ` [PATCH v4 0/2] kunit: fix minor error path mistakes Markus Elfring
2 siblings, 1 reply; 10+ messages in thread
From: Wander Lairson Costa @ 2024-04-19 13:25 UTC (permalink / raw
To: Brendan Higgins, David Gow, Rae Moar, Shuah Khan, Matti Vaittinen,
Greg Kroah-Hartman, Maxime Ripard,
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit), open list
Cc: Wander Lairson Costa
If the device register fails, free the allocated memory before
returning.
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
Fixes: d03c720e03bd ("kunit: Add APIs for managing devices")
---
lib/kunit/device.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/lib/kunit/device.c b/lib/kunit/device.c
index 25c81ed465fb..d8c09dcb3e79 100644
--- a/lib/kunit/device.c
+++ b/lib/kunit/device.c
@@ -131,6 +131,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test,
err = device_register(&kunit_dev->dev);
if (err) {
put_device(&kunit_dev->dev);
+ kfree(kunit_dev);
return ERR_PTR(err);
}
--
2.44.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] kunit: avoid memory leak on device register error
2024-04-19 13:25 ` [PATCH v4 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
@ 2024-04-19 14:03 ` Greg Kroah-Hartman
2024-04-19 14:11 ` Wander Lairson Costa
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2024-04-19 14:03 UTC (permalink / raw
To: Wander Lairson Costa
Cc: Brendan Higgins, David Gow, Rae Moar, Shuah Khan, Matti Vaittinen,
Maxime Ripard, open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit), open list
On Fri, Apr 19, 2024 at 10:25:02AM -0300, Wander Lairson Costa wrote:
> If the device register fails, free the allocated memory before
> returning.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> Fixes: d03c720e03bd ("kunit: Add APIs for managing devices")
> ---
> lib/kunit/device.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/kunit/device.c b/lib/kunit/device.c
> index 25c81ed465fb..d8c09dcb3e79 100644
> --- a/lib/kunit/device.c
> +++ b/lib/kunit/device.c
> @@ -131,6 +131,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test,
> err = device_register(&kunit_dev->dev);
> if (err) {
> put_device(&kunit_dev->dev);
> + kfree(kunit_dev);
This still looks wrong, the release function for the device should free
the memory here, not this kfree, as the reference count in the embedded
'struct device' handles the memory logic for the whole structure (if
not, then something is REALLY wrong...)
You _do_ have a release function for the device, right? If not, you
should be getting loud messages in the kernel log when releasing a
device here.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] kunit: unregister the device on error
2024-04-19 13:25 ` [PATCH v4 1/2] kunit: unregister the device on error Wander Lairson Costa
@ 2024-04-19 14:04 ` Greg Kroah-Hartman
2024-04-19 16:32 ` Markus Elfring
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2024-04-19 14:04 UTC (permalink / raw
To: Wander Lairson Costa
Cc: Brendan Higgins, David Gow, Rae Moar, Matti Vaittinen, Shuah Khan,
Maxime Ripard, open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit), open list
On Fri, Apr 19, 2024 at 10:25:01AM -0300, Wander Lairson Costa wrote:
> kunit_init_device() should unregister the device on bus register error,
> but mistakenly it tries to unregister the bus.
>
> Unregister the device instead of the bus.
>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> Fixes: d03c720e03bd ("kunit: Add APIs for managing devices")
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] kunit: avoid memory leak on device register error
2024-04-19 14:03 ` Greg Kroah-Hartman
@ 2024-04-19 14:11 ` Wander Lairson Costa
2024-04-23 8:00 ` David Gow
0 siblings, 1 reply; 10+ messages in thread
From: Wander Lairson Costa @ 2024-04-19 14:11 UTC (permalink / raw
To: Greg Kroah-Hartman
Cc: Brendan Higgins, David Gow, Rae Moar, Shuah Khan, Matti Vaittinen,
Maxime Ripard, open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit), open list
On Fri, Apr 19, 2024 at 11:03 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Fri, Apr 19, 2024 at 10:25:02AM -0300, Wander Lairson Costa wrote:
> > If the device register fails, free the allocated memory before
> > returning.
> >
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > Fixes: d03c720e03bd ("kunit: Add APIs for managing devices")
> > ---
> > lib/kunit/device.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/kunit/device.c b/lib/kunit/device.c
> > index 25c81ed465fb..d8c09dcb3e79 100644
> > --- a/lib/kunit/device.c
> > +++ b/lib/kunit/device.c
> > @@ -131,6 +131,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test,
> > err = device_register(&kunit_dev->dev);
> > if (err) {
> > put_device(&kunit_dev->dev);
> > + kfree(kunit_dev);
>
> This still looks wrong, the release function for the device should free
> the memory here, not this kfree, as the reference count in the embedded
> 'struct device' handles the memory logic for the whole structure (if
> not, then something is REALLY wrong...)
>
> You _do_ have a release function for the device, right? If not, you
> should be getting loud messages in the kernel log when releasing a
> device here.
>
Ok, I got it. Yes, there is a release function. So this patch is
wrong, ignore it. Should I send a v5 with only the other patch?
> thanks,
>
> greg k-h
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 0/2] kunit: fix minor error path mistakes
2024-04-19 13:25 [PATCH v4 0/2] kunit: fix minor error path mistakes Wander Lairson Costa
2024-04-19 13:25 ` [PATCH v4 1/2] kunit: unregister the device on error Wander Lairson Costa
2024-04-19 13:25 ` [PATCH v4 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
@ 2024-04-19 16:18 ` Markus Elfring
2 siblings, 0 replies; 10+ messages in thread
From: Markus Elfring @ 2024-04-19 16:18 UTC (permalink / raw
To: Wander Lairson Costa, kunit-dev, linux-kselftest, kernel-janitors,
Brendan Higgins, David Gow, Greg Kroah-Hartman, Matti Vaittinen,
Maxime Ripard, Rae Moar, Shuah Khan
Cc: LKML
…
> * Remove some changes requested by Marcus Elfring,
I became curious how affected software components can evolve further.
> as I was alerted he is a known troll.
I would appreciate if this interpretation will be reconsidered somehow.
Regards,
Markus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] kunit: unregister the device on error
2024-04-19 14:04 ` Greg Kroah-Hartman
@ 2024-04-19 16:32 ` Markus Elfring
2024-04-20 6:27 ` Greg Kroah-Hartman
0 siblings, 1 reply; 10+ messages in thread
From: Markus Elfring @ 2024-04-19 16:32 UTC (permalink / raw
To: Wander Lairson Costa, kunit-dev, linux-kselftest, kernel-janitors,
Brendan Higgins, David Gow, Greg Kroah-Hartman, Matti Vaittinen,
Maxime Ripard, Rae Moar, Shuah Khan
Cc: LKML
> > kunit_init_device() should unregister the device on bus register error,
> > but mistakenly it tries to unregister the bus.
> >
> > Unregister the device instead of the bus.
…
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Would you ever like to distinguish hardware register errors from
item registration failures according to further improved commit messages?
Regards,
Markus
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] kunit: unregister the device on error
2024-04-19 16:32 ` Markus Elfring
@ 2024-04-20 6:27 ` Greg Kroah-Hartman
0 siblings, 0 replies; 10+ messages in thread
From: Greg Kroah-Hartman @ 2024-04-20 6:27 UTC (permalink / raw
To: Markus Elfring
Cc: Wander Lairson Costa, kunit-dev, linux-kselftest, kernel-janitors,
Brendan Higgins, David Gow, Matti Vaittinen, Maxime Ripard,
Rae Moar, Shuah Khan, LKML
On Fri, Apr 19, 2024 at 06:32:05PM +0200, Markus Elfring wrote:
> > > kunit_init_device() should unregister the device on bus register error,
> > > but mistakenly it tries to unregister the bus.
> > >
> > > Unregister the device instead of the bus.
> …
> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Would you ever like to distinguish hardware register errors from
> item registration failures according to further improved commit messages?
Hi,
This is the semi-friendly patch-bot of Greg Kroah-Hartman.
Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list. I strongly suggest that you not do this anymore. Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.
Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all. The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback. Please feel free to also ignore emails
from them.
thanks,
greg k-h's patch email bot
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] kunit: avoid memory leak on device register error
2024-04-19 14:11 ` Wander Lairson Costa
@ 2024-04-23 8:00 ` David Gow
0 siblings, 0 replies; 10+ messages in thread
From: David Gow @ 2024-04-23 8:00 UTC (permalink / raw
To: Wander Lairson Costa
Cc: Greg Kroah-Hartman, Brendan Higgins, Rae Moar, Shuah Khan,
Matti Vaittinen, Maxime Ripard,
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit), open list
[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]
On Fri, 19 Apr 2024 at 22:11, Wander Lairson Costa <wander@redhat.com> wrote:
>
> On Fri, Apr 19, 2024 at 11:03 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Fri, Apr 19, 2024 at 10:25:02AM -0300, Wander Lairson Costa wrote:
> > > If the device register fails, free the allocated memory before
> > > returning.
> > >
> > > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> > > Fixes: d03c720e03bd ("kunit: Add APIs for managing devices")
> > > ---
> > > lib/kunit/device.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/kunit/device.c b/lib/kunit/device.c
> > > index 25c81ed465fb..d8c09dcb3e79 100644
> > > --- a/lib/kunit/device.c
> > > +++ b/lib/kunit/device.c
> > > @@ -131,6 +131,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test,
> > > err = device_register(&kunit_dev->dev);
> > > if (err) {
> > > put_device(&kunit_dev->dev);
> > > + kfree(kunit_dev);
> >
> > This still looks wrong, the release function for the device should free
> > the memory here, not this kfree, as the reference count in the embedded
> > 'struct device' handles the memory logic for the whole structure (if
> > not, then something is REALLY wrong...)
> >
> > You _do_ have a release function for the device, right? If not, you
> > should be getting loud messages in the kernel log when releasing a
> > device here.
> >
>
> Ok, I got it. Yes, there is a release function. So this patch is
> wrong, ignore it. Should I send a v5 with only the other patch?
>
Thanks. Don't worry about sending a v5: just patch 1 of v4 is now in
the kunit branch:
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit&id=63761ec5971ea47c1f2d7698f03e1c6ffb9fb22a
Cheers,
-- David
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4014 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-04-23 8:00 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-19 13:25 [PATCH v4 0/2] kunit: fix minor error path mistakes Wander Lairson Costa
2024-04-19 13:25 ` [PATCH v4 1/2] kunit: unregister the device on error Wander Lairson Costa
2024-04-19 14:04 ` Greg Kroah-Hartman
2024-04-19 16:32 ` Markus Elfring
2024-04-20 6:27 ` Greg Kroah-Hartman
2024-04-19 13:25 ` [PATCH v4 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
2024-04-19 14:03 ` Greg Kroah-Hartman
2024-04-19 14:11 ` Wander Lairson Costa
2024-04-23 8:00 ` David Gow
2024-04-19 16:18 ` [PATCH v4 0/2] kunit: fix minor error path mistakes Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).