Skip to content

Commit

Permalink
File: preadv2: Call the syscall directly rather than via glibc
Browse files Browse the repository at this point in the history
This way we don't need to worry about compatiblity with old glibc versions
and it will work on Android and musl too.  The downside is that you can't
use `LD_PRELOAD` tricks any more to intercept these calls.
  • Loading branch information
wmanley committed Mar 26, 2021
1 parent 0073710 commit 6ff4932
Showing 1 changed file with 27 additions and 1 deletion.
28 changes: 27 additions & 1 deletion tokio/src/fs/file.rs
Expand Up @@ -764,7 +764,7 @@ impl Inner {
#[cfg(all(target_os = "linux", not(test)))]
mod read_nowait {
use crate::io::ReadBuf;
use libc::{c_int, c_void, iovec, off_t, preadv2};
use libc::{c_int, c_long, c_void, iovec, off_t, ssize_t};
use std::{
os::unix::prelude::AsRawFd,
sync::atomic::{AtomicBool, Ordering},
Expand Down Expand Up @@ -822,6 +822,32 @@ mod read_nowait {
}
}
}

fn pos_to_lohi(offset: off_t) -> (c_long, c_long) {
// 64-bit offset is split over high and low 32-bits on 32-bit architectures.
// 64-bit architectures still have high and low arguments, but only the low
// one is inspected. See pos_from_hilo in linux/fs/read_write.c.
const HALF_LONG_BITS: usize = core::mem::size_of::<c_long>() * 8 / 2;
(
offset as c_long,
((offset >> HALF_LONG_BITS) >> HALF_LONG_BITS) as c_long,

This comment has been minimized.

Copy link
@Darksonn

Darksonn Mar 26, 2021

Contributor

Doing that shift twice doesn't seem quite right.

This comment has been minimized.

Copy link
@wmanley

wmanley Mar 26, 2021

Author Contributor

It's doing the opposite of this:

https://github.com/torvalds/linux/blob/db24726bfefa68c606947a86132591568a06bfb4/fs/read_write.c#L994-L998

My assumption is that it does this because shifting a 64-bit number by 64 bits is undefined behaviour in C (and implementation defined in rust(?)). So this funny bit of code handles the case where off_t is the same size as long as on x86-64 Linux, but also when it's half the size, like on x86. It does so without being explicitly conditional on the size of long or off_t.

This comment has been minimized.

Copy link
@Darksonn

Darksonn Mar 26, 2021

Contributor

Ah, I see now.

)
}

unsafe fn preadv2(
fd: c_int,
iov: *const iovec,
iovcnt: c_int,
offset: off_t,
flags: c_int,
) -> ssize_t {
if flags == 0 {
libc::preadv(fd, iov, iovcnt, offset)

This comment has been minimized.

Copy link
@Darksonn

Darksonn Mar 26, 2021

Contributor

Why do the flag check?

This comment has been minimized.

Copy link
@wmanley

wmanley Mar 26, 2021

Author Contributor

This is just a reproduction of the code that I was thinking of proposing to add to libc. When adding new versions of syscalls to libc it makes sense to delegate to the older version where possible so applications can just use the new function on old kernels and it will "just work" as long as they don't rely on the new features.

In our case we always pass nonzero to flags, and we explicitly manage compatibility ourselves, so I'll remove this if.

} else {
let (lo, hi) = pos_to_lohi(offset);
libc::syscall(libc::SYS_preadv2, fd, iov, iovcnt, lo, hi, flags) as ssize_t
}
}
}

#[cfg(any(not(target_os = "linux"), test))]
Expand Down

0 comments on commit 6ff4932

Please sign in to comment.