Re: [Xen-devel] [PATCH RFC 13/55] x86/mm: rewrite virt_to_xen_l3e

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Apr 08, 2019 at 09:55:47AM -0600, Jan Beulich wrote:
> >>> On 07.02.19 at 17:44, <wei.liu2@xxxxxxxxxx> wrote:
> > @@ -4769,45 +4769,70 @@ void free_xen_pagetable_new(mfn_t mfn)
> >  
> >  static DEFINE_SPINLOCK(map_pgdir_lock);
> >  
> > +/*
> > + * Given a virtual address, return a pointer to xen's L3 entry. Caller
> > + * needs to unmap the pointer.
> > + */
> >  static l3_pgentry_t *virt_to_xen_l3e(unsigned long v)
> >  {
> >      l4_pgentry_t *pl4e;
> > +    l3_pgentry_t *pl3e = NULL;
> >  
> >      pl4e = &idle_pg_table[l4_table_offset(v)];
> >      if ( !(l4e_get_flags(*pl4e) & _PAGE_PRESENT) )
> >      {
> >          bool locking = system_state > SYS_STATE_boot;
> > -        l3_pgentry_t *l3t = alloc_xen_pagetable();
> > +        l3_pgentry_t *l3t;
> > +        mfn_t mfn;
> > +
> > +        mfn = alloc_xen_pagetable_new();
> > +        if ( mfn_eq(mfn, INVALID_MFN) )
> > +            goto out;
> 
> Any reason not to use "return NULL" here, avoiding the goto
> and label altogether in tis function?

This can be done, but I wanted to have all virt_to_xen_lXe have the same
structure.

> 
> >  static l2_pgentry_t *virt_to_xen_l2e(unsigned long v)
> >  {
> >      l3_pgentry_t *pl3e;
> > +    l2_pgentry_t *pl2e = NULL;
> >  
> >      pl3e = virt_to_xen_l3e(v);
> >      if ( !pl3e )
> > -        return NULL;
> > +        goto out;
> 
> Why not keep this one the way it is?

Because there will be an out label anyway, why not use it to have a
single exit path?

Wei.

> 
> Jan
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel