Re: [Xen-devel] [PATCH v4 11/31] x86/mm: split out writable pagetable emulation code

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Aug 24, 2017 at 09:15:36AM -0600, Jan Beulich wrote:
> >>> On 17.08.17 at 16:44, <wei.liu2@xxxxxxxxxx> wrote:
> > Move the code to pv/emul-ptwr-op.c. Fix coding style issues while
> > moving the code.
> > 
> > Rename ptwr_emulated_read to pv_emul_ptwr_read and export it via
> > pv/mm.h because it is needed by other emulation code.
> 
> If other emulated code uses it, renaming the function would better
> imply dropping the ptwr infix from it. pv_emulated_read() perhaps?
> 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/mm.c              | 308 +-------------------------------------
> >  xen/arch/x86/pv/Makefile       |   1 +
> >  xen/arch/x86/pv/emul-ptwr-op.c | 327 
> > +++++++++++++++++++++++++++++++++++++++++
> 
> Would you mind calling this just ptwr.c?

That seems to deviate from the naming scheme we decided on, but I don't
think I care.

> 
> > +/*************************
> > + * Writable Pagetables
> > + */
> > +
> > +struct ptwr_emulate_ctxt {
> > +    struct x86_emulate_ctxt ctxt;
> > +    unsigned long cr2;
> > +    l1_pgentry_t  pte;
> > +};
> >[...]
> > +static int ptwr_emulated_update(unsigned long addr, paddr_t old, paddr_t 
> > val,
> > +                                unsigned int bytes, unsigned int 
> > do_cmpxchg,
> > +                                struct ptwr_emulate_ctxt *ptwr_ctxt)
> 
> I've meanwhile noticed that in prior patches of yours such movement
> was needlessly retaining the component prefixes. With you splitting
> things into separate files, these aren't really useful anymore - stack
> traces will have them disambiguated by being prefixed with their
> file names. They merely eat valuable serial line bandwidth / ring
> buffer space and clutter the (serial) log. I could accept the structure
> tags to stay the way they are, but please shorten the local function
> names as much as possible without losing information. That'll likely
> mean dropping more than just the ptwr_ prefix.
> 

No problem.

Do you want me to change the ones I already moved? If so, I will do it
before we release 4.10.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel