Re: [Xen-devel] [PATCH v2 2/3] xen: remove tmem from hypervisor

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Nov 28, 2018 at 07:43:25AM -0700, Jan Beulich wrote:
> >>> On 28.11.18 at 14:58, <wei.liu2@xxxxxxxxxx> wrote:
> > @@ -250,7 +249,7 @@ static void populate_physmap(struct memop_args *a)
> >  
> >                  if ( unlikely(!page) )
> >                  {
> > -                    if ( !tmem_enabled() || a->extent_order )
> > +                    if ( a->extent_order )
> >                          gdprintk(XENLOG_INFO,
> >                                   "Could not allocate order=%u extent: 
> > id=%d memflags=%#x (%u of %u)\n",
> >                                   a->extent_order, d->domain_id, 
> > a->memflags,
> 
> From an abstract pov without tmem tmem_enabled() should return constant
> "false". Which seems to mean that the if() should go away rather than its
> condition getting changed.

Ack.

> 
> > @@ -949,22 +935,6 @@ static struct page_info *alloc_heap_pages(
> >          return NULL;
> >      }
> >  
> > -    /*
> > -     * TMEM: When available memory is scarce due to tmem absorbing it, 
> > allow
> > -     * only mid-size allocations to avoid worst of fragmentation issues.
> > -     * Others try tmem pools then fail.  This is a workaround until all
> > -     * post-dom0-creation-multi-page allocations can be eliminated.
> > -     */
> > -    if ( ((order == 0) || (order >= 9)) &&
> > -         (total_avail_pages <= midsize_alloc_zone_pages) &&
> > -         tmem_freeable_pages() )
> > -    {
> > -        /* Try to free memory from tmem. */
> > -        pg = tmem_relinquish_pages(order, memflags);
> > -        spin_unlock(&heap_lock);
> > -        return pg;
> > -    }
> > -
> >      pg = get_free_buddy(zone_lo, zone_hi, order, memflags, d);
> >      /* Try getting a dirty buddy if we couldn't get a clean one. */
> >      if ( !pg && !(memflags & MEMF_no_scrub) )
> > @@ -1444,10 +1414,6 @@ static void free_heap_pages(
> >      else
> >          pg->u.free.first_dirty = INVALID_DIRTY_IDX;
> >  
> > -    if ( tmem_enabled() )
> > -        midsize_alloc_zone_pages = max(
> > -            midsize_alloc_zone_pages, total_avail_pages / 
> > MIDSIZE_ALLOC_FRAC);
> 
> Seeing these two hunks I think midsize_alloc_zone_pages and
> MIDSIZE_ALLOC_FRAC want to go away altogether.

Ack.

> 
> > --- a/xen/include/xen/mm.h
> > +++ b/xen/include/xen/mm.h
> > @@ -248,8 +248,10 @@ struct npfec {
> >  #define  MEMF_no_refcount (1U<<_MEMF_no_refcount)
> >  #define _MEMF_populate_on_demand 1
> >  #define  MEMF_populate_on_demand (1U<<_MEMF_populate_on_demand)
> > +#if 0
> >  #define _MEMF_tmem        2
> >  #define  MEMF_tmem        (1U<<_MEMF_tmem)
> > +#endif
> 
> Why "#if 0" rather than removing the two lines?

I wanted to keep it around so that later when someone reads the code
they won't be asking "why is 2 not used".

But yes I'm fine with just deleting these two lines.

> 
> With these suitably taken care of feel free to add
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Thanks.

> 
> Jan
> 
> 

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