From: Colin Ian King <colin.king@canonical.com> To: Vitaly Kuznetsov <vkuznets@redhat.com> Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, "K . Y . Srinivasan" <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Stephen Hemminger <sthemmin@microsoft.com>, Wei Liu <wei.liu@kernel.org>, linux-hyperv@vger.kernel.org Subject: Re: [PATCH][next] drivers: hv: remove redundant assignment to pointer primary_channel Date: Tue, 14 Apr 2020 17:58:35 +0100 [thread overview] Message-ID: <606c442c-1923-77d4-c350-e06878172c44@canonical.com> (raw) In-Reply-To: <87d08axb7k.fsf@vitty.brq.redhat.com> On 14/04/2020 17:51, Vitaly Kuznetsov wrote: > Colin King <colin.king@canonical.com> writes: > >> From: Colin Ian King <colin.king@canonical.com> >> >> The pointer primary_channel is being assigned with a value that is never, >> The assignment is redundant and can be removed. >> >> Addresses-Coverity: ("Unused value") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/hv/channel_mgmt.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c >> index ffd7fffa5f83..f7bbb8dc4b0f 100644 >> --- a/drivers/hv/channel_mgmt.c >> +++ b/drivers/hv/channel_mgmt.c >> @@ -425,8 +425,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel) >> >> if (channel->primary_channel == NULL) { >> list_del(&channel->listentry); >> - >> - primary_channel = channel; >> } else { >> primary_channel = channel->primary_channel; >> spin_lock_irqsave(&primary_channel->lock, flags); > > If I'm looking at the right source (5.7-rc1) it *is* beeing used: > > if (channel->primary_channel == NULL) { > list_del(&channel->listentry); > > primary_channel = channel; > } else { > primary_channel = channel->primary_channel; > spin_lock_irqsave(&primary_channel->lock, flags); > list_del(&channel->sc_list); > spin_unlock_irqrestore(&primary_channel->lock, flags); > } > > /* > * We need to free the bit for init_vp_index() to work in the case > * of sub-channel, when we reload drivers like hv_netvsc. > */ > if (channel->affinity_policy == HV_LOCALIZED) > cpumask_clear_cpu(channel->target_cpu, > &primary_channel->alloced_cpus_in_node); > ^^^^^ HERE ^^^^^ > I was basing my change on linux-next that has removed a hunk of code: commit bcefa400900739310e8ef0cb34cbe029c404455c Author: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Date: Mon Apr 6 02:15:11 2020 +0200 Drivers: hv: vmbus: Remove the unused HV_LOCALIZED channel affinity logic The logic is unused since commit 509879bdb30b8 ("Drivers: hv: Introduce a policy for controlling channel affinity"). This logic assumes that a channel target_cpu doesn't change during the lifetime of a channel, but this assumption is incompatible with the new functionality that allows changing the vCPU a channel will interrupt. Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Link: https://lore.kernel.org/r/20200406001514.19876-9-parri.andrea@gmail.com Reviewed-by: Michael Kelley <mikelley@microsoft.com> Signed-off-by: Wei Liu <wei.liu@kernel.org> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 7fb6eb647f14..476592b0bc00 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -433,14 +433,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel) spin_unlock_irqrestore(&primary_channel->lock, flags); } - /* - * We need to free the bit for init_vp_index() to work in the case - * of sub-channel, when we reload drivers like hv_netvsc. - */ - if (channel->affinity_policy == HV_LOCALIZED) - cpumask_clear_cpu(channel->target_cpu, - &primary_channel->alloced_cpus_in_node); -
WARNING: multiple messages have this Message-ID (diff)
From: Colin Ian King <colin.king@canonical.com> To: Vitaly Kuznetsov <vkuznets@redhat.com> Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org, "K . Y . Srinivasan" <kys@microsoft.com>, Haiyang Zhang <haiyangz@microsoft.com>, Stephen Hemminger <sthemmin@microsoft.com>, Wei Liu <wei.liu@kernel.org>, linux-hyperv@vger.kernel.org Subject: Re: [PATCH][next] drivers: hv: remove redundant assignment to pointer primary_channel Date: Tue, 14 Apr 2020 16:58:35 +0000 [thread overview] Message-ID: <606c442c-1923-77d4-c350-e06878172c44@canonical.com> (raw) In-Reply-To: <87d08axb7k.fsf@vitty.brq.redhat.com> On 14/04/2020 17:51, Vitaly Kuznetsov wrote: > Colin King <colin.king@canonical.com> writes: > >> From: Colin Ian King <colin.king@canonical.com> >> >> The pointer primary_channel is being assigned with a value that is never, >> The assignment is redundant and can be removed. >> >> Addresses-Coverity: ("Unused value") >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> drivers/hv/channel_mgmt.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c >> index ffd7fffa5f83..f7bbb8dc4b0f 100644 >> --- a/drivers/hv/channel_mgmt.c >> +++ b/drivers/hv/channel_mgmt.c >> @@ -425,8 +425,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel) >> >> if (channel->primary_channel = NULL) { >> list_del(&channel->listentry); >> - >> - primary_channel = channel; >> } else { >> primary_channel = channel->primary_channel; >> spin_lock_irqsave(&primary_channel->lock, flags); > > If I'm looking at the right source (5.7-rc1) it *is* beeing used: > > if (channel->primary_channel = NULL) { > list_del(&channel->listentry); > > primary_channel = channel; > } else { > primary_channel = channel->primary_channel; > spin_lock_irqsave(&primary_channel->lock, flags); > list_del(&channel->sc_list); > spin_unlock_irqrestore(&primary_channel->lock, flags); > } > > /* > * We need to free the bit for init_vp_index() to work in the case > * of sub-channel, when we reload drivers like hv_netvsc. > */ > if (channel->affinity_policy = HV_LOCALIZED) > cpumask_clear_cpu(channel->target_cpu, > &primary_channel->alloced_cpus_in_node); > ^^^^^ HERE ^^^^^ > I was basing my change on linux-next that has removed a hunk of code: commit bcefa400900739310e8ef0cb34cbe029c404455c Author: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Date: Mon Apr 6 02:15:11 2020 +0200 Drivers: hv: vmbus: Remove the unused HV_LOCALIZED channel affinity logic The logic is unused since commit 509879bdb30b8 ("Drivers: hv: Introduce a policy for controlling channel affinity"). This logic assumes that a channel target_cpu doesn't change during the lifetime of a channel, but this assumption is incompatible with the new functionality that allows changing the vCPU a channel will interrupt. Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com> Link: https://lore.kernel.org/r/20200406001514.19876-9-parri.andrea@gmail.com Reviewed-by: Michael Kelley <mikelley@microsoft.com> Signed-off-by: Wei Liu <wei.liu@kernel.org> diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 7fb6eb647f14..476592b0bc00 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -433,14 +433,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel) spin_unlock_irqrestore(&primary_channel->lock, flags); } - /* - * We need to free the bit for init_vp_index() to work in the case - * of sub-channel, when we reload drivers like hv_netvsc. - */ - if (channel->affinity_policy = HV_LOCALIZED) - cpumask_clear_cpu(channel->target_cpu, - &primary_channel->alloced_cpus_in_node); -
next prev parent reply other threads:[~2020-04-14 16:59 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-14 15:23 [PATCH][next] drivers: hv: remove redundant assignment to pointer primary_channel Colin King 2020-04-14 15:23 ` Colin King 2020-04-14 15:42 ` Julia Lawall 2020-04-14 15:42 ` Julia Lawall 2020-04-14 16:51 ` Vitaly Kuznetsov 2020-04-14 16:51 ` Vitaly Kuznetsov 2020-04-14 16:58 ` Colin Ian King [this message] 2020-04-14 16:58 ` Colin Ian King 2020-04-15 10:43 ` Vitaly Kuznetsov 2020-04-15 10:43 ` Vitaly Kuznetsov 2020-04-15 14:37 ` Wei Liu 2020-04-15 14:37 ` Wei Liu 2020-04-16 10:02 ` Dan Carpenter 2020-04-16 10:02 ` Dan Carpenter 2020-04-16 10:08 ` Colin Ian King 2020-04-16 10:08 ` Colin Ian King 2020-04-14 20:24 ` Wei Liu 2020-04-14 20:24 ` Wei Liu 2020-04-14 21:25 ` Colin Ian King 2020-04-14 21:25 ` Colin Ian King
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=606c442c-1923-77d4-c350-e06878172c44@canonical.com \ --to=colin.king@canonical.com \ --cc=haiyangz@microsoft.com \ --cc=kernel-janitors@vger.kernel.org \ --cc=kys@microsoft.com \ --cc=linux-hyperv@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=sthemmin@microsoft.com \ --cc=vkuznets@redhat.com \ --cc=wei.liu@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.