Re: [PATCH] mm/damon/dbgfs: remove unnecessary variables

[Rongwei Wang <rongwei.wang@linux.alibaba.com>]

On 10/13/21 11:04 PM, SeongJae Park wrote:
> Hi Rongwei,
> 
> 
> Thank you for this patch!  Looks good to me overall.  I left a couple of
> nitpicks below, though.
> 
> On Wed, 13 Oct 2021 21:09:01 +0800 Rongwei Wang <rongwei.wang@linux.alibaba.com> wrote:
> 
> [...]
>> @@ -352,7 +350,7 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
>>   
>>   	nrs = kbuf;
>>   
>> -	targets = str_to_target_ids(nrs, ret, &nr_targets);
>> +	targets = str_to_target_ids(nrs, count, &nr_targets);
>>   	if (!targets) {
>>   		ret = -ENOMEM;
>>   		goto out;
>> @@ -378,12 +376,12 @@ static ssize_t dbgfs_target_ids_write(struct file *file,
>>   		goto unlock_out;
>>   	}
>>   
>> -	err = damon_set_targets(ctx, targets, nr_targets);
>> -	if (err) {
>> +	ret = damon_set_targets(ctx, targets, nr_targets);
>> +	if (ret < 0) {
> 
> I'd prefer 'if (ret) {', to be consistent with other part.
Hi

In fact, explicit style, such as 'if (ret < 0)', may be more 
understandable. But I agree with your advice that to be consistent
with other part. And I will update it like your said.
> 
>>   		if (targetid_is_pid(ctx))
>>   			dbgfs_put_pids(targets, nr_targets);
>> -		ret = err;
>> -	}
>> +	} else
>> +		ret = count;
> 
> I'd prefer this to have braces:
> https://docs.kernel.org/process/coding-style.html#placing-braces-and-spaces
Agree! I will update here.

Thanks!
> 
>>   
>>   unlock_out:
>>   	mutex_unlock(&ctx->kdamond_lock);
>> @@ -548,8 +546,7 @@ static ssize_t dbgfs_mk_context_write(struct file *file,
> [...]
> 
> 
> Thanks,
> SJ
>