linux-kernel - Re: [PATCH v3 0/4] ALSA: pcm: Convert SNDRV_PCM_IOCTL_SYNC_PTR to user_access_begin/user_access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87a56ar1or.wl-tiwai@suse.de>
Date: Sat, 14 Jun 2025 13:40:36 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: Jaroslav Kysela <perex@...ex.cz>,
	Takashi Iwai <tiwai@...e.com>,
	linux-kernel@...r.kernel.org,
	linuxppc-dev@...ts.ozlabs.org,
	linux-sound@...r.kernel.org,
	Herve Codina <herve.codina@...tlin.com>
Subject: Re: [PATCH v3 0/4] ALSA: pcm: Convert SNDRV_PCM_IOCTL_SYNC_PTR to user_access_begin/user_access_end()

On Sat, 14 Jun 2025 08:43:13 +0200,
Christophe Leroy wrote:
> 
> This series converts all variants of SNDRV_PCM_IOCTL_SYNC_PTR to 
> user_access_begin/user_access_end() in order to reduce the CPU load
> measured in function snd_pcm_ioctl.
> 
> With the current implementation, "perf top" reports a high load in
> snd_pcm_iotcl(). Most calls to that function are SNDRV_PCM_IOCTL_SYNC_PTR.
> 
>     14.20%  test_perf           [.] engine_main
> ==> 12.86%  [kernel]            [k] snd_pcm_ioctl
>     11.91%  [kernel]            [k] finish_task_switch.isra.0
>      4.15%  [kernel]            [k] snd_pcm_group_unlock_irq.part.0
>      4.07%  libc.so.6           [.] __ioctl_time64
>      3.58%  libasound.so.2.0.0  [.] __snd_pcm_mmap_begin_generic
>      3.37%  [kernel]            [k] sys_ioctl
>      2.96%  libasound.so.2.0.0  [.] snd_pcm_hw_avail_update
>      2.73%  libasound.so.2.0.0  [.] __snd_pcm_mmap_begin
>      2.58%  [kernel]            [k] system_call_exception
>      1.93%  libasound.so.2.0.0  [.] sync_ptr1
>      1.85%  libasound.so.2.0.0  [.] snd_pcm_unlock
>      1.84%  libasound.so.2.0.0  [.] snd_pcm_mmap_begin
>      1.83%  libasound.so.2.0.0  [.] bad_pcm_state
>      1.68%  libasound.so.2.0.0  [.] snd_pcm_mmap_avail
>      1.67%  libasound.so.2.0.0  [.] snd_pcm_avail_update
> 
> A tentative was done with going via intermediaire structs on stack to
> replace the multiple get_user() and put_user() with copy_from_user()
> and copy_to_user(). But copy_from_user() calls _copy_from_user() and
> copy_to_user() calls _copy_to_user(). Both then call __copy_tofrom_user().
> In total it is 16.4% so it is worse than before.
> 
>     14.47%  test_perf           [.] engine_main
>     12.00%  [kernel]            [k] finish_task_switch.isra.0
> ==>  8.37%  [kernel]            [k] snd_pcm_ioctl
>      5.44%  libc.so.6           [.] __ioctl_time64
>      5.03%  [kernel]            [k] snd_pcm_group_unlock_irq.part.0
> ==>  4.86%  [kernel]            [k] __copy_tofrom_user
>      4.62%  [kernel]            [k] sys_ioctl
>      3.22%  [kernel]            [k] system_call_exception
>      2.42%  libasound.so.2.0.0  [.] snd_pcm_mmap_begin
>      2.31%  [kernel]            [k] fdget
>      2.23%  libasound.so.2.0.0  [.] __snd_pcm_mmap_begin_generic
>      2.19%  [kernel]            [k] syscall_exit_prepare
>      1.92%  libasound.so.2.0.0  [.] snd_pcm_mmap_avail
>      1.86%  libasound.so.2.0.0  [.] __snd_pcm_mmap_begin
>      1.68%  libasound.so.2.0.0  [.] snd_pcm_hw_avail_update
> ==>  1.67%  [kernel]            [k] _copy_from_user
>      1.66%  libasound.so.2.0.0  [.] bad_pcm_state
> ==>  1.53%  [kernel]            [k] _copy_to_user
>      1.40%  libasound.so.2.0.0  [.] sync_ptr1
> 
> With this series which uses unsafe_put_user() and unsafe_get_user(),
> the load is significantly reduced:
> 
>     17.46%  test_perf           [.] engine_main
>      9.14%  [kernel]            [k] finish_task_switch.isra.0
> ==>  4.92%  [kernel]            [k] snd_pcm_ioctl
>      3.99%  [kernel]            [k] snd_pcm_group_unlock_irq.part.0
>      3.71%  libc.so.6           [.] __ioctl_time64
>      3.61%  libasound.so.2.0.0  [.] __snd_pcm_mmap_begin_generic
>      2.72%  libasound.so.2.0.0  [.] sync_ptr1
>      2.65%  [kernel]            [k] system_call_exception
>      2.46%  [kernel]            [k] sys_ioctl
>      2.43%  [kernel]            [k] __rseq_handle_notify_resume
>      2.34%  [kernel]            [k] do_epoll_wait
>      2.30%  libasound.so.2.0.0  [.] __snd_pcm_mmap_commit
>      2.14%  libasound.so.2.0.0  [.] __snd_pcm_avail
>      2.04%  libasound.so.2.0.0  [.] snd_pcm_hw_avail_update
>      1.89%  libasound.so.2.0.0  [.] snd_pcm_lock
>      1.84%  libasound.so.2.0.0  [.] snd_pcm_mmap_avail
>      1.76%  libasound.so.2.0.0  [.] __snd_pcm_avail_update
>      1.61%  libasound.so.2.0.0  [.] bad_pcm_state
>      1.60%  libasound.so.2.0.0  [.] __snd_pcm_mmap_begin
>      1.49%  libasound.so.2.0.0  [.] query_status_data
> 
> Since v2:
> - Fix macros to skip user_read_access_end() when user_read_access_begin() failed
> - Fix some tabulations for properly aligning backslashes
> 
> Since RFC:
> - Added a cover letter to summarize some of the measurements done on and around the RFC
> - Fixed relevant checkpatch feedback
> - Split last patch in two
> 
> Christophe Leroy (4):
>   ALSA: pcm: refactor copy from/to user in SNDRV_PCM_IOCTL_SYNC_PTR
>   ALSA: pcm: Convert SNDRV_PCM_IOCTL_SYNC_PTR to
>     user_access_begin/user_access_end()
>   ALSA: pcm: Replace [audio_]tstamp_[n]sec by struct __snd_timespec in
>     struct snd_pcm_mmap_status32
>   ALSA: pcm: Convert snd_pcm_sync_ptr() to
>     user_access_begin/user_access_end()

Applied now all patches.  Thanks!


Takashi