Re: CVS commit: src/sys/kern
On Mon, Sep 16, 2019 at 03:39:35AM +0200, Emmanuel Dreyfus wrote: > Thor Lancelot Simon wrote: > > > I've never been a fan of the wedge: syntax (as was noted at the time, it > > conflicts with the syntax for specifying network filesystem servers, > > which actually broke automation I had in place); I would not mind seeing > > it go. > > IMO, both wedge: and NAME= are ugly, but beside your point I would be in > favor of NAME= because that is what I used in x86 bootloaders, and > lazyness commands not to redo it. :-) A naming scheme like PROTOCOL ":" SPECIFIER isn't that bad. I chose the NAME= prefix instead for several reasons. Compatibility with NFS mount paths (that we get told are deprecated whenever we use them), lookalike with other OSes, and also to not further reduce path lengths as the string sometimes needs to be stored in small buffers. > When you talk about letting it go, you means removing it, or just > keeping it as is undocumented? Removing could break some setups badly. We need to keep it for compatibility for some time, and that's how it should be documented. > About documentation, wedge: is present in > share/man/man8/man8.x86/boot.8 > share/man/man9/cpu_rootconf.9 > sys/arch/evbarm/conf/IYONIX > sys/arch/evbarm/fdt/fdt_machdep.c > sys/arch/iyonix/conf/GENERIC > usr.bin/config/config.5 sys/arch/evbarm/fdt/fdt_machdep.c is a different beast. The boot device selection in it should go completely as is. There could be a MI version that just allows to feed a bootspec to the kernel for systems that support FDT but cannot specify boot_args. We could also adopt its selection of network interfaces per MAC address for MI code. Just need to come up with a proper encoding as bootspec. Greetings, -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/kern
Thor Lancelot Simon wrote: > I've never been a fan of the wedge: syntax (as was noted at the time, it > conflicts with the syntax for specifying network filesystem servers, > which actually broke automation I had in place); I would not mind seeing > it go. IMO, both wedge: and NAME= are ugly, but beside your point I would be in favor of NAME= because that is what I used in x86 bootloaders, and lazyness commands not to redo it. :-) When you talk about letting it go, you means removing it, or just keeping it as is undocumented? Removing could break some setups badly. About documentation, wedge: is present in share/man/man8/man8.x86/boot.8 share/man/man9/cpu_rootconf.9 sys/arch/evbarm/conf/IYONIX sys/arch/evbarm/fdt/fdt_machdep.c sys/arch/iyonix/conf/GENERIC usr.bin/config/config.5 I suspect the wedge: references there could now just be replaced by NAME= after src/sys/kern/kern_subr.c 1.226-1.227 was committed. -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: CVS commit: src/sys/kern
m...@netbsd.org (Emmanuel Dreyfus) writes: >Michael van Elst wrote: >> We have to decide if we really want to switch things to the NAME= >> syntax and allow wedge: only for compatibility. >I went for NAME= here because x86 bootloader now uses that everywhere, >and my concern was multiboot command line root options passed as is from >bootloader to kernel: >menu=netbsd-8.1/amd64 XEN3_DOM0:load NAME=ffs:/netbsd81-XEN3_DOM0.gz >console=com0 root=NAME=ffs;multiboot NAME=ffs:/xen-4.11.1 dom0_mem=256M >console=com1 com1=115200,8n1 That's nice if it works, but device or even partition naming of bootloaders was always different from what the booted system uses. Think about BIOS device numbers (mapped to fdX: hdX: and cdX:). The same syntax for slightly different things is at least confusing. The load and multiboot argument is interpreted by the bootloader as a GPT partition label, but possibly also as a BIOS device name. The root argument is interpreted by the kernel as a wedge name. Only if all the world consists of GPT formatted disks, then both are the same. -- -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/kern
Michael van Elst wrote: > We have to decide if we really want to switch things to the NAME= > syntax and allow wedge: only for compatibility. I went for NAME= here because x86 bootloader now uses that everywhere, and my concern was multiboot command line root options passed as is from bootloader to kernel: menu=netbsd-8.1/amd64 XEN3_DOM0:load NAME=ffs:/netbsd81-XEN3_DOM0.gz console=com0 root=NAME=ffs;multiboot NAME=ffs:/xen-4.11.1 dom0_mem=256M console=com1 com1=115200,8n1 -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: CVS commit: src/sys/kern
On Sun, Sep 15, 2019 at 03:47:32AM +0200, Emmanuel Dreyfus wrote:
> Michael van Elst wrote:
>
> > A real solution would just support the NAME=* syntax in getwedgename().
> > It might also allow for a case-insensitive match like userland. Then
> > it would just work for config, for boot parameters and for interactive
> > entries.
>
> You mean this change?
>
> --- sys/kern/kern_subr.c27 Jan 2019 02:08:43 - 1.223
> +++ sys/kern/kern_subr.c15 Sep 2019 01:46:42 -
> @@ -678,15 +678,20 @@
>
> static const char *
> getwedgename(const char *name, int namelen)
> {
> - const char *wpfx = "wedge:";
> - const int wpfxlen = strlen(wpfx);
> + const char *wpfx1 = "wedge:";
> + const char *wpfx2 = "NAME=";
> + const int wpfx1len = strlen(wpfx1);
> + const int wpfx2len = strlen(wpfx2);
>
> - if (namelen < wpfxlen || strncmp(name, wpfx, wpfxlen) != 0)
> - return NULL;
> + if (namelen > wpfx1len && strncmp(name, wpfx1, wpfx1len) == 0)
> + return name + wpfx1len;
>
> - return name + wpfxlen;
> + if (namelen > wpfx2len && strncmp(name, wpfx2, wpfx2len) == 0)
> + return name + wpfx2len;
> +
> + return NULL;
> }
>
> static device_t
> parsedisk(char *str, int len, int defpart, dev_t *devp)
Yes. Please use strncasecmp for wpfx2, userland ignores case too.
We have to decide if we really want to switch things to the NAME=
syntax and allow wedge: only for compatibility. Then we should also
adjust what dkwedge_print_wnames() prints, currently it uses the wedge:
prefix. It is called in getdisk() when you enter something invalid
for a root device.
The wedge: syntax is documented in config(5), boot(8.x86) and cpu_rootconf(9)
and maybe in other places.
It is also used in iyonix/conf/GENERIC (as example) and evbarm/conf/IYONIX.
There is also MD code for evbarm that generates the wedge: syntax from
the FDT entry /chosen/netbsd,gpt-guid, replacing existing boot arguments.
While /chosen/netbsd,gpt-label is searched for in the wedge list to
pass a device to the MI code. But that's another story.
Greetings,
--
Michael van Elst
Internet: mlel...@serpens.de
"A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/kern
Michael van Elst wrote:
> A real solution would just support the NAME=* syntax in getwedgename().
> It might also allow for a case-insensitive match like userland. Then
> it would just work for config, for boot parameters and for interactive
> entries.
You mean this change?
--- sys/kern/kern_subr.c27 Jan 2019 02:08:43 - 1.223
+++ sys/kern/kern_subr.c15 Sep 2019 01:46:42 -
@@ -678,15 +678,20 @@
static const char *
getwedgename(const char *name, int namelen)
{
- const char *wpfx = "wedge:";
- const int wpfxlen = strlen(wpfx);
+ const char *wpfx1 = "wedge:";
+ const char *wpfx2 = "NAME=";
+ const int wpfx1len = strlen(wpfx1);
+ const int wpfx2len = strlen(wpfx2);
- if (namelen < wpfxlen || strncmp(name, wpfx, wpfxlen) != 0)
- return NULL;
+ if (namelen > wpfx1len && strncmp(name, wpfx1, wpfx1len) == 0)
+ return name + wpfx1len;
- return name + wpfxlen;
+ if (namelen > wpfx2len && strncmp(name, wpfx2, wpfx2len) == 0)
+ return name + wpfx2len;
+
+ return NULL;
}
static device_t
parsedisk(char *str, int len, int defpart, dev_t *devp)
--
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
m...@netbsd.org
Re: CVS commit: src/sys/kern
Martin Husemann wrote: > Oh, *B*ootdev vs. *R*ootdev - I see. > Not sure why bootdev is important, however we should use the same syntax! The change handles the parameter passed from bootloader when booting Xen. In boot.cfg, NAME=label is now supported everywhere, and this was the only missing bit. -- Emmanuel Dreyfus http://hcpnet.free.fr/pubz m...@netbsd.org
Re: CVS commit: src/sys/kern
mar...@duskware.de (Martin Husemann) writes: >> config netbsd root on "wedge:Guru-Root" type ffs >> for ages (in the netbsd-7 branch even). >Oh, *B*ootdev vs. *R*ootdev - I see. >Not sure why bootdev is important, however we should use the same syntax! bootdev already acts as a default for rootdev. There is no difference. There should be no difference. The wedge:* syntax worked for ages. This change either should allow wedge names (ignorant of the wedge:* syntax) or align the naming with the NAME=* syntax. But like many other ad-hoc changes it is done wrong. A real solution would just support the NAME=* syntax in getwedgename(). It might also allow for a case-insensitive match like userland. Then it would just work for config, for boot parameters and for interactive entries. You may also want to think of augmenting dkwedge_print_wnames() to list the wedges in NAME= syntax and the documentation that currently only refers to wedge:*, in particular for config files. Or we may just accept that boot/config syntax is different from userland. Greetings, -- -- Michael van Elst Internet: mlel...@serpens.de "A potential Snark may lurk in every tree."
Re: CVS commit: src/sys/kern
On Sat, Sep 14, 2019 at 06:26:34AM +, Martin Husemann wrote: > > Log Message: > > Accept root device specification as NAME=label [..] > Why is this needed? > > I have been using > > config netbsd root on "wedge:Guru-Root" type ffs > > for ages (in the netbsd-7 branch even). Oh, *B*ootdev vs. *R*ootdev - I see. Not sure why bootdev is important, however we should use the same syntax! Martin
Re: CVS commit: src/sys/kern
On Fri, Sep 13, 2019 at 01:33:20AM +, Emmanuel Dreyfus wrote: > Module Name: src > Committed By: manu > Date: Fri Sep 13 01:33:20 UTC 2019 > > Modified Files: > src/sys/kern: kern_subr.c > > Log Message: > Accept root device specification as NAME=label > > > To generate a diff of this commit: > cvs rdiff -u -r1.224 -r1.225 src/sys/kern/kern_subr.c Why is this needed? I have been using config netbsd root on "wedge:Guru-Root" type ffs for ages (in the netbsd-7 branch even). Martin
Re: CVS commit: src/sys/kern
In article <8998.1530490...@splode.eterna.com.au>, matthew green wrote: >Jason Thorpe writes: >> >> >> > On Jul 1, 2018, at 2:48 AM, Martin Husemann wrote: >> > >> > I'd rather not have this at all - instead just drop the useless timestamps >> > at boot time, they are useless here and make console output unreadable. >> > >> > They are fine after mountroot, but is there really any use for them before? >> >> +1 > >i think there is definitively a use for them as soon as clock are >running -- ie, when config_interrupts() occurs -- and having worked >with a kernel that prints TSC values before the clocks are running, >i also claim that this is useful too. it helps find slowness in >early boot. > >i wasn't against the original changed (now reverted). It will also make dmesg -T look funny if some have timestamps and others do not. It is better to be consistent. christos
re: CVS commit: src/sys/kern
Jason Thorpe writes: > > > > On Jul 1, 2018, at 2:48 AM, Martin Husemann wrote: > > > > I'd rather not have this at all - instead just drop the useless timestamps > > at boot time, they are useless here and make console output unreadable. > > > > They are fine after mountroot, but is there really any use for them before? > > +1 i think there is definitively a use for them as soon as clock are running -- ie, when config_interrupts() occurs -- and having worked with a kernel that prints TSC values before the clocks are running, i also claim that this is useful too. it helps find slowness in early boot. i wasn't against the original changed (now reverted). .mrg.
Re: CVS commit: src/sys/kern
> On Jul 1, 2018, at 2:48 AM, Martin Husemann wrote: > > I'd rather not have this at all - instead just drop the useless timestamps > at boot time, they are useless here and make console output unreadable. > > They are fine after mountroot, but is there really any use for them before? +1 -- thorpej
Re: CVS commit: src/sys/kern
On Sat, Jun 30, 2018 at 10:47:51PM +, Taylor R Campbell wrote: > Module Name: src > Committed By: riastradh > Date: Sat Jun 30 22:47:51 UTC 2018 > > Modified Files: > src/sys/kern: kern_ntptime.c kern_tc.c > > Log Message: > Sprinkle cold conditionals to make tc_ticktock before inittimecounter. > > Enables Xen to boot again. > > XXX Maybe we should have a tc_ticktock_cold instead or something so we > don't have to reach this far into the call graph. I'd rather not have this at all - instead just drop the useless timestamps at boot time, they are useless here and make console output unreadable. They are fine after mountroot, but is there really any use for them before? Martin
Re: CVS commit: src/sys/kern
On Tue, Aug 25, 2015 at 03:59:00PM +0300, Andreas Gustafsson wrote: Martin Husemann wrote: I have another evbarm machine now hanging as well, so it is not just alpha. The sparc test runs on babylon5 are affected, too. I just committed a fix, let's see if it works. Alpha worked, will re-run the tests on other machines today. Martin
Re: CVS commit: src/sys/kern
Martin Husemann wrote: I have another evbarm machine now hanging as well, so it is not just alpha. The sparc test runs on babylon5 are affected, too. I just committed a fix, let's see if it works. -- Andreas Gustafsson, g...@netbsd.org
Re: CVS commit: src/sys/kern
On Wed, Aug 19, 2015 at 12:02:55PM +, Andreas Gustafsson wrote: Module Name: src Committed By: gson Date: Wed Aug 19 12:02:55 UTC 2015 Modified Files: src/sys/kern: tty.c Log Message: When closing a tty, limit the amount of time spent waiting for the output to drain to five seconds so that exiting processes with buffered output for a serial port blocked by flow control or a pty that is not being read do not hang indefinitely. Should fix PRs kern/12534 and kern/17171. This is an updated version of the change of tty.c 1.263. This change makes my alpha reproducably wedge when doing lib/libc/ttyio/t_ttyio (272/621): 1 test cases ioctl: Can't break into ddb, kernel (alpha GENERIC) hard locks up. Reverting to r1.264 makes it work again. I'm not sure what is special in my alpha setup, an evbarm kernel from the same sources passed all tests just fine earlier today. My setup runs atf-run detached, with stdin closed. Any ideas what to look for? Martin
Re: CVS commit: src/sys/kern
In article 20140725195305.55fbb14a...@mail.netbsd.org, Mindaugas Rasiukevicius rm...@netbsd.org wrote: chris...@astron.com (Christos Zoulas) wrote: In article 53d288c7.5000...@m00nbsd.net, Maxime Villard m...@m00nbsd.net wrote: Well, if only these architectures are running with DIAGNOSTIC, that's not a big deal. Anyway, let's put KMEM_DIAGNOSTIC. I like KMEM_DIAGNOSTIC, can you add it, or should I? Yet another option? Why? to wrap all the KMEM_ related diagnostic options in one, can separate them from DIAGNOSTIC, so that low memory machines can choose not to turn them on, but still have DIAGNOSTIC turned on. christos
Re: CVS commit: src/sys/kern
On Sun, Jul 27, 2014 at 07:37:14PM +, Christos Zoulas wrote: to wrap all the KMEM_ related diagnostic options in one, can separate them from DIAGNOSTIC, so that low memory machines can choose not to turn them on, but still have DIAGNOSTIC turned on. I am not convinced this will be used ever. All low memory machines I can think of (at least all the ones I run) do not use DIAGNOSTIC in normal operation, and when debugging it hardly matters. How about enabling the kmem stuff by default with DIAGNOSTIC, but add a negative option to turn it off (KMEM_NO_EXTRA_CHECKS or something)? Martin
Re: CVS commit: src/sys/kern
On Jul 27, 9:46pm, mar...@duskware.de (Martin Husemann) wrote: -- Subject: Re: CVS commit: src/sys/kern | On Sun, Jul 27, 2014 at 07:37:14PM +, Christos Zoulas wrote: | to wrap all the KMEM_ related diagnostic options in one, can separate | them from DIAGNOSTIC, so that low memory machines can choose not to turn | them on, but still have DIAGNOSTIC turned on. | | I am not convinced this will be used ever. All low memory machines I | can think of (at least all the ones I run) do not use DIAGNOSTIC in normal | operation, and when debugging it hardly matters. | | How about enabling the kmem stuff by default with DIAGNOSTIC, but add a | negative option to turn it off (KMEM_NO_EXTRA_CHECKS or something)? That's fine too. The all-or-nothing approach is the issue here. christos
Re: CVS commit: src/sys/kern
In article 53d22353.9090...@m00nbsd.net, Maxime Villard m...@m00nbsd.net wrote: Le 23/07/2014 21:51, Greg Troxel a écrit : I realized that two things can be separated. One is whether DIAGNOSTIC turns on features that increase the size of memory allocations, which is my real concern. The other is whether the KMEM_SIZE and KMEM_REDZONE should be on by default in -current on various architectures, which is I think your primary concern. On i386 and amd64, it may well be that the performance change is not really noticeable. But increasing memory sizes on small-memory architectures is something else. What are these small-memory architectures? vax, shark, m68k and the list goes on. I think that the request is reasonable. It gives more flexibility rather than one size fits all and lets the portmasters choose the appropriate behavior for their ports. Currently the following machines have GENERIC kernels with DIAGNOSTIC on: $ fgrep DIAGNOSTIC */conf/GENERIC | grep -v :# amd64/conf/GENERIC:options DIAGNOSTIC # expensive kernel consistency checks cobalt/conf/GENERIC:options DIAGNOSTIC # extra kernel sanity checking i386/conf/GENERIC:options DIAGNOSTIC # expensive kernel consistency checks ia64/conf/GENERIC:options DIAGNOSTIC # expensive kernel consistency checks mvmeppc/conf/GENERIC:optionsDIAGNOSTIC # cheap kernel consistency checks shark/conf/GENERIC:options DIAGNOSTIC # internal consistency checks sparc64/conf/GENERIC:optionsDIAGNOSTIC # extra kernel sanity checking zaurus/conf/GENERIC:options DIAGNOSTIC # internal consistency checks christos
Re: CVS commit: src/sys/kern
Le 25/07/2014 16:51, Christos Zoulas a écrit : In article 53d22353.9090...@m00nbsd.net, Maxime Villard m...@m00nbsd.net wrote: Le 23/07/2014 21:51, Greg Troxel a écrit : I realized that two things can be separated. One is whether DIAGNOSTIC turns on features that increase the size of memory allocations, which is my real concern. The other is whether the KMEM_SIZE and KMEM_REDZONE should be on by default in -current on various architectures, which is I think your primary concern. On i386 and amd64, it may well be that the performance change is not really noticeable. But increasing memory sizes on small-memory architectures is something else. What are these small-memory architectures? vax, shark, m68k and the list goes on. Or rather, which architecture can support KMEM_REDZONE a feasible way? I think that the request is reasonable. It gives more flexibility rather than one size fits all and lets the portmasters choose the appropriate behavior for their ports. Currently the following machines have GENERIC kernels with DIAGNOSTIC on: $ fgrep DIAGNOSTIC */conf/GENERIC | grep -v :# amd64/conf/GENERIC:options DIAGNOSTIC # expensive kernel consistency checks cobalt/conf/GENERIC:options DIAGNOSTIC # extra kernel sanity checking i386/conf/GENERIC:options DIAGNOSTIC # expensive kernel consistency checks ia64/conf/GENERIC:options DIAGNOSTIC # expensive kernel consistency checks mvmeppc/conf/GENERIC:optionsDIAGNOSTIC # cheap kernel consistency checks shark/conf/GENERIC:options DIAGNOSTIC # internal consistency checks sparc64/conf/GENERIC:optionsDIAGNOSTIC # extra kernel sanity checking zaurus/conf/GENERIC:options DIAGNOSTIC # internal consistency checks Well, if only these architectures are running with DIAGNOSTIC, that's not a big deal. Anyway, let's put KMEM_DIAGNOSTIC.
Re: CVS commit: src/sys/kern
In article 53d288c7.5000...@m00nbsd.net, Maxime Villard m...@m00nbsd.net wrote: Well, if only these architectures are running with DIAGNOSTIC, that's not a big deal. Anyway, let's put KMEM_DIAGNOSTIC. I like KMEM_DIAGNOSTIC, can you add it, or should I? christos
Re: CVS commit: src/sys/kern
chris...@astron.com (Christos Zoulas) wrote: In article 53d288c7.5000...@m00nbsd.net, Maxime Villard m...@m00nbsd.net wrote: Well, if only these architectures are running with DIAGNOSTIC, that's not a big deal. Anyway, let's put KMEM_DIAGNOSTIC. I like KMEM_DIAGNOSTIC, can you add it, or should I? Yet another option? Why? -- Mindaugas
Re: CVS commit: src/sys/kern
I realized that two things can be separated. One is whether DIAGNOSTIC
turns on features that increase the size of memory allocations, which is
my real concern. The other is whether the KMEM_SIZE and KMEM_REDZONE
should be on by default in -current on various architectures, which is I
think your primary concern. On i386 and amd64, it may well be that the
performance change is not really noticeable. But increasing memory
sizes on small-memory architectures is something else. With these
rolled into DIAGNOSTIC, it becomes infeasible to enable the
invariant-checking part of DIAGNOSTIC without these on those
architectures.
So, I'd like you to still remove these from being auto-enabled, and add
them to src/sys/arch/{i386,amd64}/conf/GENERIC, and optionally on
whatever other arches make sense.
greg
pgpFizAUt0VP4.pgp
Description: PGP signature
Re: CVS commit: src/sys/kern
Maxime Villard m...@netbsd.org writes: Module Name: src Committed By: maxv Date: Tue Jul 22 07:38:41 UTC 2014 Modified Files: src/sys/kern: subr_kmem.c Log Message: Enable KMEM_REDZONE on DIAGNOSTIC. It will try to catch overflows. No comment on tech-kern@ I didn't see this on tech-kern (nor did I see anything about defining KMEM_POISON), and now that I'm aware I object. DIAGNOSTIC, by longstanding tradition, is a lightweight and not have a significant performance hit. Basically it's about KASSERT of things that must be true. This is changing the size of memory allocations, which is far more far-reaching. I am not claiming that KMEM_REDZONE is not useful. Arguably it could be enabled under DEBUG, which is documented to be expensive (and unreasonable to use on a normal basis). DIAGNOSTIC, on the other hand, I consider normal for systems that are being used (rather than only debug targets). The same goes for KMEM_POISON; these checks, while useful for debugging (and it would be nice to have regular anita runs with DEBUG), are too expensive for ordinary use. For -current, GENERIC defines DIAGNOSTIC. Please revert the automatic definition of these or change to DEBUG. (I am not objecting to the separating and spiffing up of these features, just that they are enabled when DIAGNOSTIC is defined.) pgppMGwcWEWOG.pgp Description: PGP signature
Re: CVS commit: src/sys/kern
Nick Hudson sk...@netbsd.org writes: On 07/22/14 12:49, Greg Troxel wrote: Maxime Villard m...@netbsd.org writes: Module Name:src Committed By: maxv Date: Tue Jul 22 07:38:41 UTC 2014 Modified Files: src/sys/kern: subr_kmem.c Log Message: Enable KMEM_REDZONE on DIAGNOSTIC. It will try to catch overflows. No comment on tech-kern@ I didn't see this on tech-kern (nor did I see anything about defining KMEM_POISON), and now that I'm aware I object. DIAGNOSTIC, by longstanding tradition, is a lightweight and not have a significant performance hit. Basically it's about KASSERT of things that must be true. This is changing the size of memory allocations, which is far more far-reaching. options(4) says this options DIAGNOSTIC Adds code to the kernel that does internal consistency checks. This code will cause the kernel to panic if corruption of internal data structures is detected. These checks can decrease performance up to 15%. I'd guess that KMEM_REDZONE add much less than 15%. I think that options(4) is buggy here. The historical norms for DIAGNOSTIC are nowhere near 15% being acceptable.There is also no notion of increasing memory usage under DIAGNOSTIC. pgpTjl0oTh4IJ.pgp Description: PGP signature
Re: CVS commit: src/sys/kern
On 07/22/14 12:49, Greg Troxel wrote: Maxime Villard m...@netbsd.org writes: Module Name:src Committed By: maxv Date: Tue Jul 22 07:38:41 UTC 2014 Modified Files: src/sys/kern: subr_kmem.c Log Message: Enable KMEM_REDZONE on DIAGNOSTIC. It will try to catch overflows. No comment on tech-kern@ I didn't see this on tech-kern (nor did I see anything about defining KMEM_POISON), and now that I'm aware I object. DIAGNOSTIC, by longstanding tradition, is a lightweight and not have a significant performance hit. Basically it's about KASSERT of things that must be true. This is changing the size of memory allocations, which is far more far-reaching. options(4) says this options DIAGNOSTIC Adds code to the kernel that does internal consistency checks. This code will cause the kernel to panic if corruption of internal data structures is detected. These checks can decrease performance up to 15%. I'd guess that KMEM_REDZONE add much less than 15%. Nick
Re: CVS commit: src/sys/kern
On Tue, Jul 22, 2014 at 12:54:15PM +0100, Nick Hudson wrote: I'd guess that KMEM_REDZONE add much less than 15%. Even if 15% were a value we'd be willing to tolerate (which I doubt), that's 15% total, not 15% each. Anyway, rather than shouting, can someone check what it really does cost? -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src/sys/kern
On Wed, Mar 05, 2014 at 06:04:02PM +0200, Andreas Gustafsson wrote: 2. I also object to the change of kern_syctl.c 1.247. This change attempts to work around the problems caused by the changes to the variable types by making sysctl() return different types depending on the value of the *oldlenp argument. IMO, this is a bad idea. The *oldlenp argument does *not* specify the size of the data type expected by the caller, but rather the size of a buffer. The sysctl() API allows the caller to pass a buffer larger than the variable being read, and conversely, guarantees that passing a buffer that is too small results in ENOMEM. Both of these aspects of the API are now broken: reading a 4-byte CTLTYPE_INT variable now works for any buffer size = 4 *except* 8, That wasn't the intent of the change. The intent was that if the size was 8 then the code would return a numeric value of size 8, otherwise the size would be chnaged to 4 and/or ENOMEM (stupid errno choice) returned. and attempting to read an 8-byte CTLTYPE_QUAD variable into a buffer of less than 8 bytes is now guaranteed to yield ENOMEM *except* if the buffer size happens to be 4. A request to read a CTLTYPE_QUAD variable into a buffer that is shorter than 8 bytes has always been a programming error. The intent of the change was to relax that is the length happened to be 4. IMO, this behavior violates both the letter of the sysctl() man page and the principle of least astonishment. I'm not sure about the latter. I run 'sysctl -a' and find the name of the sysctl I'm interested in. The result is a small number so I pass the address and size of a integer variable and then print the result. (Or the value is rather large and I think it might exceed 2^31 so I use an int64.) The 'principle of least astonishment' would mean that I get the value that 'sysctl -a' printed. On a BE system I have to be extremely careful with the return values from sysctl() or I see garbage. Note that code calling systctl() has to either know whether the value it is expecting is a string, structure, or number, or use the API calls that expose the kernel internals in order to find out. Also, the work-around is ineffective in the case of a caller that allocates the buffer dynamically using the size given by an initial sysctl() call with oldp = NULL. Code that does that for a numeric value will be quite happy with either a 32bit of 64bit result. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/kern
On Tue, Aug 27, 2013 at 02:01:36PM +, Taylor R Campbell wrote: This is a stop-gap on a stop-gap on a stop-gap; we really ought to back out all of these stop-gaps, make bcm2835_rng call rnd_add_data asynchronously to work around the original symptom, and design a real solution when we have time to sort this mess out properly. Having slept on it -- you're right. Most of this series of changes should be backed out (a few parts are worth keeping, like the tweaks to the softint initialization). In particular, the shortcut calls to rnd_process_events should go back into the softint schedule routines, and be removed from the extract functions. Then, I think, as a better interim solution, we should: 1) Make the hardware RNGs loop, synchronously adding entropy until they've added a pool-full, at startup. 2) Make all the users of the callback interface add entropy asynchronously at startup, so early in kernel startup other RNGs are not initialized from a near-empty pool. Thor
