linux-kernel - Re: [PATCH v7 3/4] ipc: Allow boot time extension of IPCMNI from 32k to 2M

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: <20180507231704.GX27853@wotan.suse.de>
Date:   Mon, 7 May 2018 23:17:04 +0000
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Waiman Long <longman@...hat.com>
Cc:     "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Christoph Lameter <cl@...ux.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jonathan Corbet <corbet@....net>, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-doc@...r.kernel.org,
        Al Viro <viro@...iv.linux.org.uk>, tglx@...utronix.de,
        arnd@...db.de, Matthew Wilcox <willy@...radead.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [PATCH v7 3/4] ipc: Allow boot time extension of IPCMNI from 32k
 to 2M

On Mon, May 07, 2018 at 04:59:11PM -0400, Waiman Long wrote:
> diff --git a/ipc/ipc_sysctl.c b/ipc/ipc_sysctl.c
> index 49f9bf4..d62335f 100644
> --- a/ipc/ipc_sysctl.c
> +++ b/ipc/ipc_sysctl.c
> @@ -120,7 +120,8 @@ static int proc_ipc_sem_dointvec(struct ctl_table *table, int write,
>  static int zero;
>  static int one = 1;
>  static int int_max = INT_MAX;
> -static int ipc_mni = IPCMNI;
> +int ipc_mni __read_mostly = IPCMNI;
> +int ipc_mni_shift __read_mostly = IPCMNI_SHIFT;
>  
>  static struct ctl_table ipc_kern_table[] = {
>  	{

Is use of ipc_mni and ipc_mni_shift a hot path? As per Christoph Lameter,
its use should be reserved for data that is actually used frequently in hot
paths, and typically this was done after performance traces reveal contention
because a neighboring variable was frequently written to [0]. These would also
be tightly packed, to reduce the number of cachelines needed to execute a
critical path, so we should be selective about what variables use it.

Your commit log does not describe why you'd use __read_mostly here. It would
be useful if it did.

[0] https://lkml.kernel.org/r/alpine.DEB.2.11.1504301343190.28879@gentwo.org

  Luis