svn commit: r344751 - head/sys/fs/ext2fs

[Fedor Uporov]

Author: fsu
Date: Mon Mar  4 10:42:25 2019
New Revision: 344751
URL: https://svnweb.freebsd.org/changeset/base/344751

Log:
  Make superblock reading logic more strict.
  
  Add more on-disk superblock consistency checks to ext2_compute_sb_data() 
function.
  It should decrease the probability of mounting filesystems with corrupted 
superblock data.
  
  Reviewed by:pfg
  MFC after:  1 week
  
  Differential Revision:https://reviews.freebsd.org/D19322

Modified:
  head/sys/fs/ext2fs/ext2_alloc.c
  head/sys/fs/ext2fs/ext2_extern.h
  head/sys/fs/ext2fs/ext2_vfsops.c
  head/sys/fs/ext2fs/ext2fs.h

Modified: head/sys/fs/ext2fs/ext2_alloc.c
==
--- head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 06:43:00 2019
(r344750)
+++ head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 10:42:25 2019
(r344751)
@@ -457,7 +457,7 @@ noinodes:
 /*
  * 64-bit compatible getters and setters for struct ext2_gd from ext2fs.h
  */
-static uint64_t
+uint64_t
 e2fs_gd_get_b_bitmap(struct ext2_gd *gd)
 {
 
@@ -465,7 +465,7 @@ e2fs_gd_get_b_bitmap(struct ext2_gd *gd)
gd->ext2bgd_b_bitmap);
 }
 
-static uint64_t
+uint64_t
 e2fs_gd_get_i_bitmap(struct ext2_gd *gd)
 {
 
@@ -754,7 +754,7 @@ ext2_hashalloc(struct inode *ip, int cg, long pref, in
return (0);
 }
 
-static unsigned long
+static uint64_t
 ext2_cg_number_gdb_nometa(struct m_ext2fs *fs, int cg)
 {
 
@@ -768,7 +768,7 @@ ext2_cg_number_gdb_nometa(struct m_ext2fs *fs, int cg)
EXT2_DESCS_PER_BLOCK(fs));
 }
 
-static unsigned long
+static uint64_t
 ext2_cg_number_gdb_meta(struct m_ext2fs *fs, int cg)
 {
unsigned long metagroup;
@@ -784,7 +784,7 @@ ext2_cg_number_gdb_meta(struct m_ext2fs *fs, int cg)
return (0);
 }
 
-static unsigned long
+uint64_t
 ext2_cg_number_gdb(struct m_ext2fs *fs, int cg)
 {
unsigned long first_meta_bg, metagroup;

Modified: head/sys/fs/ext2fs/ext2_extern.h
==
--- head/sys/fs/ext2fs/ext2_extern.hMon Mar  4 06:43:00 2019
(r344750)
+++ head/sys/fs/ext2fs/ext2_extern.hMon Mar  4 10:42:25 2019
(r344751)
@@ -91,6 +91,7 @@ int   ext2_dirrewrite(struct inode *,
 intext2_dirempty(struct inode *, ino_t, struct ucred *);
 intext2_checkpath(struct inode *, struct inode *, struct ucred *);
 intext2_cg_has_sb(struct m_ext2fs *fs, int cg);
+uint64_t   ext2_cg_number_gdb(struct m_ext2fs *fs, int cg);
 intext2_inactive(struct vop_inactive_args *);
 intext2_htree_add_entry(struct vnode *, struct ext2fs_direct_2 *,
struct componentname *);
@@ -104,6 +105,8 @@ int ext2_htree_lookup(struct inode *, const char *, in
 intext2_search_dirblock(struct inode *, void *, int *, const char *, int,
int *, doff_t *, doff_t *, doff_t *, struct ext2fs_searchslot *);
 uint32_t   e2fs_gd_get_ndirs(struct ext2_gd *gd);
+uint64_t   e2fs_gd_get_b_bitmap(struct ext2_gd *);
+uint64_t   e2fs_gd_get_i_bitmap(struct ext2_gd *);
 uint64_t   e2fs_gd_get_i_tables(struct ext2_gd *);
 void   ext2_sb_csum_set_seed(struct m_ext2fs *);
 intext2_sb_csum_verify(struct m_ext2fs *);

Modified: head/sys/fs/ext2fs/ext2_vfsops.c
==
--- head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 06:43:00 2019
(r344750)
+++ head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 10:42:25 2019
(r344751)
@@ -98,7 +98,7 @@ VFS_SET(ext2fs_vfsops, ext2fs, 0);
 
 static int ext2_check_sb_compat(struct ext2fs *es, struct cdev *dev,
int ronly);
-static int compute_sb_data(struct vnode * devvp,
+static int ext2_compute_sb_data(struct vnode * devvp,
struct ext2fs * es, struct m_ext2fs * fs);
 
 static const char *ext2_opts[] = { "acls", "async", "noatime", "noclusterr", 
@@ -321,7 +321,7 @@ ext2_check_sb_compat(struct ext2fs *es, struct cdev *d
 }
 
 static e4fs_daddr_t
-cg_location(struct m_ext2fs *fs, int number)
+ext2_cg_location(struct m_ext2fs *fs, int number)
 {
int cg, descpb, logical_sb, has_super = 0;
 
@@ -350,82 +350,196 @@ cg_location(struct m_ext2fs *fs, int number)
fs->e2fs->e2fs_first_dblock);
 }
 
+static int
+ext2_cg_validate(struct m_ext2fs *fs)
+{
+   uint64_t b_bitmap;
+   uint64_t i_bitmap;
+   uint64_t i_tables;
+   uint64_t first_block, last_block, last_cg_block;
+   struct ext2_gd *gd;
+   unsigned int i, cg_count;
+
+   first_block = fs->e2fs->e2fs_first_dblock;
+   last_cg_block = ext2_cg_number_gdb(fs, 0);
+   cg_count = fs->e2fs_gcount;
+
+   for (i = 0; i < fs->e2fs_gcount; i++) {
+   gd = &fs->e2fs_gd[i];
+
+   if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_FLEX_BG) ||
+   i == fs->e2fs_gcount - 1) {
+   last_block = fs->e2fs_bcount - 1;
+

svn commit: r344752 - head/sys/fs/ext2fs

[Fedor Uporov]

Author: fsu
Date: Mon Mar  4 10:55:01 2019
New Revision: 344752
URL: https://svnweb.freebsd.org/changeset/base/344752

Log:
  Add additional on-disk inode checks.
  
  Reviewed by:pfg
  MFC after:  1 week
  
  Differential Revision:https://reviews.freebsd.org/D19323

Modified:
  head/sys/fs/ext2fs/ext2_csum.c
  head/sys/fs/ext2fs/ext2_inode_cnv.c
  head/sys/fs/ext2fs/ext2_vfsops.c
  head/sys/fs/ext2fs/ext2fs.h

Modified: head/sys/fs/ext2fs/ext2_csum.c
==
--- head/sys/fs/ext2fs/ext2_csum.c  Mon Mar  4 10:42:25 2019
(r344751)
+++ head/sys/fs/ext2fs/ext2_csum.c  Mon Mar  4 10:55:01 2019
(r344752)
@@ -629,6 +629,8 @@ ext2_ei_csum_verify(struct inode *ip, struct ext2fs_di
if (!memcmp(ei, &ei_zero, sizeof(struct ext2fs_dinode)))
return (0);
 
+   printf("WARNING: Bad inode %ju csum - run fsck\n", 
ip->i_number);
+
return (EIO);
}
 

Modified: head/sys/fs/ext2fs/ext2_inode_cnv.c
==
--- head/sys/fs/ext2fs/ext2_inode_cnv.c Mon Mar  4 10:42:25 2019
(r344751)
+++ head/sys/fs/ext2fs/ext2_inode_cnv.c Mon Mar  4 10:55:01 2019
(r344752)
@@ -34,7 +34,6 @@
 #include 
 #include 
 
-#include 
 #include 
 #include 
 #include 
@@ -92,8 +91,31 @@ ext2_print_inode(struct inode *in)
 int
 ext2_ei2i(struct ext2fs_dinode *ei, struct inode *ip)
 {
+   struct m_ext2fs *fs = ip->i_e2fs;
 
+   if ((ip->i_number < EXT2_FIRST_INO(fs) && ip->i_number != EXT2_ROOTINO) 
||
+   (ip->i_number < EXT2_ROOTINO) ||
+   (ip->i_number > fs->e2fs->e2fs_icount)) {
+   printf("ext2fs: bad inode number %ju\n", ip->i_number);
+   return (EINVAL);
+   }
+
+   if (ip->i_number == EXT2_ROOTINO && ei->e2di_nlink == 0) {
+   printf("ext2fs: root inode unallocated\n");
+   return (EINVAL);
+   }
ip->i_nlink = ei->e2di_nlink;
+
+   /* Check extra inode size */
+   if (EXT2_INODE_SIZE(fs) > E2FS_REV0_INODE_SIZE) {
+   if (E2FS_REV0_INODE_SIZE + ei->e2di_extra_isize >
+   EXT2_INODE_SIZE(fs) || (ei->e2di_extra_isize & 3)) {
+   printf("ext2fs: bad extra inode size %u, inode 
size=%u\n",
+   ei->e2di_extra_isize, EXT2_INODE_SIZE(fs));
+   return (EINVAL);
+   }
+   }
+
/*
 * Godmar thinks - if the link count is zero, then the inode is
 * unused - according to ext2 standards. Ufs marks this fact by

Modified: head/sys/fs/ext2fs/ext2_vfsops.c
==
--- head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 10:42:25 2019
(r344751)
+++ head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 10:55:01 2019
(r344752)
@@ -773,11 +773,18 @@ loop:
MNT_VNODE_FOREACH_ALL_ABORT(mp, mvp);
return (error);
}
-   ext2_ei2i((struct ext2fs_dinode *)((char *)bp->b_data +
+
+   error = ext2_ei2i((struct ext2fs_dinode *)((char *)bp->b_data +
EXT2_INODE_SIZE(fs) * ino_to_fsbo(fs, ip->i_number)), ip);
+
brelse(bp);
VOP_UNLOCK(vp, 0);
vrele(vp);
+
+   if (error) {
+   MNT_VNODE_FOREACH_ALL_ABORT(mp, mvp);
+   return (error);
+   }
}
return (0);
 }
@@ -1208,8 +1215,6 @@ ext2_vget(struct mount *mp, ino_t ino, int flags, stru
error = ext2_ei2i((struct ext2fs_dinode *)((char *)bp->b_data +
EXT2_INODE_SIZE(fs) * ino_to_fsbo(fs, ino)), ip);
if (error) {
-   printf("ext2fs: Bad inode %lu csum - run fsck\n",
-   (unsigned long)ino);
brelse(bp);
vput(vp);
*vpp = NULL;

Modified: head/sys/fs/ext2fs/ext2fs.h
==
--- head/sys/fs/ext2fs/ext2fs.h Mon Mar  4 10:42:25 2019(r344751)
+++ head/sys/fs/ext2fs/ext2fs.h Mon Mar  4 10:55:01 2019(r344752)
@@ -422,4 +422,11 @@ struct ext2_gd {
EXT2F_INCOMPAT_64BIT) ? ((s)->e2fs_bsize / sizeof(struct ext2_gd)) : \
((s)->e2fs_bsize / E2FS_REV0_GD_SIZE))
 
+/*
+ * Macro-instructions used to manage inodes
+ */
+#defineEXT2_FIRST_INO(s)   ((EXT2_SB(s)->e2fs->e2fs_rev == 
E2FS_REV0) ? \
+EXT2_FIRSTINO : \
+EXT2_SB(s)->e2fs->e2fs_first_ino)
+
 #endif /* !_FS_EXT2FS_EXT2FS_H_ */
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.o

svn commit: r344753 - head/sys/fs/ext2fs

[Fedor Uporov]

Author: fsu
Date: Mon Mar  4 11:01:23 2019
New Revision: 344753
URL: https://svnweb.freebsd.org/changeset/base/344753

Log:
  Validate block bitmaps.
  
  Reviewed by:pfg
  MFC after:  1 week
  
  Differential Revision:https://reviews.freebsd.org/D19324

Modified:
  head/sys/fs/ext2fs/ext2_alloc.c

Modified: head/sys/fs/ext2fs/ext2_alloc.c
==
--- head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 10:55:01 2019
(r344752)
+++ head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 11:01:23 2019
(r344753)
@@ -902,6 +902,52 @@ ext2_cg_block_bitmap_init(struct m_ext2fs *fs, int cg,
return (0);
 }
 
+static int
+ext2_b_bitmap_validate(struct m_ext2fs *fs, struct buf *bp, int cg)
+{
+   struct ext2_gd *gd;
+   uint64_t group_first_block;
+   unsigned int offset, max_bit;
+
+   if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_FLEX_BG)) {
+   /*
+* It is not possible to check block bitmap in case of this 
feature,
+* because the inode and block bitmaps and inode table
+* blocks may not be in the group at all.
+* So, skip check in this case.
+*/
+   return (0);
+   }
+
+   gd = &fs->e2fs_gd[cg];
+   max_bit = fs->e2fs_fpg;
+   group_first_block = ((uint64_t)cg) * fs->e2fs->e2fs_fpg +
+   fs->e2fs->e2fs_first_dblock;
+
+   /* Check block bitmap block number */
+   offset = e2fs_gd_get_b_bitmap(gd) - group_first_block;
+   if (offset >= max_bit || !isset(bp->b_data, offset)) {
+   printf("ext2fs: bad block bitmap, group %d\n", cg);
+   return (EINVAL);
+   }
+
+   /* Check inode bitmap block number */
+   offset = e2fs_gd_get_i_bitmap(gd) - group_first_block;
+   if (offset >= max_bit || !isset(bp->b_data, offset)) {
+   printf("ext2fs: bad inode bitmap, group %d\n", cg);
+   return (EINVAL);
+   }
+
+   /* Check inode table */
+   offset = e2fs_gd_get_i_tables(gd) - group_first_block;
+   if (offset >= max_bit || offset + fs->e2fs_itpg >= max_bit) {
+   printf("ext2fs: bad inode table, group %d\n", cg);
+   return (EINVAL);
+   }
+
+   return (0);
+}
+
 /*
  * Determine whether a block can be allocated.
  *
@@ -922,40 +968,37 @@ ext2_alloccg(struct inode *ip, int cg, daddr_t bpref, 
ump = ip->i_ump;
if (e2fs_gd_get_nbfree(&fs->e2fs_gd[cg]) == 0)
return (0);
+
EXT2_UNLOCK(ump);
error = bread(ip->i_devvp, fsbtodb(fs,
e2fs_gd_get_b_bitmap(&fs->e2fs_gd[cg])),
(int)fs->e2fs_bsize, NOCRED, &bp);
-   if (error) {
-   brelse(bp);
-   EXT2_LOCK(ump);
-   return (0);
-   }
+   if (error)
+   goto fail;
+
if (EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_GDT_CSUM) ||
EXT2_HAS_RO_COMPAT_FEATURE(fs, EXT2F_ROCOMPAT_METADATA_CKSUM)) {
error = ext2_cg_block_bitmap_init(fs, cg, bp);
-   if (error) {
-   brelse(bp);
-   EXT2_LOCK(ump);
-   return (0);
-   }
+   if (error)
+   goto fail;
+
ext2_gd_b_bitmap_csum_set(fs, cg, bp);
}
error = ext2_gd_b_bitmap_csum_verify(fs, cg, bp);
-   if (error) {
-   brelse(bp);
-   EXT2_LOCK(ump);
-   return (0);
-   }
-   if (e2fs_gd_get_nbfree(&fs->e2fs_gd[cg]) == 0) {
-   /*
-* Another thread allocated the last block in this
-* group while we were waiting for the buffer.
-*/
-   brelse(bp);
-   EXT2_LOCK(ump);
-   return (0);
-   }
+   if (error)
+   goto fail;
+
+   error = ext2_b_bitmap_validate(fs,bp, cg);
+   if (error)
+   goto fail;
+
+   /*
+* Check, that another thread did not not allocate the last block in 
this
+* group while we were waiting for the buffer.
+*/
+   if (e2fs_gd_get_nbfree(&fs->e2fs_gd[cg]) == 0)
+   goto fail;
+
bbp = (char *)bp->b_data;
 
if (dtog(fs, bpref) != cg)
@@ -1028,11 +1071,9 @@ retry:
goto retry;
}
bno = ext2_mapsearch(fs, bbp, bpref);
-   if (bno < 0) {
-   brelse(bp);
-   EXT2_LOCK(ump);
-   return (0);
-   }
+   if (bno < 0)
+   goto fail;
+
 gotit:
 #ifdef INVARIANTS
if (isset(bbp, bno)) {
@@ -1052,6 +1093,11 @@ gotit:
ext2_gd_b_bitmap_csum_set(fs, cg, bp);
bdwrite(bp);
return (((uint64_t)cg) * fs->e2fs->e2fs_fpg + 
fs->e2fs->e2fs_first_dblock + bno);
+
+fail:
+   brelse(bp);
+   EXT2_LOCK(ump);
+   return (0);
 }
 

svn commit: r344754 - head/sys/fs/ext2fs

[Fedor Uporov]

Author: fsu
Date: Mon Mar  4 11:12:19 2019
New Revision: 344754
URL: https://svnweb.freebsd.org/changeset/base/344754

Log:
  Do not panic if inode bitmap is corrupted.
  
  admbug: 804
  Reported by:Ilja Van Sprundel 
  Reviewed by:pfg
  MFC after:  1 week
  
  Differential Revision:https://reviews.freebsd.org/D19325

Modified:
  head/sys/fs/ext2fs/ext2_alloc.c

Modified: head/sys/fs/ext2fs/ext2_alloc.c
==
--- head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 11:01:23 2019
(r344753)
+++ head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 11:12:19 2019
(r344754)
@@ -1318,10 +1318,12 @@ ext2_nodealloccg(struct inode *ip, int cg, daddr_t ipr
start = 0;
loc = memcchr(&ibp[start], 0xff, len);
if (loc == NULL) {
-   printf("cg = %d, ipref = %lld, fs = %s\n",
+   printf("ext2fs: inode bitmap corrupted: "
+   "cg = %d, ipref = %lld, fs = %s - run fsck\n",
cg, (long long)ipref, fs->e2fs_fsmnt);
-   panic("ext2fs_nodealloccg: map corrupted");
-   /* NOTREACHED */
+   brelse(bp);
+   EXT2_LOCK(ump);
+   return (0);
}
}
ipref = (loc - ibp) * NBBY + ffs(~*loc) - 1;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344755 - head/sys/fs/ext2fs

[Fedor Uporov]

Author: fsu
Date: Mon Mar  4 11:19:21 2019
New Revision: 344755
URL: https://svnweb.freebsd.org/changeset/base/344755

Log:
  Fix integer overflow possibility.
  
  Reported by:Christopher Krah 
  Reported as:FS-2-EXT2-1: Out-of-Bounds Write in nmount (ext2_vget)
  Reviewed by:pfg
  MFC after:  1 week
  
  Differential Revision:https://reviews.freebsd.org/D19326

Modified:
  head/sys/fs/ext2fs/ext2_vfsops.c

Modified: head/sys/fs/ext2fs/ext2_vfsops.c
==
--- head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 11:12:19 2019
(r344754)
+++ head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 11:19:21 2019
(r344755)
@@ -1163,8 +1163,8 @@ ext2_vget(struct mount *mp, ino_t ino, int flags, stru
struct buf *bp;
struct vnode *vp;
struct thread *td;
-   int i, error;
-   int used_blocks;
+   unsigned int i, used_blocks;
+   int error;
 
td = curthread;
error = vfs_hash_get(mp, ino, flags, td, vpp, NULL, NULL);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344756 - head/sys/fs/ext2fs

[Fedor Uporov]

Author: fsu
Date: Mon Mar  4 11:27:47 2019
New Revision: 344756
URL: https://svnweb.freebsd.org/changeset/base/344756

Log:
  Do not read the on-disk inode in case of vnode allocation.
  
  Reported by:Christopher Krah 
  Reported as:FS-6-EXT2-4: Denial Of Service in mkdir-0 (ext2_mkdir/vn_rdwr)
  Reviewed by:pfg
  MFC after:  1 week
  
  Differential Revision:https://reviews.freebsd.org/D19327

Modified:
  head/sys/fs/ext2fs/ext2_alloc.c

Modified: head/sys/fs/ext2fs/ext2_alloc.c
==
--- head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 11:19:21 2019
(r344755)
+++ head/sys/fs/ext2fs/ext2_alloc.c Mon Mar  4 11:27:47 2019
(r344756)
@@ -373,10 +373,12 @@ int
 ext2_valloc(struct vnode *pvp, int mode, struct ucred *cred, struct vnode 
**vpp)
 {
struct timespec ts;
-   struct inode *pip;
struct m_ext2fs *fs;
-   struct inode *ip;
struct ext2mount *ump;
+   struct inode *pip;
+   struct inode *ip;
+   struct vnode *vp;
+   struct thread *td;
ino_t ino, ipref;
int error, cg;
 
@@ -404,33 +406,69 @@ ext2_valloc(struct vnode *pvp, int mode, struct ucred 
}
ipref = cg * fs->e2fs->e2fs_ipg + 1;
ino = (ino_t)ext2_hashalloc(pip, cg, (long)ipref, mode, 
ext2_nodealloccg);
-
if (ino == 0)
goto noinodes;
-   error = VFS_VGET(pvp->v_mount, ino, LK_EXCLUSIVE, vpp);
+
+   td = curthread;
+   error = vfs_hash_get(ump->um_mountp, ino, LK_EXCLUSIVE, td, vpp, NULL, 
NULL);
+   if (error || *vpp != NULL) {
+   EXT2_UNLOCK(ump);
+   return (error);
+   }
+
+   ip = malloc(sizeof(struct inode), M_EXT2NODE, M_WAITOK | M_ZERO);
+   if (ip == NULL) {
+   EXT2_UNLOCK(ump);
+   return (ENOMEM);
+   }
+
+   /* Allocate a new vnode/inode. */
+   if ((error = getnewvnode("ext2fs", ump->um_mountp, &ext2_vnodeops, 
&vp)) != 0) {
+   free(ip, M_EXT2NODE);
+   EXT2_UNLOCK(ump);
+   return (error);
+   }
+
+   vp->v_data = ip;
+   ip->i_vnode = vp;
+   ip->i_e2fs = fs = ump->um_e2fs;
+   ip->i_ump = ump;
+   ip->i_number = ino;
+   ip->i_block_group = ino_to_cg(fs, ino);
+   ip->i_next_alloc_block = 0;
+   ip->i_next_alloc_goal = 0;
+
+   lockmgr(vp->v_vnlock, LK_EXCLUSIVE, NULL);
+   error = insmntque(vp, ump->um_mountp);
if (error) {
-   ext2_vfree(pvp, ino, mode);
+   free(ip, M_EXT2NODE);
+   EXT2_UNLOCK(ump);
return (error);
}
-   ip = VTOI(*vpp);
 
-   /*
-* The question is whether using VGET was such good idea at all:
-* Linux doesn't read the old inode in when it is allocating a
-* new one. I will set at least i_size and i_blocks to zero.
-*/
-   ip->i_flag = 0;
-   ip->i_size = 0;
-   ip->i_blocks = 0;
-   ip->i_mode = 0;
-   ip->i_flags = 0;
+   error = vfs_hash_insert(vp, ino, LK_EXCLUSIVE, td, vpp, NULL, NULL);
+   if (error || *vpp != NULL) {
+   *vpp = NULL;
+   free(ip, M_EXT2NODE);
+   EXT2_UNLOCK(ump);
+   return (error);
+   }
+
+   if ((error = ext2_vinit(ump->um_mountp, &ext2_fifoops, &vp)) != 0) {
+   vput(vp);
+   *vpp = NULL;
+   free(ip, M_EXT2NODE);
+   EXT2_UNLOCK(ump);
+   return (error);
+   }
+
if (EXT2_HAS_INCOMPAT_FEATURE(fs, EXT2F_INCOMPAT_EXTENTS)
&& (S_ISREG(mode) || S_ISDIR(mode)))
ext4_ext_tree_init(ip);
else
memset(ip->i_data, 0, sizeof(ip->i_data));
-   
 
+
/*
 * Set up a new generation number for this inode.
 * Avoid zero values.
@@ -443,10 +481,10 @@ ext2_valloc(struct vnode *pvp, int mode, struct ucred 
ip->i_birthtime = ts.tv_sec;
ip->i_birthnsec = ts.tv_nsec;
 
-/*
-printf("ext2_valloc: allocated inode %d\n", ino);
-*/
+   *vpp = vp;
+
return (0);
+
 noinodes:
EXT2_UNLOCK(ump);
ext2_fserr(fs, cred->cr_uid, "out of inodes");
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344757 - head/sys/fs/ext2fs

[Fedor Uporov]

Author: fsu
Date: Mon Mar  4 11:33:49 2019
New Revision: 344757
URL: https://svnweb.freebsd.org/changeset/base/344757

Log:
  Fix double free in case of mount error.
  
  Reported by:Christopher Krah 
  Reported as:FS-9-EXT3-2: Denial Of Service in nmount-5 (vm_fault_hold)
  Reviewed by:pfg
  MFC after:  1 week
  
  Differential Revision:https://reviews.freebsd.org/D19385

Modified:
  head/sys/fs/ext2fs/ext2_vfsops.c

Modified: head/sys/fs/ext2fs/ext2_vfsops.c
==
--- head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 11:27:47 2019
(r344756)
+++ head/sys/fs/ext2fs/ext2_vfsops.cMon Mar  4 11:33:49 2019
(r344757)
@@ -614,8 +614,12 @@ ext2_compute_sb_data(struct vnode *devvp, struct ext2f
fsbtodb(fs, ext2_cg_location(fs, i)),
fs->e2fs_bsize, NOCRED, &bp);
if (error) {
-   free(fs->e2fs_contigdirs, M_EXT2MNT);
-   free(fs->e2fs_gd, M_EXT2MNT);
+   /*
+* fs->e2fs_gd and fs->e2fs_contigdirs
+* will be freed later by the caller,
+* because this function could be called from
+* MNT_UPDATE path.
+*/
brelse(bp);
return (error);
}
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Edward Tomasz Napierala]

Author: trasz
Date: Mon Mar  4 13:02:36 2019
New Revision: 344758
URL: https://svnweb.freebsd.org/changeset/base/344758

Log:
  Push down td in nfsrvd_dorpc() - make it use curthread instead
  of it being explicitly passed as an argument. No functional changes.
  
  The big picture here is that I want to get rid of the 'td' argument
  being passed everywhere, and this is the first piece that affects
  the NFS server.
  
  Reviewed by:  rmacklem
  MFC after:2 weeks
  Sponsored by: DARPA, AFRL
  Differential Revision:https://reviews.freebsd.org/D19417

Modified:
  head/sys/fs/nfs/nfs_var.h
  head/sys/fs/nfsserver/nfs_nfsdkrpc.c
  head/sys/fs/nfsserver/nfs_nfsdsocket.c

Modified: head/sys/fs/nfs/nfs_var.h
==
--- head/sys/fs/nfs/nfs_var.h   Mon Mar  4 11:33:49 2019(r344757)
+++ head/sys/fs/nfs/nfs_var.h   Mon Mar  4 13:02:36 2019(r344758)
@@ -283,8 +283,7 @@ int nfsrvd_notsupp(struct nfsrv_descript *, int,
 
 /* nfs_nfsdsocket.c */
 void nfsrvd_rephead(struct nfsrv_descript *);
-void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t,
-NFSPROC_T *);
+void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t);
 
 /* nfs_nfsdcache.c */
 void nfsrvd_initcache(void);

Modified: head/sys/fs/nfsserver/nfs_nfsdkrpc.c
==
--- head/sys/fs/nfsserver/nfs_nfsdkrpc.cMon Mar  4 11:33:49 2019
(r344757)
+++ head/sys/fs/nfsserver/nfs_nfsdkrpc.cMon Mar  4 13:02:36 2019
(r344758)
@@ -323,7 +323,6 @@ static int
 nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVCXPRT *xprt,
 struct nfsrvcache **rpp)
 {
-   struct thread *td = curthread;
int cacherep = RC_DOIT, isdgram, taglen = -1;
struct mbuf *m;
u_char tag[NFSV4_SMALLSTR + 1], *tagstr = NULL;
@@ -384,7 +383,7 @@ nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVC
if (cacherep == RC_DOIT) {
if ((nd->nd_flag & ND_NFSV41) != 0)
nd->nd_xprt = xprt;
-   nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers, td);
+   nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers);
if ((nd->nd_flag & ND_NFSV41) != 0) {
if (nd->nd_repstat != NFSERR_REPLYFROMCACHE &&
(nd->nd_flag & ND_SAVEREPLY) != 0) {

Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c
==
--- head/sys/fs/nfsserver/nfs_nfsdsocket.c  Mon Mar  4 11:33:49 2019
(r344757)
+++ head/sys/fs/nfsserver/nfs_nfsdsocket.c  Mon Mar  4 13:02:36 2019
(r344758)
@@ -367,7 +367,7 @@ int nfsrv_writerpc[NFS_NPROCS] = { 0, 0, 1, 0, 0, 0, 0
 
 /* local functions */
 static void nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
-u_char *tag, int taglen, u_int32_t minorvers, NFSPROC_T *p);
+u_char *tag, int taglen, u_int32_t minorvers);
 
 
 /*
@@ -475,14 +475,17 @@ nfsrvd_statend(int op, uint64_t bytes, struct bintime 
  */
 APPLESTATIC void
 nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u_char *tag, int taglen,
-u_int32_t minorvers, NFSPROC_T *p)
+u_int32_t minorvers)
 {
int error = 0, lktype;
vnode_t vp;
mount_t mp = NULL;
struct nfsrvfh fh;
struct nfsexstuff nes;
+   struct thread *p;
 
+   p = curthread;
+
/*
 * Get a locked vnode for the first file handle
 */
@@ -557,7 +560,7 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u
 * The group is indicated by the value in nfs_retfh[].
 */
if (nd->nd_flag & ND_NFSV4) {
-   nfsrvd_compound(nd, isdgram, tag, taglen, minorvers, p);
+   nfsrvd_compound(nd, isdgram, tag, taglen, minorvers);
} else {
struct bintime start_time;
 
@@ -620,7 +623,7 @@ out:
  */
 static void
 nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
-int taglen, u_int32_t minorvers, NFSPROC_T *p)
+int taglen, u_int32_t minorvers)
 {
int i, lktype, op, op0 = 0, statsinprog = 0;
u_int32_t *tl;
@@ -635,6 +638,9 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram
fsid_t cur_fsid, save_fsid;
static u_int64_t compref = 0;
struct bintime start_time;
+   struct thread *p;
+
+   p = curthread;
 
NFSVNO_EXINIT(&vpnes);
NFSVNO_EXINIT(&savevpnes);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344759 - in head/sys/fs: nfs nfsserver

[Edward Tomasz Napierala]

Author: trasz
Date: Mon Mar  4 13:12:23 2019
New Revision: 344759
URL: https://svnweb.freebsd.org/changeset/base/344759

Log:
  Push down the thread argument in NFS server code, using curthread
  instead of passing it explicitly. No functional changes
  
  Reviewed by:  rmacklem (earlier version)
  MFC after:2 weeks
  Sponsored by: DARPA, AFRL
  Differential Revision:https://reviews.freebsd.org/D19419

Modified:
  head/sys/fs/nfs/nfs_var.h
  head/sys/fs/nfsserver/nfs_nfsdport.c
  head/sys/fs/nfsserver/nfs_nfsdserv.c
  head/sys/fs/nfsserver/nfs_nfsdsocket.c

Modified: head/sys/fs/nfs/nfs_var.h
==
--- head/sys/fs/nfs/nfs_var.h   Mon Mar  4 13:02:36 2019(r344758)
+++ head/sys/fs/nfs/nfs_var.h   Mon Mar  4 13:12:23 2019(r344759)
@@ -171,115 +171,107 @@ int nfsrv_mdscopymr(char *, char *, char *, char *, in
 
 /* nfs_nfsdserv.c */
 int nfsrvd_access(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_getattr(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_setattr(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_lookup(struct nfsrv_descript *, int,
-vnode_t, vnode_t *, fhandle_t *, NFSPROC_T *,
-struct nfsexstuff *);
+vnode_t, vnode_t *, fhandle_t *, struct nfsexstuff *);
 int nfsrvd_readlink(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_read(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_write(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_create(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_mknod(struct nfsrv_descript *, int,
-vnode_t, vnode_t *, fhandle_t *, NFSPROC_T *,
-struct nfsexstuff *);
+vnode_t, vnode_t *, fhandle_t *, struct nfsexstuff *);
 int nfsrvd_remove(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_rename(struct nfsrv_descript *, int,
-vnode_t, vnode_t, NFSPROC_T *, struct nfsexstuff *,
-struct nfsexstuff *);
+vnode_t, vnode_t, struct nfsexstuff *, struct nfsexstuff *);
 int nfsrvd_link(struct nfsrv_descript *, int,
-vnode_t, vnode_t, NFSPROC_T *, struct nfsexstuff *,
-struct nfsexstuff *);
+vnode_t, vnode_t, struct nfsexstuff *, struct nfsexstuff *);
 int nfsrvd_symlink(struct nfsrv_descript *, int,
-vnode_t, vnode_t *, fhandle_t *, NFSPROC_T *,
-struct nfsexstuff *);
+vnode_t, vnode_t *, fhandle_t *, struct nfsexstuff *);
 int nfsrvd_mkdir(struct nfsrv_descript *, int,
-vnode_t, vnode_t *, fhandle_t *, NFSPROC_T *,
-struct nfsexstuff *);
+vnode_t, vnode_t *, fhandle_t *, struct nfsexstuff *);
 int nfsrvd_readdir(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_readdirplus(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_commit(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_statfs(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_fsinfo(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_close(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_delegpurge(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_delegreturn(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_getfh(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_lock(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_lockt(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_locku(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_openconfirm(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, struct nfsexstuff *);
+vnode_t, struct nfsexstuff *);
 int nfsrvd_opendowngrade(struct nfsrv_descript *, int,
-vnode_t, NFSPROC_T *, st

svn commit: r344760 - in head/sys/fs: nfs nfsserver

[Edward Tomasz Napierala]

Author: trasz
Date: Mon Mar  4 13:18:04 2019
New Revision: 344760
URL: https://svnweb.freebsd.org/changeset/base/344760

Log:
  Don't pass td to nfsd_fhtovp(), it's unused.
  
  Reviewed by:  rmacklem (earlier version)
  MFC after:2 weeks
  Sponsored by: DARPA, AFRL
  Differential Revision:https://reviews.freebsd.org/D19421

Modified:
  head/sys/fs/nfs/nfs_var.h
  head/sys/fs/nfsserver/nfs_nfsdport.c
  head/sys/fs/nfsserver/nfs_nfsdserv.c
  head/sys/fs/nfsserver/nfs_nfsdsocket.c

Modified: head/sys/fs/nfs/nfs_var.h
==
--- head/sys/fs/nfs/nfs_var.h   Mon Mar  4 13:12:23 2019(r344759)
+++ head/sys/fs/nfs/nfs_var.h   Mon Mar  4 13:18:04 2019(r344760)
@@ -358,8 +358,7 @@ int nfscl_request(struct nfsrv_descript *, vnode_t,
 
 /* nfs_nfsdsubs.c */
 void nfsd_fhtovp(struct nfsrv_descript *, struct nfsrvfh *, int,
-vnode_t *, struct nfsexstuff *,
-mount_t *, int, NFSPROC_T *);
+vnode_t *, struct nfsexstuff *, mount_t *, int);
 int nfsd_excred(struct nfsrv_descript *, struct nfsexstuff *, struct ucred *);
 int nfsrv_mtofh(struct nfsrv_descript *, struct nfsrvfh *);
 int nfsrv_putattrbit(struct nfsrv_descript *, nfsattrbit_t *);

Modified: head/sys/fs/nfsserver/nfs_nfsdport.c
==
--- head/sys/fs/nfsserver/nfs_nfsdport.cMon Mar  4 13:12:23 2019
(r344759)
+++ head/sys/fs/nfsserver/nfs_nfsdport.cMon Mar  4 13:18:04 2019
(r344760)
@@ -3041,7 +3041,7 @@ nfsvno_fhtovp(struct mount *mp, fhandle_t *fhp, struct
 void
 nfsd_fhtovp(struct nfsrv_descript *nd, struct nfsrvfh *nfp, int lktype,
 struct vnode **vpp, struct nfsexstuff *exp,
-struct mount **mpp, int startwrite, struct thread *p)
+struct mount **mpp, int startwrite)
 {
struct mount *mp;
struct ucred *credanon;

Modified: head/sys/fs/nfsserver/nfs_nfsdserv.c
==
--- head/sys/fs/nfsserver/nfs_nfsdserv.cMon Mar  4 13:12:23 2019
(r344759)
+++ head/sys/fs/nfsserver/nfs_nfsdserv.cMon Mar  4 13:18:04 2019
(r344760)
@@ -1609,7 +1609,7 @@ nfsrvd_rename(struct nfsrv_descript *nd, int isdgram,
NFSVOPUNLOCK(dp, 0);
nd->nd_cred->cr_uid = nd->nd_saveduid;
nfsd_fhtovp(nd, &tfh, LK_EXCLUSIVE, &tdp, &tnes, NULL,
-   0, p);  /* Locks tdp. */
+   0); /* Locks tdp. */
if (tdp) {
tdirfor_ret = nfsvno_getattr(tdp, &tdirfor, nd,
p, 1, NULL);
@@ -1740,8 +1740,7 @@ nfsrvd_link(struct nfsrv_descript *nd, int isdgram,
/* tovp is always NULL unless NFSv4 */
goto out;
}
-   nfsd_fhtovp(nd, &dfh, LK_EXCLUSIVE, &dp, &tnes, NULL, 0,
-   p);
+   nfsd_fhtovp(nd, &dfh, LK_EXCLUSIVE, &dp, &tnes, NULL, 
0);
if (dp)
NFSVOPUNLOCK(dp, 0);
}
@@ -3612,7 +3611,7 @@ nfsrvd_secinfo(struct nfsrv_descript *nd, int isdgram,
vput(vp);
savflag = nd->nd_flag;
if (!nd->nd_repstat) {
-   nfsd_fhtovp(nd, &fh, LK_SHARED, &vp, &retnes, NULL, 0, p);
+   nfsd_fhtovp(nd, &fh, LK_SHARED, &vp, &retnes, NULL, 0);
if (vp)
vput(vp);
}

Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c
==
--- head/sys/fs/nfsserver/nfs_nfsdsocket.c  Mon Mar  4 13:12:23 2019
(r344759)
+++ head/sys/fs/nfsserver/nfs_nfsdsocket.c  Mon Mar  4 13:18:04 2019
(r344760)
@@ -478,10 +478,7 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u
mount_t mp = NULL;
struct nfsrvfh fh;
struct nfsexstuff nes;
-   struct thread *p;
 
-   p = curthread;
-
/*
 * Get a locked vnode for the first file handle
 */
@@ -516,10 +513,10 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u
lktype = LK_EXCLUSIVE;
if (nd->nd_flag & ND_PUBLOOKUP)
nfsd_fhtovp(nd, &nfs_pubfh, lktype, &vp, &nes,
-   &mp, nfsrv_writerpc[nd->nd_procnum], p);
+   &mp, nfsrv_writerpc[nd->nd_procnum]);
else
nfsd_fhtovp(nd, &fh, lktype, &vp, &nes,
-   &mp, nfsrv_writerpc[nd->nd_procnum], p);
+   &mp, nfsrv_writerpc[nd->nd_procnum]);
if (nd->nd_repstat == NFSERR_PROGNOTV4)
   

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Konstantin Belousov]

On Mon, Mar 04, 2019 at 01:02:36PM +, Edward Tomasz Napierala wrote:
> Author: trasz
> Date: Mon Mar  4 13:02:36 2019
> New Revision: 344758
> URL: https://svnweb.freebsd.org/changeset/base/344758
> 
> Log:
>   Push down td in nfsrvd_dorpc() - make it use curthread instead
>   of it being explicitly passed as an argument. No functional changes.
>   
>   The big picture here is that I want to get rid of the 'td' argument
>   being passed everywhere, and this is the first piece that affects
>   the NFS server.
>   
>   Reviewed by:rmacklem
>   MFC after:  2 weeks
>   Sponsored by:   DARPA, AFRL
>   Differential Revision:  https://reviews.freebsd.org/D19417
> 
> Modified:
>   head/sys/fs/nfs/nfs_var.h
>   head/sys/fs/nfsserver/nfs_nfsdkrpc.c
>   head/sys/fs/nfsserver/nfs_nfsdsocket.c
> 
> Modified: head/sys/fs/nfs/nfs_var.h
> ==
> --- head/sys/fs/nfs/nfs_var.h Mon Mar  4 11:33:49 2019(r344757)
> +++ head/sys/fs/nfs/nfs_var.h Mon Mar  4 13:02:36 2019(r344758)
> @@ -283,8 +283,7 @@ int nfsrvd_notsupp(struct nfsrv_descript *, int,
>  
>  /* nfs_nfsdsocket.c */
>  void nfsrvd_rephead(struct nfsrv_descript *);
> -void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t,
> -NFSPROC_T *);
> +void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t);
>  
>  /* nfs_nfsdcache.c */
>  void nfsrvd_initcache(void);
> 
> Modified: head/sys/fs/nfsserver/nfs_nfsdkrpc.c
> ==
> --- head/sys/fs/nfsserver/nfs_nfsdkrpc.c  Mon Mar  4 11:33:49 2019
> (r344757)
> +++ head/sys/fs/nfsserver/nfs_nfsdkrpc.c  Mon Mar  4 13:02:36 2019
> (r344758)
> @@ -323,7 +323,6 @@ static int
>  nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVCXPRT *xprt,
>  struct nfsrvcache **rpp)
>  {
> - struct thread *td = curthread;
>   int cacherep = RC_DOIT, isdgram, taglen = -1;
>   struct mbuf *m;
>   u_char tag[NFSV4_SMALLSTR + 1], *tagstr = NULL;
> @@ -384,7 +383,7 @@ nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVC
>   if (cacherep == RC_DOIT) {
>   if ((nd->nd_flag & ND_NFSV41) != 0)
>   nd->nd_xprt = xprt;
> - nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers, td);
> + nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers);
>   if ((nd->nd_flag & ND_NFSV41) != 0) {
>   if (nd->nd_repstat != NFSERR_REPLYFROMCACHE &&
>   (nd->nd_flag & ND_SAVEREPLY) != 0) {
> 
> Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c
> ==
> --- head/sys/fs/nfsserver/nfs_nfsdsocket.cMon Mar  4 11:33:49 2019
> (r344757)
> +++ head/sys/fs/nfsserver/nfs_nfsdsocket.cMon Mar  4 13:02:36 2019
> (r344758)
> @@ -367,7 +367,7 @@ int nfsrv_writerpc[NFS_NPROCS] = { 0, 0, 1, 0, 0, 0, 0
>  
>  /* local functions */
>  static void nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
> -u_char *tag, int taglen, u_int32_t minorvers, NFSPROC_T *p);
> +u_char *tag, int taglen, u_int32_t minorvers);
>  
>  
>  /*
> @@ -475,14 +475,17 @@ nfsrvd_statend(int op, uint64_t bytes, struct bintime 
>   */
>  APPLESTATIC void
>  nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u_char *tag, int taglen,
> -u_int32_t minorvers, NFSPROC_T *p)
> +u_int32_t minorvers)
>  {
>   int error = 0, lktype;
>   vnode_t vp;
>   mount_t mp = NULL;
>   struct nfsrvfh fh;
>   struct nfsexstuff nes;
> + struct thread *p;
>  
> + p = curthread;
> +
>   /*
>* Get a locked vnode for the first file handle
>*/
> @@ -557,7 +560,7 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u
>* The group is indicated by the value in nfs_retfh[].
>*/
>   if (nd->nd_flag & ND_NFSV4) {
> - nfsrvd_compound(nd, isdgram, tag, taglen, minorvers, p);
> + nfsrvd_compound(nd, isdgram, tag, taglen, minorvers);
>   } else {
>   struct bintime start_time;
>  
> @@ -620,7 +623,7 @@ out:
>   */
>  static void
>  nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
> -int taglen, u_int32_t minorvers, NFSPROC_T *p)
> +int taglen, u_int32_t minorvers)
>  {
>   int i, lktype, op, op0 = 0, statsinprog = 0;
>   u_int32_t *tl;
> @@ -635,6 +638,9 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram
>   fsid_t cur_fsid, save_fsid;
>   static u_int64_t compref = 0;
>   struct bintime start_time;
> + struct thread *p;
> +
> + p = curthread;
Why do you name it 'p', which is typical for process, and not 'td', you are
changing most of the code anyway.

Also I am curious why. It is certainly fine to remove td when it is used
as a formal placeholder argument only. But when the first action in the
f

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Edward Napierala]

pon., 4 mar 2019 o 13:20 Konstantin Belousov  napisał(a):
>
> On Mon, Mar 04, 2019 at 01:02:36PM +, Edward Tomasz Napierala wrote:
> > Author: trasz
> > Date: Mon Mar  4 13:02:36 2019
> > New Revision: 344758
> > URL: https://svnweb.freebsd.org/changeset/base/344758
> >
> > Log:
> >   Push down td in nfsrvd_dorpc() - make it use curthread instead
> >   of it being explicitly passed as an argument. No functional changes.
> >
> >   The big picture here is that I want to get rid of the 'td' argument
> >   being passed everywhere, and this is the first piece that affects
> >   the NFS server.
> >
> >   Reviewed by:rmacklem
> >   MFC after:  2 weeks
> >   Sponsored by:   DARPA, AFRL
> >   Differential Revision:  https://reviews.freebsd.org/D19417
> >
> > Modified:
> >   head/sys/fs/nfs/nfs_var.h
> >   head/sys/fs/nfsserver/nfs_nfsdkrpc.c
> >   head/sys/fs/nfsserver/nfs_nfsdsocket.c
> >
> > Modified: head/sys/fs/nfs/nfs_var.h
> > ==
> > --- head/sys/fs/nfs/nfs_var.h Mon Mar  4 11:33:49 2019(r344757)
> > +++ head/sys/fs/nfs/nfs_var.h Mon Mar  4 13:02:36 2019(r344758)
> > @@ -283,8 +283,7 @@ int nfsrvd_notsupp(struct nfsrv_descript *, int,
> >
> >  /* nfs_nfsdsocket.c */
> >  void nfsrvd_rephead(struct nfsrv_descript *);
> > -void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t,
> > -NFSPROC_T *);
> > +void nfsrvd_dorpc(struct nfsrv_descript *, int, u_char *, int, u_int32_t);
> >
> >  /* nfs_nfsdcache.c */
> >  void nfsrvd_initcache(void);
> >
> > Modified: head/sys/fs/nfsserver/nfs_nfsdkrpc.c
> > ==
> > --- head/sys/fs/nfsserver/nfs_nfsdkrpc.c  Mon Mar  4 11:33:49 2019  
> >   (r344757)
> > +++ head/sys/fs/nfsserver/nfs_nfsdkrpc.c  Mon Mar  4 13:02:36 2019  
> >   (r344758)
> > @@ -323,7 +323,6 @@ static int
> >  nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVCXPRT *xprt,
> >  struct nfsrvcache **rpp)
> >  {
> > - struct thread *td = curthread;
> >   int cacherep = RC_DOIT, isdgram, taglen = -1;
> >   struct mbuf *m;
> >   u_char tag[NFSV4_SMALLSTR + 1], *tagstr = NULL;
> > @@ -384,7 +383,7 @@ nfs_proc(struct nfsrv_descript *nd, u_int32_t xid, SVC
> >   if (cacherep == RC_DOIT) {
> >   if ((nd->nd_flag & ND_NFSV41) != 0)
> >   nd->nd_xprt = xprt;
> > - nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers, td);
> > + nfsrvd_dorpc(nd, isdgram, tagstr, taglen, minorvers);
> >   if ((nd->nd_flag & ND_NFSV41) != 0) {
> >   if (nd->nd_repstat != NFSERR_REPLYFROMCACHE &&
> >   (nd->nd_flag & ND_SAVEREPLY) != 0) {
> >
> > Modified: head/sys/fs/nfsserver/nfs_nfsdsocket.c
> > ==
> > --- head/sys/fs/nfsserver/nfs_nfsdsocket.cMon Mar  4 11:33:49 2019  
> >   (r344757)
> > +++ head/sys/fs/nfsserver/nfs_nfsdsocket.cMon Mar  4 13:02:36 2019  
> >   (r344758)
> > @@ -367,7 +367,7 @@ int nfsrv_writerpc[NFS_NPROCS] = { 0, 0, 1, 0, 0, 0, 0
> >
> >  /* local functions */
> >  static void nfsrvd_compound(struct nfsrv_descript *nd, int isdgram,
> > -u_char *tag, int taglen, u_int32_t minorvers, NFSPROC_T *p);
> > +u_char *tag, int taglen, u_int32_t minorvers);
> >
> >
> >  /*
> > @@ -475,14 +475,17 @@ nfsrvd_statend(int op, uint64_t bytes, struct bintime
> >   */
> >  APPLESTATIC void
> >  nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u_char *tag, int 
> > taglen,
> > -u_int32_t minorvers, NFSPROC_T *p)
> > +u_int32_t minorvers)
> >  {
> >   int error = 0, lktype;
> >   vnode_t vp;
> >   mount_t mp = NULL;
> >   struct nfsrvfh fh;
> >   struct nfsexstuff nes;
> > + struct thread *p;
> >
> > + p = curthread;
> > +
> >   /*
> >* Get a locked vnode for the first file handle
> >*/
> > @@ -557,7 +560,7 @@ nfsrvd_dorpc(struct nfsrv_descript *nd, int isdgram, u
> >* The group is indicated by the value in nfs_retfh[].
> >*/
> >   if (nd->nd_flag & ND_NFSV4) {
> > - nfsrvd_compound(nd, isdgram, tag, taglen, minorvers, p);
> > + nfsrvd_compound(nd, isdgram, tag, taglen, minorvers);
> >   } else {
> >   struct bintime start_time;
> >
> > @@ -620,7 +623,7 @@ out:
> >   */
> >  static void
> >  nfsrvd_compound(struct nfsrv_descript *nd, int isdgram, u_char *tag,
> > -int taglen, u_int32_t minorvers, NFSPROC_T *p)
> > +int taglen, u_int32_t minorvers)
> >  {
> >   int i, lktype, op, op0 = 0, statsinprog = 0;
> >   u_int32_t *tl;
> > @@ -635,6 +638,9 @@ nfsrvd_compound(struct nfsrv_descript *nd, int isdgram
> >   fsid_t cur_fsid, save_fsid;
> >   static u_int64_t compref = 0;
> >   struct bintime start_time;
> > + struct thread *p;
> > +

svn commit: r344761 - in head: lib/libmt usr.bin/mt

[Kenneth D. Merry]

Author: ken
Date: Mon Mar  4 14:30:37 2019
New Revision: 344761
URL: https://svnweb.freebsd.org/changeset/base/344761

Log:
  Add IBM TS1160 density codes to libmt and the mt(1) man page.
  
  These are taken directly from the density report from a TS1160
  tape drive.  (Using mt getdensity)
  
  A TS1160 drive stores 20TB raw (60TB with compression) on a JE tape.
  
  lib/libmt/mtlib.c:
Add 3592A6 encrypted/unencrypted density codes, and bpmm/bpi
values.
  
  usr.bin/mt/mt.1:
Add 3592B5 encrypted/unencrypted density codes, bpmm/bpi
values and number of tracks.  Bump the man page date.
  
  MFC after:3 days
  Sponsored by: Spectra Logic

Modified:
  head/lib/libmt/mtlib.c
  head/usr.bin/mt/mt.1

Modified: head/lib/libmt/mtlib.c
==
--- head/lib/libmt/mtlib.c  Mon Mar  4 13:18:04 2019(r344760)
+++ head/lib/libmt/mtlib.c  Mon Mar  4 14:30:37 2019(r344761)
@@ -642,6 +642,7 @@ static struct densities {
{ 0x54, 19686,  500024, "3592A4 (unencrypted)" },
{ 0x55, 20670,  525018, "3592A5 (unencrypted)" },
{ 0x56, 20670,  525018, "3592B5 (unencrypted)" },
+   { 0x57, 21850,  554990, "3592A6 (unencrypted)" },
{ 0x58, 15142,  384607, "LTO-5" },
{ 0x5A, 15142,  384607, "LTO-6" },
{ 0x5C, 19107,  485318, "LTO-7" },
@@ -653,6 +654,7 @@ static struct densities {
{ 0x74, 19686,  500024, "3592A4 (encrypted)" },
{ 0x75, 20670,  525018, "3592A5 (encrypted)" },
{ 0x76, 20670,  525018, "3592B5 (encrypted)" },
+   { 0x77, 21850,  554990, "3592A6 (encrypted)" },
{ 0x8c,  1789,   45434, "EXB-8500c" },
{ 0x90,  1703,   43245, "EXB-8200c" },
{ 0, 0, 0, NULL }

Modified: head/usr.bin/mt/mt.1
==
--- head/usr.bin/mt/mt.1Mon Mar  4 13:18:04 2019(r344760)
+++ head/usr.bin/mt/mt.1Mon Mar  4 14:30:37 2019(r344761)
@@ -29,7 +29,7 @@
 .\"@(#)mt.18.1 (Berkeley) 6/6/93
 .\" $FreeBSD$
 .\"
-.Dd November 3, 2017
+.Dd March 4, 2019
 .Dt MT 1
 .Os
 .Sh NAME
@@ -519,6 +519,7 @@ Value  WidthTracksDensity Code Typ
 0x54   12.7  (0.5) 2560  19,686 (500,024)   C   3592A4 (unencrypted)
 0x55   12.7  (0.5) 5120  20,670 (525,018)   C   3592A5 (unencrypted)
 0x56   12.7  (0.5) 7680  20,670 (525,018)   C   3592B5 (unencrypted)
+0x57   12.7  (0.5) 8704  21,850 (554,990)   C   3592A6 (unencrypted)
 0x58   12.7  (0.5) 1280  15,142 (384,607)   C   LTO-5
 0x5A   12.7  (0.5) 2176  15,142 (384,607)   C   LTO-6
 0x5C   12.7  (0.5) 3584  19,107 (485,318)   C   LTO-7
@@ -530,6 +531,7 @@ Value  WidthTracksDensity Code Typ
 0x74   12.7  (0.5) 2560  19,686 (500,024)   C   3592A4 (encrypted)
 0x75   12.7  (0.5) 5120  20,670 (525,018)   C   3592A5 (encrypted)
 0x76   12.7  (0.5) 7680  20,670 (525,018)   C   3592B5 (encrypted)
+0x77   12.7  (0.5) 8704  21,850 (554,990)   C   3592A6 (encrypted)
 0x8c8.0  (0.315)  1  1,789  (45,434)  RLL   CS  EXB-8500c5,9
 0x908.0  (0.315)  1  1,703  (43,245)  RLL   CS  EXB-8200c5,9
 .Ed
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Konstantin Belousov]

On Mon, Mar 04, 2019 at 01:31:37PM +, Edward Napierala wrote:
> pon., 4 mar 2019 o 13:20 Konstantin Belousov  napisał(a):
> > > + p = curthread;
> > Why do you name it 'p', which is typical for process, and not 'td', you are
> > changing most of the code anyway.
> 
> To keep the diff size smaller.  You're right, this touches a lot of stuff,
> but most of those added lines are temporary anyway - they will be
> removed later, when the td is pushed down even more.
But if you create code churn, doing it only half way is worse.

> 
> > Also I am curious why. It is certainly fine to remove td when it is used
> > as a formal placeholder argument only. But when the first action in the
> > function is evaluation of curthread() it becomes less obvious.
> 
> Again, many/most of those are temporary.  I'm trying to push td down
> in small steps, "layer by layer", so it's easy to review.
> 
> > curthread() become very cheap on modern amd64, I am not so sure about
> > older machines or non-x86 cases.
> 
> The main reason is readability.  Right now there's no easy way to tell whether
> a function can be passed any td, or if it must be curthread.
I must admit that this is the weirdnest argument against 'td' that I ever
heard.  I saw more or less reasonable argumentation
- that using less arguments make one more register for argument passing
  (amd64 has 6 input arg regs),
- that less arguments make smaller call code.
But trust me, in all cases where function can take td != curthread, it is
either obvious or well-known for anybody who works with that code.

Before you start doing a lot of small changes (AKA continous churn)
please formulate your goals and get some public feedback.  My immediate
question that I want answered before you ever start touching the code,
is what you plan to do with
sys_syscall(struct thread *td, uap)

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344762 - in head/sys/fs: nfs nfsserver

[Edward Tomasz Napierala]

Author: trasz
Date: Mon Mar  4 14:30:53 2019
New Revision: 344762
URL: https://svnweb.freebsd.org/changeset/base/344762

Log:
  Don't pass td to nfsvno_createsub().
  
  MFC after:2 weeks
  Sponsored by: DARPA, AFRL

Modified:
  head/sys/fs/nfs/nfs_var.h
  head/sys/fs/nfsserver/nfs_nfsdport.c
  head/sys/fs/nfsserver/nfs_nfsdserv.c

Modified: head/sys/fs/nfs/nfs_var.h
==
--- head/sys/fs/nfs/nfs_var.h   Mon Mar  4 14:30:37 2019(r344761)
+++ head/sys/fs/nfs/nfs_var.h   Mon Mar  4 14:30:53 2019(r344762)
@@ -649,7 +649,7 @@ int nfsvno_read(vnode_t, off_t, int, struct ucred *, N
 int nfsvno_write(vnode_t, off_t, int, int, int *, mbuf_t,
 char *, struct ucred *, NFSPROC_T *);
 int nfsvno_createsub(struct nfsrv_descript *, struct nameidata *,
-vnode_t *, struct nfsvattr *, int *, int32_t *, NFSDEV_T, NFSPROC_T *,
+vnode_t *, struct nfsvattr *, int *, int32_t *, NFSDEV_T,
 struct nfsexstuff *);
 int nfsvno_mknod(struct nameidata *, struct nfsvattr *, struct ucred *,
 NFSPROC_T *);

Modified: head/sys/fs/nfsserver/nfs_nfsdport.c
==
--- head/sys/fs/nfsserver/nfs_nfsdport.cMon Mar  4 14:30:37 2019
(r344761)
+++ head/sys/fs/nfsserver/nfs_nfsdport.cMon Mar  4 14:30:53 2019
(r344762)
@@ -949,10 +949,11 @@ nfsvno_write(struct vnode *vp, off_t off, int retlen, 
 int
 nfsvno_createsub(struct nfsrv_descript *nd, struct nameidata *ndp,
 struct vnode **vpp, struct nfsvattr *nvap, int *exclusive_flagp,
-int32_t *cverf, NFSDEV_T rdev, struct thread *p, struct nfsexstuff *exp)
+int32_t *cverf, NFSDEV_T rdev, struct nfsexstuff *exp)
 {
u_quad_t tempsize;
int error;
+   struct thread *p = curthread;
 
error = nd->nd_repstat;
if (!error && ndp->ni_vp == NULL) {

Modified: head/sys/fs/nfsserver/nfs_nfsdserv.c
==
--- head/sys/fs/nfsserver/nfs_nfsdserv.cMon Mar  4 14:30:37 2019
(r344761)
+++ head/sys/fs/nfsserver/nfs_nfsdserv.cMon Mar  4 14:30:53 2019
(r344762)
@@ -1185,7 +1185,7 @@ nfsrvd_create(struct nfsrv_descript *nd, __unused int 
 *   should I set the mode too ?
 */
nd->nd_repstat = nfsvno_createsub(nd, &named, &vp, &nva,
-   &exclusive_flag, cverf, rdev, p, exp);
+   &exclusive_flag, cverf, rdev, exp);
 
if (!nd->nd_repstat) {
nd->nd_repstat = nfsvno_getfh(vp, &fh, p);
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344763 - in head/sys/fs: nfs nfsserver

[Edward Tomasz Napierala]

Author: trasz
Date: Mon Mar  4 14:50:00 2019
New Revision: 344763
URL: https://svnweb.freebsd.org/changeset/base/344763

Log:
  Don't pass td to nfsvno_open().
  
  MFC after:2 weeks
  Sponsored by: DARPA, AFRL

Modified:
  head/sys/fs/nfs/nfs_var.h
  head/sys/fs/nfsserver/nfs_nfsdport.c
  head/sys/fs/nfsserver/nfs_nfsdserv.c

Modified: head/sys/fs/nfs/nfs_var.h
==
--- head/sys/fs/nfs/nfs_var.h   Mon Mar  4 14:30:53 2019(r344762)
+++ head/sys/fs/nfs/nfs_var.h   Mon Mar  4 14:50:00 2019(r344763)
@@ -673,7 +673,7 @@ int nfsvno_statfs(vnode_t, struct statfs *);
 void nfsvno_getfs(struct nfsfsinfo *, int);
 void nfsvno_open(struct nfsrv_descript *, struct nameidata *, nfsquad_t,
 nfsv4stateid_t *, struct nfsstate *, int *, struct nfsvattr *, int32_t *,
-int, NFSACL_T *, nfsattrbit_t *, struct ucred *, NFSPROC_T *,
+int, NFSACL_T *, nfsattrbit_t *, struct ucred *,
 struct nfsexstuff *, vnode_t *);
 int nfsvno_updfilerev(vnode_t, struct nfsvattr *, struct nfsrv_descript *,
 NFSPROC_T *);

Modified: head/sys/fs/nfsserver/nfs_nfsdport.c
==
--- head/sys/fs/nfsserver/nfs_nfsdport.cMon Mar  4 14:30:53 2019
(r344762)
+++ head/sys/fs/nfsserver/nfs_nfsdport.cMon Mar  4 14:50:00 2019
(r344763)
@@ -1632,12 +1632,13 @@ void
 nfsvno_open(struct nfsrv_descript *nd, struct nameidata *ndp,
 nfsquad_t clientid, nfsv4stateid_t *stateidp, struct nfsstate *stp,
 int *exclusive_flagp, struct nfsvattr *nvap, int32_t *cverf, int create,
-NFSACL_T *aclp, nfsattrbit_t *attrbitp, struct ucred *cred, struct thread 
*p,
+NFSACL_T *aclp, nfsattrbit_t *attrbitp, struct ucred *cred,
 struct nfsexstuff *exp, struct vnode **vpp)
 {
struct vnode *vp = NULL;
u_quad_t tempsize;
struct nfsexstuff nes;
+   struct thread *p = curthread;
 
if (ndp->ni_vp == NULL)
nd->nd_repstat = nfsrv_opencheck(clientid,

Modified: head/sys/fs/nfsserver/nfs_nfsdserv.c
==
--- head/sys/fs/nfsserver/nfs_nfsdserv.cMon Mar  4 14:30:53 2019
(r344762)
+++ head/sys/fs/nfsserver/nfs_nfsdserv.cMon Mar  4 14:50:00 2019
(r344763)
@@ -2991,7 +2991,7 @@ nfsrvd_open(struct nfsrv_descript *nd, __unused int is
}
nfsvno_open(nd, &named, clientid, &stateid, stp,
&exclusive_flag, &nva, cverf, create, aclp, &attrbits,
-   nd->nd_cred, p, exp, &vp);
+   nd->nd_cred, exp, &vp);
} else if (claim == NFSV4OPEN_CLAIMPREVIOUS || claim ==
NFSV4OPEN_CLAIMFH) {
if (claim == NFSV4OPEN_CLAIMPREVIOUS) {
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Edward Napierala]

pon., 4 mar 2019 o 14:30 Konstantin Belousov  napisał(a):
>
> On Mon, Mar 04, 2019 at 01:31:37PM +, Edward Napierala wrote:
> > pon., 4 mar 2019 o 13:20 Konstantin Belousov  
> > napisał(a):
> > > > + p = curthread;
> > > Why do you name it 'p', which is typical for process, and not 'td', you 
> > > are
> > > changing most of the code anyway.
> >
> > To keep the diff size smaller.  You're right, this touches a lot of stuff,
> > but most of those added lines are temporary anyway - they will be
> > removed later, when the td is pushed down even more.
> But if you create code churn, doing it only half way is worse.

But this way I create less churn - if I renamed it to 'td', then first
I'd change
the calls to other functions to take 'td' instead of 'p', and then I'd remove
this argument altogether in subsequent commit.  This would make diffs
larger for no good reason.

> > > Also I am curious why. It is certainly fine to remove td when it is used
> > > as a formal placeholder argument only. But when the first action in the
> > > function is evaluation of curthread() it becomes less obvious.
> >
> > Again, many/most of those are temporary.  I'm trying to push td down
> > in small steps, "layer by layer", so it's easy to review.
> >
> > > curthread() become very cheap on modern amd64, I am not so sure about
> > > older machines or non-x86 cases.
> >
> > The main reason is readability.  Right now there's no easy way to tell 
> > whether
> > a function can be passed any td, or if it must be curthread.
> I must admit that this is the weirdnest argument against 'td' that I ever
> heard.  I saw more or less reasonable argumentation
> - that using less arguments make one more register for argument passing
>   (amd64 has 6 input arg regs),
> - that less arguments make smaller call code.
> But trust me, in all cases where function can take td != curthread, it is
> either obvious or well-known for anybody who works with that code.

Ah, ok.  So, yes, you are right that from a high level point of view
it's a poor argument.  But at this point I'm not trying to change stuff
throughout the kernel; just the NFS server.  And the reason for doing
this is to make it obvious that the 'td' argument it passes to VOPs and
other kernel APIs is always equal to curthread.  Doing it layer by layer
makes it easy to review.

> Before you start doing a lot of small changes (AKA continous churn)
> please formulate your goals and get some public feedback.

I'll do that; I won't go changing kernel APIs like that.  But I'd like to
untangle things that complicate the picture, and the NFS server code
is one of them.

>  My immediate
> question that I want answered before you ever start touching the code,
> is what you plan to do with
> sys_syscall(struct thread *td, uap)

Probably nothing; those things pretty much always actually use the thread
pointer.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Cy Schubert]

On March 4, 2019 6:30:21 AM PST, Konstantin Belousov  
wrote:
>On Mon, Mar 04, 2019 at 01:31:37PM +, Edward Napierala wrote:
>> pon., 4 mar 2019 o 13:20 Konstantin Belousov 
>napisał(a):
>> > > + p = curthread;
>> > Why do you name it 'p', which is typical for process, and not 'td',
>you are
>> > changing most of the code anyway.
>> 
>> To keep the diff size smaller.  You're right, this touches a lot of
>stuff,
>> but most of those added lines are temporary anyway - they will be
>> removed later, when the td is pushed down even more.
>But if you create code churn, doing it only half way is worse.
>
>> 
>> > Also I am curious why. It is certainly fine to remove td when it is
>used
>> > as a formal placeholder argument only. But when the first action in
>the
>> > function is evaluation of curthread() it becomes less obvious.
>> 
>> Again, many/most of those are temporary.  I'm trying to push td down
>> in small steps, "layer by layer", so it's easy to review.
>> 
>> > curthread() become very cheap on modern amd64, I am not so sure
>about
>> > older machines or non-x86 cases.
>> 
>> The main reason is readability.  Right now there's no easy way to
>tell whether
>> a function can be passed any td, or if it must be curthread.
>I must admit that this is the weirdnest argument against 'td' that I
>ever
>heard.  I saw more or less reasonable argumentation
>- that using less arguments make one more register for argument passing
>  (amd64 has 6 input arg regs),
>- that less arguments make smaller call code.
>But trust me, in all cases where function can take td != curthread, it
>is
>either obvious or well-known for anybody who works with that code.
>
>Before you start doing a lot of small changes (AKA continous churn)
>please formulate your goals and get some public feedback.  My immediate
>question that I want answered before you ever start touching the code,
>is what you plan to do with
>   sys_syscall(struct thread *td, uap)

Agreed on all points. At the very least this group of commits should be 
reviewed on phabricator.

Can we back all these commits out until there is a proper review, please?

-- 
Pardon the typos and autocorrect, small keyboard in use.
Cheers,
Cy Schubert 
FreeBSD UNIX:  Web: http://www.FreeBSD.org

The need of the many outweighs the greed of the few.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344764 - in head/tests/sys: . common netipsec/tunnel netpfil/pf

[Kristof Provost]

Author: kp
Date: Mon Mar  4 18:15:06 2019
New Revision: 344764
URL: https://svnweb.freebsd.org/changeset/base/344764

Log:
  tests: Move common (vnet) test functions into a common file
  
  The netipsec and pf tests have a number of common test functions. These
  used to be duplicated, but it makes more sense for them to re-use the
  common functions.
  
  PR:   236223

Added:
  head/tests/sys/common/
  head/tests/sys/common/Makefile   (contents, props changed)
  head/tests/sys/common/vnet.subr   (contents, props changed)
Modified:
  head/tests/sys/Makefile
  head/tests/sys/netipsec/tunnel/utils.subr
  head/tests/sys/netpfil/pf/anchor.sh
  head/tests/sys/netpfil/pf/forward.sh
  head/tests/sys/netpfil/pf/fragmentation.sh
  head/tests/sys/netpfil/pf/names.sh
  head/tests/sys/netpfil/pf/nat.sh
  head/tests/sys/netpfil/pf/pass_block.sh
  head/tests/sys/netpfil/pf/pfsync.sh
  head/tests/sys/netpfil/pf/rdr.sh
  head/tests/sys/netpfil/pf/route_to.sh
  head/tests/sys/netpfil/pf/set_skip.sh
  head/tests/sys/netpfil/pf/set_tos.sh
  head/tests/sys/netpfil/pf/synproxy.sh
  head/tests/sys/netpfil/pf/utils.subr

Modified: head/tests/sys/Makefile
==
--- head/tests/sys/Makefile Mon Mar  4 14:50:00 2019(r344763)
+++ head/tests/sys/Makefile Mon Mar  4 18:15:06 2019(r344764)
@@ -39,4 +39,6 @@ _cddl=cddl
 # Items not integrated into kyua runs by default
 SUBDIR+=   pjdfstest
 
+SUBDIR+=   common
+
 .include 

Added: head/tests/sys/common/Makefile
==
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/tests/sys/common/Makefile  Mon Mar  4 18:15:06 2019
(r344764)
@@ -0,0 +1,7 @@
+# $FreeBSD$
+
+PACKAGE=   common
+TESTSDIR=  ${TESTSBASE}/sys/common
+${PACKAGE}FILES+=  vnet.subr
+
+.include 

Added: head/tests/sys/common/vnet.subr
==
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/tests/sys/common/vnet.subr Mon Mar  4 18:15:06 2019
(r344764)
@@ -0,0 +1,51 @@
+# $FreeBSD$
+# VNAT/jail utility functions
+##
+
+vnet_init()
+{
+   if [ "`sysctl -i -n kern.features.vimage`" != 1 ]; then
+   atf_skip "This test requires VIMAGE"
+   fi
+}
+
+vnet_mkepair()
+{
+   ifname=$(ifconfig epair create)
+   echo $ifname >> created_interfaces.lst
+   echo ${ifname%a}
+}
+
+vnet_mkjail()
+{
+   jailname=$1
+   shift
+
+   vnet_interfaces=
+   for ifname in $@
+   do
+   vnet_interfaces="${vnet_interfaces} vnet.interface=${ifname}"
+   done
+   jail -c name=${jailname} persist vnet ${vnet_interfaces}
+
+   echo $jailname >> created_jails.lst
+}
+
+vnet_cleanup()
+{
+   if [ -f created_jails.lst ]; then
+   for jailname in `cat created_jails.lst`
+   do
+   jail -r ${jailname}
+   done
+   rm created_jails.lst
+   fi
+
+   if [ -f created_interfaces.lst ]; then
+   for ifname in `cat created_interfaces.lst`
+   do
+   ifconfig ${ifname} destroy
+   done
+   rm created_interfaces.lst
+   fi
+}

Modified: head/tests/sys/netipsec/tunnel/utils.subr
==
--- head/tests/sys/netipsec/tunnel/utils.subr   Mon Mar  4 14:50:00 2019
(r344763)
+++ head/tests/sys/netipsec/tunnel/utils.subr   Mon Mar  4 18:15:06 2019
(r344764)
@@ -4,51 +4,29 @@
 
 : ${TMPDIR=/tmp}
 
+. $(atf_get_srcdir)/../../common/vnet.subr
+
 ist_init()
 {
-   if [ "$(sysctl -i -n kern.features.vimage)" != 1 ]; then
-   atf_skip "This test requires VIMAGE"
-   fi
+   vnet_init
 }
 
-pft_mkepair()
-{
-   ifname=$(ifconfig epair create)
-   echo $ifname >> created_interfaces.lst
-   echo ${ifname%a}
-}
-
-pft_mkjail()
-{
-   jailname=$1
-   shift
-
-   vnet_interfaces=
-   for ifname in $@
-   do
-   vnet_interfaces="${vnet_interfaces} vnet.interface=${ifname}"
-   done
-   jail -c name=${jailname} persist vnet ${vnet_interfaces}
-
-   echo $jailname >> created_jails.lst
-}
-
 ist_labsetup ()
 {
-   epair_LAN_A=$(pft_mkepair)
+   epair_LAN_A=$(vnet_mkepair)
ifconfig ${epair_LAN_A}a up
-   epair_PUB_A=$(pft_mkepair)
+   epair_PUB_A=$(vnet_mkepair)
ifconfig ${epair_PUB_A}a up
-   epair_LAN_B=$(pft_mkepair)
+   epair_LAN_B=$(vnet_mkepair)
ifconfig ${epair_LAN_B}a up
-   epair_PUB_B=$(pft_mkepair)
+   epair_PUB_B=$(vnet_mkepair)
ifconfig ${epair_PUB_B}a up
 
-   pft_mkjail hostA ${epair_LAN_A}a
-   pft_mkjail ipsecA ${epair_LAN_A}b ${epair_PUB_A}a

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Edward Napierala]

pon., 4 mar 2019 o 15:17 Cy Schubert  napisał(a):
>
> On March 4, 2019 6:30:21 AM PST, Konstantin Belousov  
> wrote:
> >On Mon, Mar 04, 2019 at 01:31:37PM +, Edward Napierala wrote:
> >> pon., 4 mar 2019 o 13:20 Konstantin Belousov 
> >napisał(a):
> >> > > + p = curthread;
> >> > Why do you name it 'p', which is typical for process, and not 'td',
> >you are
> >> > changing most of the code anyway.
> >>
> >> To keep the diff size smaller.  You're right, this touches a lot of
> >stuff,
> >> but most of those added lines are temporary anyway - they will be
> >> removed later, when the td is pushed down even more.
> >But if you create code churn, doing it only half way is worse.
> >
> >>
> >> > Also I am curious why. It is certainly fine to remove td when it is
> >used
> >> > as a formal placeholder argument only. But when the first action in
> >the
> >> > function is evaluation of curthread() it becomes less obvious.
> >>
> >> Again, many/most of those are temporary.  I'm trying to push td down
> >> in small steps, "layer by layer", so it's easy to review.
> >>
> >> > curthread() become very cheap on modern amd64, I am not so sure
> >about
> >> > older machines or non-x86 cases.
> >>
> >> The main reason is readability.  Right now there's no easy way to
> >tell whether
> >> a function can be passed any td, or if it must be curthread.
> >I must admit that this is the weirdnest argument against 'td' that I
> >ever
> >heard.  I saw more or less reasonable argumentation
> >- that using less arguments make one more register for argument passing
> >  (amd64 has 6 input arg regs),
> >- that less arguments make smaller call code.
> >But trust me, in all cases where function can take td != curthread, it
> >is
> >either obvious or well-known for anybody who works with that code.
> >
> >Before you start doing a lot of small changes (AKA continous churn)
> >please formulate your goals and get some public feedback.  My immediate
> >question that I want answered before you ever start touching the code,
> >is what you plan to do with
> >   sys_syscall(struct thread *td, uap)
>
> Agreed on all points. At the very least this group of commits should be 
> reviewed on phabricator.

It has been, even though they are pretty much mechanical changes.

> Can we back all these commits out until there is a proper review, please?

The review from the NFS maintainer is not enough?
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344780 - head/stand/common

[Simon J. Gerraty]

Author: sjg
Date: Mon Mar  4 19:50:59 2019
New Revision: 344780
URL: https://svnweb.freebsd.org/changeset/base/344780

Log:
  Add -d flag to load command
  
  When doing load -t manifest -d increments debug level
  
  Reviewed by:  stevek

Modified:
  head/stand/common/module.c

Modified: head/stand/common/module.c
==
--- head/stand/common/module.c  Mon Mar  4 19:39:59 2019(r344779)
+++ head/stand/common/module.c  Mon Mar  4 19:50:59 2019(r344780)
@@ -106,9 +106,9 @@ command_load(int argc, char *argv[])
 char   *typestr;
 char   *prefix;
 char   *skip;
-intdofile, dokld, ch, error;
+intdflag, dofile, dokld, ch, error;
 
-dokld = dofile = 0;
+dflag = dokld = dofile = 0;
 optind = 1;
 optreset = 1;
 typestr = NULL;
@@ -117,8 +117,11 @@ command_load(int argc, char *argv[])
return (CMD_CRIT);
 }
 prefix = skip = NULL;
-while ((ch = getopt(argc, argv, "kp:s:t:")) != -1) {
+while ((ch = getopt(argc, argv, "dkp:s:t:")) != -1) {
switch(ch) {
+   case 'd':
+   dflag++;
+   break;
case 'k':
dokld = 1;
break;
@@ -152,6 +155,8 @@ command_load(int argc, char *argv[])
 
 #ifdef LOADER_VERIEXEC
if (strncmp(typestr, "manifest", 8) == 0) {
+   if (dflag > 0)
+   ve_debug_set(dflag);
return (load_manifest(argv[1], prefix, skip, NULL));
}
 #endif
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Cy Schubert]

In message 
, Edward Napierala writes:
> pon., 4 mar 2019 o 15:17 Cy Schubert  napisał(a):
> >
> > On March 4, 2019 6:30:21 AM PST, Konstantin Belousov  
> wrote:
> > >On Mon, Mar 04, 2019 at 01:31:37PM +, Edward Napierala wrote:
> > >> pon., 4 mar 2019 o 13:20 Konstantin Belousov 
> > >napisał(a):
> > >> > > + p = curthread;
> > >> > Why do you name it 'p', which is typical for process, and not 'td',
> > >you are
> > >> > changing most of the code anyway.
> > >>
> > >> To keep the diff size smaller.  You're right, this touches a lot of
> > >stuff,
> > >> but most of those added lines are temporary anyway - they will be
> > >> removed later, when the td is pushed down even more.
> > >But if you create code churn, doing it only half way is worse.
> > >
> > >>
> > >> > Also I am curious why. It is certainly fine to remove td when it is
> > >used
> > >> > as a formal placeholder argument only. But when the first action in
> > >the
> > >> > function is evaluation of curthread() it becomes less obvious.
> > >>
> > >> Again, many/most of those are temporary.  I'm trying to push td down
> > >> in small steps, "layer by layer", so it's easy to review.
> > >>
> > >> > curthread() become very cheap on modern amd64, I am not so sure
> > >about
> > >> > older machines or non-x86 cases.
> > >>
> > >> The main reason is readability.  Right now there's no easy way to
> > >tell whether
> > >> a function can be passed any td, or if it must be curthread.
> > >I must admit that this is the weirdnest argument against 'td' that I
> > >ever
> > >heard.  I saw more or less reasonable argumentation
> > >- that using less arguments make one more register for argument passing
> > >  (amd64 has 6 input arg regs),
> > >- that less arguments make smaller call code.
> > >But trust me, in all cases where function can take td != curthread, it
> > >is
> > >either obvious or well-known for anybody who works with that code.
> > >
> > >Before you start doing a lot of small changes (AKA continous churn)
> > >please formulate your goals and get some public feedback.  My immediate
> > >question that I want answered before you ever start touching the code,
> > >is what you plan to do with
> > >   sys_syscall(struct thread *td, uap)
> >
> > Agreed on all points. At the very least this group of commits should be rev
> iewed on phabricator.
>
> It has been, even though they are pretty much mechanical changes.
>
> > Can we back all these commits out until there is a proper review, please?
>
> The review from the NFS maintainer is not enough?
>

As it's NFS-only maybe though for anything substantial, like this, the 
more eyes the better.

I'd agree with kib@ that we want to keep the amount of churn down, 
though it's understood that you want to separate the functional changes 
from the cosmetic ones. I tend to do the review and use git svn to 
separate the functional from the cosmetic changes, batching changes if 
I can. It's more work but IMO well worth it.


-- 
Cheers,
Cy Schubert 
FreeBSD UNIX: Web:  http://www.FreeBSD.org

The need of the many outweighs the greed of the few.


___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344782 - head/sys/net

[Alexander Motin]

Author: mav
Date: Mon Mar  4 22:01:09 2019
New Revision: 344782
URL: https://svnweb.freebsd.org/changeset/base/344782

Log:
  bridge: Fix spurious warnings about capabilities
  
  Mask off the bits we don't care about when checking that capabilities
  of the member interfaces have been disabled as intended.
  
  Submitted by: Ryan Moeller 
  Reviewed by:  kristof, mav
  MFC after:1 week
  Sponsored by: iXsystems, Inc.
  Differential Revision:https://reviews.freebsd.org/D18924

Modified:
  head/sys/net/if_bridge.c

Modified: head/sys/net/if_bridge.c
==
--- head/sys/net/if_bridge.cMon Mar  4 21:10:40 2019(r344781)
+++ head/sys/net/if_bridge.cMon Mar  4 22:01:09 2019(r344782)
@@ -925,7 +925,7 @@ bridge_set_ifcap(struct bridge_softc *sc, struct bridg
 {
struct ifnet *ifp = bif->bif_ifp;
struct ifreq ifr;
-   int error;
+   int error, mask, stuck;
 
BRIDGE_UNLOCK_ASSERT(sc);
 
@@ -938,10 +938,12 @@ bridge_set_ifcap(struct bridge_softc *sc, struct bridg
if_printf(sc->sc_ifp,
"error setting capabilities on %s: %d\n",
ifp->if_xname, error);
-   if ((ifp->if_capenable & ~set) != 0)
+   mask = BRIDGE_IFCAPS_MASK | BRIDGE_IFCAPS_STRIP;
+   stuck = ifp->if_capenable & mask & ~set;
+   if (stuck != 0)
if_printf(sc->sc_ifp,
"can't disable some capabilities on %s: 0x%x\n",
-   ifp->if_xname, ifp->if_capenable & ~set);
+   ifp->if_xname, stuck);
}
 }
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344784 - in head/lib/libsecureboot: . tests

[Simon J. Gerraty]

Author: sjg
Date: Mon Mar  4 22:04:21 2019
New Revision: 344784
URL: https://svnweb.freebsd.org/changeset/base/344784

Log:
  Allow for reproducible build
  
  Use SOURCE_DATE_EPOCH for BUILD_UTC if MK_REPRODUCIBLE_BUILD is yes.
  Default SOURCE_DATE_EPOCH to 2019-01-01
  
  Reviewed by:  emaste
  Sponsored by: Juniper Networks
  Differential Revision:https://reviews.freebsd.org/D19464

Modified:
  head/lib/libsecureboot/Makefile.inc
  head/lib/libsecureboot/tests/Makefile

Modified: head/lib/libsecureboot/Makefile.inc
==
--- head/lib/libsecureboot/Makefile.inc Mon Mar  4 22:03:09 2019
(r344783)
+++ head/lib/libsecureboot/Makefile.inc Mon Mar  4 22:04:21 2019
(r344784)
@@ -92,6 +92,19 @@ VE_HASH_KAT_STR?= vc_PEM
 XCFLAGS.vets+= -DVE_HASH_KAT_STR=${VE_HASH_KAT_STR}
 .endif
 
+# this should be updated occassionally this is 2019-01-01Z
+SOURCE_DATE_EPOCH?= 1546329600
+.if ${MK_REPRODUCIBLE_BUILD} == "yes"
+BUILD_UTC?= ${SOURCE_DATE_EPOCH}
+.endif
+# BUILD_UTC provides a basis for the loader's notion of time
+# By default we use the mtime of BUILD_UTC_FILE
+.if empty(BUILD_UTC_FILE)
+BUILD_UTC_FILE:= ${.PARSEDIR:tA}/${.PARSEFILE}
+.endif
+# you can of course set BUILD_UTC to any value you like
+BUILD_UTC?= ${${STAT:Ustat} -f %m ${BUILD_UTC_FILE}:L:sh}
+
 # Generate ta.h containing one or more PEM encoded trust anchors in ta_PEM.
 #
 # If we are doing self-tests, we define another arrary vc_PEM
@@ -110,9 +123,7 @@ ta.h: ${.ALLTARGETS:M[tv]*pem:O:u}
( cat ${.ALLSRC:N*crl*:Mv*.pem} /dev/null | \
file2c -sx 'static const char vc_PEM[] = {' '};'; echo ) >> ${.TARGET}
 .endif
-.if !empty(BUILD_UTC_FILE)
-   echo '#define BUILD_UTC ${${STAT:Ustat} -f %m ${BUILD_UTC_FILE}:L:sh}' 
>> ${.TARGET} ${.OODATE:MNOMETA_CMP}
-.endif
+   echo '#define BUILD_UTC ${BUILD_UTC}' >> ${.TARGET} 
${.OODATE:MNOMETA_CMP}
 
 # This header records our preference for signature extensions.
 vesigned.o vesigned.po vesigned.pico: vse.h

Modified: head/lib/libsecureboot/tests/Makefile
==
--- head/lib/libsecureboot/tests/Makefile   Mon Mar  4 22:03:09 2019
(r344783)
+++ head/lib/libsecureboot/tests/Makefile   Mon Mar  4 22:04:21 2019
(r344784)
@@ -1,5 +1,7 @@
 # $FreeBSD$
 
+.include 
+
 PROG= tvo
 
 SRCS+= tvo.c
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Rick Macklem]

Cy Schubert wrote:
>Sent: Monday, March 4, 2019 3:25 PM
>To: Edward Napierala
>Cc: Cy Schubert; Konstantin Belousov; src-committers; svn-src-...@freebsd.org; 
>svn-src->h...@freebsd.org
>Subject: Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver
>
>In message 
, Edward Napierala writes:
>> pon., 4 mar 2019 o 15:17 Cy Schubert  napisał(a):
>> >
>> > On March 4, 2019 6:30:21 AM PST, Konstantin Belousov 
>> wrote:
>> > >On Mon, Mar 04, 2019 at 01:31:37PM +, Edward Napierala wrote:
>> > >> pon., 4 mar 2019 o 13:20 Konstantin Belousov 
>> > >napisał(a):
>> > >> > > + p = curthread;
>> > >> > Why do you name it 'p', which is typical for process, and not 'td',
>> > >you are
>> > >> > changing most of the code anyway.
>> > >>
>> > >> To keep the diff size smaller.  You're right, this touches a lot of
>> > >stuff,
>> > >> but most of those added lines are temporary anyway - they will be
>> > >> removed later, when the td is pushed down even more.
>> > >But if you create code churn, doing it only half way is worse.
>> > >
>> > >>
>> > >> > Also I am curious why. It is certainly fine to remove td when it is
>> > >used
>> > >> > as a formal placeholder argument only. But when the first action in
>> > >the
>> > >> > function is evaluation of curthread() it becomes less obvious.
>> > >>
>> > >> Again, many/most of those are temporary.  I'm trying to push td down
>> > >> in small steps, "layer by layer", so it's easy to review.
>> > >>
>> > >> > curthread() become very cheap on modern amd64, I am not so sure
>> > >about
>> > >> > older machines or non-x86 cases.
>> > >>
>> > >> The main reason is readability.  Right now there's no easy way to
>> > >tell whether
>> > >> a function can be passed any td, or if it must be curthread.
>> > >I must admit that this is the weirdnest argument against 'td' that I
>> > >ever
>> > >heard.  I saw more or less reasonable argumentation
>> > >- that using less arguments make one more register for argument passing
>> > >  (amd64 has 6 input arg regs),
>> > >- that less arguments make smaller call code.
>> > >But trust me, in all cases where function can take td != curthread, it
>> > >is
>> > >either obvious or well-known for anybody who works with that code.
>> > >
>> > >Before you start doing a lot of small changes (AKA continous churn)
>> > >please formulate your goals and get some public feedback.  My immediate
>> > >question that I want answered before you ever start touching the code,
>> > >is what you plan to do with
>> > >   sys_syscall(struct thread *td, uap)
>> >
>> > Agreed on all points. At the very least this group of commits should be rev
>> iewed on phabricator.
>>
>> It has been, even though they are pretty much mechanical changes.
>>
>> > Can we back all these commits out until there is a proper review, please?
>>
>> The review from the NFS maintainer is not enough?
>>
Btw, I've never listed myself as the NFS maintainer. I need to go look to see if
someone else put me in the maintainer's list. I understand that it is mostly 
authored
by me, but I consider it FreeBSD project code once committed. (Others do commits
to it without my review and that is just fine with me.)

>As it's NFS-only maybe though for anything substantial, like this, the
>more eyes the better.
>
>I'd agree with kib@ that we want to keep the amount of churn down,
>though it's understood that you want to separate the functional changes
>from the cosmetic ones. I tend to do the review and use git svn to
>separate the functional from the cosmetic changes, batching changes if
>I can. It's more work but IMO well worth it.
This is way too technical for me. I can barely look at a "diff" and make sense 
of it.;-)

It sounds like there needs to be a discussion (on freebsd-fs@ perhaps?) of the
"big picture change" here.

All I am going to do with the patches in phabricator is take a quick look to see
if I can spot anything that will break the code.
(I did mention that I didn't understand why he was doing this in one of the 
reviews,
 but noted that it didn't break anything.)

Oh, and the variable was called "p" because the code started in OpenBSD, where 
it
was a proc ptr and I kept it portable by replacing "struct proc" with NFSPROC_T.
(This portability is no longer a consideration.)

I'll hold off on further phabricator reviews until the "big picture" change 
gets discussed
on some mailing list. (I don't see phabricator as the correct tool for "big 
picture"
discussions.)

rick



___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver

[Cy Schubert]

On March 4, 2019 3:13:05 PM PST, Rick Macklem  wrote:
>Cy Schubert wrote:
>>Sent: Monday, March 4, 2019 3:25 PM
>>To: Edward Napierala
>>Cc: Cy Schubert; Konstantin Belousov; src-committers;
>svn-src-...@freebsd.org; svn-src->h...@freebsd.org
>>Subject: Re: svn commit: r344758 - in head/sys/fs: nfs nfsserver
>>
>>In message
>il.com>
>, Edward Napierala writes:
>>> pon., 4 mar 2019 o 15:17 Cy Schubert 
>napisał(a):
>>> >
>>> > On March 4, 2019 6:30:21 AM PST, Konstantin Belousov
>
>>> wrote:
>>> > >On Mon, Mar 04, 2019 at 01:31:37PM +, Edward Napierala wrote:
>>> > >> pon., 4 mar 2019 o 13:20 Konstantin Belousov
>
>>> > >napisał(a):
>>> > >> > > + p = curthread;
>>> > >> > Why do you name it 'p', which is typical for process, and not
>'td',
>>> > >you are
>>> > >> > changing most of the code anyway.
>>> > >>
>>> > >> To keep the diff size smaller.  You're right, this touches a
>lot of
>>> > >stuff,
>>> > >> but most of those added lines are temporary anyway - they will
>be
>>> > >> removed later, when the td is pushed down even more.
>>> > >But if you create code churn, doing it only half way is worse.
>>> > >
>>> > >>
>>> > >> > Also I am curious why. It is certainly fine to remove td when
>it is
>>> > >used
>>> > >> > as a formal placeholder argument only. But when the first
>action in
>>> > >the
>>> > >> > function is evaluation of curthread() it becomes less
>obvious.
>>> > >>
>>> > >> Again, many/most of those are temporary.  I'm trying to push td
>down
>>> > >> in small steps, "layer by layer", so it's easy to review.
>>> > >>
>>> > >> > curthread() become very cheap on modern amd64, I am not so
>sure
>>> > >about
>>> > >> > older machines or non-x86 cases.
>>> > >>
>>> > >> The main reason is readability.  Right now there's no easy way
>to
>>> > >tell whether
>>> > >> a function can be passed any td, or if it must be curthread.
>>> > >I must admit that this is the weirdnest argument against 'td'
>that I
>>> > >ever
>>> > >heard.  I saw more or less reasonable argumentation
>>> > >- that using less arguments make one more register for argument
>passing
>>> > >  (amd64 has 6 input arg regs),
>>> > >- that less arguments make smaller call code.
>>> > >But trust me, in all cases where function can take td !=
>curthread, it
>>> > >is
>>> > >either obvious or well-known for anybody who works with that
>code.
>>> > >
>>> > >Before you start doing a lot of small changes (AKA continous
>churn)
>>> > >please formulate your goals and get some public feedback.  My
>immediate
>>> > >question that I want answered before you ever start touching the
>code,
>>> > >is what you plan to do with
>>> > >   sys_syscall(struct thread *td, uap)
>>> >
>>> > Agreed on all points. At the very least this group of commits
>should be rev
>>> iewed on phabricator.
>>>
>>> It has been, even though they are pretty much mechanical changes.
>>>
>>> > Can we back all these commits out until there is a proper review,
>please?
>>>
>>> The review from the NFS maintainer is not enough?
>>>
>Btw, I've never listed myself as the NFS maintainer. I need to go look
>to see if
>someone else put me in the maintainer's list. I understand that it is
>mostly authored
>by me, but I consider it FreeBSD project code once committed. (Others
>do commits
>to it without my review and that is just fine with me.)
>
>>As it's NFS-only maybe though for anything substantial, like this, the
>>more eyes the better.
>>
>>I'd agree with kib@ that we want to keep the amount of churn down,
>>though it's understood that you want to separate the functional
>changes
>>from the cosmetic ones. I tend to do the review and use git svn to
>>separate the functional from the cosmetic changes, batching changes if
>>I can. It's more work but IMO well worth it.
>This is way too technical for me. I can barely look at a "diff" and
>make sense of it.;-)
>
>It sounds like there needs to be a discussion (on freebsd-fs@ perhaps?)
>of the
>"big picture change" here.
>
>All I am going to do with the patches in phabricator is take a quick
>look to see
>if I can spot anything that will break the code.
>(I did mention that I didn't understand why he was doing this in one of
>the reviews,
> but noted that it didn't break anything.)

IMO the why is always more important than the how. If there is no reason why 
then how is irrelevant.

>
>Oh, and the variable was called "p" because the code started in
>OpenBSD, where it
>was a proc ptr and I kept it portable by replacing "struct proc" with
>NFSPROC_T.
>(This portability is no longer a consideration.)
>
>I'll hold off on further phabricator reviews until the "big picture"
>change gets discussed
>on some mailing list. (I don't see phabricator as the correct tool for
>"big picture"
>discussions.)
>
>rick

Hopefully my inline reply on this phone worked. If not I'll try again tonight.

-- 
Pardon the typos and autocorrect, small keyboard in use.
Cheers,
Cy Schubert 
FreeBSD UNIX:  Web: http://www.FreeBSD.org

The need of the ma

svn commit: r344788 - head/usr.sbin/rtadvctl

[Hiroki Sato]

Author: hrs
Date: Tue Mar  5 02:53:41 2019
New Revision: 344788
URL: https://svnweb.freebsd.org/changeset/base/344788

Log:
  Fix grammar.
  
  Submitted by: Massimiliano Stucchi

Modified:
  head/usr.sbin/rtadvctl/rtadvctl.8

Modified: head/usr.sbin/rtadvctl/rtadvctl.8
==
--- head/usr.sbin/rtadvctl/rtadvctl.8   Tue Mar  5 01:00:38 2019
(r344787)
+++ head/usr.sbin/rtadvctl/rtadvctl.8   Tue Mar  5 02:53:41 2019
(r344788)
@@ -40,7 +40,7 @@ daemon
 .Op Ar interface ...
 .Sh DESCRIPTION
 .Nm
-is a utility that communicates with
+is a utility that communicates with the
 .Xr rtadvd 8
 daemon and displays information about Router Advertisement messages being
 sent on each interface.
@@ -63,21 +63,22 @@ The subcommands are as follows:
 .Bl -tag -width indent
 .\"
 .It reload Op interfaces...
-Specifies to reload the configuration file.  If one or more
-.Ar interface
-is specified, configuration entries for the interfaces will be reloaded
+Specifies to reload the configuration file.
+If one or more
+.Ar interfaces
+are specified, configuration entries for the interfaces will be reloaded
 selectively.
 .It enable interfaces...
-Specifies to mark the interface as enable and to try to reload the
+Specifies to mark the interface as enabled and to tries to reload the
 configuration entry.
 This subcommand is useful for dynamically-added interfaces.
 .Pp
 The
 .Xr rtadvd 8
-daemon marks an interface as enable if the interface exists and the
-configuration file has a valid entry for that when it is invoked.
+daemon marks an interface as enabled if the interface exists and the
+configuration file has a valid entry for it when it is invoked.
 .It disable interfaces...
-Specifies to mark the interface as disable.
+Specifies to mark the interface as disabled.
 .It shutdown
 Makes the
 .Xr rtadvd 8
@@ -88,7 +89,7 @@ daemon will send several RAs with zero lifetime to inv
 information on each interface.
 It will take at most nine seconds.
 .It show Op interfaces...
-Displays information on Router Advertisement messages being sent
+Displays information about the Router Advertisement messages being sent
 on each interface.
 .El
 .Sh SEE ALSO
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344790 - head/sys/geom/part

[Marcel Moolenaar]

Author: marcel
Date: Tue Mar  5 04:15:34 2019
New Revision: 344790
URL: https://svnweb.freebsd.org/changeset/base/344790

Log:
  Revert revision 254095
  
  In revision 254095, gpt_entries is not set to match the on-disk
  hdr_entries, but rather is computed based on available space.
  There are 2 problems with this:
  
  1.  The GPT backend respects hdr_entries and only reads and writes
  that number of partition entries.  On top of that, CRC32 is
  computed over the table that has hdr_entries elements.  When
  the common code works on what is possibly a larger number, the
  behaviour becomes inconsistent and problematic.  In particular,
  it would be possible to add a new partition that on a reboot
  isn't there anymore.
  2.  The calculation of gpt_entries is based on flawed assumptions.
  The GPT specification does not dictate that sectors are layed
  out in a particular way that the available space can be
  determined by looking at LBAs.  In practice, implementations
  do the same thing, because there's no reason to do it any
  other way.  Still, GPT allows certain freedoms that can be
  exploited in some form or shape if the need arises.
  
  PR:   229977
  MFC after:2 weeks
  Differential Revision:https://reviews.freebsd.org/D19438

Modified:
  head/sys/geom/part/g_part_gpt.c

Modified: head/sys/geom/part/g_part_gpt.c
==
--- head/sys/geom/part/g_part_gpt.c Tue Mar  5 03:27:32 2019
(r344789)
+++ head/sys/geom/part/g_part_gpt.c Tue Mar  5 04:15:34 2019
(r344790)
@@ -990,10 +990,9 @@ g_part_gpt_read(struct g_part_table *basetable, struct
 
basetable->gpt_first = table->hdr->hdr_lba_start;
basetable->gpt_last = table->hdr->hdr_lba_end;
-   basetable->gpt_entries = (table->hdr->hdr_lba_start - 2) *
-   pp->sectorsize / table->hdr->hdr_entsz;
+   basetable->gpt_entries = table->hdr->hdr_entries;
 
-   for (index = table->hdr->hdr_entries - 1; index >= 0; index--) {
+   for (index = basetable->gpt_entries - 1; index >= 0; index--) {
if (EQUUID(&tbl[index].ent_type, &gpt_uuid_unused))
continue;
entry = (struct g_part_gpt_entry *)g_part_new_entry(
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r344791 - head/contrib/binutils/opcodes

[Justin Hibbits]

Author: jhibbits
Date: Tue Mar  5 04:16:50 2019
New Revision: 344791
URL: https://svnweb.freebsd.org/changeset/base/344791

Log:
  Fix binutils compilation error with Clang 8
  
  Summary:
  This change fixes the following compilation error when using clang 8 to cross
  compile base to powerpc64:
  
  ```
  
/usr/src/gnu/usr.bin/binutils/libopcodes/../../../../contrib/binutils/opcodes/ppc-dis.c:100:35:
  error: arithmetic on a null pointer treated as a cast from integer to pointer 
is
  a GNU extension [-Werror,-Wnull-pointer-arithmetic]
info->private_data = (char *) 0 + dialect;
   ~~ ^
  1 error generated.
  *** [ppc-dis.o] Error code 1
  
  make[6]: stopped in /usr/src/gnu/usr.bin/binutils/libopcodes
  1 error
  ```
  
  Test Plan:
  - buildworld for x86_64 (native)
  - buildworld for powerpc64 (cross)
  - buildworld for powerpc64 (native)
  
  Submitted by: alfredo.junior_eldorado.org.br
  Reviewed By:  emaste, pfg, brooks
  Differential Revision:https://reviews.freebsd.org/D19235

Modified:
  head/contrib/binutils/opcodes/ppc-dis.c

Modified: head/contrib/binutils/opcodes/ppc-dis.c
==
--- head/contrib/binutils/opcodes/ppc-dis.c Tue Mar  5 04:15:34 2019
(r344790)
+++ head/contrib/binutils/opcodes/ppc-dis.c Tue Mar  5 04:16:50 2019
(r344791)
@@ -20,6 +20,7 @@ along with this file; see the file COPYING.  If not, w
 Software Foundation, 51 Franklin Street - Fifth Floor, Boston, MA 02110-1301, 
USA.  */
 
 #include 
+#include 
 #include "sysdep.h"
 #include "dis-asm.h"
 #include "opcode/ppc.h"
@@ -97,7 +98,7 @@ powerpc_dialect (struct disassemble_info *info)
dialect |= PPC_OPCODE_64;
 }
 
-  info->private_data = (char *) 0 + dialect;
+  info->private_data = (void *)(uintptr_t)dialect;
   return dialect;
 }
 
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r344487 - in head/sys: conf gnu/gcov

[Matthew Macy]

World? It looks like there's a version mismatch between the kernel and modules.

-M

On Thu, Feb 28, 2019 at 7:11 PM Alan Somers  wrote:
>
> On Thu, Feb 28, 2019 at 6:40 PM Matthew Macy  wrote:
> >
> > to config add:
> > options LINDEBUGFS
> > options GCOV
> >
> > compile kernel with gcc (otherwise it will be a no-op)
> >
> > sysctl debug.gcov.enable=1
> >
> > mount -t debugfs debugfs /sys/kernel/debug
> >
> > (or wherever) and the output artifacts will appear under gcov/ > build path> - you need to be root to see the artifacts
> >
> > gcov can then generate the results as it would normally from the
> > profiling and the build time artifacts
> >
> > bug reports welcome
> >
> >
> > -M
>
> Here's a bug report: I rebuilt the kernel but boot fails with this
> error.  Do I need to rebuild world too?
>
> Mounting local filesystems:kldload: fdescfs.ko: lost base for reltab
> linker_load_file: /boot/kernel/fdescfs.ko - unsupported file type
> mount: fdescfs: Operation not supported by device
> Mounting /etc/fstab filesystems failed, will retry after root mount hold 
> release
> kldload: fdescfs.ko: lost base for reltab
> linker_load_file: /boot/kernel/fdescfs.ko - unsupported file type
> mount: fdescfs: Operation not supported by device
> .
> Mounting /etc/fstab filesystems failed, startup aborted
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"