Review of brl scaling fix patches.
Jeremy Allison
jra at samba.org
Fri Feb 5 21:04:37 MST 2010
Hi tridge,
As requested, I'm reviewing the brl scaling patches at :
http://git.samba.org/?p=tridge/samba.git;a=shortlog;h=refs/heads/brl-scaling-fix
They look great, but the only thing I was wondering about is the
use of "static int" for the timeout values in, recalc_brl_timeout()
specifically the change :
http://git.samba.org/?p=tridge/samba.git;a=commitdiff;h=a126003bd8e7ed14a79d681a266517c9b3163a08
+ static int max_brl_timeout = -1;
+
+ if (max_brl_timeout == -1) {
+ max_brl_timeout = lp_parm_int(-1, "brl", "recalctime", 5);
+ }
and also the static int in this patch:
http://git.samba.org/?p=tridge/samba.git;a=commitdiff;h=6640fc4e6b6666087e42de9fa85dd1fa1be019ec
+ /* call the cleanup timer, but not too often */
+ static int cleanup_time = -1;
+ if (cleanup_time == -1) {
+ cleanup_time = lp_parm_int(-1,
+ "smbd", "cleanuptime", 20);
+ }
Do these really need to be "static int"'s ? I've been
trying to reduce the number of statics we use in S3 smbd
in order to eventually get to the safe use of background
threads. I doubt these are performance critical, as it's
just setting up an event time - and I don't think it matters
if they were changed in smb.conf whilst smbd is running. So
can these be changed to :
int max_brl_timeout = lp_parm_int(-1, "brl", "recalctime", 5);
and :
int cleanup_time = lp_parm_int(-1, "smbd", "cleanuptime", 20);
instead ?
Jeremy.
More information about the samba-technical
mailing list