Thanks Mpe for reviewing the patch Michael Ellerman <[email protected]> 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 <[email protected]> Linux Technology Center, IBM India Pvt. Ltd.
