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

Michael Ellerman mpe at ellerman.id.au
Mon Nov 27 14:44:02 AEDT 2017


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


More information about the Linuxppc-dev mailing list