From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8E421C433E0 for ; Thu, 7 Jan 2021 03:29:40 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 11C00230F9 for ; Thu, 7 Jan 2021 03:29:39 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 11C00230F9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 21D006B02A4; Wed, 6 Jan 2021 22:29:39 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 1CEA16B02A7; Wed, 6 Jan 2021 22:29:39 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0E55A6B02A9; Wed, 6 Jan 2021 22:29:39 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0048.hostedemail.com [216.40.44.48]) by kanga.kvack.org (Postfix) with ESMTP id ED5536B02A4 for ; Wed, 6 Jan 2021 22:29:38 -0500 (EST) Received: from smtpin21.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 909F6181AEF32 for ; Thu, 7 Jan 2021 03:29:38 +0000 (UTC) X-FDA: 77677549236.21.book82_130b029274e7 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin21.hostedemail.com (Postfix) with ESMTP id 6FBD6180442C0 for ; Thu, 7 Jan 2021 03:29:38 +0000 (UTC) X-HE-Tag: book82_130b029274e7 X-Filterd-Recvd-Size: 8650 Received: from mail-lf1-f47.google.com (mail-lf1-f47.google.com [209.85.167.47]) by imf23.hostedemail.com (Postfix) with ESMTP for ; Thu, 7 Jan 2021 03:29:37 +0000 (UTC) Received: by mail-lf1-f47.google.com with SMTP id a12so11437222lfl.6 for ; Wed, 06 Jan 2021 19:29:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=67gzgMad2skc1/SGymVxxTd69u2O0I4adnxIqqBXpBk=; b=Iab1xh5YDex6NeMo0jSanWjTZkGUrSHm5FNKTIqGn87sEdm5HChnPAsV86glkoBGh7 yp2f3aKrhqlJeFEajcKLM6nuMtdirwokft4gWA+EjiYf/NtGaoLKknAD6OyR6o+uMyEI ZwszVCqL+m1i1ys1WEt8NgE1LnsGLSdWqN4k8Cf/f/KhzsuHfmqX18oBA4VlVyVR8eKs P5BQ7BblI1lFcs9WwiKU5yM18uz2EGcRkdnuURNGLaFoq9BVtPNSpagbzV2ZkoBl5NX0 slCI+JG4EKkYc3Pepu8NIbmGQDD40lzdFvbr9hJVL7xUTsldRyjOi0mjpXWDHSQA4/gS 54Pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=67gzgMad2skc1/SGymVxxTd69u2O0I4adnxIqqBXpBk=; b=UbsDXkYRg21ELT92JxJhlaW7Id+a86vfjhN7ygpDTBNUMKbiajOq8pgI3ROPqJNb70 P8DTl56EuRsmcC0f6pTiTiU7zp4YTrAurzVU36ffh+MZYPBiBOkfvXn8xPOQlpKsGjlW RX7uVwooe+QqhSRahIuetSiODwECpDkfS1iuhVPCJuJKStLEHeP7cqFnBtUrCoa0baMs MkkgEEDeADh6Hvt1LMkEJKai/1LMS8qZtm+INQC7lGtsa0ptdxfUC9zE1egf14ihBjRt 0lLFr7AZqBVvif55fUpe3QPzS6735HpWzkbF5XpklJFQe6FXLnUH2DoQ7huX3LTXSA1n 8wSw== X-Gm-Message-State: AOAM532BCZOVA/XXGNlfK5ysLzOS7SJ0RruxEBgzWM4jJCbmROuoIU7k 9siWDYCk/9y6MVfc9RcKq143OE5+UMHMfDEm8kc= X-Google-Smtp-Source: ABdhPJyHS3QC+4obdLCpe4zevaRNhFS19Ou7PmVG1j9dhtYp6TUL8HCz7XBtLdeHtIp3qfkNrj23Wbw3aLJnC7nLZGg= X-Received: by 2002:a19:814c:: with SMTP id c73mr2886142lfd.638.1609990175893; Wed, 06 Jan 2021 19:29:35 -0800 (PST) MIME-Version: 1.0 References: <20210106034806.GA1146@open-light-1.localdomain> In-Reply-To: From: Liang Li Date: Thu, 7 Jan 2021 11:29:22 +0800 Message-ID: Subject: Re: [PATCH 2/6] mm: let user decide page reporting option To: Alexander Duyck Cc: Alexander Duyck , Mel Gorman , Andrew Morton , Andrea Arcangeli , Dan Williams , "Michael S. Tsirkin" , David Hildenbrand , Jason Wang , Dave Hansen , Michal Hocko , Liang Li , Mike Kravetz , linux-mm , LKML , virtualization@lists.linux-foundation.org Content-Type: text/plain; charset="UTF-8" X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: > > enum { > > PAGE_REPORTING_IDLE = 0, > > @@ -44,7 +45,7 @@ __page_reporting_request(struct page_reporting_dev_info *prdev) > > * now we are limiting this to running no more than once every > > * couple of seconds. > > */ > > - schedule_delayed_work(&prdev->work, PAGE_REPORTING_DELAY); > > + schedule_delayed_work(&prdev->work, prdev->delay_jiffies); > > } > > > > So this ends up being the reason why you needed to add the batch size > value. However I don't really see it working as expected since you > could essentially have 1 page freed 4M times that could trigger your > page zeroing logic. So for example if a NIC is processing frames and > ends up freeing and then reallocating some small batch of pages this > could would be running often even though there isn't really all that > many pages that needed zeroing. Good catch, it works not like batch size means. > > /* notify prdev of free page reporting request */ > > @@ -230,7 +231,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev, > > > > /* Generate minimum watermark to be able to guarantee progress */ > > watermark = low_wmark_pages(zone) + > > - (PAGE_REPORTING_CAPACITY << PAGE_REPORTING_MIN_ORDER); > > + (PAGE_REPORTING_CAPACITY << prdev->mini_order); > > > > /* > > * Cancel request if insufficient free memory or if we failed > > With the page order being able to be greatly reduced this could have a > significant impact on if this code really has any value. Previously we > were able to guarantee a pretty significant number of higher order > pages free. With this we might only be guaranteeing something like 32 > 4K pages which is pretty small compared to what can end up being > pulled out at the higher end. I have dropped the 'buddy free page pre zero' patch, so the mini order will not change to a small value. > > @@ -240,7 +241,7 @@ page_reporting_process_zone(struct page_reporting_dev_info *prdev, > > return err; > > > > /* Process each free list starting from lowest order/mt */ > > - for (order = PAGE_REPORTING_MIN_ORDER; order < MAX_ORDER; order++) { > > + for (order = prdev->mini_order; order < MAX_ORDER; order++) { > > for (mt = 0; mt < MIGRATE_TYPES; mt++) { > > /* We do not pull pages from the isolate free list */ > > if (is_migrate_isolate(mt)) > > @@ -307,7 +308,7 @@ static void page_reporting_process(struct work_struct *work) > > */ > > state = atomic_cmpxchg(&prdev->state, state, PAGE_REPORTING_IDLE); > > if (state == PAGE_REPORTING_REQUESTED) > > - schedule_delayed_work(&prdev->work, PAGE_REPORTING_DELAY); > > + schedule_delayed_work(&prdev->work, prdev->delay_jiffies); > > } > > > > static DEFINE_MUTEX(page_reporting_mutex); > > @@ -335,6 +336,8 @@ int page_reporting_register(struct page_reporting_dev_info *prdev) > > /* Assign device to allow notifications */ > > rcu_assign_pointer(pr_dev_info, prdev); > > > > + page_report_mini_order = prdev->mini_order; > > + page_report_batch_size = prdev->batch_size; > > /* enable page reporting notification */ > > if (!static_key_enabled(&page_reporting_enabled)) { > > static_branch_enable(&page_reporting_enabled); > > @@ -352,6 +355,8 @@ void page_reporting_unregister(struct page_reporting_dev_info *prdev) > > mutex_lock(&page_reporting_mutex); > > > > if (rcu_access_pointer(pr_dev_info) == prdev) { > > + if (static_key_enabled(&page_reporting_enabled)) > > + static_branch_disable(&page_reporting_enabled); > > /* Disable page reporting notification */ > > RCU_INIT_POINTER(pr_dev_info, NULL); > > synchronize_rcu(); > > If we are going to use this we are using it. Once we NULL out the > prdev that should stop page reporting from running. We shouldn't be > relying on the static key. The benefits for this is that the function call of '__page_reporting_notify' in 'page_reporting_notify_free' can be skipped, it helps to save some cycles. > > diff --git a/mm/page_reporting.h b/mm/page_reporting.h > > index b8fb3bbb345f..86ac6ffad970 100644 > > --- a/mm/page_reporting.h > > +++ b/mm/page_reporting.h > > @@ -9,9 +9,9 @@ > > #include > > #include > > #include > > +#include > > > > -#define PAGE_REPORTING_MIN_ORDER pageblock_order > > - > > +extern int page_report_mini_order; > > extern unsigned long page_report_batch_size; > > > > #ifdef CONFIG_PAGE_REPORTING > > @@ -42,7 +42,7 @@ static inline void page_reporting_notify_free(unsigned int order) > > return; > > > > /* Determine if we have crossed reporting threshold */ > > - if (order < PAGE_REPORTING_MIN_ORDER) > > + if (order < page_report_mini_order) > > return; > > This is another case where it might make sense to look at placing the > code in __page_reporting_notify if we are going to allow this to be a > dynamically configurable value. I will change it in the next revision. Thanks! Liang