Re: [PATCH v2 4/4] mm: make unreserve highatomic functions reliable

[Michal Hocko]

On Wed 12-10-16 14:33:36, Minchan Kim wrote:
[...]
> @@ -2138,8 +2146,10 @@ static bool unreserve_highatomic_pageblock(const struct alloc_context *ac)
>  			 */
>  			set_pageblock_migratetype(page, ac->migratetype);
>  			ret = move_freepages_block(zone, page, ac->migratetype);
> -			spin_unlock_irqrestore(&zone->lock, flags);
> -			return ret;
> +			if (!drain && ret) {
> +				spin_unlock_irqrestore(&zone->lock, flags);
> +				return ret;
> +			}

I've already mentioned that during the previous discussion. This sounds
overly aggressive to me. Why do we want to drain the whole reserve and
risk that we won't be able to build up a new one after OOM. Doing one
block at the time should be sufficient IMHO.

			if (ret) {
				spin_unlock_irqrestore(&zone->lock, flags);
				return ret;
			}

will do the trick and work both for drain and !drain cases which is a
good thing. Because even !drain case would like to see a block freed.
The only difference between those two is that the drain one would really
like to free something and ignore the "at least one block" reserve.

-- 
Michal Hocko
SUSE Labs