Re: [PATCH 2/2] x86/asm/entry/32: Remove unnecessary optimization in stub32_clone

From: Ingo Molnar
Date: Fri Jun 05 2015 - 07:44:12 EST



* Josh Triplett <josh@xxxxxxxxxxxxxxxx> wrote:

> On Thu, Jun 04, 2015 at 12:07:31PM +0200, Denys Vlasenko wrote:
> > On 06/03/2015 06:38 PM, Josh Triplett wrote:
> > > On Wed, Jun 03, 2015 at 03:58:50PM +0200, Denys Vlasenko wrote:
> > >> Really swap arguments #4 and #5 in stub32_clone instead of "optimizing"
> > >> it into a move.
> > >>
> > >> Yes, tls_val is currently unused. Yes, on some CPUs XCHG is a little bit
> > >> more expensive than MOV. But a cycle or two on an expensive syscall like
> > >> clone() is way below noise floor, and this optimization is simply not worth
> > >> the obfuscation of logic.
> > > [...]
> > >> This is a resend.
> > >>
> > >> There was a patch by Josh Triplett
> > >> "x86: Opt into HAVE_COPY_THREAD_TLS, for both 32-bit and 64-bit"
> > >> sent on May 11,
> > >> which does the same thing as part of a bigger cleanup.
> > >> He was supportive of this patch because of comments.
> > >> He will simply have to drop one hunk from his patch.
> > >
> > > Strictly speaking, nothing needs this until clone starts paying
> > > attention to its tls argument, which only happens in my cleanup series
> > > that includes this change. So what's the purpose of driving this patch
> > > separately?
> >
> > You wanted my comments in this patch to go in:
> >
> > On 04/22/2015 07:10 PM, Josh Triplett wrote:
> > > I do think my two-patch HAVE_COPY_THREAD_TLS series should go in fixing
> > > this, but I'd like to see the final version of Denys' comment added on
> > > top of it (with an update to the type and name of the tls argument to
> > > match the changes to sys_clone).
> >
> > If your patch will go in first, I'll send a patch adding only the comment.
> >
> > Since for now your patch did not make it yet, I'm submitting
> > a patch which adds both a comment and the insn change.
>
> Ah, I see.
>
> My two-patch series is currently sitting in -mm; would you consider providing a
> version of the patch that adds the comment for Andrew to apply on top of those?

So because -mm is based on linux-next, and linux-next includes the x86 tree,
amongst them the x86/asm topic, the dependency is the other way around.

Unrelated to these changes to the clone stub there will be conflicts anyway due to
larger reorganization in the x86 assembly code: so I've picked up Denys's two
patches, thus your patch set in -mm will become a bit smaller in the end, because
it can rely on having these changes applied already.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/