Re: ffs2 boot

2014-07-22 Thread Kent R. Spillner
Sorry, let me split this into smaller diffs to ease review.

First up, the diff below removes a redundant cast: buf is declared a
char * so there's no need to cast it to a char *.  I noticed the same
issue in ufs.c, too, so fix it in both places.

Index: ufs.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs.c,v
retrieving revision 1.22
diff -p -u -r1.22 ufs.c
--- ufs.c   30 May 2013 19:19:09 -  1.22
+++ ufs.c   21 Jul 2014 22:30:32 -
@@ -502,7 +502,7 @@ ufs_open(char *path, struct open_file *f
if (rc)
goto out;
 
-   bcopy((char *)buf, namebuf, (unsigned)link_len);
+   bcopy(buf, namebuf, (unsigned)link_len);
}
 
/*
Index: ufs2.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
retrieving revision 1.2
diff -p -u -r1.2 ufs2.c
--- ufs2.c  29 Apr 2014 07:52:06 -  1.2
+++ ufs2.c  21 Jul 2014 22:30:48 -
@@ -498,7 +498,7 @@ ufs2_open(char *path, struct open_file *
if (rc)
goto out;
 
-   bcopy((char *)buf, namebuf, (unsigned)link_len);
+   bcopy(buf, namebuf, (unsigned)link_len);
}
 
/*



On Wed, Jul 16, 2014 at 05:55:55PM -0500, Kent R. Spillner wrote:
 *Bump*
 
  On Jul 10, 2014, at 12:33, Kent R. Spillner kspill...@acm.org wrote:
  
  Ping.
  
  On Thu, May 01, 2014 at 01:22:56PM -0500, Kent R. Spillner wrote:
  After sending my previous reply I noticed that you already committed
  your diff, so here are my comments again in the form of a proper diff:
  
  * Use NULL instead of casting 0 to pointer types
  
  * Remove unnecessary (char *) cast on buf because buf was already
   declared as char *
  
  * Simplify if ((rc = ...) != 0) idiom to equivalent if ((rc = ...))
  
  
  Index: sys/lib/libsa/ufs2.c
  ===
  RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
  retrieving revision 1.2
  diff -p -u -r1.2 ufs2.c
  --- sys/lib/libsa/ufs2.c29 Apr 2014 07:52:06 -1.2
  +++ sys/lib/libsa/ufs2.c1 May 2014 16:54:25 -
  @@ -217,7 +217,7 @@ block_map(struct open_file *f, daddr_t f
 }
  
 if (fp-f_blkno[level] != ind_block_num) {
  -if (fp-f_blk[level] == (char *)0)
  +if (fp-f_blk[level] == NULL)
 fp-f_blk[level] =
 alloc(fs-fs_bsize);
 twiddle();
  @@ -269,7 +269,7 @@ buf_read_file(struct open_file *f, char 
 if (rc)
 return (rc);
  
  -if (fp-f_buf == (char *)0)
  +if (fp-f_buf == NULL)
 fp-f_buf = alloc(fs-fs_bsize);
  
 if (disk_block == 0) {
  @@ -401,7 +401,7 @@ ufs2_open(char *path, struct open_file *
 }
  
 inumber = ROOTINO;
  -if ((rc = read_inode(inumber, f)) != 0)
  +if ((rc = read_inode(inumber, f)))
 goto out;
  
 cp = path;
  @@ -454,7 +454,7 @@ ufs2_open(char *path, struct open_file *
 /*
  * Open next component.
  */
  -if ((rc = read_inode(inumber, f)) != 0)
  +if ((rc = read_inode(inumber, f)))
 goto out;
  
 /*
  @@ -498,7 +498,7 @@ ufs2_open(char *path, struct open_file *
 if (rc)
 goto out;
  
  -bcopy((char *)buf, namebuf, (unsigned)link_len);
  +bcopy(buf, namebuf, (unsigned)link_len);
 }
  
 /*
  @@ -511,7 +511,7 @@ ufs2_open(char *path, struct open_file *
 else
 inumber = ROOTINO;
  
  -if ((rc = read_inode(inumber, f)) != 0)
  +if ((rc = read_inode(inumber, f)))
 goto out;
 }
 }
  @@ -534,8 +534,8 @@ ufs2_close(struct open_file *f)
  {
 struct file *fp = (struct file *)f-f_fsdata;
  
  -f-f_fsdata = (void *)0;
  -if (fp == (struct file *)0)
  +f-f_fsdata = NULL;
  +if (fp == NULL)
 return (0);
  
 return (ufs2_close_internal(fp));
  @@ -656,7 +656,7 @@ ufs2_readdir(struct open_file *f, char *
 }
  
 do {
  -if ((rc = buf_read_file(f, buf, buf_size)) != 0)
  +if ((rc = buf_read_file(f, buf, buf_size)))
 return rc;
  
 dp = (struct direct *)buf;
  
 



Re: ffs2 boot

2014-07-22 Thread Kent R. Spillner
Next, use NULL instead of casting 0 to pointer types.

Index: sys/lib/libsa/ufs.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs.c,v
retrieving revision 1.22
diff -p -u -r1.22 ufs.c
--- sys/lib/libsa/ufs.c 30 May 2013 19:19:09 -  1.22
+++ sys/lib/libsa/ufs.c 22 Jul 2014 17:43:12 -
@@ -221,7 +221,7 @@ block_map(struct open_file *f, daddr32_t
}
 
if (fp-f_blkno[level] != ind_block_num) {
-   if (fp-f_blk[level] == (char *)0)
+   if (fp-f_blk[level] == NULL)
fp-f_blk[level] =
alloc(fs-fs_bsize);
twiddle();
@@ -273,7 +273,7 @@ buf_read_file(struct open_file *f, char 
if (rc)
return (rc);
 
-   if (fp-f_buf == (char *)0)
+   if (fp-f_buf == NULL)
fp-f_buf = alloc(fs-fs_bsize);
 
if (disk_block == 0) {
@@ -538,8 +538,8 @@ ufs_close(struct open_file *f)
 {
struct file *fp = (struct file *)f-f_fsdata;
 
-   f-f_fsdata = (void *)0;
-   if (fp == (struct file *)0)
+   f-f_fsdata = NULL;
+   if (fp == NULL)
return (0);
 
return (ufs_close_internal(fp));
Index: sys/lib/libsa/ufs2.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
retrieving revision 1.2
diff -p -u -r1.2 ufs2.c
--- sys/lib/libsa/ufs2.c29 Apr 2014 07:52:06 -  1.2
+++ sys/lib/libsa/ufs2.c22 Jul 2014 17:45:58 -
@@ -217,7 +217,7 @@ block_map(struct open_file *f, daddr_t f
}
 
if (fp-f_blkno[level] != ind_block_num) {
-   if (fp-f_blk[level] == (char *)0)
+   if (fp-f_blk[level] == NULL)
fp-f_blk[level] =
alloc(fs-fs_bsize);
twiddle();
@@ -269,7 +269,7 @@ buf_read_file(struct open_file *f, char 
if (rc)
return (rc);
 
-   if (fp-f_buf == (char *)0)
+   if (fp-f_buf == NULL)
fp-f_buf = alloc(fs-fs_bsize);
 
if (disk_block == 0) {
@@ -534,8 +534,8 @@ ufs2_close(struct open_file *f)
 {
struct file *fp = (struct file *)f-f_fsdata;
 
-   f-f_fsdata = (void *)0;
-   if (fp == (struct file *)0)
+   f-f_fsdata = NULL;
+   if (fp == NULL)
return (0);
 
return (ufs2_close_internal(fp));


On Tue, Jul 22, 2014 at 09:54:03AM -0500, Kent R. Spillner wrote:
 Sorry, let me split this into smaller diffs to ease review.
 
 First up, the diff below removes a redundant cast: buf is declared a
 char * so there's no need to cast it to a char *.  I noticed the same
 issue in ufs.c, too, so fix it in both places.
 
 Index: ufs.c
 ===
 RCS file: /work/cvsroot/src/sys/lib/libsa/ufs.c,v
 retrieving revision 1.22
 diff -p -u -r1.22 ufs.c
 --- ufs.c 30 May 2013 19:19:09 -  1.22
 +++ ufs.c 21 Jul 2014 22:30:32 -
 @@ -502,7 +502,7 @@ ufs_open(char *path, struct open_file *f
   if (rc)
   goto out;
  
 - bcopy((char *)buf, namebuf, (unsigned)link_len);
 + bcopy(buf, namebuf, (unsigned)link_len);
   }
  
   /*
 Index: ufs2.c
 ===
 RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
 retrieving revision 1.2
 diff -p -u -r1.2 ufs2.c
 --- ufs2.c29 Apr 2014 07:52:06 -  1.2
 +++ ufs2.c21 Jul 2014 22:30:48 -
 @@ -498,7 +498,7 @@ ufs2_open(char *path, struct open_file *
   if (rc)
   goto out;
  
 - bcopy((char *)buf, namebuf, (unsigned)link_len);
 + bcopy(buf, namebuf, (unsigned)link_len);
   }
  
   /*
 
 
 
 On Wed, Jul 16, 2014 at 05:55:55PM -0500, Kent R. Spillner wrote:
  *Bump*
  
   On Jul 10, 2014, at 12:33, Kent R. Spillner kspill...@acm.org wrote:
   
   Ping.
   
   On Thu, May 01, 2014 at 01:22:56PM -0500, Kent R. Spillner wrote:
   After sending my previous reply I noticed that you already committed
   your diff, so here are my comments again in the form of a proper diff:
   
   * Use NULL instead of casting 0 to pointer types
   
   * Remove unnecessary (char *) cast on buf because buf was already
declared as char *
   
   * Simplify if ((rc = ...) != 0) idiom to equivalent if ((rc = ...))
   
   
   Index: sys/lib/libsa/ufs2.c
   ===
   RCS 

Re: ffs2 boot

2014-07-22 Thread Kent R. Spillner
Simplify if-conditions by removing explicit != 0.

Index: sys/lib/libsa/ufs.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs.c,v
retrieving revision 1.24
diff -p -u -r1.24 ufs.c
--- sys/lib/libsa/ufs.c 22 Jul 2014 18:03:03 -  1.24
+++ sys/lib/libsa/ufs.c 22 Jul 2014 19:12:22 -
@@ -405,7 +405,7 @@ ufs_open(char *path, struct open_file *f
}
 
inumber = ROOTINO;
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
 
cp = path;
@@ -458,7 +458,7 @@ ufs_open(char *path, struct open_file *f
/*
 * Open next component.
 */
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
 
/*
@@ -515,7 +515,7 @@ ufs_open(char *path, struct open_file *f
else
inumber = ROOTINO;
 
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
}
}
@@ -660,7 +660,7 @@ ufs_readdir(struct open_file *f, char *n
}
 
do {
-   if ((rc = buf_read_file(f, buf, buf_size)) != 0)
+   if ((rc = buf_read_file(f, buf, buf_size)))
return rc;
 
dp = (struct direct *)buf;
Index: sys/lib/libsa/ufs2.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
retrieving revision 1.4
diff -p -u -r1.4 ufs2.c
--- sys/lib/libsa/ufs2.c22 Jul 2014 18:03:03 -  1.4
+++ sys/lib/libsa/ufs2.c22 Jul 2014 19:12:45 -
@@ -401,7 +401,7 @@ ufs2_open(char *path, struct open_file *
}
 
inumber = ROOTINO;
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
 
cp = path;
@@ -454,7 +454,7 @@ ufs2_open(char *path, struct open_file *
/*
 * Open next component.
 */
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
 
/*
@@ -511,7 +511,7 @@ ufs2_open(char *path, struct open_file *
else
inumber = ROOTINO;
 
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
}
}
@@ -656,7 +656,7 @@ ufs2_readdir(struct open_file *f, char *
}
 
do {
-   if ((rc = buf_read_file(f, buf, buf_size)) != 0)
+   if ((rc = buf_read_file(f, buf, buf_size)))
return rc;
 
dp = (struct direct *)buf;


On Tue, Jul 22, 2014 at 01:01:05PM -0500, Kent R. Spillner wrote:
 Next, use NULL instead of casting 0 to pointer types.
 
 Index: sys/lib/libsa/ufs.c
 ===
 RCS file: /work/cvsroot/src/sys/lib/libsa/ufs.c,v
 retrieving revision 1.22
 diff -p -u -r1.22 ufs.c
 --- sys/lib/libsa/ufs.c   30 May 2013 19:19:09 -  1.22
 +++ sys/lib/libsa/ufs.c   22 Jul 2014 17:43:12 -
 @@ -221,7 +221,7 @@ block_map(struct open_file *f, daddr32_t
   }
  
   if (fp-f_blkno[level] != ind_block_num) {
 - if (fp-f_blk[level] == (char *)0)
 + if (fp-f_blk[level] == NULL)
   fp-f_blk[level] =
   alloc(fs-fs_bsize);
   twiddle();
 @@ -273,7 +273,7 @@ buf_read_file(struct open_file *f, char 
   if (rc)
   return (rc);
  
 - if (fp-f_buf == (char *)0)
 + if (fp-f_buf == NULL)
   fp-f_buf = alloc(fs-fs_bsize);
  
   if (disk_block == 0) {
 @@ -538,8 +538,8 @@ ufs_close(struct open_file *f)
  {
   struct file *fp = (struct file *)f-f_fsdata;
  
 - f-f_fsdata = (void *)0;
 - if (fp == (struct file *)0)
 + f-f_fsdata = NULL;
 + if (fp == NULL)
   return (0);
  
   return (ufs_close_internal(fp));
 Index: sys/lib/libsa/ufs2.c
 ===
 RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
 retrieving revision 1.2
 diff -p -u -r1.2 ufs2.c
 --- sys/lib/libsa/ufs2.c  29 Apr 2014 07:52:06 -  1.2
 +++ sys/lib/libsa/ufs2.c  22 Jul 2014 17:45:58 -
 @@ -217,7 +217,7 @@ block_map(struct open_file *f, daddr_t f
   }
  
   if (fp-f_blkno[level] != 

Re: ffs2 boot

2014-07-22 Thread Kent R. Spillner
di_size is u_int64_t in both struct ufs1_dinode and ufs2_dinode, and
strlen returns size_t.  Adjust link_len and len declarations to match
and drop now-redundant unsigned cost from bcopy.

Index: sys/lib/libsa/ufs.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs.c,v
retrieving revision 1.24
diff -p -u -r1.24 ufs.c
--- sys/lib/libsa/ufs.c 22 Jul 2014 18:03:03 -  1.24
+++ sys/lib/libsa/ufs.c 22 Jul 2014 19:22:47 -
@@ -465,8 +465,8 @@ ufs_open(char *path, struct open_file *f
 * Check for symbolic link.
 */
if ((fp-f_di.di_mode  IFMT) == IFLNK) {
-   int link_len = fp-f_di.di_size;
-   int len;
+   u_int64_t link_len = fp-f_di.di_size;
+   size_t len;
 
len = strlen(cp);
 
@@ -479,8 +479,7 @@ ufs_open(char *path, struct open_file *f
bcopy(cp, namebuf[link_len], len + 1);
 
if (link_len  fs-fs_maxsymlinklen) {
-   bcopy(fp-f_di.di_shortlink, namebuf,
-   (unsigned) link_len);
+   bcopy(fp-f_di.di_shortlink, namebuf, link_len);
} else {
/*
 * Read file for symbolic link
@@ -502,7 +501,7 @@ ufs_open(char *path, struct open_file *f
if (rc)
goto out;
 
-   bcopy(buf, namebuf, (unsigned)link_len);
+   bcopy(buf, namebuf, link_len);
}
 
/*
Index: sys/lib/libsa/ufs2.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
retrieving revision 1.4
diff -p -u -r1.4 ufs2.c
--- sys/lib/libsa/ufs2.c22 Jul 2014 18:03:03 -  1.4
+++ sys/lib/libsa/ufs2.c22 Jul 2014 19:24:21 -
@@ -461,8 +461,8 @@ ufs2_open(char *path, struct open_file *
 * Check for symbolic link.
 */
if ((fp-f_di.di_mode  IFMT) == IFLNK) {
-   int link_len = fp-f_di.di_size;
-   int len;
+   u_int64_t link_len = fp-f_di.di_size;
+   size_t len;
 
len = strlen(cp);
 
@@ -475,8 +475,7 @@ ufs2_open(char *path, struct open_file *
bcopy(cp, namebuf[link_len], len + 1);
 
if (link_len  fs-fs_maxsymlinklen) {
-   bcopy(fp-f_di.di_shortlink, namebuf,
-   (unsigned) link_len);
+   bcopy(fp-f_di.di_shortlink, namebuf, link_len);
} else {
/*
 * Read file for symbolic link
@@ -498,7 +497,7 @@ ufs2_open(char *path, struct open_file *
if (rc)
goto out;
 
-   bcopy(buf, namebuf, (unsigned)link_len);
+   bcopy(buf, namebuf, link_len);
}
 
/*


On Tue, Jul 22, 2014 at 02:16:26PM -0500, Kent R. Spillner wrote:
 Simplify if-conditions by removing explicit != 0.
 
 Index: sys/lib/libsa/ufs.c
 ===
 RCS file: /work/cvsroot/src/sys/lib/libsa/ufs.c,v
 retrieving revision 1.24
 diff -p -u -r1.24 ufs.c
 --- sys/lib/libsa/ufs.c   22 Jul 2014 18:03:03 -  1.24
 +++ sys/lib/libsa/ufs.c   22 Jul 2014 19:12:22 -
 @@ -405,7 +405,7 @@ ufs_open(char *path, struct open_file *f
   }
  
   inumber = ROOTINO;
 - if ((rc = read_inode(inumber, f)) != 0)
 + if ((rc = read_inode(inumber, f)))
   goto out;
  
   cp = path;
 @@ -458,7 +458,7 @@ ufs_open(char *path, struct open_file *f
   /*
* Open next component.
*/
 - if ((rc = read_inode(inumber, f)) != 0)
 + if ((rc = read_inode(inumber, f)))
   goto out;
  
   /*
 @@ -515,7 +515,7 @@ ufs_open(char *path, struct open_file *f
   else
   inumber = ROOTINO;
  
 - if ((rc = read_inode(inumber, f)) != 0)
 + if ((rc = read_inode(inumber, f)))
   goto out;
   }
   }
 @@ -660,7 +660,7 @@ ufs_readdir(struct open_file *f, char *n
   }
  
   do {
 - if ((rc = buf_read_file(f, buf, buf_size)) != 0)
 + if ((rc = buf_read_file(f, buf, buf_size)))
   return rc;
  
 

Re: ffs2 boot

2014-07-16 Thread Kent R. Spillner
*Bump*

 On Jul 10, 2014, at 12:33, Kent R. Spillner kspill...@acm.org wrote:
 
 Ping.
 
 On Thu, May 01, 2014 at 01:22:56PM -0500, Kent R. Spillner wrote:
 After sending my previous reply I noticed that you already committed
 your diff, so here are my comments again in the form of a proper diff:
 
 * Use NULL instead of casting 0 to pointer types
 
 * Remove unnecessary (char *) cast on buf because buf was already
  declared as char *
 
 * Simplify if ((rc = ...) != 0) idiom to equivalent if ((rc = ...))
 
 
 Index: sys/lib/libsa/ufs2.c
 ===
 RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
 retrieving revision 1.2
 diff -p -u -r1.2 ufs2.c
 --- sys/lib/libsa/ufs2.c29 Apr 2014 07:52:06 -1.2
 +++ sys/lib/libsa/ufs2.c1 May 2014 16:54:25 -
 @@ -217,7 +217,7 @@ block_map(struct open_file *f, daddr_t f
}
 
if (fp-f_blkno[level] != ind_block_num) {
 -if (fp-f_blk[level] == (char *)0)
 +if (fp-f_blk[level] == NULL)
fp-f_blk[level] =
alloc(fs-fs_bsize);
twiddle();
 @@ -269,7 +269,7 @@ buf_read_file(struct open_file *f, char 
if (rc)
return (rc);
 
 -if (fp-f_buf == (char *)0)
 +if (fp-f_buf == NULL)
fp-f_buf = alloc(fs-fs_bsize);
 
if (disk_block == 0) {
 @@ -401,7 +401,7 @@ ufs2_open(char *path, struct open_file *
}
 
inumber = ROOTINO;
 -if ((rc = read_inode(inumber, f)) != 0)
 +if ((rc = read_inode(inumber, f)))
goto out;
 
cp = path;
 @@ -454,7 +454,7 @@ ufs2_open(char *path, struct open_file *
/*
 * Open next component.
 */
 -if ((rc = read_inode(inumber, f)) != 0)
 +if ((rc = read_inode(inumber, f)))
goto out;
 
/*
 @@ -498,7 +498,7 @@ ufs2_open(char *path, struct open_file *
if (rc)
goto out;
 
 -bcopy((char *)buf, namebuf, (unsigned)link_len);
 +bcopy(buf, namebuf, (unsigned)link_len);
}
 
/*
 @@ -511,7 +511,7 @@ ufs2_open(char *path, struct open_file *
else
inumber = ROOTINO;
 
 -if ((rc = read_inode(inumber, f)) != 0)
 +if ((rc = read_inode(inumber, f)))
goto out;
}
}
 @@ -534,8 +534,8 @@ ufs2_close(struct open_file *f)
 {
struct file *fp = (struct file *)f-f_fsdata;
 
 -f-f_fsdata = (void *)0;
 -if (fp == (struct file *)0)
 +f-f_fsdata = NULL;
 +if (fp == NULL)
return (0);
 
return (ufs2_close_internal(fp));
 @@ -656,7 +656,7 @@ ufs2_readdir(struct open_file *f, char *
}
 
do {
 -if ((rc = buf_read_file(f, buf, buf_size)) != 0)
 +if ((rc = buf_read_file(f, buf, buf_size)))
return rc;
 
dp = (struct direct *)buf;
 



Re: ffs2 boot

2014-07-10 Thread Kent R. Spillner
Ping.

On Thu, May 01, 2014 at 01:22:56PM -0500, Kent R. Spillner wrote:
 After sending my previous reply I noticed that you already committed
 your diff, so here are my comments again in the form of a proper diff:
 
 * Use NULL instead of casting 0 to pointer types
 
 * Remove unnecessary (char *) cast on buf because buf was already
   declared as char *
 
 * Simplify if ((rc = ...) != 0) idiom to equivalent if ((rc = ...))
 
 
 Index: sys/lib/libsa/ufs2.c
 ===
 RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
 retrieving revision 1.2
 diff -p -u -r1.2 ufs2.c
 --- sys/lib/libsa/ufs2.c  29 Apr 2014 07:52:06 -  1.2
 +++ sys/lib/libsa/ufs2.c  1 May 2014 16:54:25 -
 @@ -217,7 +217,7 @@ block_map(struct open_file *f, daddr_t f
   }
  
   if (fp-f_blkno[level] != ind_block_num) {
 - if (fp-f_blk[level] == (char *)0)
 + if (fp-f_blk[level] == NULL)
   fp-f_blk[level] =
   alloc(fs-fs_bsize);
   twiddle();
 @@ -269,7 +269,7 @@ buf_read_file(struct open_file *f, char 
   if (rc)
   return (rc);
  
 - if (fp-f_buf == (char *)0)
 + if (fp-f_buf == NULL)
   fp-f_buf = alloc(fs-fs_bsize);
  
   if (disk_block == 0) {
 @@ -401,7 +401,7 @@ ufs2_open(char *path, struct open_file *
   }
  
   inumber = ROOTINO;
 - if ((rc = read_inode(inumber, f)) != 0)
 + if ((rc = read_inode(inumber, f)))
   goto out;
  
   cp = path;
 @@ -454,7 +454,7 @@ ufs2_open(char *path, struct open_file *
   /*
* Open next component.
*/
 - if ((rc = read_inode(inumber, f)) != 0)
 + if ((rc = read_inode(inumber, f)))
   goto out;
  
   /*
 @@ -498,7 +498,7 @@ ufs2_open(char *path, struct open_file *
   if (rc)
   goto out;
  
 - bcopy((char *)buf, namebuf, (unsigned)link_len);
 + bcopy(buf, namebuf, (unsigned)link_len);
   }
  
   /*
 @@ -511,7 +511,7 @@ ufs2_open(char *path, struct open_file *
   else
   inumber = ROOTINO;
  
 - if ((rc = read_inode(inumber, f)) != 0)
 + if ((rc = read_inode(inumber, f)))
   goto out;
   }
   }
 @@ -534,8 +534,8 @@ ufs2_close(struct open_file *f)
  {
   struct file *fp = (struct file *)f-f_fsdata;
  
 - f-f_fsdata = (void *)0;
 - if (fp == (struct file *)0)
 + f-f_fsdata = NULL;
 + if (fp == NULL)
   return (0);
  
   return (ufs2_close_internal(fp));
 @@ -656,7 +656,7 @@ ufs2_readdir(struct open_file *f, char *
   }
  
   do {
 - if ((rc = buf_read_file(f, buf, buf_size)) != 0)
 + if ((rc = buf_read_file(f, buf, buf_size)))
   return rc;
  
   dp = (struct direct *)buf;
 



Re: ffs2 boot

2014-05-01 Thread Kent R. Spillner
After sending my previous reply I noticed that you already committed
your diff, so here are my comments again in the form of a proper diff:

* Use NULL instead of casting 0 to pointer types

* Remove unnecessary (char *) cast on buf because buf was already
  declared as char *

* Simplify if ((rc = ...) != 0) idiom to equivalent if ((rc = ...))


Index: sys/lib/libsa/ufs2.c
===
RCS file: /work/cvsroot/src/sys/lib/libsa/ufs2.c,v
retrieving revision 1.2
diff -p -u -r1.2 ufs2.c
--- sys/lib/libsa/ufs2.c29 Apr 2014 07:52:06 -  1.2
+++ sys/lib/libsa/ufs2.c1 May 2014 16:54:25 -
@@ -217,7 +217,7 @@ block_map(struct open_file *f, daddr_t f
}
 
if (fp-f_blkno[level] != ind_block_num) {
-   if (fp-f_blk[level] == (char *)0)
+   if (fp-f_blk[level] == NULL)
fp-f_blk[level] =
alloc(fs-fs_bsize);
twiddle();
@@ -269,7 +269,7 @@ buf_read_file(struct open_file *f, char 
if (rc)
return (rc);
 
-   if (fp-f_buf == (char *)0)
+   if (fp-f_buf == NULL)
fp-f_buf = alloc(fs-fs_bsize);
 
if (disk_block == 0) {
@@ -401,7 +401,7 @@ ufs2_open(char *path, struct open_file *
}
 
inumber = ROOTINO;
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
 
cp = path;
@@ -454,7 +454,7 @@ ufs2_open(char *path, struct open_file *
/*
 * Open next component.
 */
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
 
/*
@@ -498,7 +498,7 @@ ufs2_open(char *path, struct open_file *
if (rc)
goto out;
 
-   bcopy((char *)buf, namebuf, (unsigned)link_len);
+   bcopy(buf, namebuf, (unsigned)link_len);
}
 
/*
@@ -511,7 +511,7 @@ ufs2_open(char *path, struct open_file *
else
inumber = ROOTINO;
 
-   if ((rc = read_inode(inumber, f)) != 0)
+   if ((rc = read_inode(inumber, f)))
goto out;
}
}
@@ -534,8 +534,8 @@ ufs2_close(struct open_file *f)
 {
struct file *fp = (struct file *)f-f_fsdata;
 
-   f-f_fsdata = (void *)0;
-   if (fp == (struct file *)0)
+   f-f_fsdata = NULL;
+   if (fp == NULL)
return (0);
 
return (ufs2_close_internal(fp));
@@ -656,7 +656,7 @@ ufs2_readdir(struct open_file *f, char *
}
 
do {
-   if ((rc = buf_read_file(f, buf, buf_size)) != 0)
+   if ((rc = buf_read_file(f, buf, buf_size)))
return rc;
 
dp = (struct direct *)buf;



Re: ffs2 boot

2014-04-17 Thread Philip Guenther
On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
...

 But bear in mind that ffs2 has more overhead in terms of metadata.
 IMO, making it the default is not a good idea.


You have fewer than 24 years left to enjoy FFS v1...


Re: ffs2 boot

2014-04-17 Thread Otto Moerbeek
On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:

 On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
 ...
 
  But bear in mind that ffs2 has more overhead in terms of metadata.
  IMO, making it the default is not a good idea.
 
 
 You have fewer than 24 years left to enjoy FFS v1...

and I plan to enjoy every minute of that period!

-Otto




Re: ffs2 boot

2014-04-17 Thread Brandon Mercer
It will take me about that long to newfs the 10 kvm's I plan on using ;)

On Thu, Apr 17, 2014 at 5:09 AM, Otto Moerbeek o...@drijf.net wrote:
 On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:

 On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
 ...

  But bear in mind that ffs2 has more overhead in terms of metadata.
  IMO, making it the default is not a good idea.
 

 You have fewer than 24 years left to enjoy FFS v1...

 and I plan to enjoy every minute of that period!

 -Otto





Re: ffs2 boot

2014-04-17 Thread Abel Abraham Camarillo Ojeda
On Thu, Apr 17, 2014 at 5:31 AM, Brandon Mercer
yourcomputer...@gmail.com wrote:
 It will take me about that long to newfs the 10 kvm's I plan on using ;)

 On Thu, Apr 17, 2014 at 5:09 AM, Otto Moerbeek o...@drijf.net wrote:
 On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:

 On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
 ...

  But bear in mind that ffs2 has more overhead in terms of metadata.
  IMO, making it the default is not a good idea.
 

 You have fewer than 24 years left to enjoy FFS v1...

 and I plan to enjoy every minute of that period!

 -Otto




I found it really fast to work with kvm/openbsd if you use -drive
...,if=virtio ...
like 4x-5x times faster than if=ide -the default-



Re: ffs2 boot

2014-04-17 Thread Giancarlo Razzolini
Em 17-04-2014 07:34, Abel Abraham Camarillo Ojeda escreveu:
 On Thu, Apr 17, 2014 at 5:31 AM, Brandon Mercer
 yourcomputer...@gmail.com wrote:
 It will take me about that long to newfs the 10 kvm's I plan on using ;)

 On Thu, Apr 17, 2014 at 5:09 AM, Otto Moerbeek o...@drijf.net wrote:
 On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:

 On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
 ...

 But bear in mind that ffs2 has more overhead in terms of metadata.
 IMO, making it the default is not a good idea.

 You have fewer than 24 years left to enjoy FFS v1...
 and I plan to enjoy every minute of that period!

 -Otto


 I found it really fast to work with kvm/openbsd if you use -drive
 ...,if=virtio ...
 like 4x-5x times faster than if=ide -the default-

I use everything virtio and the performance difference is quite notable.
The only complain is that openbsd won't see more than one processor, no
matter what you do.

-- 
Giancarlo Razzolini
GPG: 4096R/77B981BC



Re: ffs2 boot

2014-04-17 Thread Adam Thompson
Given the single-threaded nature of much of the kernel, what applications do 
you run where multiple CPUs makes much of a difference to OpenBSD?

Also, switching from IDE to any of the supported SCSI, SAS or SATA disk types 
also produces a noticeable improvement.  I'm not sure if those are available in 
every KVM implementation or just the Proxmox-integrated version... IIRC they're 
available in RHEV too, so most likely they're standard.

-Adam

On April 17, 2014 11:34:19 AM CDT, Giancarlo Razzolini grazzol...@gmail.com 
wrote:
Em 17-04-2014 07:34, Abel Abraham Camarillo Ojeda escreveu:
 On Thu, Apr 17, 2014 at 5:31 AM, Brandon Mercer
 yourcomputer...@gmail.com wrote:
 It will take me about that long to newfs the 10 kvm's I plan on
using ;)

 On Thu, Apr 17, 2014 at 5:09 AM, Otto Moerbeek o...@drijf.net
wrote:
 On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:

 On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
 ...

 But bear in mind that ffs2 has more overhead in terms of
metadata.
 IMO, making it the default is not a good idea.

 You have fewer than 24 years left to enjoy FFS v1...
 and I plan to enjoy every minute of that period!

 -Otto


 I found it really fast to work with kvm/openbsd if you use -drive
 ...,if=virtio ...
 like 4x-5x times faster than if=ide -the default-

I use everything virtio and the performance difference is quite
notable.
The only complain is that openbsd won't see more than one processor, no
matter what you do.

-- 
Giancarlo Razzolini
GPG: 4096R/77B981BC

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


Re: ffs2 boot

2014-04-17 Thread Otto Moerbeek
On Thu, Apr 17, 2014 at 12:22:44PM -0500, Adam Thompson wrote:

 Given the single-threaded nature of much of the kernel, what applications do 
 you run where multiple CPUs makes much of a difference to OpenBSD?


Come on, a machine runs multiple processes...

 
 Also, switching from IDE to any of the supported SCSI, SAS or SATA disk types 
 also produces a noticeable improvement.  I'm not sure if those are available 
 in every KVM implementation or just the Proxmox-integrated version... IIRC 
 they're available in RHEV too, so most likely they're standard.

 
 -Adam
 
 On April 17, 2014 11:34:19 AM CDT, Giancarlo Razzolini grazzol...@gmail.com 
 wrote:
 Em 17-04-2014 07:34, Abel Abraham Camarillo Ojeda escreveu:
  On Thu, Apr 17, 2014 at 5:31 AM, Brandon Mercer
  yourcomputer...@gmail.com wrote:
  It will take me about that long to newfs the 10 kvm's I plan on
 using ;)
 
  On Thu, Apr 17, 2014 at 5:09 AM, Otto Moerbeek o...@drijf.net
 wrote:
  On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:
 
  On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
  ...
 
  But bear in mind that ffs2 has more overhead in terms of
 metadata.
  IMO, making it the default is not a good idea.
 
  You have fewer than 24 years left to enjoy FFS v1...
  and I plan to enjoy every minute of that period!
 
  -Otto
 
 
  I found it really fast to work with kvm/openbsd if you use -drive
  ...,if=virtio ...
  like 4x-5x times faster than if=ide -the default-
 
 I use everything virtio and the performance difference is quite
 notable.
 The only complain is that openbsd won't see more than one processor, no
 matter what you do.
 
 -- 
 Giancarlo Razzolini
 GPG: 4096R/77B981BC
 
 -- 
 Sent from my Android device with K-9 Mail. Please excuse my brevity.



Re: ffs2 boot

2014-04-17 Thread Adam Thompson
Yes, but the very nature of the discussion concerns VMs, where the point is to 
multiplex the physical CPUs into multiple VMs in user-controllable chunks.  A 
VM with one vCPU is perfectly reasonable and normal.

I've found that having multiple cores available can speed up a desktop, and 
certain classes of cpu-bound server applications, and not much else. 
Those applications are not many; databases (sometimes), web servers 
(sometimes), application servers (often).
The fact my router has 8 cores available doesn't really help it very much.  
(Maybe BGP converges a little bit faster?)  Ditto for my DNS servers, my mail 
server, my proxy server, etc.
So, I would like to know what application Giancarlo has where he actually 
notices the lack of multiple cores.
-Adam

On April 17, 2014 12:23:44 PM CDT, Otto Moerbeek o...@drijf.net wrote:
On Thu, Apr 17, 2014 at 12:22:44PM -0500, Adam Thompson wrote:

 Given the single-threaded nature of much of the kernel, what
applications do you run where multiple CPUs makes much of a difference
to OpenBSD?


Come on, a machine runs multiple processes...

 
 Also, switching from IDE to any of the supported SCSI, SAS or SATA
disk types also produces a noticeable improvement.  I'm not sure if
those are available in every KVM implementation or just the
Proxmox-integrated version... IIRC they're available in RHEV too, so
most likely they're standard.

 
 -Adam
 
 On April 17, 2014 11:34:19 AM CDT, Giancarlo Razzolini
grazzol...@gmail.com wrote:
 Em 17-04-2014 07:34, Abel Abraham Camarillo Ojeda escreveu:
  On Thu, Apr 17, 2014 at 5:31 AM, Brandon Mercer
  yourcomputer...@gmail.com wrote:
  It will take me about that long to newfs the 10 kvm's I plan on
 using ;)
 
  On Thu, Apr 17, 2014 at 5:09 AM, Otto Moerbeek o...@drijf.net
 wrote:
  On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:
 
  On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net
wrote:
  ...
 
  But bear in mind that ffs2 has more overhead in terms of
 metadata.
  IMO, making it the default is not a good idea.
 
  You have fewer than 24 years left to enjoy FFS v1...
  and I plan to enjoy every minute of that period!
 
  -Otto
 
 
  I found it really fast to work with kvm/openbsd if you use -drive
  ...,if=virtio ...
  like 4x-5x times faster than if=ide -the default-
 
 I use everything virtio and the performance difference is quite
 notable.
 The only complain is that openbsd won't see more than one processor,
no
 matter what you do.
 
 -- 
 Giancarlo Razzolini
 GPG: 4096R/77B981BC
 
 -- 
 Sent from my Android device with K-9 Mail. Please excuse my brevity.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.


Re: ffs2 boot

2014-04-17 Thread Brandon Mercer
Please take this discussion elsewhere. This email is about being able
to boot off ffs2 not your ability to run vm's.

On Thu, Apr 17, 2014 at 1:30 PM, Adam Thompson athom...@athompso.net wrote:
 Yes, but the very nature of the discussion concerns VMs, where the point is
 to multiplex the physical CPUs into multiple VMs in user-controllable
 chunks. A VM with one vCPU is perfectly reasonable and normal.

 I've found that having multiple cores available can speed up a desktop, and
 certain classes of cpu-bound server applications, and not much else.
 Those applications are not many; databases (sometimes), web servers
 (sometimes), application servers (often).
 The fact my router has 8 cores available doesn't really help it very much.
 (Maybe BGP converges a little bit faster?) Ditto for my DNS servers, my mail
 server, my proxy server, etc.
 So, I would like to know what application Giancarlo has where he actually
 notices the lack of multiple cores.
 -Adam


 On April 17, 2014 12:23:44 PM CDT, Otto Moerbeek o...@drijf.net wrote:

 On Thu, Apr 17, 2014 at 12:22:44PM -0500, Adam Thompson wrote:

  Given the single-threaded nature of much of the kernel, what
 applications do you run where multiple CPUs makes much of a difference to
 OpenBSD?



 Come on, a machine runs multiple processes...


  Also, switching from IDE to any of the supported SCSI, SAS or SATA disk
 types also produces a noticeable improvement.  I'm not sure if those are
 available in every KVM implementation or just the Proxmox-integrated
 version... IIRC they're available in RHEV too, so most likely they're
 standard.



  -Adam

  On April 17, 2014 11:34:19 AM CDT, Giancarlo Razzolini
 grazzol...@gmail.com wrote:

 Em 17-04-2014 07:34, Abel Abraham Camarillo Ojeda escreveu:

  On Thu, Apr 17, 2014 at 5:31 AM, Brandon Mercer
  yourcomputer...@gmail.com wrote:

  It will take me about that long to newfs the 10 kvm's I plan on

 using ;)


  On Thu, Apr 17, 2014 at 5:09 AM, Otto Moerbeek o...@drijf.net

 wrote:

  On Wed, Apr 16, 2014 at 11:16:00PM -0700, Philip Guenther wrote:

  On Thursday, April 17, 2014, Otto Moerbeek o...@drijf.net wrote:
  ...

  But bear in mind that ffs2 has more overhead in terms of

 metadata.

  IMO, making it the default is not a good idea.


  You have fewer than 24 years left to enjoy FFS v1...

  and I plan to enjoy every minute of that period!

  -Otto



  I found it really fast to work with kvm/openbsd if you use -drive

 ...,if=virtio ...
  like 4x-5x times faster than if=ide -the default-


 I use everything virtio and the performance difference is quite
 notable.
 The only complain is that openbsd won't see more than one processor, no
 matter what you do.

 --
 Giancarlo Razzolini
 GPG: 4096R/77B981BC


  --
  Sent from my Android device with K-9 Mail. Please excuse my brevity.


 --
 Sent from my Android device with K-9 Mail. Please excuse my brevity.



Re: ffs2 boot

2014-04-17 Thread Giancarlo Razzolini
Em 17-04-2014 14:30, Adam Thompson escreveu:
 Yes, but the very nature of the discussion concerns VMs, where the
 point is to multiplex the physical CPUs into multiple VMs in
 user-controllable chunks. A VM with one vCPU is perfectly reasonable
 and normal.

 I've found that having multiple cores available can speed up a
 desktop, and certain classes of cpu-bound server applications, and not
 much else.
 Those applications are not many; databases (sometimes), web servers
 (sometimes), application servers (often).
 The fact my router has 8 cores available doesn't really help it very
 much. (Maybe BGP converges a little bit faster?) Ditto for my DNS
 servers, my mail server, my proxy server, etc.
 So, I would like to know what application Giancarlo has where he
 actually notices the lack of multiple cores.
 -Adam
I use it in my firewall. PF runs on only one core, no issue here. But
there is DNS server, web server, nagios, smtpd, squid, plus some other
things running with it. So having more cpu's would be nice. I see some
cpu throughput bottlenecks now and then. It was more of a rant than
anything else. I will only be able to take a look at why the cpu doesn't
show in a few months. And even then I'm not sure I'll be able to solve
the problem.

-- 
Giancarlo Razzolini
GPG: 4096R/77B981BC



Re: ffs2 boot

2014-04-17 Thread Henning Brauer
* Adam Thompson athom...@athompso.net [2014-04-17 19:31]:
 I've found that having multiple cores available can speed up a desktop, and 
 certain classes of cpu-bound server applications, and not much else. 

MP speeds up all userland-heavy tasks a lot.
MP doesn't yet speed up kernel-heavy tasks as much as it should.

I have one use case where 8-core-MP is more than 7 times faster than
single core.

 Those applications are not many; databases (sometimes), web servers
 (sometimes), application servers (often). 

no, there's much more. As said, most of everything userland-heavy.

 The fact my router has 8 cores available doesn't really help it very
 much.  (Maybe BGP converges a little bit faster?)

it can help bgpd indeed.

 Ditto for my DNS servers, my mail server, my proxy server, etc. 

depends on the workload. heavy content filtering on mailservers will
benefit.

-- 
Henning Brauer, h...@bsws.de, henn...@openbsd.org
BS Web Services GmbH, http://bsws.de, Full-Service ISP
Secure Hosting, Mail and DNS Services. Dedicated Servers, Root to Fully Managed
Henning Brauer Consulting, http://henningbrauer.com/



Re: ffs2 boot

2014-04-17 Thread Marc Espie
On Thu, Apr 17, 2014 at 12:22:44PM -0500, Adam Thompson wrote:
 Given the single-threaded nature of much of the kernel, what applications do 
 you run where multiple CPUs makes much of a difference to OpenBSD?
 

You're a narrow-minded troll.



Re: ffs2 boot

2014-04-17 Thread Juan Francisco Cantero Hurtado
On Wed, Apr 16, 2014 at 05:29:36PM -0500, Shawn K. Quinn wrote:
 On Wed, Apr 16, 2014, at 03:05 PM, Miod Vallat wrote:
[responding to Brandon Mercer who wrote:]
   The other day I was doing an install in qemu-kvm and newfs was taking
   forever, to the tune of hours. This is similar to formatting on arm
   boards. In my quest to track down why, I discovered that ffs2 takes far
   less time to format than ffs1 (about 30 seconds for the entire disk). 
   
   I've put together a diff that updates the boot blocks on amd64 to be 
   able to boot ffs2. From there it's a one line change to make newfs 
   format ffs2 by default. Obviously this would need to happen for other 
   architectures as well and I'd be glad to tackle that if others see 
   this as worthwhile. Please let me know your thoughts. 
  
  Awesome. You've just trimmed my todolist by a few lines (-:
  And you've done it so that you do not force UFS2 support on
  tight-space-challenged boot blocks on other arches.
 
 I'm not against adding cool features, but are there people who really
 need a root filesystem of one whole terabyte or larger? I've never

The best feature of UFS2 for me is a faster fsck.

 needed my root filesystem to be larger than, say, a gigabyte or two. The
 only case for which this might make some sense is an external hard
 drive, formatted FFS2, on a 1T+ drive nearly full of personal files that
 just happens to have a bsd.rd in the root to reinstall/upgrade a hosed
 system. For most others, there should be a note that making your root
 filesystem That Big is usually a Really Bad Idea.

-- 
Juan Francisco Cantero Hurtado http://juanfra.info



Re: ffs2 boot

2014-04-16 Thread Miod Vallat
 The other day I was doing an install in qemu-kvm and newfs was taking
 forever, to the tune of hours. This is similar to formatting on arm
 boards. In my quest to track down why, I discovered that ffs2 takes far
 less time to format than ffs1 (about 30 seconds for the entire disk). 
 
 I've put together a diff that updates the boot blocks on amd64 to be 
 able to boot ffs2. From there it's a one line change to make newfs 
 format ffs2 by default. Obviously this would need to happen for other 
 architectures as well and I'd be glad to tackle that if others see 
 this as worthwhile. Please let me know your thoughts. 

Awesome. You've just trimmed my todolist by a few lines (-:
And you've done it so that you do not force UFS2 support on
tight-space-challenged boot blocks on other arches.

All you need now is to merge your installboot changes into the MI
installboot, and we'll be able to add ffs2 support in the installer on a
per-platform basis.

However, it is way too early to make ffs2 the default for newfs.

Please commit your libsa changes at the earliest opportunity.

Miod



Re: ffs2 boot

2014-04-16 Thread Brandon Mercer
On Wed, Apr 16, 2014 at 08:05:57PM +, Miod Vallat wrote:
  The other day I was doing an install in qemu-kvm and newfs was taking
  forever, to the tune of hours. This is similar to formatting on arm
  boards. In my quest to track down why, I discovered that ffs2 takes far
  less time to format than ffs1 (about 30 seconds for the entire disk). 
  
  I've put together a diff that updates the boot blocks on amd64 to be 
  able to boot ffs2. From there it's a one line change to make newfs 
  format ffs2 by default. Obviously this would need to happen for other 
  architectures as well and I'd be glad to tackle that if others see 
  this as worthwhile. Please let me know your thoughts. 
 
 Awesome. You've just trimmed my todolist by a few lines (-:
 And you've done it so that you do not force UFS2 support on
 tight-space-challenged boot blocks on other arches.
 
 All you need now is to merge your installboot changes into the MI
 installboot, and we'll be able to add ffs2 support in the installer on a
 per-platform basis.
 
 However, it is way too early to make ffs2 the default for newfs.
 
 Please commit your libsa changes at the earliest opportunity.

Slightly revised diff. In my haste to make the compiler happy I likely
broke things. This diff properly casts the pointer. This work was done
by Pedro Martelletto for bitrig. I'll commit the diff below per miod's
request. 
 
Index: lib/libsa/ufs2.c
===
RCS file: lib/libsa/ufs2.c
diff -N lib/libsa/ufs2.c
--- /dev/null   1 Jan 1970 00:00:00 -
+++ lib/libsa/ufs2.c16 Apr 2014 21:43:19 -
@@ -0,0 +1,712 @@
+/*-
+ * Copyright (c) 1993
+ * The Regents of the University of California.  All rights reserved.
+ *
+ * This code is derived from software contributed to Berkeley by
+ * The Mach Operating System project at Carnegie-Mellon University.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ *may be used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ *
+ *
+ * Copyright (c) 1990, 1991 Carnegie Mellon University
+ * All Rights Reserved.
+ *
+ * Author: David Golub
+ *
+ * Permission to use, copy, modify and distribute this software and its
+ * documentation is hereby granted, provided that both the copyright
+ * notice and this permission notice appear in all copies of the
+ * software, derivative works or modified versions, and any portions
+ * thereof, and that both notices appear in supporting documentation.
+ *
+ * CARNEGIE MELLON ALLOWS FREE USE OF THIS SOFTWARE IN ITS AS IS
+ * CONDITION.  CARNEGIE MELLON DISCLAIMS ANY LIABILITY OF ANY KIND FOR
+ * ANY DAMAGES WHATSOEVER RESULTING FROM THE USE OF THIS SOFTWARE.
+ *
+ * Carnegie Mellon requests users of this software to return to
+ *
+ *  Software Distribution Coordinator  or  software.distribut...@cs.cmu.edu
+ *  School of Computer Science
+ *  Carnegie Mellon University
+ *  Pittsburgh PA 15213-3890
+ *
+ * any improvements or extensions that they make and grant Carnegie the
+ * rights to redistribute these changes.
+ */
+
+/*
+ * Stand-alone file reading package.
+ */
+
+#include sys/param.h
+#include sys/time.h
+#include sys/stat.h
+#include ufs/ffs/fs.h
+#include ufs/ufs/dinode.h
+#include ufs/ufs/dir.h
+#include lib/libkern/libkern.h
+
+#include stand.h
+#include ufs2.h
+
+/*
+ * In-core open file.
+ */
+struct file {
+   off_t   f_seekp;/* seek pointer */
+   struct fs   *f_fs;  /* pointer to super-block */
+   struct ufs2_dinode  f_di;   /* copy of on-disk inode */
+   int 

Re: ffs2 boot

2014-04-16 Thread Shawn K. Quinn
On Wed, Apr 16, 2014, at 03:05 PM, Miod Vallat wrote:
   [responding to Brandon Mercer who wrote:]
  The other day I was doing an install in qemu-kvm and newfs was taking
  forever, to the tune of hours. This is similar to formatting on arm
  boards. In my quest to track down why, I discovered that ffs2 takes far
  less time to format than ffs1 (about 30 seconds for the entire disk). 
  
  I've put together a diff that updates the boot blocks on amd64 to be 
  able to boot ffs2. From there it's a one line change to make newfs 
  format ffs2 by default. Obviously this would need to happen for other 
  architectures as well and I'd be glad to tackle that if others see 
  this as worthwhile. Please let me know your thoughts. 
 
 Awesome. You've just trimmed my todolist by a few lines (-:
 And you've done it so that you do not force UFS2 support on
 tight-space-challenged boot blocks on other arches.

I'm not against adding cool features, but are there people who really
need a root filesystem of one whole terabyte or larger? I've never
needed my root filesystem to be larger than, say, a gigabyte or two. The
only case for which this might make some sense is an external hard
drive, formatted FFS2, on a 1T+ drive nearly full of personal files that
just happens to have a bsd.rd in the root to reinstall/upgrade a hosed
system. For most others, there should be a note that making your root
filesystem That Big is usually a Really Bad Idea.

-- 
  Shawn K. Quinn
  skqu...@rushpost.com



Re: ffs2 boot

2014-04-16 Thread Brandon Mercer
As I mentioned previously, this does more than just allow you to boot
from TB disks. When I configure a 100GB disk in qemu-kvm it takes
forever to format. On small flash installations, formatting is faster
there as well. You don't need a 1TB disk to benefit from ffs2.

On Wed, Apr 16, 2014 at 3:57 PM, Brandon Mercer
yourcomputer...@gmail.com wrote:
 The other day I was doing an install in qemu-kvm and newfs was taking
 forever, to the tune of hours. This is similar to formatting on arm
 boards. In my quest to track down why, I discovered that ffs2 takes far
 less time to format than ffs1 (about 30 seconds for the entire disk).

 I've put together a diff that updates the boot blocks on amd64 to be
 able to boot ffs2. From there it's a one line change to make newfs
 format ffs2 by default. Obviously this would need to happen for other
 architectures as well and I'd be glad to tackle that if others see
 this as worthwhile. Please let me know your thoughts.

 Brandon


 Index: sbin/newfs/newfs.c
 ===
 RCS file: /cvs/src/sbin/newfs/newfs.c,v
 retrieving revision 1.95
 diff -u -p -u -r1.95 newfs.c
 --- sbin/newfs/newfs.c  22 Nov 2013 04:12:48 -  1.95
 +++ sbin/newfs/newfs.c  16 Apr 2014 17:47:02 -
 @@ -112,7 +112,7 @@ u_short dkcksum(struct disklabel *);

  intmfs;/* run as the memory based filesystem */
  intNflag;  /* run without writing file system */
 -intOflag = 1;  /* 0 = 4.3BSD ffs, 1 = 4.4BSD ffs, 2 = ffs2 */
 +intOflag = 2;  /* 0 = 4.3BSD ffs, 1 = 4.4BSD ffs, 2 = ffs2 */
  daddr_tfssize; /* file system size in 512-byte 
 blocks */
  long long  sectorsize; /* bytes/sector */
  intfsize = 0;  /* fragment size */

 Index: arch/amd64/stand/biosboot/biosboot.S
 ===
 RCS file: /cvs/src/sys/arch/amd64/stand/biosboot/biosboot.S,v
 retrieving revision 1.7
 diff -u -p -r1.7 biosboot.S
 --- arch/amd64/stand/biosboot/biosboot.S5 Jul 2011 17:38:54 - 
   1.7
 +++ arch/amd64/stand/biosboot/biosboot.S16 Apr 2014 17:22:25 -
 @@ -108,6 +108,9 @@
   * While this can be calculated as
   * howmany(di_size, fs_bsize) it takes us too
   * many code bytes to do it.
 + * blkskew uint8t  the skew used to parse di_db[]. this is set to four by
 + * installboot for ffs2 (due to 64-bit blocks) and should
 + * be zero for ffs1.
   *
   * All of these are patched directly into the code where they are used
   * (once only, each), to save space.
 @@ -121,7 +124,7 @@
   */

 .globl  inodeblk, inodedbl, fs_bsize_p, fsbtodb, p_offset, nblocks
 -   .globl  fs_bsize_s, force_chs
 +   .globl  fs_bsize_s, force_chs, blkskew
 .type   inodeblk, @function
 .type   inodedbl, @function
 .type   fs_bsize_p, @function
 @@ -130,6 +133,7 @@
 .type   p_offset, @function
 .type   nblocks, @function
 .type   force_chs, @function
 +   .type   blkskew, @function


  /* Clobbers %ax, maybe more */
 @@ -460,7 +464,8 @@ load_blocks:

 /* Get the next filesystem block number into %eax */
 lodsl   /* %eax = *(%si++), make sure 0x66 0xad */
 -
 +blkskew = .+2
 +   addw$0x90, %si  /* adjust %si if needed (for ffs2) */
 pushal  /* Save all 32-bit registers */

 /*
 Index: arch/amd64/stand/boot/Makefile
 ===
 RCS file: /cvs/src/sys/arch/amd64/stand/boot/Makefile,v
 retrieving revision 1.26
 diff -u -p -r1.26 Makefile
 --- arch/amd64/stand/boot/Makefile  28 Dec 2013 15:16:28 -  1.26
 +++ arch/amd64/stand/boot/Makefile  16 Apr 2014 17:22:51 -
 @@ -38,7 +38,7 @@ SRCS+=alloc.c ctime.c exit.c memcmp.c m
  SRCS+= close.c closeall.c cons.c cread.c dev.c disklabel.c dkcksum.c fstat.c 
 \
 lseek.c open.c read.c readdir.c stat.c
  SRCS+= elf32.c elf64.c loadfile.c
 -SRCS+= ufs.c
 +SRCS+= ufs.c ufs2.c
  .if ${SOFTRAID:L} == yes
  SRCS+= aes_xts.c explicit_bzero.c hmac_sha1.c pbkdf2.c rijndael.c sha1.c
  .endif
 Index: arch/amd64/stand/boot/conf.c
 ===
 RCS file: /cvs/src/sys/arch/amd64/stand/boot/conf.c,v
 retrieving revision 1.31
 diff -u -p -r1.31 conf.c
 --- arch/amd64/stand/boot/conf.c18 Feb 2014 13:56:02 -  1.31
 +++ arch/amd64/stand/boot/conf.c16 Apr 2014 17:25:17 -
 @@ -31,6 +31,7 @@
  #include netinet/in.h
  #include libsa.h
  #include lib/libsa/ufs.h
 +#include lib/libsa/ufs2.h
  #ifdef notdef
  #include lib/libsa/cd9660.h
  #include lib/libsa/fat.h
 @@ -66,6 +67,8 @@ int nibprobes = nitems(probe_list);
  struct fs_ops 

Re: ffs2 boot

2014-04-16 Thread Otto Moerbeek
On Wed, Apr 16, 2014 at 06:37:08PM -0400, Brandon Mercer wrote:

 As I mentioned previously, this does more than just allow you to boot
 from TB disks. When I configure a 100GB disk in qemu-kvm it takes
 forever to format. On small flash installations, formatting is faster
 there as well. You don't need a 1TB disk to benefit from ffs2.

But bear in mind that ffs2 has more overhead in terms of metadata.
IMO, making it the default is not a good idea.

-Otto

 
 On Wed, Apr 16, 2014 at 3:57 PM, Brandon Mercer
 yourcomputer...@gmail.com wrote:
  The other day I was doing an install in qemu-kvm and newfs was taking
  forever, to the tune of hours. This is similar to formatting on arm
  boards. In my quest to track down why, I discovered that ffs2 takes far
  less time to format than ffs1 (about 30 seconds for the entire disk).
 
  I've put together a diff that updates the boot blocks on amd64 to be
  able to boot ffs2. From there it's a one line change to make newfs
  format ffs2 by default. Obviously this would need to happen for other
  architectures as well and I'd be glad to tackle that if others see
  this as worthwhile. Please let me know your thoughts.
 
  Brandon
 
 
  Index: sbin/newfs/newfs.c
  ===
  RCS file: /cvs/src/sbin/newfs/newfs.c,v
  retrieving revision 1.95
  diff -u -p -u -r1.95 newfs.c
  --- sbin/newfs/newfs.c  22 Nov 2013 04:12:48 -  1.95
  +++ sbin/newfs/newfs.c  16 Apr 2014 17:47:02 -
  @@ -112,7 +112,7 @@ u_short dkcksum(struct disklabel *);
 
   intmfs;/* run as the memory based filesystem */
   intNflag;  /* run without writing file system */
  -intOflag = 1;  /* 0 = 4.3BSD ffs, 1 = 4.4BSD ffs, 2 = ffs2 
  */
  +intOflag = 2;  /* 0 = 4.3BSD ffs, 1 = 4.4BSD ffs, 2 = ffs2 
  */
   daddr_tfssize; /* file system size in 512-byte 
  blocks */
   long long  sectorsize; /* bytes/sector */
   intfsize = 0;  /* fragment size */
 
  Index: arch/amd64/stand/biosboot/biosboot.S
  ===
  RCS file: /cvs/src/sys/arch/amd64/stand/biosboot/biosboot.S,v
  retrieving revision 1.7
  diff -u -p -r1.7 biosboot.S
  --- arch/amd64/stand/biosboot/biosboot.S5 Jul 2011 17:38:54 -   
  1.7
  +++ arch/amd64/stand/biosboot/biosboot.S16 Apr 2014 17:22:25 -
  @@ -108,6 +108,9 @@
* While this can be calculated as
* howmany(di_size, fs_bsize) it takes us too
* many code bytes to do it.
  + * blkskew uint8t  the skew used to parse di_db[]. this is set to four 
  by
  + * installboot for ffs2 (due to 64-bit blocks) and 
  should
  + * be zero for ffs1.
*
* All of these are patched directly into the code where they are used
* (once only, each), to save space.
  @@ -121,7 +124,7 @@
*/
 
  .globl  inodeblk, inodedbl, fs_bsize_p, fsbtodb, p_offset, nblocks
  -   .globl  fs_bsize_s, force_chs
  +   .globl  fs_bsize_s, force_chs, blkskew
  .type   inodeblk, @function
  .type   inodedbl, @function
  .type   fs_bsize_p, @function
  @@ -130,6 +133,7 @@
  .type   p_offset, @function
  .type   nblocks, @function
  .type   force_chs, @function
  +   .type   blkskew, @function
 
 
   /* Clobbers %ax, maybe more */
  @@ -460,7 +464,8 @@ load_blocks:
 
  /* Get the next filesystem block number into %eax */
  lodsl   /* %eax = *(%si++), make sure 0x66 0xad */
  -
  +blkskew = .+2
  +   addw$0x90, %si  /* adjust %si if needed (for ffs2) */
  pushal  /* Save all 32-bit registers */
 
  /*
  Index: arch/amd64/stand/boot/Makefile
  ===
  RCS file: /cvs/src/sys/arch/amd64/stand/boot/Makefile,v
  retrieving revision 1.26
  diff -u -p -r1.26 Makefile
  --- arch/amd64/stand/boot/Makefile  28 Dec 2013 15:16:28 -  1.26
  +++ arch/amd64/stand/boot/Makefile  16 Apr 2014 17:22:51 -
  @@ -38,7 +38,7 @@ SRCS+=alloc.c ctime.c exit.c memcmp.c m
   SRCS+= close.c closeall.c cons.c cread.c dev.c disklabel.c dkcksum.c 
  fstat.c \
  lseek.c open.c read.c readdir.c stat.c
   SRCS+= elf32.c elf64.c loadfile.c
  -SRCS+= ufs.c
  +SRCS+= ufs.c ufs2.c
   .if ${SOFTRAID:L} == yes
   SRCS+= aes_xts.c explicit_bzero.c hmac_sha1.c pbkdf2.c rijndael.c sha1.c
   .endif
  Index: arch/amd64/stand/boot/conf.c
  ===
  RCS file: /cvs/src/sys/arch/amd64/stand/boot/conf.c,v
  retrieving revision 1.31
  diff -u -p -r1.31 conf.c
  --- arch/amd64/stand/boot/conf.c18 Feb 2014 13:56:02 -  1.31
  +++