[PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Larry Finger]

The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
del_timer() instead.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
Cc: Haggi Eran <haggai.eran@gmail.com>
---
 drivers/staging/rtl8712/rtl8712_led.c  | 2 +-
 drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl8712_led.c b/drivers/staging/rtl8712/rtl8712_led.c
index f1d47a0..8cc716c 100644
--- a/drivers/staging/rtl8712/rtl8712_led.c
+++ b/drivers/staging/rtl8712/rtl8712_led.c
@@ -921,7 +921,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
 			    IS_LED_WPS_BLINKING(pLed))
 				return;
 			if (pLed->bLedNoLinkBlinkInProgress == true) {
-				del_timer_sync(&pLed->BlinkTimer);
+				del_timer(&pLed->BlinkTimer);
 				pLed->bLedNoLinkBlinkInProgress = false;
 			}
 			if (pLed->bLedBlinkInProgress == true) {
diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index fb2b195..ace88ab 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -582,7 +582,7 @@ void r8712_surveydone_event_callback(struct _adapter *adapter, u8 *pbuf)
 	spin_lock_irqsave(&pmlmepriv->lock, irqL);
 
 	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true) {
-		del_timer_sync(&pmlmepriv->scan_to_timer);
+		del_timer(&pmlmepriv->scan_to_timer);
 
 		_clr_fwstate_(pmlmepriv, _FW_UNDER_SURVEY);
 	}
@@ -910,7 +910,7 @@ void r8712_joinbss_event_callback(struct _adapter *adapter, u8 *pbuf)
 			if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)
 				== true)
 				r8712_indicate_connect(adapter);
-			del_timer_sync(&pmlmepriv->assoc_timer);
+			del_timer(&pmlmepriv->assoc_timer);
 		} else
 			goto ignore_joinbss_callback;
 	} else {
-- 
2.1.4

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Haggai Eran]

On 24 May 2015 at 00:16, Larry Finger <Larry.Finger@lwfinger.net> wrote:
> The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
> del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
> del_timer() instead.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org>
> Cc: Haggi Eran <haggai.eran@gmail.com>

Hi,

I haven't been using kernel v4.1 so I haven't seen this warning, but looking
at the code it seems to originate from the two recent patches to remove
_cancel_timer and _cancel_timer_ex. I see that there's another patch in lkml [1]
that changes del_timer_sync back to del_timer in more places. Perhaps it
could prevent other warnings like this in the future.

Regards,
Haggai

[1] https://lkml.org/lkml/2015/5/15/226

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Larry Finger]

On 05/24/2015 02:03 PM, Haggai Eran wrote:
> On 24 May 2015 at 00:16, Larry Finger <Larry.Finger@lwfinger.net> wrote:
>> The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
>> del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
>> del_timer() instead.
>>
>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
>> Cc: Stable <stable@vger.kernel.org>
>> Cc: Haggi Eran <haggai.eran@gmail.com>
>
> Hi,
>
> I haven't been using kernel v4.1 so I haven't seen this warning, but looking
> at the code it seems to originate from the two recent patches to remove
> _cancel_timer and _cancel_timer_ex. I see that there's another patch in lkml [1]
> that changes del_timer_sync back to del_timer in more places. Perhaps it
> could prevent other warnings like this in the future.
>
> Regards,
> Haggai
>
> [1] https://lkml.org/lkml/2015/5/15/226

Yes, the script kiddies make changes they do not understand and screw everything 
up. Unfortunately, I did not catch these in review. I think I will submit V2 and 
blast the contributor.

Larry

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Dan Carpenter]

On Sun, May 24, 2015 at 07:11:40PM -0500, Larry Finger wrote:
> On 05/24/2015 02:03 PM, Haggai Eran wrote:
> >On 24 May 2015 at 00:16, Larry Finger <Larry.Finger@lwfinger.net> wrote:
> >>The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
> >>del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
> >>del_timer() instead.
> >>
> >>Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> >>Cc: Stable <stable@vger.kernel.org>
> >>Cc: Haggi Eran <haggai.eran@gmail.com>
> >
> >Hi,
> >
> >I haven't been using kernel v4.1 so I haven't seen this warning, but looking
> >at the code it seems to originate from the two recent patches to remove
> >_cancel_timer and _cancel_timer_ex. I see that there's another patch in lkml [1]
> >that changes del_timer_sync back to del_timer in more places. Perhaps it
> >could prevent other warnings like this in the future.
> >
> >Regards,
> >Haggai
> >
> >[1] https://lkml.org/lkml/2015/5/15/226
> 
> Yes, the script kiddies make changes they do not understand and
> screw everything up. Unfortunately, I did not catch these in review.
> I think I will submit V2 and blast the contributor.

Don't blast the contributor...  These are special intern patches that
dont' go through the normal review process.  The intern process is over
this year.  The lack of normal review introduced a number of bugs this
year.  I always complain to Greg about it and he says that I should join
the intern mailing list if I care so much.

regards,
dan carpenter

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Dan Carpenter]

On Mon, May 25, 2015 at 03:07:08PM +0530, Vaishali Thakkar wrote:
> I am sorry for those patches. It was me who introduced those bugs. Yes, it
> was sent during Outreachy process. But it was my mistake as a newbie. May
> be I should have taken care of interrupt mode thing.
> 
> I would like to fix it if someone is not doing it. Sorry again. I will take
> care of these things in my future patches.

No, it's not your fault for making mistakes.  We *expect* newbies to
make mistakes.

It's Greg's fault for merging these when they weren't sent to the list
or to the other maintainers.  But Greg knows I'm annoyed already since
we have been dealing with the fallout for months and I always make sure
to complain whenever I have a chance.  /me shakes a fist!  Grrr....  :P

regards,
dan carpenter

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Larry Finger]

On 05/25/2015 04:37 AM, Vaishali Thakkar wrote:
>
> On 25 May 2015 14:49, "Dan Carpenter" <dan.carpenter@oracle.com
> <mailto:dan.carpenter@oracle.com>> wrote:
>  >
>  > On Sun, May 24, 2015 at 07:11:40PM -0500, Larry Finger wrote:
>  > > On 05/24/2015 02:03 PM, Haggai Eran wrote:
>  > > >On 24 May 2015 at 00:16, Larry Finger <Larry.Finger@lwfinger.net
> <mailto:Larry.Finger@lwfinger.net>> wrote:
>  > > >>The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
>  > > >>del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
>  > > >>del_timer() instead.
>  > > >>
>  > > >>Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net
> <mailto:Larry.Finger@lwfinger.net>>
>  > > >>Cc: Stable <stable@vger.kernel.org <mailto:stable@vger.kernel.org>>
>  > > >>Cc: Haggi Eran <haggai.eran@gmail.com <mailto:haggai.eran@gmail.com>>
>  > > >
>  > > >Hi,
>  > > >
>  > > >I haven't been using kernel v4.1 so I haven't seen this warning, but looking
>  > > >at the code it seems to originate from the two recent patches to remove
>  > > >_cancel_timer and _cancel_timer_ex. I see that there's another patch in
> lkml [1]
>  > > >that changes del_timer_sync back to del_timer in more places. Perhaps it
>  > > >could prevent other warnings like this in the future.
>  > > >
>  > > >Regards,
>  > > >Haggai
>  > > >
>  > > >[1] https://lkml.org/lkml/2015/5/15/226
>  > >
>  > > Yes, the script kiddies make changes they do not understand and
>  > > screw everything up. Unfortunately, I did not catch these in review.
>  > > I think I will submit V2 and blast the contributor.
>  >
>  > Don't blast the contributor...  These are special intern patches that
>  > dont' go through the normal review process.  The intern process is over
>  > this year.  The lack of normal review introduced a number of bugs this
>  > year.  I always complain to Greg about it and he says that I should join
>  > the intern mailing list if I care so much.
>
> I am sorry for those patches. It was me who introduced those bugs. Yes, it was
> sent during Outreachy process. But it was my mistake as a newbie. May be I
> should have taken care of interrupt mode thing.
>
> I would like to fix it if someone is not doing it. Sorry again. I will take care
> of these things in my future patches.

No, one of us will fix the problems with r8712u. The hardware is needed for 
proper testing, and I doubt that you have it.

Larry

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Larry Finger]

On 05/23/2015 04:16 PM, Larry Finger wrote:
> The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
> del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
> del_timer() instead.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Stable <stable@vger.kernel.org>
> Cc: Haggi Eran <haggai.eran@gmail.com>
> ---

Greg,

Please drop this patch. The same fixes were submitted as 
https://lkml.org/lkml/2015/5/15/226.

It is crucial that this get into the 4.1 kernel where the regression was introduced.

Larry

>   drivers/staging/rtl8712/rtl8712_led.c  | 2 +-
>   drivers/staging/rtl8712/rtl871x_mlme.c | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl8712_led.c b/drivers/staging/rtl8712/rtl8712_led.c
> index f1d47a0..8cc716c 100644
> --- a/drivers/staging/rtl8712/rtl8712_led.c
> +++ b/drivers/staging/rtl8712/rtl8712_led.c
> @@ -921,7 +921,7 @@ static void SwLedControlMode1(struct _adapter *padapter,
>   			    IS_LED_WPS_BLINKING(pLed))
>   				return;
>   			if (pLed->bLedNoLinkBlinkInProgress == true) {
> -				del_timer_sync(&pLed->BlinkTimer);
> +				del_timer(&pLed->BlinkTimer);
>   				pLed->bLedNoLinkBlinkInProgress = false;
>   			}
>   			if (pLed->bLedBlinkInProgress == true) {
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index fb2b195..ace88ab 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -582,7 +582,7 @@ void r8712_surveydone_event_callback(struct _adapter *adapter, u8 *pbuf)
>   	spin_lock_irqsave(&pmlmepriv->lock, irqL);
>
>   	if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY) == true) {
> -		del_timer_sync(&pmlmepriv->scan_to_timer);
> +		del_timer(&pmlmepriv->scan_to_timer);
>
>   		_clr_fwstate_(pmlmepriv, _FW_UNDER_SURVEY);
>   	}
> @@ -910,7 +910,7 @@ void r8712_joinbss_event_callback(struct _adapter *adapter, u8 *pbuf)
>   			if (check_fwstate(pmlmepriv, WIFI_STATION_STATE)
>   				== true)
>   				r8712_indicate_connect(adapter);
> -			del_timer_sync(&pmlmepriv->assoc_timer);
> +			del_timer(&pmlmepriv->assoc_timer);
>   		} else
>   			goto ignore_joinbss_callback;
>   	} else {
>

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Joe Perches]

On Mon, 2015-05-25 at 12:17 +0300, Dan Carpenter wrote:
> These are special intern patches that
> dont' go through the normal review process.  The intern process is over
> this year.  The lack of normal review introduced a number of bugs this
> year.  I always complain to Greg about it and he says that I should join
> the intern mailing list if I care so much.

It'd be better if the approved patches from the intern list
(no idea what that is) were sent to lkml/devel@driverdev lists
for review before actually being applied.

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Sudip Mukherjee]

On Mon, May 25, 2015 at 10:37:28AM -0700, Joe Perches wrote:
> It'd be better if the approved patches from the intern list
> (no idea what that is) were sent to lkml/devel@driverdev lists
> for review before actually being applied.
Its the outreachy program. And http://kernelnewbies.org/OutreachyApply
mentions specifically: "email your first patch to the outreachy-kernel
mailing list. Do not send patches to the main Linux mailing lists"

regards
sudip

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Joe Perches]

On Tue, 2015-05-26 at 10:16 +0530, Sudip Mukherjee wrote:
> On Mon, May 25, 2015 at 10:37:28AM -0700, Joe Perches wrote:
> > It'd be better if the approved patches from the intern list
> > (no idea what that is) were sent to lkml/devel@driverdev lists
> > for review before actually being applied.
> Its the outreachy program. And http://kernelnewbies.org/OutreachyApply
> mentions specifically: "email your first patch to the outreachy-kernel
> mailing list. Do not send patches to the tmain Linux mailing lists"

The same thing happens with the eudyptula patches.

That doesn't at all make applying those novice patches
without review by a more widely read list any more sensible.

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Sudip Mukherjee]

On Mon, May 25, 2015 at 09:55:08PM -0700, Joe Perches wrote:
> On Tue, 2015-05-26 at 10:16 +0530, Sudip Mukherjee wrote:
> > On Mon, May 25, 2015 at 10:37:28AM -0700, Joe Perches wrote:
> > > It'd be better if the approved patches from the intern list
> > > (no idea what that is) were sent to lkml/devel@driverdev lists
> > > for review before actually being applied.
> > Its the outreachy program. And http://kernelnewbies.org/OutreachyApply
> > mentions specifically: "email your first patch to the outreachy-kernel
> > mailing list. Do not send patches to the tmain Linux mailing lists"
> 
> The same thing happens with the eudyptula patches.
But for eudyptula, patches have to properly submitted to lkml and
maintainers. And today I am here just because of eudyptula. My patches
and contributions here, everything started with it. :)

regards
sudip
> 
> 
> 

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Joe Perches]

On Tue, 2015-05-26 at 10:32 +0530, Sudip Mukherjee wrote:
> On Mon, May 25, 2015 at 09:55:08PM -0700, Joe Perches wrote:
> > On Tue, 2015-05-26 at 10:16 +0530, Sudip Mukherjee wrote:
> > > On Mon, May 25, 2015 at 10:37:28AM -0700, Joe Perches wrote:
> > > > It'd be better if the approved patches from the intern list
> > > > (no idea what that is) were sent to lkml/devel@driverdev lists
> > > > for review before actually being applied.
> > > Its the outreachy program. And http://kernelnewbies.org/OutreachyApply
> > > mentions specifically: "email your first patch to the outreachy-kernel
> > > mailing list. Do not send patches to the tmain Linux mailing lists"
> > 
> > The same thing happens with the eudyptula patches.
> But for eudyptula, patches have to properly submitted to lkml and
> maintainers. And today I am here just because of eudyptula. My patches
> and contributions here, everything started with it. :)

As far as I understand, the Eudyptula Challenge list has
internal mechanisms to nominally review patches before some
patch is submitted to lkml.

The main point is that patches shouldn't be applied without
being submitted to a more widely read list.

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Sudip Mukherjee]

On Mon, May 25, 2015 at 10:07:59PM -0700, Joe Perches wrote:
> On Tue, 2015-05-26 at 10:32 +0530, Sudip Mukherjee wrote:
> > On Mon, May 25, 2015 at 09:55:08PM -0700, Joe Perches wrote:
> > > On Tue, 2015-05-26 at 10:16 +0530, Sudip Mukherjee wrote:
> > > > On Mon, May 25, 2015 at 10:37:28AM -0700, Joe Perches wrote:

> > But for eudyptula, patches have to properly submitted to lkml and
> > maintainers. And today I am here just because of eudyptula. My patches
> > and contributions here, everything started with it. :)
> 
> As far as I understand, the Eudyptula Challenge list has
> internal mechanisms to nominally review patches before some
> patch is submitted to lkml.
well, yes, sort of. Submitting patch is task 10, and all the previous
tasks will train people on creating patch, sending etc.
> 
> The main point is that patches shouldn't be applied without
> being submitted to a more widely read list.
yes, and Eudyptula requires the participant to send patch to maintainer,
lkml and relevant mailing list where outreachy specifies not to send to
lkml. Next outreachy will start from September.

regards
sudip

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Greg KH]

On Mon, May 25, 2015 at 10:07:59PM -0700, Joe Perches wrote:
> On Tue, 2015-05-26 at 10:32 +0530, Sudip Mukherjee wrote:
> > On Mon, May 25, 2015 at 09:55:08PM -0700, Joe Perches wrote:
> > > On Tue, 2015-05-26 at 10:16 +0530, Sudip Mukherjee wrote:
> > > > On Mon, May 25, 2015 at 10:37:28AM -0700, Joe Perches wrote:
> > > > > It'd be better if the approved patches from the intern list
> > > > > (no idea what that is) were sent to lkml/devel@driverdev lists
> > > > > for review before actually being applied.
> > > > Its the outreachy program. And http://kernelnewbies.org/OutreachyApply
> > > > mentions specifically: "email your first patch to the outreachy-kernel
> > > > mailing list. Do not send patches to the tmain Linux mailing lists"
> > > 
> > > The same thing happens with the eudyptula patches.
> > But for eudyptula, patches have to properly submitted to lkml and
> > maintainers. And today I am here just because of eudyptula. My patches
> > and contributions here, everything started with it. :)
> 
> As far as I understand, the Eudyptula Challenge list has
> internal mechanisms to nominally review patches before some
> patch is submitted to lkml.

No it does not.

> The main point is that patches shouldn't be applied without
> being submitted to a more widely read list.

I take the blame for any problems with Outreachy patches.  Given the
huge volume of them, one bug out of 900 isn't that bad of a percentage.

thanks,

greg k-h

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Joe Perches]

On Tue, 2015-05-26 at 07:14 -0700, Greg KH wrote:
> On Mon, May 25, 2015 at 10:07:59PM -0700, Joe Perches wrote:
> > As far as I understand, the Eudyptula Challenge list has
> > internal mechanisms to nominally review patches before some
> > patch is submitted to lkml.
> 
> No it does not.

That's not quite what was described to me by someone behind the
email address:
"Little Penguin <little@eudyptula-challenge.org>" back in August.

What I understood from that email was there is a qualifying
formatting component to the challenge and then patches were to be
sent to maintainers and lists.

> > The main point is that patches shouldn't be applied without
> > being submitted to a more widely read list.
> 
> I take the blame for any problems with Outreachy patches.

Are you going to change any procedure associated to these
Outreachy patches?

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Larry Finger]

On 05/26/2015 12:30 AM, Sudip Mukherjee wrote:
> On Mon, May 25, 2015 at 10:07:59PM -0700, Joe Perches wrote:
>> On Tue, 2015-05-26 at 10:32 +0530, Sudip Mukherjee wrote:
>>> On Mon, May 25, 2015 at 09:55:08PM -0700, Joe Perches wrote:
>>>> On Tue, 2015-05-26 at 10:16 +0530, Sudip Mukherjee wrote:
>>>>> On Mon, May 25, 2015 at 10:37:28AM -0700, Joe Perches wrote:
>
>>> But for eudyptula, patches have to properly submitted to lkml and
>>> maintainers. And today I am here just because of eudyptula. My patches
>>> and contributions here, everything started with it. :)
>>
>> As far as I understand, the Eudyptula Challenge list has
>> internal mechanisms to nominally review patches before some
>> patch is submitted to lkml.
> well, yes, sort of. Submitting patch is task 10, and all the previous
> tasks will train people on creating patch, sending etc.
>>
>> The main point is that patches shouldn't be applied without
>> being submitted to a more widely read list.
> yes, and Eudyptula requires the participant to send patch to maintainer,
> lkml and relevant mailing list where outreachy specifies not to send to
> lkml. Next outreachy will start from September.

At a minimum, outreachy needs to require that patches are sent to the 
maintainer(s). I can understand not wanting to burden LKML, but getting 
defective patches into the kernel must be avoided.

Larry

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Dan Carpenter]

On Tue, May 26, 2015 at 07:14:01AM -0700, Greg KH wrote:
> I take the blame for any problems with Outreachy patches.  Given the
> huge volume of them, one bug out of 900 isn't that bad of a percentage.

We don't get many bugs through outreachy, but this isn't the first one.
For example, in March and April people complained about:

95745e9b1de2 ('staging: lustre: Use kasprintf.')
45de432775d6 ('Staging: rtl8712: Use memdup_user() instead of copy_from_user()')

There have been others but I have a short memory.  We have this
discussion every time.  How come no one caught this bug in review??  Oh,
it never went through the list.

I'm fine with 5 bugs per 900 patches or whatever.  I wish that the
patches came to the list, but I get that that would double your review
workload to review 900 patches on the outreachy list and again on the
normal dev list.

regards,
dan carpenter

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Greg KH]

On Tue, May 26, 2015 at 07:48:59AM -0700, Joe Perches wrote:
> On Tue, 2015-05-26 at 07:14 -0700, Greg KH wrote:
> > On Mon, May 25, 2015 at 10:07:59PM -0700, Joe Perches wrote:
> > > As far as I understand, the Eudyptula Challenge list has
> > > internal mechanisms to nominally review patches before some
> > > patch is submitted to lkml.
> > 
> > No it does not.
> 
> That's not quite what was described to me by someone behind the
> email address:
> "Little Penguin <little@eudyptula-challenge.org>" back in August.
> 
> What I understood from that email was there is a qualifying
> formatting component to the challenge and then patches were to be
> sent to maintainers and lists.

Hm, I don't know then, but from what I have seen, the tasks that send
patches to the kernel are not pre-reviewed by anyone.  If they were, the
quality level of the patches we get here would have been much higher :)

> > > The main point is that patches shouldn't be applied without
> > > being submitted to a more widely read list.
> > 
> > I take the blame for any problems with Outreachy patches.
> 
> Are you going to change any procedure associated to these
> Outreachy patches?

2 bugs out of 900?  Nah, I think that's good odds.

Also, the outreachy patch process would overwhelm everyone else on the
list, it's really high volume during the application phase, I'd prefer
it to stick with the mentors that wish to help out with the process.  If
you and/or Dan, or anyone else wishes to help out with this, I would
really appreciate it.  But I don't think that forcing them to post to
the driverdevel list is a good idea.

thanks,

greg k-h

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Larry Finger]

On 05/26/2015 10:48 AM, Dan Carpenter wrote:
> On Tue, May 26, 2015 at 07:14:01AM -0700, Greg KH wrote:
>> I take the blame for any problems with Outreachy patches.  Given the
>> huge volume of them, one bug out of 900 isn't that bad of a percentage.
>
> We don't get many bugs through outreachy, but this isn't the first one.
> For example, in March and April people complained about:
>
> 95745e9b1de2 ('staging: lustre: Use kasprintf.')
> 45de432775d6 ('Staging: rtl8712: Use memdup_user() instead of copy_from_user()')
>
> There have been others but I have a short memory.  We have this
> discussion every time.  How come no one caught this bug in review??  Oh,
> it never went through the list.
>
> I'm fine with 5 bugs per 900 patches or whatever.  I wish that the
> patches came to the list, but I get that that would double your review
> workload to review 900 patches on the outreachy list and again on the
> normal dev list.

Adding the maintainer to the original list would still be a good step. I'm not 
sure that I would have caught "rtl8712: Use memdup_user() instead of 
copy_from_user()" in review, but I certainly would have seen that it was not 
proper to do a blanket substitution of del_timer_sync() for a wrapper that used 
del_timer().

Obviously the awful coding in rtl8712 contains more traps to trick the novices 
than does most code. It also predates a lot of changes in checkpatch.pl and 
contains a lot of low-hanging fruit.

Larry

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Joe Perches]

On Tue, 2015-05-26 at 09:35 -0700, Greg KH wrote:
> On Tue, May 26, 2015 at 07:48:59AM -0700, Joe Perches wrote:
> > > > The main point is that patches shouldn't be applied without
> > > > being submitted to a more widely read list.
> > > 
> > > I take the blame for any problems with Outreachy patches.
> > 
> > Are you going to change any procedure associated to these
> > Outreachy patches?
> 
> 2 bugs out of 900?  Nah, I think that's good odds.
> 
> Also, the outreachy patch process would overwhelm everyone else on the
> list, it's really high volume during the application phase, I'd prefer
> it to stick with the mentors that wish to help out with the process.  If
> you and/or Dan, or anyone else wishes to help out with this, I would
> really appreciate it.  But I don't think that forcing them to post to
> the driverdevel list is a good idea.

I don't think that's necessary, but sending them to any
listed maintainer should be.

If you're collecting them, I suggest you stick them in
a separate branch, post them to your driverdev list and
cc the appropriate maintainers, wait a week, then apply
them to your main branch.

You already batch post hundreds of patches for kernel
x.y.z stable branches.  What's another few hundred?

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Dan Carpenter]

On Tue, May 26, 2015 at 09:35:55AM -0700, Greg KH wrote:
> Also, the outreachy patch process would overwhelm everyone else on the
> list, it's really high volume during the application phase, I'd prefer
> it to stick with the mentors that wish to help out with the process.  If
> you and/or Dan, or anyone else wishes to help out with this, I would
> really appreciate it.  But I don't think that forcing them to post to
> the driverdevel list is a good idea.

If it makes your life easier to merge them directly from outreachy
that's great and I'm ok with that, but don't skip the normal review
process as a favour to us.

regards,
dan carpenter

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Dan Carpenter]

On Tue, May 26, 2015 at 10:06:17AM -0700, Joe Perches wrote:
> If you're collecting them, I suggest you stick them in
> a separate branch, post them to your driverdev list and
> cc the appropriate maintainers, wait a week, then apply
> them to your main branch.

That would work.  A massive thread is easy to delete.  Or even just
apply them first, and we tell people to send follow on patches or revert
as needed.  My whole setup revolves around email so finding and
reviewing patches using git log is awkward.

regards,
dan carpenter

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Greg KH]

On Tue, May 26, 2015 at 10:06:17AM -0700, Joe Perches wrote:
> On Tue, 2015-05-26 at 09:35 -0700, Greg KH wrote:
> > On Tue, May 26, 2015 at 07:48:59AM -0700, Joe Perches wrote:
> > > > > The main point is that patches shouldn't be applied without
> > > > > being submitted to a more widely read list.
> > > > 
> > > > I take the blame for any problems with Outreachy patches.
> > > 
> > > Are you going to change any procedure associated to these
> > > Outreachy patches?
> > 
> > 2 bugs out of 900?  Nah, I think that's good odds.
> > 
> > Also, the outreachy patch process would overwhelm everyone else on the
> > list, it's really high volume during the application phase, I'd prefer
> > it to stick with the mentors that wish to help out with the process.  If
> > you and/or Dan, or anyone else wishes to help out with this, I would
> > really appreciate it.  But I don't think that forcing them to post to
> > the driverdevel list is a good idea.
> 
> I don't think that's necessary, but sending them to any
> listed maintainer should be.
> 
> If you're collecting them, I suggest you stick them in
> a separate branch, post them to your driverdev list and
> cc the appropriate maintainers, wait a week, then apply
> them to your main branch.
> 
> You already batch post hundreds of patches for kernel
> x.y.z stable branches.  What's another few hundred?
> 

Stable patches is a totally different workflow from my "normal" kernel
patch acceptance work.  I'll consider changing something for the future
outreachy application process.

thanks,

greg k-h

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Greg KH]

On Mon, May 25, 2015 at 11:02:27AM -0500, Larry Finger wrote:
> On 05/23/2015 04:16 PM, Larry Finger wrote:
> >The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
> >del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
> >del_timer() instead.
> >
> >Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> >Cc: Stable <stable@vger.kernel.org>
> >Cc: Haggi Eran <haggai.eran@gmail.com>
> >---
> 
> Greg,
> 
> Please drop this patch. The same fixes were submitted as
> https://lkml.org/lkml/2015/5/15/226.

That's not working for me at the moment, what was the subject: name?  I
think I already applied it to the testing tree...

thanks,

greg k-h

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Greg KH]

On Sun, May 31, 2015 at 11:53:47AM +0900, Greg KH wrote:
> On Mon, May 25, 2015 at 11:02:27AM -0500, Larry Finger wrote:
> > On 05/23/2015 04:16 PM, Larry Finger wrote:
> > >The driver is reporting a warning at kernel/time/timer.c:1096 due to calling
> > >del_timer_sync() while in interrupt mode. Such warnings are fixed by calling
> > >del_timer() instead.
> > >
> > >Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> > >Cc: Stable <stable@vger.kernel.org>
> > >Cc: Haggi Eran <haggai.eran@gmail.com>
> > >---
> > 
> > Greg,
> > 
> > Please drop this patch. The same fixes were submitted as
> > https://lkml.org/lkml/2015/5/15/226.
> 
> That's not working for me at the moment, what was the subject: name?  I
> think I already applied it to the testing tree...

Nevermind, found it...

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Larry Finger]

On 05/24/2015 11:42 PM, Sudip Mukherjee wrote:
>> I haven't been using kernel v4.1 so I haven't seen this warning, but looking
>> at the code it seems to originate from the two recent patches to remove
>> _cancel_timer and _cancel_timer_ex. I see that there's another patch in lkml [1]
>> that changes del_timer_sync back to del_timer in more places. Perhaps it
>> could prevent other warnings like this in the future.
>
> _cancel_timer and _cancel_timer_ex both were internally using del_timer,
> and the issue was reported in bugzilla. I have given the reference of the
> bugzilla in my patch in lkml.
> I have changed the reference of del_timer_sync to del_timer in all places
> which were in interrupt context, in some places it was not removed as
> those were not in interrupt context.

Why did you not Cc the maintainers?? That is why file MAINTAINERS exists.

Was your patch sent to Greg?

Larry

Re: [PATCH] staging: r8712u: Fix kernel warning for improper call of del_timer_sync()

[Sudip Mukherjee]

>I haven't been using kernel v4.1 so I haven't seen this warning, but looking
>at the code it seems to originate from the two recent patches to remove
>_cancel_timer and _cancel_timer_ex. I see that there's another patch in lkml [1]
>that changes del_timer_sync back to del_timer in more places. Perhaps it
>could prevent other warnings like this in the future.

_cancel_timer and _cancel_timer_ex both were internally using del_timer,
and the issue was reported in bugzilla. I have given the reference of the
bugzilla in my patch in lkml.
I have changed the reference of del_timer_sync to del_timer in all places
which were in interrupt context, in some places it was not removed as
those were not in interrupt context.

regards
sudip