diff options
author | Al Viro <viro@zeniv.linux.org.uk> | 2024-05-31 15:45:12 -0400 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2024-08-12 22:01:05 -0400 |
commit | 88a2f6468d013ca1163490dbddfc95135d1c27a1 (patch) | |
tree | 1ec4ea78a040c38d05892e80a3ce23cb040172c6 /net | |
parent | 1da91ea87aefe2c25b68c9f96947a9271ba6325d (diff) |
struct fd: representation change
We want the compiler to see that fdput() on empty instance
is a no-op. The emptiness check is that file reference is NULL,
while fdput() is "fput() if FDPUT_FPUT is present in flags".
The reason why fdput() on empty instance is a no-op is something
compiler can't see - it's that we never generate instances with
NULL file reference combined with non-zero flags.
It's not that hard to deal with - the real primitives behind
fdget() et.al. are returning an unsigned long value, unpacked by (inlined)
__to_fd() into the current struct file * + int. The lower bits are
used to store flags, while the rest encodes the pointer. Linus suggested
that keeping this unsigned long around with the extractions done by inlined
accessors should generate a sane code and that turns out to be the case.
Namely, turning struct fd into a struct-wrapped unsinged long, with
fd_empty(f) => unlikely(f.word == 0)
fd_file(f) => (struct file *)(f.word & ~3)
fdput(f) => if (f.word & 1) fput(fd_file(f))
ends up with compiler doing the right thing. The cost is the patch
footprint, of course - we need to switch f.file to fd_file(f) all over
the tree, and it's not doable with simple search and replace; there are
false positives, etc.
Note that the sole member of that structure is an opaque
unsigned long - all accesses should be done via wrappers and I don't
want to use a name that would invite manual casts to file pointers,
etc. The value of that member is equal either to (unsigned long)p | flags,
p being an address of some struct file instance, or to 0 for an empty fd.
For now the new predicate (fd_empty(f)) has no users; all the
existing checks have form (!fd_file(f)). We will convert to fd_empty()
use later; here we only define it (and tell the compiler that it's
unlikely to return true).
This commit only deals with representation change; there will
be followups.
Reviewed-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Diffstat (limited to 'net')
-rw-r--r-- | net/socket.c | 2 |
1 files changed, 1 insertions, 1 deletions
diff --git a/net/socket.c b/net/socket.c index f77a42a74510..c0d4f5032374 100644 --- a/net/socket.c +++ b/net/socket.c @@ -559,7 +559,7 @@ static struct socket *sockfd_lookup_light(int fd, int *err, int *fput_needed) if (fd_file(f)) { sock = sock_from_file(fd_file(f)); if (likely(sock)) { - *fput_needed = f.flags & FDPUT_FPUT; + *fput_needed = f.word & FDPUT_FPUT; return sock; } *err = -ENOTSOCK; |