incorrect parent refcounting in subr_firmware.c?
Navdeep Parhar
nparhar at gmail.com
Tue Nov 8 20:47:10 UTC 2011
On Tue, Nov 08, 2011 at 10:00:27AM -0500, John Baldwin wrote:
> On Wednesday, November 02, 2011 3:56:29 pm Navdeep Parhar wrote:
> > I built a KLD with multiple firmware images, as shown here:
> >
> > KMOD=foo
> > FIRMWS= foo.bin:foo:1.0.0.0
> > FIRMWS+=bar.bin:bar:1.0.0.0
> > FIRMWS+= ...
> > .include <bsd.kmod.mk>
> >
> > "foo" is the parent firmware and a firmware_get(foo) can autoload the
> > KLD. "bar" and the rest are available only if the KLD is loaded (by
> > whatever means). This is reasonable and works as expected. But if I
> > just get and then put "foo" back, the KLD is not unloaded automatically.
> >
> > The problem is that a reference is placed on the parent firmware when
> > the other firmwares are registered (during module load). I think this
> > reference should be placed during firmware_get on the child.
> >
> > What do people think about the attached patch? It fixes things for me.
>
> Hmm, what about the use case where a driver does:
>
> f = firmware_get("foo");
> firmware_put(f)
> f = firmware_get("bar");
> firmware_put(f)
>
> Is that going to trigger multiple loads/unloads with this change?
Without the patch, the get(bar) always works. But that's because the
KLD is stuck in memory (it's not auto-unloaded even after the two puts
and it can't be unloaded manually either).
With the patch, get(bar) does not work after put(foo, FIRMWARE_UNLOAD).
This is correct behaviour as documented in firmware(9) - the parent
firmware is the only one that the kernel can locate with a simple name
match. Once the KLD is unloaded, the kernel can't find bar. (The get
would probably work after a put(foo, 0) because the KLD won't get
unloaded without a FW_UNLOAD. But I'm not interested in keeping it
around forever so I always specify FW_UNLOAD).
As long as I put all the images back this way, the KLD is auto-unloaded
at the end. Without the patch it just stays around forever.
get(foo) /* autoloads foo.ko which has the "bar" image too. */
get(bar)
put(bar, FIRMWARE_UNLOAD)
put(foo, FIRMWARE_UNLOAD) /* KLD auto-unloaded, user happy. */
Regards,
Navdeep
More information about the freebsd-hackers
mailing list