lkml.org 
[lkml]   [2024]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3] net: stmmac: move the lock to struct plat_stmmacenet_data
From

On 5/8/24 8:56 PM, Serge Semin wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, May 08, 2024 at 12:52:57PM +0800, Xiaolei Wang wrote:
>> Reinitialize the whole est structure would also reset the mutex lock
>> which is embedded in the est structure, and then trigger the following
>> warning. To address this, move the lock to struct plat_stmmacenet_data.
>> We also need to require the mutex lock when doing this initialization.
>>
>> DEBUG_LOCKS_WARN_ON(lock->magic != lock)
>> WARNING: CPU: 3 PID: 505 at kernel/locking/mutex.c:587 __mutex_lock+0xd84/0x1068
>> Modules linked in:
>> CPU: 3 PID: 505 Comm: tc Not tainted 6.9.0-rc6-00053-g0106679839f7-dirty #29
>> Hardware name: NXP i.MX8MPlus EVK board (DT)
>> pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
>> pc : __mutex_lock+0xd84/0x1068
>> lr : __mutex_lock+0xd84/0x1068
>> sp : ffffffc0864e3570
>> x29: ffffffc0864e3570 x28: ffffffc0817bdc78 x27: 0000000000000003
>> x26: ffffff80c54f1808 x25: ffffff80c9164080 x24: ffffffc080d723ac
>> x23: 0000000000000000 x22: 0000000000000002 x21: 0000000000000000
>> x20: 0000000000000000 x19: ffffffc083bc3000 x18: ffffffffffffffff
>> x17: ffffffc08117b080 x16: 0000000000000002 x15: ffffff80d2d40000
>> x14: 00000000000002da x13: ffffff80d2d404b8 x12: ffffffc082b5a5c8
>> x11: ffffffc082bca680 x10: ffffffc082bb2640 x9 : ffffffc082bb2698
>> x8 : 0000000000017fe8 x7 : c0000000ffffefff x6 : 0000000000000001
>> x5 : ffffff8178fe0d48 x4 : 0000000000000000 x3 : 0000000000000027
>> x2 : ffffff8178fe0d50 x1 : 0000000000000000 x0 : 0000000000000000
>> Call trace:
>> __mutex_lock+0xd84/0x1068
>> mutex_lock_nested+0x28/0x34
>> tc_setup_taprio+0x118/0x68c
>> stmmac_setup_tc+0x50/0xf0
>> taprio_change+0x868/0xc9c
>>
>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>> ---
>> v1 -> v2:
>> - move the lock to struct plat_stmmacenet_data
>> v2 -> v3:
>> - Add require the mutex lock for reinitialization
>>
>> .../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 8 ++++----
>> .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 18 ++++++++++--------
>> include/linux/stmmac.h | 2 +-
>> 3 files changed, 15 insertions(+), 13 deletions(-)
>>
>> [...]
>>
>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>> index dfa1828cd756..316ff7eb8b33 100644
>> --- a/include/linux/stmmac.h
>> +++ b/include/linux/stmmac.h
>> @@ -117,7 +117,6 @@ struct stmmac_axi {
>>
>> #define EST_GCL 1024
>> struct stmmac_est {
>> - struct mutex lock;
>> int enable;
>> u32 btr_reserve[2];
>> u32 btr_offset[2];
>> @@ -246,6 +245,7 @@ struct plat_stmmacenet_data {
>> struct fwnode_handle *port_node;
>> struct device_node *mdio_node;
>> struct stmmac_dma_cfg *dma_cfg;
>> + struct mutex lock;
>> struct stmmac_est *est;
>> struct stmmac_fpe_cfg *fpe_cfg;
>> struct stmmac_safety_feature_cfg *safety_feat_cfg;
> Seeing you are going to move things around I suggest to move the
> entire stmmac_est instance out of the plat_stmmacenet_data structure
> and place it in the stmmac_priv instead. Why? Because the EST configs
> don't look as the platform config, but EST is enabled in runtime with
> the settings retrieved for the TC TAPRIO feature also in runtime. So
> it's better to have the EST-data preserved in the driver private date
> instead of the platform data storage. You could move the structure
> there and place the lock aside of it. Field name like "est_lock" might
> be most suitable to be looking unified with the "ptp_lock" or
> "aux_ts_lock".
>
> * The same, but with no lock-related thing should be done for the
> * stmmac_safety_feature_cfg structure,
> but it's unrelated to the subject...

I think this is good and I will send a v4 version out later, does anyone
else have any other opinions?

thanks

xiaolei

>
> -Serge(y)
>
>> --
>> 2.25.1
>>
>>

\
 
 \ /
  Last update: 2024-05-09 03:12    [W:0.042 / U:0.648 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site