Re: [PATCH 2/3] rust: file: add f_pos and set_f_pos

[Christian Brauner <brauner@kernel.org>]

On Thu, Sep 26, 2024 at 02:58:56PM GMT, Alice Ryhl wrote:
> Add accessors for the file position. Most of the time, you should not
> use these methods directly, and you should instead use a guard for the
> file position to prove that you hold the fpos lock. However, under
> limited circumstances, files are allowed to choose a different locking
> strategy for their file position. These accessors can be used to handle
> that case.
> 
> For now, these accessors are the only way to access the file position
> within the llseek and read_iter callbacks.
> 
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/fs/file.rs | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
> index e03dbe14d62a..c896a3b1d182 100644
> --- a/rust/kernel/fs/file.rs
> +++ b/rust/kernel/fs/file.rs
> @@ -333,6 +333,26 @@ pub fn flags(&self) -> u32 {
>          // FIXME(read_once): Replace with `read_once` when available on the Rust side.
>          unsafe { core::ptr::addr_of!((*self.as_ptr()).f_flags).read_volatile() }
>      }
> +
> +    /// Read the file position.
> +    ///
> +    /// # Safety
> +    ///
> +    /// You must hold the fpos lock or otherwise ensure that no data race will happen.
> +    #[inline]
> +    pub unsafe fn f_pos(&self) -> i64 {
> +        unsafe { (*self.as_ptr()).f_pos }
> +    }
> +
> +    /// Set the file position.
> +    ///
> +    /// # Safety
> +    ///
> +    /// You must hold the fpos lock or otherwise ensure that no data race will happen.
> +    #[inline]
> +    pub unsafe fn set_f_pos(&self, value: i64) {
> +        unsafe { (*self.as_ptr()).f_pos = value };
> +    }

I don't think we want to expose f_pos with its weird locking rule
through rust wrappers. Ideally, it's completely opaque to the callers
and not accessed directly at all.