Re: [PATCH v8 3/8] rust: file: add Rust abstraction for `struct file`

From: Boqun Feng
Date: Tue Aug 06 2024 - 15:30:41 EST


On Tue, Aug 06, 2024 at 10:48:11AM +0200, Alice Ryhl wrote:
[...]
> > > + /// Returns the flags associated with the file.
> > > + ///
> > > + /// The flags are a combination of the constants in [`flags`].
> > > + #[inline]
> > > + pub fn flags(&self) -> u32 {
> > > + // This `read_volatile` is intended to correspond to a READ_ONCE call.
> > > + //
> > > + // SAFETY: The file is valid because the shared reference guarantees a nonzero refcount.
> > > + //
> > > + // FIXME(read_once): Replace with `read_once` when available on the Rust side.
> >
> > Do you know the status of this?
>
> It's still unavailable.
>

I think with our own Atomic API, we can just use atomic_read() here:
yes, I know that to make this is not a UB, we need the C side to also do
atomic write on this `f_flags`, however, my reading of C code seems to
suggest that FS relies on writes to this field is atomic, therefore
unless someone is willing to convert all writes to `f_flags` in C into
a WRITE_ONCE(), nothing more we can do on Rust side. So using
atomic_read() is the correct thing to begin with.

Regards,
Boqun

> > > + unsafe { core::ptr::addr_of!((*self.as_ptr()).f_flags).read_volatile() }
> > > + }
> > > +}
[...]