Fix bogus lockdep warnings if multiple u64_stats_sync variables are
initialized in the same file.
With CONFIG_LOCKDEP, seqcount_init() is a macro which declares:
static struct lock_class_key __key;
Since u64_stats_init() is a function (albeit an inline one), all calls
within the same file end up using the same instance, effectively treating
them all as a single lock-class.
Cc: [email protected]
Fixes: 9464ca650008 ("net: make u64_stats_init() a function")
Closes: https://lore.kernel.org/netdev/[email protected]/
Signed-off-by: Petr Tesarik <[email protected]>
---
include/linux/u64_stats_sync.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/linux/u64_stats_sync.h b/include/linux/u64_stats_sync.h
index ffe48e69b3f3..457879938fc1 100644
--- a/include/linux/u64_stats_sync.h
+++ b/include/linux/u64_stats_sync.h
@@ -135,10 +135,11 @@ static inline void u64_stats_inc(u64_stats_t *p)
p->v++;
}
-static inline void u64_stats_init(struct u64_stats_sync *syncp)
-{
- seqcount_init(&syncp->seq);
-}
+#define u64_stats_init(syncp) \
+ do { \
+ struct u64_stats_sync *__s = (syncp); \
+ seqcount_init(&__s->seq); \
+ } while (0)
static inline void __u64_stats_update_begin(struct u64_stats_sync *syncp)
{
--
2.44.0
On Wed, 6 Mar 2024 12:11:57 +0100
Petr Tesarik <[email protected]> wrote:
> Fix bogus lockdep warnings if multiple u64_stats_sync variables are
> initialized in the same file.
>
> With CONFIG_LOCKDEP, seqcount_init() is a macro which declares:
>
> static struct lock_class_key __key;
>
> Since u64_stats_init() is a function (albeit an inline one), all calls
> within the same file end up using the same instance, effectively treating
> them all as a single lock-class.
What happens with this fix now?
IIUC it should be reviewed by Eric, but I don't know through which tree
it should be merged. Any plans yet?
Petr T
> Cc: [email protected]
> Fixes: 9464ca650008 ("net: make u64_stats_init() a function")
> Closes: https://lore.kernel.org/netdev/[email protected]/
> Signed-off-by: Petr Tesarik <[email protected]>
> ---
> include/linux/u64_stats_sync.h | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/u64_stats_sync.h b/include/linux/u64_stats_sync.h
> index ffe48e69b3f3..457879938fc1 100644
> --- a/include/linux/u64_stats_sync.h
> +++ b/include/linux/u64_stats_sync.h
> @@ -135,10 +135,11 @@ static inline void u64_stats_inc(u64_stats_t *p)
> p->v++;
> }
>
> -static inline void u64_stats_init(struct u64_stats_sync *syncp)
> -{
> - seqcount_init(&syncp->seq);
> -}
> +#define u64_stats_init(syncp) \
> + do { \
> + struct u64_stats_sync *__s = (syncp); \
> + seqcount_init(&__s->seq); \
> + } while (0)
>
> static inline void __u64_stats_update_begin(struct u64_stats_sync *syncp)
> {
On Mon, Mar 11, 2024 at 6:25 PM Petr Tesařík <[email protected]> wrote:
>
> On Wed, 6 Mar 2024 12:11:57 +0100
> Petr Tesarik <[email protected]> wrote:
>
> > Fix bogus lockdep warnings if multiple u64_stats_sync variables are
> > initialized in the same file.
> >
> > With CONFIG_LOCKDEP, seqcount_init() is a macro which declares:
> >
> > static struct lock_class_key __key;
> >
> > Since u64_stats_init() is a function (albeit an inline one), all calls
> > within the same file end up using the same instance, effectively treating
> > them all as a single lock-class.
>
> What happens with this fix now?
>
> IIUC it should be reviewed by Eric, but I don't know through which tree
> it should be merged. Any plans yet?
I thought I gave a reply, but apparently not .
Reviewed-by: Eric Dumazet <[email protected]>
Thanks.
On Mon, 11 Mar 2024 18:43:59 +0100
Eric Dumazet <[email protected]> wrote:
> On Mon, Mar 11, 2024 at 6:25 PM Petr Tesařík <[email protected]> wrote:
> >
> > On Wed, 6 Mar 2024 12:11:57 +0100
> > Petr Tesarik <[email protected]> wrote:
> >
> > > Fix bogus lockdep warnings if multiple u64_stats_sync variables are
> > > initialized in the same file.
> > >
> > > With CONFIG_LOCKDEP, seqcount_init() is a macro which declares:
> > >
> > > static struct lock_class_key __key;
> > >
> > > Since u64_stats_init() is a function (albeit an inline one), all calls
> > > within the same file end up using the same instance, effectively treating
> > > them all as a single lock-class.
> >
> > What happens with this fix now?
> >
> > IIUC it should be reviewed by Eric, but I don't know through which tree
> > it should be merged. Any plans yet?
>
> I thought I gave a reply, but apparently not .
>
> Reviewed-by: Eric Dumazet <[email protected]>
Thank you!
Petr T
On 11.03.24 19:21, Petr Tesařík wrote:
> On Mon, 11 Mar 2024 18:43:59 +0100
> Eric Dumazet <[email protected]> wrote:
>> On Mon, Mar 11, 2024 at 6:25 PM Petr Tesařík <[email protected]> wrote:
>>> On Wed, 6 Mar 2024 12:11:57 +0100
>>> Petr Tesarik <[email protected]> wrote:
>>>
>>>> Fix bogus lockdep warnings if multiple u64_stats_sync variables are
>>>> initialized in the same file.
>>>>
>>>> With CONFIG_LOCKDEP, seqcount_init() is a macro which declares:
>>>>
>>>> static struct lock_class_key __key;
>>>>
>>>> Since u64_stats_init() is a function (albeit an inline one), all calls
>>>> within the same file end up using the same instance, effectively treating
>>>> them all as a single lock-class.
>>> What happens with this fix now?
>>>
>>> IIUC it should be reviewed by Eric, but I don't know through which tree
>>> it should be merged. Any plans yet?
>>
>> I thought I gave a reply, but apparently not .
>>
>> Reviewed-by: Eric Dumazet <[email protected]>
>
> Thank you!
Great. Just wondering, as there afaics was no activity since about one
week: what is the plan forward here?
Is the "through which tree it should be merged" question still
unresolved? I quickly looked and it seems two of the last tree changes
to that file over the past years went through net-next (the other one
through the tip tree). That's why I CCed the other two net maintainers
and the net list now.
Or is the plan to merge this after the merge window? Or only merge it
for 6.10, as it are bogus lockdep warnings that are being fixed?
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
On Tue, Apr 2, 2024 at 3:40 PM Linux regression tracking (Thorsten
Leemhuis) <[email protected]> wrote:
>
> Hi. Top-posting for once, to make this easily accessible to everyone.
>
> Hmmm, looks like Petr's patch for a (minor) 6.8 regression didn't make
> any progress in the past two weeks.
>
> Does nobody care? Did nobody merge it because no tree feels really
> appropriate? Or am I missing something obvious and making a fool out of
> myself by asking these questions? :D
It happens, please resend the patch.
On Tue, Apr 2, 2024 at 3:42 PM Eric Dumazet <[email protected]> wrote:
>
> On Tue, Apr 2, 2024 at 3:40 PM Linux regression tracking (Thorsten
> Leemhuis) <[email protected]> wrote:
> >
> > Hi. Top-posting for once, to make this easily accessible to everyone.
> >
> > Hmmm, looks like Petr's patch for a (minor) 6.8 regression didn't make
> > any progress in the past two weeks.
> >
> > Does nobody care? Did nobody merge it because no tree feels really
> > appropriate? Or am I missing something obvious and making a fool out of
> > myself by asking these questions? :D
>
> It happens, please resend the patch.
By 'resend' the patch, I meant : sending it to netdev@ mailing list so
that netdev patchwork can see it.
Hi. Top-posting for once, to make this easily accessible to everyone.
Hmmm, looks like Petr's patch for a (minor) 6.8 regression didn't make
any progress in the past two weeks.
Does nobody care? Did nobody merge it because no tree feels really
appropriate? Or am I missing something obvious and making a fool out of
myself by asking these questions? :D
Ciao, Thorsten
#regzbot ignore-activity
On 18.03.24 15:23, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 11.03.24 19:21, Petr Tesařík wrote:
>> On Mon, 11 Mar 2024 18:43:59 +0100
>> Eric Dumazet <[email protected]> wrote:
>>> On Mon, Mar 11, 2024 at 6:25 PM Petr Tesařík <[email protected]> wrote:
>>>> On Wed, 6 Mar 2024 12:11:57 +0100
>>>> Petr Tesarik <[email protected]> wrote:
>>>>
>>>>> Fix bogus lockdep warnings if multiple u64_stats_sync variables are
>>>>> initialized in the same file.
>>>>>
>>>>> With CONFIG_LOCKDEP, seqcount_init() is a macro which declares:
>>>>>
>>>>> static struct lock_class_key __key;
>>>>>
>>>>> Since u64_stats_init() is a function (albeit an inline one), all calls
>>>>> within the same file end up using the same instance, effectively treating
>>>>> them all as a single lock-class.
>>>> What happens with this fix now?
>>>>
>>>> IIUC it should be reviewed by Eric, but I don't know through which tree
>>>> it should be merged. Any plans yet?
>>>
>>> I thought I gave a reply, but apparently not .
>>>
>>> Reviewed-by: Eric Dumazet <[email protected]>
>>
>> Thank you!
>
> Great. Just wondering, as there afaics was no activity since about one
> week: what is the plan forward here?
>
> Is the "through which tree it should be merged" question still
> unresolved? I quickly looked and it seems two of the last tree changes
> to that file over the past years went through net-next (the other one
> through the tip tree). That's why I CCed the other two net maintainers
> and the net list now.
>
> Or is the plan to merge this after the merge window? Or only merge it
> for 6.10, as it are bogus lockdep warnings that are being fixed?
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
>