On Fri, 2019-07-26 at 08:24 -0400, Nitesh Narayan Lal wrote:
> On 7/24/19 1:03 PM, Alexander Duyck wrote:
> > From: Alexander Duyck <alexander.h.duyck@xxxxxxxxxxxxxxx>
> >
> >
<snip>
> > +/*
> > + * The page hinting cycle consists of 4 stages, fill, react, drain, and idle.
> > + * We will cycle through the first 3 stages until we fail to obtain any
> > + * pages, in that case we will switch to idle.
> > + */
> > +static void page_hinting_cycle(struct zone *zone,
> > + struct page_hinting_dev_info *phdev)
> > +{
> > + /*
> > + * Guarantee boundaries and stats are populated before we
> > + * start placing hinted pages in the zone.
> > + */
> > + if (page_hinting_populate_metadata(zone))
> > + return;
> > +
> > + spin_lock(&zone->lock);
> > +
> > + /* set bit indicating boundaries are present */
> > + set_bit(ZONE_PAGE_HINTING_ACTIVE, &zone->flags);
> > +
> > + do {
> > + /* Pull pages out of allocator into a scaterlist */
> > + unsigned int num_hints = page_hinting_fill(zone, phdev);
> > +
> > + /* no pages were acquired, give up */
> > + if (!num_hints)
> > + break;
> > +
> > + spin_unlock(&zone->lock);
>
> Is there any recommendation in general about how/where we should lock and unlock
> zones in the code? For instance, over here you have a zone lock outside the loop
> and you are unlocking it inside the loop and then re-acquiring it.
> My guess is we should be fine as long as:
> 1. We are not holding the lock for a very long time.
> 2. We are making sure that if we have a zone lock we are releasing it before
> returning from the function.
So as a general rule the first two you mention work. Basically what you
want to do is work with some sort of bounded limit when you are holding
the lock so you know it will be released in a timely fashion.
The reason for dropping the lock inside of the loop s because we will end
up sleeping while we wait for the virtio-balloon device to process the
pages. So it makes sense to release the lock, process the pages, and then
reacquire the lock so that we can return the pages and grab another 16
pages.
