From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AE0972853EF; Fri, 20 Jun 2025 11:29:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750418962; cv=none; b=oVd3MNFT5rFfCEdMzie6JbMTe+8Gl4aQjPnFjN5hDlrPkmK7dHHmyUvJPtCpEc/1Q5iS9vLL7dC+giV+HgdCj3RYYxE+bm4Y0Ry+f1aI4RIK4Xg7GQQRqSbw2SWs9J1ChF2BX+UNCSZW0goaPBK9OIniihvohVCS2FRoTfJ4oEQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1750418962; c=relaxed/simple; bh=c4Rc2nFa3I49nxqi5SgaiDRGigf/JNf4RcytD5THg7g=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=exvH4P+5oMb9Eg7qu4dVkPdzyxCsAeCoRd4NNoUlnjS2fim5IZOyt25pmfKRhwcL721GPI6yVei8n4+D+MdYvdtyO0WWGwPJvb7Mm9NABOlTljsAuZuVtmefzCy/15McaXgbMua6eyiE2rQ9NTwMmUmNEJnBJzFK4lvqi6vlWWI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=PcB91OzR; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="PcB91OzR" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 98A16C4CEE3; Fri, 20 Jun 2025 11:29:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1750418962; bh=c4Rc2nFa3I49nxqi5SgaiDRGigf/JNf4RcytD5THg7g=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=PcB91OzROARD50vmekIMFW8RbjI1rBadXKSlSMxIFpR/AIGvnKl8eoxyesnMbPGve C01F65HbecH8pc0TukmBZ7HdhL1q2kFziyZmB+0spokiEsnzPvNn/D6dvkcp0jOoVk prbXUdoN89pXXEzLOjlMy1/9lDprj/pIxdLCfwYm9EdR9N06QIDMyf9tdUfM9eTQWr D74VbkA2mSdfLKLQfpza0alcj3TvhtrsU6mDGLzFhLmcQ9l6MjVh6VqZmmPC3EFFoM 7FjtcnSxMl0gFrmTpxnFyY9wB+sVLjeU+kgif//v9mhUfY/cUApC0OgzAE41vup1Gj 5LPGzkLU9dF9w== From: Andreas Hindborg To: "Benno Lossin" Cc: "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?Q?Bj=C3=B6rn?= Roy Baron , "Alice Ryhl" , "Masahiro Yamada" , "Nathan Chancellor" , "Luis Chamberlain" , "Danilo Krummrich" , "Nicolas Schier" , "Trevor Gross" , "Adam Bratschi-Kaye" , , , , "Petr Pavlu" , "Sami Tolvanen" , "Daniel Gomez" , "Simona Vetter" , "Greg KH" , "Fiona Behrens" , "Daniel Almeida" , Subject: Re: [PATCH v13 2/6] rust: introduce module_param module In-Reply-To: (Benno Lossin's message of "Thu, 19 Jun 2025 15:15:00 +0200") References: <20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org> <20250612-module-params-v3-v13-2-bc219cd1a3f8@kernel.org> User-Agent: mu4e 1.12.9; emacs 30.1 Date: Fri, 20 Jun 2025 13:29:11 +0200 Message-ID: <87ikkq648o.fsf@kernel.org> Precedence: bulk X-Mailing-List: linux-kbuild@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain "Benno Lossin" writes: > On Thu Jun 12, 2025 at 3:40 PM CEST, Andreas Hindborg wrote: >> +/// A wrapper for kernel parameters. >> +/// >> +/// This type is instantiated by the [`module!`] macro when module parameters are >> +/// defined. You should never need to instantiate this type directly. >> +/// >> +/// Note: This type is `pub` because it is used by module crates to access >> +/// parameter values. >> +#[repr(transparent)] >> +pub struct ModuleParamAccess { >> + data: core::cell::UnsafeCell, >> +} >> + >> +// SAFETY: We only create shared references to the contents of this container, >> +// so if `T` is `Sync`, so is `ModuleParamAccess`. >> +unsafe impl Sync for ModuleParamAccess {} >> + >> +impl ModuleParamAccess { >> + #[doc(hidden)] >> + pub const fn new(value: T) -> Self { >> + Self { >> + data: core::cell::UnsafeCell::new(value), >> + } >> + } >> + >> + /// Get a shared reference to the parameter value. >> + // Note: When sysfs access to parameters are enabled, we have to pass in a >> + // held lock guard here. >> + pub fn get(&self) -> &T { >> + // SAFETY: As we only support read only parameters with no sysfs >> + // exposure, the kernel will not touch the parameter data after module >> + // initialization. > > This should be a type invariant. But I'm having difficulty defining one > that's actually correct: after parsing the parameter, this is written > to, but when is that actually? For built-in modules it is during kernel initialization. For loadable modules, it during module load. No code from the module will execute before parameters are set. > Would we eventually execute other Rust > code during that time? (for example when we allow custom parameter > parsing) I don't think we will need to synchronize because of custom parameter parsing. Parameters are initialized sequentially. It is not a problem if the custom parameter parsing code name other parameters, because they are all initialized to valid values (as they are statics). > > This function also must never be `const` because of the following: > > module! { > // ... > params: { > my_param: i64 { > default: 0, > description: "", > }, > }, > } > > static BAD: &'static i64 = module_parameters::my_param.get(); > > AFAIK, this static will be executed before loading module parameters and > thus it makes writing to the parameter UB. As I understand, the static will be initialized by a constant expression evaluated at compile time. I am not sure what happens when this is evaluated in const context: pub fn get(&self) -> &T { // SAFETY: As we only support read only parameters with no sysfs // exposure, the kernel will not touch the parameter data after module // initialization. unsafe { &*self.data.get() } } Why would that not be OK? I would assume the compiler builds a dependency graph when initializing statics? Best regards, Andreas Hindborg