| Message ID | 20200113213342.8206-2-julien@xen.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/x86: Rework inclusion between struct pirq and | expand |
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
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,
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
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,
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 {