Re: Urgent Bug Report Kernel crash 6.5.2

Re: Urgent Bug Report Kernel crash 6.5.2

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 3969 bytes --]

On Thu, Sep 21, 2023 at 11:13:55AM +0300, Martin Zaharinov wrote:
> Hi Bagas,
> 
> 
> Its not easy to make this on production, have too many users on it.
> 
> i make checks and find with kernel 6.3.12-6.5.13 all is fine.
> on first machine that i have with kernel 6.4 and still work run kernel 6.4.2 and have problem.
> 
> in my investigation problem is start after migration to kernel 6.4.x 
> 
> in 6.4 kernel is add rcuref : 
> 
> https://cdn.kernel.org/pub/linux/kernel/v6.x/ChangeLog-6.4 
> 
> commit bc9d3a9f2afca189a6ae40225b6985e3c775375e
> Author: Thomas Gleixner <tglx@linutronix.de>
> Date: Thu Mar 23 21:55:32 2023 +0100
> 
> net: dst: Switch to rcuref_t reference counting

Is it the culprit you look for? Had you done the bisection and it points
the culprit to that commit

> 
> Under high contention dst_entry::__refcnt becomes a significant bottleneck.
> 
> atomic_inc_not_zero() is implemented with a cmpxchg() loop, which goes into
> high retry rates on contention.
> 
> Switch the reference count to rcuref_t which results in a significant
> performance gain. Rename the reference count member to __rcuref to reflect
> the change.
> 
> The gain depends on the micro-architecture and the number of concurrent
> operations and has been measured in the range of +25% to +130% with a
> localhost memtier/memcached benchmark which amplifies the problem
> massively.
> 
> Running the memtier/memcached benchmark over a real (1Gb) network
> connection the conversion on top of the false sharing fix for struct
> dst_entry::__refcnt results in a total gain in the 2%-5% range over the
> upstream baseline.
> 
> Reported-by: Wangyang Guo <wangyang.guo@intel.com>
> Reported-by: Arjan Van De Ven <arjan.van.de.ven@intel.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Link: https://lore.kernel.org/r/20230307125538.989175656@linutronix.de
> Link: https://lore.kernel.org/r/20230323102800.215027837@linutronix.de
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> 
> 
> and i think problem is here : 
> 
> --- a/net/core/dst.c
> +++ b/net/core/dst.c
> @@ -66,7 +66,7 @@ void dst_init(struct dst_entry *dst, str
> dst->tclassid = 0;
> #endif
> dst->lwtstate = NULL;
> - atomic_set(&dst->__refcnt, initial_ref);
> + rcuref_init(&dst->__refcnt, initial_ref);
> dst->__use = 0;
> dst->lastuse = jiffies;
> dst->flags = flags;
> @@ -162,31 +162,15 @@ EXPORT_SYMBOL(dst_dev_put);
> 
> void dst_release(struct dst_entry *dst)
> {
> - if (dst) {
> - int newrefcnt;
> -
> - newrefcnt = atomic_dec_return(&dst->__refcnt);
> - if (WARN_ONCE(newrefcnt < 0, "dst_release underflow"))
> - net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
> - __func__, dst, newrefcnt);
> - if (!newrefcnt)
> - call_rcu_hurry(&dst->rcu_head, dst_destroy_rcu);
> - }
> + if (dst && rcuref_put(&dst->__refcnt))
> + call_rcu_hurry(&dst->rcu_head, dst_destroy_rcu);
> }
> EXPORT_SYMBOL(dst_release);
> 
> void dst_release_immediate(struct dst_entry *dst)
> {
> - if (dst) {
> - int newrefcnt;
> -
> - newrefcnt = atomic_dec_return(&dst->__refcnt);
> - if (WARN_ONCE(newrefcnt < 0, "dst_release_immediate underflow"))
> - net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
> - __func__, dst, newrefcnt);
> - if (!newrefcnt)
> - dst_destroy(dst);
> - }
> + if (dst && rcuref_put(&dst->__refcnt))
> + dst_destroy(dst);
> }
> EXPORT_SYMBOL(dst_release_immediate);
> 
> 
> but this is my thinking
> 

What do you think that above causes your regression?

Confused...

[To Thorsten: I'm unsure if the reporter do the bisection and suddenly he found
the culprit commit. Should I add it to regzbot? I had dealt with this reporter
before when he reported nginx regression and he didn't respond with bisection
to the point that I had to mark it as inconclusive (see regzbot dashboard).
What advice can you provide to him?]

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: Urgent Bug Report Kernel crash 6.5.2

[Linux regression tracking (Thorsten Leemhuis)]

On 22.09.23 05:06, Bagas Sanjaya wrote:
> On Thu, Sep 21, 2023 at 11:13:55AM +0300, Martin Zaharinov wrote:
>>
>> Its not easy to make this on production, have too many users on it.
>>
>> i make checks and find with kernel 6.3.12-6.5.13 all is fine.
>> on first machine that i have with kernel 6.4 and still work run kernel 6.4.2 and have problem.

This is confusing and hard to follow. You want to describe more
carefully which kernels worked (avoid ranges, as I doubt you have tested
everything between 6.3.12-6.5.13) and try to avoid complexity (you seem
to have two machines? if everything works on one, don't even bring it up
except maybe as a side note)

>> in my investigation problem is start after migration to kernel 6.4.x 
>>
>> in 6.4 kernel is add rcuref : 
>>
>> https://cdn.kernel.org/pub/linux/kernel/v6.x/ChangeLog-6.4 
>>
>> commit bc9d3a9f2afca189a6ae40225b6985e3c775375e
>> Author: Thomas Gleixner <tglx@linutronix.de>
>> Date: Thu Mar 23 21:55:32 2023 +0100
>>
>> net: dst: Switch to rcuref_t reference counting
> 
> Is it the culprit you look for? Had you done the bisection and it points
> the culprit to that commit

Martin, if you suspect this to be the culprit try to revert it on top of
the latest kernel; if the problem then goes away it likely is the cause.

> [...]
>> but this is my thinking
> 
> What do you think that above causes your regression?
> 
> Confused...
> 
> [To Thorsten: I'm unsure if the reporter do the bisection and suddenly he found
> the culprit commit. Should I add it to regzbot?

For now: no, things are too confusing and without knowing the culprit I
guess nobody will look into this unless we are extremely lucky.

Ciao, Thorsten

Re: Urgent Bug Report Kernel crash 6.5.2

[Bagas Sanjaya]

On 22/09/2023 16:50, Linux regression tracking (Thorsten Leemhuis) wrote:
> On 22.09.23 05:06, Bagas Sanjaya wrote:
>> [To Thorsten: I'm unsure if the reporter do the bisection and suddenly he found
>> the culprit commit. Should I add it to regzbot?
> 
> For now: no, things are too confusing and without knowing the culprit I
> guess nobody will look into this unless we are extremely lucky.
> 

OK, thanks!

-- 
An old man doll... just what I always wanted! - Clara