All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] skd: fix assert typo
@ 2016-08-30 10:26 Eric Engestrom
  2016-09-12 17:43 ` Jeff Moyer
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Engestrom @ 2016-08-30 10:26 UTC (permalink / raw)
  To: linux-kernel
  Cc: Eric Engestrom, Jens Axboe, Ming Lin, Johannes Thumshirn,
	Jeff Moyer, Hannes Reinecke, Mike Christie, Dan Williams

Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
 drivers/block/skd_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 3822eae..94a7425 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
 		break;
 
 	default:
-		SKD_ASSERT("Don't know what to send");
+		SKD_ASSERT(!"Don't know what to send");
 		return;
 
 	}
@@ -2105,7 +2105,7 @@ static void skd_complete_internal(struct skd_device *skdev,
 		break;
 
 	default:
-		SKD_ASSERT("we didn't send this");
+		SKD_ASSERT(!"we didn't send this");
 	}
 }
 
-- 
Cheers,
  Eric

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] skd: fix assert typo
  2016-08-30 10:26 [PATCH] skd: fix assert typo Eric Engestrom
@ 2016-09-12 17:43 ` Jeff Moyer
  2016-09-16  9:31   ` [PATCH v2] " Eric Engestrom
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Moyer @ 2016-09-12 17:43 UTC (permalink / raw)
  To: Eric Engestrom
  Cc: linux-kernel, Jens Axboe, Ming Lin, Johannes Thumshirn,
	Hannes Reinecke, Mike Christie, Dan Williams

Eric Engestrom <eric.engestrom@imgtec.com> writes:

> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---
>  drivers/block/skd_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
> index 3822eae..94a7425 100644
> --- a/drivers/block/skd_main.c
> +++ b/drivers/block/skd_main.c
> @@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
>  		break;
>  
>  	default:
> -		SKD_ASSERT("Don't know what to send");
> +		SKD_ASSERT(!"Don't know what to send");

That's perverse.  Just change SKD_ASSERT to pr_err and Robert's your
mother's brother.

Cheers,
Jeff

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] skd: fix assert typo
  2016-09-12 17:43 ` Jeff Moyer
@ 2016-09-16  9:31   ` Eric Engestrom
  2016-09-16 11:59     ` Johannes Thumshirn
  2016-09-16 12:40     ` Jeff Moyer
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Engestrom @ 2016-09-16  9:31 UTC (permalink / raw)
  To: linux-kernel
  Cc: Eric Engestrom, Jeff Moyer, Jens Axboe, Johannes Thumshirn,
	Dan Williams, Ming Lin, Mike Christie

The assert was missing a `!` to become active, but since that would only turn
it into a complicated codepath for a pr_err(), let's simply replace it.

CC: Jeff Moyer <jmoyer@redhat.com>
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
 drivers/block/skd_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
index 3822eae..c04e92a 100644
--- a/drivers/block/skd_main.c
+++ b/drivers/block/skd_main.c
@@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
 		break;
 
 	default:
-		SKD_ASSERT("Don't know what to send");
+		pr_err("Don't know what to send");
 		return;
 
 	}
@@ -2105,7 +2105,7 @@ static void skd_complete_internal(struct skd_device *skdev,
 		break;
 
 	default:
-		SKD_ASSERT("we didn't send this");
+		pr_err("we didn't send this");
 	}
 }
 
-- 
Cheers,
  Eric

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] skd: fix assert typo
  2016-09-16  9:31   ` [PATCH v2] " Eric Engestrom
@ 2016-09-16 11:59     ` Johannes Thumshirn
  2016-09-16 12:40     ` Jeff Moyer
  1 sibling, 0 replies; 6+ messages in thread
From: Johannes Thumshirn @ 2016-09-16 11:59 UTC (permalink / raw)
  To: Eric Engestrom
  Cc: linux-kernel, Jeff Moyer, Jens Axboe, Dan Williams, Ming Lin,
	Mike Christie

On Fri, Sep 16, 2016 at 10:31:23AM +0100, Eric Engestrom wrote:
> The assert was missing a `!` to become active, but since that would only turn
> it into a complicated codepath for a pr_err(), let's simply replace it.
> 
> CC: Jeff Moyer <jmoyer@redhat.com>
> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---

Looks ok,
Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] skd: fix assert typo
  2016-09-16  9:31   ` [PATCH v2] " Eric Engestrom
  2016-09-16 11:59     ` Johannes Thumshirn
@ 2016-09-16 12:40     ` Jeff Moyer
  2016-09-16 13:06       ` Eric Engestrom
  1 sibling, 1 reply; 6+ messages in thread
From: Jeff Moyer @ 2016-09-16 12:40 UTC (permalink / raw)
  To: Eric Engestrom
  Cc: linux-kernel, Jens Axboe, Johannes Thumshirn, Dan Williams,
	Ming Lin, Mike Christie

Eric Engestrom <eric.engestrom@imgtec.com> writes:

> The assert was missing a `!` to become active, but since that would only turn
> it into a complicated codepath for a pr_err(), let's simply replace it.

The skd assert macro prints out file and line number, which would have
been nice to keep.  Sorry I didn't explicitly mention that last time.

-Jeff

>
> CC: Jeff Moyer <jmoyer@redhat.com>
> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---
>  drivers/block/skd_main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
> index 3822eae..c04e92a 100644
> --- a/drivers/block/skd_main.c
> +++ b/drivers/block/skd_main.c
> @@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
>  		break;
>  
>  	default:
> -		SKD_ASSERT("Don't know what to send");
> +		pr_err("Don't know what to send");
>  		return;
>  
>  	}
> @@ -2105,7 +2105,7 @@ static void skd_complete_internal(struct skd_device *skdev,
>  		break;
>  
>  	default:
> -		SKD_ASSERT("we didn't send this");
> +		pr_err("we didn't send this");
>  	}
>  }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] skd: fix assert typo
  2016-09-16 12:40     ` Jeff Moyer
@ 2016-09-16 13:06       ` Eric Engestrom
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Engestrom @ 2016-09-16 13:06 UTC (permalink / raw)
  To: Jeff Moyer
  Cc: linux-kernel, Jens Axboe, Johannes Thumshirn, Dan Williams,
	Ming Lin, Mike Christie

On Fri, Sep 16, 2016 at 08:40:54AM -0400, Jeff Moyer wrote:
> Eric Engestrom <eric.engestrom@imgtec.com> writes:
> 
> > The assert was missing a `!` to become active, but since that would only turn
> > it into a complicated codepath for a pr_err(), let's simply replace it.
> 
> The skd assert macro prints out file and line number, which would have
> been nice to keep.  Sorry I didn't explicitly mention that last time.

I'm not a fan of duplicating code, so if you want to keep the SKD_ASSERT()
format, my suggestion would be to go with the v1 of this patch. It might
end up having an `if` that will always be true, but the compiler will most
likely take care of optimising that away :)

Cheers,
  Eric

> 
> -Jeff
> 
> >
> > CC: Jeff Moyer <jmoyer@redhat.com>
> > Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> > ---
> >  drivers/block/skd_main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/block/skd_main.c b/drivers/block/skd_main.c
> > index 3822eae..c04e92a 100644
> > --- a/drivers/block/skd_main.c
> > +++ b/drivers/block/skd_main.c
> > @@ -1905,7 +1905,7 @@ static void skd_send_internal_skspcl(struct skd_device *skdev,
> >  		break;
> >  
> >  	default:
> > -		SKD_ASSERT("Don't know what to send");
> > +		pr_err("Don't know what to send");
> >  		return;
> >  
> >  	}
> > @@ -2105,7 +2105,7 @@ static void skd_complete_internal(struct skd_device *skdev,
> >  		break;
> >  
> >  	default:
> > -		SKD_ASSERT("we didn't send this");
> > +		pr_err("we didn't send this");
> >  	}
> >  }

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-09-16 13:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-30 10:26 [PATCH] skd: fix assert typo Eric Engestrom
2016-09-12 17:43 ` Jeff Moyer
2016-09-16  9:31   ` [PATCH v2] " Eric Engestrom
2016-09-16 11:59     ` Johannes Thumshirn
2016-09-16 12:40     ` Jeff Moyer
2016-09-16 13:06       ` Eric Engestrom

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.