Wedson Almeida Filho <wedsonaf@xxxxxxxxx> writes:
[...]
> @@ -239,6 +255,16 @@ pub fn new<T: FileSystem + ?Sized>(module: &'static ThisModule) -> impl PinInit<
> unsafe { T::Data::from_foreign(ptr) };
> }
> }
> +
> + unsafe extern "C" fn inode_init_once_callback<T: FileSystem + ?Sized>(
> + outer_inode: *mut core::ffi::c_void,
> + ) {
A docstring with intended use for this function would be nice.
> + let ptr = outer_inode.cast::<INodeWithData<T::INodeData>>();
> +
> + // SAFETY: This is only used in `new`, so we know that we have a valid `INodeWithData`
> + // instance whose inode part can be initialised.
What does "This" refer to here?
> + unsafe { bindings::inode_init_once(ptr::addr_of_mut!((*ptr).inode)) };
> + }
> }
>
> #[pinned_drop]
> @@ -280,6 +306,15 @@ pub fn super_block(&self) -> &SuperBlock<T> {
> unsafe { &*(*self.0.get()).i_sb.cast() }
I would prefer the target type of the cast to be explicit.
[...]
>
> impl<T: FileSystem + ?Sized> NewINode<T> {
> /// Initialises the new inode with the given parameters.
> - pub fn init(self, params: INodeParams) -> Result<ARef<INode<T>>> {
> - // SAFETY: This is a new inode, so it's safe to manipulate it mutably.
> - let inode = unsafe { &mut *self.0 .0.get() };
> + pub fn init(self, params: INodeParams<T::INodeData>) -> Result<ARef<INode<T>>> {
> + let outerp = container_of!(self.0 .0.get(), INodeWithData<T::INodeData>, inode);
> +
> + // SAFETY: This is a newly-created inode. No other references to it exist, so it is
> + // safe to mutably dereference it.
> + let outer = unsafe { &mut *outerp.cast_mut() };
Same
[...]
> @@ -766,6 +822,61 @@ impl<T: FileSystem + ?Sized> Tables<T> {
> shutdown: None,
> };
>
> + unsafe extern "C" fn alloc_inode_callback(
> + sb: *mut bindings::super_block,
> + ) -> *mut bindings::inode {
> + // SAFETY: The callback contract guarantees that `sb` is valid for read.
> + let super_type = unsafe { (*sb).s_type };
> +
> + // SAFETY: This callback is only used in `Registration`, so `super_type` is necessarily
> + // embedded in a `Registration`, which is guaranteed to be valid because it has a
> + // superblock associated to it.
> + let reg = unsafe { &*container_of!(super_type, Registration, fs) };
> +
> + // SAFETY: `sb` and `cache` are guaranteed to be valid by the callback contract and by
> + // the existence of a superblock respectively.
> + let ptr = unsafe {
> + bindings::alloc_inode_sb(sb, MemCache::ptr(®.inode_cache), bindings::GFP_KERNEL)
> + }
> + .cast::<INodeWithData<T::INodeData>>();
> + if ptr.is_null() {
> + return ptr::null_mut();
> + }
> + ptr::addr_of_mut!((*ptr).inode)
> + }
> +
> + unsafe extern "C" fn destroy_inode_callback(inode: *mut bindings::inode) {
> + // SAFETY: By the C contract, `inode` is a valid pointer.
> + let is_bad = unsafe { bindings::is_bad_inode(inode) };
> +
> + // SAFETY: The inode is guaranteed to be valid by the callback contract. Additionally, the
> + // superblock is also guaranteed to still be valid by the inode existence.
> + let super_type = unsafe { (*(*inode).i_sb).s_type };
> +
> + // SAFETY: This callback is only used in `Registration`, so `super_type` is necessarily
> + // embedded in a `Registration`, which is guaranteed to be valid because it has a
> + // superblock associated to it.
> + let reg = unsafe { &*container_of!(super_type, Registration, fs) };
> + let ptr = container_of!(inode, INodeWithData<T::INodeData>, inode).cast_mut();
Same
> +
> + if !is_bad {
> + // SAFETY: The code either initialises the data or marks the inode as bad. Since the
> + // inode is not bad, the data is initialised, and thus safe to drop.
> + unsafe { ptr::drop_in_place((*ptr).data.as_mut_ptr()) };
> + }
> +
> + if size_of::<T::INodeData>() == 0 {
> + // SAFETY: When the size of `INodeData` is zero, we don't use a separate mem_cache, so
> + // it is allocated from the regular mem_cache, which is what `free_inode_nonrcu` uses
> + // to free the inode.
> + unsafe { bindings::free_inode_nonrcu(inode) };
> + } else {
> + // The callback contract guarantees that the inode was previously allocated via the
> + // `alloc_inode_callback` callback, so it is safe to free it back to the cache.
> + unsafe { bindings::kmem_cache_free(MemCache::ptr(®.inode_cache), ptr.cast()) };
Same
BR Andreas
