Re: CVS commit: src/sys/dev/pci
On 30/11/2018 17:47, Jaromir Dolecek wrote:
Module Name:src
Committed By: jdolecek
Date: Fri Nov 30 17:47:54 UTC 2018
Modified Files:
src/sys/dev/pci: ahcisata_pci.c xhci_pci.c
Log Message:
simplify intr establish code - rely on pci_intr_alloc() to allow
also MSI-X, and to return interrupt types which are possible for
pci_intr_establish(); remove fallbacks to retry with MSI/MSI-X
explicitly disabled
discussed on tech-kern@
https://mail-index.netbsd.org/tech-kern/2018/11/27/msg024240.html
err, RIP *_DISABLE_MSI{,X}?
delete them from sys/dev/pci/files.pci?
Nick
Re: CVS commit: src/sys
On Fri, Nov 30, 2018 at 13:59:11 +0100, Martin Husemann wrote: > On Fri, Nov 30, 2018 at 07:47:38AM -0500, Christos Zoulas wrote: > > We need to see if the spec says so. I am not sure if we can depend > > that an implementation does it. > > The spec only talks about members not named in the list - I can't see > any hints about padding. > > Might be worth to ask some std gurus for clarification. I'm not one, but search for "padding" in 6.2.6.1 General (in 6.2.6 Representations of types). [#6] When a value is stored in an object of structure or union type, including in a member object, the bytes of the object representation that correspond to any padding bytes take unspecified values.42) 42)Thus, for example, structure assignment may be implemented element-at-a-time or via memcpy. so padding may contain garbage. -uwe
Re: CVS commit: src/sys
On Fri, Nov 30, 2018 at 07:47:38AM -0500, Christos Zoulas wrote: > We need to see if the spec says so. I am not sure if we can depend > that an implementation does it. The spec only talks about members not named in the list - I can't see any hints about padding. Might be worth to ask some std gurus for clarification. Martin
Re: CVS commit: src/sys
On Nov 30, 9:09am, mar...@duskware.de (Martin Husemann) wrote: -- Subject: Re: CVS commit: src/sys | On Thu, Nov 29, 2018 at 10:07:11PM +, Christos Zoulas wrote: | > >The pool is just called three times, so it's not a big deal. | > | > I don't know, from reading the manual page using the pool_cache_ api seems | > to be the preferred way... Plus I don't think that a language deficiency | > should be visible in many places in the code (these memsets appear | > unnecessary) and if we do them using the pool_cache code we pay the cost | > once (at construction), not every use (however minor that cost is). | | But it comes at a cost (the per-CPU cache) - how often is this code run? | | Does using compound literals clear the gaps? Example of what I mean | below (incomplete, just for clarification) We need to see if the spec says so. I am not sure if we can depend that an implementation does it. christos
Re: CVS commit: src/sys
On Thu, Nov 29, 2018 at 10:07:11PM +, Christos Zoulas wrote:
> >The pool is just called three times, so it's not a big deal.
>
> I don't know, from reading the manual page using the pool_cache_ api seems
> to be the preferred way... Plus I don't think that a language deficiency
> should be visible in many places in the code (these memsets appear
> unnecessary) and if we do them using the pool_cache code we pay the cost
> once (at construction), not every use (however minor that cost is).
But it comes at a cost (the per-CPU cache) - how often is this code run?
Does using compound literals clear the gaps? Example of what I mean
below (incomplete, just for clarification)
Martin
Index: kern_time.c
===
RCS file: /cvsroot/src/sys/kern/kern_time.c,v
retrieving revision 1.192
diff -u -p -r1.192 kern_time.c
--- kern_time.c 28 Nov 2018 15:10:40 - 1.192
+++ kern_time.c 30 Nov 2018 08:03:52 -
@@ -593,6 +593,7 @@ timer_create1(timer_t *tid, clockid_t id
struct ptimers *pts;
struct ptimer *pt;
struct proc *p;
+ struct sigevent ev;
p = l->l_proc;
@@ -602,20 +603,19 @@ timer_create1(timer_t *tid, clockid_t id
if ((pts = p->p_timers) == NULL)
pts = timers_alloc(p);
- pt = pool_get(_pool, PR_WAITOK);
- memset(pt, 0, sizeof(*pt));
if (evp != NULL) {
if (((error =
- (*fetch_event)(evp, >pt_ev, sizeof(pt->pt_ev))) != 0) ||
- ((pt->pt_ev.sigev_notify < SIGEV_NONE) ||
- (pt->pt_ev.sigev_notify > SIGEV_SA)) ||
- (pt->pt_ev.sigev_notify == SIGEV_SIGNAL &&
-(pt->pt_ev.sigev_signo <= 0 ||
- pt->pt_ev.sigev_signo >= NSIG))) {
- pool_put(_pool, pt);
+ (*fetch_event)(evp, , sizeof(ev))) != 0) ||
+ ((ev.sigev_notify < SIGEV_NONE) ||
+ (ev.sigev_notify > SIGEV_SA)) ||
+ (ev.sigev_notify == SIGEV_SIGNAL &&
+(ev.sigev_signo <= 0 ||
+ ev.sigev_signo >= NSIG))) {
return (error ? error : EINVAL);
}
}
+ pt = pool_get(_pool, PR_WAITOK);
+ *pt = (struct ptimer) { .pt_ev = ev, };
/* Find a free timer slot, skipping those reserved for setitimer(). */
mutex_spin_enter(_lock);
