Re: [PATCH net] net: ti: icssg-prueth: Fix race condition for VLAN table access

From: MD Danish Anwar
Date: Fri Oct 04 2024 - 00:55:51 EST




On 04/10/24 6:11 am, Jakub Kicinski wrote:
> On Thu, 3 Oct 2024 16:29:40 +0530 MD Danish Anwar wrote:
>> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.h b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> index bba6da2e6bd8..9a33e9ed2976 100644
>> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.h
>> @@ -296,6 +296,7 @@ struct prueth {
>> bool is_switchmode_supported;
>> unsigned char switch_id[MAX_PHYS_ITEM_ID_LEN];
>> int default_vlan;
>> + spinlock_t vtbl_lock; /* Lock for vtbl in shared memory */
>
> This needs to be kdoc, otherwise:
>
> drivers/net/ethernet/ti/icssg/icssg_prueth.h:301: warning: Function parameter or struct member 'vtbl_lock' not described in 'prueth'

Hi Jakub,

Removing the documentation from here and keeping it in kdoc results in
below checkpatch,

CHECK: spinlock_t definition without comment
#69: FILE: drivers/net/ethernet/ti/icssg/icssg_prueth.h:300:
+ spinlock_t vtbl_lock;


What should be done here? Should I,

1. Move the documentation to kdoc - This is will result in checkpatch
2. Keep the documentation in kdoc as well as inline - This will result
in no warnings but duplicate documentation which I don't think is good.

I was not sure which one takes more precedence check patch or kdoc, thus
put it inline thinking fixing checkpatch might have more weightage.

Let me know what should be done here.

--
Thanks and Regards,
Danish