Re: CVS commit: src/sys/kern

2009-03-26 Thread David Laight
On Wed, Mar 25, 2009 at 09:48:37PM +, David Young wrote:
 Module Name:  src
 Committed By: dyoung
 Date: Wed Mar 25 21:48:37 UTC 2009
 
 Modified Files:
   src/sys/kern: subr_autoconf.c
 
 Log Message:
 ctags(1) gets confused by 'typedef struct X { } X_t', so break 'typedef
 struct pmf_private { ... } pmf_private_t' into a struct definition and a
 typedef definition.

Surely it is better to fix ctags 

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2009-05-16 Thread David Laight
On Sat, May 16, 2009 at 12:02:00PM +, YAMAMOTO Takashi wrote:
 Module Name:  src
 Committed By: yamt
 Date: Sat May 16 12:02:00 UTC 2009
 
 Modified Files:
   src/sys/kern: init_sysctl.c
 
 Log Message:
 sysctl_doeproc:
   - simplify.
   - KERN_PROC: fix possible stale proc pointer dereference.
   - KERN_PROC: don't do copyout with proc_lock held.

IIRC this used to work because it locked the userspace buffer into
physical memory earlier.
I've not looked at the change, but if you release proc_lock it is
very difficult to ensure you see every process [1], and that the
count of processes is correct.

[1] consider what happens if the proc table has to be extended,
or when a process is in a fork/exit loop.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: [yamt-nfs-mp] src/sys

2009-06-25 Thread David Laight
On Wed, Jun 24, 2009 at 09:24:57PM -0700, Bill Stouder-Studenmund wrote:
   
   The fact that the caller has a reference to the vnode at all should
   really be enough, surely??
  
  do you mean vref?
  it doesn't prevent revoke(2).
  thus a filesystem can't access v_data safely.

If revoke works by changing fields of the vnode (like v_data) then
it is always going to cause grief - since v_data can then disappear
during the entry of any VOP call.

In a single-threaded, non-smp, kernel the data areas can probably be
changed in a consistent manner. With SMP you have to do things
differently.  So if revoke is making the vnode reference some other
(dummy) fs and then releasing all references to the original fs, it
just isn't going to work.

 How does Solaris do this? My understanding is you _don't_ have to lock 
 nodes any where near as often as we do (or did last I looked, which was 
 admittedly a while ago).

FWIW SYSV handled controlling terminal 'revoke' by disallowing access
to the major/minor in the specfs code.  So even if you did another open()
you couldn't access the device that had been your ctty. (Which was rather
fun when the ctty was a clone device from the network stack, and the
program was a network daemon.)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/sys

2009-08-09 Thread David Laight
On Sat, Aug 08, 2009 at 05:23:15PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Sat Aug  8 21:23:15 UTC 2009
 
 Modified Files:
   src/sys/sys: bswap.h cdefs.h endian.h termios.h
 
 Log Message:
 Create and use __CAST(type, value) in headers so that modern c++ with
 -Wold-style-casts does not bitch.

Do we really have to do that, it starts making files completely unreadable.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/sys

2009-08-10 Thread David Laight
On Mon, Aug 10, 2009 at 12:24:47AM +0200, Martin Husemann wrote:
 On Sun, Aug 09, 2009 at 09:20:02PM +, Christos Zoulas wrote:
  The new style casts are very useful because they show programming intent.

So do C casts! provided what you are casting is an integer type or a
pointer to a struct (rather than a class).
 
 Yes, new style casts are fine.

Hmm... even writing a bit of C++ I used C casts when passing buffers
to send() etc - casts which are needed because C++ doesn't have a 
working 'void *'.

 But warning about old style casts in a language that is designed to be C
 compatible is plain stupid.
 
 Besides, there should be no casts in system headers ;-}

What happens if these casts are put into inline functions inside the
'exterrn C' block. The compiler really has no excuse to warn
then, if it does it is a compler bug.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2009-08-13 Thread David Laight
On Thu, Aug 13, 2009 at 08:41:36PM +0200, Alan Barrett wrote:
 What is an undescribed, direct ioctl, and where is it documented?

Probably one that doesn't abuse the cmd field to include the length etc!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.sbin/puffs/rump_smbfs

2009-09-18 Thread David Laight
On Thu, Sep 17, 2009 at 09:05:38AM +0100, Nick Hudson wrote:
 
  People are most welcome to fix this ugliness 
  properly by helping to get rid of link sets in the kernel.
 
 I'm glad you agree that link sets are ugly. On the matter of a getting rid of 
 them I did offer to discuss a solution, but was ignored. 

Like all things they have their place, but overuse can be a problem.

For instance they could solve the problem of getting an intial entry
point (code or data) into a kernel 'module' that has being linked
into a kernel with 'ld -r'.
(when modules are loaded it is possibly to do a symbol lookup...)

There are probably other places where it is difficult to call an
explicit initialiser.

It is all rather similar to the proliferation of memory pools - for
things where 'malloc' would be fine.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/distrib

2009-09-20 Thread David Laight
On Sat, Sep 19, 2009 at 09:05:27PM +, Christos Zoulas wrote:
 
 if [ -n $1 ]; then
   awk -f .../list2sh.awk $@
 fi

Since $@ is being substituted, wouldn't a check against $#
be more appropriate?
 
 and that X$1 idiom needs to die

It is necessary to guarantee the way the expression is evaluated
if/when $1 (in the above example) is, say, -o or some other string
that might be deemed to be an operator.

Although posix defines strict rules for expressions with small
numbers of arguments, netbsd'd parser doesn't obey them!

 as well as using unquoted variables
 which will break for strings with whitespace.

Sometimes I empty IFS

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/distrib

2009-09-21 Thread David Laight
On Sun, Sep 20, 2009 at 05:32:17PM -0400, Christos Zoulas wrote:
 
 | Although posix defines strict rules for expressions with small
 | numbers of arguments, netbsd'd parser doesn't obey them!
 
 Really? Can you give me an example.

See PR/34646

$ test ! = foo

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libc/stdio

2009-10-15 Thread David Laight
On Thu, Oct 15, 2009 at 12:01:01AM +0200, Alan Barrett wrote:
 On Wed, 14 Oct 2009, David Laight wrote:
  Module Name:src
  Committed By:   dsl
  Date:   Wed Oct 14 21:25:52 UTC 2009
  
  Modified Files:
  src/lib/libc/stdio: fgets.c vfprintf.c
  
  Log Message:
  Change a while () {} into a do {} while() so that fgets(buf, 1, file)
  detects EOF on an empty file.
  Fixes most of PR/41992
 
 The changes to vfprintf.c do not seem to match the log message.

Fixed - the preserved local change was all inside #if 0 

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libc/stdio

2009-10-26 Thread David Laight
On Sun, Oct 25, 2009 at 05:42:20PM +, Christos Zoulas wrote:
 
 Can we just revert the past 2 commits? Changing:
 
 (size_t)x - x + 0u
 
 does not look like an improvement to me. At least the first shows the intent,
 the second is just confusing, specially when size_t is unsigned long.

Actually, IMHO, lint is just being too picky here.
Having to add a cast whenever an 'int' variable is passed to a function
that has a 'size_t' parameter really is just polluting the code with
pointless casts.

I have a deep dislike for casts, and especially ones between integer
types.  Probably because I've been caught out once too often by an
unexpected cast converting beteen pointers and integers.

Some of these lint checks really should have been killed once
ANSI function prototypes were invernted.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/cddl/osnet/dist/uts/common/fs/zfs

2009-10-29 Thread David Laight
On Wed, Oct 28, 2009 at 11:44:51PM +, Adam Hamsik wrote:
 Module Name:  src
 Committed By: haad
 Date: Wed Oct 28 23:44:51 UTC 2009
 
 Modified Files:
   src/external/cddl/osnet/dist/uts/common/fs/zfs: zfs_vnops.c zfs_znode.c
 
 Log Message:
 Add workaround about zfs vnode reclaiming deadlock by checking if we don't
 ehld ZFS_MUTEX_OBJ already. If we can lock OBJ_MUTEX deffer execution of
 zfs_zinactive to taskq. Code was inspired by FreeBSD zfs_freebsd_reclaim.

This doesn't sound right at all ...

If the 'If we can lock OBJ_MUTEX' test is a try_mutex_enter() then
it may fail because another lwp owns it ...
I hope it is only the checkin comment that is inverted!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2009-11-03 Thread David Laight
On Tue, Nov 03, 2009 at 05:08:19AM +, David Young wrote:
 Module Name:  src
 Committed By: dyoung
 Date: Tue Nov  3 05:08:19 UTC 2009
 
 Modified Files:
   src/sys/arch/i386/i386: copy.S
 Added Files:
   src/share/man/man9/man9.i386: return_address.9
   src/sys/arch/i386/include: return.h
 
 Log Message:
 Add return_address(9) for reading the Nth return address from the call
 stack.

And how is that supposed to be implementable 

If the kernel is compiled without a stack frame register (%ebp)
then finding return addresses further back is ~impossible.

I wouldn't want to assume that the kernel is always compiled
using %ebp as a frame pointer - x86 has few enough registers that
freeing %ebp is probably a performance gain!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/bsd/mdocml/dist

2009-11-07 Thread David Laight
On Sat, Nov 07, 2009 at 12:48:16AM -0700, M. Warner Losh wrote:
 
 which ones have the same name, but different behavior on OS X?  A quick read
 of the man pages suggests they are identical...

Check about whether they need a \n at the end of the format.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/conf

2009-11-15 Thread David Laight
On Sun, Nov 15, 2009 at 08:10:20PM +, David Holland wrote:
 On Sun, Nov 15, 2009 at 01:39:00PM +, David Laight wrote:
   Avoids problems with awk processing floating point numbers when LC_NUMERIC
   give a decimal point of ','.
 
 Isn't it a bug in awk for the input language to be broken by locale
 settings?

yes, see PR/42320


David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/gnu/dist/gdb6/gdb

2009-11-27 Thread David Laight
On Fri, Nov 27, 2009 at 12:45:18AM -0800, Matt Thomas wrote:
 
  i wonder if we can re-add proc0paddr, defined to be = lwp0.l_addr
  at some point in main, to help this work with older gdb?
  
  this seems like a worth-while change since it's part of bsd-kvm.c.
 
 would need to be in md code since L_ADDR needs to come from assym.h.
 
   .globl  _C_LABEL(proc0paddr)
 _C_LABEL(proc0paddr) = _C_LABEL(lwp0) + L_ADDR

Actually I think it might be possibly to do it in a MI .c file!

Some __asm pattern being passed offsetof(struct lwp, l_addr) generating
a global symbol.
One of the forms for defining a symbol is MI.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/gnu/dist/gdb6/gdb

2009-11-28 Thread David Laight
On Sat, Nov 28, 2009 at 08:29:43AM +0900, enami tsugutomo wrote:
   i wonder if we can re-add proc0paddr, defined to be = lwp0.l_addr
   at some point in main, to help this work with older gdb?
   
   this seems like a worth-while change since it's part of bsd-kvm.c.
  
  would need to be in md code since L_ADDR needs to come from assym.h.
  
  .globl  _C_LABEL(proc0paddr)
  _C_LABEL(proc0paddr) = _C_LABEL(lwp0) + L_ADDR
 
 Is initializing it in main() too late?  I mean putting proc0paddr =
 lwp_getpcb(lwp0) at the beginning of main().

The above is defining a global symbol that referes to the middle of
a struct, not setting a memory location to the address of the it.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/compat

2009-11-28 Thread David Laight
On Sat, Nov 28, 2009 at 11:21:30PM +0100, Joerg Sonnenberger wrote:
 Are you sure about the PR?

should have been 31368 :-(

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/share/mk

2009-11-29 Thread David Laight
On Sun, Nov 29, 2009 at 04:00:00PM +, Masao Uebayashi wrote:
 Module Name:  src
 Committed By: uebayasi
 Date: Sun Nov 29 16:00:00 UTC 2009
 
 Modified Files:
   src/share/mk: bsd.subdir.mk
 
 Log Message:
 Remove an unneeded test (.if defined(V)) in .for v in ${V} ... .endfor.
 Tested by running build.sh distribution.

Some versions of make error ${V} when V is undefined.
Safer is ${V:U}

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/distrib/sets

2009-12-05 Thread David Laight
On Sun, Dec 06, 2009 at 01:06:24AM +0900, Masao Uebayashi wrote:

 Could you elaborate how the old version failed on Mac OS X?

lists=$(
for _list in $( echo ${OPTARG} | tr , ' ' ); do
case $_list in
base)   echo ${nlists} ;;
x)  echo ${xlists} ;;
ext)echo ${extlists} ;;
esac
done
)

My guess is that the parser has terminated the first '$(' on the
case label 'base)' while trying to skip the unwanted case clause.

Quite possibly it would all work if the $(...) were replaced by `...`
(the nested one just needs a couple of \).

However since the 'for' doesn't ever generate a subshell there
is no need for the complication.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/distrib/sets

2009-12-06 Thread David Laight
On Mon, Dec 07, 2009 at 01:29:50AM +0900, Masao Uebayashi wrote:
 
 Hmm.  Is my version incompatible to POSIX?  Or do we go to write really
 portable shell script load like Autoconf, not POSIX shell? [1]

It is posix compliant, but falls foul of a common shell bug.

http://www.gnu.org/software/hello/manual/autoconf/Shell-Substitutions.html

Avoid commands that contain unbalanced parentheses in here-documents,
comments, or case statement patterns, as many shells mishandle them.
For example, Bash 3.1, ksh88, pdksh 5.2.14, and Zsh 4.2.6 all
mishandle the following valid command: 

echo $(case x in x) echo hello;; esac)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2009-12-14 Thread David Laight
On Mon, Dec 14, 2009 at 06:21:55AM +, David Holland wrote:
 On Sun, Dec 13, 2009 at 08:02:24PM +, David Laight wrote:
   Log Message:
   Another, better, fix for PR/26567.
   Only sleep once within each pipe_read/pipe_write call.
   If there is no data/space available after we wakeup return ERESTART so
   then the 'fd' number is validated again.
   A simple broadcast of the cvs is then enough to evict the correct threads
   when close() is called from an active thread.
 
 Isn't this going to cause a thundering herd problem if there are a lot
 of readers competing for the input? The first one will wake up and
 consume the available data, and then the rest will take a trip all the
 way to userspace and back.
 
 This may or may not be a practical problem for pipes but I'd expect
 the same approach to suck pretty hard for e.g. accept() on sockets. :(

Sockets have other problems - like not always having a syscall interface
for ERESTART to loop on.

I was actually thinking of making the fo_abort() call set a flag on
the socket and only do the ERESTART if that flag is set.
Then you only take the hit on 'file's associated with 'fd' that get
closed with a read/write in progress.

I also need to rename fo_abort() to fo_restart().

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2009-12-16 Thread David Laight
On Wed, Dec 16, 2009 at 12:31:21AM -0500, Michael wrote:
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.124 -r1.125 src/sys/kern/sys_pipe.c
 
 
 This causes spurious aborts with tar and gzip
 For example tar tzf pkgsrc/distfiles/mysql-4.1.22.tar.gz aborts  
 halfway through but works fine with revision 1.124

I think that is all down to one of the relevant processes not liking
a partial return from a blocking write().

I commented out that bit, but have a better fix in mind.
(In particular one that doesn't use ERESTART unless one thread closes
an fd while other threads are doing io on that fd.)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2009-12-16 Thread David Laight
On Wed, Dec 16, 2009 at 06:24:04AM +, David Holland wrote:
 On Tue, Dec 15, 2009 at 06:35:18PM +, David Laight wrote:
   Modified Files:
  src/sys/kern: sys_pipe.c
   
   Log Message:
   Don't ERESTART write() calls for now.
   I suspect some programs don't allow for the partial transfer.
 
 More likely, the pipe is stuttering and repeating itself. Once data's
 been transferred, you *have* to succeed and return the amount
 transferred; otherwise anything that tries again (whether the
 application or transparently by syscall restart) is going to send the
 same data over again.

The code to handle that is in the sys_read()/sys_write() path.
In the same place that EINTR gets converted to 'success' if
any bytes have been transferred.

David

-- 
David Laight: da...@l8s.co.uk


CVS commit: src/usr.bin/msgc

2010-01-02 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Sat Jan  2 16:08:20 UTC 2010

Modified Files:
src/usr.bin/msgc: msg_sys.def

Log Message:
Remove some sign compare warnings.


To generate a diff of this commit:
cvs rdiff -u -r1.37 -r1.38 src/usr.bin/msgc/msg_sys.def

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/distrib/utils/sysinst/arch/mac68k

2010-01-02 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Sat Jan  2 17:15:07 UTC 2010

Modified Files:
src/distrib/utils/sysinst/arch/mac68k: menus.md.de menus.md.en
menus.md.es menus.md.pl

Log Message:
Sanitise whitespace to minimise the differences between these files.


To generate a diff of this commit:
cvs rdiff -u -r1.3 -r1.4 src/distrib/utils/sysinst/arch/mac68k/menus.md.de
cvs rdiff -u -r1.21 -r1.22 src/distrib/utils/sysinst/arch/mac68k/menus.md.en
cvs rdiff -u -r1.2 -r1.3 src/distrib/utils/sysinst/arch/mac68k/menus.md.es
cvs rdiff -u -r1.8 -r1.9 src/distrib/utils/sysinst/arch/mac68k/menus.md.pl

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/distrib/utils/sysinst/arch

2010-01-02 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Sat Jan  2 20:54:46 UTC 2010

Modified Files:
src/distrib/utils/sysinst/arch/mvme68k: md.c
src/distrib/utils/sysinst/arch/x68k: md.c

Log Message:
Fix another couple of signed v unsigned comparisons of disk block numbers.


To generate a diff of this commit:
cvs rdiff -u -r1.22 -r1.23 src/distrib/utils/sysinst/arch/mvme68k/md.c
cvs rdiff -u -r1.37 -r1.38 src/distrib/utils/sysinst/arch/x68k/md.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/sys/arch/i386/i386

2010-01-10 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Sun Jan 10 15:21:36 UTC 2010

Modified Files:
src/sys/arch/i386/i386: trap.c vector.S

Log Message:
If we fault on the 'iret' during return to userpace (eg if %eip is outside
the bounds of %cs) then hack the stack to contain a normal fault frame
for the signal setup code (etc).
Previously the code assumed that the original user trap frame was still
present - at it is for faults when loading the segment registers.


To generate a diff of this commit:
cvs rdiff -u -r1.250 -r1.251 src/sys/arch/i386/i386/trap.c
cvs rdiff -u -r1.49 -r1.50 src/sys/arch/i386/i386/vector.S

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/sys/arch/i386/i386

2010-01-10 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Sun Jan 10 15:37:36 UTC 2010

Modified Files:
src/sys/arch/i386/i386: trap.c

Log Message:
If we fault on the iret during return to userspace, see if we need to
do a lazy update of %cs to make the stack executable.
If a change is made, just retry the failing sequence.
Signal handlers as gcc nested local functions now work!


To generate a diff of this commit:
cvs rdiff -u -r1.251 -r1.252 src/sys/arch/i386/i386/trap.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



CVS commit: src/sys/arch/i386/i386

2010-01-17 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Sun Jan 17 22:21:18 UTC 2010

Modified Files:
src/sys/arch/i386/i386: trap.c vector.S

Log Message:
Fix 'fault on load of %gs during retirn to userspace' to look for the
  correct instruction bytes.
Take the 'fault on load segment register' through the same path as 'fault
  on iret' so we don't have to fixup the broken stackframe that contains a
  mix of user and kernel registers,
Update comments about how the faults during return to userspace are processed.
Setting an invalid %gs in the saved context of a signal handler causes
  a SIGSEGV handler to be entered with what look like valid registers.


To generate a diff of this commit:
cvs rdiff -u -r1.252 -r1.253 src/sys/arch/i386/i386/trap.c
cvs rdiff -u -r1.50 -r1.51 src/sys/arch/i386/i386/vector.S

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



Re: CVS commit: [matt-nb5-mips64] src/sys/arch/mips

2010-01-31 Thread David Laight
On Sun, Jan 31, 2010 at 01:33:59AM +, David Holland wrote:
 
 gcc does not seem to be very deterministic about this kind of thing.

I've being trying to stop gcc using too many registers in another
project - with 24 available registers you'd think it would manage
not to spill locals to the stack! But no, the 'status function'
(which just copies info into a big buffer with a few local actions)
managed to generate temporaries all of its own!  In this case marking
the target buffer 'volatile' helped no end!

David

-- 
David Laight: da...@l8s.co.uk


CVS commit: src/crypto/external/bsd/netpgp/dist/src/lib

2010-02-06 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Sat Feb  6 10:50:52 UTC 2010

Modified Files:
src/crypto/external/bsd/netpgp/dist/src/lib: packet-parse.c validate.c

Log Message:
Fix printf formats on amd64 (and probably other 64bit systems).


To generate a diff of this commit:
cvs rdiff -u -r1.28 -r1.29 \
src/crypto/external/bsd/netpgp/dist/src/lib/packet-parse.c
cvs rdiff -u -r1.26 -r1.27 \
src/crypto/external/bsd/netpgp/dist/src/lib/validate.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



Re: CVS commit: src/sbin/mount_smbfs

2010-02-08 Thread David Laight
On Mon, Feb 08, 2010 at 07:56:07AM +, Iain Hibbert wrote:
 Module Name:  src
 Committed By: plunky
 Date: Mon Feb  8 07:56:06 UTC 2010
 
 Modified Files:
   src/sbin/mount_smbfs: Makefile.inc
 
 Log Message:
 use
 
 .if defined(HAVE_GCC)  ${HAVE_GCC} == 4
 
 rather than
 
 .if ${HAVE_GCC} == 4
 
 as HAVE_GCC may be undefined

That construct may not help size make needs to parse the entire line.
Shorter and safer is:

.if ${HAVE_GCC:U0} == 4

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys

2010-02-20 Thread David Laight
On Sun, Feb 21, 2010 at 07:18:20AM +, Mindaugas Rasiukevicius wrote:
 Darran Hunt dar...@netbsd.org wrote:
  The new code is modular - it has its own kern_dtrace.c module.  I'm  
  not sure that replacing the #ifdefs with stub functions is the best  
  idea - won't that make the proc and lwp functions a bit less efficient  
  with calls to the empty functions?
 
 Compiler will optimise them out.  We already do that in various places,
 including threading and scheduling code.

It will if they are #defined out (rather than being functions that just
return). However a moderm x86 cpu should prefetch through such a function
(all the jumps are fully predicted).

 Also:
 
 + if (dtrace_vtime_active) {
 + (*dtrace_vtime_switch_func)(newl);
 + }
 
 It is worth to give a separate cache line for dtrace_vtime_active and
 use __predict_false().

__predict_false() yes, but separating things into their own cache lines
is probably a pessimalisation - mainly because it increases the working
set (of cache lines) of the code.

David

-- 
David Laight: da...@l8s.co.uk


CVS commit: src/gnu/dist/gdb6/gdb

2010-02-22 Thread David Laight
Module Name:src
Committed By:   dsl
Date:   Mon Feb 22 08:19:38 UTC 2010

Modified Files:
src/gnu/dist/gdb6/gdb: amd64nbsd-tdep.c

Log Message:
Fix check for old trap frame layout.


To generate a diff of this commit:
cvs rdiff -u -r1.4 -r1.5 src/gnu/dist/gdb6/gdb/amd64nbsd-tdep.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.



Re: CVS commit: [matt-nb5-mips64] src/sys/arch/mips

2010-02-24 Thread David Laight
On Thu, Feb 25, 2010 at 05:53:23AM +, Matt Thomas wrote:
 
 Log Message:
 Make the UP and MP ASID allocation algorithm common.  Significantly improve
 the algorithm.  Now when we exhaust the ASIDs, interrogate the TLB for active
 ASIDS and release all the other for future allocations.  This leaves the
 TLB entries with ASIDs valid avoiding the need to re-incur TLB misses for
 them.

I presume it is willing to kill some TLB entries if the above doesn't
find any (or enough) free ASIDs ?
Or is the ASID number space guaranteed to be significantly higher that
the number of TLBs ??

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/dev/ic

2010-02-28 Thread David Laight
On Sat, Feb 27, 2010 at 04:40:12AM +, Izumi Tsutsui wrote:
 
 Log Message:
 Always call device dependent functions via pointers rather than
 using conditionals to switch inline functions for modern processors.

Eh ???
if (foo) bar(); else baz();
will probably execute faster than:
(*bar_baz)();
since the indirect jump is (IIRC) always unpredicted.
the 'if' version stands a chance of correctly predicting the jump.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys

2010-03-07 Thread David Laight
On Fri, Mar 05, 2010 at 06:35:02PM +, Antti Kantee wrote:
 
 Log Message:
 Move builtin modules to a list in init and load them from there
 instead of using linksets directly.  This has two implications:

3) It must now be almost possible to do an 'ld -r' of the kernel
(excluding any modules) to generate a 'netbsd.o' and then do a
final link to include a desired set of modules into a kernel image.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/sparc64

2010-03-07 Thread David Laight
On Sat, Mar 06, 2010 at 11:26:10PM +, matthew green wrote:
 Module Name:  src
 Committed By: mrg
 Date: Sat Mar  6 23:26:10 UTC 2010
 
 Modified Files:
   src/sys/arch/sparc64/conf: files.sparc64
   src/sys/arch/sparc64/sparc64: locore.s
 Added Files:
   src/sys/arch/sparc64/include: locore.h
   src/sys/arch/sparc64/sparc64: memcpyset.s
 
 Log Message:
 move the memcpy/memset implementations out into their own file, with the
 block copy versions as well.  move some of the definitions in locore.s
 into a new locore.h.

If you move memcpy/memset into src/common then they get auto built
as part of libc (or whatever the kernel version is called).

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys

2010-03-08 Thread David Laight
On Sun, Mar 07, 2010 at 03:11:35PM +0200, Antti Kantee wrote:
 
 What, specifically, is the problem with the netbsd.o approach?
 (changing the default link that way would be good step, especially for
 people wanting an easy-to-grow-at-home-from-sets MONOLITHIC)

Someone actually trying it ...

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/dev/ic

2010-04-06 Thread David Laight
On Wed, Mar 31, 2010 at 04:06:47PM -0500, David Young wrote:
  
  bus_space_barrier() doesn't flush ... barriers only enforce
  the ordering of operations (and, of course, with respect to
  non-overlapping addresses ... obviously reads after writes of the
  same address in code will be enforced on the bus without an explicit
  barrier).
 
 Right.  Putting the question another way, Is it important that reading
 the register we wrote lands the write as a side-effect?

It will have that effect, and, in many cases (think of writes being 'posted'
on the hardware itself - inside the final PCI device) a readback is the only
way to actually force a write to complete.

This is true almost regardless of the cpu and bus architecture.

 Do we expect that on sparc64, the bus barrier also lands the
 write as a side-effect?

The bus barrier is extremely unlikely to be able to get the write
past the first PCI bus.

The usual time this causes grief is when the write is a request to remove
an IRQ, and is executed immediately before the ISR returns.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/games/factor

2010-05-16 Thread David Laight
On Sat, May 15, 2010 at 09:22:39PM +, Joerg Sonnenberger wrote:
 
 Log Message:
 Follow the Fundamental Theory of Algebra. Disallow factorising of
 numbers less than 2 as it is not
 - naturally unique (negative numbers)
 - finite (0)
 - non-empty (1)

The 'Natural numbers (N)' are the positive integers, for which both addition
and multiplication are defined.
To make an additive group you need to include zero, zero is also needed to
make a number field.
If you include the subtraction operator then you need to include the
negative numbers - this gives the 'Integers (Z)'.
The notion of 'primes' is valid in Z - the definition of a prime is a
number that has no non-unit factors.
The units of Z are +1 and -1, so both +2 and -2 are primes (etc).

So nothing about algebra stops you factoring negative numbers.
However, since the 'prime factors' should be prime numbers, they
shouldn't include -1, but maybe the smallest factor should be negative.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/games/factor

2010-05-16 Thread David Laight
On Sun, May 16, 2010 at 11:37:43AM +0200, Martin Husemann wrote:
 On Sun, May 16, 2010 at 09:54:49AM +0100, David Laight wrote:
  The notion of 'primes' is valid in Z - the definition of a prime is a
  number that has no non-unit factors.
 
 Well, I only took the forced (for CS students) math courses at university,
 and it's been quite some time, but I would have defined a prime as a natural
 number  1. For what it's worth, wikipedia seems to agree with me ;-)
 (duck)

The definition of primality gets taken from that of the positive (or
non-negative integers) and applied more generally to other mathematical
objects - in particular 'fields'.
In field theory you need a definition that applies to any field, not
just the integers.
From memory - it is a long time since I did any field theory - a field
has a single 'zero', and possibly many 'units' (I think units are
defined as values that multiplying by doesn't change the magnitude).
For the field 'Z' (all the integers) the units are 1 and -1, and 
the mathematical definition of primality makes 'prime * unit' prime.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch

2010-05-29 Thread David Laight
On Fri, May 28, 2010 at 08:10:39AM +0200, Alan Barrett wrote:
 On Thu, 27 May 2010, David Holland wrote:
  On Thu, May 27, 2010 at 12:26:21PM +0200, Alan Barrett wrote:
I think it's a bug that .WAIT affects things declared in different
lines, but OK, thanks for the explanation.
  
  It's a separator, not a modifier. It's not a bug, just... counterintuitive.
 
 The .WAIT documentation says:
 
 If .WAIT appears in a dependency line, the sources that precede
 it are made before the sources that succeed it in the line.
 
 There's nothing about multiple dependency lines being considered
 together for the purpose of .WAIT processing.

The sources on the earlier line precede the .WAIT

Given that dependencies may get added in .for loops, .WAIT does need
to work this way.

In any case, as you know, the .WAIT processing is painfull enough as it is!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.bin/make

2010-06-09 Thread David Laight
On Wed, Jun 09, 2010 at 06:45:35PM +, Christos Zoulas wrote:
 
 This is wrong. Loop variables are not exapnded on each loop iteration.
 Each loop iteration effectively creates a new variable. The rest of his
 confusion comes from two simple facts:
 (1) += is lazy in bmake. This is different from FreeBSD, where it forces
 expansion.
 (2) The evaluation of j is lazy as well. That's why he sees the last
 loop iteration.
 
 Well, it was true when I wrote the for code (more than 16 years
 ago!). I guess dsl re-wrote it, but the net effect is the same.

I don't have a source tree handy, and the change hasn't made it to cvsweb,
but the .for loop should:
1) evaluate the list of values when the .for list is parsed
2) process the loop body such that the value substituted for the loop
   variable differs for each iteration.

xtos's version substituted the loop variable into the text prior to
the line being parsed - that gave some rather unexpected problems.

My rewrite left the expansion as a variable substitution by subtituting
${:Uvalue} for the loop vsriable. This allows other modifiers that
use variables that are only defined later (or in commands) to work.
However it did cause some different quoting issues.

I didn't think of generating a unique name for the variable on each
iteration - and just substituting that value.
In fact this would only be needed for command lines - which have delayed
expansion. For control lines (processed during file parsing) the
value can just be put into the symbol table.
It would solve all the quoting issues.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.bin/make

2010-06-09 Thread David Laight
On Wed, Jun 09, 2010 at 01:19:29PM -0600, M. Warner Losh wrote:
 
 It looks like it is lazy to me for all non-loop variables in FreeBSD:
...
 % cat M
 FOO=1
 BAR=2
 .for j in a b c
 FOO+= ${BAR} ${j}
 .endfor
 BAR=3
 
 all:
 @echo ${FOO}
 @echo ${BAR}
 %  make -f M
 1 3 a 3 b 3 c
 3
 %
 
 Not sure if this is correct or expected but it strikes me as
 useful and changes to this behavior would break things...

That is the expected/correct behaviour.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libm

2010-08-11 Thread David Laight
On Tue, Aug 10, 2010 at 05:53:09PM +, Matthias Drochner wrote:
 Module Name:  src
 Committed By: drochner
 Date: Tue Aug 10 17:53:08 UTC 2010
 
 Modified Files:
   src/lib/libm: Makefile
 
 Log Message:
 two disgusting hacks:
 -mk/bsd.lib.mk picks up a .S asm file behind our back (did it do so
  always?). s_modf.S is incorrect; I'm undecided whether it makes sense
  to fix it, so add a stupid rule to enforce the .c file to be used.

It might be worth using a .for loop to generate explicit dependencies for .o
files against the relevant source file when the files are added to $(OBJS)
instead of relying on the suffix rules.

If no commands are defined I think make will still use those from the
relevant suffix rule (as happens when the 'stupid rule' is added above).

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/netinet

2010-08-11 Thread David Laight
On Wed, Aug 11, 2010 at 09:36:45AM +, Antti Kantee wrote:
 Module Name:  src
 Committed By: pooka
 Date: Wed Aug 11 09:36:45 UTC 2010
 
 Modified Files:
   src/sys/netinet: ip_carp.c
 
 Log Message:
 Use kpause() instead of DELAY() and sleep a minimum of 1 tick.
 This is possible now since softints have a thread context.  It's
 also not a very frequent code path.  Addresses ABI issue with delay
 (kern/40505).

1) How many copies of this function can actually run at the same time ?
2) How many kernel threads are available for softints ??
3) How many other places might kernel threads sleep for timeout ???

It might be that they will all get stuck!

Actually, although kernel worker threads can sleep, in general they
shouldn't do so unless they are just waiting for locks and similar
short duration sleeps.
I remember seeing solaris systems stuck when more than 1 (or a few)
drivers slept in the kernel threads.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch

2010-08-17 Thread David Laight
On Tue, Aug 17, 2010 at 04:08:10PM +0100, Mindaugas Rasiukevicius wrote:
 
 Unifying sizes of certain types up to 64-bits would be a good choice
 from engineering standpoint.  Actually measuring the overhead would
 answer whether it is worth.

Why not use a 64bit type that is the union of a 64bit number and two
32bit numbers.
A kernel that uses 32bit paddr_t would use tha approriate (for the
endianness) 32bit value, having initialised the unused half to zero.

That would remove almost everything except the structure size overhead
while maintaining binary compatibility.

(Embedding the integer type in a struct/union also makes any places
where it is used incorrectly rather more obvious.)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libc/stdlib

2010-09-25 Thread David Laight
On Thu, Sep 23, 2010 at 12:02:42PM -0400, Christos Zoulas wrote:
 Module Name:  src
 Committed By: christos
 Date: Thu Sep 23 16:02:41 UTC 2010
 
 Modified Files:
   src/lib/libc/stdlib: setenv.c
 
 Log Message:
 PR/43899: Nicolas Joly: setenv(3)/unsetenv(3) memory leak.
 Partial fix: Don't allocate a new string if the length is equal to the
 old length, because presumably the old string was also nul terminated
 so it has the extra byte needed.
 The real fix is to keep an adjunct array of bits, one for each environment
 variable and keep track if the entry was allocated or not so that we can
 free it in unsetenv.

Hmmm I've just read the TOG page for putenv().
A conformant system isn't required to do anything other than save the
user-supplied pointer.
It even states that you can modify the value by changing the memory buffer
after calling putenv().

If the strings are copied in libc...
When libc starts, all the env strings will be adjacent, and at one end
of the stack area (placed by the kernel during exec).
So presumably the addess of the string can be compared against that
range to determine whether the string was malloc'ed or not.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/bin/cp

2010-10-25 Thread David Laight
On Mon, Oct 25, 2010 at 11:17:22AM +0200, Juergen Hannken-Illjes wrote:
 On Sun, Oct 24, 2010 at 05:21:06AM +, David Holland wrote:
  On Fri, Oct 22, 2010 at 05:56:06PM +, Antti Kantee wrote:
Disable mmap path.  With the current vnode locking scheme it has
a very annoying property: if the source media is slow (like a slow
network), the target file will be locked for the duration of the
entire max 8MB write and cause processes attempting to e.g. stat()
it to tstile (for several minutes in the worst case).  Revisit
this if/when vnode locking gets a little smarter.
  
  Wouldn't it be better to just ratchet back the block size to something
  like 64K that happens faster?
 
 Or first fault the mapped region instead of madvise() like:
 
   int pgsz = getpagesize();
   char *q;
   volatile char c;
 
   for (q = p; q  p+fs-st_size; q += pgsz)
   c = *q;

That won't really help - the pages could easily be discarded almost
immediately - eg in order to fault in a later page.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/bin/cp

2010-10-25 Thread David Laight
On Sun, Oct 24, 2010 at 10:56:40PM +, David Holland wrote:
   
   I think write() only needs to lock the the file enough to ensure that
   the file offset is correct.
 
 No, since in general the file is also being extended (certainly in
 this case it is) it also has to lock the file size, and that's going
 to deny stat() until it's done.

A stat request during a write can safely return the old size.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.bin/pathchk

2010-11-10 Thread David Laight
On Wed, Nov 10, 2010 at 06:19:25PM +, David Holland wrote:
 On Wed, Nov 10, 2010 at 01:34:02PM +, Christos Zoulas wrote:
   Christos seems to have made the same mistake as you, confusing -exec
   ... \; (which runs a separate child program for each file name) with
   -exec ... + (which passes many file names to each invocation of the
   child program).
   
   Yes, I did not know about exec {} +
 
 Neither did I; when was it invented? Did the POSIX folks come up with
 something good for a change?

Ditto - which probably means it is less portable that -print0 :-)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/powerpc/oea

2010-11-15 Thread David Laight
On Mon, Nov 15, 2010 at 08:50:34AM +, David Holland wrote:
 On Mon, Nov 15, 2010 at 02:48:29PM +0900, SODA Noriyuki wrote:
   Well, there is another thing which has to be considered.  That is
   name space pollution.
   Header files which are exported to userland (for userland visible APIs)
   should not export random symbols.  Only symbols which are defined as
   the interface of the header file are allowed to be exported.
 
 Indeed. Properly speaking though, headers that are exported to
 userland should define only the precise symbols that userland needs;
 kernel-only material should be kept elsewhere.

One start would be to add a sys/proc_internal.h so that sys/proc
can be reduced to only stuff that userspace and some kernel parts
are really expected to use.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/bin/dd

2010-11-23 Thread David Laight
On Tue, Nov 23, 2010 at 12:19:36AM +0200, Antti Kantee wrote:
  
  Surely it would be more appropriate to make thye rump kernel directly
  forward some paths to the real kernel?
 
 Can you explain how that could work?

First thoughts are something like the way /../ is used to 'escape'
from the emulation root.
But maybe just mount the real filesystem within the rump kernel?
Making something work well enough to copy files is probably not that hard!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/dev

2010-12-04 Thread David Laight
On Sat, Dec 04, 2010 at 03:50:25PM -0600, Michael Graff wrote:
 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA1
 
 I admit to a certain lack of understanding the twisty maze of pointers
 and memory mapping magic at play here, but is simply checking the length
 enough?  That is, what happens if I pass in a structure that is smaller
 than expected?
 
 That is, is there a way to check the actual size of the data passed into
 the ioctl, rather than the field in the structure we expect, or is that
 done at a higher level?

The length of the program's buffer is unknown.
The kernel uses the high 16 bits of the ioctl command to indicate
whether to read/write (2 bits) and a length (14 bits).
If either control bit is set, the ioctl syscall stub will do the
copyin/out and pass the actual device driver a pointer to the
in-kernel buffer.
So the device driver can always access the buffer length implied
from the command.

To my mind this is a horrid hack :-)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/dev

2010-12-06 Thread David Laight
On Mon, Dec 06, 2010 at 10:16:57AM -0600, Michael Graff wrote:
 
 E2BIG produces the odd message argument list too long -- my argument
 might be too long, but the list sure isn't.

This has to be a bit like EAGAIN - which always used to be no more
processes (from fork()), but now has a more general description.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand/lib

2010-12-19 Thread David Laight
On Sun, Dec 19, 2010 at 05:18:23PM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Sun Dec 19 17:18:23 UTC 2010
 
 Modified Files:
   src/sys/arch/i386/stand/lib: realprot.S
 
 Log Message:
 Compute real/protected %sp/%esp offset in 'gdt_fixup' using all 32-bits.
 Allows the case of %ss being less than %cs to work.
 Also, completely save and restore the general-purpose registers we use.

It ought to be possible to initialise the gdt (etc) as compile time.
The addresses are always fixed.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: [matt-nb5-mips64] src/sys/arch/mips

2010-12-22 Thread David Laight
On Wed, Dec 22, 2010 at 06:13:37AM +, Matt Thomas wrote:
 
 Log Message:
 Rework how fixups are processed.  Inside of generating a table, we just
 scan kernel text for jumps to locations between (__stub_start, __stub_end]
 and if found, we actually decode the instructions in the stub to find out
 where the stub would eventually jump to and then patch the original jump
 to jump directly to it bypassing the stub.  This is slightly slower than
 the previous method but it's a simplier and new stubs get automagically
 handled.

Isn't this a bit dangerous if anything other than code ends up in the
kernel text ?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand

2010-12-24 Thread David Laight
On Wed, Dec 22, 2010 at 08:46:43PM +, Jonathan A. Kollasch wrote:
 
 Log Message:
 It just so happens we don't need -Wno-attributes if we
 place __packed in the right place.

Does that need __packed (which would force unaligned access code)
be added for all accesses, or just __attribute__((aligned(n))) for
some fields?

This doesn't make much difference on i386, but will on archs that
don't support misaligned accesses.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand/lib

2010-12-24 Thread David Laight
On Sat, Dec 25, 2010 at 01:27:43AM +, Jonathan A. Kollasch wrote:
   
   libsa's printf(3) doesn't support %lld unless
   -DLIBSA_PRINTF_LONGLONG_SUPPORT is specified.
  
  True, that but it's not like this code path is often compiled.
  I should probably at least add the necessary gunk commented out
  in the makefile.
 
 That has the problem that I can't find a nice way to enable
 64-bit division runtime support in the amd64 libkern compiled
 for i386.

It is fairly easy to write the printf code so that it can convert
long long to decimal without requiring the 64bit division code
(or any division code!).
(Similarly you can avoid the double-word shifts for %x.)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/tests/fs/common

2010-12-31 Thread David Laight
On Fri, Dec 31, 2010 at 06:16:41PM +, Antti Kantee wrote:
 Module Name:  src
 Committed By: pooka
 Date: Fri Dec 31 18:16:41 UTC 2010
 
 Modified Files:
   src/tests/fs/common: h_fsmacros.h
 
 Log Message:
 Introduce r/o tests.  They do two mounts: the first one is r/w and
 runs a generator which primes the fs.  The second one is r/o and
 does the actual testing.  Also, introduce a nfsro fstype which does
 a clientside r/w mount for a r/o server export.
 
 requested by yamt
 
 (one nfsro test currently fails with EROFS vs. EACCES.  Hopefully
 someone else can debate the correct errno)

From what I remember of the NFS protocol, the following 'rules' applied:
1) If you export part of a filesystem, you export all of the filesystem.
2) If you give anyone access, you give everyone access.
3) If you give anyone write access, you give everyone write access.
This is all because it is the 'mount' protocol that verifies whether
a client has access - so a client that disobeys the mount protocol, or
fakes up valid nfs file handles can avoid the access checks.

It is possible that netbsd's nfs server does additional checks but
they are expensive to do on every nfs request.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/tests/fs/common

2011-01-02 Thread David Laight
On Sun, Jan 02, 2011 at 05:26:29AM +, David Holland wrote:
 On Sun, Jan 02, 2011 at 05:22:32AM +, David Holland wrote:
   It would also be worthwhile to test that nfsd doesn't allow writing
   via read-only handles, but that's a different issue and will require a
   different test framework that sends raw nfs packets.
 
 Hmm, no, maybe I misunderstood. Anyway, the mountd permissions issue
 is PR 3019, the longest-lived-without-attention bug in the whole bug
 database.

Hm, yes the client side is expected to check for -ro mounts.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2011-01-10 Thread David Laight
On Mon, Jan 10, 2011 at 11:11:04AM +, Juergen Hannken-Illjes wrote:
 Module Name:  src
 Committed By: hannken
 Date: Mon Jan 10 11:11:04 UTC 2011
 
 Modified Files:
   src/sys/miscfs/genfs: layer_extern.h layer_vnops.c
   src/sys/miscfs/nullfs: null_vnops.c
   src/sys/miscfs/overlay: overlay_vnops.c
   src/sys/miscfs/umapfs: umap_vnops.c
   src/tests/fs/ptyfs: t_nullpts.c
 
 Log Message:
 Add layer_revoke() that adjusts the lower vnode use count to be at least as
 high as the upper vnode count before passing down the VOP_REVOKE().
 
 This way vclean() check for active (vp-v_usecount  1) vnodes gets it right.

Randomly changing v_usecount sounds bogus to me?
Probably hiding some other bug somewhere else.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2011-01-10 Thread David Laight
On Mon, Jan 10, 2011 at 07:33:40PM +, David Holland wrote:
 On Mon, Jan 10, 2011 at 07:38:22PM +0100, Juergen Hannken-Illjes wrote:
   This is not a random change.  Layered file systems take exactly one
   reference on the lower vnode.  All references regarding layered
   vnodes are only taken on the layer vnode.  So we get a bootom vnode
   that is active (we are working on it) while it may have just one
   reference.  The upper (layer) vnode has the valid reference
   count.
   
   Now when VOP_REVOKE() goes down the stack it will revoke the lowest
   vnode but this vnode looks inactive.  Adjusting the lower vnode
   refcount gets the (in respect to activity) right usecount on the
   lowest vnode while VOP_REVOKE() works on the lowest vnode.
 
 As far as I can tell it only uses the usecount to decide whether the
 vnode is still active or not, so adding 1 temporarily should be
 sufficient if the goal is to have it not disappear.
 
 OTOH, it seems to me that it *should* disappear in that case, and also
 that vrevoke() should also end up getting called on the layer vnode.

What about other references to the 'lower' vnode?
If the revoke of the lower vnode is done one for each reference of
the layer vnode - then you need to add 'n - 1' to the ref count
of the lower vnode, not set it to 'n'.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/external/cddl/osnet

2011-01-14 Thread David Laight
On Fri, Jan 14, 2011 at 10:18:29AM +0100, haad wrote:
 
 I would like to have zfs build with -g -O0 built by default,
 this is wip code and there is high probability that someone
 will find some problem with it, therefore I would like to have
 debugging symbols already builtin.

You really don't want to be using -O0, ever.
I certainly find it easier to understand generated asm with -O2 (or
even -O3) since there is alot less crap in there.

You also need to debug code with the same compilation options as you
will use for any final builds - otherwise you can get some very
nasty surprises.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2011-01-18 Thread David Laight
On Tue, Jan 18, 2011 at 05:33:05PM +, Antti Kantee wrote:
 Module Name:  src
 Committed By: pooka
 Date: Tue Jan 18 17:33:05 UTC 2011
 
 Modified Files:
   src/sys/kern: syscalls.conf
 
 Log Message:
 Make syscallargs.h include sys/sched.h for cpuset_t typedef for
 the benefit of ports where it does not get typedef'd via autoinclusion
 of other headers.

Isn't is better to to add a local definition for the struct, and
then use that instead of the typedef?

Any code that cares about the actual struct will have included the
header that contains the real definition.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/common

2011-01-20 Thread David Laight
On Thu, Jan 20, 2011 at 11:17:59AM +, Manuel Bouyer wrote:
 Module Name:  src
 Committed By: bouyer
 Date: Thu Jan 20 11:17:59 UTC 2011
 
 Modified Files:
   src/common/include/prop: prop_array.h prop_dictionary.h
   src/common/lib/libprop: prop_kern.c
 
 Log Message:
 prop_*_copyout takes an object as second parameter, not a pointer to object.

I hope an 'object' is actually a pointer?
Or is the code passing structures by value ?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/dev/pci

2011-01-23 Thread David Laight
On Sun, Jan 23, 2011 at 03:15:07AM +, Izumi Tsutsui wrote:
 Module Name:  src
 Committed By: tsutsui
 Date: Sun Jan 23 03:15:06 UTC 2011
 
 Modified Files:
   src/sys/dev/pci: if_nfe.c
 
 Log Message:
 Pull the following fix from OpenBSD:
 http://www.openbsd.org/cgi-bin/cvsweb/src/sys/dev/pci/if_nfe.c#rev1.97
  Some nfe(4)/rlphy(4) combos don't work, because the PHY responds to all
  addresses on the mii bus.  As a countereasure, only attach the first PHY we
  encounter.  It is very unlikely we're going to ever see nfe(4) with 
  multiple
  PHYs.  The same is probably true for any modern NIC.

Not necessarily, sometimes there are 2 PHYs so that they can be switched
over if a cabling fault is detected.

There will also be the systems where the 'internal' PHY isn't used
because of specific (non-standard) media requirements.

I'm also not exactly sure what happens when MII crossover is used to
directly connect 2 MAC units on the same PCB.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2011-01-23 Thread David Laight
On Sun, Jan 23, 2011 at 11:01:09AM +, Marc Balmer wrote:
 Module Name:  src
 Committed By: mbalmer
 Date: Sun Jan 23 11:01:08 UTC 2011
 
 Modified Files:
   src/sys/kern: tty.c
 
 Log Message:
 Cast arguments to vaddr_t when using PRIxVADDR in the printf format string.

That seems strange - what type are the actual items being printed?
If they aren't already vaddr_t then PRIxVADDR probably shouldn't be
bing used.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2011-02-02 Thread David Laight
On Sun, Jan 30, 2011 at 08:03:20PM -0800, Matt Thomas wrote:
 
 I do expect the native on NetBSD to know how to get the disklabel location
 for the current machine it's running on from sysctl and not a header file.

That rather depends on where the disk came from.
Since it is relatively easy to move volumes between machines [1]
you actually need to look at the disk to find the label info,
neither a header file nor the kernel know where it will be.
(Clearly the kernel knows what it found if it managed to mount a fs.)

The i386/amd64 code plays 'hunt the disklabel' :-)

David

[1] Otherwise we wouldn't need support for byteswapped fs.

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/csu

2011-02-06 Thread David Laight
On Sun, Feb 06, 2011 at 07:46:23PM +, David Holland wrote:
 Making .PARSEDIR always absolute should fix this problem, but in the
 general case requires realpath().

It will also give problems to anyone using bmake under cygwin to
run windows binaries.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/librumpclient

2011-02-09 Thread David Laight
On Wed, Feb 09, 2011 at 02:29:58PM +, Antti Kantee wrote:
 
 Hence, prevent rumpclient from using the special fd's 0-2 for its
 purposes.

Probably worth dup'ing onto quite high numbers (if possible).
Well, above 20 anyway.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/conf

2011-02-10 Thread David Laight
On Thu, Feb 10, 2011 at 04:49:19PM +, Jean-Yves Migeon wrote:
 Module Name:  src
 Committed By: jym
 Date: Thu Feb 10 16:49:19 UTC 2011
 
 Modified Files:
   src/sys/arch/i386/conf: INSTALL
 
 Log Message:
 For i386, include MONOLITHIC for INSTALL rather than GENERIC. While here,
 remove drm drivers, we don't need them for install.
 
 i386 GENERIC has FFS and ELF support compiled as modules, so we hit
 an interesting chicken-egg situation when the kernel attempts to mount
 a ffs ramdisk, while the module might be contained inside... the ramdisk.

I'm not 100% sure it is worth having FFS and ELF support as modules
in GENERIC.
It may be nice that they CAN be modules, but I suspect 99.99% of systems
will need them.
Anyone who wants them as modules can build a kernel without them.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/conf

2011-02-11 Thread David Laight
On Fri, Feb 11, 2011 at 12:40:00AM +0100, Jean-Yves Migeon wrote:
 
 BTW, I wonder whether modules shouldn't be part of the kern-GENERIC.tgz
 set. But this is an orthogonal issue.

As is building the modules as part of the kernel build, and having the
kernel build include a kernel.o stage into which module.o can be
linked so build a custom kernel with a list of builtin module.

I might look into this when I get by systems accessible again
(should be soon).

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/conf

2011-02-13 Thread David Laight
On Sun, Feb 13, 2011 at 04:37:21AM +, Jean-Yves Migeon wrote:
 Module Name:  src
 Committed By: jym
 Date: Sun Feb 13 04:37:21 UTC 2011
 
 Modified Files:
   src/sys/arch/i386/conf: GENERIC
 
 Log Message:
 Compile FFS and NFS statically (e.g. not modular) for GENERIC. These
 file-systems can be critical for mountroot; as kernel cannot have access
 to module(7)s without having / mounted first... yes, you see the point.

Does this cause grief with existing installations where the bootloader
also loads these as modules?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.bin/grep

2011-02-17 Thread David Laight
On Thu, Feb 17, 2011 at 12:06:30PM +0900, enami tsugutomo wrote:
 
 I just did `more fastgrep.c' and found following piece of code.  The
 usage of wflag is an obvious regression from OpenBSD code.
 
 | if (fg-len = 14 
 | strncmp(pat + (fg-bol ? 1 : 0), [[::]], 7) == 0 
 | strncmp(pat + (fg-bol ? 1 : 0) + fg-len - 7, [[::]], 7) == 0) {
 | fg-len -= 14;
 | /* Word boundary is handled separately in util.c */
 | wflag = true;
 | }

It looks like that might transform:
grep '[[::]]foo.*bar[[::]]'
to
grep -w 'foo.*bar'

which isn't a valid transform.
(I presume something else translated the original '\foo.*bar\' ...)

I've known grep where searches for '\word\' were a lot faster than
using -w. Presumably because -w locates the words (start and end)
rather than just the start!

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys

2011-02-19 Thread David Laight
On Sat, Feb 19, 2011 at 08:19:54PM +, Matt Thomas wrote:
 
 Log Message:
 Default PCU_UNIT_COUNT to 0.  If 0, don't compile the contents of subr_pcu.c
 and don't include the pcu related members into struct lwp.

Making a structure layout depend on an option is probably not a good idea!
At least not unless the structure ends with a series of 'optional'
sub-stuctures that are found by access functions (which may be
#defines for things built into the kernel itself).

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/amd64/conf

2011-02-20 Thread David Laight
On Sun, Feb 20, 2011 at 06:25:06PM +0200, Antti Kantee wrote:
 On Sun Feb 20 2011 at 17:15:57 +0100, Jean-Yves Migeon wrote:
  = 1.3MiB. So, a total of 1.3M + 700k = 2MiB. Still missing 1.5MiB.
  MODULAR options seems to consume ~70kiB , so I would assume that the
  rest is due to PIC mode and ELF headers... ?
 
 Dunno where the space is going, but it's certainly not PIC overhead,
 since the kernel or kernel modules are not PIC.

The kernel modules will contain relocation info.
For comparison you need to use 'size -A' (or similar) not ls.
The amd64 objects may incluse the eh_frame section, kernel stuff
should be compiled without that section.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand/boot

2011-02-27 Thread David Laight
On Sat, Feb 26, 2011 at 06:22:59PM +, Jonathan A. Kollasch wrote:
 Module Name:  src
 Committed By: jakllsch
 Date: Sat Feb 26 18:22:59 UTC 2011
 
 Modified Files:
   src/sys/arch/i386/stand/boot: Makefile.boot
 
 Log Message:
 Enable LIBSA_PRINTF_LONGLONG_SUPPORT.
 Useful in any of the cases where we already print a (64-bit) daddr_t.

I'm surprised that doesn't explode the sizes of the boot code.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/regress/sys/net

2011-02-28 Thread David Laight
On Mon, Feb 28, 2011 at 10:08:07PM +, Jonathan A. Kollasch wrote:
 
 autogen requires ed(1), which is not provided by our toolchain.
 (In other words, this causes a odd build failure on some Linux build
 hosts.)

I've not looked, but it is probably not too hard to change the script
to use a different tool.
(Possibly just a shell script)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/regress/sys/net

2011-03-03 Thread David Laight
On Thu, Mar 03, 2011 at 12:27:19AM +, David Holland wrote:
 On Tue, Mar 01, 2011 at 08:02:40AM +, David Laight wrote:
autogen requires ed(1), which is not provided by our toolchain.
(In other words, this causes a odd build failure on some Linux build
hosts.)
   
   I've not looked, but it is probably not too hard to change the script
   to use a different tool.
   (Possibly just a shell script)
 
 This should do the trick, modulo feeding in ${TOOL_AWK} properly:
 
...
 +awk  $1 '
 +state == 0  /^ether_aton_r/ { print prev; state = 1; }
 +state == 1 { print; }
 +state == 1  /^}$/ { state = 2; }
 +{ prev = $0; }
 +'  $2

Looks like that would be even simpler in sed :-)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libc/gen

2011-03-13 Thread David Laight
On Sun, Mar 13, 2011 at 07:40:45AM +, matthew green wrote:
   src/lib/libc/gen: unvis.c
 
 Log Message:
 cast ~0 to (size_t) when passing to a size_t taking function.
 fixes lint build errors.

Is that right? My C promotion rules are getting rusty...
~0 is of type 'int', 'size_t' is an unsigned type, if 'size_t' is
larger than 'int' isn't the convertion 'value preserving' rather
than 'sign preserving'?
So you need ~(size_t)0 if you want to generate the all 1's bit pattern?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/lib/libc/gen

2011-03-15 Thread David Laight
On Tue, Mar 15, 2011 at 12:26:05PM +0100, Joerg Sonnenberger wrote:
 On Tue, Mar 15, 2011 at 11:26:05AM +0100, Joerg Sonnenberger wrote:
  On Tue, Mar 15, 2011 at 03:47:04AM +, Eric Haszlakiewicz wrote:
   Module Name:  src
   Committed By: erh
   Date: Tue Mar 15 03:47:04 UTC 2011
   
   Modified Files:
 src/lib/libc/gen: Makefile.inc
   Added Files:
 src/lib/libc/gen: commaize_number.3 commaize_number.c
   
   Log Message:
   PR#7540, add a commaize_number function, which inserts comma into a string
of digits to make it more readable.  This is soon to be used in /bin/ls.
  
  Wouldn't a flag for humanize_number be a much better approach for this?
  Also, this doesn't belong into libc.
 
 There is also the point that the ' printf modifier is supposed to do
 this if the current locale has a thousand separator. As such, I would
 like to see this reverted and done properly.

IIRC scanf() and the number processing sort are also required to parse
numbers with the locale's 1000 separator.
This is all fubar when the thousand sep is ' ' or '.'.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/sandpoint/stand/altboot

2011-03-20 Thread David Laight
On Sun, Mar 20, 2011 at 02:07:05AM +, Frank Wille wrote:
 
 Log Message:
 The DSM-G600 U-Boot is so restricted that there is no possibility to pass
 any bootargs. So we will just do the default multiuser boot from wd0: when
 altboot was started together with a Linux initrd image.

Can anything be done with the 'initrd' image - either as an image file
or something in the tar archive (IIRC it is likely to be one these days)?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2011-04-12 Thread David Laight
On Mon, Apr 11, 2011 at 08:03:46PM +, Martin Husemann wrote:
 Module Name:  src
 Committed By: martin
 Date: Mon Apr 11 20:03:45 UTC 2011
 
 Modified Files:
   src/distrib/sets/lists/etc: mi
   src/etc: Makefile
   src/etc/mtree: NetBSD.dist.base special
 
 Log Message:
 When run as root, tcpdump will chroot to /var/run/tcpdump - but it can
 not look up /etc/protcols in there. So install a copy of /etc/protocols
 into the chroot area.
 Fixes PR bin/44721.

Wouldn't it be less confusing to have opened /etc/protocols before
doing the chroot ??
This might mean persuading the library code to open /dev/fd/n

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: othersrc/external/bsd/crc

2011-05-03 Thread David Laight
On Thu, Apr 28, 2011 at 02:09:00AM +, Alistair G. Crooks wrote:
 Module Name:  othersrc
 Committed By: agc
 Date: Thu Apr 28 02:09:00 UTC 2011
 
 Update of /cvsroot/othersrc/external/bsd/crc
 In directory ivanova.netbsd.org:/tmp/cvs-serv7562
 
 Log Message:
 initial import of some routines to perform CRC calculations:

I've not looked at these routines :-) but I suspect they do byte or
nibble table lookups.

For CRC16 (hdlc etc) there is a short sequence of shifts and xors
that will update the crc for a new byte.
On superscalar architectures with barrel shifters this is almost
certainly faster than the table lookup versions (even when the
table is in the cache).

IIRC it is 3 clocks slower on non-superscaler with zero wait state memory.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.bin/make

2011-05-18 Thread David Laight
On Wed, May 18, 2011 at 02:43:03AM +, David Holland wrote:
 On Tue, May 17, 2011 at 09:56:52PM +, David Laight wrote:
   cvs rdiff -u -r1.51 -r1.52 src/usr.bin/make/Makefile
 
 uh, did you mean to commit that?

No - I've changed it back.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch/i386/stand

2011-05-21 Thread David Laight
On Fri, May 20, 2011 at 10:29:56PM +, Joerg Sonnenberger wrote:
 
 Log Message:
 Disable integrated assembler for files that use .code16 or .code32 for
 now

Would it have been better to do this with a level of indirection?
So that it could be turned off by changing a global variable instead
of having to edit all of the modified makefiles?

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/tests/syscall

2011-05-21 Thread David Laight
On Sat, May 21, 2011 at 05:14:07PM +0200, Manuel Bouyer wrote:
 On Sat, May 21, 2011 at 03:34:22PM +0100, Julio Merino wrote:
  Yes, the timeout thing is broken.  It should really be a
  specification of the test case size (e.g. 'small', 'large') and
  allow the user to define his timeout preferences for every class,
  because they will vary from machine to machine.
 
 Sure. I ran ATF on a machine with a 1Mhz CPU clock (this is a
 simulator, the real hardware is expected to run faster :).
 Lots of tests timed out just because on such hardware they can't
 complete in less than 5mn.

Perhaps the timeout(s) should be settable units that default
to 1 second.
Them the timeouts can be scaled for very slow (or fast) systems.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/usr.sbin/makefs

2011-05-25 Thread David Laight
On Tue, May 24, 2011 at 11:01:42PM +0900, Izumi Tsutsui wrote:
  On Tue, May 24, 2011 at 09:52:18PM +0900, Izumi Tsutsui wrote:
   It looks mandatory to make makefs(8) handle partition maps for CD images.
  
  I'm not sure I understand exactly what you mean and why makefs is the right
  tool here. Postprocessing the image is fine 
 
 I think the problem is that the final ISO9660 output including
 embeded MD boot fs and partition maps still has to be iso9660 compliant.
 There is no public space to store MD parameters for postprocessing
 in output image, ...

Could the extra parameters be put into a file within the image?
Then the postprocessing phase would have the required data.

Or even in a separate file !

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/dev

2011-05-26 Thread David Laight
On Thu, May 26, 2011 at 07:12:57AM +, David Holland wrote:
 On Wed, May 25, 2011 at 04:33:38PM +, Masao Uebayashi wrote:
   Modified Files:
  src/sys/dev/bluetooth: bcsp.c bthub.c btuart.c
  src/sys/dev/ieee1394: fwdev.c fwmem.c fwohci.c
   
   Log Message:
   Declare cfdrivers using extern rather than including ioconf.h.
 
 This is wrong. Please revert it.
 
 The purpose of declaring things in header files is to make sure all
 uses are consistent.

Is there another header file that could contain:
#define CFDRIVER(prefix) extern struct cfdriver prefix##_cd

Then the bcsp.h could contain:
CFDRIVER(bcsp);
which gives (effectively) the same guarantee as including ioconf.h.

I think something like that would help building drivers as kernel modules.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2011-05-26 Thread David Laight
On Thu, May 26, 2011 at 11:32:51AM -0500, David Young wrote:
 On Thu, May 26, 2011 at 06:10:42PM +0200, Lars Heidieker wrote:
  On 05/26/11 06:25, Masao Uebayashi wrote:
...
  with those changes I can't compile a kernel without USERCONF option set.
  The changes in x86_machdep.c should be with in if defs on that options
  
  Ok to commit the attached patch?
 
 Conditional compilation clutters the code up.  What if instead you add
 to kern_stub.c,
 
 #include sys/userconf.h
 __weak_alias(userconf_bootinfo, voidop);

I'd either:
leave userconf_bootinfo() defined but with no body
or:
#define userconf_bootinfo()
(or maybe both!)

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src

2011-05-29 Thread David Laight
On Sat, May 28, 2011 at 11:42:37PM +0100, Julio Merino wrote:
 On 5/28/11 10:46 PM, Christos Zoulas wrote:
 In article20110528161256.ab89817...@cvs.netbsd.org,
 Matthias Schelersource-changes-d@NetBSD.org  wrote:
 [...]
 +   assert(close(fd) == 0);
 
 Please don't create assertions that contain code, because compiled with
 -DNDEBUG they vanish.
 
 Are these test code?  If so, just replace assert by one of:
 
 - ATF_REQUIRE(boolean_expression)
 - ATF_REQUIRE_EQ(expected_value, actual_value)
 
 like:
 
 ATF_REQUIRE(pipe(fds) != 0);
 ATF_REQUIRE_EQ(1, write(fds[1], , 1));

Actually, the assert() define (and similar) are often less than useful
since they don't give any indication of why things are wrong.
The ATF_REQUIRE_EQ() might be more useful (I've not looked at its definition),
but it ought to be possible to use something like:

#define CHECK(type, fmt, v1, op, v2) \
do { \
type _v1 = (v1), _v2 = (v2); \
if (!((_v1) op (_v2))) \
check_error(#v1, #op, #v2, __LINE__, fmt, _v1, _v2); \
} while (0);

#define CHECK_I(v1, op, v1) CHECK(int, %d, v1, op, v2)

So check_error() can print the both values of the erronous test.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2011-06-02 Thread David Laight
On Thu, Jun 02, 2011 at 06:54:44PM +, David Laight wrote:
 Module Name:  src
 Committed By: dsl
 Date: Thu Jun  2 18:54:44 UTC 2011
 
 Modified Files:
   src/sys/kern: vfs_syscalls.c
 
 Log Message:
 Fix type in comment
 (before I replace the 'l' with 'curlwp')

Actually I've thought about this some more.
While, on the face of it, removing the 'struct lwp *' parameter to all system
calls might seem an optimisation, I suspect that, especially on systems where
arguments are passed in registers, it is a pessimisation.

Passing 'l' is a register rename (or copy) so is almost zero cost.

Recovering curlwp may involve a function call, and is, at best, a real
memory access of global data (possibly via an asm statement) that will
be slow and multiple accesses might need caching in a local anyway.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2011-06-04 Thread David Laight
On Sat, Jun 04, 2011 at 10:52:47AM +1000, Simon Burge wrote:
 
  On Thu, Jun 02, 2011 at 09:21:11PM +0100, David Laight wrote:
   Passing 'l' is a register rename (or copy) so is almost zero cost.
   
   Recovering curlwp may involve a function call, and is, at best, a real
   memory access of global data (possibly via an asm statement) that will
   be slow and multiple accesses might need caching in a local anyway.
...
 As far as I could tell, the change to put curlwp in a register was never
 actually benchmarked on MIPS.  I asked a few times and never got an
 answer, other than that a kernel was about 2.5kB smaller.

Removing 2.5k is probably unlikely to make it slower :-)
Was that done by telling gcc that the register always help that variable?
or by stopping gcc using it, and useing a (non-volatile?) asm statement
to get the value.

 I'd be rather curious that if other arches investigate this, especially
 if there's some performance data to back up the change this time around.

Hmmm... I wonder if I can make it a kernel compile-time option so that
different archs can do it differently, and the whole thing can be switched
back for performance measurements.

The following defines would do it (with better names):
#ifdef SYS_CALL_USE_CURLWP
#define SYSCALL_FN(fn, l, uap, r) fn(uap, r)
#define SYSCALL_CURLWP curlwp
#else
#define SYSCALL_FN(fn, l, uap, r) fn(l, uap, r)
#define SYSCALL_CURLWP l
#endif

For non-smp builds (on old archs) 'curlwp' is a simple memory access, and the 
access
to the 'l' argument is likely to be a stack access - so probably not much 
difference.

Do we support smp vax or m68k ? (or even arm!)

IIRC the arm ABI defines standard uses for a lot of the registers, one must be 
for
thread data - so could be used in the kernel for curlwp.

Thinking about SMP, both CURCPU() and curlwp could be the register - with the
other being indirected from it.  I wonder which it used most?
curproc should be readable from either.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/kern

2011-06-05 Thread David Laight
On Thu, Jun 02, 2011 at 09:21:11PM +0100, David Laight wrote:
 On Thu, Jun 02, 2011 at 06:54:44PM +, David Laight wrote:
  Module Name:src
  Committed By:   dsl
  Date:   Thu Jun  2 18:54:44 UTC 2011
  
  Modified Files:
  src/sys/kern: vfs_syscalls.c
  
  Log Message:
  Fix type in comment
  (before I replace the 'l' with 'curlwp')
 
 Actually I've thought about this some more.
 While, on the face of it, removing the 'struct lwp *' parameter to all system
 calls might seem an optimisation, I suspect that, especially on systems where
 arguments are passed in registers, it is a pessimisation.
 
 Passing 'l' is a register rename (or copy) so is almost zero cost.
 
 Recovering curlwp may involve a function call, and is, at best, a real
 memory access of global data (possibly via an asm statement) that will
 be slow and multiple accesses might need caching in a local anyway.

Well, an amd64 GENERIC kernel grows by 1.5k, and I've given up trying to get
an i386 MONOLITHIC to compile - too much crap in compat.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/sys/arch

2011-07-18 Thread David Laight
On Mon, Jul 18, 2011 at 02:18:54AM +0200, Jean-Yves Migeon wrote:
 On 18.07.2011 02:00, David Young wrote:
  Can we please use ansi function definitions in newly committed code?
  
  This was tedious enough without converting to ANSI function definitions.
  A good job for Coccinelle (spatch)?
 
 Sadly, no: last time I tried (when moving kvm(3) code to ANSI style), I
 had to do it manually. It's even the opposite, spatch has issues when
 parsing non-ANSI declarations, so you have to do the conversion all by
 yourself first...

I've a sed script that will change most of them
see ftp.netbsd.org:~dsl/protoz

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/crypto/external/bsd/openssl/lib/libcrypto/arch

2011-07-25 Thread David Laight
On Mon, Jul 25, 2011 at 11:52:52AM +0200, Joerg Sonnenberger wrote:
 Much better. One thing remains. It would be nice to replace
   .byte 0xf3,0xc3
 with either a simple ret or a ret $0, depending on whether it has a
 label on it or not. The reason for this mess seems to be a bug in
 certain generation of AMD CPUs. So essentially,

IIRC it is something to do with branch prediction?
But my memory keeps thinking of a constraint about the number
of branches/labels in a cache line - and I'm sure the non-use of
1 byte return instructions was all related.

David

-- 
David Laight: da...@l8s.co.uk


Re: CVS commit: src/crypto/external/bsd/openssl/lib/libcrypto/arch

2011-07-25 Thread David Laight
On Mon, Jul 25, 2011 at 08:38:13PM +0200, Joerg Sonnenberger wrote:
 On Mon, Jul 25, 2011 at 07:24:57PM +0100, David Laight wrote:
  On Mon, Jul 25, 2011 at 11:52:52AM +0200, Joerg Sonnenberger wrote:
   Much better. One thing remains. It would be nice to replace
 .byte 0xf3,0xc3
   with either a simple ret or a ret $0, depending on whether it has a
   label on it or not. The reason for this mess seems to be a bug in
   certain generation of AMD CPUs. So essentially,
  
  IIRC it is something to do with branch prediction?
  But my memory keeps thinking of a constraint about the number
  of branches/labels in a cache line - and I'm sure the non-use of
  1 byte return instructions was all related.
 
 When I asked around, I get the following reference, which seems to
 summarize the situation nicely:
 
 http://mikedimmick.blogspot.com/2008/03/what-heck-does-ret-mean.html

That is sort of consistent with what I remember from those guides.
I wonder what the additional cost of 'rep ret' and 'ret $0' is
on other cpus (apart from the obvious extra code byte).

Looking at the code (now with fewer 'rep ret') I notice that a fair
number of the jumps are unconditional - why have an unconditional jump
to a return instruction!
I also haven't checked what the critical paths are, and what the static
predicton will do! I also don't know the cycle times of these special
instructions to know how much it really matters!

David

-- 
David Laight: da...@l8s.co.uk


  1   2   3   >