From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-177.mta0.migadu.com (out-177.mta0.migadu.com [91.218.175.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5613815530C for ; Tue, 22 Jul 2025 23:42:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.177 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753227735; cv=none; b=kIbC3tJNssRQljVtaYvX3qMNjIVa1hOjCqQL7wx1wIPZaaXepeSZeDPdEk6gRRR2AuSWXNbjTADmGpxyd1WWjxZMRQ/Ae5OxajA0WA/6paiFVIEgBnPZyfb1sE7loUZDGuadbcjk3roi1UMcQL3xY2O2sJnBeYmMd6TzWxrs+uM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1753227735; c=relaxed/simple; bh=B1hkkvIjSuI+WzXZiy7u0jfmge0Wzh+ufwW7r6v6zGY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=r1CIr13KJirEaFHJt+P2jT3qf8bZxr+ZoByROqjHXqcsocXh0ITBJP4AfWFnNWnw1jeqJMu7VgA3vQrI+tDPbHWAWMdduNWhSrlvaYZqO4FQ8seYwWYABjxTB8pE3had5Rf1jMTL80s1tlmx1/XlaqY32bjinrGcbzJ0zxKsy0s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=P9Gg3mBM; arc=none smtp.client-ip=91.218.175.177 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="P9Gg3mBM" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1753227730; h=from:from: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; bh=9Kgq+5fehwlaS5JVRgFnnLyXq4r0QKmh0Dw2NVhQ30k=; b=P9Gg3mBMld6JIcAhL6x3g2050rhzpMOLLlB9oElYG9NMjoCVFHUN1JqflRA/1vVl5yIZU4 RQwaZV6cnvwZQCVNwB3jC6aZEQzN5+E7JW+SSGxKdUNoNx/0jQFEOSI59BWfGhmk0Qun6c 3sjRGHImSfcF3jgX07WkqqhQLfzNsD4= Date: Tue, 22 Jul 2025 16:42:03 -0700 Precedence: bulk X-Mailing-List: dwarves@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH] pahole: do not return an error when printing only a specific class To: =?UTF-8?Q?Alexis_Lothor=C3=A9_=28eBPF_Foundation=29?= , dwarves@vger.kernel.org Cc: Alan Maguire , Arnaldo Carvalho de Melo , ebpf@linuxfoundation.org, Thomas Petazzoni , Bastien Curutchet References: <20250716-lsk__abort-v1-1-586f31a9d97d@bootlin.com> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Ihor Solodrai In-Reply-To: <20250716-lsk__abort-v1-1-586f31a9d97d@bootlin.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT On 7/16/25 2:04 AM, Alexis Lothoré (eBPF Foundation) wrote: > When trying to print a specific class layout with recent pahole > versions, we get an error right after the expected class print: > > $ ./build/pahole -C test_bin_struct_packed -F dwarf tests/bin/test_bin > struct test_bin_struct_packed { > char a; /* 0 1 */ > short int b; /* 1 2 */ > int c; /* 3 4 */ > long long unsigned int d; /* 7 8 */ > > /* size: 15, cachelines: 1, members: 4 */ > /* last cacheline: 15 bytes */ > } __attribute__((__packed__)); > > pahole: /home/alexis/src/pahole/tests/bin/test_bin: Invalid argument > > This error is due to pahole_stealer returning LSK__STOP_LOADING when > dealing with a single class and when it is done. This LSK__STOP_LOADING > is then propagated to dwarf_loader__worker_thread and interpreted as an > error (and turned into a DWARF_CB_ABORT). The main issue is that this > LSK__STOP_LOADING value is sometimes generated by actual issues during > pahole stealing, sometimes by "nominal" execution. > > Add a new LSK__ABORT status to distinguish between those nominal and > faulty cases. Return an error only when pahole_stealer returns > LSK__ABORT. > > Signed-off-by: Alexis Lothoré (eBPF Foundation) > --- > See [1] for original discussions about the issue > > [1] https://lore.kernel.org/dwarves/DB5VWHU0N27I.3ETC4G47KB9Q@bootlin.com/ Hi Alexis, thank you for working on this. The change looks good to me, just one comment. > --- > dwarf_loader.c | 6 ++++-- > dwarves.h | 1 + > pahole.c | 21 +++++++++++---------- > 3 files changed, 16 insertions(+), 12 deletions(-) > > diff --git a/dwarf_loader.c b/dwarf_loader.c > index ef00cdf308166732fc0938f2eecd56d314d3354f..4e1e19c9d828e111d78012feda4c8baa0d716378 100644 > --- a/dwarf_loader.c > +++ b/dwarf_loader.c > @@ -3277,6 +3277,8 @@ static int cus__steal_now(struct cus *cus, struct cu *cu, struct conf_load *conf > break; > case LSK__KEEPIT: > break; > + case LSK__ABORT: > + break; > } > return lsk; > } > @@ -3679,7 +3681,7 @@ static void *dwarf_loader__worker_thread(void *arg) > break; > > case JOB_STEAL: > - if (cus__steal_now(dcus->cus, job->cu, dcus->conf) == LSK__STOP_LOADING) > + if (cus__steal_now(dcus->cus, job->cu, dcus->conf) == LSK__ABORT) > goto out_abort; > cus_queue__inc_next_cu_id(); > /* re-enqueue JOB_DECODE so that next CU is decoded from DWARF */ > @@ -3872,7 +3874,7 @@ static int cus__merge_and_process_cu(struct cus *cus, struct conf_load *conf, > goto out_abort; > > cu__finalize(cu, cus, conf); > - if (cus__steal_now(cus, cu, conf) == LSK__STOP_LOADING) > + if (cus__steal_now(cus, cu, conf) == LSK__ABORT) > goto out_abort; I think we should handle both LSK__STOP_LOADING and LSK__ABORT here, otherwise the worker will keep loading DWARF (which is most of the pahole's work) and passing cu-s to the stealer function. I guess in many cases it'll complete without errors, but it's just unnecessary work. Maybe add a check for LSK__STOP_LOADING here with goto_ok? > > return 0; > diff --git a/dwarves.h b/dwarves.h > index 883852f4b60a36d73bce4568cbacd8ef3cff97ea..5df1cdad9be36212bd6548e86180860daffa3f63 100644 > --- a/dwarves.h > +++ b/dwarves.h > @@ -43,6 +43,7 @@ enum load_steal_kind { > LSK__KEEPIT, > LSK__DELETE, > LSK__STOP_LOADING, > + LSK__ABORT, > }; > > /* > diff --git a/pahole.c b/pahole.c > index 333e71ab65924d2c64771be7d40f768f02e9a0f0..66448fc430c19e5137a8467026d35d2ba3125e4c 100644 > --- a/pahole.c > +++ b/pahole.c > @@ -3165,13 +3165,13 @@ static enum load_steal_kind pahole_stealer__btf_encode(struct cu *cu, struct con > > if (!btf_encoder) { > fprintf(stderr, "Error creating BTF encoder.\n"); > - return LSK__STOP_LOADING; > + return LSK__ABORT; > } > > err = btf_encoder__encode_cu(btf_encoder, cu, conf_load); > if (err < 0) { > fprintf(stderr, "Error while encoding BTF.\n"); > - return LSK__STOP_LOADING; > + return LSK__ABORT; > } > > return LSK__DELETE; > @@ -3230,7 +3230,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, struct conf_load *conf > * > * FIXME: Disabled, should use Oracle's libctf > */ > - goto dump_and_stop; > + goto dump_and_abort; > } > #endif > if (class_name == NULL) { > @@ -3281,7 +3281,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, struct conf_load *conf > > if (prototype->nr_args != 0 && !tag__is_struct(class)) { > fprintf(stderr, "pahole: attributes are only supported with 'class' and 'struct' types\n"); > - goto dump_and_stop; > + goto dump_and_abort; > } > > struct type *type = tag__type(class); > @@ -3291,7 +3291,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, struct conf_load *conf > if (type->sizeof_member == NULL) { > fprintf(stderr, "pahole: the sizeof member '%s' wasn't found in the '%s' type\n", > prototype->size, prototype->name); > - goto dump_and_stop; > + goto dump_and_abort; > } > } > > @@ -3300,7 +3300,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, struct conf_load *conf > if (type->type_member == NULL) { > fprintf(stderr, "pahole: the type member '%s' wasn't found in the '%s' type\n", > prototype->type, prototype->name); > - goto dump_and_stop; > + goto dump_and_abort; > } > } > > @@ -3315,7 +3315,7 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, struct conf_load *conf > if (type->filter == NULL) { > fprintf(stderr, "pahole: invalid filter '%s' for '%s'\n", > prototype->filter, prototype->name); > - goto dump_and_stop; > + goto dump_and_abort; > } > } > > @@ -3386,15 +3386,16 @@ static enum load_steal_kind pahole_stealer(struct cu *cu, struct conf_load *conf > /* > * If we found all the entries in --class_name, stop > */ > - if (list_empty(&class_names)) { > -dump_and_stop: > + if (list_empty(&class_names)) > ret = LSK__STOP_LOADING; > - } > dump_it: > if (first_obj_only) > ret = LSK__STOP_LOADING; > filter_it: > return ret; > +dump_and_abort: > + ret = LSK__ABORT; > + return ret; > } > > static int prototypes__add(struct list_head *prototypes, const char *entry) > > --- > base-commit: 9845339bd09f9889dd741ed94f71eb92008dcdfc > change-id: 20250716-lsk__abort-2b517860af28 > > Best regards,