linux-kernel - Re: [PATCH] shrink task

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: <20070521100400.GA11351@infradead.org>
Date:	Mon, 21 May 2007 11:04:00 +0100
From:	Christoph Hellwig <hch@...radead.org>
To:	Davi Arnaut <davi@...ent.com.br>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Dave Airlie <airlied@...ux.ie>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] shrink task_struct by 16 bytes

On Sun, May 20, 2007 at 12:40:11AM -0300, Davi Arnaut wrote:
> Hi,
> 
> Shrink task_struct by replacing the notifier callback with a
> notifier list. The block_all_signals() function (and the signal
> notifier mechanism) has only one user at the moment, which is drm.

This looks like a nice improvement.  Some comments below:

> -	int (*notifier)(void *priv);
> -	void *notifier_data;
> -	sigset_t *notifier_mask;
> -	
> +	/* signal notifier list */
> +	struct list_head notifier_list;
> +

This filed should have a more descripvtive name, e.g. singal_notifier_list.
Also it's probably fine to use a hlist to save another pointer in
struct task_struct.

> +	struct signal_notifier *tmp, *notifier;
>  	int sig = next_signal(pending, mask);
>  
> -	if (sig) {
> -		if (current->notifier) {
> -			if (sigismember(current->notifier_mask, sig)) {
> -				if (!(current->notifier)(current->notifier_data)) {
> -					clear_thread_flag(TIF_SIGPENDING);
> -					return 0;
> -				}
> +	if (unlikely(!sig))
> +		return 0;
> +
> +	list_for_each_entry_safe(notifier, tmp, &current->notifier_list, list) {
> +		if (sigismember(&notifier->mask, sig)) {
> +			if (!(notifier->func)(notifier->data)) {

	Normal kernel stile would be

			if (!notifier->func(notifier->data)) {

The function to add/remove the notifier should grow kerneldoc comments
while you're at it, and maybe more descriptive names.

Also the patch does an actual functional change because it allows for
multiple clients to register the notification, which is probably useful.
-
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/