From nobody Sun Feb 8 08:48:16 2026 Received: from mail-wm1-f73.google.com (mail-wm1-f73.google.com [209.85.128.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BB3C420B20 for ; Fri, 17 Jan 2025 14:22:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.73 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737123779; cv=none; b=ru6oWFMikKbNk7xkmIUH9K9a8GlAeZL6K8dAWuAY+zE0H45xKaVSC9pbYhrtVBpaOHyq8sIpjh2k8AzlIkHMcv/trfSFM1HIBNkJVDpssOOouhs9Z4NIo1mDE5D8PkNfJz4FK+QdzOKfQJJPM8bvNbL/TH8/MOaMMciiAD4gQQg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737123779; c=relaxed/simple; bh=k4Xpn2RmY6NCWHMXL/iKfqAqRheyeJPA8UiAb1hhX9Q=; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Content-Type; b=D+IJuc4mkadZu+x75/N7zuOD3UbBg1ql2O/LapGhKuqdxPJCNQPytWJp+hdWpqOjUFbVOW2pxPw1T3BPJXWbbxaKl2tVp4mNT5aAtE+HEkCAnOWmL72PmzJxGRLpqxUL47AkoboYnnKJ8IVJ5IBGa2sEZjuU2R4enXHj5yeDyAY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=juLpm+S+; arc=none smtp.client-ip=209.85.128.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="juLpm+S+" Received: by mail-wm1-f73.google.com with SMTP id 5b1f17b1804b1-43582d49dacso15695135e9.2 for ; Fri, 17 Jan 2025 06:22:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1737123776; x=1737728576; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=KMDIxJKKFqAnFYferYlNcN4gVN8VQWHhPTDkuCDbFUY=; b=juLpm+S+5cCiKKtKAy8h16GDcErL3I2lh6y3+hf8y6Wdvg6TuK7v1FT4ox1QT9/5qw OAfqyAOuRfFqKntmQ5KXzUSOZc0voskc7fhdIHgfWelfM6NRIcSf89LGHQRoLyN695OY kAt4fAr+rwk6t6g6UKM8OCKQc2SovS44uPbVG25zrqxdYnXgrBPxhAwYZ1ArdLHDyLO9 Mdnq4ZG6gZOk2ThPPwt4/i+ArxYJmMQQ2kELIixCCl+XqP6xi/tQC2opEMjqieZX0lDb qg/hOMBZELUGh5B5pqv0RxHNHnOq73//pwe1VCim09fobwvasjmyW78h52CwvgQWXXQb fipQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737123776; x=1737728576; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=KMDIxJKKFqAnFYferYlNcN4gVN8VQWHhPTDkuCDbFUY=; b=FZCI7wu3szZCtI2BeKNUsytyr2oT2xgyv50ZTkIHYZFuyMhiUyntZqEm6WrJh6sD+h EfAqRbBlAX3gY7PcbsXQ046TwGPYEr7sKWGMWDZdfllIm2cXq1+rouQiR78WAZFRmlVI pzXbsnj0HjpBEkmhCMiWQbPas3nyBL75E6dpCIubBdd0zaaOaPdwPtvT/6wo09KIHvgv WGpn/dgZRk661wlqm3qL7KQLymHwEJEQXQN8d/BiJMC4HgOq3vAAcJ1P0W6ZPVgUYyKL cPln4LHc6Mmq7WVxPMsBFBIK3g1AloyizMzdbDrPpv6GXxbb8tO8eiV++vWQ4bEGUwoY g52w== X-Forwarded-Encrypted: i=1; AJvYcCUDkobiOc4ZDRe1Tpd2htXcbBGyoC2FsJ1/WAzb1B1Jtwz+2WGi8NNzEs4EiGCwpYix5yhJ1uq5fKGy6c0=@vger.kernel.org X-Gm-Message-State: AOJu0YyhgG5px54eWCe5KPuSPVm5DRFzp8+5pTahsVFMx3fFQwqLP4/m v5PYw82lUkbrSRNwaScVRljGB9LlASPTQeX4M9YllRlHco/1CJkhVQ77X6Be+LjvN4Ldf7XDdU7 4DWM91ZanZNQOvA== X-Google-Smtp-Source: AGHT+IEHnEo27D6Xsau2YhGT46bG5LyVTfwZZeX6wiKnxjGwLHKC+EVmwP3+zlOCVGOY0M4JOUv83UQPRG2gzwg= X-Received: from wmbg17.prod.google.com ([2002:a05:600c:a411:b0:434:f513:bb24]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6000:1ac6:b0:385:f07b:93da with SMTP id ffacd0b85a97d-38bf57bf69bmr2558965f8f.47.1737123776195; Fri, 17 Jan 2025 06:22:56 -0800 (PST) Date: Fri, 17 Jan 2025 14:22:32 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 X-B4-Tracking: v=1; b=H4sIAKhnimcC/y2MywqAIBAAfyX2nKAbZvUr0SF0rT30wIUIpH9Po uPAzGQQSkwCQ5Uh0cXCx17A1BX4dd4XUhwKA2q02hinNhYfiuhJxeMU9VvY6sZiDF1vHZT4TBT 5/sbj9Dwv9/tfcWgAAAA= X-Developer-Key: i=aliceryhl@google.com; a=openpgp; fpr=49F6C1FAA74960F43A5B86A1EE7A392FDE96209F X-Developer-Signature: v=1; a=openpgp-sha256; l=15060; i=aliceryhl@google.com; h=from:subject:message-id; bh=k4Xpn2RmY6NCWHMXL/iKfqAqRheyeJPA8UiAb1hhX9Q=; b=owEBbQKS/ZANAwAKAQRYvu5YxjlGAcsmYgBnimeqxReb7orNnmaYOfFfrPSDGWq0wudLQtFCs 1fOsjoEie2JAjMEAAEKAB0WIQSDkqKUTWQHCvFIvbIEWL7uWMY5RgUCZ4pnqgAKCRAEWL7uWMY5 RlmEEACsoSr3E71KSv1JTxXQdyhxOL/ywLjfk1CG890FHA4qgAPEKuO6DVQtl057K51t8b8UfRR gqTLg5gEwXhao3Yc9xFZPAT7iU7P5c5aY8FVzM/6NrJ4VRzop7oV033tQO7euh2UXErbGtbeVXP ZYfoK9C18RiMBfmhuWTX6KhABn7dLwRKRSbRWyRYzBn/2YTX/Llkuy04nS8lN3B6uM4NQS5Y3w4 iDAW2Q0zu8yRyta7pwkghlx9Q4NHZAyosiiayIxmHBaIk0fKy4p3u4qySOqV7zgnKJdGJ8vSmhr LGccrtTauYPK4Fe8voFVU1zvmFAFHFAarcvwYTF2epyDLduZZDj70y8sudQDMNAVR0euzBSz+Pq +AYe7XqsLR6adqrzkXUM/bpTf0KxBbihlxGiocsj48DMLUx9Ac4lbOORT9l41/lc01ABTg/XjRe LIqklY6987aQWS4LyG7rFed1mFrhDdqVfbZ9aS6oN3xCXG+VUSrYvOsg6hu2nCqf3fDw1fEP748 BHnS+2V64ZJ2YD3zk2CEPlb0v2fpqKvlCARPeyVIZIG3IILLv+B+V+pTsbtT89jvSfgZ/+5xkB5 aYdE3QVYZB1+OsJf5eY05PG6a/nqDoSB9H31qYYiN3m1upOfJCRfTwkat0dnOmuYboq4IO69UBH nQIhKtO8w13WTrQ== X-Mailer: b4 0.13.0 Message-ID: <20250117-miscdevice-fops-change-v1-1-ec04b701c076@google.com> Subject: [PATCH] rust: miscdevice: change how f_ops vtable is constructed From: Alice Ryhl To: Greg Kroah-Hartman , Arnd Bergmann Cc: Miguel Ojeda , Alex Gaynor , Boqun Feng , Gary Guo , "=?utf-8?q?Bj=C3=B6rn_Roy_Baron?=" , Benno Lossin , Andreas Hindborg , Trevor Gross , rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org, Alice Ryhl Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable I was helping someone with writing a new Rust abstraction, and we were using the miscdevice abstraction as an example. While doing this, it became clear to me that the way I implemented the f_ops vtable is confusing to new Rust users, and that the approach used by the block abstractions is less confusing. Thus, update the miscdevice abstractions to use the same approach as rust/kernel/block/mq/operations.rs. Sorry about the large diff. This changes the indentation of a large amount of code. Signed-off-by: Alice Ryhl Reviewed-by: Christian Schrefl --- rust/kernel/miscdevice.rs | 295 ++++++++++++++++++++++--------------------= ---- 1 file changed, 141 insertions(+), 154 deletions(-) diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs index dfb363630c70..ad02434cbeaf 100644 --- a/rust/kernel/miscdevice.rs +++ b/rust/kernel/miscdevice.rs @@ -39,7 +39,7 @@ pub const fn into_raw(self) -> bindings::m= iscdevice { let mut result: bindings::miscdevice =3D unsafe { MaybeUninit::zer= oed().assume_init() }; result.minor =3D bindings::MISC_DYNAMIC_MINOR as _; result.name =3D self.name.as_char_ptr(); - result.fops =3D create_vtable::(); + result.fops =3D MiscdeviceVTable::::build(); result } } @@ -164,171 +164,158 @@ fn show_fdinfo( } } =20 -const fn create_vtable() -> &'static bindings::file_operati= ons { - const fn maybe_fn(check: bool, func: T) -> Option { - if check { - Some(func) - } else { - None +/// A vtable for the file operations of a Rust miscdevice. +struct MiscdeviceVTable(PhantomData); + +impl MiscdeviceVTable { + /// # Safety + /// + /// `file` and `inode` must be the file and inode for a file that is u= ndergoing initialization. + /// The file must be associated with a `MiscDeviceRegistration`. + unsafe extern "C" fn open(inode: *mut bindings::inode, raw_file: *mut = bindings::file) -> c_int { + // SAFETY: The pointers are valid and for a file being opened. + let ret =3D unsafe { bindings::generic_file_open(inode, raw_file) = }; + if ret !=3D 0 { + return ret; } - } =20 - struct VtableHelper { - _t: PhantomData, - } - impl VtableHelper { - const VTABLE: bindings::file_operations =3D bindings::file_operati= ons { - open: Some(fops_open::), - release: Some(fops_release::), - unlocked_ioctl: maybe_fn(T::HAS_IOCTL, fops_ioctl::), - #[cfg(CONFIG_COMPAT)] - compat_ioctl: if T::HAS_COMPAT_IOCTL { - Some(fops_compat_ioctl::) - } else if T::HAS_IOCTL { - Some(bindings::compat_ptr_ioctl) - } else { - None - }, - show_fdinfo: maybe_fn(T::HAS_SHOW_FDINFO, fops_show_fdinfo::), - // SAFETY: All zeros is a valid value for `bindings::file_oper= ations`. - ..unsafe { MaybeUninit::zeroed().assume_init() } - }; - } + // SAFETY: The open call of a file can access the private data. + let misc_ptr =3D unsafe { (*raw_file).private_data }; =20 - &VtableHelper::::VTABLE -} + // SAFETY: This is a miscdevice, so `misc_open()` set the private = data to a pointer to the + // associated `struct miscdevice` before calling into this method.= Furthermore, `misc_open()` + // ensures that the miscdevice can't be unregistered and freed dur= ing this call to `fops_open`. + let misc =3D unsafe { &*misc_ptr.cast::>= () }; =20 -/// # Safety -/// -/// `file` and `inode` must be the file and inode for a file that is under= going initialization. -/// The file must be associated with a `MiscDeviceRegistration`. -unsafe extern "C" fn fops_open( - inode: *mut bindings::inode, - raw_file: *mut bindings::file, -) -> c_int { - // SAFETY: The pointers are valid and for a file being opened. - let ret =3D unsafe { bindings::generic_file_open(inode, raw_file) }; - if ret !=3D 0 { - return ret; - } + // SAFETY: + // * This underlying file is valid for (much longer than) the dura= tion of `T::open`. + // * There is no active fdget_pos region on the file on this threa= d. + let file =3D unsafe { File::from_raw_file(raw_file) }; =20 - // SAFETY: The open call of a file can access the private data. - let misc_ptr =3D unsafe { (*raw_file).private_data }; - - // SAFETY: This is a miscdevice, so `misc_open()` set the private data= to a pointer to the - // associated `struct miscdevice` before calling into this method. Fur= thermore, `misc_open()` - // ensures that the miscdevice can't be unregistered and freed during = this call to `fops_open`. - let misc =3D unsafe { &*misc_ptr.cast::>() }; + let ptr =3D match T::open(file, misc) { + Ok(ptr) =3D> ptr, + Err(err) =3D> return err.to_errno(), + }; =20 - // SAFETY: - // * This underlying file is valid for (much longer than) the duration= of `T::open`. - // * There is no active fdget_pos region on the file on this thread. - let file =3D unsafe { File::from_raw_file(raw_file) }; + // This overwrites the private data with the value specified by th= e user, changing the type of + // this file's private data. All future accesses to the private da= ta is performed by other + // fops_* methods in this file, which all correctly cast the priva= te data to the new type. + // + // SAFETY: The open call of a file can access the private data. + unsafe { (*raw_file).private_data =3D ptr.into_foreign().cast_mut(= ) }; =20 - let ptr =3D match T::open(file, misc) { - Ok(ptr) =3D> ptr, - Err(err) =3D> return err.to_errno(), - }; - - // This overwrites the private data with the value specified by the us= er, changing the type of - // this file's private data. All future accesses to the private data i= s performed by other - // fops_* methods in this file, which all correctly cast the private d= ata to the new type. - // - // SAFETY: The open call of a file can access the private data. - unsafe { (*raw_file).private_data =3D ptr.into_foreign().cast_mut() }; + 0 + } =20 - 0 -} + /// # Safety + /// + /// `file` and `inode` must be the file and inode for a file that is b= eing released. The file must + /// be associated with a `MiscDeviceRegistration`. + unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut = bindings::file) -> c_int { + // SAFETY: The release call of a file owns the private data. + let private =3D unsafe { (*file).private_data }; + // SAFETY: The release call of a file owns the private data. + let ptr =3D unsafe { ::from_foreign(priv= ate) }; + + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this threa= d. + T::release(ptr, unsafe { File::from_raw_file(file) }); + + 0 + } =20 -/// # Safety -/// -/// `file` and `inode` must be the file and inode for a file that is being= released. The file must -/// be associated with a `MiscDeviceRegistration`. -unsafe extern "C" fn fops_release( - _inode: *mut bindings::inode, - file: *mut bindings::file, -) -> c_int { - // SAFETY: The release call of a file owns the private data. - let private =3D unsafe { (*file).private_data }; - // SAFETY: The release call of a file owns the private data. - let ptr =3D unsafe { ::from_foreign(private)= }; - - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - T::release(ptr, unsafe { File::from_raw_file(file) }); - - 0 -} + /// # Safety + /// + /// `file` must be a valid file that is associated with a `MiscDeviceR= egistration`. + unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg= : c_ulong) -> c_long { + // SAFETY: The ioctl call of a file can access the private data. + let private =3D unsafe { (*file).private_data }; + // SAFETY: Ioctl calls can borrow the private data of the file. + let device =3D unsafe { ::borrow(private= ) }; + + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this threa= d. + let file =3D unsafe { File::from_raw_file(file) }; + + match T::ioctl(device, file, cmd, arg as usize) { + Ok(ret) =3D> ret as c_long, + Err(err) =3D> err.to_errno() as c_long, + } + } =20 -/// # Safety -/// -/// `file` must be a valid file that is associated with a `MiscDeviceRegis= tration`. -unsafe extern "C" fn fops_ioctl( - file: *mut bindings::file, - cmd: c_uint, - arg: c_ulong, -) -> c_long { - // SAFETY: The ioctl call of a file can access the private data. - let private =3D unsafe { (*file).private_data }; - // SAFETY: Ioctl calls can borrow the private data of the file. - let device =3D unsafe { ::borrow(private) }; - - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - let file =3D unsafe { File::from_raw_file(file) }; - - match T::ioctl(device, file, cmd, arg as usize) { - Ok(ret) =3D> ret as c_long, - Err(err) =3D> err.to_errno() as c_long, + /// # Safety + /// + /// `file` must be a valid file that is associated with a `MiscDeviceR= egistration`. + #[cfg(CONFIG_COMPAT)] + unsafe extern "C" fn compat_ioctl( + file: *mut bindings::file, + cmd: c_uint, + arg: c_ulong, + ) -> c_long { + // SAFETY: The compat ioctl call of a file can access the private = data. + let private =3D unsafe { (*file).private_data }; + // SAFETY: Ioctl calls can borrow the private data of the file. + let device =3D unsafe { ::borrow(private= ) }; + + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this threa= d. + let file =3D unsafe { File::from_raw_file(file) }; + + match T::compat_ioctl(device, file, cmd, arg as usize) { + Ok(ret) =3D> ret as c_long, + Err(err) =3D> err.to_errno() as c_long, + } } -} =20 -/// # Safety -/// -/// `file` must be a valid file that is associated with a `MiscDeviceRegis= tration`. -#[cfg(CONFIG_COMPAT)] -unsafe extern "C" fn fops_compat_ioctl( - file: *mut bindings::file, - cmd: c_uint, - arg: c_ulong, -) -> c_long { - // SAFETY: The compat ioctl call of a file can access the private data. - let private =3D unsafe { (*file).private_data }; - // SAFETY: Ioctl calls can borrow the private data of the file. - let device =3D unsafe { ::borrow(private) }; - - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - let file =3D unsafe { File::from_raw_file(file) }; - - match T::compat_ioctl(device, file, cmd, arg as usize) { - Ok(ret) =3D> ret as c_long, - Err(err) =3D> err.to_errno() as c_long, + /// # Safety + /// + /// - `file` must be a valid file that is associated with a `MiscDevic= eRegistration`. + /// - `seq_file` must be a valid `struct seq_file` that we can write t= o. + unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, fi= le: *mut bindings::file) { + // SAFETY: The release call of a file owns the private data. + let private =3D unsafe { (*file).private_data }; + // SAFETY: Ioctl calls can borrow the private data of the file. + let device =3D unsafe { ::borrow(private= ) }; + // SAFETY: + // * The file is valid for the duration of this call. + // * There is no active fdget_pos region on the file on this threa= d. + let file =3D unsafe { File::from_raw_file(file) }; + // SAFETY: The caller ensures that the pointer is valid and exclus= ive for the duration in which + // this method is called. + let m =3D unsafe { SeqFile::from_raw(seq_file) }; + + T::show_fdinfo(device, m, file); } -} =20 -/// # Safety -/// -/// - `file` must be a valid file that is associated with a `MiscDeviceReg= istration`. -/// - `seq_file` must be a valid `struct seq_file` that we can write to. -unsafe extern "C" fn fops_show_fdinfo( - seq_file: *mut bindings::seq_file, - file: *mut bindings::file, -) { - // SAFETY: The release call of a file owns the private data. - let private =3D unsafe { (*file).private_data }; - // SAFETY: Ioctl calls can borrow the private data of the file. - let device =3D unsafe { ::borrow(private) }; - // SAFETY: - // * The file is valid for the duration of this call. - // * There is no active fdget_pos region on the file on this thread. - let file =3D unsafe { File::from_raw_file(file) }; - // SAFETY: The caller ensures that the pointer is valid and exclusive = for the duration in which - // this method is called. - let m =3D unsafe { SeqFile::from_raw(seq_file) }; - - T::show_fdinfo(device, m, file); + const VTABLE: bindings::file_operations =3D bindings::file_operations { + open: Some(Self::open), + release: Some(Self::release), + unlocked_ioctl: if T::HAS_IOCTL { + Some(Self::ioctl) + } else { + None + }, + #[cfg(CONFIG_COMPAT)] + compat_ioctl: if T::HAS_COMPAT_IOCTL { + Some(Self::compat_ioctl) + } else if T::HAS_IOCTL { + Some(Self::bindings::compat_ptr_ioctl) + } else { + None + }, + show_fdinfo: if T::HAS_SHOW_FDINFO { + Some(Self::show_fdinfo) + } else { + None + }, + // SAFETY: All zeros is a valid value for `bindings::file_operatio= ns`. + ..unsafe { MaybeUninit::zeroed().assume_init() } + }; + + const fn build() -> &'static bindings::file_operations { + &Self::VTABLE + } } --- base-commit: 01b3cb620815fc3feb90ee117d9445a5b608a9f7 change-id: 20250117-miscdevice-fops-change-260352fd8957 Best regards, --=20 Alice Ryhl