lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Apr 2022 17:16:06 +0800 From: "Zhu, Lingshan" <lingshan.zhu@...el.com> To: Jason Wang <jasowang@...hat.com>, mst@...hat.com Cc: virtualization@...ts.linux-foundation.org, netdev@...r.kernel.org, Dan Carpenter <dan.carpenter@...cle.com> Subject: Re: [PATCH] vDPA/ifcvf: assign nr_vring to the MSI vector of config_intr by default 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@...el.com> >> Reviewed-by: Dan Carpenter <dan.carpenter@...cle.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 > >
Powered by blists - more mailing lists