Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Thu, 19 Jan 2023 16:05:09 +0100 | Subject | Re: [PATCH 3/5] thermal/core: Remove unneeded mutex_destroy() |
| |
On Thu, Jan 19, 2023 at 3:13 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > On 19/01/2023 14:24, Rafael J. Wysocki wrote: > > On Thu, Jan 19, 2023 at 1:48 PM Daniel Lezcano > > <daniel.lezcano@linaro.org> wrote: > >> > >> On 19/01/2023 13:11, Rafael J. Wysocki wrote: > >>> On Thu, Jan 19, 2023 at 10:30 AM Daniel Lezcano > >>> <daniel.lezcano@linaro.org> wrote: > >>>> > >>>> On 19/01/2023 08:41, Zhang, Rui wrote: > >>>>> On Wed, 2023-01-18 at 22:11 +0100, Daniel Lezcano wrote: > >>>>>> If the thermal framework fails to initialize, the mutex can be used > >>>>>> by > >>>>>> the different functions registering a thermal zone anyway. > >>>>> > >>>>> Hmm, even with no governors and unregistered thermal sysfs class? > >>>>> > >>>>> IMO, thermal APIs for registering a thermal_zone/cooling_device should > >>>>> yield early if thermal_init fails. > >>>>> For other APIs that relies on a valid > >>>>> thermal_zone_device/thermal_cooling_device pointer, nothing needs to > >>>>> be changed. > >>>>> > >>>>> what do you think? > >>>> > >>>> I think you are right. > >>>> > >>>> It would be nice if we can check if the thermal class is registered and > >>>> bail out if not. But there is no function to check that AFAICS. > >>>> > >>>> Alternatively we can convert the thermal class static structure to a > >>>> pointer and set it to NULL in case of error in thermal_init() ? > >>> > >>> It doesn't matter if this is a NULL pointer or a static object that's > >>> clearly marked as unused. > >> > >> Without introducing another global variable, is it possible to know if > >> the class is used or not ? > > > > If thermal_class.p is cleared to NULL on class_register() failures in > > thermal_init() (unfortunately, the driver core doesn't do that, but > > maybe it should - let me cut a patch for that), then it can be used > > for that. > > It should be in class_unregister() too, right ? > > And is it possible to add a class_is_registered() ? in order to prevent > accessing class structure internals ?
I suppose so.
And we'd like it to be used some places like thermal_zone_device_register_with_trips(), wouldn't we?
| |