[PATCH] usbnet: silence an unnecessary warning

[PATCH] usbnet: silence an unnecessary warning

[Oliver Neukum]

That a kevent could not be scheduled is not an error.
Such handlers must be able to deal with multiple events anyway.
As the successful scheduling of a work is a debug event, make
the failure debug priority, too.

Signed-off-by: Oliver Neukum <oneukum@suse.com>
Reported-by: Cristian Caravena <caravena@gmail.com>
---
 drivers/net/usb/usbnet.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index d56fe32bf48d..1e0bbe23f95c 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -458,8 +458,7 @@ void usbnet_defer_kevent (struct usbnet *dev, int work)
 {
 	set_bit (work, &dev->flags);
 	if (!schedule_work (&dev->kevent)) {
-		if (net_ratelimit())
-			netdev_err(dev->net, "kevent %d may have been dropped\n", work);
+		netdev_dbg(dev->net, "kevent %d may have been dropped\n", work);
 	} else {
 		netdev_dbg(dev->net, "kevent %d scheduled\n", work);
 	}
-- 
2.13.6

Re: [PATCH] usbnet: silence an unnecessary warning

[Bjørn Mork]

Oliver Neukum <oneukum@suse.com> writes:

> That a kevent could not be scheduled is not an error.
> Such handlers must be able to deal with multiple events anyway.
> As the successful scheduling of a work is a debug event, make
> the failure debug priority, too.
>
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: Cristian Caravena <caravena@gmail.com>
> ---
>  drivers/net/usb/usbnet.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index d56fe32bf48d..1e0bbe23f95c 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -458,8 +458,7 @@ void usbnet_defer_kevent (struct usbnet *dev, int work)
>  {
>  	set_bit (work, &dev->flags);
>  	if (!schedule_work (&dev->kevent)) {
> -		if (net_ratelimit())
> -			netdev_err(dev->net, "kevent %d may have been dropped\n", work);
> +		netdev_dbg(dev->net, "kevent %d may have been dropped\n", work);
>  	} else {
>  		netdev_dbg(dev->net, "kevent %d scheduled\n", work);
>  	}

Great!  But why do you drop the ratelimit?  This can be very noisy when
it hits.  I'd like to keep it ratelimited.

But if you do decide to drop the limit, then you'll have to clean up the
braces...



Bjørn

Re: [PATCH] usbnet: silence an unnecessary warning

[Oliver Neukum]

Am Donnerstag, den 11.01.2018, 16:23 +0100 schrieb Bjørn Mork :
> Oliver Neukum <oneukum@suse.com> writes:
> 
> > 
> > That a kevent could not be scheduled is not an error.
> > Such handlers must be able to deal with multiple events anyway.
> > As the successful scheduling of a work is a debug event, make
> > the failure debug priority, too.
> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > Reported-by: Cristian Caravena <caravena@gmail.com>
> > ---
> >  drivers/net/usb/usbnet.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> > index d56fe32bf48d..1e0bbe23f95c 100644
> > --- a/drivers/net/usb/usbnet.c
> > +++ b/drivers/net/usb/usbnet.c
> > @@ -458,8 +458,7 @@ void usbnet_defer_kevent (struct usbnet *dev, int work)
> >  {
> >  	set_bit (work, &dev->flags);
> >  	if (!schedule_work (&dev->kevent)) {
> > -		if (net_ratelimit())
> > -			netdev_err(dev->net, "kevent %d may have been dropped\n", work);
> > +		netdev_dbg(dev->net, "kevent %d may have been dropped\n", work);
> >  	} else {
> >  		netdev_dbg(dev->net, "kevent %d scheduled\n", work);
> >  	}
> 
> Great!  But why do you drop the ratelimit?  This can be very noisy when
> it hits.  I'd like to keep it ratelimited.

Because this is now a debug output and if you need to debug you may need
to verify whether your kevent will be handled. So not getting either of the
messages would indicate a bug. Thus limiting the rate would defeat the purpose.

	Regards
		Oliver

Re: [PATCH] usbnet: silence an unnecessary warning

[David Miller]

From: Oliver Neukum <oneukum@suse.com>
Date: Thu, 11 Jan 2018 16:01:49 +0100

> That a kevent could not be scheduled is not an error.
> Such handlers must be able to deal with multiple events anyway.
> As the successful scheduling of a work is a debug event, make
> the failure debug priority, too.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>
> Reported-by: Cristian Caravena <caravena@gmail.com>
> ---
>  drivers/net/usb/usbnet.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index d56fe32bf48d..1e0bbe23f95c 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -458,8 +458,7 @@ void usbnet_defer_kevent (struct usbnet *dev, int work)
>  {
>  	set_bit (work, &dev->flags);
>  	if (!schedule_work (&dev->kevent)) {
> -		if (net_ratelimit())
> -			netdev_err(dev->net, "kevent %d may have been dropped\n", work);
> +		netdev_dbg(dev->net, "kevent %d may have been dropped\n", work);
>  	} else {
>  		netdev_dbg(dev->net, "kevent %d scheduled\n", work);
>  	}

As Bjørn stated, you have to clean up the braces since both arms of the
conditional are now a single line and therefore should not get curly
braces.

Thank you.