linux-kernel - Re: [PATCH v2 3/4] x86, mwaitt: introduce mwaix delay with a configurable timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1433869987.1495.53.camel@twins>
Date:	Tue, 09 Jun 2015 19:13:07 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Andy Lutomirski <luto@...capital.net>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	John Stultz <john.stultz@...aro.org>,
	Aaron Lu <aaron.lu@...el.com>, Tony Li <tony.li@....com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Suravee Suthikulanit <suravee.suthikulpanit@....com>,
	Frédéric Weisbecker <fweisbec@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Borislav Petkov <bp@...e.de>,
	Fengguang Wu <fengguang.wu@...el.com>,
	Len Brown <lenb@...nel.org>, Ken Xue <ken.xue@....com>,
	Huang Rui <ray.huang@....com>, X86 ML <x86@...nel.org>
Subject: Re: [PATCH v2 3/4] x86, mwaitt: introduce mwaix delay with a
 configurable timer

On Tue, 2015-06-09 at 09:46 -0700, Andy Lutomirski wrote:
> On Jun 9, 2015 2:30 AM, "Peter Zijlstra" <peterz@...radead.org> wrote:

> > How about you think instead and do something like:
> >
> >         rdtsc(start);
> >         rdtsc_barrier();
> 
> Other way around.  We really need a function static inline u64
> rdtsc_with_barrier().

So admittedly I have not actually looked at how the tsc barrier stuff
works, but what? We don't care if the rdtsc goes up, we just want to
make sure its done before going further.

> 
> >
> >         for (;;) {
> >                 delay = min(MWAIT_MAX_LOOPS, loops);
> >
> >                 __monitorx(&addr, 0, 0);
> >                 mwaitx(delay, true);
> 
> I don't like this hack.  The compiler is entirely within is rights to
> poke addr's cacheline (i.e. the stack) between the two instructions.
> I'd suggest either making the thing a full cacheline long or using a
> single asm statement.
> 
> Also, "addr" is a bad name for a dummy variable that isn't an address
> at all.  How about "dummy"?

Sure, and I like your question on why monitorx exists at all. But none
of that was the point here, the main point being that if loops was too
big, we should do multiple mwaitx invocations, not punt and busy loop.

> >                 rdtsc_barrier();
> >                 rdtsc(end);
> >                 rdtsc_barrier();
> 
> The second barrier is unnecessary.

By virtue of the address dependency?

> >
> >                 loops -= end - start;
> >                 if (loops <= 0)
> >                         break;
> >
> >                 start = end;
> >         }

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