* [PATCH] crypto: qat - remove redundant condition accel_dev->is_vf
@ 2019-10-25 7:23 Yunfeng Ye
2019-10-25 9:33 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Yunfeng Ye @ 2019-10-25 7:23 UTC (permalink / raw)
To: giovanni.cabiddu, Herbert Xu, davem, qat-linux
Cc: linux-crypto, linux-kernel, hushiyuan@huawei.com,
linfeilong@huawei.com
Warning is found by the code analysis tool:
"Redundant condition: accel_dev->is_vf"
So remove the redundant condition accel_dev->is_vf.
Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
---
drivers/crypto/qat/qat_common/adf_dev_mgr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/qat/qat_common/adf_dev_mgr.c b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
index 2d06409bd3c4..b54b8850fe20 100644
--- a/drivers/crypto/qat/qat_common/adf_dev_mgr.c
+++ b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
@@ -196,7 +196,7 @@ int adf_devmgr_add_dev(struct adf_accel_dev *accel_dev,
atomic_set(&accel_dev->ref_count, 0);
/* PF on host or VF on guest */
- if (!accel_dev->is_vf || (accel_dev->is_vf && !pf)) {
+ if (!accel_dev->is_vf || !pf) {
struct vf_id_map *map;
list_for_each(itr, &accel_table) {
@@ -292,7 +292,7 @@ void adf_devmgr_rm_dev(struct adf_accel_dev *accel_dev,
struct adf_accel_dev *pf)
{
mutex_lock(&table_lock);
- if (!accel_dev->is_vf || (accel_dev->is_vf && !pf)) {
+ if (!accel_dev->is_vf || !pf) {
id_map[accel_dev->accel_id] = 0;
num_devices--;
} else if (accel_dev->is_vf && pf) {
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] crypto: qat - remove redundant condition accel_dev->is_vf
2019-10-25 7:23 [PATCH] crypto: qat - remove redundant condition accel_dev->is_vf Yunfeng Ye
@ 2019-10-25 9:33 ` Ard Biesheuvel
2019-10-25 9:45 ` Yunfeng Ye
[not found] ` <77832b26242f4987877d6122ba14a0d0@irsmsx101.ger.corp.intel.com>
0 siblings, 2 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2019-10-25 9:33 UTC (permalink / raw)
To: Yunfeng Ye
Cc: Giovanni Cabiddu, Herbert Xu, David S. Miller, qat-linux,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, hushiyuan@huawei.com,
linfeilong@huawei.com
On Fri, 25 Oct 2019 at 09:24, Yunfeng Ye <yeyunfeng@huawei.com> wrote:
>
> Warning is found by the code analysis tool:
> "Redundant condition: accel_dev->is_vf"
>
> So remove the redundant condition accel_dev->is_vf.
>
> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
> ---
> drivers/crypto/qat/qat_common/adf_dev_mgr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/qat/qat_common/adf_dev_mgr.c b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
> index 2d06409bd3c4..b54b8850fe20 100644
> --- a/drivers/crypto/qat/qat_common/adf_dev_mgr.c
> +++ b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
> @@ -196,7 +196,7 @@ int adf_devmgr_add_dev(struct adf_accel_dev *accel_dev,
> atomic_set(&accel_dev->ref_count, 0);
>
> /* PF on host or VF on guest */
> - if (!accel_dev->is_vf || (accel_dev->is_vf && !pf)) {
> + if (!accel_dev->is_vf || !pf) {
I disagree with this change. There is no bug here, and the way the
condition is formulated self-documents the code, i.e.,
IF NOT is_vf
OR (is_vf BUT NOT pf)
Using an automated tool to reduce every boolean expression to its
minimal representation doesn't seem that useful to me, since the
compiler is perfectly capable of doing that when generating the object
code.
> struct vf_id_map *map;
>
> list_for_each(itr, &accel_table) {
> @@ -292,7 +292,7 @@ void adf_devmgr_rm_dev(struct adf_accel_dev *accel_dev,
> struct adf_accel_dev *pf)
> {
> mutex_lock(&table_lock);
> - if (!accel_dev->is_vf || (accel_dev->is_vf && !pf)) {
> + if (!accel_dev->is_vf || !pf) {
> id_map[accel_dev->accel_id] = 0;
> num_devices--;
> } else if (accel_dev->is_vf && pf) {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] crypto: qat - remove redundant condition accel_dev->is_vf
2019-10-25 9:33 ` Ard Biesheuvel
@ 2019-10-25 9:45 ` Yunfeng Ye
[not found] ` <77832b26242f4987877d6122ba14a0d0@irsmsx101.ger.corp.intel.com>
1 sibling, 0 replies; 4+ messages in thread
From: Yunfeng Ye @ 2019-10-25 9:45 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Giovanni Cabiddu, Herbert Xu, David S. Miller, qat-linux,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, hushiyuan@huawei.com,
linfeilong@huawei.com
On 2019/10/25 17:33, Ard Biesheuvel wrote:
> On Fri, 25 Oct 2019 at 09:24, Yunfeng Ye <yeyunfeng@huawei.com> wrote:
>>
>> Warning is found by the code analysis tool:
>> "Redundant condition: accel_dev->is_vf"
>>
>> So remove the redundant condition accel_dev->is_vf.
>>
>> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
>> ---
>> drivers/crypto/qat/qat_common/adf_dev_mgr.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/crypto/qat/qat_common/adf_dev_mgr.c b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
>> index 2d06409bd3c4..b54b8850fe20 100644
>> --- a/drivers/crypto/qat/qat_common/adf_dev_mgr.c
>> +++ b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
>> @@ -196,7 +196,7 @@ int adf_devmgr_add_dev(struct adf_accel_dev *accel_dev,
>> atomic_set(&accel_dev->ref_count, 0);
>>
>> /* PF on host or VF on guest */
>> - if (!accel_dev->is_vf || (accel_dev->is_vf && !pf)) {
>> + if (!accel_dev->is_vf || !pf) {
>
> I disagree with this change. There is no bug here, and the way the
> condition is formulated self-documents the code, i.e.,
>
> IF NOT is_vf
> OR (is_vf BUT NOT pf)
>
> Using an automated tool to reduce every boolean expression to its
> minimal representation doesn't seem that useful to me, since the
> compiler is perfectly capable of doing that when generating the object
> code.
>
ok, thanks, this modify just fix warning, and make code simple.
>
>
>
>> struct vf_id_map *map;
>>
>> list_for_each(itr, &accel_table) {
>> @@ -292,7 +292,7 @@ void adf_devmgr_rm_dev(struct adf_accel_dev *accel_dev,
>> struct adf_accel_dev *pf)
>> {
>> mutex_lock(&table_lock);
>> - if (!accel_dev->is_vf || (accel_dev->is_vf && !pf)) {
>> + if (!accel_dev->is_vf || !pf) {
>> id_map[accel_dev->accel_id] = 0;
>> num_devices--;
>> } else if (accel_dev->is_vf && pf) {
>> --
>> 2.7.4
>>
>
> .
>
^ permalink raw reply [flat|nested] 4+ messages in thread[parent not found: <77832b26242f4987877d6122ba14a0d0@irsmsx101.ger.corp.intel.com>]
* Re: [PATCH] crypto: qat - remove redundant condition accel_dev->is_vf
[not found] ` <77832b26242f4987877d6122ba14a0d0@irsmsx101.ger.corp.intel.com>
@ 2019-10-25 9:50 ` Giovanni Cabiddu
0 siblings, 0 replies; 4+ messages in thread
From: Giovanni Cabiddu @ 2019-10-25 9:50 UTC (permalink / raw)
To: Yunfeng Ye
Cc: Ard Biesheuvel, Herbert Xu, David S. Miller, qat-linux,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
Linux Kernel Mailing List, hushiyuan@huawei.com,
linfeilong@huawei.com
On Fri, Oct 25, 2019 at 10:45:31AM +0100, Yunfeng Ye wrote:
> On 2019/10/25 17:33, Ard Biesheuvel wrote:
> > On Fri, 25 Oct 2019 at 09:24, Yunfeng Ye <yeyunfeng@huawei.com> wrote:
> >>
> >> Warning is found by the code analysis tool:
> >> "Redundant condition: accel_dev->is_vf"
> >>
> >> So remove the redundant condition accel_dev->is_vf.
> >>
> >> Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com>
> >> ---
> >> drivers/crypto/qat/qat_common/adf_dev_mgr.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/crypto/qat/qat_common/adf_dev_mgr.c b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
> >> index 2d06409bd3c4..b54b8850fe20 100644
> >> --- a/drivers/crypto/qat/qat_common/adf_dev_mgr.c
> >> +++ b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
> >> @@ -196,7 +196,7 @@ int adf_devmgr_add_dev(struct adf_accel_dev *accel_dev,
> >> atomic_set(&accel_dev->ref_count, 0);
> >>
> >> /* PF on host or VF on guest */
> >> - if (!accel_dev->is_vf || (accel_dev->is_vf && !pf)) {
> >> + if (!accel_dev->is_vf || !pf) {
> >
> > I disagree with this change. There is no bug here, and the way the
> > condition is formulated self-documents the code, i.e.,
> >
> > IF NOT is_vf
> > OR (is_vf BUT NOT pf)
> >
> > Using an automated tool to reduce every boolean expression to its
> > minimal representation doesn't seem that useful to me, since the
> > compiler is perfectly capable of doing that when generating the object
> > code.
> >
> ok, thanks, this modify just fix warning, and make code simple.
This change simplifies the code but it makes it less readable.
I'd prefer to leave it as it was.
Regards,
--
Giovanni
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-25 9:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-25 7:23 [PATCH] crypto: qat - remove redundant condition accel_dev->is_vf Yunfeng Ye
2019-10-25 9:33 ` Ard Biesheuvel
2019-10-25 9:45 ` Yunfeng Ye
[not found] ` <77832b26242f4987877d6122ba14a0d0@irsmsx101.ger.corp.intel.com>
2019-10-25 9:50 ` Giovanni Cabiddu
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.