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 97410C54E65 for ; Wed, 21 May 2025 21:04:23 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A5A826B0083; Wed, 21 May 2025 17:04:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A0AF16B0085; Wed, 21 May 2025 17:04:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8F9B76B0088; Wed, 21 May 2025 17:04:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 707E56B0083 for ; Wed, 21 May 2025 17:04:22 -0400 (EDT) Received: from smtpin30.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id D43AA141470 for ; Wed, 21 May 2025 21:04:21 +0000 (UTC) X-FDA: 83468143122.30.558ED2D Received: from mail-pj1-f50.google.com (mail-pj1-f50.google.com [209.85.216.50]) by imf15.hostedemail.com (Postfix) with ESMTP id CFBBBA0019 for ; Wed, 21 May 2025 21:04:19 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=purestorage.com header.s=google2022 header.b=UnBecZsM; dmarc=pass (policy=reject) header.from=purestorage.com; spf=pass (imf15.hostedemail.com: domain of cachen@purestorage.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=cachen@purestorage.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1747861460; 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=a8E8wJ2KkrPpB4/N0ejd9jMNIZBIK2MiFknrDOo+EPg=; b=4JY2o6jDBPkY7H30T0KAX3RQ178FQETo4DAzRc6uPBTzTCdJ6mYRPjIQ34zQTCD31mp+7G SVRWN5aztLiuKzKUQrsIbKnDjBFfcYtvrD7mH13qPXA6irorSN1VtCe492HIIAs/Tn3Gcw jrg27YKbUTnZaxjQdlTKjs7ts2xTgmA= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=purestorage.com header.s=google2022 header.b=UnBecZsM; dmarc=pass (policy=reject) header.from=purestorage.com; spf=pass (imf15.hostedemail.com: domain of cachen@purestorage.com designates 209.85.216.50 as permitted sender) smtp.mailfrom=cachen@purestorage.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1747861460; a=rsa-sha256; cv=none; b=7AmKPdbgHSYN9TvrjqD1UbzJhLTzUmSnVGLteAJ5C6TZK+rSCAIO6h8qggZjDzUfocOUxP C4RczG9SP287hYT7/bYYkhA8tRcpkYk9TjeHDPWr3PEZrFgfvd3sXnFQLQzNddcW58iDR6 xX7XOvoBop9T9MewBA8MP43gq1/YgZ8= Received: by mail-pj1-f50.google.com with SMTP id 98e67ed59e1d1-30e7d9e9a47so939323a91.3 for ; Wed, 21 May 2025 14:04:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google2022; t=1747861459; x=1748466259; 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=a8E8wJ2KkrPpB4/N0ejd9jMNIZBIK2MiFknrDOo+EPg=; b=UnBecZsMjDWBBUi0uM9PjW1+cW3Xc+01iEDhe8ZgrAr650dumAXmMDFwDGJvrNKl0j irN2mVzLm/6yJd6/P5a+LiEYDtndEMkVkEqB2wEZnHsuWiFZpigFZoACv9V+iriqyaB5 iFDOYpwFoG4z+MeVIG8clFww21TNh6W/cZC1Tmp0j6Fi4xGyO8xN1AxUiU+393L2E3X0 L5HL4gsJLkWhuG1wbE2+kQfR/6fdqFbhrcnIFp0FerG3OpgV80G+3QG9gqM1w0mqJd8W to4VCiXa7W2EFP/F24pYKLb+hlP5ZHBLJ6xTbsE+nftBkOKbJADToi//lSsqJ6MDypKO dHXA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747861459; x=1748466259; 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=a8E8wJ2KkrPpB4/N0ejd9jMNIZBIK2MiFknrDOo+EPg=; b=iZy6OaBlBKpxnPkZPIp3LlHiSbO19+Axx8Odb/0TN+1H1zYdGALJDCx0mIDfp94lkg 80Iu/GywBKCAqX6t1ilKl3WvaaXidZ3Vnk3MtMDfgRCVFO+bCBcTRYiqTt7TIckS8SsY 49/B+F34w35Zp35A1dcmBM6x01/rSh2W5krkmefgni0HyiQkmxMvw8NbXOYs8yh7yUxV PG7XVG4uMtaFCpRyHlE7TJpxk+IYOktl+DrydUZJrhr9HOOAU7aDAFcVXhFvDARqDr/U k54gc3wdlw/SCWC766FCgObHT2Qw6VakxrFrl34zeR7Mm02fFQJT4a9maPW4Gfw6VVnl IRgg== X-Forwarded-Encrypted: i=1; AJvYcCWaX2FQKTRC6hKUHmv8ymgSsR0KkoWK+E3mwU+8syc6BWIecxijAG2H01RG5NrToZadcf016MZVrA==@kvack.org X-Gm-Message-State: AOJu0YwzPxZmITuKmZHmOK69UyR2yhnQg+0drPKMiVq+uETirCMDiNqf Q2EQtvnWZVr3gpus0FJ92RJggP3iUZjQFoRKp3fjmw5c2RUTjp5Zr4Dfjp1ToeuNdA+hkoa9BCb egluA589XyKo/yc26pncSfP5LFnMZxG3WJ9d7UvraOQ== X-Gm-Gg: ASbGncv6OC6pq142GtVrgJvW+62sztMJMZN6pJQ/wuB454zyXBb5XZ5fLuiB3A5LTrs 8zTFh8Btj6k6dWqkk3vA0l0eMh6WKDXCFxo2TVv7PKhsbSom4K5ByB6vuMPqhQwmQwqh61AlEnM 4ZoEd/1SiczZkFrsd2lV11weDu//qEtuoZ X-Google-Smtp-Source: AGHT+IFrKr2jOATSh5phGB7JDX3d0p2qTLIwC0enPg9MgjlANV5ZNeu2VZnogm+JbvBPSdfOD03MBfWftCNR0tBRptM= X-Received: by 2002:a17:90b:3142:b0:30e:6ac1:372a with SMTP id 98e67ed59e1d1-30e7d691099mr12156692a91.7.1747861458366; Wed, 21 May 2025 14:04:18 -0700 (PDT) MIME-Version: 1.0 References: <20250521160602.1940771-1-surenb@google.com> In-Reply-To: <20250521160602.1940771-1-surenb@google.com> From: Casey Chen Date: Wed, 21 May 2025 14:04:07 -0700 X-Gm-Features: AX0GCFvHkqtUFc-QqB9enzcf-M5GCvbCgMbexO38pxocJAzfC9FE27WWxlR2Ro8 Message-ID: Subject: Re: [PATCH 1/1] alloc_tag: handle module codetag load errors as module load failures To: Suren Baghdasaryan 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: rspam05 X-Rspamd-Queue-Id: CFBBBA0019 X-Stat-Signature: pg19a5mbq3pk1mhqaqasrmp1rrch9439 X-Rspam-User: X-HE-Tag: 1747861459-339013 X-HE-Meta: U2FsdGVkX1/Xfv6I2T5ialVuKc8IqacMiEZfz42I+5QXZuNLTBbtgG6QM8DsaDxqAo3LSLF7S2Ufh37pk2GT8+iaswLgCJEq7BcNpXw3NOkXAjpRtFeEqlcxNMLP3usiCr67K8kUt30Sf8D9ZeZiAcfxWNd8BexXkvHElkz3QO7rV7fKelUbWXwSHfhllqYwNZwRaJkOCpXQgIG4HyyixA7M1oaWM4SVw1q0YWzfIZ1zY7TR8+6tV46QA4s44zZ5dwPklpU7+vk1AiulN34ylWFcCUOe63ZrTkt93HzpS44OrvaMnhspXpSH6R6/9GrzxLF7Z2yzHJOR3+R5YpeDo5BdpP50sCPIYNXRHPsQ3Y0b/gaMBBudQaSLKmWuPtTEsF8Gy2upVaKj5K4jRAdgc8QzoV3zj8ecYHCsmRf4z2ephc0GFP0b9YxaGA4rv0JPaE0mh55AuaQwP5P0rd2a89gykxwvWLCJ7lMEphVsEGrnrirmv+4Atnv3CSDIK3VbOPz8RPUg6Tt2gWiOCTUVm8m+Ap8QshzBTxgnh3MpPMv+YY2g1HPNGeKXywp6JBJpEzHfhK5bvrCPy+bBQZFvWK+RGKSBbpkWgAG6X+BU+TIJKq9Q5YAKJj/cKeM5/oHRsReVwK+Zi22Y00LgY61WtuIx7ng3/juK4/Xqry/cUP47c30YJLEmjLhwS8Cp+hp3zL4kGx82nZBkczhEANLUe1mGCHo13G1mYEk8jEaPO8+6hHk1oABvogA+26h/SvUIJ6uFVeYluFZMQH85ByZW/0ufEgdqi3pwqOrS3VPk3f8xP11ma846Jj5pr/XwIP3Qv3QtRJw95pmJVBsjMZ6+x2pGZott4UPOhhmLUf0EGjadvDP7C6d/py+vKboVHvfLzlIHEWYwFAZj8hP5yTMX6+xQ8msVgp/UrNH9AJb/dh9EtzXsDVcMSYccQFaq+sVFxTrNCGl5OkHlZe+6wpE wOOxPQ+N EVpw4oxRQL6UHeocqHshrwKXXa/pEihEhFYge0kkn2Qa2Jmx+43hR1Id4GQ/asq7shbKRr6eTlXsd+hTg4I4epWYhIsq/CDg/6qWda3ierj0nzOBZynt2D9AzlIuu96ZlJpfDEa3e/gdeKLaDoNLuUx6ZHTn5nWYF+oKTZm7kwyFO0W/VniTiFJ7UE5YkOpQFByKK/4iy9WqEC8OO/ieUafOsdhqtztVmq0xfBCqaVU64cycFB+TSX6EokfUw9FB+eh7Yylxc29z6Xhy13Db9vA4aR/lGy7AVNhHIkz5lXRml+K2AU6iaTwYQM0UqWwUAf2BmQ0VPhcMPKCGawmEX7oiPdToZY9ZOWdqlHx0Icn9DYpaWSmxFlGGnaQ== 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 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 not > be ignored and will cause a module load failure and freeing of resources. > 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 tags= dynamically") > Reported-by: Casey Chen > Closes: https://lore.kernel.org/all/20250520231620.15259-1-cachen@puresto= rage.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 *end)= ; > #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_mod)= ; > -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, cons= t 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 mo= dule *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, co= nst 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 *mod,= bool used) > mas_unlock(&mas); > } > > -static void load_module(struct module *mod, struct codetag *start, struc= t codetag *stop) > +static int load_module(struct module *mod, struct codetag *start, struct= 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 al= located */ > 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, struct = codetag *start, struct codeta > free_percpu(tag->counters); > tag->counters =3D NULL; > } > - shutdown_mem_profiling(true); > - pr_err("Failed to allocate memory for allocation = tag percpu counters in the module %s. Memory allocation profiling is disabl= ed!\n", > + pr_err("Failed to allocate memory for allocation = 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 *c= ttype, 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_type = *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, range.= 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.start= , range.stop); > + if (!err) > + cttype->count +=3D range_size(cttype, &ra= nge); > + 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 ? 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! 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, siz= e); up_write(&cttype->mod_lock); break; } mutex_unlock(&codetag_lock); return ret; } Casey > @@ -295,17 +305,23 @@ void codetag_module_replaced(struct module *mod, st= ruct 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 >