Re: [PATCH] blk-throttle: Suppress a compiler warning

[Jens Axboe <axboe@kernel.dk>]

On 04/20/2017 09:36 AM, Bart Van Assche wrote:
> On Wed, 2017-04-19 at 18:09 -0600, Jens Axboe wrote:
>> On 04/19/2017 05:55 PM, Bart Van Assche wrote:
>>> Avoid that the following warning is reported when building with
>>> W=1 and with CONFIG_BLK_DEV_THROTTLING_LOW=n:
>>>
>>> block/blk-throttle.c: In function ‘blk_throtl_bio’:
>>> block/blk-throttle.c:2042:6: warning: variable ‘ret’ set but not used [-Wunused-but-set-variable]
>>>   int ret;
>>>       ^~~
>>>
>>> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
>>> Cc: Shaohua Li <shli@fb.com>
>>> ---
>>>  block/blk-throttle.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
>>> index c82bf9b1fe72..9081ed9a5345 100644
>>> --- a/block/blk-throttle.c
>>> +++ b/block/blk-throttle.c
>>> @@ -2059,6 +2059,10 @@ bool blk_throtl_bio(struct request_queue *q, struct blkcg_gq *blkg,
>>>  	if (ret == 0 || ret == -EBUSY)
>>>  		bio->bi_cg_private = tg;
>>>  	blk_stat_set_issue(&bio->bi_issue_stat, bio_sectors(bio));
>>> +#else
>>> +	/* Avoid that the compiler complains about not using ret */
>>> +	if (ret) {
>>> +	}
>>>  #endif
>>
>> Ugh, that may get rid of the warning, but it does not help on
>> the readability or the ifdefs. How about something like the below?
>> Naming could probably be improved...
>>
>>
>> diff --git a/block/blk-throttle.c b/block/blk-throttle.c
>> index c82bf9b1fe72..b78db2e5fdff 100644
>> --- a/block/blk-throttle.c
>> +++ b/block/blk-throttle.c
>> @@ -2030,6 +2030,20 @@ static inline void throtl_update_latency_buckets(struct throtl_data *td)
>>  }
>>  #endif
>>  
>> +static void blk_throtl_assoc_bio(struct throtl_grp *tg, struct bio *bio)
>> +{
>> +#ifdef CONFIG_BLK_DEV_THROTTLING_LOW
>> +	int ret;
>> +
>> +	ret = bio_associate_current(bio);
>> +	if (ret == 0 || ret == -EBUSY)
>> +		bio->bi_cg_private = tg;
>> +	blk_stat_set_issue(&bio->bi_issue_stat, bio_sectors(bio));
>> +#else
>> +	bio_associate_current(bio);
>> +#endif
>> +}
>> +
>>  bool blk_throtl_bio(struct request_queue *q, struct blkcg_gq *blkg,
>>  		    struct bio *bio)
>>  {
>> @@ -2039,7 +2053,6 @@ bool blk_throtl_bio(struct request_queue *q, struct blkcg_gq *blkg,
>>  	bool rw = bio_data_dir(bio);
>>  	bool throttled = false;
>>  	struct throtl_data *td = tg->td;
>> -	int ret;
>>  
>>  	WARN_ON_ONCE(!rcu_read_lock_held());
>>  
>> @@ -2054,12 +2067,7 @@ bool blk_throtl_bio(struct request_queue *q, struct blkcg_gq *blkg,
>>  	if (unlikely(blk_queue_bypass(q)))
>>  		goto out_unlock;
>>  
>> -	ret = bio_associate_current(bio);
>> -#ifdef CONFIG_BLK_DEV_THROTTLING_LOW
>> -	if (ret == 0 || ret == -EBUSY)
>> -		bio->bi_cg_private = tg;
>> -	blk_stat_set_issue(&bio->bi_issue_stat, bio_sectors(bio));
>> -#endif
>> +	blk_throtl_assoc_bio(tg, bio);
>>  	blk_throtl_update_idletime(tg);
>>  
>>  	sq = &tg->service_queue;
>>
> 
> Hello Jens,
> 
> The above patch looks fine to me. Do you want to queue this patch with
> my Reviewed-by or do you prefer that I post the above as a v2?

Thanks for checking, I'll just add it with your reviewed/reported-by
tags.

-- 
Jens Axboe