From: Frederic Weisbecker on
On Fri, May 07, 2010 at 05:11:45PM -0400, Don Zickus wrote:
> Just some code cleanup to make touch_softlockup clearer and remove the
> softlockup_tick function as it is no longer needed.
>
> Also remove the /proc softlockup_thres call as it has been changed to
> watchdog_thres.
>
> Signed-off-by: Don Zickus <dzickus(a)redhat.com>
> ---
> include/linux/sched.h | 16 +++-------------
> kernel/sysctl.c | 9 ---------
> kernel/timer.c | 1 -
> kernel/watchdog.c | 35 +++--------------------------------
> 4 files changed, 6 insertions(+), 55 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 2455ff5..e9c6c1d 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -308,18 +308,14 @@ extern void scheduler_tick(void);
> extern void sched_show_task(struct task_struct *p);
>
> #ifdef CONFIG_DETECT_SOFTLOCKUP
> -extern void softlockup_tick(void);
> extern void touch_softlockup_watchdog(void);
> extern void touch_all_softlockup_watchdogs(void);
> -extern int proc_dosoftlockup_thresh(struct ctl_table *table, int write,
> - void __user *buffer,
> - size_t *lenp, loff_t *ppos);
> extern unsigned int softlockup_panic;
> extern int softlockup_thresh;
> +extern int proc_dowatchdog_thresh(struct ctl_table *table, int write,
> + void __user *buffer,
> + size_t *lenp, loff_t *ppos);
> #else
> -static inline void softlockup_tick(void)
> -{
> -}
> static inline void touch_softlockup_watchdog(void)
> {
> }
> @@ -338,12 +334,6 @@ extern int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
> size_t *lenp, loff_t *ppos);
> #endif
>
> -#ifdef CONFIG_LOCKUP_DETECTOR
> -extern int proc_dowatchdog_thresh(struct ctl_table *table, int write,
> - void __user *buffer,
> - size_t *lenp, loff_t *ppos);
> -#endif
> -
> /* Attach to any functions which should be ignored in wchan output. */
> #define __sched __attribute__((__section__(".sched.text")))
>
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 1083897..1fec781 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -827,15 +827,6 @@ static struct ctl_table kern_table[] = {
> .extra1 = &zero,
> .extra2 = &one,
> },
> - {
> - .procname = "softlockup_thresh",
> - .data = &softlockup_thresh,
> - .maxlen = sizeof(int),
> - .mode = 0644,
> - .proc_handler = proc_dosoftlockup_thresh,
> - .extra1 = &neg_one,
> - .extra2 = &sixty,
> - },



I wonder about the ABI breakage.

But I suspect few userspace tools use it though, since this is
mostly for kernel dev.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on
On Wed, May 12, 2010 at 04:26:28PM -0400, Don Zickus wrote:
> On Wed, May 12, 2010 at 10:06:54PM +0200, Frederic Weisbecker wrote:
> > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > > index 1083897..1fec781 100644
> > > --- a/kernel/sysctl.c
> > > +++ b/kernel/sysctl.c
> > > @@ -827,15 +827,6 @@ static struct ctl_table kern_table[] = {
> > > .extra1 = &zero,
> > > .extra2 = &one,
> > > },
> > > - {
> > > - .procname = "softlockup_thresh",
> > > - .data = &softlockup_thresh,
> > > - .maxlen = sizeof(int),
> > > - .mode = 0644,
> > > - .proc_handler = proc_dosoftlockup_thresh,
> > > - .extra1 = &neg_one,
> > > - .extra2 = &sixty,
> > > - },
> >
> >
> >
> > I wonder about the ABI breakage.
> >
> > But I suspect few userspace tools use it though, since this is
> > mostly for kernel dev.
>
> There is no breakage, this chunk of code was duplicated later in the file.
> I am just removing the duplicated bits to simplify the SOFTLOCKUP Kconfig
> stuff.
>
> Cheers,
> Don


Ah right.

BTW, if you address my reviews, please do it incrementally, I'm going
to apply this set and push it to Ingo.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on
On Wed, May 12, 2010 at 04:56:16PM -0400, Don Zickus wrote:
> On Wed, May 12, 2010 at 10:28:35PM +0200, Frederic Weisbecker wrote:
> > On Wed, May 12, 2010 at 04:26:28PM -0400, Don Zickus wrote:
> > > On Wed, May 12, 2010 at 10:06:54PM +0200, Frederic Weisbecker wrote:
> > > > > diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> > > > > index 1083897..1fec781 100644
> > > > > --- a/kernel/sysctl.c
> > > > > +++ b/kernel/sysctl.c
> > > > > @@ -827,15 +827,6 @@ static struct ctl_table kern_table[] = {
> > > > > .extra1 = &zero,
> > > > > .extra2 = &one,
> > > > > },
> > > > > - {
> > > > > - .procname = "softlockup_thresh",
> > > > > - .data = &softlockup_thresh,
> > > > > - .maxlen = sizeof(int),
> > > > > - .mode = 0644,
> > > > > - .proc_handler = proc_dosoftlockup_thresh,
> > > > > - .extra1 = &neg_one,
> > > > > - .extra2 = &sixty,
> > > > > - },
> > > >
> > > >
> > > >
> > > > I wonder about the ABI breakage.
> > > >
> > > > But I suspect few userspace tools use it though, since this is
> > > > mostly for kernel dev.
> > >
> > > There is no breakage, this chunk of code was duplicated later in the file.
> > > I am just removing the duplicated bits to simplify the SOFTLOCKUP Kconfig
> > > stuff.
> > >
> > > Cheers,
> > > Don
> >
> >
> > Ah right.
> >
> > BTW, if you address my reviews, please do it incrementally, I'm going
> > to apply this set and push it to Ingo.
>
> Ok, probably easier to review too. :-)


Yeah, and it's time to flush this code as it's good globally.

Plus it would be nice to get this for .35

Ah and forget about the sysctl ABI breakages. Since this is only
used for kernel development, this is not going to break much things.
If somebody complains, we can still reintegrate what we had.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Cyrill Gorcunov on
On Wed, May 12, 2010 at 11:00:36PM +0200, Frederic Weisbecker wrote:
....
> > >
> > > Ah right.
> > >
> > > BTW, if you address my reviews, please do it incrementally, I'm going
> > > to apply this set and push it to Ingo.
> >
> > Ok, probably easier to review too. :-)
>
>
> Yeah, and it's time to flush this code as it's good globally.
>
> Plus it would be nice to get this for .35
>
> Ah and forget about the sysctl ABI breakages. Since this is only
> used for kernel development, this is not going to break much things.
> If somebody complains, we can still reintegrate what we had.
>
> Thanks.
>
....
Guys, could you please spend a few minutes and enlighten me a bit?
Does all this series mean that we eventually will drop nmi-watchdog
via io-apic (read via pic) as only the transition to perf complete?

I recall someone said about to stop using io-apic, but just to be sure.

-- Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Cyrill Gorcunov on
On Wed, May 12, 2010 at 05:50:15PM -0400, Don Zickus wrote:
....
> > Guys, could you please spend a few minutes and enlighten me a bit?
> > Does all this series mean that we eventually will drop nmi-watchdog
> > via io-apic (read via pic) as only the transition to perf complete?
> >
> > I recall someone said about to stop using io-apic, but just to be sure.
>
> Right, this code sits on top of the perf subsystem which uses lapic.
> Eventually, the old nmi watchdog code will disappear along with the
> support for io-apic.
>
> Cheers,
> Don
>

Thanks for explanation, Don! So I assume (?) that io-apic
nmi-watchdog is going to be dropped due to obsolescense of
this mode, right?

-- Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/