ufs free()

[David Hill]

Hello -

This diff adds sizes to free(), which completes ufs/ffs.

OK?

Index: ufs/ffs/ffs_inode.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_inode.c,v
retrieving revision 1.76
diff -u -p -r1.76 ffs_inode.c
--- ufs/ffs/ffs_inode.c 27 Feb 2016 18:50:38 -  1.76
+++ ufs/ffs/ffs_inode.c 29 Mar 2018 02:55:36 -
@@ -560,7 +560,7 @@ ffs_indirtrunc(struct inode *ip, daddr_t
}
}
if (copy != NULL) {
-   free(copy, M_TEMP, 0);
+   free(copy, M_TEMP, fs->fs_bsize);
} else {
bp->b_flags |= B_INVAL;
brelse(bp);
Index: ufs/ffs/ffs_softdep.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_softdep.c,v
retrieving revision 1.138
diff -u -p -r1.138 ffs_softdep.c
--- ufs/ffs/ffs_softdep.c   10 Feb 2018 05:24:23 -  1.138
+++ ufs/ffs/ffs_softdep.c   29 Mar 2018 02:55:37 -
@@ -2307,7 +2307,8 @@ check_inode_unwritten(struct inodedep *i
if (inodedep->id_state & ONWORKLIST)
WORKLIST_REMOVE(>id_list);
if (inodedep->id_savedino1 != NULL) {
-   free(inodedep->id_savedino1, M_INODEDEP, 0);
+   free(inodedep->id_savedino1, M_INODEDEP,
+   sizeof(struct ufs1_dinode));
inodedep->id_savedino1 = NULL;
}
if (free_inodedep(inodedep) == 0) {
@@ -3845,7 +3846,8 @@ softdep_disk_write_complete(struct buf *
if (indirdep->ir_state & GOINGAWAY)
panic("disk_write_complete: indirdep gone");
memcpy(bp->b_data, indirdep->ir_saveddata, 
bp->b_bcount);
-   free(indirdep->ir_saveddata, M_INDIRDEP, 0);
+   free(indirdep->ir_saveddata, M_INDIRDEP,
+   sizeof(struct ufs1_dinode));
indirdep->ir_saveddata = NULL;
indirdep->ir_state &= ~UNDONE;
indirdep->ir_state |= ATTACHED;
@@ -4034,7 +4036,8 @@ handle_written_inodeblock(struct inodede
*dp1 = *inodedep->id_savedino1;
else
*dp2 = *inodedep->id_savedino2;
-   free(inodedep->id_savedino1, M_INODEDEP, 0);
+   free(inodedep->id_savedino1, M_INODEDEP,
+   sizeof(struct ufs1_dinode));
inodedep->id_savedino1 = NULL;
if ((bp->b_flags & B_DELWRI) == 0)
stat_inode_bitmap++;
Index: ufs/ffs/ffs_vfsops.c
===
RCS file: /cvs/src/sys/ufs/ffs/ffs_vfsops.c,v
retrieving revision 1.173
diff -u -p -r1.173 ffs_vfsops.c
--- ufs/ffs/ffs_vfsops.c15 Mar 2018 04:22:16 -  1.173
+++ ufs/ffs/ffs_vfsops.c29 Mar 2018 02:55:37 -
@@ -447,7 +447,7 @@ success:
fs->fs_clean = ronly &&
(fs->fs_flags & FS_UNCLEAN) == 0 ? 1 : 0;
if (ronly)
-   free(fs->fs_contigdirs, M_UFSMNT, 0);
+   free(fs->fs_contigdirs, M_UFSMNT, fs->fs_ncg);
}
if (!ronly) {
if (mp->mnt_flag & MNT_SOFTDEP)
@@ -837,7 +837,7 @@ ffs_mountfs(struct vnode *devvp, struct 
size = (blks - i) * fs->fs_fsize;
error = bread(devvp, fsbtodb(fs, fs->fs_csaddr + i), size, );
if (error) {
-   free(fs->fs_csp, M_UFSMNT, 0);
+   free(fs->fs_csp, M_UFSMNT, fs->fs_cssize);
goto out;
}
memcpy(space, bp->b_data, size);
@@ -910,8 +910,8 @@ ffs_mountfs(struct vnode *devvp, struct 
if (ronly == 0) {
if ((fs->fs_flags & FS_DOSOFTDEP) &&
(error = softdep_mount(devvp, mp, fs, cred)) != 0) {
-   free(fs->fs_csp, M_UFSMNT, 0);
-   free(fs->fs_contigdirs, M_UFSMNT, 0);
+   free(fs->fs_csp, M_UFSMNT, fs->fs_cssize);
+   free(fs->fs_contigdirs, M_UFSMNT, fs->fs_ncg);
goto out;
}
fs->fs_fmod = 1;
@@ -1046,7 +1046,7 @@ ffs_unmount(struct mount *mp, int mntfla
fs->fs_clean = 0;
return (error);
}
-   free(fs->fs_contigdirs, M_UFSMNT, 0);
+   free(fs->fs_contigdirs, M_UFSMNT, fs->fs_ncg);
}
ump->um_devvp->v_specmountpoint = NULL;
 
@@ -1055,7 +1055,7 @@ ffs_unmount(struct mount *mp, int mntfla
(void)VOP_CLOSE(ump->um_devvp, fs->fs_ronly ? FREAD : FREAD|FWRITE,
NOCRED, p);
vput(ump->um_devvp);
-   free(fs->fs_csp, M_UFSMNT, 0);
+   free(fs->fs_csp, M_UFSMNT, 

Re: use hashfree on pcb hash tables

[David Hill]

round #2.

keep track of size (num of elements) in the inpcbtable struct.
passes regress tests.

OK?

Index: netinet/in_pcb.c
===
RCS file: /cvs/src/sys/netinet/in_pcb.c,v
retrieving revision 1.228
diff -u -p -r1.228 in_pcb.c
--- netinet/in_pcb.c19 Feb 2018 08:59:53 -  1.228
+++ netinet/in_pcb.c29 Mar 2018 02:21:34 -
@@ -206,6 +206,7 @@ in_pcbinit(struct inpcbtable *table, int
if (table->inpt_lhashtbl == NULL)
panic("in_pcbinit: hashinit failed for lport");
table->inpt_count = 0;
+   table->inpt_size = hashsize;
arc4random_buf(>inpt_key, sizeof(table->inpt_key));
 }
 
@@ -998,32 +999,34 @@ int
 in_pcbresize(struct inpcbtable *table, int hashsize)
 {
u_long nhash, nlhash;
+   int osize;
void *nhashtbl, *nlhashtbl, *ohashtbl, *olhashtbl;
struct inpcb *inp0, *inp1;
 
ohashtbl = table->inpt_hashtbl;
olhashtbl = table->inpt_lhashtbl;
+   osize = table->inpt_size;
 
nhashtbl = hashinit(hashsize, M_PCB, M_NOWAIT, );
+   if (nhashtbl == NULL)
+   return ENOBUFS;
nlhashtbl = hashinit(hashsize, M_PCB, M_NOWAIT, );
-   if (nhashtbl == NULL || nlhashtbl == NULL) {
-   if (nhashtbl != NULL)
-   free(nhashtbl, M_PCB, 0);
-   if (nlhashtbl != NULL)
-   free(nlhashtbl, M_PCB, 0);
-   return (ENOBUFS);
+   if (nlhashtbl == NULL) {
+   hashfree(nhashtbl, hashsize, M_PCB);
+   return ENOBUFS;
}
table->inpt_hashtbl = nhashtbl;
table->inpt_lhashtbl = nlhashtbl;
table->inpt_hash = nhash;
table->inpt_lhash = nlhash;
+   table->inpt_size = hashsize;
arc4random_buf(>inpt_key, sizeof(table->inpt_key));
 
TAILQ_FOREACH_SAFE(inp0, >inpt_queue, inp_queue, inp1) {
in_pcbrehash(inp0);
}
-   free(ohashtbl, M_PCB, 0);
-   free(olhashtbl, M_PCB, 0);
+   hashfree(ohashtbl, osize, M_PCB);
+   hashfree(olhashtbl, osize, M_PCB);
 
return (0);
 }
Index: netinet/in_pcb.h
===
RCS file: /cvs/src/sys/netinet/in_pcb.h,v
retrieving revision 1.106
diff -u -p -r1.106 in_pcb.h
--- netinet/in_pcb.h1 Dec 2017 10:33:33 -   1.106
+++ netinet/in_pcb.h29 Mar 2018 02:21:34 -
@@ -152,7 +152,7 @@ struct inpcbtable {
struct inpcbhead *inpt_hashtbl, *inpt_lhashtbl;
SIPHASH_KEY inpt_key;
u_longinpt_hash, inpt_lhash;
-   int   inpt_count;
+   int   inpt_count, inpt_size;
 };
 
 /* flags in inp_flags: */

Re: sem_trywait(3) and sem_wait(3) can return EINTR

[Philip Guenther]

On Wed, Mar 28, 2018 at 2:14 PM, Paul Irofti  wrote:
>
> I do not know if this is expected or not, but sem_trywait(3) can and
> does return EINTR. From the manpage I got the impression that it should
> not. Should we amend the manpage or is this something to be fixed in the
> implementation?
>
>  73 do {
>  74 r = __thrsleep(ident, CLOCK_REALTIME, abstime,
>  75 >lock, delayed_cancel);
>  76 _spinlock(>lock);
>  77 /* ignore interruptions other than cancelation
> */
>  78 if (r == EINTR && (delayed_cancel == NULL ||
>  79 *delayed_cancel == 0))
>  80 r = 0;
>  81 } while (r == 0 && sem->value == 0);
>
> Lines 78--80 are the interesting ones.


Lines 69-70 are more important for sem_trywait():
 69 } else if (tryonly) {
 70 r = EAGAIN;
 71 } else {

sem_trywait() calls _sem_wait() with tryonly=1, so it can't reach that
do{tsleep}while loop and will never return EINTR.

In the end, _sem_wait() will return EINTR only if the thread was
canceled...in which case the function that called _sem_wait() will never
return but instead call _thread_canceled() via
the LEAVE_CANCEL_POINT_INNER() macro.


Philip Guenther

Re: Anyone can suggest a BitCoin processor to the OpenBSD Foundation? BitPay has become terrible

[Bob Beck]

So, related to this topic, Apparently BitPay has now fixed us up again.

I have put the button back on the web site, if anyone wants to try a
bitcoin donation is is supposed to be possible again

sem_trywait(3) and sem_wait(3) can return EINTR

[Paul Irofti]

Hi,

I do not know if this is expected or not, but sem_trywait(3) can and
does return EINTR. From the manpage I got the impression that it should
not. Should we amend the manpage or is this something to be fixed in the
implementation?

 73 do {
 74 r = __thrsleep(ident, CLOCK_REALTIME, abstime,
 75 >lock, delayed_cancel);
 76 _spinlock(>lock);
 77 /* ignore interruptions other than cancelation */
 78 if (r == EINTR && (delayed_cancel == NULL ||
 79 *delayed_cancel == 0))
 80 r = 0;
 81 } while (r == 0 && sem->value == 0);

Lines 78--80 are the interesting ones.

Paul

Re: disklabel,fsck_ffs: division by zero on invalid frag sizes

[kwesterback]

> On Mar 28, 2018, at 11:56 AM, Tobias Stoeckmann  wrote:
> 
>> On Wed, Mar 28, 2018 at 12:25:01PM +0200, Otto Moerbeek wrote:
>> Hmm, on what platform are you doing this? I could not reproduce your
>> problem on arm64 -current,
> 
> To sum it up: It was my fault, -current is all fine.
> 
> The issue can be reproduced with 6.2-stable amd64 and somewhere between
> 6.2-release and -current, but not with latest -current.
> 
> My i386 system uses a modified kernel (for my touchpad) and due to
> being not the fastest system, I am not always keeping the kernel up
> to date, as long as everything boots up.
> 
> Didn't expect it to be a kernel issue, so after a clean install,
> applying the diff krw@ sent me and rebuilding the kernel as well as
> disklabel and fsck_ffs, the problem went away. With stock -current
> kernel, the issue is also not reproducible.
> 
> Sorry for the noise, will give the year 2016 its diff back. ;)
> 
> 
> Tobias

Cool. Thanks for following up.

 Ken

relayd http check fix

[Rivo Nurges]

Hi!

If relayd http check doesn't get any answer to its http check it marks
backend host as up.

host x.y.z, check http code (2010ms,tcp read timeout), state down ->
up, availability 14.29%

sample config:
relay test {
  listen on x.x.x.x port 
  forward to  port  check http "/" code 200
}

sample server:
while :;do nc -l ;done

fix:
Index: usr.sbin/relayd/check_tcp.c
===
RCS file: /cvs/src/usr.sbin/relayd/check_tcp.c,v
retrieving revision 1.55
diff -u -p -r1.55 check_tcp.c
--- usr.sbin/relayd/check_tcp.c 4 Jul 2017 20:27:09 -   1.55
+++ usr.sbin/relayd/check_tcp.c 28 Mar 2018 16:45:38 -
@@ -243,8 +243,10 @@ tcp_read_buf(int s, short event, void *a
if (event == EV_TIMEOUT) {
if (ibuf_size(cte->buf))
(void)cte->validate_close(cte);
-   else
+   else {
cte->host->he = HCE_TCP_READ_TIMEOUT;
+   cte->host->up = HOST_DOWN;
+   }   
tcp_close(cte, cte->host->up == HOST_UP ? 0 : HOST_DOWN);
hce_notify_done(cte->host, cte->host->he);
return;

Re: unbound 1.7.0

[Florian Obser]

On Wed, Mar 28, 2018 at 12:31:01PM +0200, Florian Obser wrote:
> I guess nobody is going to review a 10k line diff...

so benno actually read the diff and points out that we need this:

(already reported upstream)

diff --git services/authzone.c services/authzone.c
index 13e36b2c..fac8e4ed 100644
--- services/authzone.c
+++ services/authzone.c
@@ -5946,7 +5946,7 @@ static char*
 dup_all(char* str)
 {
char* result = strdup(str);
-   if(!str) {
+   if(!result) {
log_err("malloc failure");
return NULL;
}


-- 
I'm not entirely sure you are real.

Re: NFS vs NET_LOCK()

[Visa Hankala]

On Wed, Mar 28, 2018 at 11:59:46AM +0200, Martin Pieuchot wrote:
> Index: uvm/uvm_vnode.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v
> retrieving revision 1.99
> diff -u -p -r1.99 uvm_vnode.c
> --- uvm/uvm_vnode.c   8 Mar 2018 22:04:18 -   1.99
> +++ uvm/uvm_vnode.c   28 Mar 2018 09:56:51 -
> @@ -1105,6 +1105,7 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t 
>   off_t file_offset;
>   int waitf, result, mapinflags;
>   size_t got, wanted;
> + int netunlocked = 0;
>  
>   /* init values */
>   waitf = (flags & PGO_SYNCIO) ? M_WAITOK : M_NOWAIT;
> @@ -1174,18 +1175,24 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t 
>* Ideally, this kind of operation *should* work.
>*/
>   result = 0;
> - if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0)
> - result = vn_lock(vn, LK_EXCLUSIVE | LK_RECURSEFAIL, curproc);
> -
> - if (result == 0) {
> - int netlocked = (rw_status() == RW_WRITE);
> -
> + if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0) {
>   /*
>* This process may already have the NET_LOCK(), if we
>* faulted in copyin() or copyout() in the network stack.
>*/
> - if (netlocked)
> + if (rw_status() == RW_WRITE) {
> + netunlocked = 1;
> + NET_UNLOCK();
> + }
> +
> + result = vn_lock(vn, LK_EXCLUSIVE | LK_RECURSEFAIL, curproc);
> + }
> +
> + if (result == 0) {
> + if (!netunlocked && (rw_status() == RW_WRITE)) {
> + netunlocked = 1;
>   NET_UNLOCK();
> + }

Is it necessary to have two unlock cases? It looks that a single
check of netlock + unlock before
the ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0) condition would
be enough.

if (rw_status() == RW_WRITE) {
netunlocked = 1;
NET_UNLOCK();
}

result = 0;
if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0)
result = vn_lock(vn, LK_EXCLUSIVE | LK_RECURSEFAIL, curproc);

if (result == 0) {
/* NOTE: vnode now locked! */
...

Re: disklabel,fsck_ffs: division by zero on invalid frag sizes

[Tobias Stoeckmann]

On Wed, Mar 28, 2018 at 12:25:01PM +0200, Otto Moerbeek wrote:
> Hmm, on what platform are you doing this? I could not reproduce your
> problem on arm64 -current,

To sum it up: It was my fault, -current is all fine.

The issue can be reproduced with 6.2-stable amd64 and somewhere between
6.2-release and -current, but not with latest -current.

My i386 system uses a modified kernel (for my touchpad) and due to
being not the fastest system, I am not always keeping the kernel up
to date, as long as everything boots up.

Didn't expect it to be a kernel issue, so after a clean install,
applying the diff krw@ sent me and rebuilding the kernel as well as
disklabel and fsck_ffs, the problem went away. With stock -current
kernel, the issue is also not reproducible.

Sorry for the noise, will give the year 2016 its diff back. ;)


Tobias

Re: disklabel,fsck_ffs: division by zero on invalid frag sizes

[Otto Moerbeek]

On Tue, Mar 27, 2018 at 07:50:18PM +0200, Otto Moerbeek wrote:

> On Tue, Mar 27, 2018 at 12:46:03PM +0200, Tobias Stoeckmann wrote:
> 
> > Resending this old diff. Adjusted to apply with -current source:
> > 
> > Illegal fragmentation block sizes can trigger division by zero in the
> > disklabel and fsck_ffs tools.
> > 
> > See this sequence of commands to reproduce:
> > 
> > # dd if=/dev/zero of=nofrag.iso bs=1M count=1
> > # vnconfig vnd0 nofrag.iso
> > # disklabel -e vnd0
> > 
> > Create 'a' and set fsize = bsize = 1. A line like this one will do
> >   a: 20480  4.2BSD  1 1 1
> > 
> > # fsck_ffs vnd0a
> > ** /dev/vnd0a (vnd0a)
> > BAD SUPER BLOCK: MAGIC NUMBER WRONG
> > Floating point exception (core dumped)
> > # disklabel -E vnd0
> > Label editor (enter '?' for help at any prompt)
> > > m a
> > offset: [0]
> > size: [2048]
> > FS type: [4.2BSD]
> > Floating point exception (core dumped)
> > # vnconfig -u vnd0

Hmm, on what platform are you doing this? I could not reproduce your
problem on arm64 -current,

-Otto

> > 
> > A fragmentation (block) size smaller than a sector size is not valid
> > while using "disklabel -E", and really doesn't make sense. While
> > using "disklabel -e", not all validation checks are performed, which
> > makes it possible to enter invalid values.
> > 
> > If "disklabel -E" is used without the expert mode, fragmentation sizes
> > cannot be changed and will be just accepted from the parsed disklabel,
> > resulting in a division by zero if they are too small.
> > 
> > And the same happens in fsck_ffs. Instead of coming up with a guessed
> > value in fsck_ffs, I think it's better to simply fail and let the user
> > fix the disklabel. After all, it shouldn't be fsck_ffs's duty to fix
> > faulty values outside the filesystem.
> 
> This looks from reading the diff, but "fragmentation size" is not the
> correct term, see inline.
> 
>   -Otto
> 
> > 
> > Index: sbin/disklabel/disklabel.c
> > ===
> > RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
> > retrieving revision 1.227
> > diff -u -p -u -p -r1.227 disklabel.c
> > --- sbin/disklabel/disklabel.c  25 Feb 2018 17:24:44 -  1.227
> > +++ sbin/disklabel/disklabel.c  27 Mar 2018 10:42:59 -
> > @@ -1100,9 +1100,24 @@ gottype:
> >  
> > case FS_BSDFFS:
> > NXTNUM(fsize, fsize, );
> > -   if (fsize == 0)
> > +   if (fsize < lp->d_secsize ||
> > +   (fsize % lp->d_secsize) != 0) {
> > +   warnx("line %d: "
> > +   "bad fragmentation size: %s",
> 
> Should be "fragment size"
> 
> > +   lineno, cp);
> > +   errors++;
> > break;
> > +   }
> > NXTNUM(v, v, );
> > +   if (v < fsize || (fsize != v &&
> > +   fsize * 2 != v && fsize * 4 != v &&
> > +   fsize * 8 != v)) {
> > +   warnx("line %d: "
> > +   "bad block size: %s",
> > +   lineno, cp);
> > +   errors++;
> > +   break;
> > +   }
> > pp->p_fragblock =
> > DISKLABELV1_FFS_FRAGBLOCK(fsize, v / fsize);
> > NXTNUM(pp->p_cpg, pp->p_cpg, );
> > Index: sbin/disklabel/editor.c
> > ===
> > RCS file: /cvs/src/sbin/disklabel/editor.c,v
> > retrieving revision 1.327
> > diff -u -p -u -p -r1.327 editor.c
> > --- sbin/disklabel/editor.c 8 Mar 2018 22:05:17 -   1.327
> > +++ sbin/disklabel/editor.c 27 Mar 2018 10:42:59 -
> > @@ -2069,16 +2069,16 @@ get_bsize(struct disklabel *lp, int part
> > if (pp->p_fstype != FS_BSDFFS)
> > return (0);
> >  
> > +   frag = DISKLABELV1_FFS_FRAG(pp->p_fragblock);
> > +   fsize = DISKLABELV1_FFS_FSIZE(pp->p_fragblock);
> > +
> > /* Avoid dividing by zero... */
> > -   if (pp->p_fragblock == 0)
> > -   return(1);
> > +   if (frag * fsize < lp->d_secsize)
> > +   return (1);
> >  
> > if (!expert)
> > goto align;
> >  
> > -   fsize = DISKLABELV1_FFS_FSIZE(pp->p_fragblock);
> > -   frag = DISKLABELV1_FFS_FRAG(pp->p_fragblock);
> > -
> > for (;;) {
> > ui = getuint64(lp, "block size",
> > "Size of ffs blocks. 1, 2, 4 or 8 times ffs fragment size.",
> > @@ -2119,8 +2119,7 @@ align:
> > orig_size = DL_GETPSIZE(pp);
> > orig_offset = DL_GETPOFFSET(pp);
> >  
> > -   bsize = 

xhci@fdt; legacy binding support

[Mark Kettenis]

The Marvell Aramada device trees use a legacy binding for the XHCI USB
PHY.  This diff adds support for this misfeature as fixing the device
trees may take a while.  It also adds support for the "usb-nop-xceiv"
PHY type, which contains the reference to the regulator that supplies
power to the USB bus.

ok?

Index: dev/fdt/xhci_fdt.c
===
RCS file: /cvs/src/sys/dev/fdt/xhci_fdt.c,v
retrieving revision 1.6
diff -u -p -r1.6 xhci_fdt.c
--- dev/fdt/xhci_fdt.c  12 Aug 2017 16:57:07 -  1.6
+++ dev/fdt/xhci_fdt.c  28 Mar 2018 10:00:08 -
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -180,10 +181,13 @@ struct xhci_phy {
 };
 
 void exynos5_usbdrd_init(struct xhci_fdt_softc *, uint32_t *);
+void nop_xceiv_init(struct xhci_fdt_softc *, uint32_t *);
 
 struct xhci_phy xhci_phys[] = {
{ "samsung,exynos5250-usbdrd-phy", exynos5_usbdrd_init },
-   { "samsung,exynos5420-usbdrd-phy", exynos5_usbdrd_init }
+   { "samsung,exynos5420-usbdrd-phy", exynos5_usbdrd_init },
+   { "usb-nop-xceiv", nop_xceiv_init },
+   
 };
 
 uint32_t *
@@ -223,9 +227,21 @@ xhci_init_phys(struct xhci_fdt_softc *sc
 {
uint32_t *phys;
uint32_t *phy;
+   uint32_t usb_phy;
int len, idx;
 
-   /* XXX Only initialize the USB 3 PHY for now. */
+   /*
+* Legacy binding; assume there only is a single USB PHY.
+*/
+   usb_phy = OF_getpropint(sc->sc_node, "usb-phy", 0);
+   if (usb_phy) {
+   xhci_init_phy(sc, _phy);
+   return;
+   }
+
+   /*
+* Generic PHY binding; only initialize USB 3 PHY for now.
+*/
idx = OF_getindex(sc->sc_node, "usb3-phy", "phy-names");
if (idx < 0)
return;
@@ -329,4 +345,18 @@ exynos5_usbdrd_init(struct xhci_fdt_soft
delay(10);
CLR(val, EXYNOS5_PHYCLKRST_PORTRESET);
bus_space_write_4(sc->sc.iot, sc->ph_ioh, EXYNOS5_PHYCLKRST, val);
+}
+
+void
+nop_xceiv_init(struct xhci_fdt_softc *sc, uint32_t *cells)
+{
+   uint32_t vcc_supply;
+   int node;
+
+   node = OF_getnodebyphandle(cells[0]);
+   KASSERT(node != 0);
+
+   vcc_supply = OF_getpropint(node, "vcc-supply", 0);
+   if (vcc_supply)
+   regulator_enable(vcc_supply);
 }

Re: NFS vs NET_LOCK()

[Martin Pieuchot]

On 27/03/18(Tue) 15:47, Visa Hankala wrote:
> On Tue, Mar 27, 2018 at 11:00:20AM +0200, Martin Pieuchot wrote:
> > Diff below prevents a future lock ordering problem between NFSnode
> > locks (similar to Inode locks) and the NET_LOCK().
> > 
> > It ensures the NET_LOCK() is always locked *after* any NFSnode lock
> > by fixing the UVM fault case.  So we have always have:
> >   VFS -> NFS -> NFSnode lock -> socket -> NET_LOCK(). 
> > 
> > [...]
> > @@ -1195,11 +1202,12 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t 
> > (flags & PGO_PDFREECLUST) ? IO_NOCACHE : 0,
> > curproc->p_ucred);
> >  
> > +   if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0)
> > +   VOP_UNLOCK(vn, curproc);
> > +
> > if (netlocked)
> > NET_LOCK();
> >  
> > -   if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0)
> > -   VOP_UNLOCK(vn, curproc);
> > }
> 
> This can leave NET_LOCK() unrestored if vn_lock() returns non-zero.

Good catch, updated diff below.  I also rename `netlocked' into
`netunlocked' to better reflect the logic.

ok?

Index: uvm/uvm_vnode.c
===
RCS file: /cvs/src/sys/uvm/uvm_vnode.c,v
retrieving revision 1.99
diff -u -p -r1.99 uvm_vnode.c
--- uvm/uvm_vnode.c 8 Mar 2018 22:04:18 -   1.99
+++ uvm/uvm_vnode.c 28 Mar 2018 09:56:51 -
@@ -1105,6 +1105,7 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t 
off_t file_offset;
int waitf, result, mapinflags;
size_t got, wanted;
+   int netunlocked = 0;
 
/* init values */
waitf = (flags & PGO_SYNCIO) ? M_WAITOK : M_NOWAIT;
@@ -1174,18 +1175,24 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t 
 * Ideally, this kind of operation *should* work.
 */
result = 0;
-   if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0)
-   result = vn_lock(vn, LK_EXCLUSIVE | LK_RECURSEFAIL, curproc);
-
-   if (result == 0) {
-   int netlocked = (rw_status() == RW_WRITE);
-
+   if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0) {
/*
 * This process may already have the NET_LOCK(), if we
 * faulted in copyin() or copyout() in the network stack.
 */
-   if (netlocked)
+   if (rw_status() == RW_WRITE) {
+   netunlocked = 1;
+   NET_UNLOCK();
+   }
+
+   result = vn_lock(vn, LK_EXCLUSIVE | LK_RECURSEFAIL, curproc);
+   }
+
+   if (result == 0) {
+   if (!netunlocked && (rw_status() == RW_WRITE)) {
+   netunlocked = 1;
NET_UNLOCK();
+   }
 
/* NOTE: vnode now locked! */
if (rw == UIO_READ)
@@ -1195,12 +1202,14 @@ uvn_io(struct uvm_vnode *uvn, vm_page_t 
(flags & PGO_PDFREECLUST) ? IO_NOCACHE : 0,
curproc->p_ucred);
 
-   if (netlocked)
-   NET_LOCK();
-
if ((uvn->u_flags & UVM_VNODE_VNISLOCKED) == 0)
VOP_UNLOCK(vn, curproc);
+
}
+
+   if (netunlocked)
+   NET_LOCK();
+
 
/* NOTE: vnode now unlocked (unless vnislocked) */
/*

Re: com.4: -socppc

[Martin Pieuchot]

On 28/03/18(Wed) 12:46, Artturi Alm wrote:
> Hi,
> 
> socppc@attic was asking for this.

It is not in the attic, please do not waste our time :)

com.4: -socppc

[Artturi Alm]

Hi,

socppc@attic was asking for this.

-Artturi


diff --git share/man/man4/com.4 share/man/man4/com.4
index 6301f04ae60..9c5f0d23230 100644
--- share/man/man4/com.4
+++ share/man/man4/com.4
@@ -91,10 +91,6 @@
 .Cd "com0 at ioc? base 0x00020178"
 .Cd "com1 at ioc? base 0x00020170"
 .Pp
-.Cd "# socppc"
-.Cd "com0 at obio? addr 0x04500 ivec 9"
-.Cd "com1 at obio? addr 0x04600 ivec 10"
-.Pp
 .Cd "# sparc64"
 .Cd "com* at asio?"
 .Cd "com* at ebus?"

Re: interface queue transmit mitigation (again)

[Hrvoje Popovski]

On 28.3.2018. 3:28, David Gwynne wrote:
> On Thu, Mar 15, 2018 at 03:25:46PM +0100, Martin Pieuchot wrote:
>> On 14/03/18(Wed) 13:00, David Gwynne wrote:
>>> this adds transmit mitigation back to the tree.
>>>
>>> it is basically the same diff as last time. the big difference this
>>> time is that all the tunnel drivers all defer ip_output calls, which
>>> avoids having to play games with NET_LOCK in the ifq transmit paths.
>> Comments inline.
>>
>>> +   if (ifq_len(ifq) >= min(4, ifq->ifq_maxlen)) {
>> Why 4?  DragonFly recently bumped `ifsq_stage_cntmax' to 16.  Did you
>> try other values?  They also have an XXX comment that this value should
>> be per-interface.  Why?
> their default was 4, and they'd done some research on it. if they
> moved to 16 there would be a reason for it.

Hi all,

with this diff i'm getting 1.43Mpps on
12 x Intel(R) Xeon(R) CPU E5-2620 v2 @ 2.10GHz, 2400.01 MHz

with plain kernel i'm getting 1.12Mpps and with older diff's i was
getting 1.31Mpps ...

if i execute ifconfig ix0 down while generating traffic over everything stop

x3550m4# ifconfig ix0 down
^C

from ddb:

ddb{0}> ps
   PID TID   PPIDUID  S   FLAGS  WAIT  COMMAND
   287   54967   4723  0  3 0x3  tqbar ifconfig
  4723  369240  1  0  30x10008b  pause ksh
 78652   12963  1  0  30x100083  ttyin getty
 77445  380566  1  0  30x100083  ttyin getty
  7708   16964  1  0  30x100083  ttyin getty
  6466  480278  1  0  30x100083  ttyin getty
 10683  361200  1  0  30x100083  ttyin ksh
  33222446  1  0  30x100098  poll  cron
  81878185  1 99  30x100090  poll  sndiod
 38828  292448  1110  30x100090  poll  sndiod
 96602  349758  17921 95  30x100092  kqreadsmtpd
 59159  244097  17921103  30x100092  kqreadsmtpd
 72520  357622  17921 95  30x100092  kqreadsmtpd
 11196  442980  17921 95  30x100092  kqreadsmtpd
 19374  184986  17921 95  30x100092  kqreadsmtpd
 99758  239851  17921 95  30x100092  kqreadsmtpd
 17921  468052  1  0  30x100080  kqreadsmtpd
 96705  480305  1  0  30x80  selectsshd
 10784  513637  74642 83  30x100092  poll  ntpd
 74642  349129  19763 83  30x100092  poll  ntpd
 19763  118713  1  0  30x100080  poll  ntpd
 80820  281787  61744 73  30x100090  kqreadsyslogd
 61744  358228  1  0  30x100082  netio syslogd
 92438  103007  37416115  30x100092  kqreadslaacd
 27600  284603  37416115  30x100092  kqreadslaacd
 37416  302849  1  0  30x80  kqreadslaacd
 31025  461354  0  0  3 0x14200  pgzerozerothread
 87259  136299  0  0  3 0x14200  aiodoned  aiodoned
 40842   63462  0  0  3 0x14200  syncerupdate
 434254  0  0  3 0x14200  cleaner   cleaner
 20219  234861  0  0  3 0x14200  reaperreaper
 49370  510675  0  0  3 0x14200  pgdaemon  pagedaemon
 34415  210813  0  0  3 0x14200  bored crynlk
 98085  523911  0  0  3 0x14200  bored crypto
 88352  390863  0  0  3 0x14200  usbtskusbtask
 36743   62252  0  0  3 0x14200  usbatsk   usbatsk
 38389  456819  0  0  3  0x40014200  acpi0 acpi0
 93162   81423  0  0  7  0x40014200idle11
 58166   30480  0  0  7  0x40014200idle10
 55398  115831  0  0  7  0x40014200idle9
96   42736  0  0  7  0x40014200idle8
 63465  183206  0  0  7  0x40014200idle7
 79804  340505  0  0  7  0x40014200idle6
 42987  392463  0  0  7  0x40014200idle5
 94478  284414  0  0  7  0x40014200idle4
 45914   13592  0  0  7  0x40014200idle3
 14508  513956  0  0  7  0x40014200idle2
 16424  111283  0  0  7  0x40014200idle1
 60252  298958  0  0  3 0x14200  bored sensors
 80523  235128  0  0  3 0x14200  tqbar softnet
 40231  252516  0  0  3 0x14200  bored systqmp
 97996  128366  0  0  3 0x14200  bored systq
 78639  112346  0  0  3  0x40014200  bored softclock
*60124   95946  0  0  7  0x40014200idle0
 1  232514  0  0  30x82  wait  init
 0   0 -1  0  3 0x10200  scheduler swapper
ddb{0}>


ddb{0}> tr /p 0t54967
sleep_finish(800023d3c528,81a3863c) at sleep_finish+0x70

Re: interface queue transmit mitigation (again)

[Martin Pieuchot]

On 28/03/18(Wed) 11:28, David Gwynne wrote:
> On Thu, Mar 15, 2018 at 03:25:46PM +0100, Martin Pieuchot wrote:
> > On 14/03/18(Wed) 13:00, David Gwynne wrote:
> > > this adds transmit mitigation back to the tree.
> > > 
> > > it is basically the same diff as last time. the big difference this
> > > time is that all the tunnel drivers all defer ip_output calls, which
> > > avoids having to play games with NET_LOCK in the ifq transmit paths.
> > 
> > Comments inline.
> > 
> > > + if (ifq_len(ifq) >= min(4, ifq->ifq_maxlen)) {
> > 
> > Why 4?  DragonFly recently bumped `ifsq_stage_cntmax' to 16.  Did you
> > try other values?  They also have an XXX comment that this value should
> > be per-interface.  Why?
> 
> their default was 4, and they'd done some research on it. if they
> moved to 16 there would be a reason for it.
> 
> > In any case I'd be happier with a define.
> 
> i've taken your advice on board and made it per interface, defaulted
> to 16 with a macro. after this i can add ioctls to report it (under
> hwfeatures maybe) and change it with ifconfig.
> 
> > >  ifq_barrier(struct ifqueue *ifq)
> > >  {
> > >   struct cond c = COND_INITIALIZER();
> > >   struct task t = TASK_INITIALIZER(ifq_barrier_task, );
> > >  
> > > + NET_ASSERT_UNLOCKED();
> > 
> > Since you assert here...
> > 
> > > +
> > > + if (!task_del(ifq->ifq_softnet, >ifq_bundle)) {
> > > + int netlocked = (rw_status() == RW_WRITE);
> > ^
> > You can remove this code.
> 
> i can't get rid of the assert :(
> 
> ifq_barrier is called from lots of places, eg, ifq_destroy and
> ix_down. the former does not hold the net lock, but the latter does.
> putting manual NET_UNLOCK calls around places like the latter is
> too much work imo.

You're right, getting rid of the NET_LOCK() is too much work.  That's
why I'm being rude for such pattern not to spread.  I'd prefer an
assert and move the dance in the only place that needs it.

Anyway `if_sndmit' should be [d] since its a driver variable, not a
stack one.

Apart from that ok mpi@

> retrieving revision 1.548
> diff -u -p -r1.548 if.c
> --- if.c  2 Mar 2018 15:52:11 -   1.548
> +++ if.c  28 Mar 2018 01:28:03 -
> @@ -607,6 +607,7 @@ if_attach_common(struct ifnet *ifp)
>   ifp->if_snd.ifq_ifqs[0] = >if_snd;
>   ifp->if_ifqs = ifp->if_snd.ifq_ifqs;
>   ifp->if_nifqs = 1;
> + ifp->if_sndmit = IF_SNDMIT_DEFAULT;
>  
>   ifiq_init(>if_rcv, ifp, 0);
>  
> Index: if_var.h
> ===
> RCS file: /cvs/src/sys/net/if_var.h,v
> retrieving revision 1.89
> diff -u -p -r1.89 if_var.h
> --- if_var.h  10 Jan 2018 23:50:39 -  1.89
> +++ if_var.h  28 Mar 2018 01:28:03 -
> @@ -170,6 +170,7 @@ struct ifnet {/* and the 
> entries */
>   struct  ifqueue **if_ifqs;  /* [I] pointer to an array of sndqs */
>   void(*if_qstart)(struct ifqueue *);
>   unsigned int if_nifqs;  /* [I] number of output queues */
> + unsigned int if_sndmit; /* [c] tx mitigation amount */
>  
>   struct  ifiqueue if_rcv;/* rx/input queue */
>   struct  ifiqueue **if_iqs;  /* [I] pointer to the array of iqs */
> @@ -279,6 +280,9 @@ do {  
> \
>  #define  IFQ_LEN(ifq)ifq_len(ifq)
>  #define  IFQ_IS_EMPTY(ifq)   ifq_empty(ifq)
>  #define  IFQ_SET_MAXLEN(ifq, len)ifq_set_maxlen(ifq, len)
> +
> +#define IF_SNDMIT_MIN1
> +#define IF_SNDMIT_DEFAULT16
>  
>  /* default interface priorities */
>  #define IF_WIRED_DEFAULT_PRIORITY0
> Index: ifq.c
> ===
> RCS file: /cvs/src/sys/net/ifq.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 ifq.c
> --- ifq.c 25 Jan 2018 14:04:36 -  1.22
> +++ ifq.c 28 Mar 2018 01:28:03 -
> @@ -70,9 +70,16 @@ struct priq {
>  void ifq_start_task(void *);
>  void ifq_restart_task(void *);
>  void ifq_barrier_task(void *);
> +void ifq_bundle_task(void *);
>  
>  #define TASK_ONQUEUE 0x1
>  
> +static inline void
> +ifq_run_start(struct ifqueue *ifq)
> +{
> + ifq_serialize(ifq, >ifq_start);
> +}
> +
>  void
>  ifq_serialize(struct ifqueue *ifq, struct task *t)
>  {
> @@ -114,6 +121,16 @@ ifq_is_serialized(struct ifqueue *ifq)
>  }
>  
>  void
> +ifq_start(struct ifqueue *ifq)
> +{
> + if (ifq_len(ifq) >= min(ifq->ifq_if->if_sndmit, ifq->ifq_maxlen)) {
> + task_del(ifq->ifq_softnet, >ifq_bundle);
> + ifq_run_start(ifq);
> + } else
> + task_add(ifq->ifq_softnet, >ifq_bundle);
> +}
> +
> +void
>  ifq_start_task(void *p)
>  {
>   struct ifqueue *ifq = p;
> @@ -137,11 +154,31 @@ ifq_restart_task(void *p)
>  }
>  
>  void
> +ifq_bundle_task(void *p)
> +{
> + struct ifqueue *ifq = p;
> +
> + ifq_run_start(ifq);
> 

Re: cut: Fix segmentation fault

[Tobias Stoeckmann]

Hi,

On Wed, Mar 28, 2018 at 12:53:47AM +0200, Ingo Schwarze wrote:
> Ouch, you are right.  But then, the code feels counter-intuitive
> and the error message confusing.

I agree. Talking about a zero out of nothing seems weird, even though
I have learned yesterday that "a=''; echo $((a))" is "0". ;)

As this special zero case is handled by strtonum already, how about we
just stick with "invalid list value" which is true for non-digits as
well?

if (*p != '\0' || !stop || !start)
errx(1, "[-bcf] list: illegal list value");

I've extended the regression tests to cover the overflow part as well
as handling invalid list values.


Tobias

Index: usr.bin/cut/cut.c
===
RCS file: /cvs/src/usr.bin/cut/cut.c,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 cut.c
--- usr.bin/cut/cut.c   2 Dec 2015 00:56:46 -   1.23
+++ usr.bin/cut/cut.c   28 Mar 2018 07:21:28 -
@@ -154,11 +154,32 @@ int autostart, autostop, maxval;
 
 char positions[_POSIX2_LINE_MAX + 1];
 
+int
+read_number(char **p)
+{
+   size_t pos;
+   int dash, n;
+   const char *errstr;
+   char *q;
+
+   q = *p + strcspn(*p, "-");
+   dash = *q == '-';
+   *q = '\0';
+   n = strtonum(*p, 1, _POSIX2_LINE_MAX, );
+   if (errstr != NULL)
+   errx(1, "[-bcf] list: %s %s (allowed 1-%d)", *p, errstr,
+   _POSIX2_LINE_MAX);
+   if (dash)
+   *q = '-';
+   *p = q;
+
+   return n;
+}
+
 void
 get_list(char *list)
 {
int setautostart, start, stop;
-   char *pos;
char *p;
 
/*
@@ -176,30 +197,27 @@ get_list(char *list)
setautostart = 1;
}
if (isdigit((unsigned char)*p)) {
-   start = stop = strtol(p, , 10);
+   start = stop = read_number();
if (setautostart && start > autostart)
autostart = start;
}
if (*p == '-') {
-   if (isdigit((unsigned char)p[1]))
-   stop = strtol(p + 1, , 10);
+   if (isdigit((unsigned char)p[1])) {
+   ++p;
+   stop = read_number();
+   }
if (*p == '-') {
++p;
if (!autostop || autostop > stop)
autostop = stop;
}
}
-   if (*p)
+   if (*p != '\0' || !stop || !start)
errx(1, "[-bcf] list: illegal list value");
-   if (!stop || !start)
-   errx(1, "[-bcf] list: values may not include zero");
-   if (stop > _POSIX2_LINE_MAX)
-   errx(1, "[-bcf] list: %d too large (max %d)",
-   stop, _POSIX2_LINE_MAX);
if (maxval < stop)
maxval = stop;
-   for (pos = positions + start; start++ <= stop; *pos++ = 1)
-   ;
+   if (start <= stop)
+   memset(positions + start, 1, stop - start + 1);
}
 
/* overlapping ranges */
Index: regress/usr.bin/cut/cut.sh
===
RCS file: /cvs/src/regress/usr.bin/cut/cut.sh,v
retrieving revision 1.1
diff -u -p -u -p -r1.1 cut.sh
--- regress/usr.bin/cut/cut.sh  7 Oct 2016 17:22:12 -   1.1
+++ regress/usr.bin/cut/cut.sh  28 Mar 2018 07:21:28 -
@@ -16,15 +16,26 @@
 
 unset LC_ALL
 
+: ${CUT=cut}
+
 test_cut()
 {
-   args=`echo "$1"`
-   stdin=$2
-   expected=`echo "$3"`
+   expected_retval=$1
+   args=`echo "$2"`
+   stdin=$3
+   expected=`echo "$4"`
export LC_CTYPE=en_US.UTF-8
-   result=`echo -n "$stdin" | cut $args`
+   result=`echo -n "$stdin" | $CUT $args 2>/dev/null`
+   retval=$?
+   if [ "$retval" -ne "${expected_retval}" ]; then
+   echo "echo -n \"$stdin\" | $CUT $args"
+   echo -n "$stdin" | hexdump -C
+   echo "expected return value: \"${expected_retval}\""
+   echo "actual return value: \"$retval\""
+   exit 1;
+   fi
if [ "$result" != "${expected}" ]; then
-   echo "echo -n \"$stdin\" | cut $args"
+   echo "echo -n \"$stdin\" | $CUT $args"
echo -n "$stdin" | hexdump -C
echo "expected: \"$expected\""
echo -n "$expected" | hexdump -C
@@ -33,13 +44,20 @@ test_cut()
exit 1;
fi
 
-   if [ -n "$4" ]; then
-   expected=`echo "$4"`
+   if [ -n "$5" ]; then
+   expected=`echo "$5"`
fi
export LC_CTYPE=C
-   result=`echo -n "$stdin" | cut