Re: kernel stack usage

2020-07-04 Thread Jaromír Doleček
Le sam. 4 juil. 2020 à 15:30, Kamil Rytarowski  a écrit :
> > Kamil - what's the difference in gcc between -Wframe-larger-than= and
> > -Wstack-size= ?
> >
>
> -Wstack-size doesn't exist?

Sorry, meant -Wstack-usage=

> > I see according to gcc documentation -Wframe-larger-than doesn't count
> > size for alloca() and variable-length arrays, which makes it much less
> > useful than -Wstack-usage.
> >
>
> It's not a problem.
>
> Whenever alloca or VLA is in use, it's already breaking the stack
> protector. There are a few exceptions registered in sys/conf/ssp.mk.
>
> We could add -Walloca -Wvla for USE_SSP=yes builds to catch quickly
> inappropriate usage (frequently violated by code optimizer).

It's already not used in our kernel code, I checked and I found it
used only in some arch-specific stand/ code. So -Walloca should be
safe to turn on unconditionally, regardless of SSP. Unless the
compiler emits alloca() calls itself.

Jaromir


Re: kernel stack usage

2020-07-04 Thread Kamil Rytarowski
On 04.07.2020 14:00, Jaromír Doleček wrote:
> Can anybody using clang please confirm kernel build with
> -Wframe-larger-than=3584?
> 

NetBSD-current from today, amd64 GENERIC builds for me.

> Kamil - what's the difference in gcc between -Wframe-larger-than= and
> -Wstack-size= ?
> 

-Wstack-size doesn't exist?

> I see according to gcc documentation -Wframe-larger-than doesn't count
> size for alloca() and variable-length arrays, which makes it much less
> useful than -Wstack-usage.
> 

It's not a problem.

Whenever alloca or VLA is in use, it's already breaking the stack
protector. There are a few exceptions registered in sys/conf/ssp.mk.

We could add -Walloca -Wvla for USE_SSP=yes builds to catch quickly
inappropriate usage (frequently violated by code optimizer).


On 04.07.2020 14:10, Martin Husemann wrote:
> On Sat, Jul 04, 2020 at 02:00:04PM +0200, Jaromír Dole?ek wrote:
>> Can anybody using clang please confirm kernel build with
>> -Wframe-larger-than=3584?
>
> Side note: 3584 is inacceptably large, we need to trim it down to ~1k.
>

Setting it to 3584 is a good start, it can be lowered later to 1024.



signature.asc
Description: OpenPGP digital signature


Re: kernel stack usage

2020-07-04 Thread Martin Husemann
On Sat, Jul 04, 2020 at 02:00:04PM +0200, Jaromír Dole?ek wrote:
> Can anybody using clang please confirm kernel build with
> -Wframe-larger-than=3584?

Side note: 3584 is inacceptably large, we need to trim it down to ~1k.

Martin


Re: kernel stack usage

2020-07-04 Thread Jaromír Doleček
Can anybody using clang please confirm kernel build with
-Wframe-larger-than=3584?

Kamil - what's the difference in gcc between -Wframe-larger-than= and
-Wstack-size= ?

I see according to gcc documentation -Wframe-larger-than doesn't count
size for alloca() and variable-length arrays, which makes it much less
useful than -Wstack-usage.

Jaromir

Le dim. 31 mai 2020 à 16:39, Kamil Rytarowski  a écrit :
>
> Can we adopt -Wframe-larger-than=1024 and mark it fatal?
>
> This option is supported by GCC and Clang.
>
> On 31.05.2020 15:55, Jaromír Doleček wrote:
> > I think it would make sense to add -Wstack-usage=X to kernel builds.
> >
> > Either 2KB or 1KB seems to be good limit, not too many offenders
> > between 1KB and 2KB so maybe worth it to use 1KB and change them.
> >
> > I'm sure there would be something similar for LLVM too.
> >
> > Jaromir
> >
> > Le dim. 31 mai 2020 à 15:39, Simon Burge  a écrit :
> >>
> >> matthew green wrote:
> >>
> >>> glad to see this effort and the clean up already!
> >>>
> >>> ideally, we can break the kernel build if large stack consumers
> >>> are added to the kernel.  i'd be OK with it being default on,
> >>> with of course a way to skip it, and if necessary it can have
> >>> a whitelist of "OK large users."
> >>
> >> I started to look at -fstack-usage which gives a nice MI way of getting
> >> the data instead of parsing MD objdump output, but didn't get any
> >> further than the below patch.  The find(1) command referenced in the
> >> patch gives the following
> >>
> >>   1008 nfs_serv.c:2181:1:nfsrv_link
> >>   1056 procfs_linux.c:422:1:procfs_do_pid_stat
> >>   1056 vfs_subr.c:1521:1:vfs_buf_print
> >>   1072 dl_print.c:80:1:sdl_print
> >>   1104 core_elf32.c:300:1:coredump_getseghdrs_elf32
> >>   1104 core_elf32.c:300:1:coredump_getseghdrs_elf64
> >>   1104 sys_ptrace_common.c:1595:1:proc_regio
> >>   1120 subr_bufq.c:131:1:bufq_alloc
> >>   1136 db_lwp.c:64:1:db_lwp_whatis
> >>   1152 kern_ktrace.c:1269:1:ktrwrite
> >>   1168 uvm_swap.c:768:1:uvm_swap_stats.part.1
> >>   1312 nfs_serv.c:1905:1:nfsrv_rename
> >>   2112 tty_60.c:68:1:compat_60_ptmget_ioctl
> >>   2144 memmem.c:83:14:twoway_memmem
> >>
> >> Anyone else want to run with adding a bit more to this to do some build
> >> failure as mrg@ suggests and documenting for options(4)?
> >>
> >> Cheers,
> >> Simon.
> >> --
> >> Index: Makefile.kern.inc
> >> ===
> >> RCS file: /cvsroot/src/sys/conf/Makefile.kern.inc,v
> >> retrieving revision 1.270
> >> diff -d -p -u -r1.270 Makefile.kern.inc
> >> --- Makefile.kern.inc   21 May 2020 18:44:19 -  1.270
> >> +++ Makefile.kern.inc   31 May 2020 13:34:13 -
> >> @@ -104,6 +104,11 @@ CFLAGS+=   -ffreestanding -fno-zero-initia
> >>  CFLAGS+=   ${${ACTIVE_CC} == "gcc":? -fno-delete-null-pointer-checks 
> >> :}
> >>  CFLAGS+=   ${DEBUG} ${COPTS}
> >>  AFLAGS+=   -D_LOCORE -Wa,--fatal-warnings
> >> +.if defined(KERN_STACK_USAGE)
> >> +# example usage to find largest stack users in kernel compile directory:
> >> +#find . -name \*.su | xargs awk '{ printf "%6d %s\n", $2, $1 }' | 
> >> sort +1n
> >> +CFLAGS+=   -fstack-usage
> >> +.endif
> >>
> >>  # XXX
> >>  .if defined(HAVE_GCC) || defined(HAVE_LLVM)
> >> @@ -338,8 +343,8 @@ ${_s:T:R}.o: ${_s}
> >>  .if !target(__CLEANKERNEL)
> >>  __CLEANKERNEL: .USE
> >> ${_MKMSG} "${.TARGET}ing the kernel objects"
> >> -   rm -f ${KERNELS} *.map eddep tags *.[io] *.ko *.ln [a-z]*.s vers.c 
> >> \
> >> -   [Ee]rrs linterrs makelinks assym.h.tmp assym.h \
> >> +   rm -f ${KERNELS} *.map *.[io] *.ko *.ln [a-z]*.s *.su vers.c \
> >> +   eddep tags [Ee]rrs linterrs makelinks assym.h.tmp assym.h \
> >> ${EXTRA_KERNELS} ${EXTRA_CLEAN}
> >>  .endif
> >>
>
>


Re: Waiting for a bit in a register to be cleared: which strategy?

2020-07-04 Thread Mouse
>> I'd say, just use the simple loop.  Once you have that much working,
>> *then* worry about things like handling (pseudo-)hardware failures
>> or interrupts, or sleeping instead of busy-waiting.
> The loop works; I honestly think that the bit is cleared in a
> negligible amount of time,

If you're curious, then count the iterations and report the number
after the loop:

 int loops;

 for (loops=0;;loops++) { if (! (bus_space_read_... & ...)) break; }
 printf("loop count %d\n",loops);

A printf like that is a bad idea in production code, but you are
nowhere near production at this point.

> but it's right and proper to handle a failure.

True.  It's often hard to test error-handling code, but that doesn't
make it unimportant.  (Though, for emulated hardware like this,
hardware failure is effectively impossible, so hardware failure
handling code is important only for what it teaches you.)

/~\ The ASCII Mouse
\ / Ribbon Campaign
 X  Against HTMLmo...@rodents-montreal.org
/ \ Email!   7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B


Re: Waiting for a bit in a register to be cleared: which strategy?

2020-07-04 Thread Rocky Hotas
On lug 03 20:15, Mouse wrote:

> Then, yes, just loop.  First make it work, then make it better.

Ok! I did exactly this way.

> Well, be aware that DELAY() on many machines, especially for small
> arguments, _is_ just a cycle-burning loop - it just multiplies the
> argument by a constant calculated based on CPU speed.

Yes, IIUC from the manpages, DELAY(n) uses the argument n in a similar
way mstohz(9) would.

> > Yes, of coure.
> 
> If that's "of course" to you, you're not a _complete_ newbie. :-)

:) I tried to follow the logic reasoning behind your text. I have some
theory background, but reading code (and even more, writing it) is still
a big challenge.

> I'd say, just use the simple loop.  Once you have that much working,
> *then* worry about things like handling (pseudo-)hardware failures or
> interrupts, or sleeping instead of busy-waiting.

Ok, I'm relieved, because just did as you suggested. The loop works; I
honestly think that the bit is cleared in a negligible amount of time,
but it's right and proper to handle a failure. As a next improvement,
I'll try with waiting and then with an interrupt, if I manage to.
Thanks!

Rocky


Re: Waiting for a bit in a register to be cleared: which strategy?

2020-07-04 Thread Rocky Hotas
On lug 04  8:02, Iain Hibbert wrote:

> see pci_intr(9) and look at what a device in dev/pci does. I don't know 
> which is 'simple' but sv.c seems pretty simple in what it does with 
> interrupts the sv_intr() function is pretty small. You set up the 
> interrupt, enable it and your function gets called (in interrupt context). 

It was exactly what I was meaning: few lines, easy to read for someone
who has never seen an interrupt handling implementation.
Thank you, also for pci_intr(9)!

Rocky