On Sat, 13 Apr 2024, Martin Pieuchot wrote:

> > The diff below reverts the two above commits and changes the vio_ctrl_* 
> > functions to prevent any concurrent control queue requests while the 
> > device is being reset. I have seen no deadlocks or crashes with that 
> > patch.
> > 
> > Comments? OKs?
> 
> Why vio_wait_ctrl() still sleep?  If you're no longer accepting
> concurrent control requests shouldn't you be assured the previous
> request has finished when calling vio_ctrl_rx() and vio_set_rx_filter()?
> 
> Shouldn't you replace this function by a simple check?  The only sleep
> left should be in vio_wait_ctrl_done(), no?

So far, I only block control requests when the device is reset, but there 
are some cases where the device does not need a reset. However because of 
mvs@'s comments, I will introduce a lock at the start of the ioctl 
function so that there are no concurrent configuration changes at all. 
Then I can unlock/relock the netlock as needed. I will send a new diff 
in a bit.

> 
> > The whole locking in vio(4) needs some overhaul, but I would postpone that 
> > until vio(4) is unlocked and extended to multi-queue. Is there a good 
> > example of how to be able to sleep in the ioctl path in some other network 
> > driver? cad(4) uses a rwlock and seems to jump through quite a few hoops 
> > to get it right. Is there a better example?
> > 
> > Cheers,
> > Stefan