Re: PATCH to make maxfiles, maxfiles per proc boot-time tunable
On 26-Apr-01 Terry Lambert wrote: > ] The problem is that param.c is *not* included in gensetdefs scope. > ] Therefore linker set entries (ie: SYSINIT etc) are not executed. TUNABLE* > ] entries in param.c are simply not called or used. > ] > ] SYSTEM_OBJS= locore.o setdef0.o vnode_if.o ${OBJS} ioconf.o param.o > config.o \ > ] setdef1.o hack.So > ] ... > ] setdef0.c setdef1.c setdefs.h: ${OBJS} > ] @gensetdefs ${OBJS} > ] > ] param.o is not included in ${OBJS}. I dont see how this patch can work > ] as-is. > > Has this changed in -current? > > I'm running on 4.3-RELEASE (now), and it has no problems. > > Are you sure you aren't linking param.o into your kernel anywhere? > > It's the linker that does the magic, not anything else (as you know); > I can assure you it's running fine on my 4.3 box. Umm, tunable_int sets up a sysinit to do the setting of the variable. This depends on there being a linker set for the sysinit code to see. Hmmm *checks link order*. Ok, param.o is in between setdef0.o and setdef1.o so it does get included in the linker set ok, but gensetdefs doesn't see it, so the count of the number of items in a linker set is bogus and invalid. If the code depends on that count then the sysinit's in param.o will be ignored. If the code just keeps walking down the set until it hits the NULL entry at the end, it will work. Hmm, looking at the bubble sort in mi_startup(), it runs until it hits NULL and doesn't check the linker set count, which is why tunable's in param.c works, even though it is incorrect. -- John Baldwin <[EMAIL PROTECTED]> -- http://www.FreeBSD.org/~jhb/ PGP Key: http://www.baldwin.cx/~john/pgpkey.asc "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: PATCH to make maxfiles, maxfiles per proc boot-time tunable
] The problem is that param.c is *not* included in gensetdefs scope. ] Therefore linker set entries (ie: SYSINIT etc) are not executed. TUNABLE* ] entries in param.c are simply not called or used. ] ] SYSTEM_OBJS= locore.o setdef0.o vnode_if.o ${OBJS} ioconf.o param.o config.o \ ] setdef1.o hack.So ] ... ] setdef0.c setdef1.c setdefs.h: ${OBJS} ] @gensetdefs ${OBJS} ] ] param.o is not included in ${OBJS}. I dont see how this patch can work ] as-is. Has this changed in -current? I'm running on 4.3-RELEASE (now), and it has no problems. Are you sure you aren't linking param.o into your kernel anywhere? It's the linker that does the magic, not anything else (as you know); I can assure you it's running fine on my 4.3 box. Terry Lambert [EMAIL PROTECTED] --- Any opinions in this posting are my own and not those of my present or previous employers. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: PATCH to make maxfiles, maxfiles per proc boot-time tunable
Alfred Perlstein wrote: > * Terry Lambert <[EMAIL PROTECTED]> [010424 11:59] wrote: > > It seems to me that these things are not boot-time tunable, and > > should be (really, they should be runtime tunable, but there > > are some nasty pageable region allocations for networking that > > appear to require contiguous regions for no good reason which I > > can discern). That means that the best we can do right now is > > boot-time, so here it is: > > This looks good except that ncallout is still based on MAXFILES, > without this being fixed I think people might get bitten by > raising the tuneable too high then being unable to allocate > enough callouts. Can you take a look at this and make sure there's > nothing else using MAXFILES like that? The problem is that param.c is *not* included in gensetdefs scope. Therefore linker set entries (ie: SYSINIT etc) are not executed. TUNABLE* entries in param.c are simply not called or used. SYSTEM_OBJS= locore.o setdef0.o vnode_if.o ${OBJS} ioconf.o param.o config.o \ setdef1.o hack.So ... setdef0.c setdef1.c setdefs.h: ${OBJS} @gensetdefs ${OBJS} param.o is not included in ${OBJS}. I dont see how this patch can work as-is. Cheers, -Peter -- Peter Wemm - [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED] "All of this is for nothing if we don't go to the stars" - JMS/B5 To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: PATCH to make maxfiles, maxfiles per proc boot-time tunable
> > It seems to me that these things are not boot-time tunable, and > > should be (really, they should be runtime tunable, but there > > $ sysctl -a | grep maxf > kern.maxfiles: 360 > kern.maxfilesperproc: 360 > > `maxfiles' and `maxfilesperproc' have been runtime tunable for more > than 5 years (but there are still bugs in the implementation of this). > > > are some nasty pageable region allocations for networking that > > appear to require contiguous regions for no good reason which I > > can discern). That means that the best we can do right now is > > boot-time, so here it is: > > True, things based on stale values of the variables don't work right. IMO, the only useful thing to do with that many file handles is networking. The fact that the networking doesn't work right when these values are turned, because the initial allocations are required to be contiguous for no good reason to make them pageable, is really, really stupid. > > -- > > Index: conf/param.c > > === > > Don't put anything more in param.c. Almost everything in it can be > done better using tunables or sysctls. The only thing that it is now > useful for is centralizing the #defines for bogus defaults based on > MAXUSERS. This is unnecessary for tunables, since they don't need > static initializers. E.g., the tunable for kern.maxfiles can be > > TUNABLE_INT_DECL("kern.maxfiles", 2 * maxproc, maxfiles); > > instead of > > TUNABLE_INT_DECL("kern.maxfiles", MAXFILES, maxfiles); > > Then maxfiles can be declared in the right place (not here). There > would be a problem getting tunables set in the right order if maxproc > were tunable. We also have a sysctl for maxproc, but it was made > read-only due to allocation problems for exec_map which went away long > ago. Apparently the allocation problems for maxfiles and maxfilesperproc > aren't so serious, since the sysctls for these have always been > read-write. The problems with these sysctls are more with their > interactions with setrlimit(). Actually, there is a serious problem with this approach when applied to maxfiles, and I'm not just talking about param.c. The value of maxfiles is used in: Tune at FileFunction B(1)kern/init_main.cproc0_init() R kern/kern_descrip.c falloc() B(2)kern/uipc_socket2.c init_maxsockets() R svr4/svr4_misc.csvr4_sys_sysconfig() R(3)svr4/svr4_resource.csvr4_sys_getrlimit() svr4_sys_getrlimit64() (1) Before SI_SUB_INTRINSIC, SI_ORDER_FIRST (2) Before SI_SUB_TUNABLES, SI_ORDER_ANY; setting kern.ipc.maxsockets in the bootloader does not override this, since the calculation is: maxsockets = imax(maxsockets, imax(maxfiles, nmbclusters)); (3) These should really use the rlimit values from the kernel, instead of accessing maxfiles directly. The major problem is that the value of maxsockets, derived from the value of maxfiles, is used in: kern/uipc_domain.c domaininit() netinet/tcp_subr.c tcp_init() netinet/udp_usrreq.cudp_init() The domaininit() occurs at SI_SUB_PROTO_DOMAIN, SI_ORDER_FIRST; the other two are from dereferences out of inetsw[] in netinet/in_proto.c. ... all during boot, all to get contiguous swappable regions via calls to zinit(). So it's not just rlimit()... Terry Lambert [EMAIL PROTECTED] --- Any opinions in this posting are my own and not those of my present or previous employers. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: PATCH to make maxfiles, maxfiles per proc boot-time tunable
On Tue, 24 Apr 2001, Terry Lambert wrote: > It seems to me that these things are not boot-time tunable, and > should be (really, they should be runtime tunable, but there $ sysctl -a | grep maxf kern.maxfiles: 360 kern.maxfilesperproc: 360 `maxfiles' and `maxfilesperproc' have been runtime tunable for more than 5 years (but there are still bugs in the implementation of this). > are some nasty pageable region allocations for networking that > appear to require contiguous regions for no good reason which I > can discern). That means that the best we can do right now is > boot-time, so here it is: True, things based on stale values of the variables don't work right. > -- > Index: conf/param.c > === Don't put anything more in param.c. Almost everything in it can be done better using tunables or sysctls. The only thing that it is now useful for is centralizing the #defines for bogus defaults based on MAXUSERS. This is unnecessary for tunables, since they don't need static initializers. E.g., the tunable for kern.maxfiles can be TUNABLE_INT_DECL("kern.maxfiles", 2 * maxproc, maxfiles); instead of TUNABLE_INT_DECL("kern.maxfiles", MAXFILES, maxfiles); Then maxfiles can be declared in the right place (not here). There would be a problem getting tunables set in the right order if maxproc were tunable. We also have a sysctl for maxproc, but it was made read-only due to allocation problems for exec_map which went away long ago. Apparently the allocation problems for maxfiles and maxfilesperproc aren't so serious, since the sysctls for these have always been read-write. The problems with these sysctls are more with their interactions with setrlimit(). Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
RE: PATCH to make maxfiles, maxfiles per proc boot-time tunable
] Why assign them the value of 0? Why not just stick them in the BSS? ] The SI_SUB_TUNABLE checks will initialize them to a value anyways.. Mostly, to leave them where I found them, for paranoia reasons. Terry Lambert [EMAIL PROTECTED] --- Any opinions in this posting are my own and not those of my present or previous employers. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: PATCH to make maxfiles, maxfiles per proc boot-time tunable
] This looks good except that ncallout is still based on MAXFILES, ] without this being fixed I think people might get bitten by ] raising the tuneable too high then being unable to allocate ] enough callouts. Can you take a look at this and make sure there's ] nothing else using MAXFILES like that? Everywhere else uses the value of the variable, rather than the value of the MAXFILES manifest constant; this is true for 4.3-release, if not for -current, so -current should be checked too, I suppose, but I can't see someone intentionally adding another dependency. I actually also have a question for you: what bad things really happen if ncallout is (relatively) much smaller than maxfiles? As far as I can tell, it doesn't cause real problems... The "ncallout" value should technically be a power of 2; I think the code in the various machdep.c is probably broken, and that the valloc() ought to use "callwheelsize" instead of "ncallout", so that "callwheelbits" is not inaccurate, nor is "callwheelmask". In any case, I really can't see how to easily do this at runtime, short of stuffing a SYSINIT(SI_SUB_TUNABLES, SI_ORDER_ANY) into param.c; that really won't work, since the machdep.c code is executed very early on in the boot cycle. It seems that it needs to have a more direct reference to a: TUNABLE_INT_FETCH("kern.ncallout", 0, ncallout); Early on in cpu_startup(). I guess if you want to get technical, the fact that the sockets and so on are allocated based on the value of maxfiles, and set themselves based on a maximum value of both means that the sockets stuff should be ripped out as a tunable entirely, and left to rely only on maxfiles (not MAXFILES). I guess that should you also want to get technical, the sysctl for kern.maxfiles should really be read-only after boot, and not read-write, since the socket structures have already been (incorrectly) sized by the time you have a chance to adjust the number in user space. FWIW: ncallout is actually a larger can of worms than I wanted to open, which is why I didn't just make it its own tunable... would that be an acceptable compromise? Terry Lambert [EMAIL PROTECTED] --- Any opinions in this posting are my own and not those of my present or previous employers. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
RE: PATCH to make maxfiles, maxfiles per proc boot-time tunable
On 24-Apr-01 Terry Lambert wrote: > It seems to me that these things are not boot-time tunable, and > should be (really, they should be runtime tunable, but there > are some nasty pageable region allocations for networking that > appear to require contiguous regions for no good reason which I > can discern). That means that the best we can do right now is > boot-time, so here it is: > > > -- > Index: conf/param.c > === > RCS file: /home/cvs/local_repo/FreeBSD/sys.releng4/conf/param.c,v > retrieving revision 1.1.1.1 > diff -c -r1.1.1.1 param.c > *** conf/param.c 2001/03/21 00:50:42 1.1.1.1 > --- conf/param.c 2001/04/19 20:57:59 > *** > *** 44,49 > --- 44,51 > #include "opt_param.h" > > #include > + #include /* getenv_int */ > + #include /* TUNABLE_INT_DECL */ > > /* >* System parameter formulae. > *** > *** 67,74 > #endif > int maxproc = NPROC;/* maximum # of processes */ > int maxprocperuid = NPROC-1;/* maximum # of processes per user */ > ! int maxfiles = MAXFILES;/* system wide open files limit */ > ! int maxfilesperproc = MAXFILES; /* per-process open files limit */ > int ncallout = 16 + NPROC + MAXFILES; /* maximum # of timer events */ > int mbuf_wait = 32; /* mbuf sleep time in ticks */ > > --- 69,78 > #endif > int maxproc = NPROC;/* maximum # of processes */ > int maxprocperuid = NPROC-1;/* maximum # of processes per user */ > ! int maxfiles = 0; /* system wide open files limit */ > ! TUNABLE_INT_DECL("kern.maxfiles", MAXFILES, maxfiles); > ! int maxfilesperproc = 0;/* per-process open files limit */ > ! TUNABLE_INT_DECL("kern.maxfilesperproc", MAXFILES, maxfilesperproc); > int ncallout = 16 + NPROC + MAXFILES; /* maximum # of timer events */ > int mbuf_wait = 32; /* mbuf sleep time in ticks */ Why assign them the value of 0? Why not just stick them in the BSS? The SI_SUB_TUNABLE checks will initialize them to a value anyways.. -- John Baldwin <[EMAIL PROTECTED]> -- http://www.FreeBSD.org/~jhb/ PGP Key: http://www.baldwin.cx/~john/pgpkey.asc "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message
Re: PATCH to make maxfiles, maxfiles per proc boot-time tunable
* Terry Lambert <[EMAIL PROTECTED]> [010424 11:59] wrote: > It seems to me that these things are not boot-time tunable, and > should be (really, they should be runtime tunable, but there > are some nasty pageable region allocations for networking that > appear to require contiguous regions for no good reason which I > can discern). That means that the best we can do right now is > boot-time, so here it is: This looks good except that ncallout is still based on MAXFILES, without this being fixed I think people might get bitten by raising the tuneable too high then being unable to allocate enough callouts. Can you take a look at this and make sure there's nothing else using MAXFILES like that? > > > -- > Index: conf/param.c > === > RCS file: /home/cvs/local_repo/FreeBSD/sys.releng4/conf/param.c,v > retrieving revision 1.1.1.1 > diff -c -r1.1.1.1 param.c > *** conf/param.c 2001/03/21 00:50:42 1.1.1.1 > --- conf/param.c 2001/04/19 20:57:59 > *** > *** 44,49 > --- 44,51 > #include "opt_param.h" > > #include > + #include /* getenv_int */ > + #include /* TUNABLE_INT_DECL */ > > /* >* System parameter formulae. > *** > *** 67,74 > #endif > int maxproc = NPROC;/* maximum # of processes */ > int maxprocperuid = NPROC-1;/* maximum # of processes per user */ > ! int maxfiles = MAXFILES;/* system wide open files limit */ > ! int maxfilesperproc = MAXFILES; /* per-process open files limit */ > int ncallout = 16 + NPROC + MAXFILES; /* maximum # of timer events */ > int mbuf_wait = 32; /* mbuf sleep time in ticks */ > > --- 69,78 > #endif > int maxproc = NPROC;/* maximum # of processes */ > int maxprocperuid = NPROC-1;/* maximum # of processes per user */ > ! int maxfiles = 0; /* system wide open files limit */ > ! TUNABLE_INT_DECL("kern.maxfiles", MAXFILES, maxfiles); > ! int maxfilesperproc = 0;/* per-process open files limit */ > ! TUNABLE_INT_DECL("kern.maxfilesperproc", MAXFILES, maxfilesperproc); > int ncallout = 16 + NPROC + MAXFILES; /* maximum # of timer events */ > int mbuf_wait = 32; /* mbuf sleep time in ticks */ > > -- > > > Terry Lambert > [EMAIL PROTECTED] > --- > Any opinions in this posting are my own and not those of my present > or previous employers. > > To Unsubscribe: send mail to [EMAIL PROTECTED] > with "unsubscribe freebsd-current" in the body of the message -- -Alfred Perlstein - [[EMAIL PROTECTED]] Instead of asking why a piece of software is using "1970s technology," start asking why software is ignoring 30 years of accumulated wisdom. To Unsubscribe: send mail to [EMAIL PROTECTED] with "unsubscribe freebsd-current" in the body of the message