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 5A4BCC54E90 for ; Wed, 21 May 2025 21:31:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B02CC6B0085; Wed, 21 May 2025 17:30:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id AB4006B0088; Wed, 21 May 2025 17:30:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9CB256B0089; Wed, 21 May 2025 17:30:59 -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 7E4E36B0085 for ; Wed, 21 May 2025 17:30:59 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 983FB814F4 for ; Wed, 21 May 2025 21:30:58 +0000 (UTC) X-FDA: 83468210196.24.08070D2 Received: from mail-qt1-f182.google.com (mail-qt1-f182.google.com [209.85.160.182]) by imf23.hostedemail.com (Postfix) with ESMTP id B6F03140013 for ; Wed, 21 May 2025 21:30:56 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=nuPHU3cX; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.160.182 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=1747863056; 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=bWdOovJf964fqY7rP1f70MsVKty0VjlMR4j1MeuRSSY=; b=b26hhLw0LhqJuBsK7yykDVLMxtXGnEhY23WdKSqV5l9tOl/nyoHzvEob/pQBwkEbbwLFtX zqPx2phVGpveGgBJVxgvyvxFlbTNDCTHIcO+7JTgu/jjLn0zOxXzmW13mV+KWRoG6ImTfX yD0XVoC5ChOm56u3ZIghKnQdV592o7c= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=nuPHU3cX; spf=pass (imf23.hostedemail.com: domain of surenb@google.com designates 209.85.160.182 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=1747863056; a=rsa-sha256; cv=none; b=MAnSbh4ySE0dZr6ikXk0J0ioLqnfz0ub1ap7bJv9wEXX9udWPuSs6NDZVr3NjBqQv35xr2 AN/RkTJikxnI/u0T95jn+/5Wio9rr2InWx9UE2vT3uitzoM09dIOoOnuza0CjEk03ixXye 8rBYA4PMScib035Wn5L5GqzrDnSvLwU= Received: by mail-qt1-f182.google.com with SMTP id d75a77b69052e-4774611d40bso1285991cf.0 for ; Wed, 21 May 2025 14:30:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1747863056; x=1748467856; 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=bWdOovJf964fqY7rP1f70MsVKty0VjlMR4j1MeuRSSY=; b=nuPHU3cXdiLBucrseOjAcQMS04roE03/0wmQrGQO7AUcy0eZtCJvHsKSFKsrl8tyES MvzO/jA3PXQRMkrUZ/hO85QshVcdLxOyjFvBjl7UNOnksOwboJEeczx+yKj8jHDwx7b2 SJGQOnPbBjFlIjnOaZhHFJgIcgHBUE8YbUqhXQw1pvJshZB4qomWdAxHEFzdptVf11Eq E4V4Xj4l6CzuZqT5Eg33UtpXh4bHPnvC7SSMFOHCmeZyB7/xvbKAKpkjOOfuwZzPYWNZ a9oNi4calIt2Wr+qbekCcc2wQsBAHaZHI421HeuHugCNEjkmW7hiqMHuU7WZx90OFv62 YaSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747863056; x=1748467856; 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=bWdOovJf964fqY7rP1f70MsVKty0VjlMR4j1MeuRSSY=; b=ixIqviGRWBQ3Ah7BQ/E2LpgvFGiYNU30HwecLgQsnXirfBW40OxTc+gUDoMaqCj86E dfq1lqrK4k8O7AHsWSAA3aBwK4Zu2t/3cw75v9Zb4aLSAwdhv0kbqZsXtyFnLETRHyk8 XNw7R1fC5qxXiaVqO7hLogpjkN0cjjgdqHRPfzwIGvUOF7O2ntUprYBEA65gSw9n5F+n n4s5RZVj1SPrwA87ywabHF1AQJxoWwyFsfxZSnNrfQKVxZ3r6dYr3DspO61l2XjePOSZ QM/WQJMunNXIaEsfR4Xhz+Eo/6h4YYiigv8wuuQlBl4bcLARgxplF6Ou5c0H+qBwL7Vy G2JQ== X-Forwarded-Encrypted: i=1; AJvYcCXGmUu9Qn4HG7TJBpMES8g6EpAGVADtnSgDBowhPDTUk95i8+0EXBUxVh73mXKX+wZbW8uOdbuV/g==@kvack.org X-Gm-Message-State: AOJu0YxYWxdvGgcbdl7UbHUr5I5U7kmmDjggEKzauDyXGMjSdrhLllGS bvCKDuJkTJo5oE3SFaDUAqgwBUqOrobgwxVeW37jVP/cn4lzHgUHm/2oal0bA8EE47MLprLuSVx uBn9Z92jkW7PF1XExRx80s1HW7Cnu+G5DtKyuGcU2MG3e4p3mx5girv6ENJE= X-Gm-Gg: ASbGncsUoSF2RnSbhxJJJB1a4nuJwku4Tu+Tk/ExtLqQ2H6UkuFFQvkHA+e+1TaKEYJ mYGgSSQmjw8PooT6oCHewiNNU5I0NQYqkAE5Hz3fGFaikLN3z3g67jBvke0rvFUnb/3ijGeoPYh rDhl094PmwuA25br1/0gfGe0N5PMNu4No2dyg9S3xIpw== X-Google-Smtp-Source: AGHT+IGHW1Le+Th0rF31ilk1bMi7uSr/wQssgDMLiHfYo3VTiUKmx6GZR0ECB2AxON+cYr0kcIlo3a3/BRVv1Ddt61I= X-Received: by 2002:a05:622a:101:b0:494:4aa0:ad5b with SMTP id d75a77b69052e-49cefbe04a6mr422601cf.2.1747863055480; Wed, 21 May 2025 14:30:55 -0700 (PDT) MIME-Version: 1.0 References: <20250521160602.1940771-1-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Wed, 21 May 2025 14:30:43 -0700 X-Gm-Features: AX0GCFsaZ4v0KFQhyMLGDjF1IPg4de-UDXi21dVKxL3pD1IRz55dfTMPWlc_JzU Message-ID: Subject: Re: [PATCH 1/1] alloc_tag: handle module codetag load errors as module load failures To: Casey Chen Cc: akpm@linux-foundation.org, kent.overstreet@linux.dev, mcgrof@kernel.org, petr.pavlu@suse.com, samitolvanen@google.com, da.gomez@samsung.com, 00107082@163.com, linux-kernel@vger.kernel.org, linux-modules@vger.kernel.org, linux-mm@kvack.org, stable@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: B6F03140013 X-Stat-Signature: z8jit1eyupjrtgt1mc7c5jywqngqbca5 X-Rspam-User: X-HE-Tag: 1747863056-318055 X-HE-Meta: U2FsdGVkX1/F6uWVH+mrCVzHrg54HEUfJ/yF56kBD9FXMxIDCLp0+Ny/tM5UEDM0JzZgRa8rXM6hxHtG6ADMp3jvkHjlHlZMioqLsbX9wkDkr2ZIWvVIOfdKfizzNV6GHIHBBfwtGXe2yvDZQOONi2fIZf8znA9MxShA/ti+flrmc8TXyNWUJUCv7ooZfpX2S21JUPExtKjZagr2wOEzPTEUCkkryz9wR9P28CEQHh5C2JDFtRYblYA6MIY4CCt95zuk7Inn16gl9DrsohAYZxYLRjBSUr+BtlwPW8GDoXwKvaRPsKZXwugurSenTaPT++qyj4UWZ1woJJODDAFSc6N6weRGmCeVpVSbs7miEWLFk2kDXxwK4tObGmAA6hmuZq8B8fO5PisEAosBV4i8pthw0YTtQb9fVmv/fWP9uQ2uIaJuBysbjxyuNU1AvmbkN5adKL12ZvKRhPfY/EABp0P2w5rczdJCPq1eVcCBsWb9thegz4m+d3LaqUfj06l7JpFURdzkYFJkZWQQMASbPHXchB4wm5xoCc8CX0XiTjSzE1d2BD3dbePWhrXT+Cgov8K8LBTbHJpnbcWI1EPHNnti54mbTXV2XNdE9W+uEP7oumh7BKobvndwFYD2ZBh/dfxebelByOkg0oBSepy9uWuJFAZl7er5brYHNg5xg/7okaAeZgqqN9QwyaVxLRDiFiIUaZiZT64tBvLPkDEd/Xett2s0V4sq3TU99PNRK5IVdEsrHKHVPy3DqjdYDW6Gs0pFS5iaj3H6s6qlTpwXrIOCQ5k1uvswAh8CYmmMqtx9XLdUX71SI5JXf7QgJLTE/ekbF1M8RcmEGgSHlfelffpvWPeuZyfxQRB5+gmHqJKuwIfNloyXB171kImTVqWnTZ3iv/pguXvQGYSFIGNcCjUhH7XeAfqjyD0KtEP7nCM8ufVeYc7GZVIEdf8PNnc2yoCdqHylcU1XgiuaIMr DHnA94+J 3UF2SkuMwVCe5SIAFKt0VSrdFQnVmAKVQ35FrxuyeW8D9N1cODDX1AQXmeA6vKI2zoiQ2sbmta6ZIov2XbDwzyH0xd0PBJpJRQlKvFVgCk+SnkzQjy+jIm9vRQ8kfW1NLGp/3jJ9Ti1TVsJtnDj+A4HaHBFPbUVPKcny89EMx5vkf7YZY6u4sIZ0JCo6pJ3+pNLM/t2F0+Drs+UhOpCJyugxnFmL6OgGAKmEmBuEjaTcmx14/acnwZNmoKK1MpyfrUz72vzMEXhS5wQiaclgmzZHRPDmZuro+JE73IB7lXhLX6pGEmIreCiUaCAw1JEFFXT89ela7oiI5pfLPPKSsEG5UDMUqCr7L53Gl 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 2:04=E2=80=AFPM Casey Chen = wrote: > > On Wed, May 21, 2025 at 9:06=E2=80=AFAM Suren Baghdasaryan wrote: > > > > Failures inside codetag_load_module() are currently ignored. As a > > result an error there would not cause a module load failure and freeing > > of the associated resources. Correct this behavior by propagating the > > error code to the caller and handling possible errors. With this change= , > > error to allocate percpu counters, which happens at this stage, will no= t > > be ignored and will cause a module load failure and freeing of resource= s. > > With this change we also do not need to disable memory allocation > > profiling when this error happens, instead we fail to load the module. > > > > Fixes: 10075262888b ("alloc_tag: allocate percpu counters for module ta= gs dynamically") > > Reported-by: Casey Chen > > Closes: https://lore.kernel.org/all/20250520231620.15259-1-cachen@pures= torage.com/ > > Signed-off-by: Suren Baghdasaryan > > Cc: stable@vger.kernel.org > > --- > > include/linux/codetag.h | 8 ++++---- > > kernel/module/main.c | 5 +++-- > > lib/alloc_tag.c | 12 +++++++----- > > lib/codetag.c | 34 +++++++++++++++++++++++++--------- > > 4 files changed, 39 insertions(+), 20 deletions(-) > > > > diff --git a/include/linux/codetag.h b/include/linux/codetag.h > > index 0ee4c21c6dbc..5f2b9a1f722c 100644 > > --- a/include/linux/codetag.h > > +++ b/include/linux/codetag.h > > @@ -36,8 +36,8 @@ union codetag_ref { > > struct codetag_type_desc { > > const char *section; > > size_t tag_size; > > - void (*module_load)(struct module *mod, > > - struct codetag *start, struct codetag *end)= ; > > + int (*module_load)(struct module *mod, > > + struct codetag *start, struct codetag *end); > > void (*module_unload)(struct module *mod, > > struct codetag *start, struct codetag *en= d); > > #ifdef CONFIG_MODULES > > @@ -89,7 +89,7 @@ void *codetag_alloc_module_section(struct module *mod= , const char *name, > > unsigned long align); > > void codetag_free_module_sections(struct module *mod); > > void codetag_module_replaced(struct module *mod, struct module *new_mo= d); > > -void codetag_load_module(struct module *mod); > > +int codetag_load_module(struct module *mod); > > void codetag_unload_module(struct module *mod); > > > > #else /* defined(CONFIG_CODE_TAGGING) && defined(CONFIG_MODULES) */ > > @@ -103,7 +103,7 @@ codetag_alloc_module_section(struct module *mod, co= nst char *name, > > unsigned long align) { return NULL; } > > static inline void codetag_free_module_sections(struct module *mod) {} > > static inline void codetag_module_replaced(struct module *mod, struct = module *new_mod) {} > > -static inline void codetag_load_module(struct module *mod) {} > > +static inline int codetag_load_module(struct module *mod) { return 0; = } > > static inline void codetag_unload_module(struct module *mod) {} > > > > #endif /* defined(CONFIG_CODE_TAGGING) && defined(CONFIG_MODULES) */ > > diff --git a/kernel/module/main.c b/kernel/module/main.c > > index 5c6ab20240a6..9861c2ac5fd5 100644 > > --- a/kernel/module/main.c > > +++ b/kernel/module/main.c > > @@ -3399,11 +3399,12 @@ static int load_module(struct load_info *info, = const char __user *uargs, > > goto sysfs_cleanup; > > } > > > > + if (codetag_load_module(mod)) > > + goto sysfs_cleanup; > > + > > /* Get rid of temporary copy. */ > > free_copy(info, flags); > > > > - codetag_load_module(mod); > > - > > /* Done! */ > > trace_module_load(mod); > > > > diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c > > index 45dae7da70e1..d48b80f3f007 100644 > > --- a/lib/alloc_tag.c > > +++ b/lib/alloc_tag.c > > @@ -607,15 +607,16 @@ static void release_module_tags(struct module *mo= d, bool used) > > mas_unlock(&mas); > > } > > > > -static void load_module(struct module *mod, struct codetag *start, str= uct codetag *stop) > > +static int load_module(struct module *mod, struct codetag *start, stru= ct codetag *stop) > > { > > /* Allocate module alloc_tag percpu counters */ > > struct alloc_tag *start_tag; > > struct alloc_tag *stop_tag; > > struct alloc_tag *tag; > > > > + /* percpu counters for core allocations are already statically = allocated */ > > if (!mod) > > - return; > > + return 0; > > > > start_tag =3D ct_to_alloc_tag(start); > > stop_tag =3D ct_to_alloc_tag(stop); > > @@ -627,12 +628,13 @@ static void load_module(struct module *mod, struc= t codetag *start, struct codeta > > free_percpu(tag->counters); > > tag->counters =3D NULL; > > } > > - shutdown_mem_profiling(true); > > - pr_err("Failed to allocate memory for allocatio= n tag percpu counters in the module %s. Memory allocation profiling is disa= bled!\n", > > + pr_err("Failed to allocate memory for allocatio= n tag percpu counters in the module %s\n", > > mod->name); > > - break; > > + return -ENOMEM; > > } > > } > > + > > + return 0; > > } > > > > static void replace_module(struct module *mod, struct module *new_mod) > > diff --git a/lib/codetag.c b/lib/codetag.c > > index de332e98d6f5..650d54d7e14d 100644 > > --- a/lib/codetag.c > > +++ b/lib/codetag.c > > @@ -167,6 +167,7 @@ static int codetag_module_init(struct codetag_type = *cttype, struct module *mod) > > { > > struct codetag_range range; > > struct codetag_module *cmod; > > + int mod_id; > > int err; > > > > range =3D get_section_range(mod, cttype->desc.section); > > @@ -190,11 +191,20 @@ static int codetag_module_init(struct codetag_typ= e *cttype, struct module *mod) > > cmod->range =3D range; > > > > down_write(&cttype->mod_lock); > > - err =3D idr_alloc(&cttype->mod_idr, cmod, 0, 0, GFP_KERNEL); > > - if (err >=3D 0) { > > - cttype->count +=3D range_size(cttype, &range); > > - if (cttype->desc.module_load) > > - cttype->desc.module_load(mod, range.start, rang= e.stop); > > + mod_id =3D idr_alloc(&cttype->mod_idr, cmod, 0, 0, GFP_KERNEL); > > + if (mod_id >=3D 0) { > > + if (cttype->desc.module_load) { > > + err =3D cttype->desc.module_load(mod, range.sta= rt, range.stop); > > + if (!err) > > + cttype->count +=3D range_size(cttype, &= range); > > + else > > + idr_remove(&cttype->mod_idr, mod_id); > > + } else { > > + cttype->count +=3D range_size(cttype, &range); > > + err =3D 0; > > + } > > + } else { > > + err =3D mod_id; > > } > > up_write(&cttype->mod_lock); > > > > Overall looks good, just one small nit: should we not increase > cttype->count if there is no module_load callback ? No, a codetag type might not require module_load callback but can still have a non-empty tags section. > Personally I prefer having tag allocation and counter allocation at > the same place in move_module() by calling something like > codetag_alloc_module_tag_counter(). But your approach looks more > modular. I don't have a strong preference, you can choose what you > want. Thanks! Yeah, I try to keep the codetagging footprint in the module loading code as small as possible, so let's avoid adding more hooks there. Thanks, Suren. > > int codetag_alloc_module_tag_counter(struct module *mod, void *start_addr= , > unsigned long size) > { > struct codetag_type *cttype; > int ret =3D -ENODEV; > > mutex_lock(&codetag_lock); > list_for_each_entry(cttype, &codetag_types, link) { > if (WARN_ON(!cttype->desc.alloc_counter_mem)) > break; > > down_write(&cttype->mod_lock); > ret =3D cttype->desc.alloc_counter_mem(mod, start_addr, s= ize); > up_write(&cttype->mod_lock); > break; > } > mutex_unlock(&codetag_lock); > > return ret; > } > > Casey > > > @@ -295,17 +305,23 @@ void codetag_module_replaced(struct module *mod, = struct module *new_mod) > > mutex_unlock(&codetag_lock); > > } > > > > -void codetag_load_module(struct module *mod) > > +int codetag_load_module(struct module *mod) > > { > > struct codetag_type *cttype; > > + int ret =3D 0; > > > > if (!mod) > > - return; > > + return 0; > > > > mutex_lock(&codetag_lock); > > - list_for_each_entry(cttype, &codetag_types, link) > > - codetag_module_init(cttype, mod); > > + list_for_each_entry(cttype, &codetag_types, link) { > > + ret =3D codetag_module_init(cttype, mod); > > + if (ret) > > + break; > > + } > > mutex_unlock(&codetag_lock); > > + > > + return ret; > > } > > > > void codetag_unload_module(struct module *mod) > > > > base-commit: 9f3e87f6c8d4b28b96eb8bddb22d3ba4b846e10b > > -- > > 2.49.0.1112.g889b7c5bd8-goog > >