All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ye Liu <ye.liu@linux.dev>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: tj@kernel.org, jiangshanlai@gmail.com,
	linux-kernel@vger.kernel.org, Ye Liu <liuye@kylinos.cn>
Subject: Re: [PATCH] workqueue: Fix incorrect error return value in apply_workqueue_attrs_locked
Date: Tue, 25 Mar 2025 13:53:31 +0800	[thread overview]
Message-ID: <3baba5be-2ea9-4a21-9c31-c3ded392008d@linux.dev> (raw)
In-Reply-To: <9fc752f5-239d-4734-a437-77a3bccf74ec@stanley.mountain>


在 2025/3/25 13:24, Dan Carpenter 写道:
> On Mon, Mar 24, 2025 at 05:07:48PM +0800, Ye Liu wrote:
> v> From: Ye Liu <liuye@kylinos.cn>
>> Commit 84193c07105c ("workqueue: Generalize unbound CPU pods") introduced
>> a change that caused apply_workqueue_attrs_locked() to return error
>> pointers using PTR_ERR() on failure instead of a negative error code.
> PTR_ERR() does return negative error codes.  Unless you pass it a NULL
> pointer, then it returns success.  Or if you pass it a valid pointer it
> returns garbage.
I misunderstood PTR_ERR(). Thanks for pointing it out.
>> This caused unexpected behavior in functions that rely on the return value
>> of apply_workqueue_attrs_locked, such as alloc_and_link_pwqs().
>>
>> Specifically, alloc_and_link_pwqs() expects apply_workqueue_attrs_locked()
>> to return 0 on success and a negative error code on failure. However,
>> returning PTR_ERR(ctx) instead of -ENOMEM led to incorrect error handling
>> in __alloc_workqueue, potentially causing system instability or crashes.
>>
>> This patch ensures apply_workqueue_attrs_locked() returns a proper negative
>> error code (-ENOMEM) in case of failure, restoring expected behavior.
>>
>> Fixes: 84193c07105c ("workqueue: Generalize unbound CPU pods")
>> Signed-off-by: Ye Liu <liuye@kylinos.cn>
>> ---
>>  kernel/workqueue.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
>> index bfe030b443e2..8ba679d9b566 100644
>> --- a/kernel/workqueue.c
>> +++ b/kernel/workqueue.c
>> @@ -5363,7 +5363,7 @@ static int apply_workqueue_attrs_locked(struct workqueue_struct *wq,
>>  
>>  	ctx = apply_wqattrs_prepare(wq, attrs, wq_unbound_cpumask);
>>  	if (IS_ERR(ctx))
>> -		return PTR_ERR(ctx);
>> +		return -ENOMEM;
>>  
> The original code was correct and the patch is wrong.
Drop it.
> regards,
> dan carpenter
>

Thanks,

Ye Liu


      reply	other threads:[~2025-03-25  5:53 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-24  9:07 [PATCH] workqueue: Fix incorrect error return value in apply_workqueue_attrs_locked Ye Liu
2025-03-24 16:10 ` Markus Elfring
2025-03-25  1:33   ` Ye Liu
2025-03-25  5:24 ` Dan Carpenter
2025-03-25  5:53   ` Ye Liu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3baba5be-2ea9-4a21-9c31-c3ded392008d@linux.dev \
    --to=ye.liu@linux.dev \
    --cc=dan.carpenter@linaro.org \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=liuye@kylinos.cn \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.