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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 8 Aug 2023 10:13:04 +0200
From:   Christian Brauner <brauner@...nel.org>
To:     Mateusz Guzik <mjguzik@...il.com>
Cc:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, oleg@...hat.com,
        Matthew Wilcox <willy@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] fs: use __fput_sync in close(2)

> Are there any real world gains if close(2) is the only place this
> optimization can be applied?  Is the added maintenance burden worth the
> speed up?

Tbh, none of this patch makes me exited.

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.

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.

And making filp_close() fully sync again is also really not great.
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.

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.

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.

(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.)

> Unless you can find some real world performance gains this looks like
> a bad idea.

I agree.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ