linux-kernel - Re: [PATCH 1/3] irq_work: Implement remote queueing

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: <20140514124150.GB11096@twins.programming.kicks-ass.net>
Date:	Wed, 14 May 2014 14:41:50 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	Kevin Hilman <khilman@...aro.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Viresh Kumar <viresh.kumar@...aro.org>
Subject: Re: [PATCH 1/3] irq_work: Implement remote queueing

On Wed, May 14, 2014 at 02:11:25PM +0200, Frederic Weisbecker wrote:
> > I don't think it is, most apic calls do apic_wait_icr_idle() then the
> > apic op, if an NMI happens in between and writes to the APIC, the return
> > context will see a !idle icr and fail.
> > 
> > This is why arch_irq_work_raise() again idles the icr after sending the
> > IPI.
> > 
> > Also, I think, seeing what benh said earlier, its unsafe for other archs
> > too.
> 
> Ah I don't know much these archs details, so I concede it.

Yeah, I didn't either, had to figure it out when someone asked WTH there
was an wait_icr_idle call in there.

> > Then do the remote irq_work_raised thing. But it really stinks you broke
> > this very nice and simple thing.
> 
> I tried not to break boot with printk overhead. That said I've considered having
> a very simple "tick work" that can rely on irq work when the tick is stopped
> and use it for printk. That would restore the initial simplicity.

But but but.. did you even try without the lazy thing?

Don't fix what ain't broken, keep it simple, etc..

Anyway, if it turns out to really be needed, the split list doesn't
sound bad.

> > > Also note that nohz is the only user for now and irq_work_claim() thus
> > > prevents from double IPI. Of course if more users come up the issue arise
> > > again.
> > 
> > DANGER, half arsed engineering at work, seriously? Just write proper
> > code already.
> > 
> > There's no fucking way the next user will check the implementation to
> > make sure its 'sane'.
> 
> Are you competing with tglx on grumpiness? You guys are free to treat us
> like shit but don't be surprised if one day you'll be alone in kernel/*

There's really only so much nonsense one can take on any one day before
getting seriously grumpy.

And arguing that because there's only one user so we can skimp a core
function really tops the day.

So maybe I need a holiday, but shees.

Content of type "application/pgp-signature" skipped