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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 41FF8C83F17 for ; Tue, 15 Jul 2025 20:09:20 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D68616B0096; Tue, 15 Jul 2025 16:09:19 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D3FE86B0099; Tue, 15 Jul 2025 16:09:19 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C7C866B009A; Tue, 15 Jul 2025 16:09:19 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id B9D2D6B0096 for ; Tue, 15 Jul 2025 16:09:19 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 4AC6B1A01B7 for ; Tue, 15 Jul 2025 20:09:19 +0000 (UTC) X-FDA: 83667588438.27.AEACBD3 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf18.hostedemail.com (Postfix) with ESMTP id A158B1C000D for ; Tue, 15 Jul 2025 20:09:17 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=TU2D23OA; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf18.hostedemail.com: domain of namhyung@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=namhyung@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1752610157; a=rsa-sha256; cv=none; b=ROtl625wvAhG3TOKCfeiXKmo4G74hqMXuMbvovlaG8xRDBHesb0v+TAXWUTxLHlzJigU1g ZRy+FtxIjABKYNf8pPuNuhMkSpxnS/XYWyE8qpQ6zOZj03IFr99GnAiXg6PBZNvlvTQsIg hO3LWUwVJGR/MkCEv/aGqTQvpwev/JM= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=TU2D23OA; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf18.hostedemail.com: domain of namhyung@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=namhyung@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1752610157; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=t3e6II1IAb88C+jJkAPXbSIOYFBcISRlvJ38bfeZj08=; b=eZ5trgTC7v/3fRvgNpMk+iimy151T0ozLenVic/7lHWjc6lSaxCbd3S9ZnbFrclY612yIb J+eqL3n+YNbI4DHIeSktYSVH8gicH5HCCV1EYFaCMumDkPPbjmst684BaxY0iMRW4GtYTV p1uUkfsODc/c5/dtTunXULg+Nm11k5M= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id AEE6A5C64DE; Tue, 15 Jul 2025 20:09:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 91BD7C4CEE3; Tue, 15 Jul 2025 20:09:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1752610156; bh=pTKBkYQxvCJDM7hqD1VYZUb+zcoxH0Ug4ny+3/JvRhc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=TU2D23OAqudCoNC4V8XWPYNNIjzy6FGC8nqR25Kxg7VgDawOAK1O3lfmWdLC30Wp5 BKTqCdbpmLyDzha6pYAFXZX/7in3ZLjRsHM+GyamQL1Bb7Wn1XcCoYmGW8yneXQAW6 +yHXSlJGCqUYSHtb5akY2bQXjhNrEGZJO+iCBuwRNukU2shBv5BaeIbhrfAu+X5WSj 3ayHxiUyZJ8KycRteji/XFpcX05vyj0O/TB3oA4y79M9z4JwoLxL3VvuqQ7W1eoo6Y esa5xXwSW4HQMajyD0A8k3NTDCreyD7A1JxfXTDF3zyzr4x6PliWx73pzU56hce9UE q73YunaeeSZhg== Date: Tue, 15 Jul 2025 13:09:14 -0700 From: Namhyung Kim To: Ankur Arora Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org, x86@kernel.org, akpm@linux-foundation.org, david@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com, mingo@redhat.com, mjguzik@gmail.com, luto@kernel.org, peterz@infradead.org, acme@kernel.org, tglx@linutronix.de, willy@infradead.org, raghavendra.kt@amd.com, boris.ostrovsky@oracle.com, konrad.wilk@oracle.com Subject: Re: [PATCH v5 04/14] perf bench mem: Pull out init/fini logic Message-ID: References: <20250710005926.1159009-1-ankur.a.arora@oracle.com> <20250710005926.1159009-5-ankur.a.arora@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20250710005926.1159009-5-ankur.a.arora@oracle.com> X-Rspamd-Queue-Id: A158B1C000D X-Stat-Signature: s9uyeqjsxx6s9z3ptcuq6jdemhnwf5nk X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1752610157-260998 X-HE-Meta: U2FsdGVkX1+5HV2iML4m+wpd/MAN4dDOBHs/FvVpjv8nmNTyY3OqU4YXPEIXt5WfEQGhJfqhXE+o0lIn180mytPPNVdQ1zKipecKScv3hdkV8grO0N4xJyvtuwTUoqNIAGBjL76cNBtj1KHgJ/X2Bkxr3sdp66GOSbWx5CLZ5wKHgwSPOVCIf7nAueCAVGFFDF6IttVK2iN/4PPuZCdNUqQgkB/ZBSwuAaY7jWsIG4nTMQeOZfdIv5o4LM6nxwndiJoMn+xMbt74GVRInJTsLEZP1IBCzpHQ9aWyU61GZRSNURZCW+doU5+1hs4Wdc3/+N6OkOdCosULmQIBvOd7d1HbHXye4CGtdWtoS+y49woPEyRyABBu4nJG2BLyuDLPP5Rnffr7hX4HZO0oDqrFTPRaPd2PfzZUb5QYrDYH4mUV85VoGSiyK643O/1kOyug5AmenX/3uCMeCsa7/5r9wMo9/TK09vrbOPKMLpkiZUlLcBKvGeL5hCIWbDXkC6NvQteIxhshVKdBirrt6L7cNUr6bi4b59iq78miedSol7Tv4IJrwOKitFxad2xyZSCxORuFVbjeePd6Jab0ExcGzhwtp+uSsAXFW66JHmFXdyEhbWb/ye2XeiHT0Gxs8zBR+cGkl55Qk/AHeltwIl1YlWbDUuxP7iBEbJy//REXJhGTSQbmCJTi/ekUFjHUGb5mGQQ/k58ui9ZS4spFzojHQvEcoGAVVW2wwbOCcs1rzKsWSKKyh7HEU/i0gTmRDnc5P1TlNDyvy8m4Yw4gqaYC0t6gIolLwOxuPNIiJ6KAaOZtpKouWXHpe/OIW+dwvELlml/jyQPmTnmy7OnjLmFy/VLhE/g+y+m9uWk8CU8xg+khIG6NsRaS3vuVkswjw/zik9KQecZvvOXAcqJXoReCGdAO33ZxdvfqexbBD8SwoY9qGrju6GTcRnRvP1IXcD8Vo0sUIhwAxadlGPQIj8y h6UMg4Dn IYmmtx6iiUzXiqrCsXmWhlhaCgEM9Dsu3/mlng7robS2RJAQDK8pLrj9egruKmI6n6IrkiJuZVJF4tPzgXtLhDVbzn4V2j0WllQhuZAE9JPD+vkUN0lPbTDHb8RNPnhqs3Gz0h/O/hUSPFkmeDvTzMmCDeQyVtPuYvTCudCygvDhmFTZU3r4jVRcuZd9EaQp7hh0AH1gphjzcvPgp5Kbc0zzf4cwZBGUaYYh5JY+I99cRLF2f01Mhp+NggSP+BQGMJqx6m2BCBvIY/C7Hu5kxpe6nCpmNkI63l0yH9FZZl8siHR0= 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: List-Subscribe: List-Unsubscribe: On Wed, Jul 09, 2025 at 05:59:16PM -0700, Ankur Arora wrote: > No functional change. > > Signed-off-by: Ankur Arora Reviewed-by: Namhyung Kim A nitpick below. > --- > tools/perf/bench/mem-functions.c | 103 +++++++++++++------ > tools/perf/bench/mem-memcpy-arch.h | 2 +- > tools/perf/bench/mem-memcpy-x86-64-asm-def.h | 4 + > tools/perf/bench/mem-memset-arch.h | 2 +- > tools/perf/bench/mem-memset-x86-64-asm-def.h | 4 + > 5 files changed, 81 insertions(+), 34 deletions(-) > > diff --git a/tools/perf/bench/mem-functions.c b/tools/perf/bench/mem-functions.c > index fb17d36a6f6c..06d3ee6f5d69 100644 > --- a/tools/perf/bench/mem-functions.c > +++ b/tools/perf/bench/mem-functions.c > @@ -62,15 +62,31 @@ struct bench_params { > unsigned int nr_loops; > }; > > +struct bench_mem_info { > + const struct function *functions; > + int (*do_op)(const struct function *r, struct bench_params *p, > + void *src, void *dst, union bench_clock *rt); > + const char *const *usage; > + bool alloc_src; > +}; > + > +typedef bool (*mem_init_t)(struct bench_mem_info *, struct bench_params *, > + void **, void **); > +typedef void (*mem_fini_t)(struct bench_mem_info *, struct bench_params *, > + void **, void **); > typedef void *(*memcpy_t)(void *, const void *, size_t); > typedef void *(*memset_t)(void *, int, size_t); > > struct function { > const char *name; > const char *desc; > - union { > - memcpy_t memcpy; > - memset_t memset; > + struct { > + mem_init_t init; > + mem_fini_t fini; > + union { > + memcpy_t memcpy; > + memset_t memset; > + }; > } fn; > }; > > @@ -138,37 +154,24 @@ static double timeval2double(struct timeval *ts) > printf(" %14lf GB/sec\n", x / K / K / K); \ > } while (0) > > -struct bench_mem_info { > - const struct function *functions; > - union bench_clock (*do_op)(const struct function *r, struct bench_params *p, > - void *src, void *dst); > - const char *const *usage; > - bool alloc_src; > -}; > - > static void __bench_mem_function(struct bench_mem_info *info, struct bench_params *p, > int r_idx) > { > const struct function *r = &info->functions[r_idx]; > double result_bps = 0.0; > union bench_clock rt = { 0 }; > - void *src = NULL, *dst = zalloc(p->size); > + void *src = NULL, *dst = NULL; > > printf("# function '%s' (%s)\n", r->name, r->desc); > > - if (dst == NULL) > - goto out_alloc_failed; > - > - if (info->alloc_src) { > - src = zalloc(p->size); > - if (src == NULL) > - goto out_alloc_failed; > - } > + if (r->fn.init && r->fn.init(info, p, &src, &dst)) > + goto out_init_failed; > > if (bench_format == BENCH_FORMAT_DEFAULT) > printf("# Copying %s bytes ...\n\n", size_str); > > - rt = info->do_op(r, p, src, dst); > + if (info->do_op(r, p, src, dst, &rt)) > + goto out_test_failed; > > switch (bench_format) { > case BENCH_FORMAT_DEFAULT: > @@ -194,11 +197,11 @@ static void __bench_mem_function(struct bench_mem_info *info, struct bench_param > break; > } > > +out_test_failed: > out_free: > - free(src); > - free(dst); > + if (r->fn.fini) r->fn.fini(info, p, &src, &dst); > return; > -out_alloc_failed: > +out_init_failed: > printf("# Memory allocation failed - maybe size (%s) is too large?\n", size_str); > goto out_free; > } > @@ -264,8 +267,8 @@ static void memcpy_prefault(memcpy_t fn, size_t size, void *src, void *dst) > fn(dst, src, size); > } > > -static union bench_clock do_memcpy(const struct function *r, struct bench_params *p, > - void *src, void *dst) > +static int do_memcpy(const struct function *r, struct bench_params *p, > + void *src, void *dst, union bench_clock *rt) > { > union bench_clock start, end; > memcpy_t fn = r->fn.memcpy; > @@ -277,16 +280,47 @@ static union bench_clock do_memcpy(const struct function *r, struct bench_params > fn(dst, src, p->size); > clock_get(&end); > > - return clock_diff(&start, &end); > + *rt = clock_diff(&start, &end); > + > + return 0; > +} > + > +static bool mem_alloc(struct bench_mem_info *info, struct bench_params *p, > + void **src, void **dst) > +{ > + bool failed; > + > + *dst = zalloc(p->size); > + failed = *dst == NULL; > + > + if (info->alloc_src) { > + *src = zalloc(p->size); > + failed = failed || *src == NULL; > + } > + > + return failed; > +} > + > +static void mem_free(struct bench_mem_info *info __maybe_unused, > + struct bench_params *p __maybe_unused, > + void **src, void **dst) > +{ > + free(*dst); > + free(*src); > + > + *dst = *src = NULL; There's zfree() to handle free and reset together. But probably not needed as you want to convert it to mmap later. Thanks, Namhyung > } > > struct function memcpy_functions[] = { > { .name = "default", > .desc = "Default memcpy() provided by glibc", > + .fn.init = mem_alloc, > + .fn.fini = mem_free, > .fn.memcpy = memcpy }, > > #ifdef HAVE_ARCH_X86_64_SUPPORT > -# define MEMCPY_FN(_fn, _name, _desc) {.name = _name, .desc = _desc, .fn.memcpy = _fn}, > +# define MEMCPY_FN(_fn, _init, _fini, _name, _desc) \ > + {.name = _name, .desc = _desc, .fn.memcpy = _fn, .fn.init = _init, .fn.fini = _fini }, > # include "mem-memcpy-x86-64-asm-def.h" > # undef MEMCPY_FN > #endif > @@ -311,8 +345,8 @@ int bench_mem_memcpy(int argc, const char **argv) > return bench_mem_common(argc, argv, &info); > } > > -static union bench_clock do_memset(const struct function *r, struct bench_params *p, > - void *src __maybe_unused, void *dst) > +static int do_memset(const struct function *r, struct bench_params *p, > + void *src __maybe_unused, void *dst, union bench_clock *rt) > { > union bench_clock start, end; > memset_t fn = r->fn.memset; > @@ -328,7 +362,9 @@ static union bench_clock do_memset(const struct function *r, struct bench_params > fn(dst, i, p->size); > clock_get(&end); > > - return clock_diff(&start, &end); > + *rt = clock_diff(&start, &end); > + > + return 0; > } > > static const char * const bench_mem_memset_usage[] = { > @@ -339,10 +375,13 @@ static const char * const bench_mem_memset_usage[] = { > static const struct function memset_functions[] = { > { .name = "default", > .desc = "Default memset() provided by glibc", > + .fn.init = mem_alloc, > + .fn.fini = mem_free, > .fn.memset = memset }, > > #ifdef HAVE_ARCH_X86_64_SUPPORT > -# define MEMSET_FN(_fn, _name, _desc) { .name = _name, .desc = _desc, .fn.memset = _fn }, > +# define MEMSET_FN(_fn, _init, _fini, _name, _desc) \ > + {.name = _name, .desc = _desc, .fn.memset = _fn, .fn.init = _init, .fn.fini = _fini }, > # include "mem-memset-x86-64-asm-def.h" > # undef MEMSET_FN > #endif > diff --git a/tools/perf/bench/mem-memcpy-arch.h b/tools/perf/bench/mem-memcpy-arch.h > index 5bcaec5601a8..852e48cfd8fe 100644 > --- a/tools/perf/bench/mem-memcpy-arch.h > +++ b/tools/perf/bench/mem-memcpy-arch.h > @@ -2,7 +2,7 @@ > > #ifdef HAVE_ARCH_X86_64_SUPPORT > > -#define MEMCPY_FN(fn, name, desc) \ > +#define MEMCPY_FN(fn, init, fini, name, desc) \ > void *fn(void *, const void *, size_t); > > #include "mem-memcpy-x86-64-asm-def.h" > diff --git a/tools/perf/bench/mem-memcpy-x86-64-asm-def.h b/tools/perf/bench/mem-memcpy-x86-64-asm-def.h > index 6188e19d3129..f43038f4448b 100644 > --- a/tools/perf/bench/mem-memcpy-x86-64-asm-def.h > +++ b/tools/perf/bench/mem-memcpy-x86-64-asm-def.h > @@ -1,9 +1,13 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > > MEMCPY_FN(memcpy_orig, > + mem_alloc, > + mem_free, > "x86-64-unrolled", > "unrolled memcpy() in arch/x86/lib/memcpy_64.S") > > MEMCPY_FN(__memcpy, > + mem_alloc, > + mem_free, > "x86-64-movsq", > "movsq-based memcpy() in arch/x86/lib/memcpy_64.S") > diff --git a/tools/perf/bench/mem-memset-arch.h b/tools/perf/bench/mem-memset-arch.h > index 53f45482663f..278c5da12d63 100644 > --- a/tools/perf/bench/mem-memset-arch.h > +++ b/tools/perf/bench/mem-memset-arch.h > @@ -2,7 +2,7 @@ > > #ifdef HAVE_ARCH_X86_64_SUPPORT > > -#define MEMSET_FN(fn, name, desc) \ > +#define MEMSET_FN(fn, init, fini, name, desc) \ > void *fn(void *, int, size_t); > > #include "mem-memset-x86-64-asm-def.h" > diff --git a/tools/perf/bench/mem-memset-x86-64-asm-def.h b/tools/perf/bench/mem-memset-x86-64-asm-def.h > index 247c72fdfb9d..80ad1b7ea770 100644 > --- a/tools/perf/bench/mem-memset-x86-64-asm-def.h > +++ b/tools/perf/bench/mem-memset-x86-64-asm-def.h > @@ -1,9 +1,13 @@ > /* SPDX-License-Identifier: GPL-2.0 */ > > MEMSET_FN(memset_orig, > + mem_alloc, > + mem_free, > "x86-64-unrolled", > "unrolled memset() in arch/x86/lib/memset_64.S") > > MEMSET_FN(__memset, > + mem_alloc, > + mem_free, > "x86-64-stosq", > "movsq-based memset() in arch/x86/lib/memset_64.S") > -- > 2.43.5 >