LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
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.

  reply	other threads:[~2019-05-24 21:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-23 18:21 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

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 \
    --subject='Re: [PATCH v2 0/2] close_range()' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).