Re: [RFC] Huge sysctl patch for the kernel coming - work in progress
Hi, I've updated my patch according to all comments received: Please review and test before it hits the tree. http://home.selasky.org:8192/sysctl_tunable.diff --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [RFC] Huge sysctl patch for the kernel coming - work in progress
On 06/18/14 23:13, John Baldwin wrote: On Wednesday, June 18, 2014 4:36:24 pm Hans Petter Selasky wrote: On 06/18/14 15:44, John Baldwin wrote: On Wednesday, June 18, 2014 7:36:53 am Hans Petter Selasky wrote: Hi, I'll probably put it into the tree next week. I think having CTLFLAG_TUN do this by default is probably correct in the long term. The vast majority of places that use a tunable to prime a sysctl are safe. Why not do this for the initial patch: - Add your change to auto-fetch values when CTLFLAG_TUN is set. - Instead of adding a CTLFLAG_FETCH, add a CTLFLAG_NOFETCH to disable getenv(). - Make a pass over the existing places that use CTLFLAG_TUN seeing which ones are safe (so TUNABLE_* can just be removed), and which ones aren't (in which case add CTLFLAG_NOFETCH). Followup changes can work on converting other places that don't currently use CTLFLAG_TUN but have a SYSCTL + TUNABLE to use CTLFLAG_TUN instead as well as fixing places that use CTLFLAG_NOFETCH to not need them. I would suggest you commit some of the style changes (like using explicit initializers in SYSCTL_OID()) as a separate change beforehand. Hi, See: http://svnweb.freebsd.org/changeset/base/267633 And updated patch: http://home.selasky.org:8192/sysctl_tunable.diff --HPS ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [RFC] Huge sysctl patch for the kernel coming - work in progress
On Wednesday, June 18, 2014 4:36:24 pm Hans Petter Selasky wrote:
> On 06/18/14 15:44, John Baldwin wrote:
> > On Wednesday, June 18, 2014 7:36:53 am Hans Petter Selasky wrote:
> >> Hi,
> >>
> >> Sometimes sysctl's default value needs to be setup at boot time and not
> >> when the rc.d/sysctl is running. Currently this is done by having two
> >> statements in the kernel:
> >>
> >> TUNABLE_INT("net.graph.mppe.log_max_rekey", &mppe_log_max_rekey);
> >> SYSCTL_INT(_net_graph_mppe, OID_AUTO, log_max_rekey, CTLFLAG_RW,
> >>
> >> I want to simplify this to:
> >>
> >> SYSCTL_INT(_net_graph_mppe, OID_AUTO, log_max_rekey, CTLFLAG_RWTUN,
> >>
> >> In other words if the existing CTLFLAG_TUN is set, the sysctl will
> >> automatically be pre-loaded with values from /boot/loader.conf.
> >>
> >> The reason we don't want the current approach is:
> >>
> >> 1) It duplicates the sysctl path in the TUNABLE statement.
> >> 2) It does not work very well for dynamically attached sysctls. There is
> >> a lot of code overhead computing the TUNABLE() path before the TUNABLE()
> >> can be fetched.
> >>
> >> Here is a work in progress:
> >>
> >> http://home.selasky.org:8192/sysctl_tunable.diff
> >>
> >> In most cases my patch is fine, but in some other cases I need some
> >> input, like in the VM subsystem when doing init, I'm not sure if the
> >> SYSINIT() for subsystem SI_SUB_KMEM, which sysctl's are using, has
> >> already been executed.
> >
> > I think this is a good idea, but it's also true you can just leave separate
> > TUNABLE_ statements without setting the CTLFLAG_TUN flag for cases you
> > aren't
> > sure about for now. It probably makes sense to do these changes in stages.
> >
> > I was going to suggest using sbuf() for building the tunable name, but that
> > doesn't work since you have to build it in reverse.
> >
>
> Hi,
>
> After going through a lot of existing code, I've decided to make a new
> flag, CTLFLAG_FETCH rather than overload CTLFLAG_TUN, so that the new
> functionality can be added to drivers and tested. For example sysctls
> which implement function callbacks and are not trivial, this might cause
> locking of non-initialized mutexes and so on. And also I see some
> dependencies, that values are fetched at a certain point in the boot
> process and that existing CTLFLAG_TUN might confuse existing logic.
>
> I've updated my patch (same link):
>
> http://home.selasky.org:8192/sysctl_tunable.diff
>
> BTW: Can someone which have a beefy machine run a universe with this
> patch applied?
>
> I'll probably put it into the tree next week.
I think having CTLFLAG_TUN do this by default is probably correct in the
long term. The vast majority of places that use a tunable to prime a sysctl
are safe. Why not do this for the initial patch:
- Add your change to auto-fetch values when CTLFLAG_TUN is set.
- Instead of adding a CTLFLAG_FETCH, add a CTLFLAG_NOFETCH to disable
getenv().
- Make a pass over the existing places that use CTLFLAG_TUN seeing which
ones are safe (so TUNABLE_* can just be removed), and which ones aren't
(in which case add CTLFLAG_NOFETCH).
Followup changes can work on converting other places that don't currently
use CTLFLAG_TUN but have a SYSCTL + TUNABLE to use CTLFLAG_TUN instead as
well as fixing places that use CTLFLAG_NOFETCH to not need them.
I would suggest you commit some of the style changes (like using explicit
initializers in SYSCTL_OID()) as a separate change beforehand.
--
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [RFC] Huge sysctl patch for the kernel coming - work in progress
On 06/18/14 15:44, John Baldwin wrote:
On Wednesday, June 18, 2014 7:36:53 am Hans Petter Selasky wrote:
Hi,
Sometimes sysctl's default value needs to be setup at boot time and not
when the rc.d/sysctl is running. Currently this is done by having two
statements in the kernel:
TUNABLE_INT("net.graph.mppe.log_max_rekey", &mppe_log_max_rekey);
SYSCTL_INT(_net_graph_mppe, OID_AUTO, log_max_rekey, CTLFLAG_RW,
I want to simplify this to:
SYSCTL_INT(_net_graph_mppe, OID_AUTO, log_max_rekey, CTLFLAG_RWTUN,
In other words if the existing CTLFLAG_TUN is set, the sysctl will
automatically be pre-loaded with values from /boot/loader.conf.
The reason we don't want the current approach is:
1) It duplicates the sysctl path in the TUNABLE statement.
2) It does not work very well for dynamically attached sysctls. There is
a lot of code overhead computing the TUNABLE() path before the TUNABLE()
can be fetched.
Here is a work in progress:
http://home.selasky.org:8192/sysctl_tunable.diff
In most cases my patch is fine, but in some other cases I need some
input, like in the VM subsystem when doing init, I'm not sure if the
SYSINIT() for subsystem SI_SUB_KMEM, which sysctl's are using, has
already been executed.
I think this is a good idea, but it's also true you can just leave separate
TUNABLE_ statements without setting the CTLFLAG_TUN flag for cases you aren't
sure about for now. It probably makes sense to do these changes in stages.
I was going to suggest using sbuf() for building the tunable name, but that
doesn't work since you have to build it in reverse.
Hi,
After going through a lot of existing code, I've decided to make a new
flag, CTLFLAG_FETCH rather than overload CTLFLAG_TUN, so that the new
functionality can be added to drivers and tested. For example sysctls
which implement function callbacks and are not trivial, this might cause
locking of non-initialized mutexes and so on. And also I see some
dependencies, that values are fetched at a certain point in the boot
process and that existing CTLFLAG_TUN might confuse existing logic.
I've updated my patch (same link):
http://home.selasky.org:8192/sysctl_tunable.diff
BTW: Can someone which have a beefy machine run a universe with this
patch applied?
I'll probably put it into the tree next week.
--HPS
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: [RFC] Huge sysctl patch for the kernel coming - work in progress
On Wednesday, June 18, 2014 7:36:53 am Hans Petter Selasky wrote:
> Hi,
>
> Sometimes sysctl's default value needs to be setup at boot time and not
> when the rc.d/sysctl is running. Currently this is done by having two
> statements in the kernel:
>
> TUNABLE_INT("net.graph.mppe.log_max_rekey", &mppe_log_max_rekey);
> SYSCTL_INT(_net_graph_mppe, OID_AUTO, log_max_rekey, CTLFLAG_RW,
>
> I want to simplify this to:
>
> SYSCTL_INT(_net_graph_mppe, OID_AUTO, log_max_rekey, CTLFLAG_RWTUN,
>
> In other words if the existing CTLFLAG_TUN is set, the sysctl will
> automatically be pre-loaded with values from /boot/loader.conf.
>
> The reason we don't want the current approach is:
>
> 1) It duplicates the sysctl path in the TUNABLE statement.
> 2) It does not work very well for dynamically attached sysctls. There is
> a lot of code overhead computing the TUNABLE() path before the TUNABLE()
> can be fetched.
>
> Here is a work in progress:
>
> http://home.selasky.org:8192/sysctl_tunable.diff
>
> In most cases my patch is fine, but in some other cases I need some
> input, like in the VM subsystem when doing init, I'm not sure if the
> SYSINIT() for subsystem SI_SUB_KMEM, which sysctl's are using, has
> already been executed.
I think this is a good idea, but it's also true you can just leave separate
TUNABLE_ statements without setting the CTLFLAG_TUN flag for cases you aren't
sure about for now. It probably makes sense to do these changes in stages.
I was going to suggest using sbuf() for building the tunable name, but that
doesn't work since you have to build it in reverse.
--
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
