RE: [PATCH net] hv_netvsc: Fix race condition between netvsc_probe and netvsc_remove

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

 



> From: Souradeep Chakrabarti <schakrabarti@xxxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, January 30, 2024 2:16 AM
> [...]
> In commit ac5047671758 ("hv_netvsc: Disable NAPI before closing the
> VMBus channel"), napi_disable was getting called for all channels,
> including all subchannels without confirming if they are enabled or not.

s/enabled/created/

> Which caused hv_netvsc getting hung at napi_disable, when
> netvsc_probe()
> and netvsc_remove() are happening simultaneously and netvsc_remove()

Technically, they are not happening simultaneously: netvsc_probe() itself has
finished, but the work item scheduled by it has not started yet.

> calls cancel_work_sync(&nvdev->subchan_work) before netvsc_sc_open()
> calls napi_enable for the sub channels. Which causes NAPIF_STATE_SCHED

Technically, nvdev->subchan_work has not started to run yet, i.e.
netvsc_subchan_work() -> rndis_set_subchannel() has not created the
sub-channels yet, so netvsc_sc_open() can't run.

It would be great if you could briefly explain how the NAPIF_STATE_SCHED bit
is set and cleared, e.g. it's pre-set in rndis_filter_device_add() -> netif_napi_add()
so if the sub-channels are not created, netvsc_sc_open() -> napi_enable() won't
clear the flag and the flag remains set for ever for the sub-channels. 

> bit not getting cleared for the subchannels.
> 
> Now during netvsc_device_remove(), when napi_disable is called for those
> subchannels, napi_disable gets stuck on infinite msleep.

The patch body looks good to me. Please post v2 with an updated changelog.

Reviewed-by: Dexuan Cui <decui@xxxxxxxxxxxxx>





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux