netdev - Re: iopoll abstraction

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zv6pW3Mn6qxHxTGE@boqun-archlinux>
Date: Thu, 3 Oct 2024 07:25:31 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: dirk.behme@...bosch.com, andrew@...n.ch, aliceryhl@...gle.com,
	netdev@...r.kernel.org, rust-for-linux@...r.kernel.org,
	hkallweit1@...il.com, tmgross@...ch.edu, ojeda@...nel.org,
	alex.gaynor@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
	benno.lossin@...ton.me, a.hindborg@...sung.com
Subject: Re: iopoll abstraction

On Thu, Oct 03, 2024 at 01:45:18PM +0000, FUJITA Tomonori wrote:
> On Thu, 3 Oct 2024 04:52:48 -0700
> Boqun Feng <boqun.feng@...il.com> wrote:
> 
> > You could use closure as a parameter to avoid macro interface, something
> > like:
> > 
> > 	fn read_poll_timeout<Op, Cond, T>(
> > 	    op: Op,
> > 	    cond: Cond,
> > 	    sleep: Delta,
> > 	    timeout: Delta,
> > 	) -> Result<T> where
> > 	    Op: Fn() -> T,
> > 	    cond: Fn() -> bool {
> > 
> > 	    let __timeout = kernel::Ktime::ktime_get() + timeout;
> > 
> > 	    let val = loop {
> > 		let val = op();
> > 		if cond() {
> > 		    break Some(val);
> > 		}
> > 		kernel::delay::sleep(sleep);
> > 
> > 		if __timeout.after(kernel::Ktime::ktime_get()) {
> > 		    break None;
> > 		}
> > 	    };
> > 
> > 	    if cond() {
> > 		val
> > 	    } else {
> > 		Err(kernel::error::code::ETIMEDOUT)
> > 	    }
> > 	}
> 
> Great! I changed couple of things.
> 
> 1. Op typically reads a value from a register and could need mut objects. So I use FnMut.
> 2. reading from hardware could fail so Op had better to return an error [1].
> 3. Cond needs val; typically check the value from a register.
> 
> [1] https://lore.kernel.org/netdev/ec7267b5-ae77-4c4a-94f8-aa933c87a9a2@lunn.ch
> 
> Seems that the following works QT2025 driver. How does it look?
> 

Makes sense to me. Of course, more users will probably tell us whether
we cover all the cases, but this is a good starting point. I would put
the function at kernel::io::poll, but that's my personal preference ;-)

Regards,
Boqun

> fn read_poll_timeout<Op, Cond, T: Copy>(
>     mut op: Op,
>     cond: Cond,
>     sleep: Delta,
>     timeout: Delta,
> ) -> Result<T>
> where
>     Op: FnMut() -> Result<T>,
>     Cond: Fn(T) -> bool,
> {
>     let timeout = Ktime::ktime_get() + timeout;
>     let ret = loop {
>         let val = op()?;
>         if cond(val) {
>             break Ok(val);
>         }
>         kernel::delay::sleep(sleep);
> 
>         if Ktime::ktime_get() > timeout {
>             break Err(code::ETIMEDOUT);
>         }
>     };
> 
>     ret
> }