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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 12 Oct 2016 09:33:28 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Minchan Kim <minchan@...nel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Vlastimil Babka <vbabka@...e.cz>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        Sangseok Lee <sangseok.lee@....com>
Subject: Re: [PATCH v2 4/4] mm: make unreserve highatomic functions reliable

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