Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753541AbdHWHuO convert rfc822-to-8bit (ORCPT ); Wed, 23 Aug 2017 03:50:14 -0400 Received: from tyo161.gate.nec.co.jp ([114.179.232.161]:55273 "EHLO tyo161.gate.nec.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753360AbdHWHuM (ORCPT ); Wed, 23 Aug 2017 03:50:12 -0400 From: Naoya Horiguchi To: Zi Yan CC: Greg Kroah-Hartman , "Kirill A . Shutemov" , "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "Zi Yan" Subject: Re: [RFC PATCH 1/4] mm: madvise: read loop's step size beforehand in madvise_inject_error(), prepare for THP support. Thread-Topic: [RFC PATCH 1/4] mm: madvise: read loop's step size beforehand in madvise_inject_error(), prepare for THP support. Thread-Index: AQHTFWkxcFWra770dUSZM2UhfIHOyKKRBweA Date: Wed, 23 Aug 2017 07:49:36 +0000 Message-ID: <20170823074933.GA3527@hori1.linux.bs1.fc.nec.co.jp> References: <20170815015216.31827-1-zi.yan@sent.com> <20170815015216.31827-2-zi.yan@sent.com> In-Reply-To: <20170815015216.31827-2-zi.yan@sent.com> Accept-Language: en-US, ja-JP Content-Language: ja-JP X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.128.101.16] Content-Type: text/plain; charset="iso-2022-jp" Content-ID: <38E2DBFF1986C04786357EDC7E522137@gisp.nec.co.jp> Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-TM-AS-MML: disable Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3349 Lines: 103 On Mon, Aug 14, 2017 at 09:52:13PM -0400, Zi Yan wrote: > From: Zi Yan > > The loop in madvise_inject_error() reads its step size from a page > after it is soft-offlined. It works because the page is: > 1) a hugetlb page: the page size does not change; > 2) a base page: the page size does not change; > 3) a THP: soft-offline always splits THPs, thus, it is OK to use > PAGE_SIZE as step size. > > It will be a problem when soft-offline supports THP migrations. > When a THP is migrated without split during soft-offlining, the THP > is split after migration, thus, before and after soft-offlining page > sizes do not match. This causes a THP to be unnecessarily soft-lined, > at most, 511 times, wasting free space. Hi Zi Yan, Thank you for the suggestion. I think that when madvise(MADV_SOFT_OFFLINE) is called with some range over more than one 4kB page, the caller clearly intends to call soft_offline_page() over all 4kB pages within the range in order to simulate the multiple soft-offline events. Please note that the caller only knows that specific pages are half-broken, and expect that all such pages are offlined. So the end result should be same, whether the given range is backed by thp or not. > > Signed-off-by: Zi Yan > --- > mm/madvise.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/mm/madvise.c b/mm/madvise.c > index 47d8d8a25eae..49f6774db259 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -612,19 +612,22 @@ static long madvise_remove(struct vm_area_struct *vma, > static int madvise_inject_error(int behavior, > unsigned long start, unsigned long end) > { > - struct page *page; > + struct page *page = NULL; > + unsigned long page_size = PAGE_SIZE; > > if (!capable(CAP_SYS_ADMIN)) > return -EPERM; > > - for (; start < end; start += PAGE_SIZE << > - compound_order(compound_head(page))) { > + for (; start < end; start += page_size) { > int ret; > > ret = get_user_pages_fast(start, 1, 0, &page); > if (ret != 1) > return ret; > > + page_size = (PAGE_SIZE << compound_order(compound_head(page))) - > + (PAGE_SIZE * (page - compound_head(page))); > + Assigning a value which is not 4kB or some hugepage size into page_size might be confusing because that's not what the name says. You can introduce 'next' virtual address and ALIGN() might be helpful to calculate it. Thanks, Naoya Horiguchi > if (PageHWPoison(page)) { > put_page(page); > continue; > @@ -637,6 +640,12 @@ static int madvise_inject_error(int behavior, > ret = soft_offline_page(page, MF_COUNT_INCREASED); > if (ret) > return ret; > + /* > + * Non hugetlb pages either have PAGE_SIZE > + * or are split into PAGE_SIZE > + */ > + if (!PageHuge(page)) > + page_size = PAGE_SIZE; > continue; > } > pr_info("Injecting memory failure for pfn %#lx at process virtual address %#lx\n", > @@ -645,6 +654,12 @@ static int madvise_inject_error(int behavior, > ret = memory_failure(page_to_pfn(page), 0, MF_COUNT_INCREASED); > if (ret) > return ret; > + /* > + * Non hugetlb pages either have PAGE_SIZE > + * or are split into PAGE_SIZE > + */ > + if (!PageHuge(page)) > + page_size = PAGE_SIZE; > } > return 0; > } > -- > 2.13.2 > >