linux-kernel - Re: [PATCH] posix-cpu-timers: Merge running and checking

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:	Tue, 20 Oct 2015 11:15:45 -0700
From:	Jason Low <jason.low@....com>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	Thomas Gleixner <tglx@...utronix.de>,
	LKML <linux-kernel@...r.kernel.org>,
	George Spelvin <linux@...izon.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Davidlohr Bueso <dave@...olabs.net>,
	Oleg Nesterov <oleg@...hat.com>,
	"Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
	Jason Low <jason.low2@...com>, Ingo Molnar <mingo@...nel.org>,
	jason.low2@...com
Subject: Re: [PATCH] posix-cpu-timers: Merge running and checking_timer
 state in one field

On Tue, 2015-10-20 at 02:18 +0200, Frederic Weisbecker wrote:
> This way we might consume less space in the signal struct (well,
> depending on bool size or padding) and we don't need to worry about
> ordering between the running and checking_timers fields.

This looks fine to me. I ended up going with booleans since I thought
that makes the code more readable, but this method would be okay too.

I do have 1 question below.

> +/* struct thread_group_cputimer::state bits */
> +#define CPUTIMER_STATE_RUNNING		1
> +#define CPUTIMER_STATE_CHECKING		2
> +
>  /**
>   * struct thread_group_cputimer - thread group interval timer counts
>   * @cputime_atomic:	atomic thread group interval timers.
> - * @running:		true when there are timers running and
> - *			@cputime_atomic receives updates.
> - * @checking_timer:	true when a thread in the group is in the
> - *			process of checking for thread group timers.
> - *
> + * @state:		flags describing the current state of the cputimer.
> + *			CPUTIMER_STATE_RUNNING bit means the timers is elapsing.
> + * 			CPUTIMER_STATE_CHECKING bit means that the cputimer has
> + *			expired and a thread in the group is checking the
> + *			callback list.
>   * This structure contains the version of task_cputime, above, that is
>   * used for thread group CPU timer calculations.
>   */
>  struct thread_group_cputimer {
> -	struct task_cputime_atomic cputime_atomic;
> -	bool running;
> -	bool checking_timer;
> +	struct task_cputime_atomic	cputime_atomic;
> +	unsigned int 			state;

Here are we actually increasing the overhead from 2 bytes -> 4 bytes? If
we want to use less space, I was thinking 'unsigned char'.

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