| Message ID | 20230724023735.2751602-1-shaozhengchao@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f080864a9d906678e050f10f0e81add711b86fbc |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: remove redundant NULL check in remove_xps_queue() | expand |
On Mon, Jul 24, 2023 at 10:37:35AM +0800, Zhengchao Shao wrote: > There are currently two paths that call remove_xps_queue(): > 1. __netif_set_xps_queue -> remove_xps_queue > 2. clean_xps_maps -> remove_xps_queue_cpu -> remove_xps_queue > There is no need to check dev_maps in remove_xps_queue() because > dev_maps has been checked on these two paths. > > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> I have verified the reasoning above is correct. I am, however, slightly less sure that this is a good idea. > --- > net/core/dev.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/net/core/dev.c b/net/core/dev.c > index f95e0674570f..76a91b849829 100644 > --- a/net/core/dev.c > +++ b/net/core/dev.c > @@ -2384,8 +2384,7 @@ static bool remove_xps_queue(struct xps_dev_maps *dev_maps, > struct xps_map *map = NULL; > int pos; > > - if (dev_maps) > - map = xmap_dereference(dev_maps->attr_map[tci]); > + map = xmap_dereference(dev_maps->attr_map[tci]); > if (!map) > return false; > > -- > 2.34.1 >
On 2023/7/25 15:04, Simon Horman wrote: > On Mon, Jul 24, 2023 at 10:37:35AM +0800, Zhengchao Shao wrote: >> There are currently two paths that call remove_xps_queue(): >> 1. __netif_set_xps_queue -> remove_xps_queue >> 2. clean_xps_maps -> remove_xps_queue_cpu -> remove_xps_queue >> There is no need to check dev_maps in remove_xps_queue() because >> dev_maps has been checked on these two paths. >> >> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > > I have verified the reasoning above is correct. > I am, however, slightly less sure that this is a good idea. > Since commit 044ab86d431b("net: move the xps maps to an array") and commit 867d0ad476db("net: fix XPS static_key accounting are incorporated"), it is meaningless to reserve the null judgment. Perhaps after removing this null check, add comment, like / * the caller must make sure that dev_maps != NULL */ ? Thank you. Zhengchao Shao >> --- >> net/core/dev.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/net/core/dev.c b/net/core/dev.c >> index f95e0674570f..76a91b849829 100644 >> --- a/net/core/dev.c >> +++ b/net/core/dev.c >> @@ -2384,8 +2384,7 @@ static bool remove_xps_queue(struct xps_dev_maps *dev_maps, >> struct xps_map *map = NULL; >> int pos; >> >> - if (dev_maps) >> - map = xmap_dereference(dev_maps->attr_map[tci]); >> + map = xmap_dereference(dev_maps->attr_map[tci]); >> if (!map) >> return false; >> >> -- >> 2.34.1 >>
Hello: This patch was applied to netdev/net-next.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 24 Jul 2023 10:37:35 +0800 you wrote: > There are currently two paths that call remove_xps_queue(): > 1. __netif_set_xps_queue -> remove_xps_queue > 2. clean_xps_maps -> remove_xps_queue_cpu -> remove_xps_queue > There is no need to check dev_maps in remove_xps_queue() because > dev_maps has been checked on these two paths. > > Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> > > [...] Here is the summary with links: - [net-next] net: remove redundant NULL check in remove_xps_queue() https://git.kernel.org/netdev/net-next/c/f080864a9d90 You are awesome, thank you!
diff --git a/net/core/dev.c b/net/core/dev.c index f95e0674570f..76a91b849829 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -2384,8 +2384,7 @@ static bool remove_xps_queue(struct xps_dev_maps *dev_maps, struct xps_map *map = NULL; int pos; - if (dev_maps) - map = xmap_dereference(dev_maps->attr_map[tci]); + map = xmap_dereference(dev_maps->attr_map[tci]); if (!map) return false;
There are currently two paths that call remove_xps_queue(): 1. __netif_set_xps_queue -> remove_xps_queue 2. clean_xps_maps -> remove_xps_queue_cpu -> remove_xps_queue There is no need to check dev_maps in remove_xps_queue() because dev_maps has been checked on these two paths. Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com> --- net/core/dev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)