linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Weimer <fw@deneb.enyo.de>
To: Christian Brauner <christian.brauner@ubuntu.com>
Cc: torvalds@linux-foundation.org, linux-kernel@vger.kernel.org,
	Kyle Evans <self@kyle-evans.net>,
	Victor Stinner <victor.stinner@gmail.com>,
	viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
	linux-api@vger.kernel.org, fweimer@redhat.com, jannh@google.com,
	oleg@redhat.com, arnd@arndb.de, shuah@kernel.org,
	dhowells@redhat.com, ldv@altlinux.org
Subject: Re: [PATCH v5 1/3] open: add close_range()
Date: Wed, 03 Jun 2020 01:30:57 +0200	[thread overview]
Message-ID: <87d06hdozy.fsf@mid.deneb.enyo.de> (raw)
In-Reply-To: <20200602204219.186620-2-christian.brauner@ubuntu.com> (Christian Brauner's message of "Tue, 2 Jun 2020 22:42:17 +0200")

* Christian Brauner:

> The performance is striking. For good measure, comparing the following
> simple close_all_fds() userspace implementation that is essentially just
> glibc's version in [6]:
>
> static int close_all_fds(void)
> {
>         int dir_fd;
>         DIR *dir;
>         struct dirent *direntp;
>
>         dir = opendir("/proc/self/fd");
>         if (!dir)
>                 return -1;
>         dir_fd = dirfd(dir);
>         while ((direntp = readdir(dir))) {
>                 int fd;
>                 if (strcmp(direntp->d_name, ".") == 0)
>                         continue;
>                 if (strcmp(direntp->d_name, "..") == 0)
>                         continue;
>                 fd = atoi(direntp->d_name);
>                 if (fd == dir_fd || fd == 0 || fd == 1 || fd == 2)
>                         continue;
>                 close(fd);
>         }
>         closedir(dir);
>         return 0;
> }
>

> [6]: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/unix/sysv/linux/grantpt.c;h=2030e07fa6e652aac32c775b8c6e005844c3c4eb;hb=HEAD#l17
>      Note that this is an internal implementation that is not exported.
>      Currently, libc seems to not provide an exported version of this
>      because of missing kernel support to do this.

Just to be clear, this code is not compiled into glibc anymore in
typical configurations.  I have posted a patch to turn grantpt into a
no-op: <https://sourceware.org/pipermail/libc-alpha/2020-May/114379.html>

I'm not entirely convinced that it's safe to keep iterating over
/proc/self/fd while also closing descriptors.  Ideally, I think an
application should call getdents64, process the file names for
descriptors in the buffer, and if any have been closed, seek to zero
before the next getdents64 call.  Maybe procfs is different, but with
other file systems, unlinking files can trigger directory reordering,
and then you get strange effects.  The d_ino behavior for
/proc/self/fd is a bit strange as well (it's not consistently
descriptor plus 3).

  reply	other threads:[~2020-06-02 23:31 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-02 20:42 [PATCH v5 0/3] close_range() Christian Brauner
2020-06-02 20:42 ` [PATCH v5 1/3] open: add close_range() Christian Brauner
2020-06-02 23:30   ` Florian Weimer [this message]
2020-06-02 23:37     ` Christian Brauner
2020-06-03 10:24   ` Michael Kerrisk (man-pages)
2020-09-17  7:52     ` Michael Kerrisk (man-pages)
2020-06-05 14:55   ` Szabolcs Nagy
2020-06-06  2:54     ` Kyle Evans
2020-06-06  3:11       ` Kyle Evans
2020-06-06 11:55       ` Szabolcs Nagy
2020-06-06 14:43         ` Kyle Evans
2020-06-07 13:22     ` David Laight
2020-06-02 20:42 ` [PATCH v5 2/3] arch: wire-up close_range() Christian Brauner
2020-06-02 20:42 ` [PATCH v5 3/3] tests: add close_range() tests Christian Brauner
2020-06-02 21:03 ` [PATCH v5 0/3] close_range() Linus Torvalds
2020-06-02 23:33   ` Christian Brauner
2020-06-03  0:08     ` Linus Torvalds
2020-06-03 23:24       ` Christian Brauner
2020-06-04  0:13         ` Linus Torvalds
2020-06-04  1:15           ` Christian Brauner
2020-06-07 12:31         ` David Laight

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=87d06hdozy.fsf@mid.deneb.enyo.de \
    --to=fw@deneb.enyo.de \
    --cc=arnd@arndb.de \
    --cc=christian.brauner@ubuntu.com \
    --cc=dhowells@redhat.com \
    --cc=fweimer@redhat.com \
    --cc=jannh@google.com \
    --cc=ldv@altlinux.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=self@kyle-evans.net \
    --cc=shuah@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=victor.stinner@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    /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 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).