Re: [PATCH] net/9p/trans_virtio.c: decrease the refcount of 9p virtio device when removing it

[piaojun]

Hi Dominique,

On 2018/8/8 16:36, Dominique Martinet wrote:
> piaojun wrote on Wed, Aug 08, 2018:
>> I found that 9pnet_virtio.ko could not be removed by rmmod command, and I
>> could still found it by lsmod. The reason is that we forgot decrease the
>> refcount of 9p virtio device by kobject_put. So we should put refcount in
>> p9_virtio_remove.
> 
> 
> Hmm, I cannot seem to reproduce this. Can you give more details on how
> to get into stuck state?
> I tried mounting something, accessing the sysfs files, etc to no avail.
> 
> lsmod gives a counter to how many references there are to the module,
> you can use that to debug a bit.
> For example here I get this line just after loading the module:
>   9pnet_virtio           32768  0
> 
> Then after mounting something there is one reference:
>   9pnet_virtio           32768  1
> 
> Then unmounting puts that back to 0 and 'modprobe -r' (or rmmod) works.
> 
I try to remove 9pnet_virtio.ko by 'rmmod 9pnet_virtio' as I want to
replace it without rebooting system. Here I have not mount 9pfs yet, so
the refcount is still 0.

Before rmmod:
# lsmod | grep 9p
9pnet_virtio           20480  0
9pnet                 106496  1 9pnet_virtio
virtio_ring            28672  5 virtio_scsi,9pnet_virtio,virtio_pci,virtio_blk,virtio_net
virtio                 16384  5 virtio_scsi,9pnet_virtio,virtio_pci,virtio_blk,virtio_net

After rmmod:
# lsmod | grep 9p
9pnet_virtio           20480  0
9pnet                 106496  1 9pnet_virtio
virtio_ring            28672  5 virtio_scsi,9pnet_virtio,virtio_pci,virtio_blk,virtio_net
virtio                 16384  5 virtio_scsi,9pnet_virtio,virtio_pci,virtio_blk,virtio_net

Normally 9pnet_virtio should be invisible after rmmod like this:
# lsmod | grep 9p
9pnet                 106496  0

> 
> 
> 
> I dislike saying the next part but I think form also is important,
> please bear with me:
> 
>  - shorter subject line, please. For example, you can lose 20 characters
> by reodering words so there is no need for pronouns
> "net/9p/virtio: decrease 9p virtio device refcount on removal"
> 
>  - I personally dislike commit messages that are "novelized" (that is,
> put yourself as an actor and describe what you were doing)
> That seems to be somewhat accepted as looking at the kernel's git log I
> see some (few) commits using "I ..." that are not pull request messages
> but if possible please avoid this style and try to describe facts, how
> things are wrong, what got fixed and if required how.
> To give an example again, this says the same thing:
> "The 9pnet_virtio module could not be unloaded because we forgot to
> decrease the refcount of the 9p virtio device with kobject_put.
> 
> Put the reference in 9p_virtio_remove"
> 
Your suggestion really makes sense, and I will make some improvment later.

Thanks,
Jun
> 
> Thanks,
>