Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1311167 > unrolled thread

Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required Guenter Roeck <linux@roeck-us.net> - 2016-01-17 18:20 +0100
    RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-01-17 22:00 +0100
      Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required Guenter Roeck <linux@roeck-us.net> - 2016-01-17 22:50 +0100
        RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-01-18 14:20 +0100
          Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required Guenter Roeck <linux@roeck-us.net> - 2016-01-18 16:50 +0100
            RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-01-18 20:40 +0100
              Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required Guenter Roeck <linux@roeck-us.net> - 2016-01-18 21:30 +0100
                RE: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device  only if required "Winkler, Tomas" <tomas.winkler@intel.com> - 2016-01-18 23:00 +0100

#1311167 — Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only if required

Hi Tomas,

On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> From: Alexander Usyskin <alexander.usyskin@intel.com>
> 
> For Intel Broadwell and newer platforms, the ME device can inform
> the host whether the watchdog functionality is activated or not.
> If the watchdog functionality is not activated then the watchdog interface
> can be not registered and eliminate unnecessary pings and hence lower the
> power consumption by avoiding waking up the device.
> The feature can be deactivated also without reboot
> in that case the watchdog device should be unregistered at runtime.
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
> V2: rework unregistration
> V3: rebase; implement unregistraion also at runtime
> V4: Rebase the code over patchset : "watchdog: Replace driver based refcounting"
> 
>  drivers/watchdog/mei_wdt.c | 196 ++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 187 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> index e7e3f144f2b0..85b27fc5d4ec 100644
> --- a/drivers/watchdog/mei_wdt.c
> +++ b/drivers/watchdog/mei_wdt.c
>  
[ ... ]

> +static void mei_wdt_unregister_work(struct work_struct *work)
> +{
> +	struct mei_wdt *wdt = container_of(work, struct mei_wdt, unregister);
> +
> +	mei_wdt_unregister(wdt);
> +}

Registration is synchronous, unregistration is asynchronous.

Assuming that is on purpose, I think it warrants an explanation.

Thanks,
Guenter

[toc] | [next] | [standalone]

#1311185

> -----Original Message-----
> From: Guenter Roeck [mailto:linux@roeck-us.net]
> Sent: Sunday, January 17, 2016 19:13
> To: Winkler, Tomas
> Cc: Greg Kroah-Hartman; Wim Van Sebroeck; Usyskin, Alexander; linux-
> watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only
> if required
> 
> Hi Tomas,
> 
> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> > From: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> > For Intel Broadwell and newer platforms, the ME device can inform
> > the host whether the watchdog functionality is activated or not.
> > If the watchdog functionality is not activated then the watchdog interface
> > can be not registered and eliminate unnecessary pings and hence lower the
> > power consumption by avoiding waking up the device.
> > The feature can be deactivated also without reboot
> > in that case the watchdog device should be unregistered at runtime.
> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> > ---
> > V2: rework unregistration
> > V3: rebase; implement unregistraion also at runtime
> > V4: Rebase the code over patchset : "watchdog: Replace driver based
> refcounting"
> >
> >  drivers/watchdog/mei_wdt.c | 196
> ++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 187 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> > index e7e3f144f2b0..85b27fc5d4ec 100644
> > --- a/drivers/watchdog/mei_wdt.c
> > +++ b/drivers/watchdog/mei_wdt.c
> >
> [ ... ]
> 
> > +static void mei_wdt_unregister_work(struct work_struct *work)
> > +{
> > +	struct mei_wdt *wdt = container_of(work, struct mei_wdt, unregister);
> > +
> > +	mei_wdt_unregister(wdt);
> > +}
> 
> Registration is synchronous, unregistration is asynchronous.
> 
> Assuming that is on purpose, I think it warrants an explanation.
> 
The unregistration is detected on response from the  ping, which is run under same mutex as unregistration.
Tomas

[toc] | [prev] | [next] | [standalone]

#1311192

On 01/17/2016 12:54 PM, Winkler, Tomas wrote:
>
>
>> -----Original Message-----
>> From: Guenter Roeck [mailto:linux@roeck-us.net]
>> Sent: Sunday, January 17, 2016 19:13
>> To: Winkler, Tomas
>> Cc: Greg Kroah-Hartman; Wim Van Sebroeck; Usyskin, Alexander; linux-
>> watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
>> Subject: Re: [char-misc-next, v4, 5/7] watchdog: mei_wdt: register wd device only
>> if required
>>
>> Hi Tomas,
>>
>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
>>>
>>> For Intel Broadwell and newer platforms, the ME device can inform
>>> the host whether the watchdog functionality is activated or not.
>>> If the watchdog functionality is not activated then the watchdog interface
>>> can be not registered and eliminate unnecessary pings and hence lower the
>>> power consumption by avoiding waking up the device.
>>> The feature can be deactivated also without reboot
>>> in that case the watchdog device should be unregistered at runtime.
>>>
>>> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
>>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>>> ---
>>> V2: rework unregistration
>>> V3: rebase; implement unregistraion also at runtime
>>> V4: Rebase the code over patchset : "watchdog: Replace driver based
>> refcounting"
>>>
>>>   drivers/watchdog/mei_wdt.c | 196
>> ++++++++++++++++++++++++++++++++++++++++++---
>>>   1 file changed, 187 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
>>> index e7e3f144f2b0..85b27fc5d4ec 100644
>>> --- a/drivers/watchdog/mei_wdt.c
>>> +++ b/drivers/watchdog/mei_wdt.c
>>>
>> [ ... ]
>>
>>> +static void mei_wdt_unregister_work(struct work_struct *work)
>>> +{
>>> +	struct mei_wdt *wdt = container_of(work, struct mei_wdt, unregister);
>>> +
>>> +	mei_wdt_unregister(wdt);
>>> +}
>>
>> Registration is synchronous, unregistration is asynchronous.
>>
>> Assuming that is on purpose, I think it warrants an explanation.
>>
> The unregistration is detected on response from the  ping, which is run under same mutex as unregistration.
> Tomas
>
>
And that explains why registration can be synchronous and unregistration
has to be asynchronous ?

Guenter

[toc] | [prev] | [next] | [standalone]

#1311537

> only
> >> if required
> >>
> >> Hi Tomas,
> >>
> >> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> >>> From: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>
> >>> For Intel Broadwell and newer platforms, the ME device can inform
> >>> the host whether the watchdog functionality is activated or not.
> >>> If the watchdog functionality is not activated then the watchdog interface
> >>> can be not registered and eliminate unnecessary pings and hence lower the
> >>> power consumption by avoiding waking up the device.
> >>> The feature can be deactivated also without reboot
> >>> in that case the watchdog device should be unregistered at runtime.
> >>>
> >>> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> >>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> >>> ---
> >>> V2: rework unregistration
> >>> V3: rebase; implement unregistraion also at runtime
> >>> V4: Rebase the code over patchset : "watchdog: Replace driver based
> >> refcounting"
> >>>
> >>>   drivers/watchdog/mei_wdt.c | 196
> >> ++++++++++++++++++++++++++++++++++++++++++---
> >>>   1 file changed, 187 insertions(+), 9 deletions(-)
> >>>
> >>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> >>> index e7e3f144f2b0..85b27fc5d4ec 100644
> >>> --- a/drivers/watchdog/mei_wdt.c
> >>> +++ b/drivers/watchdog/mei_wdt.c
> >>>
> >> [ ... ]
> >>
> >>> +static void mei_wdt_unregister_work(struct work_struct *work)
> >>> +{
> >>> +	struct mei_wdt *wdt = container_of(work, struct mei_wdt, unregister);
> >>> +
> >>> +	mei_wdt_unregister(wdt);
> >>> +}
> >>
> >> Registration is synchronous, unregistration is asynchronous.
> >>
> >> Assuming that is on purpose, I think it warrants an explanation.
> >>
> > The unregistration is detected on response from the  ping, which is run under
> same mutex as unregistration.
> > Tomas
> >
> >
> And that explains why registration can be synchronous and unregistration
> has to be asynchronous ?

You need to connect the dots but yes.
The registration is run from the internal ping request (in probe) or from an internal event (in runtime), so the flow is not already locked by the watchdog mutex. 
Hope it helps.
Tomas 

[toc] | [prev] | [next] | [standalone]

#1311629

On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
>
>
>> only
>>>> if required
>>>>
>>>> Hi Tomas,
>>>>
>>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
>>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
>>>>>
>>>>> For Intel Broadwell and newer platforms, the ME device can inform
>>>>> the host whether the watchdog functionality is activated or not.
>>>>> If the watchdog functionality is not activated then the watchdog interface
>>>>> can be not registered and eliminate unnecessary pings and hence lower the
>>>>> power consumption by avoiding waking up the device.
>>>>> The feature can be deactivated also without reboot
>>>>> in that case the watchdog device should be unregistered at runtime.
>>>>>
>>>>> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
>>>>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>>>>> ---
>>>>> V2: rework unregistration
>>>>> V3: rebase; implement unregistraion also at runtime
>>>>> V4: Rebase the code over patchset : "watchdog: Replace driver based
>>>> refcounting"
>>>>>
>>>>>    drivers/watchdog/mei_wdt.c | 196
>>>> ++++++++++++++++++++++++++++++++++++++++++---
>>>>>    1 file changed, 187 insertions(+), 9 deletions(-)
>>>>>
>>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
>>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
>>>>> --- a/drivers/watchdog/mei_wdt.c
>>>>> +++ b/drivers/watchdog/mei_wdt.c
>>>>>
>>>> [ ... ]
>>>>
>>>>> +static void mei_wdt_unregister_work(struct work_struct *work)
>>>>> +{
>>>>> +	struct mei_wdt *wdt = container_of(work, struct mei_wdt, unregister);
>>>>> +
>>>>> +	mei_wdt_unregister(wdt);
>>>>> +}
>>>>
>>>> Registration is synchronous, unregistration is asynchronous.
>>>>
>>>> Assuming that is on purpose, I think it warrants an explanation.
>>>>
>>> The unregistration is detected on response from the  ping, which is run under
>> same mutex as unregistration.
>>> Tomas
>>>
>>>
>> And that explains why registration can be synchronous and unregistration
>> has to be asynchronous ?
>
> You need to connect the dots but yes.
> The registration is run from the internal ping request (in probe) or from an internal event (in runtime), so the flow is not already locked by the watchdog mutex.
> Hope it helps.

What I was asking for is a comment such as

"We can not unregister directly because a ping operation (triggered
through the watchdog subsystem) is pending and must be completed first."

Guenter

[toc] | [prev] | [next] | [standalone]

#1311772

> 
> On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
> >
> >
> >> only
> >>>> if required
> >>>>
> >>>> Hi Tomas,
> >>>>
> >>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> >>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>>>
> >>>>> For Intel Broadwell and newer platforms, the ME device can inform
> >>>>> the host whether the watchdog functionality is activated or not.
> >>>>> If the watchdog functionality is not activated then the watchdog interface
> >>>>> can be not registered and eliminate unnecessary pings and hence lower
> the
> >>>>> power consumption by avoiding waking up the device.
> >>>>> The feature can be deactivated also without reboot
> >>>>> in that case the watchdog device should be unregistered at runtime.
> >>>>>
> >>>>> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> >>>>> ---
> >>>>> V2: rework unregistration
> >>>>> V3: rebase; implement unregistraion also at runtime
> >>>>> V4: Rebase the code over patchset : "watchdog: Replace driver based
> >>>> refcounting"
> >>>>>
> >>>>>    drivers/watchdog/mei_wdt.c | 196
> >>>> ++++++++++++++++++++++++++++++++++++++++++---
> >>>>>    1 file changed, 187 insertions(+), 9 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> >>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
> >>>>> --- a/drivers/watchdog/mei_wdt.c
> >>>>> +++ b/drivers/watchdog/mei_wdt.c
> >>>>>
> >>>> [ ... ]
> >>>>
> >>>>> +static void mei_wdt_unregister_work(struct work_struct *work)
> >>>>> +{
> >>>>> +	struct mei_wdt *wdt = container_of(work, struct mei_wdt,
> unregister);
> >>>>> +
> >>>>> +	mei_wdt_unregister(wdt);
> >>>>> +}
> >>>>
> >>>> Registration is synchronous, unregistration is asynchronous.
> >>>>
> >>>> Assuming that is on purpose, I think it warrants an explanation.
> >>>>
> >>> The unregistration is detected on response from the  ping, which is run under
> >> same mutex as unregistration.
> >>> Tomas
> >>>
> >>>
> >> And that explains why registration can be synchronous and unregistration
> >> has to be asynchronous ?
> >
> > You need to connect the dots but yes.
> > The registration is run from the internal ping request (in probe) or from an
> internal event (in runtime), so the flow is not already locked by the watchdog
> mutex.
> > Hope it helps.
> 
> What I was asking for is a comment such as
> 
> "We can not unregister directly because a ping operation (triggered
> through the watchdog subsystem) is pending and must be completed first."

You can put it also that way, but in the bottom line you will get deadlock. 
According your comment I'm guessing  that you are asking me to update the commit message, please be more direct, 
I'm not native English speaker and may miss little nuances. 

Thanks
Tomas

[toc] | [prev] | [next] | [standalone]

#1311793

On 01/18/2016 11:36 AM, Winkler, Tomas wrote:
>>
>> On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
>>>
>>>
>>>> only
>>>>>> if required
>>>>>>
>>>>>> Hi Tomas,
>>>>>>
>>>>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
>>>>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
>>>>>>>
>>>>>>> For Intel Broadwell and newer platforms, the ME device can inform
>>>>>>> the host whether the watchdog functionality is activated or not.
>>>>>>> If the watchdog functionality is not activated then the watchdog interface
>>>>>>> can be not registered and eliminate unnecessary pings and hence lower
>> the
>>>>>>> power consumption by avoiding waking up the device.
>>>>>>> The feature can be deactivated also without reboot
>>>>>>> in that case the watchdog device should be unregistered at runtime.
>>>>>>>
>>>>>>> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
>>>>>>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>>>>>>> ---
>>>>>>> V2: rework unregistration
>>>>>>> V3: rebase; implement unregistraion also at runtime
>>>>>>> V4: Rebase the code over patchset : "watchdog: Replace driver based
>>>>>> refcounting"
>>>>>>>
>>>>>>>     drivers/watchdog/mei_wdt.c | 196
>>>>>> ++++++++++++++++++++++++++++++++++++++++++---
>>>>>>>     1 file changed, 187 insertions(+), 9 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
>>>>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
>>>>>>> --- a/drivers/watchdog/mei_wdt.c
>>>>>>> +++ b/drivers/watchdog/mei_wdt.c
>>>>>>>
>>>>>> [ ... ]
>>>>>>
>>>>>>> +static void mei_wdt_unregister_work(struct work_struct *work)
>>>>>>> +{
>>>>>>> +	struct mei_wdt *wdt = container_of(work, struct mei_wdt,
>> unregister);
>>>>>>> +
>>>>>>> +	mei_wdt_unregister(wdt);
>>>>>>> +}
>>>>>>
>>>>>> Registration is synchronous, unregistration is asynchronous.
>>>>>>
>>>>>> Assuming that is on purpose, I think it warrants an explanation.
>>>>>>
>>>>> The unregistration is detected on response from the  ping, which is run under
>>>> same mutex as unregistration.
>>>>> Tomas
>>>>>
>>>>>
>>>> And that explains why registration can be synchronous and unregistration
>>>> has to be asynchronous ?
>>>
>>> You need to connect the dots but yes.
>>> The registration is run from the internal ping request (in probe) or from an
>> internal event (in runtime), so the flow is not already locked by the watchdog
>> mutex.
>>> Hope it helps.
>>
>> What I was asking for is a comment such as
>>
>> "We can not unregister directly because a ping operation (triggered
>> through the watchdog subsystem) is pending and must be completed first."
>
> You can put it also that way, but in the bottom line you will get deadlock.

Yes, understood. Maybe that should be part of the comment.

> According your comment I'm guessing  that you are asking me to update the commit message, please be more direct,
> I'm not native English speaker and may miss little nuances.
>
Hi Tomas,

please add a comment into the source code, describing why unregistration has to be
asynchronous. It took me a while to understand the context, and we want to make
sure that others don't have to repeat that exercise.

Other than that, the series is fine with me, except that the patches
affecting the mei directory don't apply to the current mainline
as of this morning (possibly due to some other changes in that directory).

I would suggest to add the comment, wait for -rc1, rebase, and re-send
the series with my Acked-by: added to all patches.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1311815

> 
> On 01/18/2016 11:36 AM, Winkler, Tomas wrote:
> >>
> >> On 01/18/2016 05:19 AM, Winkler, Tomas wrote:
> >>>
> >>>
> >>>> only
> >>>>>> if required
> >>>>>>
> >>>>>> Hi Tomas,
> >>>>>>
> >>>>>> On Fri, Jan 08, 2016 at 12:49:25AM +0200, Winkler, Tomas wrote:
> >>>>>>> From: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>>>>>
> >>>>>>> For Intel Broadwell and newer platforms, the ME device can inform
> >>>>>>> the host whether the watchdog functionality is activated or not.
> >>>>>>> If the watchdog functionality is not activated then the watchdog
> interface
> >>>>>>> can be not registered and eliminate unnecessary pings and hence lower
> >> the
> >>>>>>> power consumption by avoiding waking up the device.
> >>>>>>> The feature can be deactivated also without reboot
> >>>>>>> in that case the watchdog device should be unregistered at runtime.
> >>>>>>>
> >>>>>>> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> >>>>>>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> >>>>>>> ---
> >>>>>>> V2: rework unregistration
> >>>>>>> V3: rebase; implement unregistraion also at runtime
> >>>>>>> V4: Rebase the code over patchset : "watchdog: Replace driver based
> >>>>>> refcounting"
> >>>>>>>
> >>>>>>>     drivers/watchdog/mei_wdt.c | 196
> >>>>>> ++++++++++++++++++++++++++++++++++++++++++---
> >>>>>>>     1 file changed, 187 insertions(+), 9 deletions(-)
> >>>>>>>
> >>>>>>> diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c
> >>>>>>> index e7e3f144f2b0..85b27fc5d4ec 100644
> >>>>>>> --- a/drivers/watchdog/mei_wdt.c
> >>>>>>> +++ b/drivers/watchdog/mei_wdt.c
> >>>>>>>
> >>>>>> [ ... ]
> >>>>>>
> >>>>>>> +static void mei_wdt_unregister_work(struct work_struct *work)
> >>>>>>> +{
> >>>>>>> +	struct mei_wdt *wdt = container_of(work, struct mei_wdt,
> >> unregister);
> >>>>>>> +
> >>>>>>> +	mei_wdt_unregister(wdt);
> >>>>>>> +}
> >>>>>>
> >>>>>> Registration is synchronous, unregistration is asynchronous.
> >>>>>>
> >>>>>> Assuming that is on purpose, I think it warrants an explanation.
> >>>>>>
> >>>>> The unregistration is detected on response from the  ping, which is run
> under
> >>>> same mutex as unregistration.
> >>>>> Tomas
> >>>>>
> >>>>>
> >>>> And that explains why registration can be synchronous and unregistration
> >>>> has to be asynchronous ?
> >>>
> >>> You need to connect the dots but yes.
> >>> The registration is run from the internal ping request (in probe) or from an
> >> internal event (in runtime), so the flow is not already locked by the watchdog
> >> mutex.
> >>> Hope it helps.
> >>
> >> What I was asking for is a comment such as
> >>
> >> "We can not unregister directly because a ping operation (triggered
> >> through the watchdog subsystem) is pending and must be completed first."
> >
> > You can put it also that way, but in the bottom line you will get deadlock.
> 
> Yes, understood. Maybe that should be part of the comment.
> 
> > According your comment I'm guessing  that you are asking me to update the
> commit message, please be more direct,
> > I'm not native English speaker and may miss little nuances.
> >
> Hi Tomas,
> 
> please add a comment into the source code, describing why unregistration has to
> be
> asynchronous. It took me a while to understand the context, and we want to
> make
> sure that others don't have to repeat that exercise.

Sure, no problem. 

> Other than that, the series is fine with me, except that the patches
> affecting the mei directory don't apply to the current mainline
> as of this morning (possibly due to some other changes in that directory).

There are other pending patches that weren't merged yet and are not related to watchdog, we see if they require rebase after the merging window is closed. 

> 
> I would suggest to add the comment, wait for -rc1, rebase, and re-send
> the series with my Acked-by: added to all patches.

Thanks and I appreciate your time reviewing this series.  
Tomas 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web