linux-kernel - Re: [RFC PATCH 1/2] ipc: introduce obtaining a lockless ipc object

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] [day] [month] [year] [list]

Date:	Sat, 2 Mar 2013 22:08:15 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Davidlohr Bueso <davidlohr.bueso@...com>
Cc:	Emmanuel Benisty <benisty.e@...il.com>,
	Rik van Riel <riel@...hat.com>,
	"Vinod, Chegu" <chegu_vinod@...com>,
	"Low, Jason" <jason.low2@...com>,
	linux-tip-commits@...r.kernel.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>, aquini@...hat.com,
	Michel Lespinasse <walken@...gle.com>,
	Ingo Molnar <mingo@...nel.org>,
	Larry Woodman <lwoodman@...hat.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [RFC PATCH 1/2] ipc: introduce obtaining a lockless ipc object

On Sat, Mar 2, 2013 at 9:32 PM, Davidlohr Bueso <davidlohr.bueso@...com> wrote:
>
> After updating the callers, [msgctl, semctl, shmctl]_down, to acquire
> the lock for IPC_RMID and IPC_SET commands, I'm no longer seeing these
> issues - so far on my regular laptop and two big boxes running my Oracle
> benchmarks for a few hours. Something like below (yes, I will address
> the open coded spin_lock calls):

Actually, please do me a favor, and do *not* change the semantics of
these helper calls without changing the name of the helper.

So I'd ask that instead of changing the callers, do this:

 - make the version that does *not* lock be called ipcctl_pre_down_nolock()

 - then, keep the old name, and just make it do the
ipcctl_pre_down_nolock() followed by the spin_lock() (or rather, the
non-open-coded one).

Then, you can make each caller individually use the "nolock" version
instead, and move the locking into the caller. But don't do the same
mistake as the original patch, which was to change the locking
semantics while keeping the same name of the function.

In other words, it's much better to make these changes in small
gradual pieces, and make each piece very obviously not change any
semantics. So the first piece is the introduce the helper functions
with new semantics (and new names), while keeping the old helpers with
the old semantics and old names. Make it really clear that no
semantics change. And the second piece is then to start using the
split-up helpers that have different locking semantics and new names,
and do it on a call-by-call basis.

Ok?

              Linus
--
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/