Re: video(1): use _NET_WM_STATE_FULLSCREEN

2016-09-25 Thread Joerg Jung

Am 26.09.2016 um 00:11 schrieb Dmitrij D. Czarkoff :

>> The diff below fixes fullscreen mode on window managers that prevent
>> applications from resizing their windows.  

For example, which WM?

>> This is done by setting
>> _NET_WM_STATE_FULLSCREEN atom.
> 
> Ping.

Shouldn't the WM fixed instead?

> --
> Dmitrij D. Czarkoff
> 
> Index: video.c
> ===
> RCS file: /var/cvs/xenocara/app/video/video.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 video.c
> --- video.c6 Jun 2016 19:31:22 -   1.19
> +++ video.c18 Sep 2016 21:33:29 -
> @@ -160,6 +160,7 @@ struct video {
>  int   conv_type;
>  int   enc;
>  int   full_screen;
> +  int  net_wm;
>  int   width;
>  int   height;
>  int   bpf;
> @@ -178,6 +179,7 @@ int xv_get_info(struct video *);
> int xv_sel_adap(struct video *);
> void xv_dump_info(struct video *);
> int xv_init(struct video *);
> +void net_wm_supported(struct video *);
> void resize_window(struct video *, int);
> void display_event(struct video *);
> 
> @@ -500,34 +502,84 @@ xv_init(struct video *vid)
> }
> 
> void
> +net_wm_supported(struct video *vid)
> +{
> +  struct xdsp *x = >xdsp;
> +  Atom query, fullscreen;
> +  Atom type;
> +  Atom *data;
> +  long off;
> +  int fmt, len = 12;
> +  unsigned long nitems, remain;
> +  int i;
> +
> +  query = XInternAtom(x->dpy, "_NET_SUPPORTED", True);
> +  fullscreen = XInternAtom(x->dpy, "_NET_WM_STATE_FULLSCREEN", \
> True);
> +
> +  for (off = 0; XGetWindowProperty(x->dpy, x->rwin, query, off,
> +len, False, XA_ATOM, , , \
> , ,
> +(unsigned char **)) == Success; \
> off += len) {
> +  if (type == XA_ATOM && fmt == 32) {
> +  for (i = 0; i < nitems; i++) {
> +  if (data[i] == fullscreen) {
> +  vid->net_wm = 1;
> +  XFree(data);
> +  return;
> +  }
> +  }
> +  }
> +  XFree(data);
> +  }
> +
> +  return;
> +}
> +
> +void
> resize_window(struct video *vid, int fullscreen)
> {
>  struct xdsp *x = >xdsp;
>  XWindowAttributes winatt;
>  Window junk;
> -  int new_width;
> -  int new_height;
>  int new_x;
>  int new_y;
> +  XEvent ev;
> +  Atom property, value;
> 
>  if (fullscreen == 1) {
>   if (vid->full_screen == 1) {
> -  new_width = x->saved_w;
> -  new_height = x->saved_h;
> +  x->width = x->saved_w;
> +  x->height = x->saved_h;
>new_x = x->saved_x;
>new_y = x->saved_y;
> -  vid->full_screen = 0;
> } else {
> -  new_width = x->max_width;
> -  new_height = x->max_height;
> +  x->width = x->max_width;
> +  x->height = x->max_height;
>new_x = 0;
>new_y = 0;
> -  vid->full_screen = 1;
> }
> -  x->width = new_width;
> -  x->height = new_height;
> -  XMoveResizeWindow(x->dpy, x->window, new_x, new_y,
> -  new_width, new_height);
> +
> +  vid->full_screen = !vid->full_screen;
> +
> +  if (vid->net_wm) {
> +  property = XInternAtom(x->dpy, "_NET_WM_STATE", \
> False);
> +  value = XInternAtom(x->dpy, "_NET\
> _WM_STATE_FULLSCREEN",
> +  False);
> +
> +  memset(, 0, sizeof(ev));
> +  ev.type = ClientMessage;
> +  ev.xclient.window = x->window;
> +  ev.xclient.message_type = property;
> +  ev.xclient.format = 32;
> +  ev.xclient.data.l[0] = vid->full_screen;
> +  ev.xclient.data.l[1] = value;
> +
> +  XSendEvent(x->dpy, x->rwin, False,
> +  SubstructureNotifyMask | Subs\
> tructureRedirectMask,
> +  );
> +  } else {
> +  XMoveResizeWindow(x->dpy, x->window, x->width,
> +  x->height, new_x, new_y);
> +  }
> } else if (!vid->full_screen) {
>   XGetWindowAttributes(x->dpy, x->window, );
>   XTranslateCoordinates(x->dpy, x->window, x->rwin,
> @@ -536,8 +588,8 @@ resize_window(struct video *vid, int ful
>   x->saved_h = x->height = winatt.height;
>   x->saved_x = new_x;
>   x->saved_y = new_y;
> -  XResizeWindow(x->dpy, x->window, x->width, x->height);
> }
> +
>  XSync(x->dpy, False);
>  XSync(x->dpy, True);
> }
> @@ -1435,6 +1487,9 @@ setup(struct video *vid)
>   if (!mmap_init(vid))
>return 0;
> }
> +
> +

Re: timeout_set_proc(9)

2016-09-25 Thread David Gwynne

> On 26 Sep 2016, at 13:36, Ted Unangst  wrote:
> 
> David Gwynne wrote:
>> +mtx_enter(_mutex);
>> +while (!CIRCQ_EMPTY(_proc)) {
>> +to = timeout_from_circq(CIRCQ_FIRST(_proc));
>> +CIRCQ_REMOVE(>to_list);
>   leave();
>> +timeout_run(to);
>   enter();
>> +}
>> +mtx_leave(_mutex);
> 
> you need to drop the mutex when running the timeout. with those changes, looks
> pretty good.

timeout_run drops the mutex.


Re: timeout_set_proc(9)

2016-09-25 Thread Ted Unangst
David Gwynne wrote:
> + mtx_enter(_mutex);
> + while (!CIRCQ_EMPTY(_proc)) {
> + to = timeout_from_circq(CIRCQ_FIRST(_proc));
> + CIRCQ_REMOVE(>to_list);
leave();
> + timeout_run(to);
enter();
> + }
> + mtx_leave(_mutex);

you need to drop the mutex when running the timeout. with those changes, looks
pretty good.



Re: timeout_set_proc(9)

2016-09-25 Thread David Gwynne
On Sat, Sep 24, 2016 at 12:20:02PM +0200, Christiano F. Haesbaert wrote:
> Am Samstag, 24. September 2016 schrieb David Gwynne :
> 
> > On Fri, Sep 23, 2016 at 10:16:34PM -0700, Philip Guenther wrote:
> > > On Fri, 23 Sep 2016, Christiano F. Haesbaert wrote:
> > > ...
> > > > The diff as it is will deadlock against SCHED_LOCK.
> > > > tsleep() calls sleep_setup(), which grabs SCHED_LOCK,
> > > > Then sleep_setup_timeout() will grab timeout_mutex in timeout_add()
> > > >
> > > > On softclock() you have the opposite:
> > > > Grabs timeout_mutex then does a wakeup, which grabs SCHED_LOCK.
> >
> > nice.
> >
> > >
> > > Hmm, yes. And softclock_thread() has the same problem: msleep() needs to
> > > grab SCHED_LOCK while the passed in mutex is held, so it'll hold
> > > timeout_mutex while grabbing SCHED_LOCK too.
> > >
> > > I just played around with using a separate mutex for protecting
> > > timeout_proc, a mutex which would be 'outside' SCHED_LOCK, unlike
> > > timeout_mutex which is 'inside' SCHED_LOCK.  The catch is that supporting
> > > all the functionality of timeout_add() and timeout_del() becomes ugly and
> > > fragile: they would need to check whether the mutex has
> > > TIMEOUT_NEEDPROCCTX set and, if so, grab the new mutex before grabbing
> > > timeout_mutex. ??That's "safe" from being a lock loop from tsleep()
> > because
> > > the thread's p_sleep_to *can't* have that flag set, so the 'outside'
> > mutex
> > > wouldn't be neededbut geez is this ugly.  I'm also unsure what
> > defined
> > > semantics, if any, timeout_triggered() should have for NEEDPROCCTX
> > > timeouts.  Should it return true once the timeout has been dequeued from
> > > the timeout wheel, or should it only be set once softclock_thread is
> > > actually going to run it?
> > >
> > >
> > > ...or maybe this makes people think we should toss this out and go
> > > directly to dlg@'s proposal...
> >
> > let's not go crazy.
> >
> > i believe the deadlock can be fixed by moving the wakeup outside
> > the hold of timeout_mutex in timeout_add. you only need to wake up
> > the thread once even if you queued multiple timeouts, cos the thread
> > will loop until it completes all pending work. think of this as
> > interrupt mitigation.
> 
> 
> Yes, that was my solution as well, i don't have access to the code right
> now, but I think this won't fix the msleep case guenther pointed out.

yeah. that one took me a bit longer to understand.

the below includes my first diff, but also gets rid of the hold of
the timeout mutex while waiting for entries in the timeout_proc
list. it basically sleeps on CIRCQ_EMPTY outside the lock.

Index: kern/kern_timeout.c
===
RCS file: /cvs/src/sys/kern/kern_timeout.c,v
retrieving revision 1.49
diff -u -p -r1.49 kern_timeout.c
--- kern/kern_timeout.c 22 Sep 2016 12:55:24 -  1.49
+++ kern/kern_timeout.c 26 Sep 2016 02:56:21 -
@@ -375,6 +375,7 @@ softclock(void *arg)
int delta;
struct circq *bucket;
struct timeout *to;
+   int needsproc = 0;
 
mtx_enter(_mutex);
while (!CIRCQ_EMPTY(_todo)) {
@@ -391,7 +392,7 @@ softclock(void *arg)
CIRCQ_INSERT(>to_list, bucket);
} else if (to->to_flags & TIMEOUT_NEEDPROCCTX) {
CIRCQ_INSERT(>to_list, _proc);
-   wakeup(_proc);
+   needsproc = 1;
} else {
 #ifdef DEBUG
if (delta < 0)
@@ -401,6 +402,9 @@ softclock(void *arg)
}
}
mtx_leave(_mutex);
+
+   if (needsproc)
+   wakeup(_proc);
 }
 
 void
@@ -415,6 +419,7 @@ softclock_thread(void *arg)
 {
CPU_INFO_ITERATOR cii;
struct cpu_info *ci;
+   struct sleep_state sls;
struct timeout *to;
 
KERNEL_ASSERT_LOCKED();
@@ -427,16 +432,18 @@ softclock_thread(void *arg)
KASSERT(ci != NULL);
sched_peg_curproc(ci);
 
-   mtx_enter(_mutex);
for (;;) {
-   while (CIRCQ_EMPTY(_proc))
-   msleep(_proc, _mutex, PSWP, "bored", 0);
+   sleep_setup(, _proc, PSWP, "bored");
+   sleep_finish(, CIRCQ_EMPTY(_proc));
 
-   to = timeout_from_circq(CIRCQ_FIRST(_proc));
-   CIRCQ_REMOVE(>to_list);
-   timeout_run(to);
+   mtx_enter(_mutex);
+   while (!CIRCQ_EMPTY(_proc)) {
+   to = timeout_from_circq(CIRCQ_FIRST(_proc));
+   CIRCQ_REMOVE(>to_list);
+   timeout_run(to);
+   }
+   mtx_leave(_mutex);
}
-   mtx_leave(_mutex);
 }
 
 #ifndef SMALL_KERNEL



Re: etherip alignment issues

2016-09-25 Thread YASUOKA Masahiko
Hi,

I could repeat the problem on my octeon.  Tested the diff on it.

ok?

Use a new mbufs for a tunnel header to make sure it is aligned correctly.

Index: sys/net/if_etherip.c
===
RCS file: /cvs/src/sys/net/if_etherip.c,v
retrieving revision 1.7
diff -u -p -r1.7 if_etherip.c
--- sys/net/if_etherip.c13 Apr 2016 11:41:15 -  1.7
+++ sys/net/if_etherip.c26 Sep 2016 01:20:08 -
@@ -354,6 +354,7 @@ ip_etherip_output(struct ifnet *ifp, str
struct sockaddr_in *src, *dst;
struct etherip_header *eip;
struct ip *ip;
+   struct mbuf *m0;
 
src = (struct sockaddr_in *)>sc_src;
dst = (struct sockaddr_in *)>sc_dst;
@@ -370,24 +371,25 @@ ip_etherip_output(struct ifnet *ifp, str
 
m->m_flags &= ~(M_BCAST|M_MCAST);
 
-   M_PREPEND(m, sizeof(struct etherip_header), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
+   MGETHDR(m0, M_DONTWAIT, MT_HEADER);
+   if (m0 == NULL) {
+   m_freem(m);
return ENOBUFS;
}
-   eip = mtod(m, struct etherip_header *);
-   eip->eip_ver = ETHERIP_VERSION;
-   eip->eip_res = 0;
-   eip->eip_pad = 0;
+   M_MOVE_HDR(m0, m);
+   m0->m_len = sizeof(struct ip) + sizeof(struct etherip_header);
+   m0->m_pkthdr.len += m0->m_len;
+   m0->m_next = m;
+   m = m0;
 
-   M_PREPEND(m, sizeof(struct ip), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
-   return ENOBUFS;
-   }
ip = mtod(m, struct ip *);
memset(ip, 0, sizeof(struct ip));
 
+   eip = (struct etherip_header *)(ip + 1);
+   eip->eip_ver = ETHERIP_VERSION;
+   eip->eip_res = 0;
+   eip->eip_pad = 0;
+
ip->ip_v = IPVERSION;
ip->ip_hl = sizeof(struct ip) >> 2;
ip->ip_id = htons(ip_randomid());
@@ -514,6 +516,7 @@ ip6_etherip_output(struct ifnet *ifp, st
struct sockaddr_in6 *src, *dst;
struct etherip_header *eip;
struct ip6_hdr *ip6;
+   struct mbuf *m0;
 
src = (struct sockaddr_in6 *)>sc_src;
dst = (struct sockaddr_in6 *)>sc_dst;
@@ -530,22 +533,24 @@ ip6_etherip_output(struct ifnet *ifp, st
 
m->m_flags &= ~(M_BCAST|M_MCAST);
 
-   M_PREPEND(m, sizeof(struct etherip_header), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
+   MGETHDR(m0, M_DONTWAIT, MT_HEADER);
+   if (m0 == NULL) {
+   m_freem(m);
return ENOBUFS;
}
-   eip = mtod(m, struct etherip_header *);
+   M_MOVE_PKTHDR(m0, m);
+   m0->m_len = sizeof(struct ip6_hdr) + sizeof(struct etherip_header);
+   m0->m_pkthdr.len += m0->m_len;
+   m0->m_next = m;
+   m = m0;
+
+   ip6 = mtod(m, struct ip6_hdr *);
+
+   eip = (struct etherip_header *)(ip6 + 1);
eip->eip_ver = ETHERIP_VERSION;
eip->eip_res = 0;
eip->eip_pad = 0;
 
-   M_PREPEND(m, sizeof(struct ip6_hdr), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
-   return ENOBUFS;
-   }
-   ip6 = mtod(m, struct ip6_hdr *);
ip6->ip6_flow = 0;
ip6->ip6_vfc &= ~IPV6_VERSION_MASK;
ip6->ip6_vfc |= IPV6_VERSION;



Re: clang and

2016-09-25 Thread David Gwynne

> On 25 Sep 2016, at 01:32, Mark Kettenis  wrote:
> 
> clang warns by default about static functions that are unused.  It
> does this even for static inline functions, except when those
> functions are defined in a header file.  The RBT code in 
> has macros that define static inline functions.  Unfortunately that
> means that the inline functions generated by those macros trigger the
> warning when they're not used.  The diff below marks them as __unused,
> to suppress this warning.  With this change, both gcc and clang only
> emit the functions that are actually used.
> 
> ok?

ok.

> 
> 
> Index: sys/tree.h
> ===
> RCS file: /cvs/src/sys/sys/tree.h,v
> retrieving revision 1.24
> diff -u -p -r1.24 tree.h
> --- sys/tree.h15 Sep 2016 06:07:22 -  1.24
> +++ sys/tree.h24 Sep 2016 15:26:41 -
> @@ -823,97 +823,97 @@ int  _rb_check(const struct rb_type *, v
> #define RBT_PROTOTYPE(_name, _type, _field, _cmp) \
> extern const struct rb_type *const _name##_RBT_TYPE;  \
>   \
> -static inline void   \
> +__unused static inline void  \
> _name##_RBT_INIT(struct _name *head)  \
> { \
>   _rb_init(>rbh_root);  \
> } \
>   \
> -static inline struct _type * \
> +__unused static inline struct _type *
> \
> _name##_RBT_INSERT(struct _name *head, struct _type *elm) \
> { \
>   return _rb_insert(_name##_RBT_TYPE, >rbh_root, elm);  \
> } \
>   \
> -static inline struct _type * \
> +__unused static inline struct _type *
> \
> _name##_RBT_REMOVE(struct _name *head, struct _type *elm) \
> { \
>   return _rb_remove(_name##_RBT_TYPE, >rbh_root, elm);  \
> } \
>   \
> -static inline struct _type * \
> +__unused static inline struct _type *
> \
> _name##_RBT_FIND(struct _name *head, const struct _type *key) \
> { \
>   return _rb_find(_name##_RBT_TYPE, >rbh_root, key);\
> } \
>   \
> -static inline struct _type * \
> +__unused static inline struct _type *
> \
> _name##_RBT_NFIND(struct _name *head, const struct _type *key)
> \
> { \
>   return _rb_nfind(_name##_RBT_TYPE, >rbh_root, key);   \
> } \
>   \
> -static inline struct _type * \
> +__unused static inline struct _type *
> \
> _name##_RBT_ROOT(struct _name *head)  \
> { \
>   return _rb_root(_name##_RBT_TYPE, >rbh_root); \
> } \
>   \
> -static inline int\
> +__unused static inline int   \
> _name##_RBT_EMPTY(struct _name *head) \
> { \
>   return _rb_empty(>rbh_root);  \
> } \
>   \
> -static inline struct _type * \
> +__unused static inline struct _type *  

Re: video(1): use _NET_WM_STATE_FULLSCREEN

2016-09-25 Thread Dmitrij D. Czarkoff
>The diff below fixes fullscreen mode on window managers that prevent
>applications from resizing their windows.  This is done by setting
>_NET_WM_STATE_FULLSCREEN atom.

Ping.

--
Dmitrij D. Czarkoff

Index: video.c
===
RCS file: /var/cvs/xenocara/app/video/video.c,v
retrieving revision 1.19
diff -u -p -r1.19 video.c
--- video.c6 Jun 2016 19:31:22 -   1.19
+++ video.c18 Sep 2016 21:33:29 -
@@ -160,6 +160,7 @@ struct video {
  int   conv_type;
  int   enc;
  int   full_screen;
+  int  net_wm;
  int   width;
  int   height;
  int   bpf;
@@ -178,6 +179,7 @@ int xv_get_info(struct video *);
 int xv_sel_adap(struct video *);
 void xv_dump_info(struct video *);
 int xv_init(struct video *);
+void net_wm_supported(struct video *);
 void resize_window(struct video *, int);
 void display_event(struct video *);
 
@@ -500,34 +502,84 @@ xv_init(struct video *vid)
}
 
 void
+net_wm_supported(struct video *vid)
+{
+  struct xdsp *x = >xdsp;
+  Atom query, fullscreen;
+  Atom type;
+  Atom *data;
+  long off;
+  int fmt, len = 12;
+  unsigned long nitems, remain;
+  int i;
+
+  query = XInternAtom(x->dpy, "_NET_SUPPORTED", True);
+  fullscreen = XInternAtom(x->dpy, "_NET_WM_STATE_FULLSCREEN", \
True);
+
+  for (off = 0; XGetWindowProperty(x->dpy, x->rwin, query, off,
+len, False, XA_ATOM, , , \
, ,
+(unsigned char **)) == Success; \
off += len) {
+  if (type == XA_ATOM && fmt == 32) {
+  for (i = 0; i < nitems; i++) {
+  if (data[i] == fullscreen) {
+  vid->net_wm = 1;
+  XFree(data);
+  return;
+  }
+  }
+  }
+  XFree(data);
+  }
+
+  return;
+}
+
+void
 resize_window(struct video *vid, int fullscreen)
 {
  struct xdsp *x = >xdsp;
  XWindowAttributes winatt;
  Window junk;
-  int new_width;
-  int new_height;
  int new_x;
  int new_y;
+  XEvent ev;
+  Atom property, value;
 
  if (fullscreen == 1) {
   if (vid->full_screen == 1) {
-  new_width = x->saved_w;
-  new_height = x->saved_h;
+  x->width = x->saved_w;
+  x->height = x->saved_h;
new_x = x->saved_x;
new_y = x->saved_y;
-  vid->full_screen = 0;
} else {
-  new_width = x->max_width;
-  new_height = x->max_height;
+  x->width = x->max_width;
+  x->height = x->max_height;
new_x = 0;
new_y = 0;
-  vid->full_screen = 1;
}
-  x->width = new_width;
-  x->height = new_height;
-  XMoveResizeWindow(x->dpy, x->window, new_x, new_y,
-  new_width, new_height);
+
+  vid->full_screen = !vid->full_screen;
+
+  if (vid->net_wm) {
+  property = XInternAtom(x->dpy, "_NET_WM_STATE", \
False);
+  value = XInternAtom(x->dpy, "_NET\
_WM_STATE_FULLSCREEN",
+  False);
+
+  memset(, 0, sizeof(ev));
+  ev.type = ClientMessage;
+  ev.xclient.window = x->window;
+  ev.xclient.message_type = property;
+  ev.xclient.format = 32;
+  ev.xclient.data.l[0] = vid->full_screen;
+  ev.xclient.data.l[1] = value;
+
+  XSendEvent(x->dpy, x->rwin, False,
+  SubstructureNotifyMask | Subs\
tructureRedirectMask,
+  );
+  } else {
+  XMoveResizeWindow(x->dpy, x->window, x->width,
+  x->height, new_x, new_y);
+  }
} else if (!vid->full_screen) {
   XGetWindowAttributes(x->dpy, x->window, );
   XTranslateCoordinates(x->dpy, x->window, x->rwin,
@@ -536,8 +588,8 @@ resize_window(struct video *vid, int ful
   x->saved_h = x->height = winatt.height;
   x->saved_x = new_x;
   x->saved_y = new_y;
-  XResizeWindow(x->dpy, x->window, x->width, x->height);
}
+
  XSync(x->dpy, False);
  XSync(x->dpy, True);
}
@@ -1435,6 +1487,9 @@ setup(struct video *vid)
   if (!mmap_init(vid))
return 0;
}
+
+  if (vid->mode & M_OUT_XV)
+  net_wm_supported(vid);
 
  return 1;
}



cdce(4): Move include within the #if NBPFILTER directive

2016-09-25 Thread Frederic Cambus
Hi tech@,

Move the net/bpf.h include within the #if NBPFILTER directive.

Index: sys/dev/usb/if_cdce.c
===
RCS file: /cvs/src/sys/dev/usb/if_cdce.c,v
retrieving revision 1.70
diff -u -p -r1.70 if_cdce.c
--- sys/dev/usb/if_cdce.c   13 Apr 2016 11:03:37 -  1.70
+++ sys/dev/usb/if_cdce.c   25 Sep 2016 19:46:43 -
@@ -52,8 +52,8 @@
 
 #include 
 
-#include 
 #if NBPFILTER > 0
+#include 
 #endif
 
 #include 



Re: /usr/share/man owner fixes for noperm builds

2016-09-25 Thread Martin Natano
On Sun, Sep 25, 2016 at 01:14:17AM +0200, Theo Buehler wrote:
> usr/share/mandoc.db is currently installed as the build user:
> 
> -rw-r--r--   1 builder  wheel  396748 Sep 24 23:07 mandoc.db
> 
> bsd.own.mk defines MANOWN=root, MANGRP=bin, MANMODE=444 and uses these
> for installing the manpages themselves. Therefore I think we should use
> these variables rather than using BINOWN, BINGRP and hardcoded 444 for
> /usr/share/man/COPYRIGHT, and it makes more sense to me to have the
> mandoc.db owned by root:bin as well. The diff below does this.
> 
> However, the /etc/weekly script will change the ownership of mandoc.db
> to root:wheel (the current owner of the parent /usr/share/man), so I
> wonder if the entire man hierarchies should be switched to be owned by
> root:bin in the /etc/mtree/*BSD*.dist files. But that's a lot of churn
> and the benefit is unclear to me.
> 
> On the other hand, just doing 'chown root:wheel' for mandoc.db and be
> done with it would be a lot simpler...
> 
> Index: share/man/Makefile
> ===
> RCS file: /cvs/src/share/man/Makefile,v
> retrieving revision 1.10
> diff -u -p -r1.10 Makefile
> --- share/man/Makefile18 Apr 2014 10:00:48 -  1.10
> +++ share/man/Makefile24 Sep 2016 22:39:03 -
> @@ -4,10 +4,11 @@
>  SUBDIR=  man1 man3 man4 man5 man6 man7 man8 man9
>  
>  afterinstall:
> - ${INSTALL} -c -o ${BINOWN} -g ${BINGRP} -m 444 man0/COPYRIGHT \
> - ${DESTDIR}/usr/share/man/COPYRIGHT
> + ${INSTALL} ${INSTALL_COPY} -o ${MANOWN} -g ${MANGRP} -m ${MANMODE} \
> + man0/COPYRIGHT ${DESTDIR}/usr/share/man/COPYRIGHT
>  
>  makedb:
>   /usr/sbin/makewhatis -Qv ${DESTDIR}/usr/share/man
> + chown ${MANOWN}:${MANGRP} ${DESTDIR}/usr/share/man/mandoc.db

I think this should be root:wheel as long as /etc/weekly will reset it
to that. With root:wheel: OK.


>  
>  .include 
> 



Re: makes struct kinfo_file to provide va_nlink

2016-09-25 Thread Sebastien Marie
On Sat, Sep 24, 2016 at 04:35:39PM -0700, Philip Guenther wrote:
> On Sat, 24 Sep 2016, Sebastien Marie wrote:
> ...
> > The following diff adds a `va_nlink' member in `struct kinfo_file'. The
> > information become available though sysctl(3) via KERN_FILE interface,
> > as some others members of `struct vattr'.
> 
> The idea seems sound, but the placement of the member in the structure is 
> a problem:
> 
> > --- sys/sys/sysctl.h21 Sep 2016 14:06:50 -  1.166
> > +++ sys/sys/sysctl.h24 Sep 2016 11:26:25 -
> > @@ -697,6 +697,7 @@ struct kinfo_file {
> > uint64_tva_size;/* UINT64_T: file size in bytes */
> > uint32_tva_mode;/* MODE_T: file access mode and type */
> > uint32_tva_fsid;/* DEV_T: filesystem device */
> > +   uint32_tva_nlink;   /* NLINK_T: number of references to 
> > file */
> > charf_mntonname[KI_MNAMELEN];
> 
> That changes the layout of the structure, changing the ABI.  If instead 
> the new member is placed at the end of structure then there's no ABI 
> break, as the API permits additions to the end of structure.
> 
> Also, libkvm/kvm_file2.c should be updated to match the change to 
> kern_sysctl.c, so that the information is returned when debugging kernel 
> crash dumps.

I updated my diff:
  - adding the new member at end of kinfo_file : no more ABI change.
  - updating libkvm to match the change

One remark about libkvm changes: msdos and ntfs doesn't track va_nlink
in kernel (always set it to 1). So I keep the same behaviour in libkvm.
Maybe it could be revised later (and update kernel and libkvm to track
at least unlinking).

For the other part (adding mmap references to files to KERN_FILE API), I
need more time to understand the internals and make a diff.

> > Formally, for the final purpose, the information could be only partial: 
> > a library dynamically linked isn't referenced via KERN_FILE interface (I 
> > am unsure about the availability of the information). But as proper 
> > packages will have their signature changed by any library crank, a 
> > package update will make old program to be remplaced by a new one.
> 
> Yeah, mmap's hold references to files, but the KERN_FILE API doesn't see 
> them.  To get that you would need to walk the maps in the uvmspace and 
> pick out the non-anonymous mappings, skipping the base executable (which 
> _is_ already returned by KERN_FILE).  Doing that without races in the 
> kernel would be tricky, as copyout() can sleep if there's a page fault.  
> Could you deadlock the kernel by asking for information about your own 
> mappings, with sysctl() writing to a mapping which would need to be 
> faulted in?
> 
> Perhaps better to not hold a lock over copyout() but instead just track 
> the vaddr range that was being reported, and then restart the tree walk 
> from there.  That guarantees progress with no locks held.
> 

-- 
Sebastien Marie


Index: lib/libkvm/kvm_cd9660.c
===
RCS file: /cvs/src/lib/libkvm/kvm_cd9660.c,v
retrieving revision 1.6
diff -u -p -r1.6 kvm_cd9660.c
--- lib/libkvm/kvm_cd9660.c 16 Dec 2014 03:21:10 -  1.6
+++ lib/libkvm/kvm_cd9660.c 25 Sep 2016 15:50:08 -
@@ -50,6 +50,7 @@ _kvm_stat_cd9660(kvm_t *kd, struct kinfo
kf->va_mode = inode.inode.iso_mode;
kf->va_size = inode.i_size;
kf->va_rdev = inode.i_dev;
+   kf->va_nlink = inode.inode.iso_links;
 
return (0);
 }
Index: lib/libkvm/kvm_file2.c
===
RCS file: /cvs/src/lib/libkvm/kvm_file2.c,v
retrieving revision 1.49
diff -u -p -r1.49 kvm_file2.c
--- lib/libkvm/kvm_file2.c  4 May 2016 01:28:42 -   1.49
+++ lib/libkvm/kvm_file2.c  25 Sep 2016 15:50:08 -
@@ -798,6 +798,7 @@ ufs_filestat(kvm_t *kd, struct kinfo_fil
kf->va_mode = inode.i_ffs1_mode;
kf->va_size = inode.i_ffs1_size;
kf->va_rdev = inode.i_ffs1_rdev;
+   kf->va_nlink = inode.i_ffs1_nlink;
 
return (0);
 }
@@ -826,6 +827,7 @@ ext2fs_filestat(kvm_t *kd, struct kinfo_
kf->va_mode = inode.i_e2fs_mode;
kf->va_size = inode.i_e2fs_size;
kf->va_rdev = 0;/* XXX */
+   kf->va_nlink = inode.i_e2fs_nlink;
 
return (0);
 }
@@ -851,6 +853,7 @@ msdos_filestat(kvm_t *kd, struct kinfo_f
kf->va_mode = (mp.pm_mask & 0777) | _kvm_getftype(vp->v_type);
kf->va_size = de.de_FileSize;
kf->va_rdev = 0;  /* msdosfs doesn't support device files */
+   kf->va_nlink = 1;
 
return (0);
 }
@@ -870,6 +873,7 @@ nfs_filestat(kvm_t *kd, struct kinfo_fil
kf->va_size = nfsnode.n_size;
kf->va_rdev = nfsnode.n_vattr.va_rdev;
kf->va_mode = (mode_t)nfsnode.n_vattr.va_mode | 
_kvm_getftype(vp->v_type);
+   kf->va_nlink = nfsnode.n_vattr.va_nlink;
 
return (0);
 }
Index: lib/libkvm/kvm_ntfs.c

Re: Remove more duplicated includes

2016-09-25 Thread Frederic Cambus
On Fri, Sep 23, 2016 at 08:02:03PM +0200, Jeremie Courreges-Anglas wrote:
> >
> > A few remaining duplicated includes to remove.
> >
> > Comments? OK?
> 
> ok except for ixgbe.h
> 
> Better remove the unguarded #include, I think.  Even if a kernel
> configured without BPFILTER will not build...

That makes sense, here is an updated diff. OK?

Index: sys/arch/sparc64/include/asm.h
===
RCS file: /cvs/src/sys/arch/sparc64/include/asm.h,v
retrieving revision 1.10
diff -u -p -r1.10 asm.h
--- sys/arch/sparc64/include/asm.h  27 May 2016 16:32:39 -  1.10
+++ sys/arch/sparc64/include/asm.h  25 Sep 2016 13:46:14 -
@@ -41,11 +41,6 @@
 #ifndef _MACHINE_ASM_H_
 #define _MACHINE_ASM_H_
 
-#ifndef _LOCORE
-#define _LOCORE
-#endif
-#include 
-
 /* Pull in CCFSZ, CC64FSZ, and BIAS from frame.h */
 #ifndef _LOCORE
 #define _LOCORE
Index: sys/dev/pci/ixgbe.h
===
RCS file: /cvs/src/sys/dev/pci/ixgbe.h,v
retrieving revision 1.22
diff -u -p -r1.22 ixgbe.h
--- sys/dev/pci/ixgbe.h 15 Mar 2016 16:45:52 -  1.22
+++ sys/dev/pci/ixgbe.h 25 Sep 2016 13:46:15 -
@@ -55,7 +55,6 @@
 #include 
 
 #include 
-#include 
 #include 
 
 #include 
Index: usr.sbin/snmpd/snmpd.h
===
RCS file: /cvs/src/usr.sbin/snmpd/snmpd.h,v
retrieving revision 1.67
diff -u -p -r1.67 snmpd.h
--- usr.sbin/snmpd/snmpd.h  16 Aug 2016 18:41:57 -  1.67
+++ usr.sbin/snmpd/snmpd.h  25 Sep 2016 13:46:17 -
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 



Re: etherip alignment issues

2016-09-25 Thread YASUOKA Masahiko
On Sat, 24 Sep 2016 16:45:09 -0400 (EDT)
Martin Brandenburg  wrote:
> On Sun, 25 Sep 2016, YASUOKA Masahiko wrote:
> 
>> On Sat, 24 Sep 2016 13:08:18 -0400 (EDT)
>> Martin Brandenburg  wrote:
>> > On Sat, 24 Sep 2016, YASUOKA Masahiko wrote:
>> >> The problem doesn't repeat on my Octeon.
>> >>
>> >> Can you try the diff below?
>> >>
>> >>   - I assume the diff fixes the problem
>> >>   - A kernel message is added.  please let me know if it appears.
>> >
>> > I got the message, but had another panic.
>> >
>> > ip_etherip_output: leading space is small 2
>>
>> Thanks.  The diff was wrong.  I'm sorry.
>>
>> Let me update the diff.
> 
> Still panicking.

Sorry again.  Can try the diff below?

Finally I understand that any precondition about m->m_data alignment
cannot be assumed.

diff --git a/sys/net/if_etherip.c b/sys/net/if_etherip.c
index ad58c52..74dba63 100644
--- a/sys/net/if_etherip.c
+++ b/sys/net/if_etherip.c
@@ -354,6 +354,8 @@ ip_etherip_output(struct ifnet *ifp, struct mbuf *m)
struct sockaddr_in *src, *dst;
struct etherip_header *eip;
struct ip *ip;
+   struct mbuf *m1;
+   int len;
 
src = (struct sockaddr_in *)>sc_src;
dst = (struct sockaddr_in *)>sc_dst;
@@ -370,24 +372,27 @@ ip_etherip_output(struct ifnet *ifp, struct mbuf *m)
 
m->m_flags &= ~(M_BCAST|M_MCAST);
 
-   M_PREPEND(m, sizeof(struct etherip_header), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
+   len = sizeof(struct ip) + sizeof(struct etherip_header);
+   MGET(m1, M_DONTWAIT, m->m_type);
+   if (m1 == NULL) {
+   m_freem(m);
return ENOBUFS;
}
-   eip = mtod(m, struct etherip_header *);
-   eip->eip_ver = ETHERIP_VERSION;
-   eip->eip_res = 0;
-   eip->eip_pad = 0;
+   M_MOVE_PKTHDR(m1, m);
+   m1->m_next = m;
+   m = m1;
+   m->m_len = len;
+   m->m_pkthdr.len += len;
 
-   M_PREPEND(m, sizeof(struct ip), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
-   return ENOBUFS;
-   }
ip = mtod(m, struct ip *);
memset(ip, 0, sizeof(struct ip));
 
+   eip = (struct etherip_header *)(ip + 1);
+
+   eip->eip_ver = ETHERIP_VERSION;
+   eip->eip_res = 0;
+   eip->eip_pad = 0;
+
ip->ip_v = IPVERSION;
ip->ip_hl = sizeof(struct ip) >> 2;
ip->ip_id = htons(ip_randomid());
@@ -514,6 +519,8 @@ ip6_etherip_output(struct ifnet *ifp, struct mbuf *m)
struct sockaddr_in6 *src, *dst;
struct etherip_header *eip;
struct ip6_hdr *ip6;
+   struct mbuf *m1;
+   int len;
 
src = (struct sockaddr_in6 *)>sc_src;
dst = (struct sockaddr_in6 *)>sc_dst;
@@ -530,22 +537,24 @@ ip6_etherip_output(struct ifnet *ifp, struct mbuf *m)
 
m->m_flags &= ~(M_BCAST|M_MCAST);
 
-   M_PREPEND(m, sizeof(struct etherip_header), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
+   MGET(m1, M_DONTWAIT, m->m_type);
+   if (m1 == NULL) {
+   m_freem(m);
return ENOBUFS;
}
-   eip = mtod(m, struct etherip_header *);
+   M_MOVE_PKTHDR(m1, m);
+   m1->m_next = m;
+   m = m1;
+   m->m_len = len;
+   m->m_pkthdr.len += len;
+
+   ip6 = mtod(m, struct ip6_hdr *);
+   eip = (struct etherip_header *)(ip6 + 1);
+
eip->eip_ver = ETHERIP_VERSION;
eip->eip_res = 0;
eip->eip_pad = 0;
 
-   M_PREPEND(m, sizeof(struct ip6_hdr), M_DONTWAIT);
-   if (m == NULL) {
-   etheripstat.etherip_adrops++;
-   return ENOBUFS;
-   }
-   ip6 = mtod(m, struct ip6_hdr *);
ip6->ip6_flow = 0;
ip6->ip6_vfc &= ~IPV6_VERSION_MASK;
ip6->ip6_vfc |= IPV6_VERSION;