[1/4] xen/x86: Remove unused forward declaration in asm-x86/irq.h
diff mbox series

Message ID 20200113213342.8206-2-julien@xen.org
State New, archived
Headers show
Series
  • xen/x86: Rework inclusion between struct pirq and
Related show

Commit Message

Julien Grall Jan. 13, 2020, 9:33 p.m. UTC
From: Julien Grall <jgrall@amazon.com>

None of the prototypes within the header asm-x86/irq.h actually requires
the forward declaration of "struct pirq". So remove it.

No functional changes intended.

Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/include/asm-x86/irq.h | 1 -
 1 file changed, 1 deletion(-)

Comments

Jan Beulich Jan. 14, 2020, 9:31 a.m. UTC | #1
On 13.01.2020 22:33, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> None of the prototypes within the header asm-x86/irq.h actually requires
> the forward declaration of "struct pirq". So remove it.
> 
> No functional changes intended.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

It is generally nice to identify if this was missed cleanup (the
need indeed went away in 4.12), or if such has never really been
needed.

Jan
Julien Grall Jan. 14, 2020, 10:05 a.m. UTC | #2
Hi Jan,

On 14/01/2020 09:31, Jan Beulich wrote:
> On 13.01.2020 22:33, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> None of the prototypes within the header asm-x86/irq.h actually requires
>> the forward declaration of "struct pirq". So remove it.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> It is generally nice to identify if this was missed cleanup (the
> need indeed went away in 4.12), or if such has never really been
> needed.

Yes it is nice to have but this is a best effort basis for cleanup. They 
are not fixes and therefore not going to be backported. So I don't feel 
the need to browse more than 15 years worth of history and check whether 
a cleanup were missed.

What matter for cleanup is the current context and whether they make 
sense now.

Anyway, I would be happy to add a word in the commit message if you 
point me to the commit removing the need.

Cheers,
Jan Beulich Jan. 14, 2020, 10:15 a.m. UTC | #3
On 14.01.2020 11:05, Julien Grall wrote:
> On 14/01/2020 09:31, Jan Beulich wrote:
>> On 13.01.2020 22:33, Julien Grall wrote:
>>> From: Julien Grall <jgrall@amazon.com>
>>>
>>> None of the prototypes within the header asm-x86/irq.h actually requires
>>> the forward declaration of "struct pirq". So remove it.
>>>
>>> No functional changes intended.
>>>
>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>
>> It is generally nice to identify if this was missed cleanup (the
>> need indeed went away in 4.12), or if such has never really been
>> needed.
> 
> Yes it is nice to have but this is a best effort basis for cleanup. They 
> are not fixes and therefore not going to be backported. So I don't feel 
> the need to browse more than 15 years worth of history and check whether 
> a cleanup were missed.

15 years? It took me less than a minute (a single grep) to figure
the version this became unnecessary in. And I wouldn't ask for
such on a pretty simple patch like this one if I anticipated a
lot of effort to be needed.

> What matter for cleanup is the current context and whether they make 
> sense now.

I disagree. History often helps understand whether something was done
in a certain way without an obvious (from current state of things)
reason.

> Anyway, I would be happy to add a word in the commit message if you 
> point me to the commit removing the need.

Me having told you the version it disappeared in would have made this
very low effort to you. Anyway: c759fb5bc303 ("x86: move
hvm_domain_use_pirq to hvm files").

Jan
Julien Grall Jan. 14, 2020, 10:34 a.m. UTC | #4
On 14/01/2020 10:15, Jan Beulich wrote:
> On 14.01.2020 11:05, Julien Grall wrote:
>> On 14/01/2020 09:31, Jan Beulich wrote:
>>> On 13.01.2020 22:33, Julien Grall wrote:
>>>> From: Julien Grall <jgrall@amazon.com>
>>>>
>>>> None of the prototypes within the header asm-x86/irq.h actually requires
>>>> the forward declaration of "struct pirq". So remove it.
>>>>
>>>> No functional changes intended.
>>>>
>>>> Signed-off-by: Julien Grall <jgrall@amazon.com>
>>>
>>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>>
>>> It is generally nice to identify if this was missed cleanup (the
>>> need indeed went away in 4.12), or if such has never really been
>>> needed.
>>
>> Yes it is nice to have but this is a best effort basis for cleanup. They
>> are not fixes and therefore not going to be backported. So I don't feel
>> the need to browse more than 15 years worth of history and check whether
>> a cleanup were missed.
> 
> 15 years? It took me less than a minute (a single grep) to figure
> the version this became unnecessary in. And I wouldn't ask for
> such on a pretty simple patch like this one if I anticipated a
> lot of effort to be needed.

My comment is generic to cleanup... As I said, this is a best effort 
basis. Maybe I could have done it here, but I didn't feel the need to do it.

> 
>> What matter for cleanup is the current context and whether they make
>> sense now.
> 
> I disagree. History often helps understand whether something was done
> in a certain way without an obvious (from current state of things)
> reason.
> 
>> Anyway, I would be happy to add a word in the commit message if you
>> point me to the commit removing the need.
> 
> Me having told you the version it disappeared in would have made this
> very low effort to you. 
That's pretty much withdrawing knowledge you may have. I don't think 
this is suitable mindset for a collaborative project. The more you have 
more knowledge than me on the x86/pirq part.

Anyway, thank you for thinking I put little effort on this patch... 
That's a pretty great way to encourage people...

Cheers,

Patch
diff mbox series

diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h
index 7c825e9d9c..44aefc8f03 100644
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -131,7 +131,6 @@  extern unsigned int io_apic_irqs;
 
 DECLARE_PER_CPU(unsigned int, irq_count);
 
-struct pirq;
 struct arch_pirq {
     int irq;
     union {