linux-kernel - Re: [PATCH 3/5] ACPI: battery: Allow battery hooks to be registered multiple times.

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: <a60266df-bf73-57e7-129a-f5395d373f1f@redhat.com>
Date:   Mon, 19 Sep 2022 12:18:53 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Armin Wolf <W_Armin@....de>,
        Barnabás Pőcze <pobrn@...tonmail.com>
Cc:     markgross@...nel.org, rafael@...nel.org, lenb@...nel.org,
        hmh@....eng.br, matan@...alib.org, corentin.chary@...il.com,
        jeremy@...tem76.com, productdev@...tem76.com,
        platform-driver-x86@...r.kernel.org, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/5] ACPI: battery: Allow battery hooks to be registered
 multiple times.

Hi,

On 9/12/22 18:29, Armin Wolf wrote:
> Am 12.09.22 um 18:42 schrieb Barnabás Pőcze:
> 
>> Hi
>>
>> 2022. szeptember 12., hétfő 14:53 keltezéssel, Armin Wolf írta:
>>
>>> Registering multiple instances of a battery hook is beneficial
>>> for drivers which can be instantiated multiple times. Until now,
>>> this would mean that such a driver would have to implement some
>>> logic to manage battery hooks.
>>>
>>> Extend the battery hook handling instead.
>> I think this is already possible by embedding the acpi_battery_hook
>> object inside the driver's device specific data object, no?
>>
>> Regards,
>> Barnabás Pőcze
>>
>>
> Yes, it indeed is. However afaik it is not possible to pass instance-specific
> data to such an embedded battery hook. It could be possible by passing the
> battery hook as an argument to add_battery()/remove_battery() and using container_of(),
> but in my opinion this would be too much of a quick hack.

Actually thinking more about this (after my reviewed-by of 4/5) I believe
that leaving the lifetime management of the struct acpi_battery_hook hook
in the caller and then modifying 4/4 to pass the hook to the callback,
so that the callback can indeed do container_of to get its top-level
driver-data struct would be a better/cleaner solution then this patch +
patch 4/5 .

Doing things this way is quite normal in the kernel and doing a single
large alloc is better then a bunch of small allocs. In this case it does
not really matter, but if we do things like this over all drivers
eventually all the small mallocs add up.

Doing things this way would also reduce the amount of churn in this
series a bit.

Regards,

Hans