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 173A6C54E71 for ; Wed, 21 May 2025 16:17:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id AAF986B0088; Wed, 21 May 2025 12:17:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A872F6B008C; Wed, 21 May 2025 12:17:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 99D926B0092; Wed, 21 May 2025 12:17:07 -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 81BE06B0088 for ; Wed, 21 May 2025 12:17:07 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 20A52C0AF6 for ; Wed, 21 May 2025 16:17:07 +0000 (UTC) X-FDA: 83467419294.04.2D8CC3D Received: from mail-qt1-f177.google.com (mail-qt1-f177.google.com [209.85.160.177]) by imf06.hostedemail.com (Postfix) with ESMTP id 2815D180012 for ; Wed, 21 May 2025 16:17:05 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UGGQVX2I; spf=pass (imf06.hostedemail.com: domain of surenb@google.com designates 209.85.160.177 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=1747844225; 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=2su35M/1woi88PA/W3LYgGvlRpYxMTPNv9iFOVqohJU=; b=6oXNvSrbBUtUZ6CxjTnTiEx1/K00WVeiFnecoP0gH81ROq8jr8VPEae26KF1RzJcgXvPqq if6voDq0G54ogQ85TqSieVbAWD0oNCpwMA4b6yYcbkgDRA7q2mGEYVJTcTxS1Sl0I7w6Au iVEOkmcQ5RW+drmNmSEKzpinWegAFwU= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UGGQVX2I; spf=pass (imf06.hostedemail.com: domain of surenb@google.com designates 209.85.160.177 as permitted sender) smtp.mailfrom=surenb@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1747844225; a=rsa-sha256; cv=none; b=VzYsdeRq0zy5MruRr+eZAhcf2w8cY/tgf8kx3E0dcmkUjvY4vjNphkBeZu7IrGk84bQUU8 YVzbMZrKrNLlRDHfU7OSjaeLyBiBSYAldfRK+S/WnghVXbXvzgSjUCtUaWlRb1Ft9vujRo qA4ReEUEe1aXAJS8ECx30r70nwyVUUg= Received: by mail-qt1-f177.google.com with SMTP id d75a77b69052e-4774611d40bso1196311cf.0 for ; Wed, 21 May 2025 09:17:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1747844224; x=1748449024; 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=2su35M/1woi88PA/W3LYgGvlRpYxMTPNv9iFOVqohJU=; b=UGGQVX2I8CoxHmq0sehMrkmHwVb+tw5lTdkUWRTAu36iDP1UmCoF4ngSSKwtIgGboc bObCqqnQUppq5sq3WO83e8rJVbydMzOslx0LuS90A7gy7Rvm1vK78htX8yVuqwZD5vAL Gfpp6lq4jUlRp4lnZs1MjCawxLNUMCVKwWYfwzeiH7kKG0axs/fPHD/2ttVObfo3BJgO u34v+zGIvuwfbXIqyNWmSM3X7Fp8cW0dJKzu6wi5nCpvc6bc+SG2kXBZGLxd+xdE0ccl KvmRZjyRaBINMYcjQC9av9HBtkMtwCgB7vLPmcq+r4O/RGe2kmSfHtSVFp8pI7J/j0FH ++2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747844224; x=1748449024; 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=2su35M/1woi88PA/W3LYgGvlRpYxMTPNv9iFOVqohJU=; b=iy8seBktKSSYy4vzbVGTxZnspNCOAT44gV2ZVIxyxj/Xl8TP+dgpTw6T3cE1fAkhWa y3nc8/gc3F4a9IOA2oc1yH3KTGixWhUC4iThZQUxqC9Lx6Y88Iyiu4xMYAnouhKzOnb5 4/0b1K7h7oxdDIhztF28m3PoOsBt2RXzSBhKAziaO45cvFxAdBECw4+0GBsLPBhoayFp RZfYTqo3CQ14LzGIObl4SHtROx0qJlxQNvg/y7q2pxDN80PFRWV/zkl+9iADJA3ktqyv B3gh1d0Uzt9b/u2WnvYRVIqPPAy3t/4wjjsbXw0p8wkvtYJuYmhNdqhIvUBlJhQLUv1e TSBg== X-Forwarded-Encrypted: i=1; AJvYcCW4MJeyumH98128eQYlx2KFzFAJEa7PwOnx1dY0N2KPKb+8bVyJOibJmFw6OzRQZ0IearVjneVvRQ==@kvack.org X-Gm-Message-State: AOJu0YwNwwRx6wuTFrUQehCbl8DHSkfUVXFYgmvCNjxKnGqnoj8f8EyM G/l5Kmhty3LDDysVcqrSpwEw7jLa4Xnp4ODGB6dxwF8dHt9Copy7nlsA6O8wsyrl3RmV5BXDzVu sj4F7Y/c/STqxC43lPuy/hSVj/M4yG5ayPE1h1Jzd X-Gm-Gg: ASbGnctgCusGYHdIYI7jWkRFn9vu2NuuvzRMO+5n8QTsfNe6teSI8NB8LAm6KwD/fdx c+g7G6M/p3GZ8H2rGon3ix3LSyQTG7x8mvoVNH5Aytg+aDd64QlKbrAIbuexcrzk4NUMbeWQJzO KJj82K4CE0+1tLafmBBVZwR13U6xV9/3t93GoUwYAjxN9a7qC0h9qQQIgzPCRZO+BLLgVVoWHpP Q== X-Google-Smtp-Source: AGHT+IEuQqqcPpxUbfAIyCjihAWxW6OtiHWuZOiPIv9/nbxruNyxnYM8BWux4MlQrcGYdr4v2ORrHqQZzS9xhb9/zhE= X-Received: by 2002:a05:622a:449:b0:494:4aa0:ad5b with SMTP id d75a77b69052e-4958cc22bc1mr16836641cf.2.1747844223803; Wed, 21 May 2025 09:17:03 -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 09:16:52 -0700 X-Gm-Features: AX0GCFu09bEI8eb2_SwUKuimcpX9e4GHOBrkheslsnz0f2SFy9hm_tjUlV8oiaA 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-Rspam-User: X-Rspamd-Queue-Id: 2815D180012 X-Rspamd-Server: rspam09 X-Stat-Signature: 4zbs8eqb1jo1bzm98kyjmjj4diefidm5 X-HE-Tag: 1747844225-404623 X-HE-Meta: U2FsdGVkX19ypsYy+JuE79D5DIr9a4YzOXp/6iy9NXZIZCwD5gUqJPgGX3h3cUfV5YA8W7jieQAF4Y9SuEwK8GaylQYF27l88jx5gx3tf4hZ92cjeTwSAY5vv1cA+WnE9Ru+eD9Cm3IXyr8xK0+8ZCUsjVmoXPVTkCudpj6rCBNGE94I7413xwvgXeoN/ENY8/H7oapAR8DYC9NL7EPvTyWt8g93rZ9hLZHrTttgddcAeFazp1H9GMvLOWlQAFL5Ua/uiFFHoVpdm/t6sFu+BfrMH5rz6T4AD2jX+abB/YTf0eBJmwvKhFNbazdRQpS4w3OFb4xNXOiLaz5+F6Tv4WM7YTn1cdeLiFzHPxCE2WEgSZ0hDCXI+877mcqso2gGnLoMv8Xbb/PiHrlBR9nVrRKXJoPbzi05KuG2ZUs3mbiT42qJJXWfpWbNCnOOfbcuBvZzJwBbhWvpzfzcNJg+/dksBPLk+4FBFuitLsYOeWb0z8yOPMQxJiR6gC+pplahARKwo8sxzsAvocIwY74wE2e/QYRkJGQyYXqlt/2gONx7BWWyS8fnm1i/g6etMWnH6uKdtz5AggCkzZKK1ncgG8uGsQuU0/MqU/MsUCECa3YZxAW7RSWSfHFS2aep0Y36YQYeD72Ko74gW67iF41UT/Pluj/e3bN/qfUTMJ0H/iuGTyw2Bz2dEV5c/YaKU43er3/DN1vGih11ac09U+nM2cSjokURf0RTtZz6643MwpmkjQ/uGctsxFf4DJO9RJQlWylJq5s8qinFtybkzj4mtc9ER8zzSqlXYlY3CT+n4IuWQ1orzpkQfMVzXef8cBqMr1dzzEMVhZ0HlaHEJMNmmtcnDEHzUeKJ+7dzX9TvJU3HSQYzdCSjNsy7JOw1GmeIUyJbldSlx6bU0ZWloCvzxCZdvA5p/mAh4MIq8ny2tfWC2SkrG8tWmSwakp6sdlsc3M7Vk//HqdB+xKnQIdQ qHawwOXG ekc5tUn0wzu3bvf/19wGK4XDPw7mAX9xC24B/75lExbhcd4+FovW1F5yoLD59QNkm/12xEshwuhd0mErKKpC2/bEJuxKOfaPvcW8aSHmZbss6iOUoAjELnUR4PjYmzKT2phwV/SCpMiFOvFhhuxo87PDmctB6CcPPeXiVwsF1bmiJaSBlG6y6bggMGH9c8wsSRS/2RtjW7vf9KqK+o/nVUuhW944blQ4Pt1oXEeG6Qjqjuf2bwvY/PwoWIiB8FppU0T18sLK/6syiAO5b79qWAGINu3vzaMrn95ymfmVvMpKGHRDPX5ddz63qLb0pKUJXpLTLIFvd27LxdXp1Xq4ooncOWq+RUH99gYopqq+uX0GnmqVImyIDL7GOQA== 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 Tue, May 20, 2025 at 6:22=E2=80=AFPM Suren Baghdasaryan wrote: > > 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,= should we call codetag_free_module_sections() to clean up module tags memo= ry ? > > > > > > > > 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 = always > > > 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() w= here codetag_alloc_module_section() is called ? So they can be cleaned up t= ogether. > > > > > > > > That would not work because tag->counters are initialized with NULL > > > > after move_module() executes, so if we allocate there our allocatio= ns > > > > 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. Posted the fix at https://lore.kernel.org/all/20250521160602.1940771-1-surenb@google.com/ > > > > > > 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(mo= d, > > > sname, shdr->sh_size, > > > 2652 > > > arch_mod_section_prepend(mod, i), shdr->sh_addralign); > > > > > > > Thanks, > > > > Suren. > > > > > > > > > > > > > > Thanks, > > > > > Casey