From owner-p4-projects@FreeBSD.ORG Sun Jan 7 23:54:13 2007 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 69CDD16A417; Sun, 7 Jan 2007 23:54:13 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 465F516A403 for ; Sun, 7 Jan 2007 23:54:13 +0000 (UTC) (envelope-from piso@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [69.147.83.41]) by mx1.freebsd.org (Postfix) with ESMTP id 369D813C448 for ; Sun, 7 Jan 2007 23:54:13 +0000 (UTC) (envelope-from piso@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id l07NsDOS073365 for ; Sun, 7 Jan 2007 23:54:13 GMT (envelope-from piso@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id l07NsC0x073362 for perforce@freebsd.org; Sun, 7 Jan 2007 23:54:12 GMT (envelope-from piso@freebsd.org) Date: Sun, 7 Jan 2007 23:54:12 GMT Message-Id: <200701072354.l07NsC0x073362@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to piso@freebsd.org using -f From: Paolo Pisati To: Perforce Change Reviews Cc: Subject: PERFORCE change 112667 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Jan 2007 23:54:13 -0000 http://perforce.freebsd.org/chv.cgi?CH=112667 Change 112667 by piso@piso_newluxor on 2007/01/07 23:53:54 o get rid of the switch statement. o for i386 and amd64: move the stray code into intr_stray_event() Still, different archs behave in different ways for the stray case: if (ie == NULL) o powerpc and ia64: panic (the former with a KASSERT, the latter with an explicit panic()) o i386, amd64 and sparc64: log the condition o arm: do nothing else if (ie->ie_handlers == NULL) o ia64: panic() o i386, amd64, powerpc and sparc64: log the condition o arm: do nothing IMHO we should consolidate all the archs around the same MI behaviour: if (ie == NULL) panic() else if (ie->ie_handlers == NULL) let the new stray mechanism takes care of it: o disab the line and schedule a callout in the callout handler: o if the event is still pending, call intr_filter_loop() o if someone claimed the event, ok o else, disab again the line and reschedule the the callout with a longer timeout The problem with the new stray mechanism is that we rely on the interrupt controller to let us check if an event is still pending and, while i386 and amd64 have this feature, i don't know about all the other archs/controllers. XXX - what if i simply call intr_filter_loop() without checking the status of the line?!?!? Affected files ... .. //depot/projects/soc2006/intr_filter/amd64/amd64/intr_machdep.c#19 edit .. //depot/projects/soc2006/intr_filter/i386/i386/intr_machdep.c#27 edit .. //depot/projects/soc2006/intr_filter/ia64/ia64/interrupt.c#19 edit .. //depot/projects/soc2006/intr_filter/powerpc/powerpc/intr_machdep.c#24 edit .. //depot/projects/soc2006/intr_filter/sparc64/sparc64/intr_machdep.c#19 edit Differences ... ==== //depot/projects/soc2006/intr_filter/amd64/amd64/intr_machdep.c#19 (text+ko) ==== @@ -251,7 +251,7 @@ { struct thread *td; struct intr_event *ie; - int res, vector; + int vector; td = curthread; @@ -274,28 +274,28 @@ if (vector == 0) clkintr_pending = 1; - res = intr_event_handle(ie, frame); - switch(res) { - case 0: - break; - case EINVAL: - /* - * For stray interrupts, mask and EOI the source, bump the - * stray count, and log the condition. - */ - isrc->is_pic->pic_disable_source(isrc, PIC_EOI); - (*isrc->is_straycount)++; - if (*isrc->is_straycount < MAX_STRAY_LOG) - log(LOG_ERR, "stray irq%d\n", vector); - else if (*isrc->is_straycount == MAX_STRAY_LOG) - log(LOG_CRIT, - "too many stray irq %d's: not logging anymore\n", - vector); - break; - default: - printf("Ouch! Return code from mi_handle_intr()" - "not expected.\n"); - } + if (intr_event_handle(ie, frame) != 0) + intr_event_stray(isrc); +} + +static void +intr_event_stray(void *cookie) +{ + struct intsrc *isrc; + + isrc = cookie; + /* + * For stray interrupts, mask and EOI the source, bump the + * stray count, and log the condition. + */ + isrc->is_pic->pic_disable_source(isrc, PIC_EOI); + (*isrc->is_straycount)++; + if (*isrc->is_straycount < MAX_STRAY_LOG) + log(LOG_ERR, "stray irq%d\n", isrc->is_pic->pic_vector(isrc)); + else if (*isrc->is_straycount == MAX_STRAY_LOG) + log(LOG_CRIT, + "too many stray irq %d's: not logging anymore\n", + isrc->is_pic->pic_vector(isrc)); } static void ==== //depot/projects/soc2006/intr_filter/i386/i386/intr_machdep.c#27 (text+ko) ==== @@ -70,6 +70,7 @@ static void intr_eoi_src(void *arg); static void intr_disab_eoi_src(void *arg); void intr_callout_reset(void); +static void intr_event_stray(void *cookie); #ifdef SMP static int assign_cpu; @@ -232,7 +233,7 @@ { struct thread *td; struct intr_event *ie; - int res, vector; + int vector; td = curthread; @@ -255,28 +256,28 @@ if (vector == 0) clkintr_pending = 1; - res = intr_event_handle(ie, frame); - switch(res) { - case 0: - break; - case EINVAL: - /* - * For stray interrupts, mask and EOI the source, bump the - * stray count, and log the condition. - */ - isrc->is_pic->pic_disable_source(isrc, PIC_EOI); - (*isrc->is_straycount)++; - if (*isrc->is_straycount < MAX_STRAY_LOG) - log(LOG_ERR, "stray irq%d\n", vector); - else if (*isrc->is_straycount == MAX_STRAY_LOG) - log(LOG_CRIT, - "too many stray irq %d's: not logging anymore\n", - vector); - break; - default: - printf("Ouch! Return code from mi_handle_intr()" - "not expected.\n"); - } + if (intr_event_handle(ie, frame) != 0) + intr_event_stray(isrc); +} + +static void +intr_event_stray(void *cookie) +{ + struct intsrc *isrc; + + isrc = cookie; + /* + * For stray interrupts, mask and EOI the source, bump the + * stray count, and log the condition. + */ + isrc->is_pic->pic_disable_source(isrc, PIC_EOI); + (*isrc->is_straycount)++; + if (*isrc->is_straycount < MAX_STRAY_LOG) + log(LOG_ERR, "stray irq%d\n", isrc->is_pic->pic_vector(isrc)); + else if (*isrc->is_straycount == MAX_STRAY_LOG) + log(LOG_CRIT, + "too many stray irq %d's: not logging anymore\n", + isrc->is_pic->pic_vector(isrc)); } void ==== //depot/projects/soc2006/intr_filter/ia64/ia64/interrupt.c#19 (text+ko) ==== @@ -395,8 +395,6 @@ ia64_dispatch_intr(void *frame, unsigned long vector) { struct ia64_intr *i; - int res; - /* * Find the interrupt thread for this vector. */ @@ -407,17 +405,8 @@ if (i->cntp) atomic_add_long(i->cntp, 1); - res = intr_event_handle(i->event, frame); - switch (res) { - case 0: - break; - case EINVAL: + if (intr_event_handle(i->event, frame) != 0) panic("Interrupt vector without an event\n"); - break; - default: - printf("Ouch! Return code from mi_handle_intr()" - "not expected.\n"); - } } #ifdef DDB ==== //depot/projects/soc2006/intr_filter/powerpc/powerpc/intr_machdep.c#24 (text+ko) ==== @@ -278,7 +278,6 @@ { struct ppc_intr *i; struct intr_event *ie; - int error; i = ppc_intrs[irq]; if (i == NULL) { @@ -291,15 +290,6 @@ ie = i->event; KASSERT(ie != NULL, ("%s: interrupt without an event", __func__)); - res = intr_event_handle(ie, NULL); - switch(res) { - case 0: - break; - case EINVAL: + if (intr_event_handle(ie, NULL) != 0) stray_int(irq); - break; - default: - printf("Ouch! Return code from mi_handle_intr()" - "not expected.\n"); - } } ==== //depot/projects/soc2006/intr_filter/sparc64/sparc64/intr_machdep.c#19 (text+ko) ==== @@ -268,21 +268,11 @@ { struct intr_vector *iv; struct intr_event *ie; - int res; iv = cookie; ie = iv->iv_event; - res = intr_event_handle(ie, NULL); - switch (res) { - case 0: - break; - case EINVAL: + if (intr_event_handle(ie, NULL) != 0) intr_stray_vector(iv); - break; - default: - printf("Ouch! Return code from mi_handle_intr()" - "not expected.\n"); - } } int