Re: [PATCH] blk-throttle: Fix W=1 build issue in tg_prfill_limit()

John Garry <john.g.garry@xxxxxxxxxx> · Tue, 26 Mar 2024 08:46:24 +0000

On 26/03/2024 07:03, Christoph Hellwig wrote:
On Mon, Mar 25, 2024 at 12:19:05PM +0000, John Garry wrote:
For when using gcc 8 and above, the following warnings can be seen when
compiling blk-throttle.c with W=1:

Why is this function even using these local buffers vs a sequence
of separate seq_printf calls that would get rid of these pointless
on-stack buffers?

Currently a combo of snprintf and seq_printf is used, a strategy which 
seems to go as far back as 2ee867dcfa2ea (2015), when it was a much 
simpler print. All the other code in this area only uses seq_printf, so 
it seems that the author(s) prefer this way here.

Here's how the current code could look (using only seq_printf):

static u64 tg_prfill_limit(struct seq_file *sf, struct blkg_policy_data *pd,
			 int off)
{
	struct throtl_grp *tg = pd_to_tg(pd);
	const char *dname = blkg_dev_name(pd->blkg);
	u64 bps_dft;
	unsigned int iops_dft;

	if (!dname)
		return 0;

	if (off == LIMIT_LOW) {
		bps_dft = 0;
		iops_dft = 0;
	} else {
		bps_dft = U64_MAX;
		iops_dft = UINT_MAX;
	}

	if (tg->bps_conf[READ][off] == bps_dft &&
	    tg->bps_conf[WRITE][off] == bps_dft &&
	    tg->iops_conf[READ][off] == iops_dft &&
	    tg->iops_conf[WRITE][off] == iops_dft &&
	    (off != LIMIT_LOW ||
	     (tg->idletime_threshold_conf == DFL_IDLE_THRESHOLD &&
	      tg->latency_target_conf == DFL_LATENCY_TARGET)))
		return 0;

	seq_printf(sf, "%s rbps=", dname);
	if (tg->bps_conf[READ][off] == U64_MAX)
		seq_printf(sf, "max");
	else
		seq_printf(sf, "%llu", tg->bps_conf[READ][off]);

	if (tg->bps_conf[WRITE][off] == U64_MAX)
		seq_printf(sf, " wbps=max ");
	else
		seq_printf(sf, " wbps=%llu ", tg->bps_conf[WRITE][off]);

	if (tg->iops_conf[READ][off] == UINT_MAX)
		seq_printf(sf, " riops=max");
	else
		seq_printf(sf, " riops=%u", tg->iops_conf[READ][off]);

	if (tg->iops_conf[WRITE][off] == UINT_MAX)
		seq_printf(sf, " wiops=max");
	else
		seq_printf(sf, " wiops=%u", tg->iops_conf[WRITE][off]);

	if (off == LIMIT_LOW) {
		if (tg->idletime_threshold_conf == ULONG_MAX)
			seq_printf(sf, " idle=max");
		else
			seq_printf(sf, " idle=%lu", tg->idletime_threshold_conf);

		if (tg->latency_target_conf == ULONG_MAX)
			seq_printf(sf, " latency=max");
		else
			seq_printf(sf, " latency=%lu", tg->latency_target_conf);
	}

	return 0;
}

better?

Thanks,
John