tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Introducing localcount(9)



> Date: Tue, 16 May 2017 15:18:35 -0700
> From: Chuck Silvers <chuq%chuq.com@localhost>
> 
> is the intent that in the long term everything should be converted to use
> the new interfaces and that the non-"acquire" interfaces be removed?

That was what I had in mind.

> (that would be my preference, as well as keeping the original names...
> the new names are long enough that they make it harder to read the code,
> at least for me.)

Changing the name means code yet to be audited -- including code
out-of-tree -- will fail to compile when we remove the old names.

> if so, is the reason that you aren't converting everything now just that
> you wanted to keep this change to a managable size?  I could help with
> converting the rest of the drivers now if that would be useful.

Not just diffs but testing.

> rather than require every driver to use the "DEVSW_MODULE_INIT" hack
> when defining a devsw structure, it would be better to have bdevsw_attach()
> and cdevsw_attach() take care of the localcount details without the
> driver author needing to be aware of them.  I'd also think it would be
> better to have the d_localcount field be the actual "struct localcount"
> rather than a pointer to one, since you already enforce that the d_localcount
> pointer is non-NULL and not shared between structures.

That's what I would have done originally, but embedding a mutable
structure inside a `const struct cdevsw' doesn't work very well.

> device_acquire() seems of dubious value... it's only safe to use when one
> already holds a reference on the device_t, so having it as an interface
> is just inviting people to try to do fancy (and probably wrong) things
> with the reference counting.  in your branch, you only use it in ld.c
> (where it's either unnecessary or you're using it unsafely) and in fss.c
> (which is just working around the fact that config_attach_pseudo()
> returns a device_t with no reference, and it would seem better to fix
> config_attach_pseudo() rather than work around that in the fss driver.)
> I think it would be better to not have a device_acquire() interface
> exposed to drivers.

Concur.


Home | Main Index | Thread Index | Old Index