[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5d6222a80708090002y7fdc6f2dw1dbacd3d27a86433@mail.gmail.com>
Date: Thu, 9 Aug 2007 04:02:14 -0300
From: "Glauber de Oliveira Costa" <glommer@...il.com>
To: "Jeremy Fitzhardinge" <jeremy@...p.org>
Cc: "Glauber de Oliveira Costa" <gcosta@...hat.com>,
linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
rusty@...tcorp.com.au, ak@...e.de, mingo@...e.hu,
chrisw@...s-sol.org, avi@...ranet.com, anthony@...emonkey.ws,
virtualization@...ts.linux-foundation.org, lguest@...abs.org,
"Steven Rostedt" <rostedt@...dmis.org>
Subject: Re: [PATCH 25/25] [PATCH] add paravirtualization support for x86_64
> > + case PARAVIRT_PATCH(make_pgd):
> > + case PARAVIRT_PATCH(pgd_val):
> > + case PARAVIRT_PATCH(make_pte):
> > + case PARAVIRT_PATCH(pte_val):
> > + case PARAVIRT_PATCH(make_pmd):
> > + case PARAVIRT_PATCH(pmd_val):
> > + case PARAVIRT_PATCH(make_pud):
> > + case PARAVIRT_PATCH(pud_val):
> > + /* These functions end up returning what
> > + they're passed in the first argument */
> >
>
> Is this still true with 64-bit? Either way, I don't think its worth
> having this here. The damage to codegen around all those sites has
> already happened, and the additional cost of a noop direct call is
> pretty trivial. I think this is a nanooptimisation which risks more
> problems than it could possibly be worth.
No it is not. But it is just the comment that is broken. (I forgot to
update it). The case here, is that they put in rax what they receive
in rdi.
> > + case PARAVIRT_PATCH(set_pte):
> > + case PARAVIRT_PATCH(set_pmd):
> > + case PARAVIRT_PATCH(set_pud):
> > + case PARAVIRT_PATCH(set_pgd):
> > + /* These functions end up storing the second
> > + * argument in the location pointed by the first */
> > + ret = paravirt_patch_store_reg(insns, len);
> > + break;
> >
>
> Ditto, really. Do this in a later patch if it actually seems to help.
Okay, I can remove them both.
> > +/*
> > + * integers must be use with care here. They can break the PARAVIRT_PATCH(x)
> > + * macro, that divides the offset in the structure by 8, to get a number
> > + * associated with the hook. Dividing by four would be a solution, but it
> > + * would limit the future growth of the structure if needed.
> >
>
> Why not just stick them at the end of the structure?
Does it really matter?
--
Glauber de Oliveira Costa.
"Free as in Freedom"
http://glommer.net
"The less confident you are, the more serious you have to act."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists