workqueue: use type int instead of bool to index array

PATCH In review zijun_hu 2 participants 6 messages Sep 6, 2017

Subsystems (2)

Memory management Workqueue

lore

Share & tools

lore

zijun_hu

author

2017-09-06 03:51 UTC #0 lore raw

PATCH 1/1 workqueue: use type int instead of bool to index array

From: zijun_hu <[email protected]>

type bool is used to index three arrays in alloc_and_link_pwqs() it doesn't look like conventional.

it is fixed by using type int to index the relevant arrays.

Signed-off-by zijun_hu <[email protected]>

---
 kernel/workqueue.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

kernel/workqueue.c +5 -4

index ab3c0dc8c7ed..0c7a16792949 100644
@@ -3917,7 +3917,7 @@ static void wq_update_unbound_numa(struct workqueue_struct *wq, int cpu,
 
 static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 {
-	bool highpri = wq->flags & WQ_HIGHPRI;
+	int pool_idx = (wq->flags & WQ_HIGHPRI) ? 1 : 0;
 	int cpu, ret;
 
 	if (!(wq->flags & WQ_UNBOUND)) {

@@ -3931,7 +3931,7 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 			struct worker_pool *cpu_pools =
 				per_cpu(cpu_worker_pools, cpu);
 
-			init_pwq(pwq, wq, &cpu_pools[highpri]);
+			init_pwq(pwq, wq, &cpu_pools[pool_idx]);
 
 			mutex_lock(&wq->mutex);
 			link_pwq(pwq);

@@ -3939,14 +3939,15 @@ static int alloc_and_link_pwqs(struct workqueue_struct *wq)
 		}
 		return 0;
 	} else if (wq->flags & __WQ_ORDERED) {
-		ret = apply_workqueue_attrs(wq, ordered_wq_attrs[highpri]);
+		ret = apply_workqueue_attrs(wq, ordered_wq_attrs[pool_idx]);
 		/* there should only be single pwq for ordering guarantee */
 		WARN(!ret && (wq->pwqs.next != &wq->dfl_pwq->pwqs_node ||
 			      wq->pwqs.prev != &wq->dfl_pwq->pwqs_node),
 		     "ordering guarantee broken for workqueue %s\n", wq->name);
 		return ret;
 	} else {
-		return apply_workqueue_attrs(wq, unbound_std_wq_attrs[highpri]);
+		return apply_workqueue_attrs(wq,
+					unbound_std_wq_attrs[pool_idx]);
 	}
 }

Signature · 1 line

1.9.1

Tejun Heo

maintainer

2017-09-06 14:33 UTC #1 lore raw

Hello,

Quoted 6 lines · zijun_hu

From: zijun_hu <[email protected]>
type bool is used to index three arrays in alloc_and_link_pwqs() it doesn't look like conventional.
it is fixed by using type int to index the relevant arrays.

bool is a uint type which can be either 0 or 1. I don't see what the benefit of this patch is.q

Thanks.

Signature · 1 line

tejun

zijun_hu

author

2017-09-06 16:05 UTC #2 lore raw

Quoted 5 lines · Tejun Heo

Hello,
Quoted 2 lines · zijun_hu
From: zijun_hu <[email protected]>

Quoted 8 lines

Quoted 4 lines
type bool is used to index three arrays in alloc_and_link_pwqs() it doesn't look like conventional.
it is fixed by using type int to index the relevant arrays.
bool is a uint type which can be either 0 or 1. I don't see what the benefit of this patch is.q

bool is NOT a uint type now, it is a new type introduced by gcc, it is rather different with "typedef int bool" historically
see following code segments for more info about type bool

	bool v = 0x10;
	printf("v = %d\n", v);

the output is v = 1.

it maybe cause a invalid array index if bool is represented as uint bool highpri = wq->flags & WQ_HIGHPRI; WQ_HIGHPRI = 1 << 4, @highpri maybe 16, but the number of array elements is 2.

bool is a logic value, the valid value is true or false. indexing array by type bool is not a good program custom

it is more extendable to use type int, type bool maybe is improper if the number of array elements is extended to more than 2 in future

besides, the relevant array is indexed by type int in many other places of the same source file. this patch can keep consistency

Quoted 1 line

Tejun Heo

maintainer

2017-09-06 16:40 UTC #3 lore raw

Quoted 9 lines · zijun_hu

Quoted 8 lines · Tejun Heo
Hello,
Quoted 5 lines · zijun_hu
From: zijun_hu <[email protected]>
type bool is used to index three arrays in alloc_and_link_pwqs() it doesn't look like conventional.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
it is fixed by using type int to index the relevant arrays.
bool is a uint type which can be either 0 or 1. I don't see what the benefit of this patch is.q
bool is NOT a uint type now, it is a new type introduced by gcc, it is rather different with "typedef int bool" historically

http://www.open-std.org/jtc1/sc22/wg14/www/docs/n815.htm

Because C has existed for so long without a Boolean type, however, the new standard must coexist with the old remedies. Therefore, the type name is taken from the reserved identifier space. To maintain orthogonal promotion rules, the Boolean type is defined as an unsigned integer type capable of representing the values 0 and 1. The more conventional names for the type and its values are then made available only with the inclusion of the <stdbool.h> header. In addition, the header defines a feature test macro to aid in integrating new code with old code that defines its own Boolean type.

Signature · 7 lines

tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

zijun_hu

author

2017-09-06 17:07 UTC #4 lore raw

Quoted 20 lines · Tejun Heo

Quoted 9 lines · zijun_hu
Quoted 8 lines · Tejun Heo
Hello,
Quoted 5 lines · zijun_hu
From: zijun_hu <[email protected]>
type bool is used to index three arrays in alloc_and_link_pwqs() it doesn't look like conventional.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
it is fixed by using type int to index the relevant arrays.
bool is a uint type which can be either 0 or 1. I don't see what the benefit of this patch is.q
bool is NOT a uint type now, it is a new type introduced by gcc, it is rather different with "typedef int bool" historically
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n815.htm

Quoted 10 lines

Because C has existed for so long without a Boolean type, however, the new standard must coexist with the old remedies. Therefore, the type name is taken from the reserved identifier space. To maintain orthogonal promotion rules, the Boolean type is defined as an unsigned integer type capable of representing the values 0 and 1. The more conventional names for the type and its values are then made available only with the inclusion of the <stdbool.h> header. In addition, the header defines a feature test macro to aid in integrating new code with old code that defines its own Boolean type.

in this case, i think type int is more suitable than bool in aspects of extendibility, program custom and consistency.

Tejun Heo

maintainer

2017-09-06 17:22 UTC #5 lore raw

Quoted 2 lines · zijun_hu

in this case, i think type int is more suitable than bool in aspects of extendibility, program custom and consistency.

Please stop.

Signature · 7 lines

tejun

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>