linux-kernel - Re: [PATCH v5 1/8] drivers: kunit: Generic helpers for test device creation

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABVgOSmSEYRqxTXCEttZvpE33euxvRYE-3scZ8DrOSibeZW=bg@mail.gmail.com>
Date:   Sat, 25 Mar 2023 12:35:55 +0800
From:   David Gow <davidgow@...gle.com>
To:     Matti Vaittinen <mazziesaccount@...il.com>
Cc:     Maxime Ripard <maxime@...no.tech>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.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 v5 1/8] drivers: kunit: Generic helpers for test device creation

On Fri, 24 Mar 2023 at 18:17, Matti Vaittinen <mazziesaccount@...il.com> wrote:
>
> On 3/24/23 12:05, Matti Vaittinen wrote:
> > On 3/24/23 11:52, David Gow wrote:
> >> On Fri, 24 Mar 2023 at 14:51, Matti Vaittinen
> >> <mazziesaccount@...il.com> wrote:
> >>>
> >>> On 3/24/23 08:34, David Gow wrote:
> >>>> On Fri, 24 Mar 2023 at 14:11, Matti Vaittinen
> >>>> <mazziesaccount@...il.com> wrote:
> >
> >>>> I think that sounds like a good strategy for now, and we can work on a
> >>>> set of 'generic helpers' which have an associated bus and struct
> >>>> kunit_device in the meantime. If we can continue to use
> >>>> root_device_register until those are ready, that'd be very convenient.
> >>>
> >>> Would it be a tiny bit more acceptable if we did add a very simple:
> >>>
> >>> #define kunit_root_device_register(name) root_device_register(name)
> >>> #define kunit_root_device_unregister(dev) root_device_unregister(dev)
> >>>
> >>> to include/kunit/device.h (or somesuch)
> >>>
> >>> This should help us later to at least spot the places where
> >>> root_device_[un]register() is abused and (potentially mass-)covert them
> >>> to use the proper helpers when they're available.
> >>>
> >>
> >> Great idea.
> >>
> >> The code I've been playing with has the following in
> >> include/kunit/device.h:
> >>
> >> /* Register a new device against a KUnit test. */
> >> struct device *kunit_device_register(struct kunit *test, const char
> >> *name);
> >> /* Unregister a device created by kunit_device_register() early (i.e.,
> >> before test cleanup). */
> >> void kunit_device_unregister(struct kunit *test, struct device *dev);
> >>
> >> If we used the same names, and just forwarded them to
> >> root_device_register() and root_device_unregister() for now
> >> (discarding the struct kunit pointer), then I expect we could just
> >> swap out the implementation to gain the extra functionality.
>
> There's one thing though. If the goal is to do a direct replacement and
> if automatic device deletion upon test completion / test abort is
> planned - then it should be there also for these initial wrappers.
>

Yeah, that's an excellent point. It's a pretty subtle change in
behaviour to suddenly introduce that, so changing it behind the scenes
is probably unwise.

> If these wrappers don't yet include the automatic device clean-up - then
> it probably makes more sense to just do the kunit_root_device_* defines
> because the tests are likely to need removing the explicit device
> clean-ups when proper APIs are finished.
>

I sent out my prototype implementation of this here, which does do the
automatic cleanup:
https://lore.kernel.org/linux-kselftest/20230325043104.3761770-1-davidgow@google.com/T/#mf797239a8bce11630875fdf60aab9ed627add1f0

It's probably overkill to squeeze into your patch series, though,
given it also adds and uses a whole new kunit_defer() API.

Cheers,
-- David

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4003 bytes)