On 8/8/23, Christian Brauner <brauner@xxxxxxxxxx> wrote: > It adds two new exports of filp_close_sync() and close_fd_sync() without > any users. That's not something we do and we also shouldn't encourage > random drivers to switch to sync behavior. > They don't need to be exported for the patch to work. > That rseq thing is completely orthogonal and maybe that needs to be > fixed and you can go and convince the glibc people to do it. > This is not a glibc problem, but rseq problem -- it should not be lumped into any case which uses task_work_add. > And making filp_close() fully sync again is also really not great. The patch is not doing it. > Simplicity wins out and if all codepaths share the same behavior we're > better off then having parts use task work and other parts just not. > The difference is not particularly complicated. > Yes, we just did re-added the f_pos optimization because it may have had > an impact. And that makes more sense because that was something we had > changed just a few days/weeks before. > I don't think perf tax on something becomes more sensible the longer it is there. > But this is over 10 year old behavior and this micro benchmarking isn't > a strong selling point imho. We could make close(2) go sync, sure. But > I'm skeptical even about this without real-world data or from a proper > testsuite. > I responded to this in my mail to Eric. > (There's also a stray sysctl_fput_sync there which is scary to think that > we'd ever allow a sysctl that allows userspace to control how we close > fds.) > This is a leftover from my tests -- I added a runtime switch so can I flip back and forth, most definitely not something I would expect to be shipped. -- Mateusz Guzik <mjguzik gmail.com>
