Linux Perf Users
 help / color / mirror / Atom feed
From: "Lai, Yi" <yi1.lai@linux.intel.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Frederic Weisbecker <frederic@kernel.org>,
	Oleg Nesterov <oleg@redhat.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Adrian Hunter <adrian.hunter@intel.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Daniel Bristot de Oliveira <bristot@kernel.org>,
	Ian Rogers <irogers@google.com>, Ingo Molnar <mingo@redhat.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Kan Liang <kan.liang@linux.intel.com>,
	Marco Elver <elver@google.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	yi1.lai@intel.com, syzkaller-bugs@googlegroups.com
Subject: Re: [PATCH v4 2/6] perf: Enqueue SIGTRAP always via task_work.
Date: Wed, 4 Dec 2024 11:02:28 +0800	[thread overview]
Message-ID: <Z0/GRGpARFh89rVh@ly-workstation> (raw)
In-Reply-To: <20241111120857.5cWFpNkJ@linutronix.de>

On Mon, Nov 11, 2024 at 01:08:57PM +0100, Sebastian Andrzej Siewior wrote:
> On 2024-11-08 23:26:36 [+0100], Frederic Weisbecker wrote:
> > > Please see
> > > https://lore.kernel.org/all/1440816150.8932.123.camel@edumazet-glaptop2.roam.corp.google.com/
> > > and the whole thread.
> 
> Thank you for this Oleg.
> 
> > > I don't think raw_spin_lock_irq + cmpxchg for each work is a good
> > > idea, but quite possibly I misunderstood this change.
> > 
> > I did not realize there could be gazillion files released in a row. So there
> > could be noticeable performance issues I guess...
> 
> I made a testcase to open 2M (2 *10^6) files and then exit. This led
> task_work_run() run 2M + 3 callbacks (+ stdin/out/err) for the task.
> 
> Running 70 samples on the "orig" kernel:
> - avg callback time 1.156.470,3 us
> - 63 samples are starting with 11 (1,1 sec) avg: 1.128.046,7 us
> - 6 samples are starting with 14 (1,4 sec) avg: 1.435.294,8us
> 
> Running 70 samples on the "patched" kernel:
> - avg callback time 1.278.938,8 us
> - 59 samples are starting with 12 (1,2 sec) avg: 1.230.189,1 us
> - 10 samples are starting with 15 (1,5sec) avg: 1.555.934,5 us
> 
> With the extra lock the task_work_run() runtime extends by approximately
> ~122ms for the 2M invoked callbacks. 
> The spike 1,1sec -> 1,4sec or 1,2sec -> 1,5 sec is due to context
> switching (there are few cond_resched()/ might_sleep()).
> 
> It is not that bad, is it?
>

Hi,

Do we reach consensus about the fix? The issue can still be reproduced
using v6.13-rc1 kernel. Call trace:

[  300.429498]  ? lock_acquire+0x80/0xb0
[  300.429695]  ? schedule+0x216/0x3f0
[  300.429888]  schedule+0xf6/0x3f0
[  300.430068]  _free_event+0x531/0x14c0
[  300.430277]  perf_event_release_kernel+0x648/0x870
[  300.430680]  ? __pfx_perf_event_release_kernel+0x10/0x10
[  300.430969]  ? __this_cpu_preempt_check+0x21/0x30
[  300.431228]  ? __sanitizer_cov_trace_const_cmp2+0x1c/0x30
[  300.431519]  ? __pfx_perf_release+0x10/0x10
[  300.431746]  perf_release+0x3a/0x50
[  300.431940]  __fput+0x414/0xb60
[  300.432127]  ____fput+0x22/0x30
[  300.432303]  task_work_run+0x19c/0x2b0
[  300.432518]  ? __pfx_task_work_run+0x10/0x10
[  300.432752]  ? __sanitizer_cov_trace_const_cmp4+0x1a/0x20
[  300.433038]  ? switch_task_namespaces+0xc6/0x110
[  300.433295]  do_exit+0xb19/0x2a30
[  300.433484]  ? lockdep_hardirqs_on+0x89/0x110
[  300.433728]  ? __pfx_do_exit+0x10/0x10
[  300.433942]  do_group_exit+0xe4/0x2c0
[  300.434147]  get_signal+0x2279/0x24c0
[  300.434360]  ? __pfx_get_signal+0x10/0x10
[  300.434601]  ? __might_fault+0xf1/0x1b0
[  300.434824]  arch_do_signal_or_restart+0x8e/0x7d0
[  300.435083]  ? __pfx_arch_do_signal_or_restart+0x10/0x10
[  300.435375]  ? __this_cpu_preempt_check+0x21/0x30
[  300.435629]  ? syscall_exit_to_user_mode+0x10f/0x200
[  300.435901]  syscall_exit_to_user_mode+0x144/0x200
[  300.436161]  do_syscall_64+0x79/0x140
[  300.436366]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
[  300.436643] RIP: 0033:0x7f5add83ee5d
[  300.436843] RSP: 002b:00007fff21a70e68 EFLAGS: 00000206 ORIG_RAX: 000000000000012a
[  300.437240] RAX: 0000000000000003 RBX: 0000000000000000 RCX: 00007f5add83ee5d
[  300.437610] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000020000080
[  300.437969] RBP: 00007fff21a70e70 R08: 000000000000000b R09: 00007fff21a70ea0
[  300.438329] R10: 00000000ffffffff R11: 0000000000000206 R12: 00007fff21a70fc8
[  300.438703] R13: 0000000000401b4f R14: 0000000000403e08 R15: 00007f5addc0e000
[  300.439075]  </TASK>
[  300.439197]
[  300.439197] Showing all locks held in the system:
[  300.439513] 1 lock held by khungtaskd/33:
[  300.439723]  #0: ffffffff8705ca40 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks+0x73/0x3c0
[  300.440199]
[  300.440288] =============================================
[  300.440288]

Regards,
Yi Lai

> > Thanks.
> > > 
> > > Oleg.
> 
> Sebastian

  reply	other threads:[~2024-12-04  3:03 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-24 15:15 [PATCH v4 0/6] perf: Make SIGTRAP and __perf_pending_irq() work on RT Sebastian Andrzej Siewior
2024-06-24 15:15 ` [PATCH v4 1/6] perf: Move irq_work_queue() where the event is prepared Sebastian Andrzej Siewior
2024-06-24 15:15 ` [PATCH v4 2/6] perf: Enqueue SIGTRAP always via task_work Sebastian Andrzej Siewior
2024-07-01 12:28   ` Peter Zijlstra
2024-07-01 13:27     ` Sebastian Andrzej Siewior
2024-07-02  9:01       ` Peter Zijlstra
2024-10-28  8:30   ` Lai, Yi
2024-10-28 12:21     ` Frederic Weisbecker
2024-10-29 17:21       ` Sebastian Andrzej Siewior
2024-10-30 14:07         ` Sebastian Andrzej Siewior
2024-10-30 15:46           ` Frederic Weisbecker
2024-11-07 14:46             ` Sebastian Andrzej Siewior
2024-11-08 13:11               ` Frederic Weisbecker
2024-11-08 19:08                 ` Oleg Nesterov
2024-11-08 22:26                   ` Frederic Weisbecker
2024-11-11 12:08                     ` Sebastian Andrzej Siewior
2024-12-04  3:02                       ` Lai, Yi [this message]
2024-12-04 13:48                       ` Oleg Nesterov
2024-12-05  0:19                         ` Frederic Weisbecker
2024-12-05  9:20                           ` Oleg Nesterov
2024-12-05 10:05                             ` Frederic Weisbecker
2024-12-05 10:28                               ` Oleg Nesterov
2024-12-13 22:52                                 ` Frederic Weisbecker
2024-12-16 19:19                                   ` Oleg Nesterov
2024-06-24 15:15 ` [PATCH v4 3/6] perf: Shrink the size of the recursion counter Sebastian Andrzej Siewior
2024-07-01 12:31   ` Peter Zijlstra
2024-07-01 12:56     ` Sebastian Andrzej Siewior
2024-07-01 13:10       ` Peter Zijlstra
2024-06-24 15:15 ` [PATCH v4 4/6] perf: Move swevent_htable::recursion into task_struct Sebastian Andrzej Siewior
2024-06-24 15:15 ` [PATCH v4 5/6] perf: Don't disable preemption in perf_pending_task() Sebastian Andrzej Siewior
2024-06-24 15:15 ` [PATCH v4 6/6] perf: Split __perf_pending_irq() out of perf_pending_irq() Sebastian Andrzej Siewior
2024-06-25 13:42 ` [PATCH v4 0/6] perf: Make SIGTRAP and __perf_pending_irq() work on RT Marco Elver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z0/GRGpARFh89rVh@ly-workstation \
    --to=yi1.lai@linux.intel.com \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bigeasy@linutronix.de \
    --cc=bristot@kernel.org \
    --cc=elver@google.com \
    --cc=frederic@kernel.org \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tglx@linutronix.de \
    --cc=yi1.lai@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox