[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

Roman Bogorodskiy  changed:

   What|Removed |Added

 Status|New |In Progress

--- Comment #17 from Roman Bogorodskiy  ---
Kristof, thanks for reviewing and committing this!

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

--- Comment #16 from commit-h...@freebsd.org ---
A commit references this bug:

Author: kp
Date: Sat May 14 06:07:15 UTC 2016
New revision: 299725
URL: https://svnweb.freebsd.org/changeset/base/299725

Log:
  vtnet: fix panic on unload

  Since r276367 added the virtio_mmio support vtnet_modevent() gets called
twice.
  This resulted in a memory leak during load and a panic on unload.

  Count the loads so we only initialise once (just like cxgbe(4)), and only
clean
  up in the final unload.

  PR:   209428
  Submitted by: no...@freebsd.org
  MFC after:1 week

Changes:
  head/sys/dev/virtio/network/if_vtnet.c

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

Roman Bogorodskiy  changed:

   What|Removed |Added

 Attachment #170229|0   |1
is obsolete||

--- Comment #14 from Roman Bogorodskiy  ---
Created attachment 170230
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=170230=edit
patch v5

Good point, I've moved out EBUSY check out of the --loader == 0 condition, as
all the MOD_QUIESCE calls will be _before_ the last MOD_UNLOAD where we clean
things up.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

--- Comment #11 from Kristof Provost  ---
I *think* cxgbe is wrong (or overly cautious). I'd propose doing it without the
mutex in vtnet.

You're not going to make things any worse than the currently are by not adding
it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

--- Comment #13 from Kristof Provost  ---
Hmm, can you verify that uma_zdestroy() still gets called?
I think the order is MOD_LOAD on load, but an unload first gets MOD_QUIESCE and
then MOD_UNLOAD.
That'd mean we do loaded++ twice, but --loaded four times.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

--- Comment #10 from Roman Bogorodskiy  ---
(In reply to Kristof Provost from comment #9)

Ah, I think that's right:

https://svnweb.freebsd.org/base/head/sys/kern/kern_module.c?revision=289111=markup#l260

It's not obvious why cxgbe uses it then.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

--- Comment #9 from Kristof Provost  ---
That looks correct to me, although I'm not sure if the mutex is actually
required.
The code calling the module load/unload function seems to do so with Giant
held.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

Roman Bogorodskiy  changed:

   What|Removed |Added

 Attachment #170217|0   |1
is obsolete||

--- Comment #8 from Roman Bogorodskiy  ---
Created attachment 170227
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=170227=edit
patch v3

Updated the patch to use an approach similar to the one used in cxgbe. Basic
testing didn't reveal any issues.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

Roman Bogorodskiy  changed:

   What|Removed |Added

 Attachment #170182|0   |1
is obsolete||

--- Comment #4 from Roman Bogorodskiy  ---
Created attachment 170217
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=170217=edit
updated patch

Yeah, you're right, vtnet_modevent() is called twice, so we don't need to
duplicate uma_zcreate() calls indeed.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"

[Bug 209428] if_vtnet(4) panics when doing kldunload if_vtnet

[bugzilla-noreply]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=209428

Kristof Provost  changed:

   What|Removed |Added

 CC||and...@freebsd.org,
   ||k...@freebsd.org

--- Comment #2 from Kristof Provost  ---
That fix looked odd to me, because I did not expect vtnet_modevent() to be
called twice with MOD_UNLOAD.
That is the cause though, because there are two DRIVER_MODULE()s for vtnet.

I think this problem was introduced with r276367. Cc-ing Andrew so he can take
a look too.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
freebsd-virtualization@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-virtualization
To unsubscribe, send any mail to 
"freebsd-virtualization-unsubscr...@freebsd.org"