linux-kernel - [kernel/workqueue.c] Unnecessary WORK_OFFQ_POOL_NONE in the worker_pool_assign_id(), idr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <CAHOvCC4JU-f0-H=x1uWLjpCn4yeHr1Ze1DrXXoTmsGqW6-fYSw@mail.gmail.com>
Date:   Tue, 7 Feb 2023 19:11:38 +0900
From:   JaeJoon Jung <rgbi3307@...il.com>
To:     Tejun Heo <tj@...nel.org>
Cc:     David Woodhouse <dwmw2@...radead.org>,
        "Theodore Ts'o" <tytso@....edu>, linux-kernel@...r.kernel.org
Subject: [kernel/workqueue.c] Unnecessary WORK_OFFQ_POOL_NONE in the
 worker_pool_assign_id(), idr_alloc().

When determining worker_pool->id using idr_alloc() in the
worker_pool_assign_id() function,
I think You need not to pass WORK_OFFQ_POOL_NONE to the idr_alloc() function.
This is because the idr_alloc() function checks the maximum value of
id as INT_MAX.
WORK_OFFQ_POOL_NONE is equivalent to INT_MAX.
So, I think Any code related to WORK_OFFQ_POOL_NONE is not needed.


diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index a0143dd24430..ac65e66f8fc3 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -79,7 +79,6 @@ enum {
        WORK_OFFQ_POOL_SHIFT    = WORK_OFFQ_FLAG_BASE + WORK_OFFQ_FLAG_BITS,
        WORK_OFFQ_LEFT          = BITS_PER_LONG - WORK_OFFQ_POOL_SHIFT,
        WORK_OFFQ_POOL_BITS     = WORK_OFFQ_LEFT <= 31 ? WORK_OFFQ_LEFT : 31,
-       WORK_OFFQ_POOL_NONE     = (1LU << WORK_OFFQ_POOL_BITS) - 1,

        /* convenience constants */
        WORK_STRUCT_FLAG_MASK   = (1UL << WORK_STRUCT_FLAG_BITS) - 1,
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 7cd5f5e7e0a1..c38e93266987 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -546,7 +546,7 @@ static inline void debug_work_deactivate(struct
work_struct *work) { }
  * worker_pool_assign_id - allocate ID and assign it to @pool
  * @pool: the pool pointer of interest
  *
- * Returns 0 if ID in [0, WORK_OFFQ_POOL_NONE) is allocated and assigned
+ * Returns 0 if ID in [0, INT_MAX) is allocated and assigned
  * successfully, -errno on failure.
  */
 static int worker_pool_assign_id(struct worker_pool *pool)
@@ -555,8 +555,7 @@ static int worker_pool_assign_id(struct worker_pool *pool)

        lockdep_assert_held(&wq_pool_mutex);

-       ret = idr_alloc(&worker_pool_idr, pool, 0, WORK_OFFQ_POOL_NONE,
-                       GFP_KERNEL);
+       ret = idr_alloc(&worker_pool_idr, pool, 0, 0, GFP_KERNEL);
        if (ret >= 0) {
                pool->id = ret;
                return 0;
@@ -735,8 +734,6 @@ static struct worker_pool *get_work_pool(struct
work_struct *work)
                        (data & WORK_STRUCT_WQ_DATA_MASK))->pool;

        pool_id = data >> WORK_OFFQ_POOL_SHIFT;
-       if (pool_id == WORK_OFFQ_POOL_NONE)
-               return NULL;

        return idr_find(&worker_pool_idr, pool_id);
 }

Please check the above.
Thanks,
>From JaeJoon Jung.