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=-14.0 required=3.0 tests=BAYES_00,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham 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 53C7DC4338F for ; Tue, 24 Aug 2021 02:14:07 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C940161361 for ; Tue, 24 Aug 2021 02:14:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org C940161361 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 220266B006C; Mon, 23 Aug 2021 22:14:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 1CF5B8D0001; Mon, 23 Aug 2021 22:14:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0706C6B0072; Mon, 23 Aug 2021 22:14:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0032.hostedemail.com [216.40.44.32]) by kanga.kvack.org (Postfix) with ESMTP id DE3126B006C for ; Mon, 23 Aug 2021 22:14:05 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 800081C9AC for ; Tue, 24 Aug 2021 02:14:05 +0000 (UTC) X-FDA: 78508354050.10.CEE5C11 Received: from mail-qv1-f44.google.com (mail-qv1-f44.google.com [209.85.219.44]) by imf14.hostedemail.com (Postfix) with ESMTP id 3F1806001980 for ; Tue, 24 Aug 2021 02:14:05 +0000 (UTC) Received: by mail-qv1-f44.google.com with SMTP id q6so10876455qvs.12 for ; Mon, 23 Aug 2021 19:14:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=ya8SlNQty6mh8457AcvIZ0Nm3y/y4F76ffUOZipML+g=; b=L1vfWxYF1DD+KXmgE4r5w3CVbxZmOHmxFzbhAC8T/ycndVVOULcvyTCFqA22+4VbQe +liTWGp3ObDVxZxMcb+O4+9QUS+9YXtnZhl6QD01QqCPyoTdHRjnoE4MJlPzKrbRrNjR t/5m0qmQZtmtGD5q/N/LiTI/OsmALmvaoOOu86+f4j00Rzt5kzOP89OFDM+FhFdA1CET nfhOZn8M2KPsuP9Le6jivasD3NlXSSV/433CFVnqdmpVUHmU9HHlD82lX2IX3Zir2EOM xTqkXtyIyOgj9cwl2nwPaa1mY5T+aHMOVokQwmWvNCF4L8aghzUw+uW3+trBPwSW468z G1Gg== X-Gm-Message-State: AOAM533KRp8CTdVDF9sxGY0bxEmXCKolCH5182BpUrTNCJQwDArMerf2 jCW3XNFmldsyc1eSR9GhahE= X-Google-Smtp-Source: ABdhPJxQ1I/xffrIBH6ZM+mqHO9342ko5BbBIy58xf3OF33L+6GgLhzMQNmzCSGIRcydxSH+XUahBg== X-Received: by 2002:a05:6214:ca2:: with SMTP id s2mr36569213qvs.35.1629771244512; Mon, 23 Aug 2021 19:14:04 -0700 (PDT) Received: from fedora (pool-173-68-57-129.nycmny.fios.verizon.net. [173.68.57.129]) by smtp.gmail.com with ESMTPSA id g20sm9599605qki.73.2021.08.23.19.14.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Aug 2021 19:14:03 -0700 (PDT) Date: Mon, 23 Aug 2021 22:14:02 -0400 From: Dennis Zhou To: Ritesh Harjani Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Tejun Heo , Christoph Lameter , "Aneesh Kumar K . V" , Vaibhav Jain , kernel test robot Subject: Re: [PATCHv2 2/2] lib/percpu_test: Add extra tests in percpu_test Message-ID: References: <5afc2a0c4da65e71ccf24fe65396710d34fc662e.1629751104.git.riteshh@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Authentication-Results: imf14.hostedemail.com; dkim=none; dmarc=fail reason="SPF not aligned (relaxed), No valid DKIM" header.from=kernel.org (policy=none); spf=pass (imf14.hostedemail.com: domain of dennisszhou@gmail.com designates 209.85.219.44 as permitted sender) smtp.mailfrom=dennisszhou@gmail.com X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: 3F1806001980 X-Stat-Signature: cj39awj8q718t8a5sufn3ep5jqbrqg3c X-HE-Tag: 1629771245-178217 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: Hello, On Tue, Aug 24, 2021 at 02:12:30AM +0530, Ritesh Harjani wrote: > While debugging a issue, we needed to stress test the percpu alloc/free > path. Hence added some tests in lib/percpu_test to stress test > percpu subsystem for allocation with different sizes. > Can you explain more about the problem you are trying to debug and why it required stressing the percpu allocator? > This patch keeps the default behavior of insmod module same for default > test. But when given insmod with different option, it can run a > percpu_stressd daemon (percpu_test_num=2) which does a stress test > evey 10secs unless the module is unloaded. > > We found this to be helpful in our testing, since with this we could > easily excercise percpu allo/free path. Hence cleaned this up for > inclusion in percpu_test module. > > Logs > ====== > qemu-> sudo insmod /mnt/percpu_test.ko percpu_test_num=0 > [ 334.362973] percpu_test: INIT, interval: 1000, max_shift: 13, run_tests: percpu_verify > [ 334.364946] TEST Starts: percpu_verify > [ 334.365601] TEST Completed: percpu_verify > insmod: ERROR: could not insert module /mnt/percpu_test.ko: Resource temporarily unavailable ^ this seems wrong? What am I missing. > > qemu-> sudo insmod /mnt/percpu_test.ko percpu_test_num=1 > [ 336.556464] percpu_test: INIT, interval: 1000, max_shift: 13, run_tests: percpu_stress > [ 336.558388] TEST Starts: percpu_stress > [ 336.560611] TEST Completed: percpu_stress > insmod: ERROR: could not insert module /mnt/percpu_test.ko: Resource temporarily unavailable > > qemu-> sudo insmod /mnt/percpu_test.ko percpu_test_num=2 > [ 339.164406] percpu_test: INIT, interval: 1000, max_shift: 13, run_tests: percpu_stressd > [ 339.165935] TEST Starts: percpu_stressd > [ 339.167033] TEST Completed: percpu_stressd > [ 339.167082] DAEMON: starts percpu_stressd > [ 339.168498] TEST Starts: percpu_stressd: iter (1) > [ 339.182530] TEST Completed: percpu_stressd: iter (1) > [ 349.341109] TEST Starts: percpu_stressd: iter (2) > [ 349.344447] TEST Completed: percpu_stressd: iter (2) > [ 359.580829] TEST Starts: percpu_stressd: iter (3) > [ 359.584315] TEST Completed: percpu_stressd: iter (3) > [ 369.820471] TEST Starts: percpu_stressd: iter (4) > [ 369.844402] TEST Completed: percpu_stressd: iter (4) > > qemu-> sudo rmmod percpu_test > [ 375.001098] percpu_test: EXIT > [qemu][~] > > Cc: Aneesh Kumar K.V > Cc: Vaibhav Jain > Reported-by: kernel test robot > Signed-off-by: Ritesh Harjani > --- > [v1 -> v2]: Fix warnings from kernel test robot > > lib/percpu_test.c | 240 ++++++++++++++++++++++++++++++++++++---------- > 1 file changed, 191 insertions(+), 49 deletions(-) > > diff --git a/lib/percpu_test.c b/lib/percpu_test.c > index 4a3d70bbc1a0..68c57c288dc6 100644 > --- a/lib/percpu_test.c > +++ b/lib/percpu_test.c > @@ -1,4 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0-only > +#include > +#include > +#include > #include > > /* validate @native and @pcp counter values match @expected */ > @@ -14,10 +17,25 @@ > (long long)(expected), (long long)(expected)); \ > } while (0) > > -static DEFINE_PER_CPU(long, long_counter); > -static DEFINE_PER_CPU(unsigned long, ulong_counter); ^ These are good to keep fwiw as it's one of the few instances of using the reserved region of the first chunk of percpu memory. > +/* upto max alloc size tests for percpu var */ > +static char __percpu *counters[1 << PAGE_SHIFT]; > +static struct task_struct *percpu_stressd_thread; > > -static int __init percpu_test_init(void) > +/* let's not trigger OOM */ > +int percpu_alloc_max_size_shift = PAGE_SHIFT - 3; > +module_param(percpu_alloc_max_size_shift, int, 0644); > +MODULE_PARM_DESC(percpu_alloc_max_size_shift, "max size of allocation in stress test will be upto 1 << percpu_alloc_max_size_shift"); > + > +static long percpu_stressd_interval = 1 * 10 * HZ; > +module_param(percpu_stressd_interval, long, 0644); > +MODULE_PARM_DESC(percpu_stressd_interval, "percpu_stressd internal"); > + > +/* keep the default test same */ > +static int percpu_test_num; > +module_param(percpu_test_num, int, 0644); > +MODULE_PARM_DESC(percpu_test_num, "Test number percpu_test_num"); > + > +static int percpu_test_verify(void) > { > /* > * volatile prevents compiler from optimizing it uses, otherwise the > @@ -26,109 +44,233 @@ static int __init percpu_test_init(void) > volatile unsigned int ui_one = 1; > long l = 0; > unsigned long ul = 0; > + long __percpu *long_counter = alloc_percpu(long); > + unsigned long __percpu *ulong_counter = alloc_percpu(unsigned long); > > - pr_info("percpu test start\n"); > + if (!long_counter || !ulong_counter) > + goto out; > + > + pr_debug("percpu_test: %s start cpu: %d\n", __func__, smp_processor_id()); > > preempt_disable(); > > l += -1; > - __this_cpu_add(long_counter, -1); > - CHECK(l, long_counter, -1); > + __this_cpu_add(*long_counter, -1); > + CHECK(l, *long_counter, -1); > > l += 1; > - __this_cpu_add(long_counter, 1); > - CHECK(l, long_counter, 0); > + __this_cpu_add(*long_counter, 1); > + CHECK(l, *long_counter, 0); > > ul = 0; > - __this_cpu_write(ulong_counter, 0); > + __this_cpu_write(*ulong_counter, 0); > > ul += 1UL; > - __this_cpu_add(ulong_counter, 1UL); > - CHECK(ul, ulong_counter, 1); > + __this_cpu_add(*ulong_counter, 1UL); > + CHECK(ul, *ulong_counter, 1); > > ul += -1UL; > - __this_cpu_add(ulong_counter, -1UL); > - CHECK(ul, ulong_counter, 0); > + __this_cpu_add(*ulong_counter, -1UL); > + CHECK(ul, *ulong_counter, 0); > > ul += -(unsigned long)1; > - __this_cpu_add(ulong_counter, -(unsigned long)1); > - CHECK(ul, ulong_counter, -1); > + __this_cpu_add(*ulong_counter, -(unsigned long)1); > + CHECK(ul, *ulong_counter, -1); > > ul = 0; > - __this_cpu_write(ulong_counter, 0); > + __this_cpu_write(*ulong_counter, 0); > > ul -= 1; > - __this_cpu_dec(ulong_counter); > - CHECK(ul, ulong_counter, -1); > - CHECK(ul, ulong_counter, ULONG_MAX); > + __this_cpu_dec(*ulong_counter); > + CHECK(ul, *ulong_counter, -1); > + CHECK(ul, *ulong_counter, ULONG_MAX); > > l += -ui_one; > - __this_cpu_add(long_counter, -ui_one); > - CHECK(l, long_counter, 0xffffffff); > + __this_cpu_add(*long_counter, -ui_one); > + CHECK(l, *long_counter, 0xffffffff); > > l += ui_one; > - __this_cpu_add(long_counter, ui_one); > - CHECK(l, long_counter, (long)0x100000000LL); > + __this_cpu_add(*long_counter, ui_one); > + CHECK(l, *long_counter, (long)0x100000000LL); > > > l = 0; > - __this_cpu_write(long_counter, 0); > + __this_cpu_write(*long_counter, 0); > > l -= ui_one; > - __this_cpu_sub(long_counter, ui_one); > - CHECK(l, long_counter, -1); > + __this_cpu_sub(*long_counter, ui_one); > + CHECK(l, *long_counter, -1); > > l = 0; > - __this_cpu_write(long_counter, 0); > + __this_cpu_write(*long_counter, 0); > > l += ui_one; > - __this_cpu_add(long_counter, ui_one); > - CHECK(l, long_counter, 1); > + __this_cpu_add(*long_counter, ui_one); > + CHECK(l, *long_counter, 1); > > l += -ui_one; > - __this_cpu_add(long_counter, -ui_one); > - CHECK(l, long_counter, (long)0x100000000LL); > + __this_cpu_add(*long_counter, -ui_one); > + CHECK(l, *long_counter, (long)0x100000000LL); > > l = 0; > - __this_cpu_write(long_counter, 0); > + __this_cpu_write(*long_counter, 0); > > l -= ui_one; > - this_cpu_sub(long_counter, ui_one); > - CHECK(l, long_counter, -1); > - CHECK(l, long_counter, ULONG_MAX); > + this_cpu_sub(*long_counter, ui_one); > + CHECK(l, *long_counter, -1); > + CHECK(l, *long_counter, ULONG_MAX); > > ul = 0; > - __this_cpu_write(ulong_counter, 0); > + __this_cpu_write(*ulong_counter, 0); > > ul += ui_one; > - __this_cpu_add(ulong_counter, ui_one); > - CHECK(ul, ulong_counter, 1); > + __this_cpu_add(*ulong_counter, ui_one); > + CHECK(ul, *ulong_counter, 1); > > ul = 0; > - __this_cpu_write(ulong_counter, 0); > + __this_cpu_write(*ulong_counter, 0); > > ul -= ui_one; > - __this_cpu_sub(ulong_counter, ui_one); > - CHECK(ul, ulong_counter, -1); > - CHECK(ul, ulong_counter, ULONG_MAX); > + __this_cpu_sub(*ulong_counter, ui_one); > + CHECK(ul, *ulong_counter, -1); > + CHECK(ul, *ulong_counter, ULONG_MAX); > > ul = 3; > - __this_cpu_write(ulong_counter, 3); > + __this_cpu_write(*ulong_counter, 3); > > - ul = this_cpu_sub_return(ulong_counter, ui_one); > - CHECK(ul, ulong_counter, 2); > + ul = this_cpu_sub_return(*ulong_counter, ui_one); > + CHECK(ul, *ulong_counter, 2); > > - ul = __this_cpu_sub_return(ulong_counter, ui_one); > - CHECK(ul, ulong_counter, 1); > + ul = __this_cpu_sub_return(*ulong_counter, ui_one); > + CHECK(ul, *ulong_counter, 1); > > preempt_enable(); > > - pr_info("percpu test done\n"); > - return -EAGAIN; /* Fail will directly unload the module */ > +out: > + free_percpu(long_counter); > + free_percpu(ulong_counter); > + pr_debug("percpu_test: %s done cpu: %d\n", __func__, smp_processor_id()); > + > + /* > + * Keep the default functionality same. > + * Fail will directly unload this module. > + */ > + return -EAGAIN; > +} > + > +static void percpu_test_verify_work(struct work_struct *work) > +{ > + percpu_test_verify(); > +} > + > +static int percpu_test_stress(void) > +{ > + int i; > + > + for (i = 1; i < (1 << percpu_alloc_max_size_shift); i++) { > + size_t size = i; > + > + if (size > PCPU_MIN_ALLOC_SIZE) > + break; > + counters[i] = (char __percpu *)__alloc_percpu(size, __alignof__(char)); > + if (!counters[i]) > + break; > + cond_resched(); > + } > + > + schedule_on_each_cpu(percpu_test_verify_work); I'm not understanding the value of scheduling on each cpu here? Each CHECK() call is accessing the value via __this_cpu_read() meaning it has nothing to do with any other cpu's copy of the variable? > + > + for (i = 0; i < (1 << percpu_alloc_max_size_shift); i++) { > + free_percpu(counters[i]); > + cond_resched(); > + } > + return -EAGAIN; > +} > + > +static int percpu_stressd(void *v) > +{ > + int iter = 0; > + > + pr_info("DAEMON: starts %s\n", __func__); > + do { > + if (kthread_should_stop()) > + break; > + iter++; > + pr_info("TEST Starts: %s: iter (%d)\n", __func__, iter); > + percpu_test_stress(); > + pr_info("TEST Completed: %s: iter (%d)\n", __func__, iter); > + > + set_current_state(TASK_INTERRUPTIBLE); > + schedule_timeout(percpu_stressd_interval); > + } while (1); > + > + return 0; > +} > + > +static int percpu_test_stressd(void) > +{ > + percpu_stressd_thread = kthread_run(percpu_stressd, NULL, "percpu_stressd"); > + if (IS_ERR(percpu_stressd_thread)) > + percpu_stressd_thread = NULL; > + return 0; > +} > + > +enum test_type { > + PERCPU_VERIFY, > + PERCPU_STRESS, > + PERCPU_STRESSD, > + NR_TESTS, > +}; > + > +const char *test_names[NR_TESTS] = { > + [PERCPU_VERIFY] = "percpu_verify", > + [PERCPU_STRESS] = "percpu_stress", > + [PERCPU_STRESSD] = "percpu_stressd", > +}; > + > +static int __init percpu_test_init(void) > +{ > + int i, ret = 0; > + typedef int (*percpu_tests)(void); > + const percpu_tests test_funcs[NR_TESTS] = { > + [PERCPU_VERIFY] = percpu_test_verify, > + [PERCPU_STRESS] = percpu_test_stress, > + [PERCPU_STRESSD] = percpu_test_stressd, > + }; > + > + /* sanity checks */ > + if (percpu_alloc_max_size_shift > PAGE_SHIFT) > + percpu_alloc_max_size_shift = PAGE_SHIFT; > + if (percpu_test_num > NR_TESTS) > + percpu_test_num = NR_TESTS; > + > + pr_info("percpu_test: INIT, interval: %ld, max_shift: %d, run_tests: %s\n", > + percpu_stressd_interval, percpu_alloc_max_size_shift, > + percpu_test_num == NR_TESTS ? "run all tests" : > + test_names[percpu_test_num]); > + > + /* run a given test */ > + if (percpu_test_num < NR_TESTS) { > + pr_info("TEST Starts: %s\n", test_names[percpu_test_num]); > + ret = test_funcs[percpu_test_num](); > + pr_info("TEST Completed: %s\n", test_names[percpu_test_num]); > + goto out; > + } > + > + for (i = 0; i < NR_TESTS; i++) { > + pr_info("TEST Starts: %s\n", test_names[i]); > + test_funcs[i](); > + pr_info("TEST Completed: %s\n", test_names[i]); > + } > +out: > + return ret; > } > > static void __exit percpu_test_exit(void) > { > + if (percpu_stressd_thread) > + kthread_stop(percpu_stressd_thread); > + pr_info("percpu_test: EXIT\n"); > } > > module_init(percpu_test_init) > -- > 2.31.1 > Thanks, Dennis