netdev - Re: [PATCH 08/15] devlink: add devres managed devlinkm_alloc and devlinkm

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: <b7481d02-1635-9855-13a6-97016eaf4004@intel.com>
Date:   Fri, 31 Jan 2020 10:16:59 -0800
From:   Jacob Keller <jacob.e.keller@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     netdev@...r.kernel.org, jiri@...nulli.us, valex@...lanox.com,
        linyunsheng@...wei.com, lihong.yang@...el.com
Subject: Re: [PATCH 08/15] devlink: add devres managed devlinkm_alloc and
 devlinkm_free

On 1/31/2020 10:07 AM, Jakub Kicinski wrote:
> On Thu, 30 Jan 2020 14:59:03 -0800, Jacob Keller wrote:
>> Add devres managed allocation functions for allocating a devlink
>> instance. These can be used by device drivers based on the devres
>> framework which want to allocate a devlink instance.
>>
>> For simplicity and to reduce churn in the devlink core code, the devres
>> management works by creating a node with a double-pointer. The devlink
>> instance is allocated using the normal devlink_alloc and released using
>> the normal devlink_free.
>>
>> An alternative solution where the raw memory for devlink is allocated
>> directly via devres_alloc could be done. Such an implementation would
>> either significantly increase code duplication or code churn in order to
>> refactor the setup from the allocation.
>>
>> The new devres managed allocation function will be used by the ice
>> driver in a following change to implement initial devlink support.
>>
>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> 
> How much are you actually going to gain by doing this given you still
> have to deal with registering an unregistering all devlink things...
> 

So, the main problem is that the ice driver's private PF structure is
allocated using devm_alloc.. and if we switch to using devlink_alloc
then that memory is no longer managed by devres and it becomes more
difficult to manage...

That being said, after looking at exactly how the ice driver uses devres
and managing things.. Probably not much gain in general. The ice driver
could implement its own call for this if we still need it.

In theory the register/unregister of devlink resources could be managed
by using devres_add_action to handle the teardown.. but that's a fairly
major refactor to get everything working.