On 3/27/23 15:01, Greg Kroah-Hartman wrote:
On Mon, Mar 27, 2023 at 02:34:02PM +0300, Matti Vaittinen wrote:
A few tests need to have a valid struct device. One such example is
tests which want to be testing devm-managed interfaces.
Add kunit wrapper for root_device_[un]register(), which create a root
device and also add a kunit managed clean-up routine for the device
destruction upon test exit.
I really do not like this as a "root device" is a horrible hack and
should only be used if you have to hang other devices off of it and you
don't have a real device to tie those devices to.
Here you are abusing it and attempting to treat it as a real device,
which it is not at all, because:
Special note: In some cases the device reference-count does not reach
zero and devm-unwinding is not done if device is not sitting on a bus.
The root_device_[un]register() are dealing with such devices and thus
this interface may not be usable by all in its current form. More
information can be found from:
https://lore.kernel.org/dri-devel/20221117165311.vovrc7usy4efiytl@houat/
See, not a real device, doesn't follow normal "struct device" rules and
lifetimes, don't try to use it for a test as it will only cause problems
and you will be forced to work around that in a test.
Ok. I understood using the root-device has been a work-around in some
other tests. Thus continuing use it for tests where we don't need the
bus until we have a proper alternative was suggested by David.
Do the right thing here, create a fake bus and add devices to it.
Heck, I'll even write that code if you want it, what's the requirement,
something like:
struct device *kunit_device_create(struct kunit *test, const char *name);
void kunit_device_destroy(struct device *dev);
Thanks for the offer Greg. This, however, is being already worked on by
David. I don't want to step on his toes by writing the same thing, nor
do I think I should be pushing him to rush on his work.
Why do you want a "match" function? You don't provide documentation
here for it so I have no idea.
Anything else needed?
The use of root-devices in the kunit helpers is intended to be an
intermediate solution to allow tests which do not require device to sit
on a bus avoid directly abusing the root_device_[un]register() while
proper kunit device solution is being worked on. Related discussion can be
found from:
https://lore.kernel.org/lkml/CABVgOSmx3A4Vwos2_8xO-XQrQAw5gvY0nc5zLpLmcJ7FtA-dTQ@xxxxxxxxxxxxxx/
Again, no, please let's not get this wrong now and say "we will fix this
later" as that's not how kernel development should work...
Ok. In that case I need to drop the tests from the series until we get
the new APIs in place. It really sucks but I guess I understand the
rationale for not wanting to "intermediate" solutions merged. Yes, I
hoped it'd be Ok as David is already working on it - but I was still
kind of expecting your response. This is why I made it very clear in the
cover-letter and this commit message what is suggested here.
Jonathan, should I re-spin the series without patches 3/7 and 5/7 or can
you please review this and I'll just drop those for the next version?
Thanks for the review Greg, I think this case is now "closed".
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
