* [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default @ 2022-04-08 12:10 Zhu Lingshan 2022-04-13 8:14 ` Jason Wang 0 siblings, 1 reply; 6+ messages in thread From: Zhu Lingshan @ 2022-04-08 12:10 UTC (permalink / raw) To: jasowang, mst; +Cc: virtualization, netdev, Zhu Lingshan, Dan Carpenter This commit assign struct ifcvf_hw.nr_vring to the MSIX vector of the config interrupt by default in ifcvf_request_config_irq(). ifcvf_hw.nr_vring is the most likely and the ideal case for the device config interrupt handling, means every virtqueue has an individual MSIX vector(0 ~ nr_vring - 1), and the config interrupt has its own MSIX vector(number nr_vring). This change can also make GCC W = 2 happy, silence the "uninitialized" warning. Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c index 4366320fb68d..b500fb941dab 100644 --- a/drivers/vdpa/ifcvf/ifcvf_main.c +++ b/drivers/vdpa/ifcvf/ifcvf_main.c @@ -290,13 +290,13 @@ static int ifcvf_request_config_irq(struct ifcvf_adapter *adapter) struct ifcvf_hw *vf = &adapter->vf; int config_vector, ret; + /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt */ + config_vector = vf->nr_vring; + + /* re-use the vqs vector */ if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) return 0; - if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG) - /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt */ - config_vector = vf->nr_vring; - if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) /* vector 0 for vqs and 1 for config interrupt */ config_vector = 1; -- 2.31.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default 2022-04-08 12:10 [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default Zhu Lingshan @ 2022-04-13 8:14 ` Jason Wang 0 siblings, 0 replies; 6+ messages in thread From: Jason Wang @ 2022-04-13 8:14 UTC (permalink / raw) To: Zhu Lingshan, mst; +Cc: virtualization, netdev, Dan Carpenter 在 2022/4/8 下午8:10, Zhu Lingshan 写道: > This commit assign struct ifcvf_hw.nr_vring to the MSIX vector of the > config interrupt by default in ifcvf_request_config_irq(). > ifcvf_hw.nr_vring is the most likely and the ideal case for > the device config interrupt handling, means every virtqueue has > an individual MSIX vector(0 ~ nr_vring - 1), and the config interrupt has > its own MSIX vector(number nr_vring). > > This change can also make GCC W = 2 happy, silence the > "uninitialized" warning. > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 4366320fb68d..b500fb941dab 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -290,13 +290,13 @@ static int ifcvf_request_config_irq(struct ifcvf_adapter *adapter) > struct ifcvf_hw *vf = &adapter->vf; > int config_vector, ret; > > + /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt */ The comment is right before this patch, but probably wrong for MSIX_VECTOR_DEV_SHARED. > + config_vector = vf->nr_vring; > + > + /* re-use the vqs vector */ > if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > return 0; > > - if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG) > - /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt */ > - config_vector = vf->nr_vring; > - > if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) > /* vector 0 for vqs and 1 for config interrupt */ > config_vector = 1; Actually, I prefer to use if ... else ... here. Thanks ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default @ 2022-04-13 8:14 ` Jason Wang 0 siblings, 0 replies; 6+ messages in thread From: Jason Wang @ 2022-04-13 8:14 UTC (permalink / raw) To: Zhu Lingshan, mst; +Cc: netdev, Dan Carpenter, virtualization 在 2022/4/8 下午8:10, Zhu Lingshan 写道: > This commit assign struct ifcvf_hw.nr_vring to the MSIX vector of the > config interrupt by default in ifcvf_request_config_irq(). > ifcvf_hw.nr_vring is the most likely and the ideal case for > the device config interrupt handling, means every virtqueue has > an individual MSIX vector(0 ~ nr_vring - 1), and the config interrupt has > its own MSIX vector(number nr_vring). > > This change can also make GCC W = 2 happy, silence the > "uninitialized" warning. > > Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c > index 4366320fb68d..b500fb941dab 100644 > --- a/drivers/vdpa/ifcvf/ifcvf_main.c > +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > @@ -290,13 +290,13 @@ static int ifcvf_request_config_irq(struct ifcvf_adapter *adapter) > struct ifcvf_hw *vf = &adapter->vf; > int config_vector, ret; > > + /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt */ The comment is right before this patch, but probably wrong for MSIX_VECTOR_DEV_SHARED. > + config_vector = vf->nr_vring; > + > + /* re-use the vqs vector */ > if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > return 0; > > - if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG) > - /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt */ > - config_vector = vf->nr_vring; > - > if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) > /* vector 0 for vqs and 1 for config interrupt */ > config_vector = 1; Actually, I prefer to use if ... else ... here. Thanks _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default 2022-04-13 8:14 ` Jason Wang (?) @ 2022-04-13 9:16 ` Zhu, Lingshan 2022-04-15 5:51 ` Jason Wang -1 siblings, 1 reply; 6+ messages in thread From: Zhu, Lingshan @ 2022-04-13 9:16 UTC (permalink / raw) To: Jason Wang, mst; +Cc: virtualization, netdev, Dan Carpenter On 4/13/2022 4:14 PM, Jason Wang wrote: > > 在 2022/4/8 下午8:10, Zhu Lingshan 写道: >> This commit assign struct ifcvf_hw.nr_vring to the MSIX vector of the >> config interrupt by default in ifcvf_request_config_irq(). >> ifcvf_hw.nr_vring is the most likely and the ideal case for >> the device config interrupt handling, means every virtqueue has >> an individual MSIX vector(0 ~ nr_vring - 1), and the config interrupt >> has >> its own MSIX vector(number nr_vring). >> >> This change can also make GCC W = 2 happy, silence the >> "uninitialized" warning. >> >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> >> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> >> --- >> drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c >> b/drivers/vdpa/ifcvf/ifcvf_main.c >> index 4366320fb68d..b500fb941dab 100644 >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c >> @@ -290,13 +290,13 @@ static int ifcvf_request_config_irq(struct >> ifcvf_adapter *adapter) >> struct ifcvf_hw *vf = &adapter->vf; >> int config_vector, ret; >> + /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector >> for config interrupt */ > > > The comment is right before this patch, but probably wrong for > MSIX_VECTOR_DEV_SHARED. This comment is for the case when every vq and config interrupt has its own vector, how about a better comment "The ideal the default case, vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt" > > >> + config_vector = vf->nr_vring; >> + >> + /* re-use the vqs vector */ >> if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) >> return 0; >> - if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG) >> - /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector >> for config interrupt */ >> - config_vector = vf->nr_vring; >> - >> if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) >> /* vector 0 for vqs and 1 for config interrupt */ >> config_vector = 1; > > > Actually, I prefer to use if ... else ... here. IMHO, if else may lead to mistakes. The code: /* The ideal the default case, vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt */ config_vector = vf->nr_vring; /* re-use the vqs vector */ if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) return 0; if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) /* vector 0 for vqs and 1 for config interrupt */ config_vector = 1; here by default config_vector = vf->nr_vring; If msix_vector_status == MSIX_VECTOR_DEV_SHARED, it will reuse the dev shared vector, means using the vector(value 0) for data-vqs. If msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG, it will use vector=1(vector 0 for data-vqs). If we use if...else, it will be: /* re-use the vqs vector */ if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) return 0; else config_vector = 1; This looks like config_vector can only be 0(re-used vector for the data-vqs, which is 0) or 1. It shadows the ideal and default case config_vector = vf->nr_vring Thanks, Zhu Lingshan > > Thanks > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default 2022-04-13 9:16 ` Zhu, Lingshan @ 2022-04-15 5:51 ` Jason Wang 0 siblings, 0 replies; 6+ messages in thread From: Jason Wang @ 2022-04-15 5:51 UTC (permalink / raw) To: Zhu, Lingshan; +Cc: mst, virtualization, netdev, Dan Carpenter On Wed, Apr 13, 2022 at 5:16 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote: > > > > On 4/13/2022 4:14 PM, Jason Wang wrote: > > > > 在 2022/4/8 下午8:10, Zhu Lingshan 写道: > >> This commit assign struct ifcvf_hw.nr_vring to the MSIX vector of the > >> config interrupt by default in ifcvf_request_config_irq(). > >> ifcvf_hw.nr_vring is the most likely and the ideal case for > >> the device config interrupt handling, means every virtqueue has > >> an individual MSIX vector(0 ~ nr_vring - 1), and the config interrupt > >> has > >> its own MSIX vector(number nr_vring). > >> > >> This change can also make GCC W = 2 happy, silence the > >> "uninitialized" warning. > >> > >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > >> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > >> --- > >> drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c > >> b/drivers/vdpa/ifcvf/ifcvf_main.c > >> index 4366320fb68d..b500fb941dab 100644 > >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c > >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > >> @@ -290,13 +290,13 @@ static int ifcvf_request_config_irq(struct > >> ifcvf_adapter *adapter) > >> struct ifcvf_hw *vf = &adapter->vf; > >> int config_vector, ret; > >> + /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector > >> for config interrupt */ > > > > > > The comment is right before this patch, but probably wrong for > > MSIX_VECTOR_DEV_SHARED. > This comment is for the case when every vq and config interrupt has its > own vector, how > about a better comment "The ideal the default case, vector 0 ~ > vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt" Actually, I suggest to remove the comment since the code can explain itself. > > > > > >> + config_vector = vf->nr_vring; > >> + > >> + /* re-use the vqs vector */ > >> if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > >> return 0; > >> - if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG) > >> - /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector > >> for config interrupt */ > >> - config_vector = vf->nr_vring; > >> - > >> if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) > >> /* vector 0 for vqs and 1 for config interrupt */ > >> config_vector = 1; > > > > > > Actually, I prefer to use if ... else ... here. > IMHO, if else may lead to mistakes. > > The code: > /* The ideal the default case, vector 0 ~ vf->nr_vring for vqs, > num vf->nr_vring vector for config interrupt */ > config_vector = vf->nr_vring; > > /* re-use the vqs vector */ > if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > return 0; > > if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) > /* vector 0 for vqs and 1 for config interrupt */ > config_vector = 1; > > > here by default config_vector = vf->nr_vring; > If msix_vector_status == MSIX_VECTOR_DEV_SHARED, it will reuse the dev > shared vector, means using the vector(value 0) for data-vqs. > If msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG, it will use > vector=1(vector 0 for data-vqs). > > If we use if...else, it will be: > > /* re-use the vqs vector */ > if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > return 0; > else > config_vector = 1; > > This looks like config_vector can only be 0(re-used vector for the > data-vqs, which is 0) or 1. It shadows the ideal and default case > config_vector = vf->nr_vring I meant something like if (DEV_SHARED) return 0; else if (SHARED_VQ_AND_CONFIG) config_vector = 1 else if (PER_VQ_AND_CONFIG) config_vector = vf->nr_vring else return -EINVAL; or using a switch here. (We get the warning because there's no way for the checker to know that msix_vector_status must be DEV_SHARED, SHARED_VQ_AND_CONFIG and PER_VQ_AND_CONFIG). Thanks > > Thanks, > Zhu Lingshan > > > > > Thanks > > > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default @ 2022-04-15 5:51 ` Jason Wang 0 siblings, 0 replies; 6+ messages in thread From: Jason Wang @ 2022-04-15 5:51 UTC (permalink / raw) To: Zhu, Lingshan; +Cc: netdev, virtualization, Dan Carpenter, mst On Wed, Apr 13, 2022 at 5:16 PM Zhu, Lingshan <lingshan.zhu@intel.com> wrote: > > > > On 4/13/2022 4:14 PM, Jason Wang wrote: > > > > 在 2022/4/8 下午8:10, Zhu Lingshan 写道: > >> This commit assign struct ifcvf_hw.nr_vring to the MSIX vector of the > >> config interrupt by default in ifcvf_request_config_irq(). > >> ifcvf_hw.nr_vring is the most likely and the ideal case for > >> the device config interrupt handling, means every virtqueue has > >> an individual MSIX vector(0 ~ nr_vring - 1), and the config interrupt > >> has > >> its own MSIX vector(number nr_vring). > >> > >> This change can also make GCC W = 2 happy, silence the > >> "uninitialized" warning. > >> > >> Signed-off-by: Zhu Lingshan <lingshan.zhu@intel.com> > >> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> > >> --- > >> drivers/vdpa/ifcvf/ifcvf_main.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c > >> b/drivers/vdpa/ifcvf/ifcvf_main.c > >> index 4366320fb68d..b500fb941dab 100644 > >> --- a/drivers/vdpa/ifcvf/ifcvf_main.c > >> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c > >> @@ -290,13 +290,13 @@ static int ifcvf_request_config_irq(struct > >> ifcvf_adapter *adapter) > >> struct ifcvf_hw *vf = &adapter->vf; > >> int config_vector, ret; > >> + /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector > >> for config interrupt */ > > > > > > The comment is right before this patch, but probably wrong for > > MSIX_VECTOR_DEV_SHARED. > This comment is for the case when every vq and config interrupt has its > own vector, how > about a better comment "The ideal the default case, vector 0 ~ > vf->nr_vring for vqs, num vf->nr_vring vector for config interrupt" Actually, I suggest to remove the comment since the code can explain itself. > > > > > >> + config_vector = vf->nr_vring; > >> + > >> + /* re-use the vqs vector */ > >> if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > >> return 0; > >> - if (vf->msix_vector_status == MSIX_VECTOR_PER_VQ_AND_CONFIG) > >> - /* vector 0 ~ vf->nr_vring for vqs, num vf->nr_vring vector > >> for config interrupt */ > >> - config_vector = vf->nr_vring; > >> - > >> if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) > >> /* vector 0 for vqs and 1 for config interrupt */ > >> config_vector = 1; > > > > > > Actually, I prefer to use if ... else ... here. > IMHO, if else may lead to mistakes. > > The code: > /* The ideal the default case, vector 0 ~ vf->nr_vring for vqs, > num vf->nr_vring vector for config interrupt */ > config_vector = vf->nr_vring; > > /* re-use the vqs vector */ > if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > return 0; > > if (vf->msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG) > /* vector 0 for vqs and 1 for config interrupt */ > config_vector = 1; > > > here by default config_vector = vf->nr_vring; > If msix_vector_status == MSIX_VECTOR_DEV_SHARED, it will reuse the dev > shared vector, means using the vector(value 0) for data-vqs. > If msix_vector_status == MSIX_VECTOR_SHARED_VQ_AND_CONFIG, it will use > vector=1(vector 0 for data-vqs). > > If we use if...else, it will be: > > /* re-use the vqs vector */ > if (vf->msix_vector_status == MSIX_VECTOR_DEV_SHARED) > return 0; > else > config_vector = 1; > > This looks like config_vector can only be 0(re-used vector for the > data-vqs, which is 0) or 1. It shadows the ideal and default case > config_vector = vf->nr_vring I meant something like if (DEV_SHARED) return 0; else if (SHARED_VQ_AND_CONFIG) config_vector = 1 else if (PER_VQ_AND_CONFIG) config_vector = vf->nr_vring else return -EINVAL; or using a switch here. (We get the warning because there's no way for the checker to know that msix_vector_status must be DEV_SHARED, SHARED_VQ_AND_CONFIG and PER_VQ_AND_CONFIG). Thanks > > Thanks, > Zhu Lingshan > > > > > Thanks > > > > > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-15 5:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-08 12:10 [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default Zhu Lingshan 2022-04-13 8:14 ` Jason Wang 2022-04-13 8:14 ` Jason Wang 2022-04-13 9:16 ` Zhu, Lingshan 2022-04-15 5:51 ` Jason Wang 2022-04-15 5:51 ` Jason Wang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.