From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1753799AbdHIQPo (ORCPT <rfc822;w@1wt.eu>);
        Wed, 9 Aug 2017 12:15:44 -0400
Received: from mail-wr0-f173.google.com ([209.85.128.173]:34798 "EHLO
        mail-wr0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752642AbdHIQPn (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Wed, 9 Aug 2017 12:15:43 -0400
Date: Wed, 9 Aug 2017 18:15:37 +0200
From: Guillaume Knispel <guillaume.knispel@supersonicimagine.com>
To: Davidlohr Bueso <dave@stgolabs.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
        Manfred Spraul <manfred@colorfullife.com>,
        Kees Cook <keescook@chromium.org>,
        Alexey Dobriyan <adobriyan@gmail.com>,
        "Eric W. Biederman" <ebiederm@xmission.com>,
        "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
        Serge Hallyn <serge@hallyn.com>, Andrey Vagin <avagin@openvz.org>,
        Marc Pardo <marc.pardo@supersonicimagine.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ipc: optimize semget/shmget/msgget for lots of keys
Message-ID: <20170809161537.gvuoc6c2p7wdluh4@debix>
References: <20170731084237.GA123231@ubuntu>
 <20170807182103.GC25038@linux-80c1.suse>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20170807182103.GC25038@linux-80c1.suse>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Aug 07, 2017 at 11:21:03AM -0700, Davidlohr Bueso wrote:
> On Mon, 31 Jul 2017, Guillaume Knispel wrote:
> > struct ipc_ids {
> > 	int in_use;
> > 	unsigned short seq;
> > +	bool tables_initialized;
> 
> So this is really ugly to have, but I understand why you added it. I
> wonder what folks would think if we just panic() in the rhashtable_init()
> ENOMEM case, and convert the EINVALs to WARNs. This way the function
> would always be called successfully. This is similar to what futex_init
> does, with the underlying hash table allocator panicing. sems and msg
> would probably have to be converted to pure_initcall, but hey, we could
> at least get the symmetry back.

I think we could only afford to panic() on ENOMEM during boot, but
ipc_init_ids() is also called through create_ipc_ns() on namespace
creation. Besides, I would not be very comfortable with only warning on
EINVAL but continuing execution using potentially uninitialized data.
Granted, this will probably never happen in production, but the intent
was to leave the system usable (except that it would not be possible to
create sysv ipc objects) with no risk of additionnal crash for cases
like people hacking rhashtable and testing their modifications, if they
merely introduce a correctly reported error.

Cheers!
Guillaume