Re: CVS commit: src/sys/dev/usb
> Even for the ones without the widths specified. E.g. I personally > prefer zero printed as 0x0, not as 0, so I assume that when people > choose either one that reflects their preference. Why mess with it? > It's all so unnecessary. Yes, now we are discussing cosmetics (if 0 should be printed as 0 or 0x0 mostly in debugging messages), since this is the only change remaining. In retrospect, perhaps I should have left it alone, but now aside the cosmetics part, we are strictly better off because all the formats have been fixed (including the 2 ones which we would not have found if I did not make the %# change). christos signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/sys/dev/usb
On Sat, Mar 14, 2020 at 09:57:36 -0400, Christos Zoulas wrote: > > Even for the ones without the widths specified. E.g. I personally > > prefer zero printed as 0x0, not as 0, so I assume that when people > > choose either one that reflects their preference. Why mess with it? > > It's all so unnecessary. > > Yes, now we are discussing cosmetics (if 0 should be printed as 0 or > 0x0 mostly in debugging messages), since this is the only change > remaining. > > In retrospect, perhaps I should have left it alone, It would have been better to just leave them the hell alone as they are to begin with. This is, I think, the third time in recent memory when people try to "fix" 0x%x -> %#x and each time it goes wrong. We should have learned from that. > but now aside the cosmetics part, we are strictly better off because > all the formats have been fixed Which is true but non sequitur. > (including the 2 ones which we would not have found if I did not > make the %# change). I don't belive that "if". It's like claiming you got rid of a stain on a wallpaper after you demolish a wall (not load-bearing, fortunately) and have to put it back and put new wallpaper. :) Get rid of the stain, sure; but may be looking closely with a bit of patience might have been less drastic and as effective. -uwe
Re: CVS commit: src/sys/dev/usb
> I don't belive that "if". It's like claiming you got rid of a stain
> on a wallpaper after you demolish a wall (not load-bearing,
> fortunately) and have to put it back and put new wallpaper. :) Get rid
> of the stain, sure; but may be looking closely with a bit of patience
> might have been less drastic and as effective.
To fix the kernhist ones I looked with a lot of patience and even then,
I missed quite a few ones (the ones in the final commit). It is really
difficult to find them, specially because the DPRINTF macros are
used sometimes for regular debugging and other times for kernhist.
In the end I had to add a fake printf function in kernhist.h like below,
and then filter out the error messages about too many arguments for
format.
christos
--- kernhist.h 2020-03-13 23:03:13.973939910 -0400
+++ kernhist.h.orig 2020-03-13 22:59:37.237495925 -0400
@@ -207,6 +207,11 @@
#define KERNHIST_PRINTNOW(E) /* nothing */
#endif
+// Just for format checking
+static __inline __printflike(1, 2) void
+__kernhist_printf(const char *fmt __unused, ...) {
+}
+
#define KERNHIST_LOG(NAME,FMT,A,B,C,D) \
do { \
unsigned int _i_, _j_; \
@@ -227,6 +232,7 @@
_e_->v[1] = (uintmax_t)(B); \
_e_->v[2] = (uintmax_t)(C); \
_e_->v[3] = (uintmax_t)(D); \
+ __kernhist_printf(FMT, _e_->v[0], _e_->v[1], _e_->v[2], _e_->v[3]); \
KERNHIST_PRINTNOW(_e_); \
} while (/*CONSTCOND*/ 0)
signature.asc
Description: Message signed with OpenPGP
Re: CVS commit: src/sys/dev/usb
On Sat, Mar 14, 2020 at 10:27:27 -0400, Christos Zoulas wrote:
> > I don't belive that "if". It's like claiming you got rid of a stain
> > on a wallpaper after you demolish a wall (not load-bearing,
> > fortunately) and have to put it back and put new wallpaper. :) Get rid
> > of the stain, sure; but may be looking closely with a bit of patience
> > might have been less drastic and as effective.
>
> To fix the kernhist ones I looked with a lot of patience and even then,
> I missed quite a few ones (the ones in the final commit). It is really
> difficult to find them, specially because the DPRINTF macros are
> used sometimes for regular debugging and other times for kernhist.
> In the end I had to add a fake printf function in kernhist.h like below,
> and then filter out the error messages about too many arguments for
> format.
>
> christos
>
> --- kernhist.h 2020-03-13 23:03:13.973939910 -0400
> +++ kernhist.h.orig 2020-03-13 22:59:37.237495925 -0400
> @@ -207,6 +207,11 @@
> #define KERNHIST_PRINTNOW(E) /* nothing */
> #endif
>
> +// Just for format checking
> +static __inline __printflike(1, 2) void
> +__kernhist_printf(const char *fmt __unused, ...) {
> +}
> +
> #define KERNHIST_LOG(NAME,FMT,A,B,C,D) \
> do { \
> unsigned int _i_, _j_; \
> @@ -227,6 +232,7 @@
> _e_->v[1] = (uintmax_t)(B); \
> _e_->v[2] = (uintmax_t)(C); \
> _e_->v[3] = (uintmax_t)(D); \
> + __kernhist_printf(FMT, _e_->v[0], _e_->v[1], _e_->v[2], _e_->v[3]); \
> KERNHIST_PRINTNOW(_e_); \
> } while (/*CONSTCOND*/ 0)
How is is affected by the decision to change (or not) 0x%x to %#x?
-uwe
Re: CVS commit: src/sys/dev/usb
In article <20200314143238.gr5...@pony.stderr.spb.ru>, Valery Ushakov wrote: >How is is affected by the decision to change (or not) 0x%x to %#x? > This was in response to the statement: ... with a bit of patience might have been less drastic and as effective. christos
Re: CVS commit: src/external/gpl3/gdb/dist/bfd
On 26.02.2016 17:28, Christos Zoulas wrote:
> Module Name: src
> Committed By: christos
> Date: Fri Feb 26 16:28:14 UTC 2016
>
> Modified Files:
> src/external/gpl3/gdb/dist/bfd: merge.c
>
> Log Message:
> CID 420802: Avoid NULL deref.
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.1.1.4 -r1.2 src/external/gpl3/gdb/dist/bfd/merge.c
>
> Please note that diffs are not public domain; they are subject to the
> copyright notices on the relevant files.
>
>
> Modified files:
>
> Index: src/external/gpl3/gdb/dist/bfd/merge.c
> diff -u src/external/gpl3/gdb/dist/bfd/merge.c:1.1.1.4
> src/external/gpl3/gdb/dist/bfd/merge.c:1.2
> --- src/external/gpl3/gdb/dist/bfd/merge.c:1.1.1.4Tue Feb 2 22:00:11 2016
> +++ src/external/gpl3/gdb/dist/bfd/merge.cFri Feb 26 11:28:14 2016
> @@ -334,7 +334,7 @@ sec_merge_emit (bfd *abfd, struct sec_me
>
>/* Trailing alignment needed? */
>off = sec->size - off;
> - if (off != 0)
> + if (pad != NULL && off != 0)
> {
>if (contents)
> memcpy (contents + offset, pad, off);
>
It looks to me like a false positive.
pad is checked just after bfd_zmalloc():
pad = (char *) bfd_zmalloc (pad_len);
if (pad == NULL)
return FALSE;
If I am not overlooking something, I will drop this local patch as not
upstreamable.
signature.asc
Description: OpenPGP digital signature
Re: CVS commit: src/external/gpl3/gdb/dist/bfd
Yup, undo it.
christos
> On Mar 14, 2020, at 2:35 PM, Kamil Rytarowski wrote:
>
> Signed PGP part
> On 26.02.2016 17:28, Christos Zoulas wrote:
>> Module Name: src
>> Committed By:christos
>> Date:Fri Feb 26 16:28:14 UTC 2016
>>
>> Modified Files:
>> src/external/gpl3/gdb/dist/bfd: merge.c
>>
>> Log Message:
>> CID 420802: Avoid NULL deref.
>>
>>
>> To generate a diff of this commit:
>> cvs rdiff -u -r1.1.1.4 -r1.2 src/external/gpl3/gdb/dist/bfd/merge.c
>>
>> Please note that diffs are not public domain; they are subject to the
>> copyright notices on the relevant files.
>>
>>
>> Modified files:
>>
>> Index: src/external/gpl3/gdb/dist/bfd/merge.c
>> diff -u src/external/gpl3/gdb/dist/bfd/merge.c:1.1.1.4
>> src/external/gpl3/gdb/dist/bfd/merge.c:1.2
>> --- src/external/gpl3/gdb/dist/bfd/merge.c:1.1.1.4 Tue Feb 2 22:00:11 2016
>> +++ src/external/gpl3/gdb/dist/bfd/merge.c Fri Feb 26 11:28:14 2016
>> @@ -334,7 +334,7 @@ sec_merge_emit (bfd *abfd, struct sec_me
>>
>> /* Trailing alignment needed? */
>> off = sec->size - off;
>> - if (off != 0)
>> + if (pad != NULL && off != 0)
>> {
>> if (contents)
>> memcpy (contents + offset, pad, off);
>>
>
> It looks to me like a false positive.
>
> pad is checked just after bfd_zmalloc():
>
> pad = (char *) bfd_zmalloc (pad_len);
> if (pad == NULL)
>return FALSE;
>
> If I am not overlooking something, I will drop this local patch as not
> upstreamable.
>
>
>
signature.asc
Description: Message signed with OpenPGP
re: CVS commit: src/sys/arch/sparc/sparc
"Andrew Doran" writes: > Module Name: src > Committed By: ad > Date: Sat Mar 14 13:34:44 UTC 2020 > > Modified Files: > src/sys/arch/sparc/sparc: intr.c > > Log Message: > sparc cpu_intr_p(): try to work around l_cpu not being set early on by > using curcpu(). ah, good idea. this will involve one fewer deref, curcpu() is mapped at a fixed address, so it might even be faster now than it used to be. .mrg.
Modularize sun2 kernel (Re: CVS commit: src/sys/arch/sun2/conf)
(added port-sun2 and thorpej) On 2020/03/08 17:40, matthew green wrote: "Rin Okuyama" writes: Module Name:src Committed By: rin Date: Sun Mar 8 06:25:10 UTC 2020 Modified Files: src/sys/arch/sun2/conf: GENERIC Log Message: Retire md(4) in favor of tmpfs provided by module, though both are not useful for 8MB RAM system... ... it's cool you got modules on sun2 working... however, the module version of a thing is going to use more real memory than the built-in version, so i'm not sure where this is going yet... Sorry for the late reply. I was confused by weird kernel behaviors when memory shortage. But, I think that I slowly came to understand that :). Yes, modular kernel requires more memory, mainly because KERN_AS is changed from ``library'' to ``obj'': https://nxr.netbsd.org/xref/src/sys/lib/libkern/Makefile.inc#8 This pulls in all libkern routines to kernel. However, I think that modularization is the only way to prolong sun2 port as far as I can tell. We need to reduce kernel text as much as possible. This is because (1) Our bootloader mapps only 2MB for kernel image. (2) Even though bootloader can load kernel, it hangs here and there, if kernel size is near 2MB. For (1), our GENERIC is very close to this limit, even though almost all additional features are stripped off. I managed to map more 128KB memory, (maybe) not used by firmware, at least on TME: http://www.netbsd.org/~rin/sun2_bootloader_20200315.patch But I don't know whether it works or not on real hardwares. (2) is more serious. When kernel hangs, it is trapped in pagedaemon. I built a kernel with DIAGNOSTIC and LOCKDEBUG, but no failures are detected. Now, I imagine its scenario as follows: Our kernel allocates many memory on demand. Hangs may be the normal operation (not a bug) when memory is allocated with KM_SLEEP or M_WAITOK, but pagedaemon cannot free memory. Swap is not ready in early boot stage, or all memory is wired, or thrashing takes place like ping-pong; suppose two memory segments are required at the same time. pagedaemon swaps out segment 1 to swap in segment 2, and immediately after, it frees segment 2 to pull in segment 1. Then, ``deadlock'' is effectively formed, which is cannot detected by neither DIAGNOSTIC nor LOCKDEBUG. Hangs due to (2) are remarkably mitigated by modularization. More features can be loaded by modload(8), even if kernel stops working when that are statically linked into kernel. This is because kernel text is wired, whereas modules are pageable. 8MB? luxury? :) i've been testing in 4MB tme :) Sorry, s/8MB/7MB/ in the commit log. Framebuffer and etc. reside at PA 0x70 and above. I think that we can no longer support 4MB system because of (2); hangs due to (2) are much more serious for 4MB system than it is in 7MB system. Modern kernel allocates too much things on demand rather than statically allocating them in data segment... GENERIC became not working again due to (2) (even for 7MB). Can I go further, i.e., strip more features that can be provided by modules, or do you have any other ideas? P.S. In many cases of hangs due to (2), I can break into DDB from console. However, in some cases, especially due to "ifconfig lo0 127.0.0.1" in /etc/rc, it does not even accept input from console. I suspect that someone in our network code allocates memory with KM_SLEEP or M_WAITOK in the interrupt context, or with spl raised... Thanks, rin
Re: Modularize sun2 kernel (Re: CVS commit: src/sys/arch/sun2/conf)
> On Mar 14, 2020, at 7:57 PM, Rin Okuyama wrote: > > I think that we can no longer support 4MB system because of (2); hangs > due to (2) are much more serious for 4MB system than it is in 7MB system. > Modern kernel allocates too much things on demand rather than statically > allocating them in data segment... We should be striving to make that work. There are some redundant things (e.g. extent vs vmem) that we should explore getting rid of... and find ways to trim optionally functionality so we can keep these old systems running. -- thorpej
Re: Modularize sun2 kernel (Re: CVS commit: src/sys/arch/sun2/conf)
> On Mar 14, 2020, at 8:10 PM, Jason Thorpe wrote: > > redundant things (e.g. extent vs vmem) In case this wasn't obvious, I favor ejecting extent in favor of vmem. -- thorpej
