diff mbox series

[4/5] genirq: Use the common function irq_expand_nr_irqs()

Message ID 20230130005725.3517597-5-sdonthineni@nvidia.com
State New
Headers
Series Increase the number of IRQ descriptors for SPARSEIRQ |

Commit Message

Shanker Donthineni Jan. 30, 2023, 12:57 a.m. UTC 
  When the !SPARSEIRQ code path is executed, the function
irq_expand_nr_irqs() returns -ENOMEM. However, the SPARSEIRQ
version of the function can be safely used in both cases, as
nr_irqs = MAX_SPARSE_IRQS = NR_IRQS.

Signed-off-by: Shanker Donthineni <sdonthineni@nvidia.com>
---
 kernel/irq/irqdesc.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

Comments

Thomas Gleixner Jan. 31, 2023, 9:35 a.m. UTC | #1 
On Sun, Jan 29 2023 at 18:57, Shanker Donthineni wrote:

> Subject: genirq: Use the common function ...

  genirq: Unify irq_expand_nr_irqs()

  irq_expand_nr_irqs() is implemented as a stub function for !SPARSEIRQ
  builds. That's not necessary as the SPARSEIRQ version returns -ENOMEM
  correctly even for the !SPARSEIRQ case as the ....


But this common function is non-obvious for the !SPARSEIRQ case. It at
least needs a comment

> +static int irq_expand_nr_irqs(unsigned int nr)
> +{
> +	if (nr > MAX_SPARSE_IRQS)
> +		return -ENOMEM;
> +	nr_irqs = nr;
> +	return 0;
> +}

or preferrably something like this:

	if (!IS_ENABLED(CONFIG_SPARSEIRQ) || nr > MAX_SPARSE_IRQS)
		return -ENOMEM;

which makes it entirely clear and also allows the compiler to optimize
is down to a 'return -ENOMEM'.

Thanks,

        tglx
Shanker Donthineni Jan. 31, 2023, 4:43 p.m. UTC | #2 
On 1/31/23 03:35, Thomas Gleixner wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Sun, Jan 29 2023 at 18:57, Shanker Donthineni wrote:
> 
>> Subject: genirq: Use the common function ...
> 
>    genirq: Unify irq_expand_nr_irqs()
> 
>    irq_expand_nr_irqs() is implemented as a stub function for !SPARSEIRQ
>    builds. That's not necessary as the SPARSEIRQ version returns -ENOMEM
>    correctly even for the !SPARSEIRQ case as the ....
> 
> 
> But this common function is non-obvious for the !SPARSEIRQ case. It at
> least needs a comment
> 
>> +static int irq_expand_nr_irqs(unsigned int nr)
>> +{
>> +     if (nr > MAX_SPARSE_IRQS)
>> +             return -ENOMEM;
>> +     nr_irqs = nr;
>> +     return 0;
>> +}
> 
> or preferrably something like this:
> 
>          if (!IS_ENABLED(CONFIG_SPARSEIRQ) || nr > MAX_SPARSE_IRQS)
>                  return -ENOMEM;
> 
> which makes it entirely clear and also allows the compiler to optimize
> is down to a 'return -ENOMEM'.
> 

I'll drop this patch since you're suggesting to remove !SPARSEIRQ support.
Thomas Gleixner Feb. 7, 2023, 10:29 a.m. UTC | #3 
On Tue, Jan 31 2023 at 10:43, Shanker Donthineni wrote:
> On 1/31/23 03:35, Thomas Gleixner wrote:
>>> +static int irq_expand_nr_irqs(unsigned int nr)
>>> +{
>>> +     if (nr > MAX_SPARSE_IRQS)
>>> +             return -ENOMEM;
>>> +     nr_irqs = nr;
>>> +     return 0;
>>> +}
>> 
>> or preferrably something like this:
>> 
>>          if (!IS_ENABLED(CONFIG_SPARSEIRQ) || nr > MAX_SPARSE_IRQS)
>>                  return -ENOMEM;
>> 
>> which makes it entirely clear and also allows the compiler to optimize
>> is down to a 'return -ENOMEM'.
>> 
> I'll drop this patch since you're suggesting to remove !SPARSEIRQ support.

Sometime in the future when I analyzed what the implications are. So
just keep it and make it readable.

Thanks,

        tglx
diff mbox series

Patch

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index aacfb4826c5e..247a0718d028 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -144,6 +144,14 @@  static unsigned int irq_find_next_irq(unsigned int offset)
 	return find_next_bit(allocated_irqs, nr_irqs, offset);
 }
 
+static int irq_expand_nr_irqs(unsigned int nr)
+{
+	if (nr > MAX_SPARSE_IRQS)
+		return -ENOMEM;
+	nr_irqs = nr;
+	return 0;
+}
+
 #ifdef CONFIG_SPARSE_IRQ
 
 static void irq_kobj_release(struct kobject *kobj);
@@ -528,14 +536,6 @@  static int alloc_descs(unsigned int start, unsigned int cnt, int node,
 	return -ENOMEM;
 }
 
-static int irq_expand_nr_irqs(unsigned int nr)
-{
-	if (nr > MAX_SPARSE_IRQS)
-		return -ENOMEM;
-	nr_irqs = nr;
-	return 0;
-}
-
 int __init early_irq_init(void)
 {
 	int i, initcnt, node = first_online_node;
@@ -630,11 +630,6 @@  static inline int alloc_descs(unsigned int start, unsigned int cnt, int node,
 	return start;
 }
 
-static int irq_expand_nr_irqs(unsigned int nr)
-{
-	return -ENOMEM;
-}
-
 void irq_mark_irq(unsigned int irq)
 {
 	mutex_lock(&sparse_irq_lock);