Subject: [PATCH] pci/msi: remove redundant calculation in msi_setup_msi_desc

Whether to support 64-bit address status has been calculated before,
and the calculation result can be used directly afterwards, so use
msi_attrib.is_64 to avoid double calculation.

Suggested-by: weipengliang <weipengliang@xiaomi.com>
Signed-off-by: sparkhuang <huangshaobo3@xiaomi.com>
---
 drivers/pci/msi/msi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--
2.7.4

#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

On Fri, 18 Aug 2023, 黄少波 wrote:

You should add () to the function name in the subject and use capital 
PCI/MSI.

> Whether to support 64-bit address status has been calculated before,
> and the calculation result can be used directly afterwards, so use
> msi_attrib.is_64 to avoid double calculation.
> 
> Suggested-by: weipengliang <weipengliang@xiaomi.com>
> Signed-off-by: sparkhuang <huangshaobo3@xiaomi.com>
> ---
>  drivers/pci/msi/msi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
> index ef1d885..304e889 100644
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -303,7 +303,7 @@ static int msi_setup_msi_desc(struct pci_dev *dev, int nvec,
>         desc.pci.msi_attrib.multiple    = ilog2(__roundup_pow_of_two(nvec));
>         desc.affinity                   = masks;
> 
> -       if (control & PCI_MSI_FLAGS_64BIT)
> +       if (desc.pci.msi_attrib.is_64)
>                 desc.pci.mask_pos = dev->msi_cap + PCI_MSI_MASK_64;
>         else
>                 desc.pci.mask_pos = dev->msi_cap + PCI_MSI_MASK_32;
> --
> 2.7.4

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

On Fri, Aug 18 2023 at 07:26, 黄少波 wrote:

Your patch is corrupt:

 Applying: Subject: [PATCH] pci/msi: remove redundant calculation in msi_setup_msi_desc
 error: corrupt patch at line 12

It's DOS mangled and whitespace damaged.

> Whether to support 64-bit address status has been calculated before,
> and the calculation result can be used directly afterwards, so use
> msi_attrib.is_64 to avoid double calculation.

I'm not seeing what this solves:

> -       if (control & PCI_MSI_FLAGS_64BIT)
> +       if (desc.pci.msi_attrib.is_64)

Both variants resolve to a test of a bit and a conditional instruction
on the result. It's exactly zero difference in terms of "calculation".

So all this does is change the memory location to test. Not more not
less. It does not generate better code and does not save anything.

Thanks,

        tglx

On Wed, 23 Aug 2023 16:15:27 +0200, Thomas Gleixner wrote:
> On Fri, Aug 18 2023 at 07:26, 黄少波 wrote:

> Your patch is corrupt:

>  Applying: Subject: [PATCH] pci/msi: remove redundant calculation in msi_setup_msi_desc
>  error: corrupt patch at line 12

> It's DOS mangled and whitespace damaged.

Very sorry, may have accidentally broken the patch format,
and will make sure to use plain text next time.

> > Whether to support 64-bit address status has been calculated before,
> > and the calculation result can be used directly afterwards, so use
> > msi_attrib.is_64 to avoid double calculation.

> I'm not seeing what this solves:

> > -       if (control & PCI_MSI_FLAGS_64BIT)
> > +       if (desc.pci.msi_attrib.is_64)

> Both variants resolve to a test of a bit and a conditional instruction
> on the result. It's exactly zero difference in terms of "calculation".

> So all this does is change the memory location to test. Not more not
> less. It does not generate better code and does not save anything.

It may not be appropriate to write to eliminate duplicate calculations,
can it be proposed again with clean code?

> Thanks,

>         tglx

thanks,
sparkhuang
#/******本邮件及其附件含有小米公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#

On Thu, Aug 24 2023 at 00:27, huangshaobo3@xiaomi.com wrote:
> On Wed, 23 Aug 2023 16:15:27 +0200, Thomas Gleixner wrote:
>> I'm not seeing what this solves:
>
>> > -       if (control & PCI_MSI_FLAGS_64BIT)
>> > +       if (desc.pci.msi_attrib.is_64)
>
>> Both variants resolve to a test of a bit and a conditional instruction
>> on the result. It's exactly zero difference in terms of "calculation".
>
>> So all this does is change the memory location to test. Not more not
>> less. It does not generate better code and does not save anything.
>
> It may not be appropriate to write to eliminate duplicate calculations,
> can it be proposed again with clean code?

What's wrong with the existing code?

On Thu, 24 Aug 2023 11:04:16 +0200, Thomas Gleixner wrote:
> On Thu, Aug 24 2023 at 00:27, huangshaobo3@xiaomi.com wrote:
> > On Wed, 23 Aug 2023 16:15:27 +0200, Thomas Gleixner wrote:
> >> I'm not seeing what this solves:
> >
> >> > -       if (control & PCI_MSI_FLAGS_64BIT)
> >> > +       if (desc.pci.msi_attrib.is_64)
> >
> >> Both variants resolve to a test of a bit and a conditional instruction
> >> on the result. It's exactly zero difference in terms of "calculation".
> >
> >> So all this does is change the memory location to test. Not more not
> >> less. It does not generate better code and does not save anything.
> >
> > It may not be appropriate to write to eliminate duplicate calculations,
> > can it be proposed again with clean code?
>
> What's wrong with the existing code?

There is nothing wrong with the current code,
using desc.pci.msi_attrib.is_64 does not solve any problems,
thank you very much for reviewing comments.

Thanks,
sparkhuang
#/******±¾Óʼþ¼°Æ丽¼þº¬ÓÐСÃ×¹«Ë¾µÄ±£ÃÜÐÅÏ¢£¬½öÏÞÓÚ·¢Ë͸øÉÏÃæµØÖ·ÖÐÁгöµÄ¸öÈË»òȺ×é¡£½ûÖ¹ÈκÎÆäËûÈËÒÔÈκÎÐÎʽʹÓ㨰üÀ¨µ«²»ÏÞÓÚÈ«²¿»ò²¿·ÖµØй¶¡¢¸´ÖÆ¡¢»òÉ¢·¢£©±¾ÓʼþÖеÄÐÅÏ¢¡£Èç¹ûÄú´íÊÕÁ˱¾Óʼþ£¬ÇëÄúÁ¢¼´µç»°»òÓʼþ֪ͨ·¢¼þÈ˲¢É¾³ý±¾Óʼþ£¡ This e-mail and its attachments contain confidential information from XIAOMI, which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction, or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender by phone or email immediately and delete it!******/#