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