Re: [PATCH v3 0/5] rust: miscdevice: Provide sample driver using the new MiscDevice bindings

[Alice Ryhl]

On Fri, Dec 6, 2024 at 9:11 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, Dec 06, 2024 at 07:44:43AM +0000, Lee Jones wrote:
> > On Fri, 06 Dec 2024, Greg KH wrote:
> >
> > > On Thu, Dec 05, 2024 at 04:25:17PM +0000, Lee Jones wrote:
> > > > It has been suggested that the driver should use dev_info() instead of
> > > > pr_info() however there is currently no scaffolding to successfully pull
> > > > a 'struct device' out from driver data post register().  This is being
> > > > worked on and we will convert this over in due course.
> > >
> > > But the miscdevice.rs change provides this to you, right?  Or if not,
> > > why not?
> >
> > This does allow us to pull the 'struct device *` out from `struct
> > miscdevice`; however, since this resides in MiscDeviceRegistration,
> > which we lose access to after .init, we have no means to call it.
> >
> > Alice is going to work on a way to use ThisModule to get the
> > MiscDeviceRegistration reference back from anywhere in the module. Until
> > that piece lands, we can't call MiscDeviceRegistration::device() outside
> > of RustMiscDeviceModule.
>
> That seems crazy, as ThisModule shouldn't be dealing with a static misc
> device, what happens for dynamically created ones like all
> normal/sane/non-example drivers do?  This should "just" be a dynamic
> object that is NOT tied to the module object, or worst case, a "static"
> structure that is tied to the module I guess?
>
> Anyway, I'll let you all work it out, good luck!

If you store it somewhere else, you're probably okay. The current
place is just hard to access.

The problem is that the Rust module abstractions generate a global
variable that holds an RustMiscDeviceModule which is initialized in
init_module() and destroyed in cleanup_module(). To have safe access
to this global, we need to ensure that you access it only between
init_module() and cleanup_module(). For loadable modules, the
try_module_get() logic seems perfect, so in Miscdevice::open we have a
file pointer, which implies that the fs infrastructure took a refcount
on fops->owner, which it can only do once init_module() is done.

Unfortunately, this doesn't translate to built-in modules since the
owner pointer is just null, and try_module_get performs no checks at
all.

Also, I'm realizing now that try_module_get() succeeds even if `state
== MODULE_STATE_COMING`. :(

So in conclusion, I don't know of any way to provide safe access to
the global RustMiscDeviceModule value.

Alice