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]
Message-ID: <484E27C9.6030106@bull.net>
Date:	Tue, 10 Jun 2008 09:05:45 +0200
From:	Nadia Derbey <Nadia.Derbey@...l.net>
To:	Solofo.Ramangalahy@...l.net
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [RFC -mm 5/6] sysv ipc: deconnect msgmnb and msgmni deactivation
 and reactivation

Solofo.Ramangalahy@...l.net wrote:
> From: Solofo Ramangalahy <Solofo.Ramangalahy@...l.net>
> 
> The msgmnb and msgmni values are coupled for deactivation and
> reactivation of value computation.
> 
> The uncoupling of msgmn{b,i} for deactivation/reactivation of
> recomputation adds flexibility and testability.
> 
> . Flexibility was discussed during the msgmni series development and
>   ended up with reactivation by negative value on /proc.
> 
> . Testability allows to experiment with the automatic computation of
>   msgmn{b,i} values. For example, if current algorithm does not fit
>   application needs.
> 
> 
> Signed-off-by: Solofo Ramangalahy <Solofo.Ramangalahy@...l.net>
> 
> ---
>  include/linux/ipc_namespace.h |    3 +-
>  ipc/ipc_sysctl.c              |   45 ++++++++++++++++++++++++++++++++----------
>  ipc/ipcns_notifier.c          |    9 --------
>  ipc/msg.c                     |    6 +++++
>  4 files changed, 43 insertions(+), 20 deletions(-)
> 
> Index: b/include/linux/ipc_namespace.h
> ===================================================================
> --- a/include/linux/ipc_namespace.h
> +++ b/include/linux/ipc_namespace.h
> @@ -34,7 +34,9 @@ struct ipc_namespace {
>  
>  	int		msg_ctlmax;
>  	int		msg_ctlmnb;
> +	bool		msg_ctlmnb_activated; /* recompute_msgmnb activation */
>  	int		msg_ctlmni;
> +	bool		msg_ctlmni_activated; /* recompute_msgmni activation */
>  	atomic_t	msg_bytes;
>  	atomic_t	msg_hdrs;
>  
> @@ -53,7 +55,6 @@ extern atomic_t nr_ipc_ns;
>  #define INIT_IPC_NS(ns)		.ns		= &init_ipc_ns,
>  
>  extern int register_ipcns_notifier(struct ipc_namespace *);
> -extern int cond_register_ipcns_notifier(struct ipc_namespace *);
>  extern int unregister_ipcns_notifier(struct ipc_namespace *);
>  extern int ipcns_notify(unsigned long);
>  
> Index: b/ipc/msg.c
> ===================================================================
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -91,6 +91,8 @@ void recompute_msgmni(struct ipc_namespa
>  	unsigned long allowed;
>  	int nb_ns;
>  
> +	if (!ns->msg_ctlmni_activated)
> +		return;
>  	si_meminfo(&i);
>  	allowed = (((i.totalram - i.totalhigh) / MSG_MEM_SCALE) * i.mem_unit)
>  		/ ns->msg_ctlmnb;
> @@ -116,6 +118,8 @@ void recompute_msgmni(struct ipc_namespa
>   */
>  void recompute_msgmnb(struct ipc_namespace *ns)
>  {
> +	if (!ns->msg_ctlmnb_activated)
> +		return;
>  	ns->msg_ctlmnb =
>  		min(MSGMNB * num_online_cpus(), MSGMNB * MSG_CPU_SCALE);
>  }
> @@ -123,6 +127,8 @@ void recompute_msgmnb(struct ipc_namespa
>  void msg_init_ns(struct ipc_namespace *ns)
>  {
>  	ns->msg_ctlmax = MSGMAX;
> +	ns->msg_ctlmnb_activated = true;
> +	ns->msg_ctlmni_activated = true;
>  	recompute_msgmnb(ns);
>  
>  	recompute_msgmni(ns);
> Index: b/ipc/ipc_sysctl.c
> ===================================================================
> --- a/ipc/ipc_sysctl.c
> +++ b/ipc/ipc_sysctl.c
> @@ -33,18 +33,42 @@ static void *get_ipc(ctl_table *table)
>   * add/remove or ipc namespace creation/removal.
>   * They can come back to a recomputable state by being set to a <0 value.
>   */
> -static void tunable_set_callback(int val)
> +static void tunable_set_callback(int val, ctl_table *table)
>  {
> -	if (val >= 0)
> -		unregister_ipcns_notifier(current->nsproxy->ipc_ns);
> -	else {
> +	int tunable = table->ctl_name;
> +
> +	if (val >= 0) {
> +		switch (tunable) {
> +		case KERN_MSGMNB:
> +			current->nsproxy->ipc_ns->msg_ctlmnb_activated = false;
> +			break;
> +		case KERN_MSGMNI:
> +			current->nsproxy->ipc_ns->msg_ctlmni_activated = false;
> +			break;
> +		default:
> +			printk(KERN_ERR "ipc: unexpected value %s\n",
> +			       table->procname);
> +			break;
> +		}
> +	} else {
>  		/*
>  		 * Re-enable automatic recomputing only if not already
>  		 * enabled.
>  		 */
> -		recompute_msgmnb(current->nsproxy->ipc_ns);
> -		recompute_msgmni(current->nsproxy->ipc_ns);
> -		cond_register_ipcns_notifier(current->nsproxy->ipc_ns);
> +		switch (tunable) {
> +		case KERN_MSGMNB:
> +			current->nsproxy->ipc_ns->msg_ctlmnb_activated = true;
> +			recompute_msgmnb(current->nsproxy->ipc_ns);
> +							/* fall through */

You shouldn't be falling through here: if you do that, re-enablng msgmnb 
will re-enable msgmni too.

> +		case KERN_MSGMNI:
> +			current->nsproxy->ipc_ns->msg_ctlmni_activated = true;
> +			recompute_msgmni(current->nsproxy->ipc_ns);
> +			break;
> +		default:
> +			printk(KERN_ERR "ipc: unexpected value %s\n",
> +			       table->procname);
> +			break;
> +		}
>  	}
>  }
>  
> @@ -72,7 +96,8 @@ static int proc_ipc_callback_dointvec(ct
>  	rc = proc_dointvec(&ipc_table, write, filp, buffer, lenp, ppos);
>  
>  	if (write && !rc && lenp_bef == *lenp)
> -		tunable_set_callback(*((int *)(ipc_table.data)));
> +		BUG_ON(table == NULL);
> +		tunable_set_callback(*((int *)(ipc_table.data)), table);
>  
>  	return rc;
>  }
> @@ -148,8 +173,8 @@ static int sysctl_ipc_registered_data(ct
>  		 * Tunable has successfully been changed from userland
>  		 */
>  		int *data = get_ipc(table);
> -
> -		tunable_set_callback(*data);
> +		BUG_ON(table == NULL);
> +		tunable_set_callback(*data, table);
>  	}
>  
>  	return rc;
> Index: b/ipc/ipcns_notifier.c
> ===================================================================
> --- a/ipc/ipcns_notifier.c
> +++ b/ipc/ipcns_notifier.c
> @@ -65,15 +65,6 @@ int register_ipcns_notifier(struct ipc_n
>  	return blocking_notifier_chain_register(&ipcns_chain, &ns->ipcns_nb);
>  }
>  
> -int cond_register_ipcns_notifier(struct ipc_namespace *ns)
> -{
> -	memset(&ns->ipcns_nb, 0, sizeof(ns->ipcns_nb));
> -	ns->ipcns_nb.notifier_call = ipcns_callback;
> -	ns->ipcns_nb.priority = IPCNS_CALLBACK_PRI;
> -	return blocking_notifier_chain_cond_register(&ipcns_chain,
> -							&ns->ipcns_nb);
> -}
> -
>  int unregister_ipcns_notifier(struct ipc_namespace *ns)
>  {
>  	return blocking_notifier_chain_unregister(&ipcns_chain,
> 

Doing this, we are completly loosing the benefits of the notification 
chains: since the the notifier blocks remain registered + we are 
unconditionally adding a test at the top of each recompute routine. But 
the other choice would hve been to define another notifier chain 
dedicated to msgmnb. I'm not convinced about what is the best solution?

Regards,
Nadia

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ