On 8/8/23, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Tue, 8 Aug 2023 at 10:10, Mateusz Guzik <mjguzik@xxxxxxxxx> wrote: >> >> Few hours ago I sent another version which very closely resembles what >> you did :) >> 2 main differences: >> - i somehow missed close_fd_get_file so I hacked my own based on close_fd >> - you need to whack the kthread assert in __fput_sync > > Good call on teh __fput_sync() test. > > That BUG_ON() made sense ten years ago when this was all re-organized, > not so much now. > Christian proposes a dedicated routine, I have 0 opinion, you guys sort it out ;) >> I'm offended you ask, it's all in my opening e-mail. > > Heh. I wasn't actually cc'd on that, so I'm going by context and then > peeking at web links.. > Here it is again with some typos fixed and slightly reworded, not necessarily with quality of English improved. Feel free to quote in whatever extent in your commit message (including none). [quote] fs: use __fput_sync in close(2) close(2) is a special case which guarantees a shallow kernel stack, making delegation to task_work machinery unnecessary. Said delegation is problematic as it involves atomic ops and interrupt masking trips, none of which are cheap on x86-64. Forcing close(2) to do it looks like an oversight in the original work. Moreover presence of CONFIG_RSEQ adds an additional overhead as fput() -> task_work_add(..., TWA_RESUME) -> set_notify_resume() makes the thread returning to userspace land in resume_user_mode_work(), where rseq_handle_notify_resume takes a SMAP round-trip if rseq is enabled for the thread (and it is by default with contemporary glibc). Sample result when benchmarking open1_processes -t 1 from will-it-scale (that's an open + close loop) + tmpfs on /tmp, running on the Sapphire Rapid CPU (ops/s): stock+RSEQ: 1329857 stock-RSEQ: 1421667 (+7%) patched: 1523521 (+14.5% / +7%) (with / without rseq) Patched result is the same regardless of rseq as the codepath is avoided. [/quote] -- Mateusz Guzik <mjguzik gmail.com>
