Re: CVS commit: src/bin/dd

2010-11-23 Thread Antti Kantee
On Tue Nov 23 2010 at 12:28:48 +, Matthias Scheler wrote:
 On Mon, Nov 22, 2010 at 09:04:28PM +, Antti Kantee wrote:
  Module Name:src
  Committed By:   pooka
  Date:   Mon Nov 22 21:04:28 UTC 2010
  
  Modified Files:
  src/bin/dd: Makefile args.c dd.c dd.h extern.h position.c
  
  Log Message:
  Add two new operands: rif and rof.  They operate exactly like
  if and of with the exception that the communicate with a rump
  kernel instead of the host kernel.
 
 This makes dd unusable if only / but not /usr is mounted because
 it now depends on /usr/lib/librumpclient.so.0.
 
 Utilities in /bin and /sbin are supposed to work without
 /usr mounted.

Good catch.  Fixed.  Sorry 'bout that oversight.

(on a slight tangent, wasn't getting rid of /usr vs. / on the roadmap?)


Re: CVS commit: src/bin/dd

2010-11-23 Thread Matthias Scheler
On Tue, Nov 23, 2010 at 02:42:04PM +0200, Antti Kantee wrote:
 (on a slight tangent, wasn't getting rid of /usr vs. / on the roadmap?)

I don't know that. I personally don't create /usr partitions on my
NetBSD systems anymore. But they usually have hard disks with multiple
gigabytes of storage space.

/usr over NFS is useful for a system with a shortage of local disk space.

Kind regards

-- 
Matthias Scheler  http://zhadum.org.uk/


Re: CVS commit: src/bin/dd

2010-11-23 Thread Bernd Ernesti
On Tue, Nov 23, 2010 at 02:42:04PM +0200, Antti Kantee wrote:
 On Tue Nov 23 2010 at 12:28:48 +, Matthias Scheler wrote:
  On Mon, Nov 22, 2010 at 09:04:28PM +, Antti Kantee wrote:
   Module Name:  src
   Committed By: pooka
   Date: Mon Nov 22 21:04:28 UTC 2010
   
   Modified Files:
 src/bin/dd: Makefile args.c dd.c dd.h extern.h position.c
   
   Log Message:
   Add two new operands: rif and rof.  They operate exactly like
   if and of with the exception that the communicate with a rump
   kernel instead of the host kernel.
  
  This makes dd unusable if only / but not /usr is mounted because
  it now depends on /usr/lib/librumpclient.so.0.
  
  Utilities in /bin and /sbin are supposed to work without
  /usr mounted.
 
 Good catch.  Fixed.  Sorry 'bout that oversight.

That doesn't resolve the concerns why this non standard options were added
to dd.

Can we go back to the previous versions for the moment and discuss it and
then do further changes?

Bernd



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/bin/dd

2010-11-23 Thread Antti Kantee
On Tue Nov 23 2010 at 18:41:21 +, David Laight wrote:
 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?

The client may be on a different machine, so you'd have to, if not invent
a completely new protocol, at least include server side functionality
on the client and add some weird pingpong/adjust logic into the server.

 Making something work well enough to copy files is probably not that hard!

Adding a ton of special case code to copy files is extremely poor design!
Doubly so since the only reason is because you don't happen to like
some flags which bring 0 cost when not used.  If you don't like them,
don't use them.

Unless someone can actually name a *problem*, move along.


Re: CVS commit: src/bin/dd

2010-11-23 Thread David Holland
On Tue, Nov 23, 2010 at 02:42:04PM +0200, Antti Kantee wrote:
  (on a slight tangent, wasn't getting rid of /usr vs. / on the roadmap?)

No.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/kern

2010-11-23 Thread Mindaugas Rasiukevicius
Hello,

Sorry for late reply.

y...@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
  
  how about the attached patch?
  
  Consider offset = (INT64_MAX - PAGE_SIZE) and len = PAGE_SIZE.  That
  would still panic..
  
  heh, right.
  
  then, how about:
  if (round_page(offset)  trunc_page(endoffset)) {
 
offset  round_page(offset)

I think that should be correct, except there is off-by-one (since offset
at PAGE_SIZE boundary is valid).  Should be:

...  offset = round_page(offset)

Do you want to commit this?

-- 
Mindaugas


Re: CVS commit: src/bin/dd

2010-11-23 Thread David Holland
On Tue, Nov 23, 2010 at 08:50:54PM +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?
  
  The client may be on a different machine, so you'd have to, if not invent
  a completely new protocol, at least include server side functionality
  on the client and add some weird pingpong/adjust logic into the server.

But you must already have this protocol in order to be able to reach
the client, no? But if not, we already have nfs, there's no need to
reinvent it.

   Making something work well enough to copy files is probably not that hard!
  
  Adding a ton of special case code to copy files is extremely poor design!

Adding weird special case remote access hacks to dd (of all random
tools) is also poor design. Why not for the next round add support for

   dd ifurl=http://www.netbsd.org/index.html of=mycopy

or

   dd ifurl=http://www.netbsd.org/ recurse=true of=mytree/

After all, all you have to do to implement this is open a pipe to ftp
or wget.

  Doubly so since the only reason is because you don't happen to like
  some flags which bring 0 cost when not used.  If you don't like them,
  don't use them.

Poorly thought-out features cause a lot of long-term overhead.

  Unless someone can actually name a *problem*, move along.

I'd say the first problem is that this wasn't discussed anywhere.

-- 
David A. Holland
dholl...@netbsd.org


Re: CVS commit: src/sys/dev

2010-11-23 Thread Mindaugas Rasiukevicius
Hello,

Juergen Hannken-Illjes hann...@netbsd.org wrote:
 Module Name:  src
 Committed By: hannken
 Date: Tue Nov 23 09:30:43 UTC 2010
 
 Modified Files:
   src/sys/dev: md.c
 
 Log Message:
 Make md(4) mp-safe.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.64 -r1.65 src/sys/dev/md.c

Few comments:

 @@ -597,15 +626,18 @@ md_server_loop(struct md_softc *sc)
   int error;
   bool is_read;
  
 + KASSERT(mutex_owned(sc-sc_lock));
 +
   for (;;) {
   /* Wait for some work to arrive. */
   while ((bp = bufq_get(sc-sc_buflist)) == NULL) {
 - error = tsleep((void *)sc, md_sleep_pri, md_idle, 0);
 + error = cv_wait_sig(sc-sc_cv, sc-sc_lock);
 ...
   biodone(bp);
 + mutex_enter(sc-sc_lock);
   }

Is this (as well as other parts of code) are safe in respect of mdclose()?
For example, what happens if other thread executes close(2) while the lock
is dropped here?

 @@ -383,7 +396,8 @@ mdstrategy(struct buf *bp)
   case MD_UMEM_SERVER:
   /* Just add this job to the server's queue. */
   bufq_put(sc-sc_buflist, bp);
 - wakeup((void *)sc);
 + cv_signal(sc-sc_cv);
 + mutex_exit(sc-sc_lock);

It should be cv_broadcast(9).

 @@ -421,6 +435,8 @@ mdstrategy(struct buf *bp)
   }
   done:
   biodone(bp);
 +
 + mutex_exit(sc-sc_lock);

Any reason why biodone() is under lock?

 @@ -534,6 +561,8 @@ md_ioctl_kalloc(struct md_softc *sc, str
   vaddr_t addr;
   vsize_t size;
  
 + KASSERT(mutex_owned(sc-sc_lock));

Ideally, allocations should be outside the locks (just FYI).

 + kmutex_t sc_lock;   /* Protect self. */
 + kcondvar_t sc_cv;   /* Signal work. */

I think Signal work is missleading. :)

-- 
Mindaugas