From: Zijun Hu <zijun.hu@oss.qualcomm.com>

Variable @match_size is fixed in auxiliary_match_id().

Optimize the function by moving the logic calculating the variable
out of the for loop.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/base/auxiliary.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
index 12ffdd8437567f282374bbf3775d9de7ca0dc4c7..9a53ada043045031e565ade57fd7ba781e7d20ea 100644
--- a/drivers/base/auxiliary.c
+++ b/drivers/base/auxiliary.c
@@ -171,14 +171,14 @@
 static const struct auxiliary_device_id *auxiliary_match_id(const struct auxiliary_device_id *id,
 							    const struct auxiliary_device *auxdev)
 {
-	for (; id->name[0]; id++) {
-		const char *p = strrchr(dev_name(&auxdev->dev), '.');
-		int match_size;
+	const char *p = strrchr(dev_name(&auxdev->dev), '.');
+	int match_size;
 
-		if (!p)
-			continue;
-		match_size = p - dev_name(&auxdev->dev);
+	if (!p)
+		return NULL;
+	match_size = p - dev_name(&auxdev->dev);
 
+	for (; id->name[0]; id++) {
 		/* use dev_name(&auxdev->dev) prefix before last '.' char to match to */
 		if (strlen(id->name) == match_size &&
 		    !strncmp(dev_name(&auxdev->dev), id->name, match_size))

---
base-commit: 1b237f190eb3d36f52dffe07a40b5eb210280e00
change-id: 20250902-fix_auxbus-76bec91376db

Best regards,
-- 
Zijun Hu <zijun.hu@oss.qualcomm.com>

On Tue, Sep 02, 2025 at 08:05:32PM +0800, Zijun Hu wrote:
> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
> 
> Variable @match_size is fixed in auxiliary_match_id().
> 
> Optimize the function by moving the logic calculating the variable
> out of the for loop.

Optimize it how?  Does this actually result in a difference somehow, if
so, what?

Please be specific.

> Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
> ---
>  drivers/base/auxiliary.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/base/auxiliary.c b/drivers/base/auxiliary.c
> index 12ffdd8437567f282374bbf3775d9de7ca0dc4c7..9a53ada043045031e565ade57fd7ba781e7d20ea 100644
> --- a/drivers/base/auxiliary.c
> +++ b/drivers/base/auxiliary.c
> @@ -171,14 +171,14 @@
>  static const struct auxiliary_device_id *auxiliary_match_id(const struct auxiliary_device_id *id,
>  							    const struct auxiliary_device *auxdev)
>  {
> -	for (; id->name[0]; id++) {
> -		const char *p = strrchr(dev_name(&auxdev->dev), '.');
> -		int match_size;
> +	const char *p = strrchr(dev_name(&auxdev->dev), '.');
> +	int match_size;
>  
> -		if (!p)
> -			continue;
> -		match_size = p - dev_name(&auxdev->dev);
> +	if (!p)
> +		return NULL;
> +	match_size = p - dev_name(&auxdev->dev);
>  
> +	for (; id->name[0]; id++) {
>  		/* use dev_name(&auxdev->dev) prefix before last '.' char to match to */
>  		if (strlen(id->name) == match_size &&
>  		    !strncmp(dev_name(&auxdev->dev), id->name, match_size))
> 

thanks,

greg k-h

On 2025/9/2 20:20, Greg Kroah-Hartman wrote:
> On Tue, Sep 02, 2025 at 08:05:32PM +0800, Zijun Hu wrote:
>> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
>>
>> Variable @match_size is fixed in auxiliary_match_id().
>>
>> Optimize the function by moving the logic calculating the variable
>> out of the for loop.
> Optimize it how?  Does this actually result in a difference somehow, if
> so, what?
> 

auxiliary_match_id() repeatedly calculates variable @match_size in the
for loop. however, the variable is fixed actually. so it is enough to
calculate the variable once.

Optimize it by moving the logic calculating the variable out of the for
loop, and it result in:

1) calculate the variable once instead of repeatedly.
2) it will return as early as possible if device name is unexpected,
namely, (@p == NULL).

so this fix will improve performance.

> Please be specific.

will give more commit message in v2.

thanks

On Tue, Sep 02, 2025 at 08:42:24PM +0800, Zijun Hu wrote:
> On 2025/9/2 20:20, Greg Kroah-Hartman wrote:
> > On Tue, Sep 02, 2025 at 08:05:32PM +0800, Zijun Hu wrote:
> >> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
> >>
> >> Variable @match_size is fixed in auxiliary_match_id().
> >>
> >> Optimize the function by moving the logic calculating the variable
> >> out of the for loop.
> > Optimize it how?  Does this actually result in a difference somehow, if
> > so, what?
> > 
> 
> auxiliary_match_id() repeatedly calculates variable @match_size in the
> for loop. however, the variable is fixed actually. so it is enough to
> calculate the variable once.
> 
> Optimize it by moving the logic calculating the variable out of the for
> loop, and it result in:
> 
> 1) calculate the variable once instead of repeatedly.
> 2) it will return as early as possible if device name is unexpected,
> namely, (@p == NULL).
> 
> so this fix will improve performance.

This isn't a "performant" path at all, is it?

I'm not disagreeing that calculating this all the time is a bad idea,
but please be reasonable.  If it can't be measured, you can't really say
it will "improve performance" :)

thanks,

greg k-h

On 2025/9/2 20:51, Greg Kroah-Hartman wrote:
>> auxiliary_match_id() repeatedly calculates variable @match_size in the
>> for loop. however, the variable is fixed actually. so it is enough to
>> calculate the variable once.
>>
>> Optimize it by moving the logic calculating the variable out of the for
>> loop, and it result in:
>>
>> 1) calculate the variable once instead of repeatedly.
>> 2) it will return as early as possible if device name is unexpected,
>> namely, (@p == NULL).
>>
>> so this fix will improve performance.
> This isn't a "performant" path at all, is it?
> 

agree.

the patch is to improve the function's logic a bit.

> I'm not disagreeing that calculating this all the time is a bad idea,
> but please be reasonable.  If it can't be measured, you can't really say
> it will "improve performance" 🙂

On Tue, Sep 02, 2025 at 08:42:24PM +0800, Zijun Hu wrote:
> On 2025/9/2 20:20, Greg Kroah-Hartman wrote:
> > On Tue, Sep 02, 2025 at 08:05:32PM +0800, Zijun Hu wrote:
> >> From: Zijun Hu <zijun.hu@oss.qualcomm.com>
> >>
> >> Variable @match_size is fixed in auxiliary_match_id().
> >>
> >> Optimize the function by moving the logic calculating the variable
> >> out of the for loop.
> > Optimize it how?  Does this actually result in a difference somehow, if
> > so, what?
> > 
> 
> auxiliary_match_id() repeatedly calculates variable @match_size in the
> for loop. however, the variable is fixed actually. so it is enough to
> calculate the variable once.
> 
> Optimize it by moving the logic calculating the variable out of the for
> loop, and it result in:
> 
> 1) calculate the variable once instead of repeatedly.
> 2) it will return as early as possible if device name is unexpected,
> namely, (@p == NULL).
> 
> so this fix will improve performance.

While the proposed change is valid, it is not a fix and unlikely give
any performance benefits too. First, smart compilers should optimize this
code and perform single calculation instead in the loop and second the
result will be in cache anyway.

Thanks

> 
> > Please be specific.
> 
> will give more commit message in v2.
> 
> thanks