| Message ID | 1705410693-118895-2-git-send-email-hengqi@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | virtio-net: a fix and some updates for virtio dim | expand |
On Tue, Jan 16, 2024 at 09:11:31PM +0800, Heng Qi wrote: > When the dim worker is scheduled, if it fails to acquire the lock, > dim may not be able to return to the working state later. > > For example, the following single queue scenario: > 1. The dim worker of rxq0 is scheduled, and the dim status is > changed to DIM_APPLY_NEW_PROFILE; > 2. The ethtool command is holding rtnl lock; > 3. Since the rtnl lock is already held, virtnet_rx_dim_work fails > to acquire the lock and exits; > > Then, even if net_dim is invoked again, it cannot work because the > state is not restored to DIM_START_MEASURE. > > Fixes: 6208799553a8 ("virtio-net: support rx netdim") > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com> > --- > Belongs to the net branch. > > drivers/net/virtio_net.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index d7ce4a1..f6ac3e7 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -3524,8 +3524,10 @@ static void virtnet_rx_dim_work(struct work_struct *work) > struct dim_cq_moder update_moder; > int i, qnum, err; > > - if (!rtnl_trylock()) > + if (!rtnl_trylock()) { > + schedule_work(&dim->work); > return; > + } > > /* Each rxq's work is queued by "net_dim()->schedule_work()" > * in response to NAPI traffic changes. Note that dim->profile_ix OK but this means that in cleanup it is not sufficient to flush dim work - it can requeue itself. > -- > 1.8.3.1
在 2024/1/16 下午9:15, Michael S. Tsirkin 写道: > On Tue, Jan 16, 2024 at 09:11:31PM +0800, Heng Qi wrote: >> When the dim worker is scheduled, if it fails to acquire the lock, >> dim may not be able to return to the working state later. >> >> For example, the following single queue scenario: >> 1. The dim worker of rxq0 is scheduled, and the dim status is >> changed to DIM_APPLY_NEW_PROFILE; >> 2. The ethtool command is holding rtnl lock; >> 3. Since the rtnl lock is already held, virtnet_rx_dim_work fails >> to acquire the lock and exits; >> >> Then, even if net_dim is invoked again, it cannot work because the >> state is not restored to DIM_START_MEASURE. >> >> Fixes: 6208799553a8 ("virtio-net: support rx netdim") >> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com> >> --- >> Belongs to the net branch. >> >> drivers/net/virtio_net.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c >> index d7ce4a1..f6ac3e7 100644 >> --- a/drivers/net/virtio_net.c >> +++ b/drivers/net/virtio_net.c >> @@ -3524,8 +3524,10 @@ static void virtnet_rx_dim_work(struct work_struct *work) >> struct dim_cq_moder update_moder; >> int i, qnum, err; >> >> - if (!rtnl_trylock()) >> + if (!rtnl_trylock()) { >> + schedule_work(&dim->work); >> return; >> + } >> >> /* Each rxq's work is queued by "net_dim()->schedule_work()" >> * in response to NAPI traffic changes. Note that dim->profile_ix > OK but this means that in cleanup it is not sufficient to flush > dim work - it can requeue itself. We did not use the flush work operation, cancel_work_sync will handle the re-queue situation: "Cancel @work and wait for its execution to finish. This function can be used even if the work re-queues itself or migrates to another workqueue. On return from this function, @work is guaranteed to be not pending or executing on any CPU." Thanks. > > >> -- >> 1.8.3.1
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index d7ce4a1..f6ac3e7 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -3524,8 +3524,10 @@ static void virtnet_rx_dim_work(struct work_struct *work) struct dim_cq_moder update_moder; int i, qnum, err; - if (!rtnl_trylock()) + if (!rtnl_trylock()) { + schedule_work(&dim->work); return; + } /* Each rxq's work is queued by "net_dim()->schedule_work()" * in response to NAPI traffic changes. Note that dim->profile_ix
When the dim worker is scheduled, if it fails to acquire the lock, dim may not be able to return to the working state later. For example, the following single queue scenario: 1. The dim worker of rxq0 is scheduled, and the dim status is changed to DIM_APPLY_NEW_PROFILE; 2. The ethtool command is holding rtnl lock; 3. Since the rtnl lock is already held, virtnet_rx_dim_work fails to acquire the lock and exits; Then, even if net_dim is invoked again, it cannot work because the state is not restored to DIM_START_MEASURE. Fixes: 6208799553a8 ("virtio-net: support rx netdim") Signed-off-by: Heng Qi <hengqi@linux.alibaba.com> --- Belongs to the net branch. drivers/net/virtio_net.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)