On Wed, 2016-03-30 at 12:44 +0530, Hari Bathini wrote:

On 03/30/2016 05:55 AM, Michael Ellerman wrote:

quoted

On Tue, 2016-29-03 at 18:34:37 UTC, Hari Bathini wrote:

quoted

@@ -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