[PATCH -next 0/2] Fix a bug and commit a code optimization

[PATCH -next 0/2] Fix a bug and commit a code optimization

[Xu Qiang]

Xu Qiang (2):
  irqdomain: fix possible uninitialized variable in irq_find_mapping()
  irqdomain: Replace revmap_direct_max_irq field with hwirq_max field

 kernel/irq/irqdomain.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH -next 1/2] irqdomain: fix possible uninitialized variable in irq_find_mapping()

[Xu Qiang]

In irq_find_mapping,ret value may be uninitialized.However,even if
the local variable irq is initialized, it only solves the uninitialized
problem and ret value is still an incorrect virq, so my modification
method is to set virq in __irq_resolve_mapping function.

Fixes: d22558dd0a6c (“irqdomain: Introduce irq_resolve_mapping()”)
Signed-off-by: Xu Qiang <xuqiang36@huawei.com>
---
 kernel/irq/irqdomain.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index d5ce96510549..481abb885d61 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -910,6 +910,8 @@ struct irq_desc *__irq_resolve_mapping(struct irq_domain *domain,
 			data = irq_domain_get_irq_data(domain, hwirq);
 			if (data && data->hwirq == hwirq)
 				desc = irq_data_to_desc(data);
+			if (irq && desc)
+				*irq = hwirq;
 		}
 
 		return desc;
-- 
2.17.1

Re: [PATCH -next 1/2] irqdomain: fix possible uninitialized variable in irq_find_mapping()

[Marc Zyngier]

On Mon, 18 Jul 2022 14:07:58 +0100,
Xu Qiang <xuqiang36@huawei.com> wrote:
> 
> In irq_find_mapping,ret value may be uninitialized.However,even if
> the local variable irq is initialized, it only solves the uninitialized
> problem and ret value is still an incorrect virq, so my modification
> method is to set virq in __irq_resolve_mapping function.

I think I understand what you are fixing, but I sadly don't understand
the commit message. Here's what I suggest as a commit message:

<commit>
When using a NOMAP domain, __irq_resolve_mapping() doesn't store the
Linux IRQ number at the address optionally provided by the caller.

While this isn't a huge deal (the returned value is guaranteed to the
hwirq that was passed as a parameter), let's honour the letter of the
API by writing the expected value.
</commit>

Does this match what you expected?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

[PATCH -next 2/2] irqdomain: Replace revmap_direct_max_irq field with hwirq_max field

[Xu Qiang]

In commit "4f86a06e2d6e irqdomain: Make normal and nomap irqdomains exclusive",
use revmap_size field instead of revmap_direct_max_irq. revmap_size field
originally indicates the maximum hwirq of linear Mapping. This results in
revmap_size having two different layers of meaning that can be confusing.

This patch optimization point is to solve this confusion point. During
direct mapping, the values of hwirq_max and revmap_direct_max_irq are the same
and have the same meanings. They both indicate the maximum hwirq supported by
direct Mapping. The optimization method is to delete revmap_direct_max_irq
field and use hwirq_max instead of revmap_direct_max_irq.

Signed-off-by: Xu Qiang <xuqiang36@huawei.com>
---
 kernel/irq/irqdomain.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 481abb885d61..384eb9166804 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -147,7 +147,8 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
 	static atomic_t unknown_domains;
 
 	if (WARN_ON((size && direct_max) ||
-		    (!IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) && direct_max)))
+		    (!IS_ENABLED(CONFIG_IRQ_DOMAIN_NOMAP) && direct_max) ||
+		    (direct_max && (direct_max != hwirq_max))))
 		return NULL;
 
 	domain = kzalloc_node(struct_size(domain, revmap, size),
@@ -219,7 +220,6 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle *fwnode, unsigned int s
 	domain->hwirq_max = hwirq_max;
 
 	if (direct_max) {
-		size = direct_max;
 		domain->flags |= IRQ_DOMAIN_FLAG_NO_MAP;
 	}
 
@@ -650,9 +650,9 @@ unsigned int irq_create_direct_mapping(struct irq_domain *domain)
 		pr_debug("create_direct virq allocation failed\n");
 		return 0;
 	}
-	if (virq >= domain->revmap_size) {
+	if (virq >= domain->hwirq_max) {
 		pr_err("ERROR: no free irqs available below %i maximum\n",
-			domain->revmap_size);
+			domain->hwirq_max);
 		irq_free_desc(virq);
 		return 0;
 	}
@@ -906,7 +906,7 @@ struct irq_desc *__irq_resolve_mapping(struct irq_domain *domain,
 		return desc;
 
 	if (irq_domain_is_nomap(domain)) {
-		if (hwirq < domain->revmap_size) {
+		if (hwirq < domain->hwirq_max) {
 			data = irq_domain_get_irq_data(domain, hwirq);
 			if (data && data->hwirq == hwirq)
 				desc = irq_data_to_desc(data);
-- 
2.17.1

Re: [PATCH -next 2/2] irqdomain: Replace revmap_direct_max_irq field with hwirq_max field

[kernel test robot]

Hi Xu,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20220718]

url:    https://github.com/intel-lab-lkp/linux/commits/Xu-Qiang/Fix-a-bug-and-commit-a-code-optimization/20220718-211349
base:    036ad6daa8f0fd357af7f50f9da58539eaa6f68c
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20220719/202207190020.iGdy9c8j-lkp@intel.com/config)
compiler: powerpc-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/36d8f25dc578d22517f40deeb705f0a9e1817a42
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Xu-Qiang/Fix-a-bug-and-commit-a-code-optimization/20220718-211349
        git checkout 36d8f25dc578d22517f40deeb705f0a9e1817a42
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash kernel/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/kernel.h:29,
                    from include/linux/cpumask.h:10,
                    from include/linux/smp.h:13,
                    from include/linux/lockdep.h:14,
                    from include/linux/mutex.h:17,
                    from include/linux/kernfs.h:11,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:17,
                    from include/linux/irqdomain.h:35,
                    from include/linux/acpi.h:13,
                    from kernel/irq/irqdomain.c:5:
   kernel/irq/irqdomain.c: In function 'irq_create_direct_mapping':
>> include/linux/kern_levels.h:5:25: warning: format '%i' expects argument of type 'int', but argument 2 has type 'irq_hw_number_t' {aka 'long unsigned int'} [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:436:25: note: in definition of macro 'printk_index_wrap'
     436 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:507:9: note: in expansion of macro 'printk'
     507 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:507:16: note: in expansion of macro 'KERN_ERR'
     507 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   kernel/irq/irqdomain.c:654:17: note: in expansion of macro 'pr_err'
     654 |                 pr_err("ERROR: no free irqs available below %i maximum\n",
         |                 ^~~~~~
   kernel/irq/irqdomain.c: At top level:
   kernel/irq/irqdomain.c:1921:13: warning: no previous prototype for 'irq_domain_debugfs_init' [-Wmissing-prototypes]
    1921 | void __init irq_domain_debugfs_init(struct dentry *root)
         |             ^~~~~~~~~~~~~~~~~~~~~~~


vim +5 include/linux/kern_levels.h

314ba3520e513a Joe Perches 2012-07-30  4  
04d2c8c83d0e3a Joe Perches 2012-07-30 @5  #define KERN_SOH	"\001"		/* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30  6  #define KERN_SOH_ASCII	'\001'
04d2c8c83d0e3a Joe Perches 2012-07-30  7  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH -next 0/2] Fix a bug and commit a code optimization

[Marc Zyngier]

On Mon, 18 Jul 2022 14:07:57 +0100,
Xu Qiang <xuqiang36@huawei.com> wrote:
> 
> Xu Qiang (2):
>   irqdomain: fix possible uninitialized variable in irq_find_mapping()
>   irqdomain: Replace revmap_direct_max_irq field with hwirq_max field
> 
>  kernel/irq/irqdomain.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)

Although I'm supportive of your effort to have sent a cover letter (I
wish people did that more often), you probably want to add some actual
information here.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.