Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752586AbbEFHzk (ORCPT ); Wed, 6 May 2015 03:55:40 -0400 Received: from cantor2.suse.de ([195.135.220.15]:33539 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751410AbbEFHzj (ORCPT ); Wed, 6 May 2015 03:55:39 -0400 Message-ID: <5549C8FB.7080404@suse.cz> Date: Wed, 06 May 2015 09:55:39 +0200 From: Vlastimil Babka User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Joonsoo Kim , Hui Zhu CC: akpm@linux-foundation.org, lauraa@codeaurora.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, teawater@gmail.com Subject: Re: [PATCH v2] CMA: page_isolation: check buddy before access it References: <1430732477-16977-1-git-send-email-zhuhui@xiaomi.com> <1430796179-1795-1-git-send-email-zhuhui@xiaomi.com> <20150506062801.GA12737@js1304-P5Q-DELUXE> In-Reply-To: <20150506062801.GA12737@js1304-P5Q-DELUXE> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1665 Lines: 44 On 6.5.2015 8:28, Joonsoo Kim wrote: > On Tue, May 05, 2015 at 11:22:59AM +0800, Hui Zhu wrote: >> >> diff --git a/mm/page_isolation.c b/mm/page_isolation.c >> index 755a42c..eb22d1f 100644 >> --- a/mm/page_isolation.c >> +++ b/mm/page_isolation.c >> @@ -101,7 +101,8 @@ void unset_migratetype_isolate(struct page *page, unsigned migratetype) >> buddy_idx = __find_buddy_index(page_idx, order); >> buddy = page + (buddy_idx - page_idx); >> >> - if (!is_migrate_isolate_page(buddy)) { >> + if (!pfn_valid_within(page_to_pfn(buddy)) >> + || !is_migrate_isolate_page(buddy)) { >> __isolate_free_page(page, order); >> kernel_map_pages(page, (1 << order), 1); >> set_page_refcounted(page); > > Hello, > > This isolation is for merging buddy pages. If buddy is not valid, we > don't need to isolate page, because we can't merge them. > I think that correct code would be: > > pfn_valid_within(page_to_pfn(buddy)) && > !is_migrate_isolate_page(buddy) > > But, isolation and free here is safe operation so your code will work > fine. Ah damnit, you're right. But now you got me thinking about it more, and paranoid... I thought I saw more bugs since the buddy might be in different zone and we are not locking that zone, but then again it's probably fine, just very tricky. Then I thought it could be simplified but then not again. Guess I'll just run away fast :) > Thanks. > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/