Messages in this thread | | | Date | Fri, 5 Nov 2010 12:58:55 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] watchdog: touch_nmi_watchdog should only touch local cpu not every one |
| |
On Fri, 5 Nov 2010 15:51:18 +0200 Sergey Senozhatsky <sergey.senozhatsky@gmail.com> wrote:
> On (11/04/10 21:18), Don Zickus wrote: > > void touch_nmi_watchdog(void) > > { > > + /* > > + * Using __raw here because some code paths have > > + * preemption enabled. If preemption is enabled > > + * then interrupts should be enabled too, in which > > + * case we shouldn't have to worry about the watchdog > > + * going off. > > + */ > > + __raw_get_cpu_var(watchdog_nmi_touch) = true; > > + > > + touch_softlockup_watchdog(); > > +} > > +EXPORT_SYMBOL(touch_nmi_watchdog); > > + > > +void touch_all_nmi_watchdogs(void) > > +{ > > if (watchdog_enabled) { > > unsigned cpu; > > > > @@ -151,7 +166,7 @@ void touch_nmi_watchdog(void) > > } > > touch_softlockup_watchdog(); > > } > > -EXPORT_SYMBOL(touch_nmi_watchdog); > > +EXPORT_SYMBOL(touch_all_nmi_watchdogs); > > > > Hello, > Seems like no one is actually calling touch_all_nmi_watchdogs, as for now. > Right?
Yes, there doesn't seem a lot of point in adding the interface unless we have callers.
> > Minor nit > > touch_all_nmi_watchdogs: > ... > for_each_present_cpu(cpu) { > if (per_cpu(watchdog_nmi_touch, cpu) != true) > per_cpu(watchdog_nmi_touch, cpu) = true; > } > > > which is, I belive, could be simplified to > for_each_present_cpu(cpu) { > per_cpu(watchdog_nmi_touch, cpu) = true; > }
We sometimes do this trick to avoid dirtying lots of cachelines which already held the correct value. It'll be extra-benefical when dealing with other CPU's data, I expect.
| |