lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <e027fc0c-83e0-be6f-d62b-dac00ce9b761@gmail.com> Date: Mon, 27 Mar 2023 15:20:06 +0300 From: Matti Vaittinen <mazziesaccount@...il.com> To: Greg Kroah-Hartman <gregkh@...uxfoundation.org> Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Brendan Higgins <brendan.higgins@...ux.dev>, David Gow <davidgow@...gle.com>, linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, Stephen Boyd <sboyd@...nel.org>, Jonathan Cameron <jic23@...nel.org>, linux-iio@...r.kernel.org Subject: Re: [PATCH v6 3/7] kunit: Add kunit wrappers for (root) device creation 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@mail.gmail.com/ > > 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! ~~
Powered by blists - more mailing lists