RE: SMTC lookup in smtc_distribute_timer
|
View:
New views
7 Messages
—
Rating Filter:
Alert me
|
 |
RE: SMTC lookup in smtc_distribute_timer
by Mikael Starvik
::
Rate this Message:
Ok, my guess is something like this:
1. At the end of smtc_distribute_timer, nextstamp is valid and has already passed so we goto repeat. 2. Nothing updates nextstamp (only updated if the timeout is in the future And we just decided it is in the past) 3. At the end nextstamp still has the same value so it is still valid and in the past. 4. This repeats until read_c0_count has a value which causes nextstamp to be in the future. One possible patch that seams to solve it for me below. This is probably not the correct solution so I'll need help from the SMTC experts to review it and come up with the correct solution. Best Regards /Mikael Index: cevt-smtc.c =================================================================== RCS file: /usr/local/cvs/linux/os/linux-2.6/arch/mips/kernel/cevt-smtc.c,v retrieving revision 1.2 diff -u -r1.2 cevt-smtc.c --- cevt-smtc.c 2 Sep 2009 10:07:51 -0000 1.2 +++ cevt-smtc.c 10 Nov 2009 11:40:31 -0000 @@ -223,8 +223,10 @@ write_c0_compare(nextstamp); ehb(); if ((nextstamp - (unsigned long)read_c0_count()) - > (unsigned long)LONG_MAX) + > (unsigned long)LONG_MAX) { + nextstamp = 0L; goto repeat; + } } } |
|
|
Re: SMTC lookup in smtc_distribute_timer
by Kevin D. Kissell-2
::
Rate this Message:
Your failure scenario looks plausible. Mea culpa. However, I think that
a more elegant and slightly smaller (depending on just how good the optimizer is) fix would be: diff --git a/arch/mips/kernel/cevt-smtc.c b/arch/mips/kernel/cevt-smtc.c index 98bd7de..b102e4f 100644 --- a/arch/mips/kernel/cevt-smtc.c +++ b/arch/mips/kernel/cevt-smtc.c @@ -173,11 +173,12 @@ void smtc_distribute_timer(int vpe) unsigned int mtflags; int cpu; struct clock_event_device *cd; - unsigned long nextstamp = 0L; + unsigned long nextstamp; unsigned long reference; repeat: + nextstamp = 0L; for_each_online_cpu(cpu) { /* * Find virtual CPUs within the current VPE who have I don't have access to SMTC-capable hardware just now, but I guess the way to test this would be to have a test program or kernel test stub program two events separated by the smallest possible increment, so that the second will have passed by the time interrupt services for the first. Regards, Kevin K. Mikael Starvik wrote: > Ok, my guess is something like this: > > 1. At the end of smtc_distribute_timer, nextstamp is valid and has already > passed so we goto repeat. > 2. Nothing updates nextstamp (only updated if the timeout is in the future > And we just decided it is in the past) > 3. At the end nextstamp still has the same value so it is still valid and > in the past. > 4. This repeats until read_c0_count has a value which causes nextstamp to > be in the future. > > One possible patch that seams to solve it for me below. This is probably > not the correct solution so I'll need help from the SMTC experts to review > it and come up with the correct solution. > > Best Regards > /Mikael > > Index: cevt-smtc.c > =================================================================== > RCS file: /usr/local/cvs/linux/os/linux-2.6/arch/mips/kernel/cevt-smtc.c,v > retrieving revision 1.2 > diff -u -r1.2 cevt-smtc.c > --- cevt-smtc.c 2 Sep 2009 10:07:51 -0000 1.2 > +++ cevt-smtc.c 10 Nov 2009 11:40:31 -0000 > @@ -223,8 +223,10 @@ > write_c0_compare(nextstamp); > ehb(); > if ((nextstamp - (unsigned long)read_c0_count()) > - > (unsigned long)LONG_MAX) > + > (unsigned long)LONG_MAX) { > + nextstamp = 0L; > goto repeat; > + } > } > } > > > |
|
|
RE: SMTC lookup in smtc_distribute_timer
by Mikael Starvik
::
Rate this Message:
Yes, I thought of that variant after I sent the email yesterday.
I'll change our local implementation. If you don't hear anything it works as expected in our case (it was pretty easy for us to repeat). /Mikael -----Original Message----- From: Kevin D. Kissell [mailto:kevink@...] Sent: den 10 november 2009 20:46 To: Mikael Starvik Cc: linux-mips@...; Jesper Nilsson Subject: Re: SMTC lookup in smtc_distribute_timer Your failure scenario looks plausible. Mea culpa. However, I think that a more elegant and slightly smaller (depending on just how good the optimizer is) fix would be: diff --git a/arch/mips/kernel/cevt-smtc.c b/arch/mips/kernel/cevt-smtc.c index 98bd7de..b102e4f 100644 --- a/arch/mips/kernel/cevt-smtc.c +++ b/arch/mips/kernel/cevt-smtc.c @@ -173,11 +173,12 @@ void smtc_distribute_timer(int vpe) unsigned int mtflags; int cpu; struct clock_event_device *cd; - unsigned long nextstamp = 0L; + unsigned long nextstamp; unsigned long reference; repeat: + nextstamp = 0L; for_each_online_cpu(cpu) { /* * Find virtual CPUs within the current VPE who have I don't have access to SMTC-capable hardware just now, but I guess the way to test this would be to have a test program or kernel test stub program two events separated by the smallest possible increment, so that the second will have passed by the time interrupt services for the first. Regards, Kevin K. Mikael Starvik wrote: > Ok, my guess is something like this: > > 1. At the end of smtc_distribute_timer, nextstamp is valid and has already > passed so we goto repeat. > 2. Nothing updates nextstamp (only updated if the timeout is in the future > And we just decided it is in the past) > 3. At the end nextstamp still has the same value so it is still valid and > in the past. > 4. This repeats until read_c0_count has a value which causes nextstamp to > be in the future. > > One possible patch that seams to solve it for me below. This is probably > not the correct solution so I'll need help from the SMTC experts to review > it and come up with the correct solution. > > Best Regards > /Mikael > > Index: cevt-smtc.c > =================================================================== > RCS file: /usr/local/cvs/linux/os/linux-2.6/arch/mips/kernel/cevt-smtc.c,v > retrieving revision 1.2 > diff -u -r1.2 cevt-smtc.c > --- cevt-smtc.c 2 Sep 2009 10:07:51 -0000 1.2 > +++ cevt-smtc.c 10 Nov 2009 11:40:31 -0000 > @@ -223,8 +223,10 @@ > write_c0_compare(nextstamp); > ehb(); > if ((nextstamp - (unsigned long)read_c0_count()) > - > (unsigned long)LONG_MAX) > + > (unsigned long)LONG_MAX) { > + nextstamp = 0L; > goto repeat; > + } > } > } > > > |
|
|
Re: SMTC lookup in smtc_distribute_timer
by Kevin D. Kissell-2
::
Rate this Message:
Rather than just assume all is well, I really would appreciate it
of you could send a positive acknowledgement that it solves the problem without causing the universe to implode, so that Ralf can queue up the patch for the repository. Regards, Kevin K. Mikael Starvik wrote: > Yes, I thought of that variant after I sent the email yesterday. > I'll change our local implementation. If you don't hear anything > it works as expected in our case (it was pretty easy for us to > repeat). > > /Mikael > > -----Original Message----- > From: Kevin D. Kissell [mailto:kevink@...] > Sent: den 10 november 2009 20:46 > To: Mikael Starvik > Cc: linux-mips@...; Jesper Nilsson > Subject: Re: SMTC lookup in smtc_distribute_timer > > Your failure scenario looks plausible. Mea culpa. However, I think that > a more elegant and slightly smaller (depending on just how good > the optimizer is) fix would be: > > diff --git a/arch/mips/kernel/cevt-smtc.c b/arch/mips/kernel/cevt-smtc.c > index 98bd7de..b102e4f 100644 > --- a/arch/mips/kernel/cevt-smtc.c > +++ b/arch/mips/kernel/cevt-smtc.c > @@ -173,11 +173,12 @@ void smtc_distribute_timer(int vpe) > unsigned int mtflags; > int cpu; > struct clock_event_device *cd; > - unsigned long nextstamp = 0L; > + unsigned long nextstamp; > unsigned long reference; > > > repeat: > + nextstamp = 0L; > for_each_online_cpu(cpu) { > /* > * Find virtual CPUs within the current VPE who have > > > > I don't have access to SMTC-capable hardware just now, but > I guess the way to test this would be to have a test program > or kernel test stub program two events separated by the smallest > possible increment, so that the second will have passed by the > time interrupt services for the first. > > Regards, > > Kevin K. > > Mikael Starvik wrote: > >> Ok, my guess is something like this: >> >> 1. At the end of smtc_distribute_timer, nextstamp is valid and has already >> passed so we goto repeat. >> 2. Nothing updates nextstamp (only updated if the timeout is in the future >> And we just decided it is in the past) >> 3. At the end nextstamp still has the same value so it is still valid and >> in the past. >> 4. This repeats until read_c0_count has a value which causes nextstamp to >> be in the future. >> >> One possible patch that seams to solve it for me below. This is probably >> not the correct solution so I'll need help from the SMTC experts to review >> it and come up with the correct solution. >> >> Best Regards >> /Mikael >> >> Index: cevt-smtc.c >> =================================================================== >> RCS file: /usr/local/cvs/linux/os/linux-2.6/arch/mips/kernel/cevt-smtc.c,v >> retrieving revision 1.2 >> diff -u -r1.2 cevt-smtc.c >> --- cevt-smtc.c 2 Sep 2009 10:07:51 -0000 1.2 >> +++ cevt-smtc.c 10 Nov 2009 11:40:31 -0000 >> @@ -223,8 +223,10 @@ >> write_c0_compare(nextstamp); >> ehb(); >> if ((nextstamp - (unsigned long)read_c0_count()) >> - > (unsigned long)LONG_MAX) >> + > (unsigned long)LONG_MAX) { >> + nextstamp = 0L; >> goto repeat; >> + } >> } >> } >> >> >> >> > > |
|
|
RE: SMTC lookup in smtc_distribute_timer
by Mikael Starvik
::
Rate this Message:
Ok. Yes, it works in our case. Tests have run overnight without any problems.
Regards /Mikael -----Original Message----- From: Kevin D. Kissell [mailto:kevink@...] Sent: den 11 november 2009 20:23 To: Mikael Starvik Cc: linux-mips@...; Jesper Nilsson Subject: Re: SMTC lookup in smtc_distribute_timer Rather than just assume all is well, I really would appreciate it of you could send a positive acknowledgement that it solves the problem without causing the universe to implode, so that Ralf can queue up the patch for the repository. Regards, Kevin K. Mikael Starvik wrote: > Yes, I thought of that variant after I sent the email yesterday. > I'll change our local implementation. If you don't hear anything > it works as expected in our case (it was pretty easy for us to > repeat). > > /Mikael > > -----Original Message----- > From: Kevin D. Kissell [mailto:kevink@...] > Sent: den 10 november 2009 20:46 > To: Mikael Starvik > Cc: linux-mips@...; Jesper Nilsson > Subject: Re: SMTC lookup in smtc_distribute_timer > > Your failure scenario looks plausible. Mea culpa. However, I think that > a more elegant and slightly smaller (depending on just how good > the optimizer is) fix would be: > > diff --git a/arch/mips/kernel/cevt-smtc.c b/arch/mips/kernel/cevt-smtc.c > index 98bd7de..b102e4f 100644 > --- a/arch/mips/kernel/cevt-smtc.c > +++ b/arch/mips/kernel/cevt-smtc.c > @@ -173,11 +173,12 @@ void smtc_distribute_timer(int vpe) > unsigned int mtflags; > int cpu; > struct clock_event_device *cd; > - unsigned long nextstamp = 0L; > + unsigned long nextstamp; > unsigned long reference; > > > repeat: > + nextstamp = 0L; > for_each_online_cpu(cpu) { > /* > * Find virtual CPUs within the current VPE who have > > > > I don't have access to SMTC-capable hardware just now, but > I guess the way to test this would be to have a test program > or kernel test stub program two events separated by the smallest > possible increment, so that the second will have passed by the > time interrupt services for the first. > > Regards, > > Kevin K. > > Mikael Starvik wrote: > >> Ok, my guess is something like this: >> >> 1. At the end of smtc_distribute_timer, nextstamp is valid and has already >> passed so we goto repeat. >> 2. Nothing updates nextstamp (only updated if the timeout is in the future >> And we just decided it is in the past) >> 3. At the end nextstamp still has the same value so it is still valid and >> in the past. >> 4. This repeats until read_c0_count has a value which causes nextstamp to >> be in the future. >> >> One possible patch that seams to solve it for me below. This is probably >> not the correct solution so I'll need help from the SMTC experts to review >> it and come up with the correct solution. >> >> Best Regards >> /Mikael >> >> Index: cevt-smtc.c >> =================================================================== >> RCS file: /usr/local/cvs/linux/os/linux-2.6/arch/mips/kernel/cevt-smtc.c,v >> retrieving revision 1.2 >> diff -u -r1.2 cevt-smtc.c >> --- cevt-smtc.c 2 Sep 2009 10:07:51 -0000 1.2 >> +++ cevt-smtc.c 10 Nov 2009 11:40:31 -0000 >> @@ -223,8 +223,10 @@ >> write_c0_compare(nextstamp); >> ehb(); >> if ((nextstamp - (unsigned long)read_c0_count()) >> - > (unsigned long)LONG_MAX) >> + > (unsigned long)LONG_MAX) { >> + nextstamp = 0L; >> goto repeat; >> + } >> } >> } >> >> >> >> > > |
|
|
Re: SMTC lookup in smtc_distribute_timer
by Kevin D. Kissell-2
::
Rate this Message:
OK, thanks. Ralf, can we consider this one queued? It does seem to
have been captured correctly by Patchwork. Regards, Kevin K. On 11/12/2009 09:26 AM, Mikael Starvik wrote: > Ok. Yes, it works in our case. Tests have run overnight without any problems. > > Regards > /Mikael > > -----Original Message----- > From: Kevin D. Kissell [mailto:kevink@...] > Sent: den 11 november 2009 20:23 > To: Mikael Starvik > Cc: linux-mips@...; Jesper Nilsson > Subject: Re: SMTC lookup in smtc_distribute_timer > > Rather than just assume all is well, I really would appreciate it > of you could send a positive acknowledgement that it solves > the problem without causing the universe to implode, so that > Ralf can queue up the patch for the repository. > > Regards, > > Kevin K. > > Mikael Starvik wrote: > >> Yes, I thought of that variant after I sent the email yesterday. >> I'll change our local implementation. If you don't hear anything >> it works as expected in our case (it was pretty easy for us to >> repeat). >> >> /Mikael >> >> -----Original Message----- >> From: Kevin D. Kissell [mailto:kevink@...] >> Sent: den 10 november 2009 20:46 >> To: Mikael Starvik >> Cc: linux-mips@...; Jesper Nilsson >> Subject: Re: SMTC lookup in smtc_distribute_timer >> >> Your failure scenario looks plausible. Mea culpa. However, I think that >> a more elegant and slightly smaller (depending on just how good >> the optimizer is) fix would be: >> >> diff --git a/arch/mips/kernel/cevt-smtc.c b/arch/mips/kernel/cevt-smtc.c >> index 98bd7de..b102e4f 100644 >> --- a/arch/mips/kernel/cevt-smtc.c >> +++ b/arch/mips/kernel/cevt-smtc.c >> @@ -173,11 +173,12 @@ void smtc_distribute_timer(int vpe) >> unsigned int mtflags; >> int cpu; >> struct clock_event_device *cd; >> - unsigned long nextstamp = 0L; >> + unsigned long nextstamp; >> unsigned long reference; >> >> >> repeat: >> + nextstamp = 0L; >> for_each_online_cpu(cpu) { >> /* >> * Find virtual CPUs within the current VPE who have >> >> >> >> I don't have access to SMTC-capable hardware just now, but >> I guess the way to test this would be to have a test program >> or kernel test stub program two events separated by the smallest >> possible increment, so that the second will have passed by the >> time interrupt services for the first. >> >> Regards, >> >> Kevin K. >> >> Mikael Starvik wrote: >> >> >>> Ok, my guess is something like this: >>> >>> 1. At the end of smtc_distribute_timer, nextstamp is valid and has already >>> passed so we goto repeat. >>> 2. Nothing updates nextstamp (only updated if the timeout is in the future >>> And we just decided it is in the past) >>> 3. At the end nextstamp still has the same value so it is still valid and >>> in the past. >>> 4. This repeats until read_c0_count has a value which causes nextstamp to >>> be in the future. >>> >>> One possible patch that seams to solve it for me below. This is probably >>> not the correct solution so I'll need help from the SMTC experts to review >>> it and come up with the correct solution. >>> >>> Best Regards >>> /Mikael >>> >>> Index: cevt-smtc.c >>> =================================================================== >>> RCS file: /usr/local/cvs/linux/os/linux-2.6/arch/mips/kernel/cevt-smtc.c,v >>> retrieving revision 1.2 >>> diff -u -r1.2 cevt-smtc.c >>> --- cevt-smtc.c 2 Sep 2009 10:07:51 -0000 1.2 >>> +++ cevt-smtc.c 10 Nov 2009 11:40:31 -0000 >>> @@ -223,8 +223,10 @@ >>> write_c0_compare(nextstamp); >>> ehb(); >>> if ((nextstamp - (unsigned long)read_c0_count()) >>> - > (unsigned long)LONG_MAX) >>> + > (unsigned long)LONG_MAX) { >>> + nextstamp = 0L; >>> goto repeat; >>> + } >>> } >>> } >>> >>> >>> >>> >>> >> >> > |
|
|
Re: SMTC lookup in smtc_distribute_timer
by Ralf Baechle DL5RB
::
Rate this Message:
On Thu, Nov 12, 2009 at 10:39:33AM +0100, Kevin D. Kissell wrote:
> OK, thanks. Ralf, can we consider this one queued? It does seem to > have been captured correctly by Patchwork. Just applied it to master; this should go into 2.6.32 and the -stable branches. Patchwork only tracks patches so people know the status of their patches and I don't drop it by accident. A patch just showing up in patchwork doesn't yet mean it has been accepted. Thanks everybody! Ralf |
| Free embeddable forum powered by Nabble | Forum Help |
