netdev - Re: [PATCH net-next v4 1/2] driver core: auxiliary bus: show auxiliary device IRQs

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: <b3baab48-647e-4ba2-9d0a-fcef64984e56@nvidia.com>
Date: Sun, 12 May 2024 10:30:02 +0300
From: Shay Drori <shayd@...dia.com>
To: Greg KH <gregkh@...uxfoundation.org>, Przemek Kitszel
	<przemyslaw.kitszel@...el.com>
CC: <netdev@...r.kernel.org>, <pabeni@...hat.com>, <davem@...emloft.net>,
	<kuba@...nel.org>, <edumazet@...gle.com>, <david.m.ertman@...el.com>,
	<rafael@...nel.org>, <ira.weiny@...el.com>, <linux-rdma@...r.kernel.org>,
	<leon@...nel.org>, <tariqt@...dia.com>, Parav Pandit <parav@...dia.com>
Subject: Re: [PATCH net-next v4 1/2] driver core: auxiliary bus: show
 auxiliary device IRQs



On 11/05/2024 10:44, Greg KH wrote:
> On Fri, May 10, 2024 at 04:01:01PM +0200, Przemek Kitszel wrote:
>> On 5/10/24 15:07, Greg KH wrote:
>>> On Fri, May 10, 2024 at 02:54:49PM +0200, Przemek Kitszel wrote:
>>>>>> +static ssize_t auxiliary_irq_mode_show(struct device *dev,
>>>>>> +				       struct device_attribute *attr, char *buf)
>>>>>> +{
>>>>>> +	struct auxiliary_irq_info *info =
>>>>>> +		container_of(attr, struct auxiliary_irq_info, sysfs_attr);
>>>>>> +
>>>>>> +	if (refcount_read(xa_load(&irqs, info->irq)) > 1)
>>>>>
>>>>> refcount combined with xa?  That feels wrong, why is refcount used for
>>>>> this at all?
>>>>
>>>> Not long ago I commented on similar usage for ice driver,
>>>> ~"since you are locking anyway this could be a plain counter",
>>>> and author replied
>>>> ~"additional semantics (like saturation) of refcount make me feel warm
>>>> and fuzzy" (sorry if misquoting too much).
>>>> That convinced me back then, so I kept quiet about that here.
>>>
>>> But why is this being incremented / decremented at all?  What is that
>>> for?
>>
>> [global]
>> This is just a counter, it is used to tell if given IRQ is shared or
>> exclusive. Hence there is a global xarray for that.
>> And my argument is for this case precisely.
>>
>> [other]
>> There is also a separate xarray for each auxdev (IIRC) which is used as
>> generic dynamic container [that stores sysfs attrs], any other would
>> work (with different characteristics), but I see no problems with
>> picking xarray here.
> 
> Again, why is an xarray needed, why isn't this part of the auxdevice
> structure to start with?

If I understand you correctly, you are referring to the xarray of the
auxdevice (not the global one).
If so, instead of xarray what can be used by the auxdevice?

> 
> thanks,
> 
> greg k-h
>