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 4DF1B1A239D; Wed, 8 Jan 2025 12:45:32 +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=1736340332; cv=none; b=O52mOWhkOJ2YK6SahCNOXZuFympIy69Qc8stKNg4wu1BhoTnWlJ7Vcj3/gHkvMhEeSOQliMSOiHDGEipCNgzZ0m6jFvda7jfSgu3q7JD2CfQVsfCwljeu+7lmbjnffJVgxQ83XinjWUAvlLjldOZ+7olTDZhUb8TIkFS6rrShfE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736340332; c=relaxed/simple; bh=IF8cbp+7gNJeGLCETAgpwe4NyF9qqSHtjma2BuF+uMY=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=BYf/reCx7JpKd37iEcKuXDbiqdoDisnd0neZx6hOBk1eS5Poz0uOTWIpNGDZqV32lTFNQnNPmuUr1tAy4K35wEXGfOs+qyJefcxZkZ7m45p3HmRCH3hKsWMlaQmY8NnoV/1utB3WGVcOtQkoOOOwxMhBzEVkMOO3kW4+vAsAs50= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=t5XSw59i; 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="t5XSw59i" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 3B776C4CEDD; Wed, 8 Jan 2025 12:45:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736340331; bh=IF8cbp+7gNJeGLCETAgpwe4NyF9qqSHtjma2BuF+uMY=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=t5XSw59iEASl0rRz8F6kTdUMM3UrJl11HK/sDXy8g8wMeMvG1XJcUhscfigclcKds jZuFiaurMx5sQWi+CBTF6aK8zLpT5RRLEhj8Enb4gU4wED1DkSSWHLWVdajiH+C6Ub o/DOA4kcuH6dw+mWJh44Vj6UfehxgFcCKMUgkbr7Fwn/0IChYNCc/yB1Jaev2oM52L 9A3F1pSwC/alTWyku/eyeQCoJXtxPjAw5JJJyVMujFcJuxllgb7LDLfYxnBsUgQjd6 j7i9cWgl1ek4fUYVzxRbS8fN1q0YbcwdVoh150cecj4+XYyw544ggg7bFEvH85EsbC 3nkPEsaP7mDFg== From: Andreas Hindborg To: "Miguel Ojeda" Cc: "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , =?utf-8?Q?Bj=C3=B6rn?= Roy Baron , "Benno Lossin" , "Alice Ryhl" , "Masahiro Yamada" , "Nathan Chancellor" , "Nicolas Schier" , "Trevor Gross" , "Adam Bratschi-Kaye" , , , Subject: Re: [PATCH v3 4/4] rust: add parameter support to the `module!` macro In-Reply-To: (Miguel Ojeda's message of "Fri, 13 Dec 2024 18:14:37 +0100") References: <20241213-module-params-v3-v3-0-485a015ac2cf@kernel.org> <20241213-module-params-v3-v3-4-485a015ac2cf@kernel.org> <87y10jd8o0.fsf@kernel.org> User-Agent: mu4e 1.12.7; emacs 29.4 Date: Wed, 08 Jan 2025 13:45:20 +0100 Message-ID: <87tta9bhjz.fsf@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable "Miguel Ojeda" writes: > On Fri, Dec 13, 2024 at 2:17=E2=80=AFPM Andreas Hindborg wrote: >> >> scheduled for removal. Interior mutability via `SyncUnsafeCell` provides >> the same functionality and it is my understanding that this feature is >> on track to be stabilized. > > I am not sure about the last bit, but even if it is on track, we do > not want to start using new language features or APIs that could > potentially change. > > And even if it is a feature that we are sure will not change, we > should still let upstream Rust know before using it, since we are > actively discussing with them the remaining unstable items that the > kernel needs and they are putting the kernel in their roadmap. > > So I suggest we mention it next week in the Rust/Rust for Linux meeting. I don't think we ever discussed this? I was going to put this in the commit trailer for v4: --- Version 3 of this patch enabled the unstable feature `sync_unsafe_cell` [1]= to avoid `static mut` variables as suggested by Trevor Tross and Benno Lossin = [2]. As we are moving closer to a new edition, it is now clear that `static mut`= is not being deprecated in the 2024 edition as first anticipated [3]. Still, `SyncUnsafeCell` allows us to use safe code when referring to the parameter value: ``` {param_name}.as_mut_ptr().cast() ``` rather than unsafe code: ``` unsafe { addr_of_mut!(__{name}_{param_name}_value) }.cast() ``` Thus, this version (4) keeps the feature enabled. [1] https://github.com/rust-lang/rust/issues/95439 [2] https://lore.kernel.org/all/CALNs47sqt=3D=3Do+hM5M1b0vTayKH177naybg_Kur= cirXszYAa22A@mail.gmail.com/ [3] https://github.com/rust-lang/rust/issues/53639#issuecomment-2434023115 --- What do you think? > >> Not sure. `val` being null not supposed to happen in the current >> configuration. It should be an unreachable state. So BUG is the right th= ing? > > Since you can easily return an error in this situation, I would say > ideally a `WARN_ON_ONCE` + returning an error would be the best > option, and covers you in case the rest changes and someone forgets to > update this. Returning an error and `pr_warn!` is doable. As far as I can tell, we do not have `WARN_ON_ONCE` yet? > >> Not in the current configuration. The parameters can only be declared >> "read only". It should be impossible for anyone to call this function. > > What I meant is, can you avoid writing the function to begin with, by > leaving a null function pointer in the `kernel_param_ops` struct, i.e. > `None`? > It turns out we can! Best regards, Andreas Hindborg