On Wed, 2016-03-30 at 12:44 +0530, Hari Bathini wrote: > On 03/30/2016 05:55 AM, Michael Ellerman wrote: > > On Tue, 2016-29-03 at 18:34:37 UTC, Hari Bathini wrote: > > > @@ -1244,6 +1235,16 @@ __end_handlers: > > > STD_RELON_EXCEPTION_PSERIES_OOL(0xf60, facility_unavailable) > > > STD_RELON_EXCEPTION_HV_OOL(0xf80, hv_facility_unavailable) > > > > > > + /* FIXME: For now, let us move the __end_interrupts marker down past > > Why is it FIXME? > > > > In general I don't want to merge code that adds a FIXME unless there is some > > very good reason. > > > > AFAICS this is a permanent solution isn't it? > > Except for a few short interrupt vectors like 0x4f00, 04f20, etc., all other > vectors defined till __end_interrupts marker ensure that LOAD_HANDLER() is > used for branching to labels like system_call_entry, data_access_common, > etc. that are currently not copied to real 0 in relocation case. > > So, we are forced to move the __end_interrupts marker down only to handle > space constraint in the short vectors. So, I added the FIXME to remind the > scope for improvement in the code. But after thinking over again now, moving > the marker down makes us copy an additional 1~2 KB along with the 21~22 KB > that we are copying already. So, not much of an improvement to lose > sleep over or to add a FIXME, I guess. Your thoughts?
OK, that makes sense. I still wouldn't add a FIXME unless you have a plan to fix it "properly" in the medium term. And I don't think we really do. So if we merge it with a FIXME there's a good chance the FIXME will be there in 10 years, which is not really helpful. A comment saying "we only need this here because of xx, and we could move it to y if z happened" is helpful. As far as the size of the copy, that doesn't really concern me at all. What I want is a solution that's robust into the future. This has been badly broken for quite a while and we didn't notice :/ cheers _______________________________________________ Linuxppc-dev mailing list [email protected] https://lists.ozlabs.org/listinfo/linuxppc-dev
