From: David Howells <dhowells@redhat.com>
To: linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-fsdevel@vger.kernel.org, viro@ZenIV.linux.org.uk
Cc: hpa@zytor.com, torvalds@osdl.org, arnd@arndb.de, dhowells@redhat.com
Subject: [PATCH 0/3] Eliminating __FD_*() functions from the kernel
Date: Thu, 16 Feb 2012 17:49:30 +0000 [thread overview]
Message-ID: <20120216174930.23314.69764.stgit@warthog.procyon.org.uk> (raw)
In-Reply-To: 1328677745-20121-1-git-send-email-hpa@zytor.com
Having examined H. Peter Anvin's patches to merge asm/posix_types.h together, I
think that we can go further: The FD_SET(), FD_CLR() and FD_ISSET() macros are
only used within the kernel itself in relation to the arrays of fd-is-open
flags and close-on-exec flags. FD_ZERO() is not used at all within the kernel.
Now, the FD_SET() & co. wrappers must be exported to userspace, so they must be
kept, but the __FD_SET() & co. implementations behind them on most arches are
_not_ so exported. Possibly PowerPC, MN10300 and Xtensa would survive their
removal entirely from the userspace API as per Peter's patches.
Examining the two remaining usages of struct fd_set, and FD_*() and __FD_*()
for the arrays of bits in struct fdtable, it would seem that we could just move
to using unsigned long directly rather than fd_set, and using __set_bit(),
__clear_bit() and test_bit() upon those.
This would also get rid of two abuses of the fd_set struct within this code:
(1) Most of the time, the two fd_sets pointed to by struct fdtable are not the
length they're supposed to be, and are, in fact, allocated short. This
means FD_ZERO() may not be used on them, for example.
Basically, in effect, alloc_fdmem() allocates an array of unsigned longs
just long enough and then casts it to an fd_set - so why bother with the
fd_set at all?
(2) Non-core VFS code (such as SELinux) sometimes wants to look through the
array of unsigned longs inside the fd_set for the purposes of more
efficient iteration over the entire set of open fds - so why not just
eliminate the fd_set and go directly with an array of unsigned longs?
Furthermore, within the kernel, __FD_SET() and co. are redundant bitops and,
in the original implementation, may actually be less efficient. For instance,
on x86/x86_64, __set_bit() uses the BTS instruction, but __FD_SET() may well
not (depending on the optimiser). I note that Peter's patch does alter this.
So my patches do the following:
(1) Wrap the normal set/bit/test operations on the open_fds and close_on_exec
fd_sets so that the access to them is, for the most part, not open coded.
There's still the issue of the things that poke around inside the fd_sets
behind the scenes (SELinux for example).
(2) Replace the fd_set with an unsigned long array and use __set_bit() and
co. instead of FD_SET() and co.
It might just be better to replace the uses of FD_SET() and co. with
__set_bit() and co. directly, and not bother with wrappers, but the
wrappers are more greppable.
(3) Delete __FD_SET() and co. from linux/time.h.
And are implemented to go on top of Peter's posix_types patchset.
David
---
David Howells (3):
Delete the __FD_*() funcs for operating on fd_set from linux/time.h
Replace the fd_sets in struct fdtable with an array of unsigned longs
Wrap accesses to the fd_sets in struct fdtable
Documentation/filesystems/files.txt | 4 +-
arch/powerpc/platforms/cell/spufs/coredump.c | 2 -
drivers/staging/android/binder.c | 10 ++---
fs/autofs4/dev-ioctl.c | 2 -
fs/exec.c | 8 ++--
fs/fcntl.c | 18 ++++-----
fs/file.c | 54 +++++++++++++-------------
fs/open.c | 4 +-
fs/proc/base.c | 2 -
fs/select.c | 2 -
include/linux/fdtable.h | 46 ++++++++++++++++------
include/linux/time.h | 22 -----------
kernel/exit.c | 2 -
security/selinux/hooks.c | 2 -
14 files changed, 88 insertions(+), 90 deletions(-)
next prev parent reply other threads:[~2012-02-16 17:49 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-08 5:08 [PATCH 00/21] RFC: Make all arches use <asm-generic/posix_types.h> H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 01/21] posix_types: Make __kernel_[ug]id32_t default to unsigned int H. Peter Anvin
2012-02-08 5:08 ` [PATCH 02/21] posix_types: Make it possible to override __kernel_fsid_t H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 03/21] alpha: Use generic posix_types.h H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 04/21] arm: " H. Peter Anvin
2012-02-09 0:57 ` Russell King - ARM Linux
2012-02-08 5:08 ` [PATCH 05/21] avr32: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 06/21] cris: " H. Peter Anvin
2012-02-08 13:21 ` Jesper Nilsson
2012-02-08 5:08 ` [PATCH 07/21] frv: " H. Peter Anvin
2012-02-08 5:08 ` [PATCH 08/21] h8300: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 09/21] ia64: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 10/21] m32r: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 11/21] m68k: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-19 10:28 ` Geert Uytterhoeven
2012-02-08 5:08 ` [PATCH 12/21] mips: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 13/21] mn10300: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 14/21] parisc: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-08 5:08 ` [PATCH 15/21] powerpc: " H. Peter Anvin
2012-02-08 5:08 ` H. Peter Anvin
2012-02-09 6:14 ` Benjamin Herrenschmidt
2012-02-08 5:09 ` [PATCH 16/21] s390: " H. Peter Anvin
2012-02-08 5:09 ` H. Peter Anvin
2012-02-08 9:04 ` Martin Schwidefsky
2012-02-08 16:55 ` H. Peter Anvin
2012-02-08 18:01 ` Martin Schwidefsky
2012-02-08 5:09 ` [PATCH 17/21] sh: Remove unnecessary posix_types.h type overrides H. Peter Anvin
2012-02-08 5:09 ` [PATCH 18/21] sparc: Use generic posix_types.h H. Peter Anvin
2012-02-08 5:09 ` H. Peter Anvin
2012-02-09 1:27 ` David Miller
2012-02-09 1:27 ` David Miller
2012-02-08 5:09 ` [PATCH 19/21] x86: " H. Peter Anvin
2012-02-08 5:09 ` H. Peter Anvin
2012-02-08 5:09 ` [PATCH 20/21] xtensa: " H. Peter Anvin
2012-02-08 5:09 ` H. Peter Anvin
2012-02-08 5:09 ` [PATCH 21/21] posix_types: Remove fd_set macros H. Peter Anvin
2012-02-08 5:09 ` H. Peter Anvin
2012-02-08 12:20 ` David Howells
2012-02-08 16:57 ` H. Peter Anvin
2012-02-08 21:24 ` David Howells
2012-02-08 21:24 ` David Howells
2012-02-08 21:30 ` H. Peter Anvin
2012-02-08 21:30 ` H. Peter Anvin
2012-02-14 18:59 ` Tony Luck
2012-02-14 19:18 ` David Howells
2012-02-14 19:44 ` H. Peter Anvin
2012-02-14 20:14 ` H. Peter Anvin
2012-02-16 13:42 ` [PATCH 20/21] xtensa: Use generic posix_types.h David Howells
2012-02-16 17:45 ` Marc Gauthier
2012-02-16 13:44 ` [PATCH 13/21] mn10300: " David Howells
2012-02-16 13:44 ` [PATCH 15/21] powerpc: " David Howells
2012-02-16 20:26 ` Benjamin Herrenschmidt
2012-02-16 20:58 ` H. Peter Anvin
2012-02-16 17:49 ` David Howells [this message]
2012-02-16 17:49 ` [PATCH 0/3] Eliminating __FD_*() functions from the kernel David Howells
2012-02-16 17:49 ` [PATCH 1/3] Wrap accesses to the fd_sets in struct fdtable David Howells
2012-02-16 17:49 ` [PATCH 2/3] Replace the fd_sets in struct fdtable with an array of unsigned longs David Howells
2012-02-16 17:50 ` [PATCH 3/3] Delete the __FD_*() funcs for operating on fd_set from linux/time.h David Howells
2012-02-16 17:50 ` David Howells
2012-02-20 21:12 ` [PATCH 13/21] mn10300: Use generic posix_types.h David Howells
2012-02-20 21:12 ` David Howells
2012-02-20 21:12 ` [PATCH 07/21] frv: " David Howells
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=20120216174930.23314.69764.stgit@warthog.procyon.org.uk \
--to=dhowells@redhat.com \
--cc=arnd@arndb.de \
--cc=hpa@zytor.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@osdl.org \
--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).