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

[Vaibhav Jain <vaibhav@linux.vnet.ibm.com>]

Thanks Mpe for reviewing the patch

Michael Ellerman <mpe@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/

set_thread_tidr() is a wrapper around assign_thread_tidr() which follows
similar calling convention i.e return -ve values for error and  +ve
value successfully allocated tidr. The way assign_thread_tidr() is
implemented it will never return a '0'.

Currently set_thread_tidr() will wrongly assign an incorrect tidr value
to SPRN_TIDR in case assign_thread_tidr() returns an error. This patch
fixes this issue and should not impact the existing calling convention
of set_thread_tidr(). The patch should not have an impact on the calling
convention of set_thread_tidr().

I also have unintentionally left a tidr leak in this patch and will send
a v2 with the fix and also updating the patch description mentioning the
calling convention of the function.

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