Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752545AbdIAVDK (ORCPT ); Fri, 1 Sep 2017 17:03:10 -0400 Received: from mail-io0-f181.google.com ([209.85.223.181]:33803 "EHLO mail-io0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752465AbdIAVDI (ORCPT ); Fri, 1 Sep 2017 17:03:08 -0400 X-Google-Smtp-Source: ADKCNb6M67rbgYSa0718yua/dcVN798ltG2x+OsezJ51+Vzc5kEJjHGS1xWliA66qPdKdrEcMVKuD70ZgVyGZU7UrLM= MIME-Version: 1.0 In-Reply-To: References: <1503556593-10720-1-git-send-email-iamjoonsoo.kim@lge.com> <1503556593-10720-2-git-send-email-iamjoonsoo.kim@lge.com> <20170831014048.GA24271@js1304-P5Q-DELUXE> From: Kees Cook Date: Fri, 1 Sep 2017 14:03:06 -0700 X-Google-Sender-Auth: blPFJZ0zAfaY7u3g4FyAxDzkRIM Message-ID: Subject: Re: [PATCH 1/3] mm/cma: manage the memory of the CMA area by using the ZONE_MOVABLE To: Vlastimil Babka Cc: Joonsoo Kim , Andrew Morton , Rik van Riel , Johannes Weiner , mgorman@techsingularity.net, Laura Abbott , Minchan Kim , Marek Szyprowski , Michal Nazarewicz , "Aneesh Kumar K . V" , Russell King , Will Deacon , Linux-MM , LKML , kernel-team@lge.com Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2922 Lines: 75 On Thu, Aug 31, 2017 at 4:32 AM, Vlastimil Babka wrote: > On 08/31/2017 03:40 AM, Joonsoo Kim wrote: >> On Tue, Aug 29, 2017 at 11:16:18AM +0200, Vlastimil Babka wrote: >>> On 08/24/2017 08:36 AM, js1304@gmail.com wrote: >>>> From: Joonsoo Kim >>>> >>>> 0. History >>>> >>>> This patchset is the follow-up of the discussion about the >>>> "Introduce ZONE_CMA (v7)" [1]. Please reference it if more information >>>> is needed. >>>> >>> >>> [...] >>> >>>> >>>> [1]: lkml.kernel.org/r/1491880640-9944-1-git-send-email-iamjoonsoo.kim@lge.com >>>> [2]: https://lkml.org/lkml/2014/10/15/623 >>>> [3]: http://www.spinics.net/lists/linux-mm/msg100562.html >>>> >>>> Reviewed-by: Aneesh Kumar K.V >>>> Acked-by: Vlastimil Babka >>> >>> The previous version has introduced ZONE_CMA, so I would think switching >>> to ZONE_MOVABLE is enough to drop previous reviews. Perhaps most of the >>> code involved is basically the same, though? >> >> Yes, most of the code involved is the same. I considered to drop >> previous review tags but most of the code and concept is the same so I >> decide to keep review tags. I should mention it in cover-letter but I >> forgot to mention it. Sorry about that. >> >>> Anyway I checked the current patch and did some basic tests with qemu, >>> so you can keep my ack. >> >> Thanks! >> >>> >>> BTW, if we dropped NR_FREE_CMA_PAGES, could we also drop MIGRATE_CMA and >>> related hooks? Is that counter really that useful as it works right now? >>> It will decrease both by CMA allocations (which has to be explicitly >>> freed) and by movable allocations (which can be migrated). What if only >>> CMA alloc/release touched it? >> >> I think that NR_FREE_CMA_PAGES would not be as useful as previous. We >> can remove it. >> >> However, removing MIGRATE_CMA has a problem. There is an usecase to >> check if the page comes from the CMA area or not. See >> check_page_span() in mm/usercopy.c. I can implement it differently by >> iterating whole CMA area and finding the match, but I'm not sure it's >> performance effect. I guess that it would be marginal. > > +CC Kees Cook > > Hmm, seems like this check is to make sure we don't copy from/to parts > of kernel memory we're not supposed to? Then I believe checking that > pages are in ZONE_MOVABLE should then give the same guarantees as > MIGRATE_CMA. Yeah, as Laura said, the idea is to make sure that a copy doesn't exceed the bounds of the allocation (and that means a single page when not __GFP_COMP nor CMA nor Reserved). The trouble with this check, which I'd like see fixed, is that there are portions of the kernel that make separate adjacent page allocations and then copy across individual allocations in a single usercopy. It's not clear to me if that is fixable just by adding __GFP_COMP or not, though. -Kees -- Kees Cook Pixel Security