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@xxxxxxxxxxx>
>>> Cc: Shaohua Li <shli@xxxxxx>
>>> ---
>>> 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
