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
