Re: [PATCH dwarves 4/5] btf_encoder: represent "."-suffixed optimized functions (".isra.0") in BTF

[Arnaldo Carvalho de Melo <acme@kernel.org>]

Em Wed, Jan 25, 2023 at 09:54:26AM -0800, Kui-Feng Lee escreveu:
> 
> On 1/24/23 05:45, Alan Maguire wrote:
> > +/*
> > + * static functions with suffixes are not added yet - we need to
> > + * observe across all CUs to see if the static function has
> > + * optimized parameters in any CU, since in such a case it should
> > + * not be included in the final BTF.  NF_HOOK.constprop.0() is
> > + * a case in point - it has optimized-out parameters in some CUs
> > + * but not others.  In order to have consistency (since we do not
> > + * know which instance the BTF-specified function signature will
> > + * apply to), we simply skip adding functions which have optimized
> > + * out parameters anywhere.
> > + */
> > +static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct function *fn)
> > +{
> > +	struct btf_encoder *parent = encoder->parent ? encoder->parent : encoder;
> > +	const char *name = function__name(fn);
> > +	struct function **nodep;
> > +	int ret = 0;
> > +
> > +	pthread_mutex_lock(&parent->saved_func_lock);
> 
> Do you have the number of static functions with suffices?

⬢[acme@toolbox linux]$ readelf -sW ../build/v6.2-rc5+/vmlinux | grep -w FUNC | grep '[[:alnum:]]\.' | wc -l
7245
⬢[acme@toolbox linux]$ readelf -sW ../build/v6.2-rc5+/vmlinux | grep -w FUNC | wc -l
122336
⬢[acme@toolbox linux]$ readelf -sW ../build/v6.2-rc5+/vmlinux | grep -w FUNC | grep '[[:alnum:]]\.constprop' | wc -l
1292
⬢[acme@toolbox linux]$ readelf -sW ../build/v6.2-rc5+/vmlinux | grep -w FUNC | grep '[[:alnum:]]\.isra' | wc -l
1148
⬢[acme@toolbox linux]$ readelf -sW ../build/v6.2-rc5+/vmlinux | grep -w FUNC | grep '[[:alnum:]]\.cold' | wc -l
4066
⬢[acme@toolbox linux]$ readelf -sW ../build/v6.2-rc5+/vmlinux | grep -w FUNC | grep '[[:alnum:]]\.part' | wc -l
1013
⬢[acme@toolbox linux]$
 
> If the number of static functions with suffices is high, the contention of
> the lock would be an issue.
> 
> Is it possible to keep a local pool of static functions with suffices? The
> pool will be combined with its parent either at the completion of a CU,
> before ending the thread or when merging into the main thread.

May help, but I think maybe premature optimization is the root of... :-)

- Arnaldo
 
> > +	nodep = tsearch(fn, &parent->saved_func_tree, function__compare);
> > +	if (nodep == NULL) {
> > +		fprintf(stderr, "error: out of memory adding local function '%s'\n",
> > +			name);
> > +		ret = -1;
> > +		goto out;
> > +	}
> > +	/* If we find an existing entry, we want to merge observations
> > +	 * across both functions, checking that the "seen optimized-out
> > +	 * parameters" status is reflected in our tree entry.
> > +	 * If the entry is new, record encoder state required
> > +	 * to add the local function later (encoder + type_id_off)
> > +	 * such that we can add the function later.
> > +	 */
> > +	if (*nodep != fn) {
> > +		(*nodep)->proto.optimized_parms |= fn->proto.optimized_parms;
> > +	} else {
> > +		struct btf_encoder_state *state = zalloc(sizeof(*state));
> > +
> > +		if (state == NULL) {
> > +			fprintf(stderr, "error: out of memory adding local function '%s'\n",
> > +				name);
> > +			ret = -1;
> > +			goto out;
> > +		}
> > +		state->encoder = encoder;
> > +		state->type_id_off = encoder->type_id_off;
> > +		fn->priv = state;
> > +		encoder->saved_func_cnt++;
> > +		if (encoder->verbose)
> > +			printf("added local function '%s'%s\n", name,
> > +			       fn->proto.optimized_parms ?
> > +			       ", optimized-out params" : "");
> > +	}
> > +out:
> > +	pthread_mutex_unlock(&parent->saved_func_lock);
> > +	return ret;
> > +}
> > +

-- 

- Arnaldo