Re: [Xen-devel] [PATCH v5 4/8] mm: Scrub memory from idle loop

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

> > @@ -1050,17 +1120,42 @@ static void scrub_free_pages(unsigned int node)
> >                           scrub_one_page(&pg[i]);
> >                           pg[i].count_info &= ~PGC_need_scrub;
> >                           node_need_scrub[node]--;
> > +                        cnt += 100; /* scrubbed pages add heavier weight. */
> > +                    }
> > +                    else
> > +                        cnt++;
> > +
> > +                    /*
> > +                     * Scrub a few (8) pages before becoming eligible for
> > +                     * preemption. But also count non-scrubbing loop iterations
> > +                     * so that we don't get stuck here with an almost clean
> > +                     * heap.
> > +                     */
> > +                    if ( cnt > 800 && softirq_pending(cpu) )
> > +                    {
> > +                        preempt = true;
> > +                        break;
> >                       }
> >                   }
> >   
> > -                page_list_del(pg, &heap(node, zone, order));
> > -                page_list_add_scrub(pg, node, zone, order, INVALID_DIRTY_IDX);
> > +                if ( i >= (1U << order) - 1 )
> > +                {
> > +                    page_list_del(pg, &heap(node, zone, order));
> > +                    page_list_add_scrub(pg, node, zone, order, 
> > INVALID_DIRTY_IDX);
> > +                }
> > +                else
> > +                    pg->u.free.first_dirty = i + 1;
> >   
> > -                if ( node_need_scrub[node] == 0 )
> > -                    return;
> > +                if ( preempt || (node_need_scrub[node] == 0) )
> > +                    goto out;
> >               }
> >           } while ( order-- != 0 );
> >       }
> > +
> > + out:
> > +    spin_unlock(&heap_lock);
> > +    node_clear(node, node_scrubbing);
> > +    return softirq_pending(cpu) || (node_to_scrub(false) != NUMA_NO_NODE);
> While I can see why you use it here, the softirq_pending() looks sort of
> misplaced: While invoking it twice in the caller will look a little odd too,
> I still think that's where the check belongs.

scrub_free_pages is called from idle loop as

        else if ( !softirq_pending(cpu) && !scrub_free_pages() )
            pm_idle();

so softirq_pending() is unnecessary here.

(Not sure why you are saying it would be invoked twice)

-boris

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