linux-kernel - Re: [PATCH, RFC, tip/core/rcu] scalable classic RCU implementation

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]

Date:	Sat, 23 Aug 2008 18:07:35 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org, cl@...ux-foundation.org,
	akpm@...ux-foundation.org, manfred@...orfullife.com,
	dipankar@...ibm.com, josht@...ux.vnet.ibm.com, schamp@....com,
	niv@...ibm.com, dvhltc@...ibm.com, ego@...ibm.com,
	laijs@...fujitsu.com, rostedt@...dmis.org
Subject: Re: [PATCH, RFC, tip/core/rcu] scalable classic RCU implementation


* Paul E. McKenney <paulmck@...ux.vnet.ibm.com> wrote:

> Is this a sufficient improvement?

yeah - looks much better. This was the block that meets the eye for the 
first time in the patch so it stuck out.

just one more small pet peeve of mine: please use vertical alignment too 
to improve readability. Instead of:

> #define MAX_RCU_LEVELS 3
> #define RCU_FANOUT (CONFIG_RCU_FANOUT)
> #define RCU_FANOUT_SQ (RCU_FANOUT * RCU_FANOUT)
> #define RCU_FANOUT_CUBE (RCU_FANOUT_SQ * RCU_FANOUT)

this looks a bit more structured IMO:

> #define MAX_RCU_LEVELS	3
> #define RCU_FANOUT		(CONFIG_RCU_FANOUT)
> #define RCU_FANOUT_SQ		(RCU_FANOUT * RCU_FANOUT)
> #define RCU_FANOUT_CUBE	(RCU_FANOUT_SQ * RCU_FANOUT)

maybe even this:

> #if (NR_CPUS) <= RCU_FANOUT
> #  define NUM_RCU_LVLS	1
> #  define NUM_RCU_LVL_0	1
> #  define NUM_RCU_LVL_1	(NR_CPUS)
> #  define NUM_RCU_LVL_2	0
> #  define NUM_RCU_LVL_3	0
> #elif (NR_CPUS) <= RCU_FANOUT_SQ
> #  define NUM_RCU_LVLS	2
> #  define NUM_RCU_LVL_0	1
> #  define NUM_RCU_LVL_1	(((NR_CPUS) + RCU_FANOUT - 1) / RCU_FANOUT)
> #  define NUM_RCU_LVL_2	(NR_CPUS)
> #  define NUM_RCU_LVL_3	0
> #elif (NR_CPUS) <= RCU_FANOUT_CUBE
> #  define NUM_RCU_LVLS	3
> #  define NUM_RCU_LVL_0	1
> #  define NUM_RCU_LVL_1	(((NR_CPUS) + RCU_FANOUT_SQ - 1) / RCU_FANOUT_SQ)
> #  define NUM_RCU_LVL_2	(((NR_CPUS) + (RCU_FANOUT) - 1) / (RCU_FANOUT))
> #  define NUM_RCU_LVL_3	NR_CPUS
> #else
> # error "CONFIG_RCU_FANOUT insufficient for NR_CPUS"
> #endif /* #if (NR_CPUS) <= RCU_FANOUT */

but no strong feelings on that one. (maybe inserting a space at the 
right places helps too, no need for a full tab)

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