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 5C0F2C3DA6D for ; Wed, 21 May 2025 01:22:18 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C90E26B0082; Tue, 20 May 2025 21:22:17 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C416C6B0083; Tue, 20 May 2025 21:22:17 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B2FB76B0088; Tue, 20 May 2025 21:22:17 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 950CA6B0082 for ; Tue, 20 May 2025 21:22:17 -0400 (EDT) Received: from smtpin21.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 3E777161994 for ; Wed, 21 May 2025 01:22:17 +0000 (UTC) X-FDA: 83465164314.21.D4AE36A Received: from mail-qt1-f175.google.com (mail-qt1-f175.google.com [209.85.160.175]) by imf30.hostedemail.com (Postfix) with ESMTP id 64BD980005 for ; Wed, 21 May 2025 01:22:15 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=xWYW7IMC; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.160.175 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=xWYW7IMC; spf=pass (imf30.hostedemail.com: domain of surenb@google.com designates 209.85.160.175 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1747790535; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=SHzCa2XQRYctkgISU9ssviPmqc9h+JaAqrCbnkEZAFo=; b=z3QBVCSy5CX/YFhjAsbAuXGGxES06vJt78UEr+0v23YdLJ8ah7LyQaorfkKi0HAEOxQqtG p97FhuLb12xOSQQME0Vh3hzTWYEpisPoC97cenl+BUd039s/WL8azdtjQOf6j6Ty0CzeyX wrrN+b2bj3wOSYZwoynpsOI+WZjVk4M= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1747790535; a=rsa-sha256; cv=none; b=WuwY25uOcegLBvbgYYx0whDvLm5M071tXHRZ5URE3iAL8MVADtMKIqLiAaxxfNiTh3/Nq6 KJJdbiRnEFNyapDh0VPtpa6OWfkVYVi4dWAwFnvuCyzrZGesBh6Ixf4Q1zYK2EyuRFqI9d FoPXPcKRFpy0fPiNf+UG1Oz9LL+ZRbw= Received: by mail-qt1-f175.google.com with SMTP id d75a77b69052e-4774611d40bso1007091cf.0 for ; Tue, 20 May 2025 18:22:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1747790534; x=1748395334; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=SHzCa2XQRYctkgISU9ssviPmqc9h+JaAqrCbnkEZAFo=; b=xWYW7IMCbfrZlKcPTWqIMRGDaBbKf8PWVKLvzvjyIOn6QKWC6FkUJJ3m+3XpHy/fTr 99b+PiUmPXOrVOuQRV/qhNWr9aJ8GD5+ieBEZvSTOnRu3avH3LVXyzIHLJaVU6G3ETup JkYJSf0e62Qc9DvQkOgr/yxbiI7rn44d4l+8ug/U9TbCnSNReegqZk5x3CdHPvGZVkbk k972sRBnuEFYeR6k7KfQVuhI3PnepP51tyPLxZri3XlC5oPF2GLCRqOUVWonajcaH929 r6dJb3uf75ydalqhjFghmRmRdTgrljcT0dbAhAib3uWXeQ+QzmSg0EXu3QZ5tt0Pjg49 l86g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747790534; x=1748395334; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=SHzCa2XQRYctkgISU9ssviPmqc9h+JaAqrCbnkEZAFo=; b=jKfh8383wp7Ypz1SRLSMU995HlhDrDnfWyCa5DGGY7LaZrLkUMTlAXxszhUg4VO6Jm NM1LEYvd/IMGHAo3/yfWRcURRo+PjOF6Z8IFgliLw36Mp9a+IpO0LqyDvMYS2ZahmpF+ lcNxCUlii4RLB1KOgjZT2RcpBMwGYiKp3GR1k/HqCzTsGSzd2cu4uclr9BTiOYCLkybq ikJ/9Ahnk3nhpJTI/vMv1Ow6B+UMt14O7/m5cig45vCrzTeUiV1RiyAV2KA1uoPyK07A hT1xs4ZFQnLlj8zNNWg/a9plgcpcdv8Y9ghMdoN7RLLXz4Ks8OJ6jirgykkf7nTBWIC/ OdrA== X-Forwarded-Encrypted: i=1; AJvYcCVUQNsUXVR+dCSlQa9Q2CtnLk13hbmc8SGiIDWputLnZcB/UQWaYrymQfCxffb77MPTaSvF2xPgBg==@kvack.org X-Gm-Message-State: AOJu0Yx/oXHwkGip3l39SXfMR+6Va2bhdJaiFnsVZncdl0HzkhV1ZHjj Ar5ppC22373+PH9M0yyT3cv8volrlSXYzcvQCehxgmXkt6fBqB2O++yyDUjj4nlwTK1TGo8J7Lf Nq4YrKB7cU8ycNMqdt22eJhLecvMEMHmsO01HVgBY X-Gm-Gg: ASbGncuk+GpsVGmgctVpnsZL5sQ0phXoKirMlwk58vGooldcUyllYuifZwHsGmVqHGQ u+Q1ITZydZA4wrkGPvimVbq681eB3hJ9GyVtosIdw4s86V49/QR6fHP/AAAQxuW7qzLXY/ta1o1 KZCBSsJcBcIxIZ6QofJgEBzO8moeq0noNWabqIQ8rYt9x3NDVwq0S+QL+8r/Ye23FNKxEl+XE= X-Google-Smtp-Source: AGHT+IETk9JsouTomTSfYWPNhg8HW1BBXFdg36hX2MSkaoPaIDDGxmR2D+QNx+kdjYOJPlI/lvzsR89BaSYoRQgZ2X4= X-Received: by 2002:ac8:7c52:0:b0:47b:840:7f5b with SMTP id d75a77b69052e-49601269821mr14330491cf.29.1747790534110; Tue, 20 May 2025 18:22:14 -0700 (PDT) MIME-Version: 1.0 References: <20250517000739.5930-1-surenb@google.com> <20250520231620.15259-1-cachen@purestorage.com> In-Reply-To: From: Suren Baghdasaryan Date: Wed, 21 May 2025 01:22:03 +0000 X-Gm-Features: AX0GCFvylRrIG4sSJ_j5oVEsG8ZIbb2QwvzTcJFZ13RR_HWNZPB6irPD3XQ7IRs Message-ID: Subject: Re: comments on patch "alloc_tag: allocate percpu counters for module tags dynamically" To: Casey Chen Cc: 00107082@163.com, akpm@linux-foundation.org, cl@gentwo.org, dennis@kernel.org, kent.overstreet@linux.dev, linux-kernel@vger.kernel.org, linux-mm@kvack.org, pasha.tatashin@soleen.com, tj@kernel.org, yzhong@purestorage.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 64BD980005 X-Stat-Signature: ho7g1i4bs7yc4kiuwy9irmypniwsqn79 X-Rspam-User: X-HE-Tag: 1747790535-396934 X-HE-Meta: U2FsdGVkX19LJ3EMIYISb9zib9IcFhtEPS/Lul90nOFwAF+KFDvbfdd7N/6tlB6TghaCVh1bClhEA2tMK0y5mWGi7SVh59+5wRQZ3RK8hc/PK4MtKWj34dh0OUYozdFUCG4KxhIzwCENh0QmHenv69JJ37HeuNnSYjSNZBZPvxeo1/dsnkgGuDGDNRf8FLxRq2oAKGb1PA9SLvNoAR8uExge380vgFoDU2gSWfvvvhUlnq2bNLMAYR1U/T812MbTVPtHTvHhCXS/TgumQ0o+SKi7pcRmyrF9HOCwmWRcpdb7URXpxBo3eqhp+mpMsDeTdOK+fP9wm35JRXFuC0FsyUWnyCNF3BWb7Lh6TbqvaVY6WP1s85Nq2BDrFH53E88LTReOJ2f8gU4l0yH7cklhk+AoOJKJxegIBd18fTOzP6QhQwwHsQEvM9VlNLVAyHVIMS6wBETm2uqTUJDYv294WO7rcYZb2Hhnzvx2ug0vsoF6m8DshmJEtgbmXyE8+W5gds9kX3piRaP0r42K/MeFlkmN5vClEyxqOgS9w2+Ro/ZghKAPJAd+kpL51em4CTk/bEawo8ke9beTqnQhlfpy/ULtSdukOgGvL66FcfT2R55xGxeHrpgVG1UxMEER8wM5/PdF4VZrRTVB8oEnWgC1Chn8xy1wm26XNrrCbg4T/MMDcfSTHXkMU0WXU7ju//7w36/Ct5nq6MXRV8LSIdbicIta3tVaIv4qv15c4I4EfFOeIi2CBJuhlNTF8vRWw8mnGsApQTTjYSXsvXbjdhhrdCsb31q0tmRJqJIavZLIQmu9hLwy8Cz1e/XfT51pNjXoZHwuDoMxHwzrF9Cj+3Na/plfVtyHzCdjdbRCCLmuKVK3zVd0YrbY2LT9yawWmIUT0ffLTYN5MACBbULfiuW2iw4my38JHhLyyTVJWPhToULPTdjepFaj+lN/M7SgaZ1osva1qS/Vvo9FT9WvYrZ Cund7Xrd xWsKlgLXvbsIzDGuTjdKv748oUKcrEj80HIEzRD+Lhl1ld8Q53QUg9uQhKbrZ5Q3t66YqZ4YYxik4+ueVja+rt8N859UlsGAikDQGYZtFWUTxlw5XgK8UcZ5Mw2XbdnniUVC5OEDMqbWPZAmFwjyat2ghb2H1dnhWE7745KdWrBVwecFBkSGwPrK9Z0QJTdtxI/oNfejoGlSH6uhsCwDENw3eLDKsrNFlZPFG4kQfBiVYdReAFCWNyjNpymVANZghOr0Z7Ld3nI1kyy/22JV1FYRjCmJJbBN88lcp5BJW6+V8HR2NWQKxVYK10GqDO4+NT+yPjuH/4cwy1ZO2a96aJ+jOdd2/7yoEooUh4MfRItI7jvMPeNeZTkbwUw== 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, May 21, 2025 at 12:45=E2=80=AFAM Suren Baghdasaryan wrote: > > On Tue, May 20, 2025 at 11:48=E2=80=AFPM Casey Chen wrote: > > > > On Tue, May 20, 2025 at 4:26=E2=80=AFPM Suren Baghdasaryan wrote: > > > > > > On Tue, May 20, 2025 at 4:16=E2=80=AFPM Casey Chen wrote: > > > > > > > > Hi Suren, > > > > > > Hi Casey, > > > > > > > > > > > I have two questions on this patch. > > > > 1. If load_module() fails to allocate memory for percpu counters, s= hould we call codetag_free_module_sections() to clean up module tags memory= ? > > > > > > Does this address your question: > > > https://lore.kernel.org/all/20250518101212.19930-1-00107082@163.com/ > > > > > > > module_deallocate() is called in error handling of load_module(). And > > codetag_load_module() is at the very end of load_module(). If counter > > allocation fails, it doesn't go to the error path to clean up module > > tag memory. > > Ah, right. I didn't have the code in front of me but now I see what > you mean. codetag_load_module() does not return a fault if percpu > counters fail to allocate. > > > > > My code base is at a5806cd506af ("Linux 6.15-rc7") > > 3250 /* > > 3251 * Allocate and load the module: note that size of section 0 is al= ways > > 3252 * zero, and we rely on this for optional sections. > > 3253 */ > > 3254 static int load_module(struct load_info *info, const char __user *= uargs, > > 3255 int flags) > > 3256 { > > ... > > 3403 > > 3404 codetag_load_module(mod); > > 3405 > > 3406 /* Done! */ > > 3407 trace_module_load(mod); > > 3408 > > 3409 return do_init_module(mod); > > ... > > 3445 free_module: > > 3446 mod_stat_bump_invalid(info, flags); > > 3447 /* Free lock-classes; relies on the preceding sync_rcu() *= / > > 3448 for_class_mod_mem_type(type, core_data) { > > 3449 lockdep_free_key_range(mod->mem[type].base, > > 3450 mod->mem[type].size); > > 3451 } > > 3452 > > 3453 module_memory_restore_rox(mod); > > 3454 module_deallocate(mod, info); > > > > > > > > 2. How about moving percpu counters allocation to move_module() whe= re codetag_alloc_module_section() is called ? So they can be cleaned up tog= ether. > > > > > > That would not work because tag->counters are initialized with NULL > > > after move_module() executes, so if we allocate there our allocations > > > will be overridden. We have to do that at the end of load_module() > > > where codetag_load_module() is. > > > > codetag_alloc_module_section() is called in move_module() to allocate > > module tag memory. I mean we can also allocate memory for percpu > > counters inside move_module(). > > I thought you were suggesting to allocate percpu counters inside > codetag_alloc_module_section(). I guess we could introduce another > callback to allocate these counters at the end of the move_module(). I > think simpler option is to let codetag_load_module() to fail and > handle that failure, OTOH that means that we do more work before > failing... Let me think some more on which way is preferable and I'll > post a fixup to my earlier patch. After inspecting the code some more I'm leaning towards a simpler solution of letting codetag_load_module() to return failure and handling it with codetag_free_module_sections(). This would avoid introducing additional hook inside move_module(). I'll wait until tomorrow and if no objections received will post a fixup to my previous patch. > > > We have implemented such a thing in our code base and it works fine. > > Just do it right after copying ELF sections to memory. If it fails it > > goes to the error path and calls codetag_free_module_sections() to > > clean up. > > > > 2650 if (codetag_needs_module_section(mod, sname, > > shdr->sh_size)) { > > 2651 dest =3D codetag_alloc_module_section(mod, > > sname, shdr->sh_size, > > 2652 > > arch_mod_section_prepend(mod, i), shdr->sh_addralign); > > > > > Thanks, > > > Suren. > > > > > > > > > > > Thanks, > > > > Casey