> -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Wednesday, June 26, 2019 10:01 AM > To: Noa Ezra <n...@mellanox.com> > Cc: Matan Azrad <ma...@mellanox.com>; dev@dpdk.org; sta...@dpdk.org > Subject: Re: [Suspected-Phishing][PATCH] net/vhost: fix redundant queue > state event > > > > On 6/26/19 8:37 AM, Noa Ezra wrote: > > Hi, > > > >> -----Original Message----- > >> From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > >> Sent: Tuesday, June 25, 2019 11:24 AM > >> To: Noa Ezra <n...@mellanox.com> > >> Cc: Matan Azrad <ma...@mellanox.com>; dev@dpdk.org; > sta...@dpdk.org > >> Subject: Re: [Suspected-Phishing][PATCH] net/vhost: fix redundant > >> queue state event > >> > >> > >> > >> On 6/25/19 9:04 AM, Noa Ezra wrote: > >>> Hi, > >>> What do you think about this patch? > >>> > >>> Thanks, > >>> Noa. > >>> > >>>> -----Original Message----- > >>>> From: Noa Ezra [mailto:n...@mellanox.com] > >>>> Sent: Wednesday, June 19, 2019 9:16 AM > >>>> To: maxime.coque...@redhat.com > >>>> Cc: Matan Azrad <ma...@mellanox.com>; dev@dpdk.org; Noa Ezra > >>>> <n...@mellanox.com>; sta...@dpdk.org > >>>> Subject: [Suspected-Phishing][PATCH] net/vhost: fix redundant queue > >>>> state event > >>>> > >>>> In some situations, when a virtual machine is starting, > >>>> vring_state_changed can be called while there was no change in the > >>>> queue state. This fix makes sure that there was really a change in > >>>> the queue state before calling the callback for EVENT_QUEUE_STATE. > >>>> > >>>> Fixes: ee584e9710b9 ("vhost: add driver on top of the library") > >>>> Cc: sta...@dpdk.org > >>>> Reviewed-by: Matan Azrad <ma...@mellanox.com> > >>>> --- > >>>> drivers/net/vhost/rte_eth_vhost.c | 4 ++++ > >>>> 1 file changed, 4 insertions(+) > >>>> > >>>> diff --git a/drivers/net/vhost/rte_eth_vhost.c > >>>> b/drivers/net/vhost/rte_eth_vhost.c > >>>> index cad1e5c..fbe7a37 100644 > >>>> --- a/drivers/net/vhost/rte_eth_vhost.c > >>>> +++ b/drivers/net/vhost/rte_eth_vhost.c > >>>> @@ -855,6 +855,10 @@ struct vhost_xstats_name_off { > >>>> /* won't be NULL */ > >>>> state = vring_states[eth_dev->data->port_id]; > >>>> rte_spinlock_lock(&state->lock); > >>>> + if (state->cur[vring] == enable) { > >>>> + rte_spinlock_unlock(&state->lock); > >>>> + return 0; > >>>> + } > >>>> > >>>> state->cur[vring] = enable; > >>>> state->max_vring = RTE_MAX(vring, state->max_vring); > >> > >> Maybe the application would want to be notified a new queue is > >> available, even if it is disabled? > > > > Can you please look again? As I understand it, "enable" is the "new state" > parameter (can be enable/disable). > > In this fix I make sure that there was really a change in the state before > calling EVENT_QUEUE_STATE (with no change in the state). > > Ok, I get it. > > Maybe we would want to update state->max_vring even if we don't sent the > event?
Ok, so do you think we should move the check (if there was a change) to after we update the max_vring? > Thanks, > Maxime > >> > >>>> -- > >>>> 1.8.3.1 > >>>
