| Message ID | 20190411094944.12245-5-brgl@bgdev.pl |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | um: build and irq fixes | expand |
On Thu, Apr 11, 2019 at 11:50 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > From: Bartosz Golaszewski <bgolaszewski@baylibre.com> > > Setting a chip for an interrupt marks it as allocated. Since UM doesn't > support dynamic interrupt numbers (yet), it means we cannot simply > increase NR_IRQS and then use the free irqs between LAST_IRQ and NR_IRQS > with gpio-mockup or iio testing drivers as irq_alloc_descs() will fail > after not being able to neither find an unallocated range of interrupts > nor expand the range. > > Only call irq_set_chip_and_handler() for irqs until LAST_IRQ. > > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> > Reviewed-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> > Acked-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> Just noticed that this triggers the following errors while bootup: Trying to reregister IRQ 11 FD 8 TYPE 0 ID (null) write_sigio_irq : um_request_irq failed, err = -16 Trying to reregister IRQ 11 FD 8 TYPE 0 ID (null) write_sigio_irq : um_request_irq failed, err = -16 VFS: Mounted root (hostfs filesystem) readonly on Can you please check? This patch is already queued in -next. So we need to decide whether to revert or fix it now.
On 07/05/2019 22:26, Richard Weinberger wrote: > On Thu, Apr 11, 2019 at 11:50 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: >> >> From: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> >> Setting a chip for an interrupt marks it as allocated. Since UM doesn't >> support dynamic interrupt numbers (yet), it means we cannot simply >> increase NR_IRQS and then use the free irqs between LAST_IRQ and NR_IRQS >> with gpio-mockup or iio testing drivers as irq_alloc_descs() will fail >> after not being able to neither find an unallocated range of interrupts >> nor expand the range. >> >> Only call irq_set_chip_and_handler() for irqs until LAST_IRQ. >> >> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com> >> Reviewed-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> Acked-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> > > Just noticed that this triggers the following errors while bootup: > Trying to reregister IRQ 11 FD 8 TYPE 0 ID (null) > write_sigio_irq : um_request_irq failed, err = -16 > Trying to reregister IRQ 11 FD 8 TYPE 0 ID (null) > write_sigio_irq : um_request_irq failed, err = -16 > VFS: Mounted root (hostfs filesystem) readonly on > > Can you please check? > This patch is already queued in -next. So we need to decide whether to > revert or fix it now. > I am looking at it. It passed tests in my case (I did the usual round).
----- Ursprüngliche Mail ----- >> Can you please check? >> This patch is already queued in -next. So we need to decide whether to >> revert or fix it now. >> > I am looking at it. It passed tests in my case (I did the usual round). It works here too. That's why I never noticed. Yesterday I noticed just because I looked for something else in the kernel logs. Thanks, //richard
śr., 8 maj 2019 o 09:13 Richard Weinberger <richard@nod.at> napisał(a): > > ----- Ursprüngliche Mail ----- > >> Can you please check? > >> This patch is already queued in -next. So we need to decide whether to > >> revert or fix it now. > >> > > I am looking at it. It passed tests in my case (I did the usual round). > > It works here too. That's why I never noticed. > Yesterday I noticed just because I looked for something else in the kernel logs. > > Thanks, > //richard Hi, sorry for the late reply - I just came back from vacation. I see it here too, I'll check if I can find the culprit and fix it today. Bart
pt., 10 maj 2019 o 11:16 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > > śr., 8 maj 2019 o 09:13 Richard Weinberger <richard@nod.at> napisał(a): > > > > ----- Ursprüngliche Mail ----- > > >> Can you please check? > > >> This patch is already queued in -next. So we need to decide whether to > > >> revert or fix it now. > > >> > > > I am looking at it. It passed tests in my case (I did the usual round). > > > > It works here too. That's why I never noticed. > > Yesterday I noticed just because I looked for something else in the kernel logs. > > > > Thanks, > > //richard > > Hi, > > sorry for the late reply - I just came back from vacation. > > I see it here too, I'll check if I can find the culprit and fix it today. > > Bart Hi Richard, Anton, I'm not sure yet what this is caused by. It doesn't seem to break anything for me but since it's a new error message I guess it's best to revert this patch (others are good) and revisit it for v5.3. Bart
On 10/05/2019 17:20, Bartosz Golaszewski wrote: > pt., 10 maj 2019 o 11:16 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): >> śr., 8 maj 2019 o 09:13 Richard Weinberger <richard@nod.at> napisał(a): >>> ----- Ursprüngliche Mail ----- >>>>> Can you please check? >>>>> This patch is already queued in -next. So we need to decide whether to >>>>> revert or fix it now. >>>>> >>>> I am looking at it. It passed tests in my case (I did the usual round). >>> It works here too. That's why I never noticed. >>> Yesterday I noticed just because I looked for something else in the kernel logs. >>> >>> Thanks, >>> //richard >> Hi, >> >> sorry for the late reply - I just came back from vacation. >> >> I see it here too, I'll check if I can find the culprit and fix it today. >> >> Bart > Hi Richard, Anton, > > I'm not sure yet what this is caused by. It doesn't seem to break > anything for me but since it's a new error message I guess it's best > to revert this patch (others are good) and revisit it for v5.3. Can you send me your command line and .config so I can try to reproduce it. Brgds, > > Bart >
pt., 10 maj 2019 o 18:22 Anton Ivanov <anton.ivanov@cambridgegreys.com> napisał(a): > > > On 10/05/2019 17:20, Bartosz Golaszewski wrote: > > pt., 10 maj 2019 o 11:16 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): > >> śr., 8 maj 2019 o 09:13 Richard Weinberger <richard@nod.at> napisał(a): > >>> ----- Ursprüngliche Mail ----- > >>>>> Can you please check? > >>>>> This patch is already queued in -next. So we need to decide whether to > >>>>> revert or fix it now. > >>>>> > >>>> I am looking at it. It passed tests in my case (I did the usual round). > >>> It works here too. That's why I never noticed. > >>> Yesterday I noticed just because I looked for something else in the kernel logs. > >>> > >>> Thanks, > >>> //richard > >> Hi, > >> > >> sorry for the late reply - I just came back from vacation. > >> > >> I see it here too, I'll check if I can find the culprit and fix it today. > >> > >> Bart > > Hi Richard, Anton, > > > > I'm not sure yet what this is caused by. It doesn't seem to break > > anything for me but since it's a new error message I guess it's best > > to revert this patch (others are good) and revisit it for v5.3. > > Can you send me your command line and .config so I can try to reproduce it. > Sure, the command line is: ./linux rootfstype=hostfs rootflags=<path to a regular buildroot rootfs> rw mem=48M The config is the regular x86_64_defconfig from arch/um. Bart
On 5/11/19 5:48 AM, Bartosz Golaszewski wrote: > pt., 10 maj 2019 o 18:22 Anton Ivanov > <anton.ivanov@cambridgegreys.com> napisał(a): >> >> >> On 10/05/2019 17:20, Bartosz Golaszewski wrote: >>> pt., 10 maj 2019 o 11:16 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): >>>> śr., 8 maj 2019 o 09:13 Richard Weinberger <richard@nod.at> napisał(a): >>>>> ----- Ursprüngliche Mail ----- >>>>>>> Can you please check? >>>>>>> This patch is already queued in -next. So we need to decide whether to >>>>>>> revert or fix it now. >>>>>>> >>>>>> I am looking at it. It passed tests in my case (I did the usual round). >>>>> It works here too. That's why I never noticed. >>>>> Yesterday I noticed just because I looked for something else in the kernel logs. >>>>> >>>>> Thanks, >>>>> //richard >>>> Hi, >>>> >>>> sorry for the late reply - I just came back from vacation. >>>> >>>> I see it here too, I'll check if I can find the culprit and fix it today. >>>> >>>> Bart >>> Hi Richard, Anton, >>> >>> I'm not sure yet what this is caused by. It doesn't seem to break >>> anything for me but since it's a new error message I guess it's best >>> to revert this patch (others are good) and revisit it for v5.3. >> >> Can you send me your command line and .config so I can try to reproduce it. >> > > Sure, > > the command line is: > > ./linux rootfstype=hostfs rootflags=<path to a regular buildroot > rootfs> rw mem=48M > > The config is the regular x86_64_defconfig from arch/um. > > Bart > I hit this compiling 5.2.0-rc2-00024-gbec7550cca10 I can trigger: Trying to reregister IRQ 11 FD 9 TYPE 0 ID (____ptrval____) write_sigio_irq : um_request_irq failed, err = -16 during boot when: # CONFIG_UML_NET_VECTOR is not set Looking at the code there seems to be a one off error in the LAST_IRQ calculation when CONFIG_UML_NET_VECTOR is enabled. In that case the patch should be reverted and the following applied: diff --git a/arch/um/include/asm/irq.h b/arch/um/include/asm/irq.h index 49ed3e35b35a..ce7a78c3bcf2 100644 --- a/arch/um/include/asm/irq.h +++ b/arch/um/include/asm/irq.h @@ -23,7 +23,7 @@ #define VECTOR_BASE_IRQ 15 #define VECTOR_IRQ_SPACE 8 -#define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ) +#define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ - 1) #else Dana
On 31/05/2019 11:30, Dana Johnson wrote: > On 5/11/19 5:48 AM, Bartosz Golaszewski wrote: >> pt., 10 maj 2019 o 18:22 Anton Ivanov >> <anton.ivanov@cambridgegreys.com> napisał(a): >>> >>> >>> On 10/05/2019 17:20, Bartosz Golaszewski wrote: >>>> pt., 10 maj 2019 o 11:16 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): >>>>> śr., 8 maj 2019 o 09:13 Richard Weinberger <richard@nod.at> napisał(a): >>>>>> ----- Ursprüngliche Mail ----- >>>>>>>> Can you please check? >>>>>>>> This patch is already queued in -next. So we need to decide whether to >>>>>>>> revert or fix it now. >>>>>>>> >>>>>>> I am looking at it. It passed tests in my case (I did the usual round). >>>>>> It works here too. That's why I never noticed. >>>>>> Yesterday I noticed just because I looked for something else in the kernel logs. >>>>>> >>>>>> Thanks, >>>>>> //richard >>>>> Hi, >>>>> >>>>> sorry for the late reply - I just came back from vacation. >>>>> >>>>> I see it here too, I'll check if I can find the culprit and fix it today. >>>>> >>>>> Bart >>>> Hi Richard, Anton, >>>> >>>> I'm not sure yet what this is caused by. It doesn't seem to break >>>> anything for me but since it's a new error message I guess it's best >>>> to revert this patch (others are good) and revisit it for v5.3. >>> >>> Can you send me your command line and .config so I can try to reproduce it. >>> >> >> Sure, >> >> the command line is: >> >> ./linux rootfstype=hostfs rootflags=<path to a regular buildroot >> rootfs> rw mem=48M >> >> The config is the regular x86_64_defconfig from arch/um. >> >> Bart >> > > I hit this compiling 5.2.0-rc2-00024-gbec7550cca10 > > I can trigger: > > Trying to reregister IRQ 11 FD 9 TYPE 0 ID (____ptrval____) > write_sigio_irq : um_request_irq failed, err = -16 > > during boot when: > > # CONFIG_UML_NET_VECTOR is not set > > Looking at the code there seems to be a one off error > in the LAST_IRQ calculation when CONFIG_UML_NET_VECTOR is enabled. > In that case the patch should be reverted and the following applied: > > > diff --git a/arch/um/include/asm/irq.h b/arch/um/include/asm/irq.h > index 49ed3e35b35a..ce7a78c3bcf2 100644 > --- a/arch/um/include/asm/irq.h > +++ b/arch/um/include/asm/irq.h > @@ -23,7 +23,7 @@ > #define VECTOR_BASE_IRQ 15 > #define VECTOR_IRQ_SPACE 8 > > -#define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ) > +#define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ - 1) > That does not work. Vector IRQs are allocated modulo VECTOR_IRQ_SPACE. Line 1219 and 1234 in vector_kern.c. irq_rr = (irq_rr + 1) % VECTOR_IRQ_SPACE; #define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ - 1) will definitely give off-by-one if vector network drivers are enabled. IRQ 11 is sigio, this is something else. Can I have your .config please so I can try to reproduce it. > #else > > Dana > > _______________________________________________ > linux-um mailing list > linux-um@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-um >
On 6/2/19 11:54 PM, Anton Ivanov wrote: > > > On 31/05/2019 11:30, Dana Johnson wrote: >> On 5/11/19 5:48 AM, Bartosz Golaszewski wrote: >>> pt., 10 maj 2019 o 18:22 Anton Ivanov >>> <anton.ivanov@cambridgegreys.com> napisał(a): >>>> >>>> >>>> On 10/05/2019 17:20, Bartosz Golaszewski wrote: >>>>> pt., 10 maj 2019 o 11:16 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a): >>>>>> śr., 8 maj 2019 o 09:13 Richard Weinberger <richard@nod.at> napisał(a): >>>>>>> ----- Ursprüngliche Mail ----- >>>>>>>>> Can you please check? >>>>>>>>> This patch is already queued in -next. So we need to decide whether to >>>>>>>>> revert or fix it now. >>>>>>>>> >>>>>>>> I am looking at it. It passed tests in my case (I did the usual round). >>>>>>> It works here too. That's why I never noticed. >>>>>>> Yesterday I noticed just because I looked for something else in the kernel logs. >>>>>>> >>>>>>> Thanks, >>>>>>> //richard >>>>>> Hi, >>>>>> >>>>>> sorry for the late reply - I just came back from vacation. >>>>>> >>>>>> I see it here too, I'll check if I can find the culprit and fix it today. >>>>>> >>>>>> Bart >>>>> Hi Richard, Anton, >>>>> >>>>> I'm not sure yet what this is caused by. It doesn't seem to break >>>>> anything for me but since it's a new error message I guess it's best >>>>> to revert this patch (others are good) and revisit it for v5.3. >>>> >>>> Can you send me your command line and .config so I can try to reproduce it. >>>> >>> >>> Sure, >>> >>> the command line is: >>> >>> ./linux rootfstype=hostfs rootflags=<path to a regular buildroot >>> rootfs> rw mem=48M >>> >>> The config is the regular x86_64_defconfig from arch/um. >>> >>> Bart >>> >> I hit this compiling 5.2.0-rc2-00024-gbec7550cca10 >> >> I can trigger: >> >> Trying to reregister IRQ 11 FD 9 TYPE 0 ID (____ptrval____) >> write_sigio_irq : um_request_irq failed, err = -16 >> >> during boot when: >> >> # CONFIG_UML_NET_VECTOR is not set >> >> Looking at the code there seems to be a one off error >> in the LAST_IRQ calculation when CONFIG_UML_NET_VECTOR is enabled. >> In that case the patch should be reverted and the following applied: >> >> >> diff --git a/arch/um/include/asm/irq.h b/arch/um/include/asm/irq.h >> index 49ed3e35b35a..ce7a78c3bcf2 100644 >> --- a/arch/um/include/asm/irq.h >> +++ b/arch/um/include/asm/irq.h >> @@ -23,7 +23,7 @@ >> #define VECTOR_BASE_IRQ 15 >> #define VECTOR_IRQ_SPACE 8 >> >> -#define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ) >> +#define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ - 1) >> > > That does not work. Vector IRQs are allocated modulo VECTOR_IRQ_SPACE. Line 1219 and 1234 in vector_kern.c. > > irq_rr = (irq_rr + 1) % VECTOR_IRQ_SPACE; > > #define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ - 1) will definitely give off-by-one if vector network drivers are enabled. > > IRQ 11 is sigio, this is something else. > > Can I have your .config please so I can try to reproduce it. > >> #else >> >> Dana >> > Yes exactly irq_rr takes on the values: 0..(VECTOR_IRQ_SPACE - 1). The maximum value of irq_rr is (VECTOR_IRQ_SPACE - 1) So LAST_IRQ is: #define LAST_IRQ (VECTOR_BASE_IRQ + (VECTOR_IRQ_SPACE - 1)) Before the patch we benignly initialized an extra interrupt using #define LAST_IRQ (VECTOR_IRQ_SPACE + VECTOR_BASE_IRQ) After the patch everything is still good for vector because of the benign claim of an extra interrupt, but When: # CONFIG_UML_NET_VECTOR is not set changing in kernel/irq.c from: for (i = 1; i < NR_IRQS; i++) to: for (i = 1; i < LAST_IRQ; i++) We stop one short in initalizing with leads to the burble. So keeping the patch we need to use '<=" not '<' The test needs to be: for (i = 1; i <= LAST_IRQ; i++) and LAST_IRQ: #define LAST_IRQ (VECTOR_BASE_IRQ + (VECTOR_IRQ_SPACE - 1)) Dana
diff --git a/arch/um/kernel/irq.c b/arch/um/kernel/irq.c index f4874b7ec503..598d7b3d9355 100644 --- a/arch/um/kernel/irq.c +++ b/arch/um/kernel/irq.c @@ -479,7 +479,7 @@ void __init init_IRQ(void) irq_set_chip_and_handler(TIMER_IRQ, &SIGVTALRM_irq_type, handle_edge_irq); - for (i = 1; i < NR_IRQS; i++) + for (i = 1; i < LAST_IRQ; i++) irq_set_chip_and_handler(i, &normal_irq_type, handle_edge_irq); /* Initialize EPOLL Loop */ os_setup_epoll();