[PATCH] tools, power: correct precedence of operators in init_maxfreq_mode

[PATCH] tools, power: correct precedence of operators in init_maxfreq_mode

[Sasha Levin]

With the current code, the condition in the if() doesn't make much sense due to
precedence of operators.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/power/cpupower/utils/idle_monitor/mperf_monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
index 5650ab5..90a8c4f 100644
--- a/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
+++ b/tools/power/cpupower/utils/idle_monitor/mperf_monitor.c
@@ -237,7 +237,7 @@ static int init_maxfreq_mode(void)
 	unsigned long long hwcr;
 	unsigned long min;
 
-	if (!cpupower_cpu_info.caps & CPUPOWER_CAP_INV_TSC)
+	if (!(cpupower_cpu_info.caps & CPUPOWER_CAP_INV_TSC))
 		goto use_sysfs;
 
 	if (cpupower_cpu_info.vendor == X86_VENDOR_AMD) {
-- 
1.8.0

[PATCH] kvm tools: remove unneeded checks in qcow code

[Sasha Levin]

We already know q!=NULL at that point, no need to retest.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/kvm/disk/qcow.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index ee2992e..64a2550 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -1361,8 +1361,7 @@ free_header:
 	if (q->header)
 		free(q->header);
 free_qcow:
-	if (q)
-		free(q);
+	free(q);
 
 	return NULL;
 }
@@ -1492,8 +1491,7 @@ free_header:
 	if (q->header)
 		free(q->header);
 free_qcow:
-	if (q)
-		free(q);
+	free(q);
 
 	return NULL;
 }
-- 
1.8.0

Re: [PATCH] kvm tools: remove unneeded checks in qcow code

[Jesper Juhl]

On Thu, 20 Dec 2012, Sasha Levin wrote:

> We already know q!=NULL at that point, no need to retest.
> 
Right, and even if it was ==NULL then 'free(NULL)' is guaranteed to be a 
NOP, so it would still be fine.

Reviewed-by: Jesper Juhl <jj@chaosbits.net>


> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  tools/kvm/disk/qcow.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
> index ee2992e..64a2550 100644
> --- a/tools/kvm/disk/qcow.c
> +++ b/tools/kvm/disk/qcow.c
> @@ -1361,8 +1361,7 @@ free_header:
>  	if (q->header)
>  		free(q->header);
>  free_qcow:
> -	if (q)
> -		free(q);
> +	free(q);
>  
>  	return NULL;
>  }
> @@ -1492,8 +1491,7 @@ free_header:
>  	if (q->header)
>  		free(q->header);
>  free_qcow:
> -	if (q)
> -		free(q);
> +	free(q);
>  
>  	return NULL;
>  }
> 

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

[PATCH] kvm tools: remove unneeded check from disk code

[Sasha Levin]

We already know 'disk' is non-null.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/kvm/disk/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index dd59751..4e9bda0 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -105,7 +105,7 @@ struct disk_image *disk_image__new(int fd, u64 size,
 	}
 
 #ifdef CONFIG_HAS_AIO
-	if (disk) {
+	{
 		pthread_t thread;
 
 		disk->evt = eventfd(0, 0);
-- 
1.8.0

[PATCH] kvm tools: correctly compare pointer with NULL instead of 0

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/kvm/hw/pci-shmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kvm/hw/pci-shmem.c b/tools/kvm/hw/pci-shmem.c
index 00e5d93..ec3f771 100644
--- a/tools/kvm/hw/pci-shmem.c
+++ b/tools/kvm/hw/pci-shmem.c
@@ -356,7 +356,7 @@ int pci_shmem__init(struct kvm *kvm)
 	char *mem;
 	int r;
 
-	if (shmem_region == 0)
+	if (shmem_region == NULL)
 		return 0;
 
 	/* Register good old INTx */
-- 
1.8.0

[PATCH] tools lib traceevent: remove redundant check from process_dynamic_array

[Sasha Levin]

We check twice that 'field' is non-NULL, no need to do that.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/lib/traceevent/event-parse.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 5a824e3..f7dc053 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2481,11 +2481,6 @@ process_dynamic_array(struct event_format *event, struct print_arg *arg, char **
 
 	free_token(token);
 	arg = alloc_arg();
-	if (!field) {
-		do_warning("%s: not enough memory!", __func__);
-		*tok = NULL;
-		return EVENT_ERROR;
-	}
 
 	type = process_arg(event, arg, &token);
 	if (type == EVENT_ERROR)
-- 
1.8.0

Re: [PATCH] tools lib traceevent: remove redundant check from process_dynamic_array

[Namhyung Kim]

On Thu, Dec 20, 2012 at 02:11:13PM -0500, Sasha Levin wrote:
> We check twice that 'field' is non-NULL, no need to do that.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  tools/lib/traceevent/event-parse.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
> index 5a824e3..f7dc053 100644
> --- a/tools/lib/traceevent/event-parse.c
> +++ b/tools/lib/traceevent/event-parse.c
> @@ -2481,11 +2481,6 @@ process_dynamic_array(struct event_format *event, struct print_arg *arg, char **
>  
>  	free_token(token);
>  	arg = alloc_arg();
> -	if (!field) {

I think it should be 'if (!arg)' instead.

Thanks,
Namhyung


> -		do_warning("%s: not enough memory!", __func__);
> -		*tok = NULL;
> -		return EVENT_ERROR;
> -	}
>  
>  	type = process_arg(event, arg, &token);
>  	if (type == EVENT_ERROR)
> -- 
> 1.8.0

[PATCH] epoll: stop comparing pointers with 0 in self-test app

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/testing/selftests/epoll/test_epoll.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/epoll/test_epoll.c b/tools/testing/selftests/epoll/test_epoll.c
index 4a14a7f..1034ed4 100644
--- a/tools/testing/selftests/epoll/test_epoll.c
+++ b/tools/testing/selftests/epoll/test_epoll.c
@@ -199,8 +199,8 @@ int main(int argc, char **argv)
 
 	epoll_items = malloc(n_epoll_items * sizeof(struct epoll_item_private));
 
-	if (epoll_set < 0 || epoll_items == 0 || write_thread_data.fds == 0 ||
-		read_thread_data == 0 || read_threads == 0)
+	if (epoll_set < 0 || !epoll_items || write_thread_data.fds == NULL ||
+		!read_thread_data || !read_threads)
 		goto error;
 
 	if (sysconf(_SC_NPROCESSORS_ONLN) < 2) {
-- 
1.8.0

Re: [PATCH] epoll: stop comparing pointers with 0 in self-test app

[Daniel Hazelton]

I don't see anything obviously wrong here...
Reviewed-By: Daniel Hazelton <dshadowwolf@gmail.com>

On 12/20/2012 02:11 PM, Sasha Levin wrote:
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>   tools/testing/selftests/epoll/test_epoll.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/epoll/test_epoll.c b/tools/testing/selftests/epoll/test_epoll.c
> index 4a14a7f..1034ed4 100644
> --- a/tools/testing/selftests/epoll/test_epoll.c
> +++ b/tools/testing/selftests/epoll/test_epoll.c
> @@ -199,8 +199,8 @@ int main(int argc, char **argv)
>
>   	epoll_items = malloc(n_epoll_items * sizeof(struct epoll_item_private));
>
> -	if (epoll_set < 0 || epoll_items == 0 || write_thread_data.fds == 0 ||
> -		read_thread_data == 0 || read_threads == 0)
> +	if (epoll_set < 0 || !epoll_items || write_thread_data.fds == NULL ||
> +		!read_thread_data || !read_threads)
>   		goto error;
>
>   	if (sysconf(_SC_NPROCESSORS_ONLN) < 2) {
>

[PATCH] tools/usb: remove unneeded 'continue' and simplify condition

[Sasha Levin]

Basically remove unneeded code. Since that 'continue' is at the end
of the for() there's no need for it.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/usb/testusb.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/usb/testusb.c b/tools/usb/testusb.c
index 68d0734..643cd77 100644
--- a/tools/usb/testusb.c
+++ b/tools/usb/testusb.c
@@ -507,10 +507,8 @@ usage:
 			return handle_testdev (entry) != entry;
 		}
 		status = pthread_create (&entry->thread, 0, handle_testdev, entry);
-		if (status) {
+		if (status)
 			perror ("pthread_create");
-			continue;
-		}
 	}
 	if (device) {
 		struct testdev		dev;
-- 
1.8.0

Re: [PATCH] tools/usb: remove unneeded 'continue' and simplify condition

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

On Thu, Dec 20, 2012 at 02:11:15PM -0500, Sasha Levin wrote:
> Basically remove unneeded code. Since that 'continue' is at the end
> of the for() there's no need for it.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Acked-by: Felipe Balbi <balbi@ti.com>

> ---
>  tools/usb/testusb.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/tools/usb/testusb.c b/tools/usb/testusb.c
> index 68d0734..643cd77 100644
> --- a/tools/usb/testusb.c
> +++ b/tools/usb/testusb.c
> @@ -507,10 +507,8 @@ usage:
>  			return handle_testdev (entry) != entry;
>  		}
>  		status = pthread_create (&entry->thread, 0, handle_testdev, entry);
> -		if (status) {
> +		if (status)
>  			perror ("pthread_create");
> -			continue;
> -		}
>  	}
>  	if (device) {
>  		struct testdev		dev;
> -- 
> 1.8.0
> 

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] perf kmem: use ARRAY_SIZE instead of reinventing it

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/perf/builtin-kmem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 0b4b796..266031f 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -614,8 +614,7 @@ static struct sort_dimension *avail_sorts[] = {
 	&pingpong_sort_dimension,
 };
 
-#define NUM_AVAIL_SORTS	\
-	(int)(sizeof(avail_sorts) / sizeof(struct sort_dimension *))
+#define NUM_AVAIL_SORTS	((int)ARRAY_SIZE(avail_sorts))
 
 static int sort_dimension__add(const char *tok, struct list_head *list)
 {
-- 
1.8.0

[tip:perf/core] perf kmem: use ARRAY_SIZE instead of reinventing it

[tip-bot for Sasha Levin]

Commit-ID:  49e4ba54592435cbd150aea8f5ae6b776fc687f9
Gitweb:     http://git.kernel.org/tip/49e4ba54592435cbd150aea8f5ae6b776fc687f9
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:16 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:15 -0300

perf kmem: use ARRAY_SIZE instead of reinventing it

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1356030701-16284-8-git-send-email-sasha.levin@oracle.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-kmem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 9fddca7..c746108 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -609,8 +609,7 @@ static struct sort_dimension *avail_sorts[] = {
 	&pingpong_sort_dimension,
 };
 
-#define NUM_AVAIL_SORTS	\
-	(int)(sizeof(avail_sorts) / sizeof(struct sort_dimension *))
+#define NUM_AVAIL_SORTS	((int)ARRAY_SIZE(avail_sorts))
 
 static int sort_dimension__add(const char *tok, struct list_head *list)
 {

[PATCH] cpupowerutils: don't compare pointers with 0

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/power/cpupower/utils/helpers/bitmask.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/power/cpupower/utils/helpers/bitmask.c b/tools/power/cpupower/utils/helpers/bitmask.c
index 5c074c6..3b6e03d 100644
--- a/tools/power/cpupower/utils/helpers/bitmask.c
+++ b/tools/power/cpupower/utils/helpers/bitmask.c
@@ -25,11 +25,11 @@ struct bitmask *bitmask_alloc(unsigned int n)
 	struct bitmask *bmp;
 
 	bmp = malloc(sizeof(*bmp));
-	if (bmp == 0)
+	if (bmp == NULL)
 		return 0;
 	bmp->size = n;
 	bmp->maskp = calloc(longsperbits(n), sizeof(unsigned long));
-	if (bmp->maskp == 0) {
+	if (bmp->maskp == NULL) {
 		free(bmp);
 		return 0;
 	}
@@ -39,7 +39,7 @@ struct bitmask *bitmask_alloc(unsigned int n)
 /* Free `struct bitmask` */
 void bitmask_free(struct bitmask *bmp)
 {
-	if (bmp == 0)
+	if (bmp == NULL)
 		return;
 	free(bmp->maskp);
 	bmp->maskp = (unsigned long *)0xdeadcdef;  /* double free tripwire */
-- 
1.8.0

Re: [PATCH] cpupowerutils: don't compare pointers with 0

[Thomas Renninger]

On Thursday, December 20, 2012 02:11:17 PM Sasha Levin wrote:
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Both look fine.
I'll catch them in the new year.

Thanks,

    Thomas

[PATCH] m2m-deinterlace: use correct check for kzalloc failure

[Sasha Levin]

There is no point in PTR_ERR()ing a NULL pointer, use a real error
instead.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/media/platform/m2m-deinterlace.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/platform/m2m-deinterlace.c b/drivers/media/platform/m2m-deinterlace.c
index 05c560f..64db74e 100644
--- a/drivers/media/platform/m2m-deinterlace.c
+++ b/drivers/media/platform/m2m-deinterlace.c
@@ -917,10 +917,8 @@ static int deinterlace_open(struct file *file)
 	ctx->xt = kzalloc(sizeof(struct dma_async_tx_descriptor) +
 				sizeof(struct data_chunk), GFP_KERNEL);
 	if (!ctx->xt) {
-		int ret = PTR_ERR(ctx->xt);
-
 		kfree(ctx);
-		return ret;
+		return -ENOMEM;
 	}
 
 	ctx->colorspace = V4L2_COLORSPACE_REC709;
-- 
1.8.0

[PATCH] perf script: use ARRAY_SIZE instead of reinventing it

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/perf/builtin-script.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b363e7b..3314ef2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -692,7 +692,7 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 			    const char *arg, int unset __maybe_unused)
 {
 	char *tok;
-	int i, imax = sizeof(all_output_options) / sizeof(struct output_option);
+	int i, imax = ARRAY_SIZE(all_output_options);
 	int j;
 	int rc = 0;
 	char *str = strdup(arg);
-- 
1.8.0

[tip:perf/core] perf script: use ARRAY_SIZE instead of reinventing it

[tip-bot for Sasha Levin]

Commit-ID:  50ca19aed8ad2e264ef6bce6d6d5cb7f8bfae8c9
Gitweb:     http://git.kernel.org/tip/50ca19aed8ad2e264ef6bce6d6d5cb7f8bfae8c9
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:19 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:15 -0300

perf script: use ARRAY_SIZE instead of reinventing it

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1356030701-16284-11-git-send-email-sasha.levin@oracle.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b363e7b..3314ef2 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -692,7 +692,7 @@ static int parse_output_fields(const struct option *opt __maybe_unused,
 			    const char *arg, int unset __maybe_unused)
 {
 	char *tok;
-	int i, imax = sizeof(all_output_options) / sizeof(struct output_option);
+	int i, imax = ARRAY_SIZE(all_output_options);
 	int j;
 	int rc = 0;
 	char *str = strdup(arg);

[PATCH] perf tools: remove redundant checks from _sort__sym_cmp

[Sasha Levin]

We already check that sym_l and sum_r are non-NULLs, no need to
do it twice.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 tools/perf/util/sort.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index cfd1c0f..dbaded9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -143,10 +143,8 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 	if (sym_l == sym_r)
 		return 0;
 
-	if (sym_l)
-		ip_l = sym_l->start;
-	if (sym_r)
-		ip_r = sym_r->start;
+	ip_l = sym_l->start;
+	ip_r = sym_r->start;
 
 	return (int64_t)(ip_r - ip_l);
 }
-- 
1.8.0

Re: [PATCH] perf tools: remove redundant checks from _sort__sym_cmp

[Namhyung Kim]

On Thu, Dec 20, 2012 at 02:11:20PM -0500, Sasha Levin wrote:
> We already check that sym_l and sum_r are non-NULLs, no need to
> do it twice.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

[tip:perf/core] perf tools: remove redundant checks from _sort__sym_cmp

[tip-bot for Sasha Levin]

Commit-ID:  53985a7bfa7b947c91f35c6adcdc9cacc6fef07f
Gitweb:     http://git.kernel.org/tip/53985a7bfa7b947c91f35c6adcdc9cacc6fef07f
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:20 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:14 -0300

perf tools: remove redundant checks from _sort__sym_cmp

We already check that sym_l and sum_r are non-NULLs, no need to do it
twice.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1356030701-16284-12-git-send-email-sasha.levin@oracle.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index cfd1c0f..dbaded9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -143,10 +143,8 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 	if (sym_l == sym_r)
 		return 0;
 
-	if (sym_l)
-		ip_l = sym_l->start;
-	if (sym_r)
-		ip_r = sym_r->start;
+	ip_l = sym_l->start;
+	ip_r = sym_r->start;
 
 	return (int64_t)(ip_r - ip_l);
 }

[PATCH] virtio_console: correct error message on failure of debugfs_create_dir

[Sasha Levin]

debugfs_create_dir() returns NULL if it fails, there's little point in
calling PTR_ERR on it.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/char/virtio_console.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index c594cb1..490b70e 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2212,10 +2212,9 @@ static int __init init(void)
 	}
 
 	pdrvdata.debugfs_dir = debugfs_create_dir("virtio-ports", NULL);
-	if (!pdrvdata.debugfs_dir) {
-		pr_warning("Error %ld creating debugfs dir for virtio-ports\n",
-			   PTR_ERR(pdrvdata.debugfs_dir));
-	}
+	if (!pdrvdata.debugfs_dir)
+		pr_warn("Error creating debugfs dir for virtio-ports\n");
+
 	INIT_LIST_HEAD(&pdrvdata.consoles);
 	INIT_LIST_HEAD(&pdrvdata.portdevs);
 
-- 
1.8.0

Re: [PATCH] virtio_console: correct error message on failure of debugfs_create_dir

[Amit Shah]

On (Thu) 20 Dec 2012 [14:11:21], Sasha Levin wrote:
> debugfs_create_dir() returns NULL if it fails, there's little point in
> calling PTR_ERR on it.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  drivers/char/virtio_console.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index c594cb1..490b70e 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -2212,10 +2212,9 @@ static int __init init(void)
>  	}
>  
>  	pdrvdata.debugfs_dir = debugfs_create_dir("virtio-ports", NULL);
> -	if (!pdrvdata.debugfs_dir) {
> -		pr_warning("Error %ld creating debugfs dir for virtio-ports\n",
> -			   PTR_ERR(pdrvdata.debugfs_dir));
> -	}
> +	if (!pdrvdata.debugfs_dir)
> +		pr_warn("Error creating debugfs dir for virtio-ports\n");

debugfs_create_dir() does return an error value if debugfs is not
enabled.

This check for !pdrvdata.debugfs_dir should infact use
IS_ERR_OR_NULL().  Care to submit a patch for that?

		Amit

Re: [PATCH] virtio_console: correct error message on failure of debugfs_create_dir

[Arnd Bergmann]

On Friday 21 December 2012, Amit Shah wrote:
> On (Thu) 20 Dec 2012 [14:11:21], Sasha Levin wrote:
> > debugfs_create_dir() returns NULL if it fails, there's little point in
> > calling PTR_ERR on it.
>
> debugfs_create_dir() does return an error value if debugfs is not
> enabled.
> 
> This check for !pdrvdata.debugfs_dir should infact use
> IS_ERR_OR_NULL().  Care to submit a patch for that?

How about we fix the stub instead to return NULL when debugfs is disabled?

	Arnd

Re: [PATCH] virtio_console: correct error message on failure of debugfs_create_dir

[Greg Kroah-Hartman]

On Mon, Dec 24, 2012 at 11:55:46AM +0000, Arnd Bergmann wrote:
> On Friday 21 December 2012, Amit Shah wrote:
> > On (Thu) 20 Dec 2012 [14:11:21], Sasha Levin wrote:
> > > debugfs_create_dir() returns NULL if it fails, there's little point in
> > > calling PTR_ERR on it.
> >
> > debugfs_create_dir() does return an error value if debugfs is not
> > enabled.
> > 
> > This check for !pdrvdata.debugfs_dir should infact use
> > IS_ERR_OR_NULL().  Care to submit a patch for that?
> 
> How about we fix the stub instead to return NULL when debugfs is disabled?

No, please read debugfs.h for why I decided to not do this (i.e. we try
to learn from our mistakes...)

thanks,

greg k-h

Re: [PATCH] virtio_console: correct error message on failure of debugfs_create_dir

[Sasha Levin]

On Mon, Dec 24, 2012 at 12:39 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Dec 24, 2012 at 11:55:46AM +0000, Arnd Bergmann wrote:
>> On Friday 21 December 2012, Amit Shah wrote:
>> > On (Thu) 20 Dec 2012 [14:11:21], Sasha Levin wrote:
>> > > debugfs_create_dir() returns NULL if it fails, there's little point in
>> > > calling PTR_ERR on it.
>> >
>> > debugfs_create_dir() does return an error value if debugfs is not
>> > enabled.
>> >
>> > This check for !pdrvdata.debugfs_dir should infact use
>> > IS_ERR_OR_NULL().  Care to submit a patch for that?
>>
>> How about we fix the stub instead to return NULL when debugfs is disabled?
>
> No, please read debugfs.h for why I decided to not do this (i.e. we try
> to learn from our mistakes...)

Why won't we fix it the other way around and return an actual error
code instead of a NULL on failure?


Thanks,
Sasha

Re: [PATCH] virtio_console: correct error message on failure of debugfs_create_dir

[Greg Kroah-Hartman]

On Mon, Dec 24, 2012 at 01:24:52PM -0500, Sasha Levin wrote:
> On Mon, Dec 24, 2012 at 12:39 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Mon, Dec 24, 2012 at 11:55:46AM +0000, Arnd Bergmann wrote:
> >> On Friday 21 December 2012, Amit Shah wrote:
> >> > On (Thu) 20 Dec 2012 [14:11:21], Sasha Levin wrote:
> >> > > debugfs_create_dir() returns NULL if it fails, there's little point in
> >> > > calling PTR_ERR on it.
> >> >
> >> > debugfs_create_dir() does return an error value if debugfs is not
> >> > enabled.
> >> >
> >> > This check for !pdrvdata.debugfs_dir should infact use
> >> > IS_ERR_OR_NULL().  Care to submit a patch for that?
> >>
> >> How about we fix the stub instead to return NULL when debugfs is disabled?
> >
> > No, please read debugfs.h for why I decided to not do this (i.e. we try
> > to learn from our mistakes...)
> 
> Why won't we fix it the other way around and return an actual error
> code instead of a NULL on failure?

Again, no, that doesn't make your code simpler/faster.  You really don't
need to check the return value of debugfs, just handle it as if it
always works, and look, the code handles it all properly, no matter if
debugfs is enabled or not.  It's only debugging code, you shouldn't
really care if it is present or not, right?

If you do, perhaps you shouldn't be using debugfs?

greg k-h

[PATCH] debugfs: remove redundant initialization of dentry

[Sasha Levin]

We already initialize it to NULL when declaring it, no need to do
that twice.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 fs/debugfs/inode.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 153bb1e..d329f9c 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -322,7 +322,6 @@ static struct dentry *__create_file(const char *name, umode_t mode,
 	if (!parent)
 		parent = debugfs_mount->mnt_root;
 
-	dentry = NULL;
 	mutex_lock(&parent->d_inode->i_mutex);
 	dentry = lookup_one_len(name, parent, strlen(name));
 	if (!IS_ERR(dentry)) {
-- 
1.8.0

[PATCH] PCI: cpqphp: minor cleanup and removal of unreachable paths

[Sasha Levin]

Remove cases of redundant checks and remote unreachable paths.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/pci/hotplug/cpqphp_ctrl.c | 57 ++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 37 deletions(-)

diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c
index 36112fe..d282019 100644
--- a/drivers/pci/hotplug/cpqphp_ctrl.c
+++ b/drivers/pci/hotplug/cpqphp_ctrl.c
@@ -1900,8 +1900,7 @@ static void interrupt_event_handler(struct controller *ctrl)
 					dbg("power fault\n");
 				} else {
 					/* refresh notification */
-					if (p_slot)
-						update_slot_info(ctrl, p_slot);
+					update_slot_info(ctrl, p_slot);
 				}
 
 				ctrl->event_queue[loop].event_type = 0;
@@ -2520,44 +2519,28 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *func
 
 		/* If we have IO resources copy them and fill in the bridge's
 		 * IO range registers */
-		if (io_node) {
-			memcpy(hold_IO_node, io_node, sizeof(struct pci_resource));
-			io_node->next = NULL;
+		memcpy(hold_IO_node, io_node, sizeof(struct pci_resource));
+		io_node->next = NULL;
 
-			/* set IO base and Limit registers */
-			temp_byte = io_node->base >> 8;
-			rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_BASE, temp_byte);
+		/* set IO base and Limit registers */
+		temp_byte = io_node->base >> 8;
+		rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_BASE, temp_byte);
 
-			temp_byte = (io_node->base + io_node->length - 1) >> 8;
-			rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_LIMIT, temp_byte);
-		} else {
-			kfree(hold_IO_node);
-			hold_IO_node = NULL;
-		}
-
-		/* If we have memory resources copy them and fill in the
-		 * bridge's memory range registers.  Otherwise, fill in the
-		 * range registers with values that disable them. */
-		if (mem_node) {
-			memcpy(hold_mem_node, mem_node, sizeof(struct pci_resource));
-			mem_node->next = NULL;
-
-			/* set Mem base and Limit registers */
-			temp_word = mem_node->base >> 16;
-			rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_BASE, temp_word);
+		temp_byte = (io_node->base + io_node->length - 1) >> 8;
+		rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_LIMIT, temp_byte);
 
-			temp_word = (mem_node->base + mem_node->length - 1) >> 16;
-			rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_LIMIT, temp_word);
-		} else {
-			temp_word = 0xFFFF;
-			rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_BASE, temp_word);
+		/* Copy the memory resources and fill in the bridge's memory
+		 * range registers.
+		 */
+		memcpy(hold_mem_node, mem_node, sizeof(struct pci_resource));
+		mem_node->next = NULL;
 
-			temp_word = 0x0000;
-			rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_LIMIT, temp_word);
+		/* set Mem base and Limit registers */
+		temp_word = mem_node->base >> 16;
+		rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_BASE, temp_word);
 
-			kfree(hold_mem_node);
-			hold_mem_node = NULL;
-		}
+		temp_word = (mem_node->base + mem_node->length - 1) >> 16;
+		rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_LIMIT, temp_word);
 
 		memcpy(hold_p_mem_node, p_mem_node, sizeof(struct pci_resource));
 		p_mem_node->next = NULL;
@@ -2627,7 +2610,7 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *func
 		/* Return unused bus resources
 		 * First use the temporary node to store information for
 		 * the board */
-		if (hold_bus_node && bus_node && temp_resources.bus_head) {
+		if (bus_node && temp_resources.bus_head) {
 			hold_bus_node->length = bus_node->base - hold_bus_node->base;
 
 			hold_bus_node->next = func->bus_head;
@@ -2751,7 +2734,7 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *func
 		}
 		/* If we have prefetchable memory space available and there
 		 * is some left at the end, return the unused portion */
-		if (hold_p_mem_node && temp_resources.p_mem_head) {
+		if (temp_resources.p_mem_head) {
 			p_mem_node = do_pre_bridge_resource_split(&(temp_resources.p_mem_head),
 								  &hold_p_mem_node, 0x100000);
 
-- 
1.8.0

Re: [PATCH] PCI: cpqphp: minor cleanup and removal of unreachable paths

[Bjorn Helgaas]

On Thu, Dec 20, 2012 at 12:11 PM, Sasha Levin <sasha.levin@oracle.com> wrote:
> Remove cases of redundant checks and remote unreachable paths.
>
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Thanks, Sasha.  I applied this for v3.9.

> ---
>  drivers/pci/hotplug/cpqphp_ctrl.c | 57 ++++++++++++++-------------------------
>  1 file changed, 20 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/pci/hotplug/cpqphp_ctrl.c b/drivers/pci/hotplug/cpqphp_ctrl.c
> index 36112fe..d282019 100644
> --- a/drivers/pci/hotplug/cpqphp_ctrl.c
> +++ b/drivers/pci/hotplug/cpqphp_ctrl.c
> @@ -1900,8 +1900,7 @@ static void interrupt_event_handler(struct controller *ctrl)
>                                         dbg("power fault\n");
>                                 } else {
>                                         /* refresh notification */
> -                                       if (p_slot)
> -                                               update_slot_info(ctrl, p_slot);
> +                                       update_slot_info(ctrl, p_slot);
>                                 }
>
>                                 ctrl->event_queue[loop].event_type = 0;
> @@ -2520,44 +2519,28 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *func
>
>                 /* If we have IO resources copy them and fill in the bridge's
>                  * IO range registers */
> -               if (io_node) {
> -                       memcpy(hold_IO_node, io_node, sizeof(struct pci_resource));
> -                       io_node->next = NULL;
> +               memcpy(hold_IO_node, io_node, sizeof(struct pci_resource));
> +               io_node->next = NULL;
>
> -                       /* set IO base and Limit registers */
> -                       temp_byte = io_node->base >> 8;
> -                       rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_BASE, temp_byte);
> +               /* set IO base and Limit registers */
> +               temp_byte = io_node->base >> 8;
> +               rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_BASE, temp_byte);
>
> -                       temp_byte = (io_node->base + io_node->length - 1) >> 8;
> -                       rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_LIMIT, temp_byte);
> -               } else {
> -                       kfree(hold_IO_node);
> -                       hold_IO_node = NULL;
> -               }
> -
> -               /* If we have memory resources copy them and fill in the
> -                * bridge's memory range registers.  Otherwise, fill in the
> -                * range registers with values that disable them. */
> -               if (mem_node) {
> -                       memcpy(hold_mem_node, mem_node, sizeof(struct pci_resource));
> -                       mem_node->next = NULL;
> -
> -                       /* set Mem base and Limit registers */
> -                       temp_word = mem_node->base >> 16;
> -                       rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_BASE, temp_word);
> +               temp_byte = (io_node->base + io_node->length - 1) >> 8;
> +               rc = pci_bus_write_config_byte(pci_bus, devfn, PCI_IO_LIMIT, temp_byte);
>
> -                       temp_word = (mem_node->base + mem_node->length - 1) >> 16;
> -                       rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_LIMIT, temp_word);
> -               } else {
> -                       temp_word = 0xFFFF;
> -                       rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_BASE, temp_word);
> +               /* Copy the memory resources and fill in the bridge's memory
> +                * range registers.
> +                */
> +               memcpy(hold_mem_node, mem_node, sizeof(struct pci_resource));
> +               mem_node->next = NULL;
>
> -                       temp_word = 0x0000;
> -                       rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_LIMIT, temp_word);
> +               /* set Mem base and Limit registers */
> +               temp_word = mem_node->base >> 16;
> +               rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_BASE, temp_word);
>
> -                       kfree(hold_mem_node);
> -                       hold_mem_node = NULL;
> -               }
> +               temp_word = (mem_node->base + mem_node->length - 1) >> 16;
> +               rc = pci_bus_write_config_word(pci_bus, devfn, PCI_MEMORY_LIMIT, temp_word);
>
>                 memcpy(hold_p_mem_node, p_mem_node, sizeof(struct pci_resource));
>                 p_mem_node->next = NULL;
> @@ -2627,7 +2610,7 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *func
>                 /* Return unused bus resources
>                  * First use the temporary node to store information for
>                  * the board */
> -               if (hold_bus_node && bus_node && temp_resources.bus_head) {
> +               if (bus_node && temp_resources.bus_head) {
>                         hold_bus_node->length = bus_node->base - hold_bus_node->base;
>
>                         hold_bus_node->next = func->bus_head;
> @@ -2751,7 +2734,7 @@ static int configure_new_function(struct controller *ctrl, struct pci_func *func
>                 }
>                 /* If we have prefetchable memory space available and there
>                  * is some left at the end, return the unused portion */
> -               if (hold_p_mem_node && temp_resources.p_mem_head) {
> +               if (temp_resources.p_mem_head) {
>                         p_mem_node = do_pre_bridge_resource_split(&(temp_resources.p_mem_head),
>                                                                   &hold_p_mem_node, 0x100000);
>
> --
> 1.8.0
>

[PATCH] bnx2x: use ARRAY_SIZE where possible

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
index 09096b4..cb41f54 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c
@@ -3659,7 +3659,7 @@ static void bnx2x_warpcore_enable_AN_KR2(struct bnx2x_phy *phy,
 	bnx2x_cl45_read_or_write(bp, phy, MDIO_WC_DEVAD,
 				 MDIO_WC_REG_CL49_USERB0_CTRL, (3<<6));
 
-	for (i = 0; i < sizeof(reg_set)/sizeof(struct bnx2x_reg_set); i++)
+	for (i = 0; i < ARRAY_SIZE(reg_set); i++)
 		bnx2x_cl45_write(bp, phy, reg_set[i].devad, reg_set[i].reg,
 				 reg_set[i].val);
 
@@ -3713,7 +3713,7 @@ static void bnx2x_warpcore_enable_AN_KR(struct bnx2x_phy *phy,
 	};
 	DP(NETIF_MSG_LINK, "Enable Auto Negotiation for KR\n");
 	/* Set to default registers that may be overriden by 10G force */
-	for (i = 0; i < sizeof(reg_set)/sizeof(struct bnx2x_reg_set); i++)
+	for (i = 0; i < ARRAY_SIZE(reg_set); i++)
 		bnx2x_cl45_write(bp, phy, reg_set[i].devad, reg_set[i].reg,
 				 reg_set[i].val);
 
@@ -3854,7 +3854,7 @@ static void bnx2x_warpcore_set_10G_KR(struct bnx2x_phy *phy,
 		{MDIO_PMA_DEVAD, MDIO_WC_REG_PMD_KR_CONTROL, 0x2}
 	};
 
-	for (i = 0; i < sizeof(reg_set)/sizeof(struct bnx2x_reg_set); i++)
+	for (i = 0; i < ARRAY_SIZE(reg_set); i++)
 		bnx2x_cl45_write(bp, phy, reg_set[i].devad, reg_set[i].reg,
 				 reg_set[i].val);
 
@@ -4242,7 +4242,7 @@ static void bnx2x_warpcore_clear_regs(struct bnx2x_phy *phy,
 	bnx2x_cl45_read_or_write(bp, phy, MDIO_WC_DEVAD,
 				 MDIO_WC_REG_RX66_CONTROL, (3<<13));
 
-	for (i = 0; i < sizeof(wc_regs)/sizeof(struct bnx2x_reg_set); i++)
+	for (i = 0; i < ARRAY_SIZE(wc_regs); i++)
 		bnx2x_cl45_write(bp, phy, wc_regs[i].devad, wc_regs[i].reg,
 				 wc_regs[i].val);
 
@@ -9520,7 +9520,7 @@ static void bnx2x_save_848xx_spirom_version(struct bnx2x_phy *phy,
 	} else {
 		/* For 32-bit registers in 848xx, access via MDIO2ARM i/f. */
 		/* (1) set reg 0xc200_0014(SPI_BRIDGE_CTRL_2) to 0x03000000 */
-		for (i = 0; i < sizeof(reg_set)/sizeof(struct bnx2x_reg_set);
+		for (i = 0; i < ARRAY_SIZE(reg_set);
 		      i++)
 			bnx2x_cl45_write(bp, phy, reg_set[i].devad,
 					 reg_set[i].reg, reg_set[i].val);
@@ -9592,7 +9592,7 @@ static void bnx2x_848xx_set_led(struct bnx2x *bp,
 			 MDIO_PMA_DEVAD,
 			 MDIO_PMA_REG_8481_LINK_SIGNAL, val);
 
-	for (i = 0; i < sizeof(reg_set)/sizeof(struct bnx2x_reg_set); i++)
+	for (i = 0; i < ARRAY_SIZE(reg_set); i++)
 		bnx2x_cl45_write(bp, phy, reg_set[i].devad, reg_set[i].reg,
 				 reg_set[i].val);
 
@@ -13395,7 +13395,7 @@ static void bnx2x_disable_kr2(struct link_params *params,
 	};
 	DP(NETIF_MSG_LINK, "Disabling 20G-KR2\n");
 
-	for (i = 0; i < sizeof(reg_set)/sizeof(struct bnx2x_reg_set); i++)
+	for (i = 0; i < ARRAY_SIZE(reg_set); i++)
 		bnx2x_cl45_write(bp, phy, reg_set[i].devad, reg_set[i].reg,
 				 reg_set[i].val);
 	vars->link_attr_sync &= ~LINK_ATTR_SYNC_KR2_ENABLE;
-- 
1.8.0

Re: [PATCH] bnx2x: use ARRAY_SIZE where possible

[Eilon Greenstein]

On Thu, 2012-12-20 at 14:11 -0500, Sasha Levin wrote:
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Hi Sasha,

I have no problem with this cosmetic change, but it should probably be
sent to net-next when it is open again. When you do resubmit it, you can
add my ACK:

Acked-by Eilon Greenstein <eilong@broadcom.com>

Thanks,
Eilon

Re: [PATCH] bnx2x: use ARRAY_SIZE where possible

[David Miller]

From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Sun, 23 Dec 2012 10:49:27 +0200

> On Thu, 2012-12-20 at 14:11 -0500, Sasha Levin wrote:
>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> 
> Acked-by Eilon Greenstein <eilong@broadcom.com>

Applied.

[PATCH] wireless: mwifiex: remove unreachable paths

[Sasha Levin]

We know 'firmware' is non-NULL from the beginning of mwifiex_prog_fw_w_helper,
remove all !firmware paths from the rest of the function.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/net/wireless/mwifiex/usb.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/usb.c b/drivers/net/wireless/mwifiex/usb.c
index 63ac9f2..8bd7098 100644
--- a/drivers/net/wireless/mwifiex/usb.c
+++ b/drivers/net/wireless/mwifiex/usb.c
@@ -836,23 +836,14 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
 			dlen = 0;
 		} else {
 			/* copy the header of the fw_data to get the length */
-			if (firmware)
-				memcpy(&fwdata->fw_hdr, &firmware[tlen],
-				       sizeof(struct fw_header));
-			else
-				mwifiex_get_fw_data(adapter, tlen,
-						    sizeof(struct fw_header),
-						    (u8 *)&fwdata->fw_hdr);
+			memcpy(&fwdata->fw_hdr, &firmware[tlen],
+			       sizeof(struct fw_header));
 
 			dlen = le32_to_cpu(fwdata->fw_hdr.data_len);
 			dnld_cmd = le32_to_cpu(fwdata->fw_hdr.dnld_cmd);
 			tlen += sizeof(struct fw_header);
 
-			if (firmware)
-				memcpy(fwdata->data, &firmware[tlen], dlen);
-			else
-				mwifiex_get_fw_data(adapter, tlen, dlen,
-						    (u8 *)fwdata->data);
+			memcpy(fwdata->data, &firmware[tlen], dlen);
 
 			fwdata->seq_num = cpu_to_le32(fw_seqnum);
 			tlen += dlen;
-- 
1.8.0

RE: [PATCH] wireless: mwifiex: remove unreachable paths

[Bing Zhao]

Hi Sasha,

Thanks for the patch.

> We know 'firmware' is non-NULL from the beginning of mwifiex_prog_fw_w_helper,
> remove all !firmware paths from the rest of the function.

After removing all !firmware paths the function 'mwifiex_get_fw_data' becomes an orphan.

Could you please remove that function as well and resend a v2 patch?

Thanks,
Bing

> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  drivers/net/wireless/mwifiex/usb.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/wireless/mwifiex/usb.c b/drivers/net/wireless/mwifiex/usb.c
> index 63ac9f2..8bd7098 100644
> --- a/drivers/net/wireless/mwifiex/usb.c
> +++ b/drivers/net/wireless/mwifiex/usb.c
> @@ -836,23 +836,14 @@ static int mwifiex_prog_fw_w_helper(struct mwifiex_adapter *adapter,
>  			dlen = 0;
>  		} else {
>  			/* copy the header of the fw_data to get the length */
> -			if (firmware)
> -				memcpy(&fwdata->fw_hdr, &firmware[tlen],
> -				       sizeof(struct fw_header));
> -			else
> -				mwifiex_get_fw_data(adapter, tlen,
> -						    sizeof(struct fw_header),
> -						    (u8 *)&fwdata->fw_hdr);
> +			memcpy(&fwdata->fw_hdr, &firmware[tlen],
> +			       sizeof(struct fw_header));
> 
>  			dlen = le32_to_cpu(fwdata->fw_hdr.data_len);
>  			dnld_cmd = le32_to_cpu(fwdata->fw_hdr.dnld_cmd);
>  			tlen += sizeof(struct fw_header);
> 
> -			if (firmware)
> -				memcpy(fwdata->data, &firmware[tlen], dlen);
> -			else
> -				mwifiex_get_fw_data(adapter, tlen, dlen,
> -						    (u8 *)fwdata->data);
> +			memcpy(fwdata->data, &firmware[tlen], dlen);
> 
>  			fwdata->seq_num = cpu_to_le32(fw_seqnum);
>  			tlen += dlen;
> --
> 1.8.0

[PATCH] mmc: omap_hsmmc: correct precedence of operators

[Sasha Levin]

With the current code, the condition in the if() doesn't make much sense due to
precedence of operators.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 drivers/mmc/host/omap_hsmmc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index bc58078..3ee2664 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -611,7 +611,7 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
 	if (host->context_loss == context_loss)
 		return 1;
 
-	if (!OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE)
+	if (!(OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE))
 		return 1;
 
 	if (host->pdata->controller_flags & OMAP_HSMMC_SUPPORTS_DUAL_VOLT) {
-- 
1.8.0

Re: [PATCH] mmc: omap_hsmmc: correct precedence of operators

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

On Thu, Dec 20, 2012 at 02:11:26PM -0500, Sasha Levin wrote:
> With the current code, the condition in the if() doesn't make much sense due to
> precedence of operators.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

nice catch!!!

Acked-by: Felipe Balbi <balbi@ti.com>

> ---
>  drivers/mmc/host/omap_hsmmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index bc58078..3ee2664 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -611,7 +611,7 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
>  	if (host->context_loss == context_loss)
>  		return 1;
>  
> -	if (!OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE)
> +	if (!(OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE))
>  		return 1;
>  
>  	if (host->pdata->controller_flags & OMAP_HSMMC_SUPPORTS_DUAL_VOLT) {
> -- 
> 1.8.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mmc: omap_hsmmc: correct precedence of operators

[Paul Walmsley]

(cc Tero)

Hi,

On Thu, 20 Dec 2012, Sasha Levin wrote:

> With the current code, the condition in the if() doesn't make much sense due to
> precedence of operators.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  drivers/mmc/host/omap_hsmmc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index bc58078..3ee2664 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -611,7 +611,7 @@ static int omap_hsmmc_context_restore(struct omap_hsmmc_host *host)
>  	if (host->context_loss == context_loss)
>  		return 1;
>  
> -	if (!OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE)
> +	if (!(OMAP_HSMMC_READ(host->base, SYSSTATUS) & RESETDONE))
>  		return 1;
>  
>  	if (host->pdata->controller_flags & OMAP_HSMMC_SUPPORTS_DUAL_VOLT) {

Makes sense to me, but I wonder if this code is even necessary after:

commit 613ad0e98c3596cd2524172fae2a795c3fc57e4a
Author: Tero Kristo <t-kristo@ti.com>
Date:   Mon Oct 29 22:02:13 2012 -0600

    ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod
    

?  You might just be able to drop those lines completely now.



- Paul

[PATCH] kdb: use ARRAY_SIZE where possible

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/debug/kdb/kdb_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 4d5f8d5..18ce064 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -124,7 +124,7 @@ static kdbmsg_t kdbmsgs[] = {
 };
 #undef KDBMSG
 
-static const int __nkdb_err = sizeof(kdbmsgs) / sizeof(kdbmsg_t);
+static const int __nkdb_err = ARRAY_SIZE(kdbmsgs);
 
 
 /*
@@ -175,7 +175,7 @@ static char *__env[] = {
  (char *)0,
 };
 
-static const int __nenv = (sizeof(__env) / sizeof(char *));
+static const int __nenv = ARRAY_SIZE(__env);
 
 struct task_struct *kdb_curr_task(int cpu)
 {
-- 
1.8.0

[PATCH] rcutorture: don't compare ptr with 0

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/rcutorture.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
index 31dea01..0249800 100644
--- a/kernel/rcutorture.c
+++ b/kernel/rcutorture.c
@@ -1749,7 +1749,7 @@ static int rcu_torture_barrier_init(void)
 	barrier_cbs_wq =
 		kzalloc(n_barrier_cbs * sizeof(barrier_cbs_wq[0]),
 			GFP_KERNEL);
-	if (barrier_cbs_tasks == NULL || barrier_cbs_wq == 0)
+	if (barrier_cbs_tasks == NULL || !barrier_cbs_wq)
 		return -ENOMEM;
 	for (i = 0; i < n_barrier_cbs; i++) {
 		init_waitqueue_head(&barrier_cbs_wq[i]);
-- 
1.8.0

Re: [PATCH] rcutorture: don't compare ptr with 0

[Josh Triplett]

On Thu, Dec 20, 2012 at 02:11:28PM -0500, Sasha Levin wrote:
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Fair enough.  You might as well change the first half of the conditional
too, though.  In any case:

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

>  kernel/rcutorture.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> index 31dea01..0249800 100644
> --- a/kernel/rcutorture.c
> +++ b/kernel/rcutorture.c
> @@ -1749,7 +1749,7 @@ static int rcu_torture_barrier_init(void)
>  	barrier_cbs_wq =
>  		kzalloc(n_barrier_cbs * sizeof(barrier_cbs_wq[0]),
>  			GFP_KERNEL);
> -	if (barrier_cbs_tasks == NULL || barrier_cbs_wq == 0)
> +	if (barrier_cbs_tasks == NULL || !barrier_cbs_wq)
>  		return -ENOMEM;
>  	for (i = 0; i < n_barrier_cbs; i++) {
>  		init_waitqueue_head(&barrier_cbs_wq[i]);
> -- 
> 1.8.0
> 

Re: [PATCH] rcutorture: don't compare ptr with 0

[Paul E. McKenney]

On Thu, Dec 20, 2012 at 02:39:12PM -0800, Josh Triplett wrote:
> On Thu, Dec 20, 2012 at 02:11:28PM -0500, Sasha Levin wrote:
> > Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> 
> Fair enough.  You might as well change the first half of the conditional
> too, though.  In any case:
> 
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Queued, thank you both!

							Thanx, Paul

> >  kernel/rcutorture.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/rcutorture.c b/kernel/rcutorture.c
> > index 31dea01..0249800 100644
> > --- a/kernel/rcutorture.c
> > +++ b/kernel/rcutorture.c
> > @@ -1749,7 +1749,7 @@ static int rcu_torture_barrier_init(void)
> >  	barrier_cbs_wq =
> >  		kzalloc(n_barrier_cbs * sizeof(barrier_cbs_wq[0]),
> >  			GFP_KERNEL);
> > -	if (barrier_cbs_tasks == NULL || barrier_cbs_wq == 0)
> > +	if (barrier_cbs_tasks == NULL || !barrier_cbs_wq)
> >  		return -ENOMEM;
> >  	for (i = 0; i < n_barrier_cbs; i++) {
> >  		init_waitqueue_head(&barrier_cbs_wq[i]);
> > -- 
> > 1.8.0
> > 
> 

[PATCH] sched: fair: simplify can_migrate_task a bit

[Sasha Levin]

We already know that tsk_cache_hot is true at the end of
can_migrate_task, no need to double check that.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/sched/fair.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 4603d6c..e03940e0 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3935,11 +3935,8 @@ int can_migrate_task(struct task_struct *p, struct lb_env *env)
 		return 1;
 	}
 
-	if (tsk_cache_hot) {
-		schedstat_inc(p, se.statistics.nr_failed_migrations_hot);
-		return 0;
-	}
-	return 1;
+	schedstat_inc(p, se.statistics.nr_failed_migrations_hot);
+	return 0;
 }
 
 /*
-- 
1.8.0

[PATCH] uprobes: remove redundant check

[Sasha Levin]

We checked for uprobe==NULL earlier, no need to redo that.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/events/uprobes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index dea7acf..30ea9a4 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -901,8 +901,7 @@ void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consume
 	}
 
 	mutex_unlock(uprobes_hash(inode));
-	if (uprobe)
-		put_uprobe(uprobe);
+	put_uprobe(uprobe);
 }
 
 static struct rb_node *
-- 
1.8.0

[tip:perf/core] uprobes: remove redundant check

[tip-bot for Sasha Levin]

Commit-ID:  c91368c4889a0ee5dd06552adbb50ae54f5096fd
Gitweb:     http://git.kernel.org/tip/c91368c4889a0ee5dd06552adbb50ae54f5096fd
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:30 -0500
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:15 -0300

uprobes: remove redundant check

We checked for uprobe==NULL earlier, no need to redo that.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1356030701-16284-22-git-send-email-sasha.levin@oracle.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 kernel/events/uprobes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index dea7acf..30ea9a4 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -901,8 +901,7 @@ void uprobe_unregister(struct inode *inode, loff_t offset, struct uprobe_consume
 	}
 
 	mutex_unlock(uprobes_hash(inode));
-	if (uprobe)
-		put_uprobe(uprobe);
+	put_uprobe(uprobe);
 }
 
 static struct rb_node *

[PATCH] cgroup: remove redundant check

[Sasha Levin]

We already know that 'event' is non-NULL, remove the redundant check.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 4855892..8d6c3f2 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -3898,7 +3898,7 @@ fail:
 	if (cfile)
 		fput(cfile);
 
-	if (event && event->eventfd && !IS_ERR(event->eventfd))
+	if (event->eventfd && !IS_ERR(event->eventfd))
 		eventfd_ctx_put(event->eventfd);
 
 	if (!IS_ERR_OR_NULL(efile))
-- 
1.8.0

Re: [PATCH] cgroup: remove redundant check

[Tejun Heo]

On Thu, Dec 20, 2012 at 02:11:31PM -0500, Sasha Levin wrote:
> We already know that 'event' is non-NULL, remove the redundant check.
> 
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

I don't know.  I kinda like that test there.  It's not an error path
and tearing down everything which could have been allocated.  If the
test not being used is bothering, convert return -ENOMEM to goto fail?

-- 
tejun

Re: [PATCH] cgroup: remove redundant check

[Sasha Levin]

On 12/20/2012 02:18 PM, Tejun Heo wrote:
> On Thu, Dec 20, 2012 at 02:11:31PM -0500, Sasha Levin wrote:
>> We already know that 'event' is non-NULL, remove the redundant check.
>>
>> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> 
> I don't know.  I kinda like that test there.  It's not an error path
> and tearing down everything which could have been allocated.  If the
> test not being used is bothering, convert return -ENOMEM to goto fail?

Can't say it's bothering. If you'd prefer to leave it as is I'm okay with
doing that.


Thanks,
Sasha

[PATCH] mm: memory_hotplug: no need to check res twice in add_memory

[Sasha Levin]

Remove one redundant check of res.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 mm/memory_hotplug.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 962e353..4082244 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -898,8 +898,7 @@ error:
 	/* rollback pgdat allocation and others */
 	if (new_pgdat)
 		rollback_node_hotadd(nid, pgdat);
-	if (res)
-		release_memory_resource(res);
+	release_memory_resource(res);
 
 out:
 	unlock_memory_hotplug();
-- 
1.8.0

[PATCH] x86, efi: correct precedence of operators in setup_efi_pci

[Sasha Levin]

With the current code, the condition in the if() doesn't make much sense due to
precedence of operators.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/x86/boot/compressed/eboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index b1942e2..18e329c 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -302,7 +302,7 @@ static efi_status_t setup_efi_pci(struct boot_params *params)
 		if (status != EFI_SUCCESS)
 			continue;
 
-		if (!attributes & EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM)
+		if (!(attributes & EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM))
 			continue;
 
 		if (!pci->romimage || !pci->romsize)
-- 
1.8.0

[tip:x86/urgent] x86, efi: correct precedence of operators in setup_efi_pci

[tip-bot for Sasha Levin]

Commit-ID:  886d751a2ea99a160f2d0a472231566d9cb0cf58
Gitweb:     http://git.kernel.org/tip/886d751a2ea99a160f2d0a472231566d9cb0cf58
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:33 -0500
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Dec 2012 11:47:14 -0800

x86, efi: correct precedence of operators in setup_efi_pci

With the current code, the condition in the if() doesn't make much sense due to
precedence of operators.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Link: http://lkml.kernel.org/r/1356030701-16284-25-git-send-email-sasha.levin@oracle.com
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/boot/compressed/eboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index b1942e2..18e329c 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -302,7 +302,7 @@ static efi_status_t setup_efi_pci(struct boot_params *params)
 		if (status != EFI_SUCCESS)
 			continue;
 
-		if (!attributes & EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM)
+		if (!(attributes & EFI_PCI_IO_ATTRIBUTE_EMBEDDED_ROM))
 			continue;
 
 		if (!pci->romimage || !pci->romsize)

[PATCH] arch/x86/platform/uv: use ARRAY_SIZE where possible

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/x86/platform/uv/tlb_uv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index dbbdca5..0f92173 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -1467,7 +1467,7 @@ static ssize_t ptc_proc_write(struct file *file, const char __user *user,
 	}
 
 	if (input_arg == 0) {
-		elements = sizeof(stat_description)/sizeof(*stat_description);
+		elements = ARRAY_SIZE(stat_description);
 		printk(KERN_DEBUG "# cpu:      cpu number\n");
 		printk(KERN_DEBUG "Sender statistics:\n");
 		for (i = 0; i < elements; i++)
@@ -1508,7 +1508,7 @@ static int parse_tunables_write(struct bau_control *bcp, char *instr,
 	char *q;
 	int cnt = 0;
 	int val;
-	int e = sizeof(tunables) / sizeof(*tunables);
+	int e = ARRAY_SIZE(tunables);
 
 	p = instr + strspn(instr, WHITESPACE);
 	q = p;
-- 
1.8.0

[tip:x86/cleanups] arch/x86/platform/uv: use ARRAY_SIZE where possible

[tip-bot for Sasha Levin]

Commit-ID:  6444174548f6556fcf26c84d5296defd295914c4
Gitweb:     http://git.kernel.org/tip/6444174548f6556fcf26c84d5296defd295914c4
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:34 -0500
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Dec 2012 11:49:39 -0800

arch/x86/platform/uv: use ARRAY_SIZE where possible

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Link: http://lkml.kernel.org/r/1356030701-16284-26-git-send-email-sasha.levin@oracle.com
Cc: Cliff Wickman <cpw@sgi.com>
Cc: Alex Shi <alex.shi@intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/platform/uv/tlb_uv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/platform/uv/tlb_uv.c b/arch/x86/platform/uv/tlb_uv.c
index b8b3a37..933bea5 100644
--- a/arch/x86/platform/uv/tlb_uv.c
+++ b/arch/x86/platform/uv/tlb_uv.c
@@ -1463,7 +1463,7 @@ static ssize_t ptc_proc_write(struct file *file, const char __user *user,
 	}
 
 	if (input_arg == 0) {
-		elements = sizeof(stat_description)/sizeof(*stat_description);
+		elements = ARRAY_SIZE(stat_description);
 		printk(KERN_DEBUG "# cpu:      cpu number\n");
 		printk(KERN_DEBUG "Sender statistics:\n");
 		for (i = 0; i < elements; i++)
@@ -1504,7 +1504,7 @@ static int parse_tunables_write(struct bau_control *bcp, char *instr,
 	char *q;
 	int cnt = 0;
 	int val;
-	int e = sizeof(tunables) / sizeof(*tunables);
+	int e = ARRAY_SIZE(tunables);
 
 	p = instr + strspn(instr, WHITESPACE);
 	q = p;

[PATCH] um: don't compare a pointer to 0

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/x86/um/fault.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/um/fault.c b/arch/x86/um/fault.c
index 8784ab3..84ac7f7 100644
--- a/arch/x86/um/fault.c
+++ b/arch/x86/um/fault.c
@@ -20,7 +20,7 @@ int arch_fixup(unsigned long address, struct uml_pt_regs *regs)
 	const struct exception_table_entry *fixup;
 
 	fixup = search_exception_tables(address);
-	if (fixup != 0) {
+	if (fixup) {
 		UPT_IP(regs) = fixup->fixup;
 		return 1;
 	}
-- 
1.8.0

[tip:x86/cleanups] um: don't compare a pointer to 0

[tip-bot for Sasha Levin]

Commit-ID:  7be0b06520e8219e28f54cd05e1310e7e2d921ee
Gitweb:     http://git.kernel.org/tip/7be0b06520e8219e28f54cd05e1310e7e2d921ee
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:35 -0500
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Dec 2012 11:49:39 -0800

um: don't compare a pointer to 0

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Link: http://lkml.kernel.org/r/1356030701-16284-27-git-send-email-sasha.levin@oracle.com
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/um/fault.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/um/fault.c b/arch/x86/um/fault.c
index 8784ab3..84ac7f7 100644
--- a/arch/x86/um/fault.c
+++ b/arch/x86/um/fault.c
@@ -20,7 +20,7 @@ int arch_fixup(unsigned long address, struct uml_pt_regs *regs)
 	const struct exception_table_entry *fixup;
 
 	fixup = search_exception_tables(address);
-	if (fixup != 0) {
+	if (fixup) {
 		UPT_IP(regs) = fixup->fixup;
 		return 1;
 	}

[PATCH] x86, apb_timer: remove unused variable percpu_timer

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 arch/x86/kernel/apb_timer.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/apb_timer.c b/arch/x86/kernel/apb_timer.c
index afdc3f75..cc74fd0 100644
--- a/arch/x86/kernel/apb_timer.c
+++ b/arch/x86/kernel/apb_timer.c
@@ -311,7 +311,6 @@ void __init apbt_time_init(void)
 #ifdef CONFIG_SMP
 	int i;
 	struct sfi_timer_table_entry *p_mtmr;
-	unsigned int percpu_timer;
 	struct apbt_dev *adev;
 #endif
 
@@ -346,13 +345,10 @@ void __init apbt_time_init(void)
 		return;
 	}
 	pr_debug("%s: %d CPUs online\n", __func__, num_online_cpus());
-	if (num_possible_cpus() <= sfi_mtimer_num) {
-		percpu_timer = 1;
+	if (num_possible_cpus() <= sfi_mtimer_num)
 		apbt_num_timers_used = num_possible_cpus();
-	} else {
-		percpu_timer = 0;
+	else
 		apbt_num_timers_used = 1;
-	}
 	pr_debug("%s: %d APB timers used\n", __func__, apbt_num_timers_used);
 
 	/* here we set up per CPU timer data structure */
-- 
1.8.0

[tip:x86/cleanups] x86, apb_timer: remove unused variable percpu_timer

[tip-bot for Sasha Levin]

Commit-ID:  8f170faeb458532282dbfa870f456e42c11d1ebb
Gitweb:     http://git.kernel.org/tip/8f170faeb458532282dbfa870f456e42c11d1ebb
Author:     Sasha Levin <sasha.levin@oracle.com>
AuthorDate: Thu, 20 Dec 2012 14:11:36 -0500
Committer:  H. Peter Anvin <hpa@linux.intel.com>
CommitDate: Thu, 20 Dec 2012 11:49:39 -0800

x86, apb_timer: remove unused variable percpu_timer

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
Link: http://lkml.kernel.org/r/1356030701-16284-28-git-send-email-sasha.levin@oracle.com
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/kernel/apb_timer.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/apb_timer.c b/arch/x86/kernel/apb_timer.c
index afdc3f75..cc74fd0 100644
--- a/arch/x86/kernel/apb_timer.c
+++ b/arch/x86/kernel/apb_timer.c
@@ -311,7 +311,6 @@ void __init apbt_time_init(void)
 #ifdef CONFIG_SMP
 	int i;
 	struct sfi_timer_table_entry *p_mtmr;
-	unsigned int percpu_timer;
 	struct apbt_dev *adev;
 #endif
 
@@ -346,13 +345,10 @@ void __init apbt_time_init(void)
 		return;
 	}
 	pr_debug("%s: %d CPUs online\n", __func__, num_online_cpus());
-	if (num_possible_cpus() <= sfi_mtimer_num) {
-		percpu_timer = 1;
+	if (num_possible_cpus() <= sfi_mtimer_num)
 		apbt_num_timers_used = num_possible_cpus();
-	} else {
-		percpu_timer = 0;
+	else
 		apbt_num_timers_used = 1;
-	}
 	pr_debug("%s: %d APB timers used\n", __func__, apbt_num_timers_used);
 
 	/* here we set up per CPU timer data structure */

[PATCH] kdb: use DIV_ROUND_UP where possible

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/debug/kdb/kdb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 18ce064..199abdc 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1670,7 +1670,7 @@ static int kdb_md(int argc, const char **argv)
 			    num, repeat, phys);
 		addr += bytesperword * n;
 		repeat -= n;
-		z = (z + num - 1) / num;
+		z = DIV_ROUND_UP(z, num);
 		if (z > 2) {
 			int s = num * (z-2);
 			kdb_printf(kdb_machreg_fmt0 "-" kdb_machreg_fmt0
-- 
1.8.0

[PATCH] mm/sparse: don't check return value of alloc_bootmem calls

[Sasha Levin]

These calls will panic if they can't allocate the memory, so we can
assume that we get actual memory back. This simplifies some functions
and removes unneeded checks.

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 mm/sparse.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/mm/sparse.c b/mm/sparse.c
index 6b5fb76..ae64d6e 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -403,15 +403,13 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
 	size = PAGE_ALIGN(size);
 	map = __alloc_bootmem_node_high(NODE_DATA(nodeid), size * map_count,
 					 PAGE_SIZE, __pa(MAX_DMA_ADDRESS));
-	if (map) {
-		for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
-			if (!present_section_nr(pnum))
-				continue;
-			map_map[pnum] = map;
-			map += size;
-		}
-		return;
+	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
+		if (!present_section_nr(pnum))
+			continue;
+		map_map[pnum] = map;
+		map += size;
 	}
+	return;
 
 	/* fallback */
 	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
@@ -497,8 +495,6 @@ void __init sparse_init(void)
 	 */
 	size = sizeof(unsigned long *) * NR_MEM_SECTIONS;
 	usemap_map = alloc_bootmem(size);
-	if (!usemap_map)
-		panic("can not allocate usemap_map\n");
 
 	for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
 		struct mem_section *ms;
@@ -538,8 +534,6 @@ void __init sparse_init(void)
 #ifdef CONFIG_SPARSEMEM_ALLOC_MEM_MAP_TOGETHER
 	size2 = sizeof(struct page *) * NR_MEM_SECTIONS;
 	map_map = alloc_bootmem(size2);
-	if (!map_map)
-		panic("can not allocate map_map\n");
 
 	for (pnum = 0; pnum < NR_MEM_SECTIONS; pnum++) {
 		struct mem_section *ms;
-- 
1.8.0

Re: [PATCH] mm/sparse: don't check return value of alloc_bootmem calls

[David Rientjes]

On Thu, 20 Dec 2012, Sasha Levin wrote:

> diff --git a/mm/sparse.c b/mm/sparse.c
> index 6b5fb76..ae64d6e 100644
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -403,15 +403,13 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>  	size = PAGE_ALIGN(size);
>  	map = __alloc_bootmem_node_high(NODE_DATA(nodeid), size * map_count,
>  					 PAGE_SIZE, __pa(MAX_DMA_ADDRESS));
> -	if (map) {
> -		for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> -			if (!present_section_nr(pnum))
> -				continue;
> -			map_map[pnum] = map;
> -			map += size;
> -		}
> -		return;
> +	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> +		if (!present_section_nr(pnum))
> +			continue;
> +		map_map[pnum] = map;
> +		map += size;
>  	}
> +	return;
>  
>  	/* fallback */
>  	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {

That's not true when slab_is_available() and why would you possibly add a 
return statement right before fallback code in such cases?

Re: [PATCH] mm/sparse: don't check return value of alloc_bootmem calls

[Sasha Levin]

On 12/20/2012 03:23 PM, David Rientjes wrote:
> On Thu, 20 Dec 2012, Sasha Levin wrote:
> 
>> diff --git a/mm/sparse.c b/mm/sparse.c
>> index 6b5fb76..ae64d6e 100644
>> --- a/mm/sparse.c
>> +++ b/mm/sparse.c
>> @@ -403,15 +403,13 @@ void __init sparse_mem_maps_populate_node(struct page **map_map,
>>  	size = PAGE_ALIGN(size);
>>  	map = __alloc_bootmem_node_high(NODE_DATA(nodeid), size * map_count,
>>  					 PAGE_SIZE, __pa(MAX_DMA_ADDRESS));
>> -	if (map) {
>> -		for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
>> -			if (!present_section_nr(pnum))
>> -				continue;
>> -			map_map[pnum] = map;
>> -			map += size;
>> -		}
>> -		return;
>> +	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
>> +		if (!present_section_nr(pnum))
>> +			continue;
>> +		map_map[pnum] = map;
>> +		map += size;
>>  	}
>> +	return;
>>  
>>  	/* fallback */
>>  	for (pnum = pnum_begin; pnum < pnum_end; pnum++) {
> 
> That's not true when slab_is_available() and why would you possibly add a 
> return statement right before fallback code in such cases?

So what we really need is to update the documentation of __alloc_bootmem_node, I'll send
a patch that does that instead.

I've dragged the 'return' out of the if(map) condition, I should have removed the fallback
as well and so the return would also be gone, but that's irrelevant now.


Thanks,
Sasha

Re: [PATCH] mm/sparse: don't check return value of alloc_bootmem calls

[David Rientjes]

On Thu, 20 Dec 2012, Sasha Levin wrote:

> So what we really need is to update the documentation of __alloc_bootmem_node, I'll send
> a patch that does that instead.
> 

It panics iff slab is not available to allocate from yet, otherwise it's 
just a wrapper around kmalloc().  This emits a warning to the kernel log, 
though, so __alloc_bootmem_node() should certainly not be called that late 
in the boot sequence.

Since __alloc_bootmem_node_nopanic() is the way to avoid the panic, I 
think the change that should be made here so to panic even when the 
kmalloc() fails in __alloc_bootmem_node(), __alloc_bootmem_node_high(), 
and __alloc_bootmem_low_node().

[PATCH] slob: use DIV_ROUND_UP where possible

[Sasha Levin]

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 mm/slob.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slob.c b/mm/slob.c
index a99fdf7..f729c46 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -122,7 +122,7 @@ static inline void clear_slob_page_free(struct page *sp)
 }
 
 #define SLOB_UNIT sizeof(slob_t)
-#define SLOB_UNITS(size) (((size) + SLOB_UNIT - 1)/SLOB_UNIT)
+#define SLOB_UNITS(size) DIV_ROUND_UP(size, SLOB_UNIT)
 
 /*
  * struct slob_rcu is inserted at the tail of allocated slob blocks, which
-- 
1.8.0

Re: [PATCH] slob: use DIV_ROUND_UP where possible

[Christoph Lameter]

Obviously good.

Acked-by: Christoph Lameter <cl@linux.com>