Re: CVS commit: src/sys

2011-12-28 Thread YAMAMOTO Takashi
hi,

 On Dec 21,  5:07am, y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
 -- Subject: Re: CVS commit: src/sys
 
 | why?
 | i thought the reason of having this as a separate member was performance.
 
 All the code paths that use NBIO check bits in so_state... I would like to
 see the benchmark that proves this.

it's about the fcntl side, for which you added more locking.

YAMAMOTO Takashi

 
 christos


Re: CVS commit: src/sys

2011-12-28 Thread Christos Zoulas
In article 20111228124228.c4d8614a...@mail.netbsd.org,
YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote:
hi,

 On Dec 21,  5:07am, y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
 -- Subject: Re: CVS commit: src/sys
 
 | why?
 | i thought the reason of having this as a separate member was performance.
 
 All the code paths that use NBIO check bits in so_state... I would like to
 see the benchmark that proves this.

it's about the fcntl side, for which you added more locking.


For FIONBIO... Even if you construct a synthetic benchmark that
turns on and off blocking I/O for a socket continuously, and the
locking overhead proves to be larger than the system call overhead
(which it may very well be), I doubt that you'll find any real
program that calls ioctl(so, FIONBIO, ...) frequently enough to
make a difference.

I really think that Andy confused it with FIONREAD (hence the comment),
which can be called quite frequently (and it is still unlocked).

christos



Re: CVS commit: src/lib/libperfuse

2011-12-28 Thread Alistair Crooks
On Thu, Dec 29, 2011 at 01:40:33AM +, Jeff Rizzo wrote:
 Module Name:  src
 Committed By: riz
 Date: Thu Dec 29 01:40:32 UTC 2011
 
 Modified Files:
   src/lib/libperfuse: debug.c
 
 Log Message:
 Cast time_t to intmax_t for printf purposes, and format with %j.  Fixes
 build on amd64 and probably i386 as well.

Thanks for fixing this one, Jeff.

Any reason we're not using the PRI* definitions in inttypes.h?

Best,
Alistair


Re: CVS commit: src/lib/libperfuse

2011-12-28 Thread Jeff Rizzo

On 12/28/11 5:57 PM, Alistair Crooks wrote:

On Thu, Dec 29, 2011 at 01:40:33AM +, Jeff Rizzo wrote:

Module Name:src
Committed By:   riz
Date:   Thu Dec 29 01:40:32 UTC 2011

Modified Files:
src/lib/libperfuse: debug.c

Log Message:
Cast time_t to intmax_t for printf purposes, and format with %j.  Fixes
build on amd64 and probably i386 as well.

Thanks for fixing this one, Jeff.

Any reason we're not using the PRI* definitions ininttypes.h?




They didn't seem to add anything in this case, and muddled readability;  
am I missing something?


+j



re: CVS commit: src/lib/libperfuse

2011-12-28 Thread matthew green

 On 12/28/11 5:57 PM, Alistair Crooks wrote:
  On Thu, Dec 29, 2011 at 01:40:33AM +, Jeff Rizzo wrote:
  Module Name:   src
  Committed By:  riz
  Date:  Thu Dec 29 01:40:32 UTC 2011
 
  Modified Files:
 src/lib/libperfuse: debug.c
 
  Log Message:
  Cast time_t to intmax_t for printf purposes, and format with %j.  Fixes
  build on amd64 and probably i386 as well.
  Thanks for fixing this one, Jeff.
 
  Any reason we're not using the PRI* definitions ininttypes.h?
 
 They didn't seem to add anything in this case, and muddled readability;  
 am I missing something?

two things come to mind about this change:

- time_t can be a floating point type

- there's no PRI* for time_t

in general using PRI* and not casting is always a better solution.


.mrg.


Re: CVS commit: src/lib/libperfuse

2011-12-28 Thread Christos Zoulas



In article 20111229014033.2f33917...@cvs.netbsd.org you write:
-=-=-=-=-=-

Module Name:   src
Committed By:  riz
Date:  Thu Dec 29 01:40:32 UTC 2011

Modified Files:
   src/lib/libperfuse: debug.c

Log Message:
Cast time_t to intmax_t for printf purposes, and format with %j.  Fixes
build on amd64 and probably i386 as well.
 
-  fprintf(fp, %lu.%09ld %s %s%s%s %s ,  
-  pt-pt_start.tv_sec, pt-pt_start.tv_nsec,
+  fprintf(fp, %ju.%09jd %s %s%s%s %s ,  
+  (intmax_t)pt-pt_start.tv_sec,
+  (intmax_t)pt-pt_start.tv_nsec,

This is slightly wrong: intmax_t is a signed type so %jd; tv_nsec is guaranteed
to be long so %09ld without a cast works (what you have there is ok too,
but slightly less efficient on 32 bit systems).

christos




Re: CVS commit: src/lib/libperfuse

2011-12-28 Thread Warner Losh

On Dec 28, 2011, at 8:40 PM, matthew green wrote:
   - time_t can be a floating point type

For my information, have there ever been any systems that represent time_t as a 
float?

Warner