[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <16579efc-97b0-4a0a-b70c-7362904ddfee@intel.com>
Date: Wed, 3 Dec 2025 14:23:06 -0800
From: Jacob Keller <jacob.e.keller@...el.com>
To: Simon Horman <horms@...nel.org>
CC: Aleksandr Loktionov <aleksandr.loktionov@...el.com>, Alexander Lobakin
<aleksander.lobakin@...el.com>, Tony Nguyen <anthony.l.nguyen@...el.com>,
Przemek Kitszel <przemyslaw.kitszel@...el.com>,
<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH iwl-next v4 1/6] ice: initialize ring_stats->syncp
On 11/25/2025 2:15 AM, Simon Horman wrote:
> On Thu, Nov 20, 2025 at 12:20:41PM -0800, Jacob Keller wrote:
>> The u64_stats_sync structure is empty on 64-bit systems. However, on 32-bit
>> systems it contains a seqcount_t which needs to be initialized. While the
>> memory is zero-initialized, a lack of u64_stats_init means that lockdep
>> won't get initialized properly. Fix this by adding u64_stats_init() calls
>> to the rings just after allocation.
>>
>> Fixes: 2b245cb29421 ("ice: Implement transmit and NAPI support")
>
> I think that either this patch should be routed via net. Or the Fixes tag
> should be removed, and optionally something about commit 2b245cb29421
> ("ice: Implement transmit and NAPI support") included in the commit message
> above the tags.
>
>> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
>> ---
>> drivers/net/ethernet/intel/ice/ice_lib.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
>> index 44f3c2bab308..116a4f4ef91d 100644
>> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
>> @@ -400,7 +400,10 @@ static int ice_vsi_alloc_ring_stats(struct ice_vsi *vsi)
>> if (!ring_stats)
>> goto err_out;
>>
>> + u64_stats_init(&ring_stats->syncp);
>> +
>> WRITE_ONCE(tx_ring_stats[i], ring_stats);
>> +
>
> nit: perhaps adding this blank line is unintentional.
>
Indeed it was. I believe its a result of the rebasing I did to split the
changes into separate patches, and this mistake slipped through.
>> }
>>
>> ring->ring_stats = ring_stats;
>> @@ -419,6 +422,8 @@ static int ice_vsi_alloc_ring_stats(struct ice_vsi *vsi)
>> if (!ring_stats)
>> goto err_out;
>>
>> + u64_stats_init(&ring_stats->syncp);
>> +
>> WRITE_ONCE(rx_ring_stats[i], ring_stats);
>> }
>
> The above comments not withstanding, this looks good to me.
>
> Reviewed-by: Simon Horman <horms@...nel.org>
>
Tony is going to help work with me to separate this lone patch from the
series and prepare it for submission through net separate from the rest
of the series. Unless there's other review that requires it, I likely
won't post a v5 to Intel Wired LAN, but wanted to confirm that we'll
submit this fix through net.
Thanks,
Jake
Download attachment "OpenPGP_signature.asc" of type "application/pgp-signature" (237 bytes)
Powered by blists - more mailing lists