[Devel] [PATCH RH8] ploop: Provide more info about ENOSPC

[Kirill Tkhai]

On 20.10.2021 22:22, Cyrill Gorcunov wrote:
> On Wed, Oct 20, 2021 at 06:13:01PM +0300, Kirill Tkhai wrote:
> ...
>> diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
>> index 327095f75359..bd68d5fb272b 100644
>> --- a/drivers/md/dm-ploop-target.c
>> +++ b/drivers/md/dm-ploop-target.c
>> @@ -455,6 +455,8 @@ static void ploop_status(struct dm_target *ti, status_type_t type,
>>  		p += sprintf(p, "t");
>>  	if (READ_ONCE(ploop->noresume))
>>  		p += sprintf(p, "n");
>> +	if (READ_ONCE(ploop->event_enospc))
>> +		p += sprintf(p, "s");
>>  	if (p == stat)
>>  		p += sprintf(p, "o");
>>  	if (ploop->skip_off)
> 
> While I've no clue what is going on here with this status I wonder why
> we use sprintf here at all? The sprintf is _very_ heavy function which
> consumes too much cycles for nothing, we don't even need any formatting
> here. Why not some simple
> 
> static void ploop_status(struct dm_target *ti, status_type_t type,
> 			 unsigned int status_flags, char *result,
> 			 unsigned int maxlen)
> {
> 	struct ploop *ploop = ti->private;
> 	char stat[16], *p = stat;
> 	ssize_t sz = 0;
> 
> 	down_read(&ploop->ctl_rwsem);
> 	if (ploop->falloc_new_clu)
> 		*p++ = 'f';
> 	if (ploop->tracking_bitmap)
> 		*p++ = 't';
> 	if (READ_ONCE(ploop->noresume))
> 		*p++ = 'n';
> 	if (p == stat)
> 		*p++ = 'o';
> 	*p = '\0';
> 	up_read(&ploop->ctl_rwsem);
> 
> 	BUG_ON(p - stat >= sizeof(stat));
> 	DMEMIT("%u v2 %u %s", ploop->nr_deltas, (u32)CLU_TO_SEC(ploop, 1), stat);
> }
> 
> or I miss something obvious?

Good idea. Could you please provide a proper patch reworking this function on top of my patch?