Skip to content

rust: iopoll: add read_poll_timeout function #701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: rust
Choose a base branch
from

Conversation

m-falkowski1
Copy link

This patch adds read_poll_timeout() macro rewritten in Rust.
It is intended for indirect use by wrapping any specialization of
this macro in a concrete function definition such as readx_poll_timeout()
for a given read() function. This provides additional type-safety for the interface
as well as encapsulates the read_poll_timeout!() macro.

This patch is a dependency of a Samsung Exynos trng driver provided initially in #554.

Signed-off-by: Maciej Falkowski m.falkowski@samsung.com

Maciej Falkowski added 3 commits March 9, 2022 17:22
This patch adds minimal bindings of the delay module that
is `usleep_range()` function.

Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
This patch adds bindings for the ktime module including `Ktime` type,
`ktime_get(), `ktime_add_us()`, and `ktime_compare()`.

Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
This patch adds `read_poll_timeout()` macro rewritten in Rust.
It is intended for indirect use by wrapping any specialization of
this macro in a concrete function definition such as `readx_poll_timeout()`
for a given `read()` function. This provides additional type-safety for the interface
as well as encapsulates the `read_poll_timeout!()` macro.

Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
@m-falkowski1
Copy link
Author

The current version of readx_poll_timeout is pretty verbose with unsafe at the moment:

pub unsafe fn readx_poll_timeout<T, F: Fn(&T) -> bool>(
    op: unsafe extern "C" fn(*const c_types::c_void) -> T,
    cond: F,
    sleep_us: usize,
    timeout_us: u64,
    addr: *const c_types::c_void,
) -> Result<T>;

With a little bit of work by wrapping read()/write() functions and providing the type invariant
for an T-bit address (example IoAddr<T>), the definition will not contain any unsafe code:

pub fn readx_poll_timeout<T, F: Fn(&T) -> bool>(
    op: fn(&IoAddr<T>) -> T,
    cond: F,
    sleep_us: usize,
    timeout_us: u64,
    addr: &IoAddr<T>,
) -> Result<T>;

The read()/write() functions simply take corresponding invariant as argument:

pub struct IoAddr<T> {
    ptr: usize,
    phantom: PhantomData<T>,
}

/// generic defintion
pub fn read(&IoAddr<T>) -> T;

/// with specializations for byte, word, etc:
pub fn readb(&IoAddr<u8>) -> u8;
pub fn readw(&IoAddr<u16>) -> u16;
pub fn readl(&IoAddr<u32>) -> u32;
pub fn readq(&IoAddr<u64>) -> u64;

// same for write()
pub fn write(&mut IoAddr<T>, val: T);

In this fashion, I may refactor whole io_mem module, removing many of unsafe code and repetitions as a separate patch.

ojeda pushed a commit that referenced this pull request May 9, 2023
afs_make_call() calls rxrpc_kernel_begin_call() to begin a call (which may
get stalled in the background waiting for a connection to become
available); it then calls rxrpc_kernel_set_max_life() to set the timeouts -
but that starts the call timer so the call timer might then expire before
we get a connection assigned - leading to the following oops if the call
stalled:

	BUG: kernel NULL pointer dereference, address: 0000000000000000
	...
	CPU: 1 PID: 5111 Comm: krxrpcio/0 Not tainted 6.3.0-rc7-build3+ #701
	RIP: 0010:rxrpc_alloc_txbuf+0xc0/0x157
	...
	Call Trace:
	 <TASK>
	 rxrpc_send_ACK+0x50/0x13b
	 rxrpc_input_call_event+0x16a/0x67d
	 rxrpc_io_thread+0x1b6/0x45f
	 ? _raw_spin_unlock_irqrestore+0x1f/0x35
	 ? rxrpc_input_packet+0x519/0x519
	 kthread+0xe7/0xef
	 ? kthread_complete_and_exit+0x1b/0x1b
	 ret_from_fork+0x22/0x30

Fix this by noting the timeouts in struct rxrpc_call when the call is
created.  The timer will be started when the first packet is transmitted.

It shouldn't be possible to trigger this directly from userspace through
AF_RXRPC as sendmsg() will return EBUSY if the call is in the
waiting-for-conn state if it dropped out of the wait due to a signal.

Fixes: 9d35d88 ("rxrpc: Move client call connection to the I/O thread")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: "David S. Miller" <davem@davemloft.net>
cc: Eric Dumazet <edumazet@google.com>
cc: Jakub Kicinski <kuba@kernel.org>
cc: Paolo Abeni <pabeni@redhat.com>
cc: linux-afs@lists.infradead.org
cc: netdev@vger.kernel.org
cc: linux-kernel@vger.kernel.org
Signed-off-by: David S. Miller <davem@davemloft.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant