Date: Fri, 8 Jul 2011 10:43:52 -0700 From: Peter Wemm <peter@wemm.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: Attilio Rao <attilio@freebsd.org>, Sergey Kandaurov <pluknet@freebsd.org>, freebsd-arch@freebsd.org Subject: Re: [PATCH] Add MAXCPU as a kernel config option and quality discussion on this Message-ID: <CAGE5yCpiXSXEWSjqizcNMn9mWSxYTTGdx7c0cAYbG79MD_LGuw@mail.gmail.com> In-Reply-To: <20110708164844.GZ48734@deviant.kiev.zoral.com.ua> References: <CAJ-FndDZu0cBrVbH3W%2B8Tj86T5h%2BwwWqUVnjJO1rtXopKodNOA@mail.gmail.com> <20110708164844.GZ48734@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Jul 8, 2011 at 9:48 AM, Kostik Belousov <kostikbel@gmail.com> wrote: > On Fri, Jul 08, 2011 at 05:37:17PM +0200, Attilio Rao wrote: >> I've made this patch for making MAXCPU a kernel config option: >> http://www.freebsd.org/~attilio/maxcpu_kernel_opt.diff >> >> Besides if this is a good idea or not (which I think it is) I want to >> discuss this implementation and similar related problems. >> In this case I've been forced to include opt_maxcpu.h in all the MD >> param.h implementations. A similar case, KSTACK_PAGES, includes the >> opt_kstack_pages.h only in the consumers. While this is possible for >> KSTACK_PAGES, because there are very little consumers, it would be >> impratical for MAXCPU. Besides, this is a very dangerous practice >> IMHO: if a consumer fails to add opt_kstack_pages it may end up with a >> faulty value, introducing a breakage that would go unnoticed. >> >> In my case, I think that including opt_maxcpu is a viable panacea, but >> in general, after discussing with peter@, probabilly the better idea >> would be having a centralized script that does pre-processing before >> to start compiling and set with the right values all those constants >> (something like genassym.c, but of course with a different purpose). >> >> What are your ideas on that? Do you think that including opt_maxcpu.h >> would be acceptable for the time being? > > I vote for putting MAXCPU in opt_global.h. That isn't right either. Among other things, it makes the problem for kernel modules even worse because it makes non-updated 3rd party modules that happen to be impacted by a MAXCPU change to silently fail by successfully compiling. Part of the problem is that we put default values in include files, and that those defaults can cause side effects if the correct override isn't brought in. Sometimes the defaults are fail-safe, and other times they're not "safe" at all. A more correct solution would be something vaguely along these lines: - remove the #ifndef MAXCPU / #define MAXCPU 32 / #endif from the include files - put "options MAXCPU=32" into the machine/conf/DEFAULTS files - put MAXCPU into opt_maxcpu.h - replace code constructs in include files that will fail with things that will safely fail: eg: sys/pcpu.h: #define PCPU_NAME_LEN (sizeof("CPU ") + sizeof(__XSTRING(MAXCPU) + 1)) becomes.. #ifdef MAXCPU #define PCPU_NAME_LEN (sizeof("CPU ") + sizeof(__XSTRING(MAXCPU) + 1)) #endif and extern struct pcpu *cpuid_to_pcpu[MAXCPU]; should have been: extern struct pcpu *cpuid_to_pcpu[]; or amd64/include/intr_machdep.h: #define INTRCNT_COUNT (1 + NUM_IO_INTS * 2 + (1 + 7) * MAXCPU) becomes #ifdef MAXCPU #define INTRCNT_COUNT (1 + NUM_IO_INTS * 2 + (1 + 7) * MAXCPU) #endif and so on. The idea being that if the requisite opt_maxcpu.h file (or whatever it ends up in) isn't correctly pulled into scope, then you correctly get a compile failure. And if a kernel module somehow depends on the value of INTRCNT_COUNT, then it will correctly fail to compile unless specific kld-safety measures are taken care of. The second point of doing it that way is that you get to have a single source of truth for values. If a value can be set in config(8) then the default value should come from config(8). The other issue is opt_global.h causes massive multiplication of compile dependencies. There really is no need to force a recompile of if_bge.o or ffs_softdep.o just because somebody changes MAXCPU. Dependencies are still supported and work - eg, My home machine: FreeBSD 9.0-CURRENT #1143: Fri Jun 12 20:34:47 PDT 2011 However, detecting that if_bge.o needed to be recompiled is crucial. The alternative I suggested above would solve that. -- Peter Wemm - peter@wemm.org; peter@FreeBSD.org; peter@yahoo-inc.com; KI6FJV "All of this is for nothing if we don't go to the stars" - JMS/B5 "If Java had true garbage collection, most programs would delete themselves upon execution." -- Robert Sewell
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGE5yCpiXSXEWSjqizcNMn9mWSxYTTGdx7c0cAYbG79MD_LGuw>