[PATCH 1/2] powerpc: Avoid signed to unsigned conversion in set_thread_tidr()

[Vaibhav Jain]

Michael Ellerman <mpe at ellerman.id.au> writes:

> Vaibhav Jain <vaibhav at linux.vnet.ibm.com> writes:
>
>> Thanks Mpe for reviewing the patch
>>
>> Michael Ellerman <mpe at ellerman.id.au> writes:
>>
>>>> To fix this the patch assigns the return value of assign_thread_tidr()
>>>> to a temporary int and assigns it to thread.tidr iff its '> 0'.
>>>
>>> .. and changes the calling convention of the function.
>>>
>>> Now it returns -ve error values, or a +ve TIDR value when it succeeds,
>>> or possibly 0 if that's returned by assign_thread_tidr().
>>>
>>> Which I'm not sure you meant to do. If you did, you should at least
>>> document it.
>>
>> Yes this is intentional and this was supposed to be the calling
>> convention of set_thread_tidr() in first place. At-least that what I
>> gather from subsequent cxl patch to add its support
>> http://patchwork.ozlabs.org/patch/840719/
>
> That's not at all what I gather from that patch.
>
> +		/* Assign a unique TIDR (thread id) for the current thread */
> +		rc = set_thread_tidr(current);
> +		if (!rc)
> +			ctx->tid = current->thread.tidr;
>
> That expects 0 on success, anything else is an error.
>
> Which is what set_thread_tidr() currently implements, and is the most
> common calling convention in kernel code.
>
> Please don't change that as part of an unrelated fix.
>
> If you want to change the calling convention, send a patch to do that
> and only that.
>
> cheers
>
Agreed Mpe, checked with Christophe and he too echoed similar inputs. I
will update my v2 patch by not causing a change to he call convention.

-- 
Vaibhav Jain <vaibhav at linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.