From: Alexey Dobriyan <adobriyan@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Christian Brauner <christian@brauner.io>,
Linux List Kernel Mailing <linux-kernel@vger.kernel.org>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v2 0/2] close_range()
Date: Sat, 25 May 2019 00:27:40 +0300 [thread overview]
Message-ID: <20190524212740.GA7165@avx2> (raw)
In-Reply-To: <CAHk-=wjaCygWXyGP-D2=ER0x8UbwdvyifH2Jfnf1KHUwR3sedw@mail.gmail.com>
On Fri, May 24, 2019 at 11:55:44AM -0700, Linus Torvalds wrote:
> On Fri, May 24, 2019 at 11:39 AM Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >
> > > Would there ever be any other reason to traverse unknown open files
> > > than to close them?
> >
> > This is what lsof(1) does:
>
> I repeat: Would there ever be any other reason to traverse unknown
> open files than to close them?
>
> lsof is not AT ALL a relevant argument.
>
> lsof fundamentally wants /proc, because lsof looks at *other*
> processes. That has absolutely zero to do with fdmap. lsof does *not*
> want fdmap at all. It wants "list other processes files". Which is
> very much what /proc is all about.
>
> So your argument that "fdmap is more generic" is bogus.
>
> fdmap is entirely pointless unless you can show a real and relevant
> (to performance) use of it.
>
> When you would *possibly* have a "let me get a list of all the file
> descriptors I have open, because I didn't track them myself"
> situation? That makes no sense. Particularly from a performance
> standpoint.
>
> In contrast, "close_range()" makes sense as an operation.
What about orthogonality of interfaces?
fdmap()
bulk_close()
Now fdmap() can be reused for lsof/criu and it is only 2 system calls
for close-everything usecase which is OK because readdir is 4(!) minimum:
open
getdents
getdents() = 0
close
Writing all of this I understood how fdmap can be made more faster which
neither getdents() nor even read() have the luxury of: it can return
a flag if more data is available so that application would do next fdmap()
only if truly necessary.
> I can
> explain exactly when it would be used, and I can easily see a
> situation where "I've opened a ton of files, now I want to release
> them" is a valid model of operation. And it's a valid optimization to
> do a bulk operation like that.
>
> IOW, close_range() makes sense as an operation even if you could just
> say "ok, I know exactly what files I have open". But it also makes
> sense as an operation for the case of "I don't even care what files I
> have open, I just want to close them".
>
> In contrast, the "I have opened a ton of files, and I don't even know
> what the hell I did, so can you list them for me" makes no sense.
>
> Because outside of "close them", there's no bulk operation that makes
> sense on random file handles that you don't know what they are. Unless
> you iterate over them and do the stat thing or whatever to figure it
> out - which is lsof, but as mentioned, it's about *other* peoples
> files.
What you're doing is making exactly one usecase take exactly one system
call and leaving everything else deal with /proc. Stracing lsof shows
very clearly how stupid and how wasteful it is. Especially now that
we're post-meltdown era caring about system call costs (yeah suure).
I'm suggesting make close-universe usecase take only 2 system calls.
which is still better than anything /proc can offer.
next prev parent reply other threads:[~2019-05-24 21:27 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-23 18:21 [PATCH v2 0/2] close_range() Alexey Dobriyan
2019-05-23 21:34 ` Linus Torvalds
2019-05-24 10:27 ` Christian Brauner
2019-05-24 18:39 ` Alexey Dobriyan
2019-05-24 18:55 ` Linus Torvalds
2019-05-24 21:27 ` Alexey Dobriyan [this message]
2019-05-24 23:45 ` Al Viro
-- strict thread matches above, loose matches on Subject: below --
2019-05-23 15:47 Christian Brauner
2019-05-23 15:47 ` Christian Brauner
2019-05-23 15:47 ` Christian Brauner
2019-05-23 15:47 ` Christian Brauner
2019-05-23 15:47 ` christian
2019-05-23 15:47 ` Christian Brauner
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=20190524212740.GA7165@avx2 \
--to=adobriyan@gmail.com \
--cc=christian@brauner.io \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.