On 2020/05/18 23:31, Bart Van Assche wrote:
> On 2020-05-17 20:12, Damien Le Moal wrote:
>> On 2020/05/18 11:56, Bart Van Assche wrote:
>>> On 2020-05-17 19:10, Damien Le Moal wrote:
>>>> On 2020/05/18 10:32, Bart Van Assche wrote:
>>>>> On 2020-05-17 18:12, Damien Le Moal wrote:
>>>>>> On 2020/05/16 9:19, Bart Van Assche wrote:
>>>>>>> +static void nullb_zero_rq_data_buffer(const struct request *rq)
>>>>>>> +{
>>>>>>> + struct req_iterator iter;
>>>>>>> + struct bio_vec bvec;
>>>>>>> +
>>>>>>> + rq_for_each_bvec(bvec, rq, iter)
>>>>>>> + zero_fill_bvec(&bvec);
>>>>>>> +}
>>>>>>> +
>>>>>>> +static void nullb_zero_read_cmd_buffer(struct nullb_cmd *cmd)
>>>>>>> +{
>>>>>>> + struct nullb_device *dev = cmd->nq->dev;
>>>>>>> +
>>>>>>> + if (dev->queue_mode == NULL_Q_BIO && bio_op(cmd->bio) == REQ_OP_READ)
>>>>>>> + zero_fill_bio(cmd->bio);
>>>>>>> + else if (req_op(cmd->rq) == REQ_OP_READ)
>>>>>>> + nullb_zero_rq_data_buffer(cmd->rq);
>>>>>>> +}
>>>>>>
>>>>>> Shouldn't the definition of these two functions be under a "#ifdef CONFIG_KMSAN" ?
>>>>>
>>>>> It is on purpose that I used IS_ENABLED(CONFIG_KMSAN) below instead of
>>>>> #ifdef CONFIG_KMSAN. CONFIG_KMSAN is not yet upstream and I want to
>>>>> expose the above code to the build robot.
>>>>
>>>> But then you will get a "defined but unused" build warning, no ?
>>>
>>> Not when using IS_ENABLED(...).
>>
>> I do not understand: the "if (IS_ENABLED(CONFIG_KMSAN))" will be compiled out if
>> CONFIG_KMSAN is not enabled/defined, but the function definitions will still
>> remain, won't they ? That will lead to "defined but unused" warning, no ? What
>> am I missing here ?
>
> "if (IS_ENABLED(CONFIG_KMSAN))" won't be removed by the preprocessor.
> The preprocessor will convert it into if (0).
>
> This is what I found in the gcc documentation about -Wunused-function:
> "-Wunused-function
> Warn whenever a static function is declared but not defined or a
> non-inline static function is unused. This warning is enabled by -Wall."
> I think that "if (0) func(...)" counts as using func().
Makes sense. Thanks for the explanation.
But from code-size perspective, I think it would still make sense to add the
#ifdef CONFIG_KMSAN around the zeroing functions.
>
> Bart.
>
--
Damien Le Moal
Western Digital Research
