* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-23 17:17 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, kvm, virtualization, netdev, sgarzare, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260723124908-mutt-send-email-mst@kernel.org>
On 7/23/26 7:50 PM, Michael S. Tsirkin wrote:
> On Thu, Jul 23, 2026 at 07:46:58PM +0300, Andrey Drobyshev wrote:
>> On 7/23/26 6:18 PM, Michael S. Tsirkin wrote:
>>> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>>>> vq->worker and queues work on it. vhost_workers_free() however clears
>>>> the vq->worker pointers and immediately frees the workers, without
>>>> waiting for a grace period. A caller that fetched the worker right
>>>> before the pointer was cleared can therefore still be queueing work on
>>>> it while it is freed. And even when the queueing itself wins the race,
>>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>>>> future attempts to queue it are silently skipped.
>>>>
>>>> None of the current callers can actually hit this: net and scsi stop
>>>> their virtqueues before the workers are freed, and vsock unhashes the
>>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>>>> in vhost-vsock keeps the device hashed while its workers are freed, so
>>>> the lockless send/cancel paths become able to race with the teardown.
>>>>
>>>> Fix this by clearing the vq->worker pointers, waiting for a grace
>>>> period, and then flushing the workers so any work the last readers
>>>> queued runs before the workers are freed.
>>>
>>>
>>>
>>>
>>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>> ---
>>>> drivers/vhost/vhost.c | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>> index 4c525b3e16ea..d6e235c25254 100644
>>>> --- a/drivers/vhost/vhost.c
>>>> +++ b/drivers/vhost/vhost.c
>>>> @@ -729,6 +729,17 @@ static void vhost_workers_free(struct vhost_dev *dev)
>>>>
>>>> for (i = 0; i < dev->nvqs; i++)
>>>> rcu_assign_pointer(dev->vqs[i]->worker, NULL);
>>>> +
>>>> + /*
>>>> + * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
>>>> + * reader that fetched a worker before we cleared the pointers above
>>>> + * may still be queueing work on it. Wait for those readers to
>>>> + * finish, then flush so any work they queued runs (clearing
>>>> + * VHOST_WORK_QUEUED) before the workers are freed.
>>>> + */
>>>> + synchronize_rcu();
>>>
>>>
>>>
>>> Any way not to add this for all devices that don't need it?
>>> Or preferably, even for vsock in absense of the new ioctl?
>>>
>>
>> This code was initially local to vsock, and was moved here in v3->v4
>> after we discussed with Stefano that the issue looks more generic and
>> should probably be fixed in vhost.c (see
>> https://lore.kernel.org/virtualization/akO6tps94iFxCAWv@sgarzare-redhat).
>>
>> As a compromise, we can keep this code here, but only call it
>> conditionally. Namely, create a bool flag on 'struct vhost_dev' which
>> is always false, only set it to true on RESET_OWNER, and only call this
>> code once it's set. Clumsy, but this way no other code path would have
>> to wait the full grace period.
>>
>> What are your thoughts on that?
>>
>> Thanks,
>> Andrey
>
> Or just thread a bool parameter to it?
>
Yep, that should work as well. So we'll have to do:
vhost_dev_cleanup(struct vhost_dev *dev, bool sync)
vhost_workers_free(struct vhost_dev *dev, bool sync)
if (sync) {
synchronize_rcu();
vhost_dev_flush(dev);
}
And then vhost_dev_reset_owner() will call vhost_dev_cleanup() with
sync=true, while everybody else (net_release, scsi_release, even
vhost_vsock_dev_release) will call it with sync=false.
I'll squash it all into the same 4th commit and resend, unless you have
any objections.
Thanks,
Andrey
>>>
>>>> + vhost_dev_flush(dev);
>>>> +
>>>> /*
>>>> * Free the default worker we created and cleanup workers userspace
>>>> * created but couldn't clean up (it forgot or crashed).
>>>> --
>>>> 2.47.1
>>>
>
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Michael S. Tsirkin @ 2026-07-23 16:50 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, sgarzare, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <0c7172c4-0dc4-4d7e-9310-4f045e27efb9@virtuozzo.com>
On Thu, Jul 23, 2026 at 07:46:58PM +0300, Andrey Drobyshev wrote:
> On 7/23/26 6:18 PM, Michael S. Tsirkin wrote:
> > On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
> >> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
> >> vq->worker and queues work on it. vhost_workers_free() however clears
> >> the vq->worker pointers and immediately frees the workers, without
> >> waiting for a grace period. A caller that fetched the worker right
> >> before the pointer was cleared can therefore still be queueing work on
> >> it while it is freed. And even when the queueing itself wins the race,
> >> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
> >> future attempts to queue it are silently skipped.
> >>
> >> None of the current callers can actually hit this: net and scsi stop
> >> their virtqueues before the workers are freed, and vsock unhashes the
> >> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
> >> before the workers go away. But the upcoming VHOST_RESET_OWNER support
> >> in vhost-vsock keeps the device hashed while its workers are freed, so
> >> the lockless send/cancel paths become able to race with the teardown.
> >>
> >> Fix this by clearing the vq->worker pointers, waiting for a grace
> >> period, and then flushing the workers so any work the last readers
> >> queued runs before the workers are freed.
> >
> >
> >
> >
> >> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
> >> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> >> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> >> ---
> >> drivers/vhost/vhost.c | 11 +++++++++++
> >> 1 file changed, 11 insertions(+)
> >>
> >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >> index 4c525b3e16ea..d6e235c25254 100644
> >> --- a/drivers/vhost/vhost.c
> >> +++ b/drivers/vhost/vhost.c
> >> @@ -729,6 +729,17 @@ static void vhost_workers_free(struct vhost_dev *dev)
> >>
> >> for (i = 0; i < dev->nvqs; i++)
> >> rcu_assign_pointer(dev->vqs[i]->worker, NULL);
> >> +
> >> + /*
> >> + * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
> >> + * reader that fetched a worker before we cleared the pointers above
> >> + * may still be queueing work on it. Wait for those readers to
> >> + * finish, then flush so any work they queued runs (clearing
> >> + * VHOST_WORK_QUEUED) before the workers are freed.
> >> + */
> >> + synchronize_rcu();
> >
> >
> >
> > Any way not to add this for all devices that don't need it?
> > Or preferably, even for vsock in absense of the new ioctl?
> >
>
> This code was initially local to vsock, and was moved here in v3->v4
> after we discussed with Stefano that the issue looks more generic and
> should probably be fixed in vhost.c (see
> https://lore.kernel.org/virtualization/akO6tps94iFxCAWv@sgarzare-redhat).
>
> As a compromise, we can keep this code here, but only call it
> conditionally. Namely, create a bool flag on 'struct vhost_dev' which
> is always false, only set it to true on RESET_OWNER, and only call this
> code once it's set. Clumsy, but this way no other code path would have
> to wait the full grace period.
>
> What are your thoughts on that?
>
> Thanks,
> Andrey
Or just thread a bool parameter to it?
> >
> >> + vhost_dev_flush(dev);
> >> +
> >> /*
> >> * Free the default worker we created and cleanup workers userspace
> >> * created but couldn't clean up (it forgot or crashed).
> >> --
> >> 2.47.1
> >
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-23 16:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, kvm, virtualization, netdev, sgarzare, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260723111557-mutt-send-email-mst@kernel.org>
On 7/23/26 6:18 PM, Michael S. Tsirkin wrote:
> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>> vq->worker and queues work on it. vhost_workers_free() however clears
>> the vq->worker pointers and immediately frees the workers, without
>> waiting for a grace period. A caller that fetched the worker right
>> before the pointer was cleared can therefore still be queueing work on
>> it while it is freed. And even when the queueing itself wins the race,
>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>> future attempts to queue it are silently skipped.
>>
>> None of the current callers can actually hit this: net and scsi stop
>> their virtqueues before the workers are freed, and vsock unhashes the
>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>> in vhost-vsock keeps the device hashed while its workers are freed, so
>> the lockless send/cancel paths become able to race with the teardown.
>>
>> Fix this by clearing the vq->worker pointers, waiting for a grace
>> period, and then flushing the workers so any work the last readers
>> queued runs before the workers are freed.
>
>
>
>
>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> drivers/vhost/vhost.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>> index 4c525b3e16ea..d6e235c25254 100644
>> --- a/drivers/vhost/vhost.c
>> +++ b/drivers/vhost/vhost.c
>> @@ -729,6 +729,17 @@ static void vhost_workers_free(struct vhost_dev *dev)
>>
>> for (i = 0; i < dev->nvqs; i++)
>> rcu_assign_pointer(dev->vqs[i]->worker, NULL);
>> +
>> + /*
>> + * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
>> + * reader that fetched a worker before we cleared the pointers above
>> + * may still be queueing work on it. Wait for those readers to
>> + * finish, then flush so any work they queued runs (clearing
>> + * VHOST_WORK_QUEUED) before the workers are freed.
>> + */
>> + synchronize_rcu();
>
>
>
> Any way not to add this for all devices that don't need it?
> Or preferably, even for vsock in absense of the new ioctl?
>
This code was initially local to vsock, and was moved here in v3->v4
after we discussed with Stefano that the issue looks more generic and
should probably be fixed in vhost.c (see
https://lore.kernel.org/virtualization/akO6tps94iFxCAWv@sgarzare-redhat).
As a compromise, we can keep this code here, but only call it
conditionally. Namely, create a bool flag on 'struct vhost_dev' which
is always false, only set it to true on RESET_OWNER, and only call this
code once it's set. Clumsy, but this way no other code path would have
to wait the full grace period.
What are your thoughts on that?
Thanks,
Andrey
>
>> + vhost_dev_flush(dev);
>> +
>> /*
>> * Free the default worker we created and cleanup workers userspace
>> * created but couldn't clean up (it forgot or crashed).
>> --
>> 2.47.1
>
^ permalink raw reply
* [PATCH] vhost: clear vq->worker under vq->mutex when freeing workers
From: Andrey Drobyshev @ 2026-07-23 15:33 UTC (permalink / raw)
To: linux-kernel
Cc: kvm, virtualization, netdev, sgarzare, mst, stefanha, jasowangio,
eperezma, andrey.drobyshev
Every other update of vq->worker is done under vq->mutex - the worker
attach/swap ioctls and vhost_worker_killed(). vhost_workers_free() is
the sole exception: it clears vq->worker without holding the lock.
The effect is harmless in practice, as this only happens while the
owning process (and thus the whole device) is dying, but the lockless
write is inconsistent with the rest of the code. Clear vq->worker under
vq->mutex, like everyone else, so that all writers of vq->worker follow
the same locking rule.
This issue was found by Sashiko AI review.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
drivers/vhost/vhost.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 4c525b3e16ea..dbb6cb5eccea 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -722,13 +722,19 @@ static void vhost_worker_destroy(struct vhost_dev *dev,
static void vhost_workers_free(struct vhost_dev *dev)
{
struct vhost_worker *worker;
+ struct vhost_virtqueue *vq;
unsigned long i;
if (!dev->use_worker)
return;
- for (i = 0; i < dev->nvqs; i++)
- rcu_assign_pointer(dev->vqs[i]->worker, NULL);
+ for (i = 0; i < dev->nvqs; i++) {
+ vq = dev->vqs[i];
+
+ mutex_lock(&vq->mutex);
+ rcu_assign_pointer(vq->worker, NULL);
+ mutex_unlock(&vq->mutex);
+ }
/*
* Free the default worker we created and cleanup workers userspace
* created but couldn't clean up (it forgot or crashed).
--
2.47.1
^ permalink raw reply related
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Michael S. Tsirkin @ 2026-07-23 15:18 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, sgarzare, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260720102241.371610-5-andrey.drobyshev@virtuozzo.com>
On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
> vq->worker and queues work on it. vhost_workers_free() however clears
> the vq->worker pointers and immediately frees the workers, without
> waiting for a grace period. A caller that fetched the worker right
> before the pointer was cleared can therefore still be queueing work on
> it while it is freed. And even when the queueing itself wins the race,
> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
> future attempts to queue it are silently skipped.
>
> None of the current callers can actually hit this: net and scsi stop
> their virtqueues before the workers are freed, and vsock unhashes the
> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
> before the workers go away. But the upcoming VHOST_RESET_OWNER support
> in vhost-vsock keeps the device hashed while its workers are freed, so
> the lockless send/cancel paths become able to race with the teardown.
>
> Fix this by clearing the vq->worker pointers, waiting for a grace
> period, and then flushing the workers so any work the last readers
> queued runs before the workers are freed.
> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> drivers/vhost/vhost.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 4c525b3e16ea..d6e235c25254 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -729,6 +729,17 @@ static void vhost_workers_free(struct vhost_dev *dev)
>
> for (i = 0; i < dev->nvqs; i++)
> rcu_assign_pointer(dev->vqs[i]->worker, NULL);
> +
> + /*
> + * vhost_vq_work_queue() reads vq->worker under rcu_read_lock(), so a
> + * reader that fetched a worker before we cleared the pointers above
> + * may still be queueing work on it. Wait for those readers to
> + * finish, then flush so any work they queued runs (clearing
> + * VHOST_WORK_QUEUED) before the workers are freed.
> + */
> + synchronize_rcu();
Any way not to add this for all devices that don't need it?
Or preferably, even for vsock in absense of the new ioctl?
> + vhost_dev_flush(dev);
> +
> /*
> * Free the default worker we created and cleanup workers userspace
> * created but couldn't clean up (it forgot or crashed).
> --
> 2.47.1
^ permalink raw reply
* Re: [PATCH] tools: Use fputc() calls in some functions
From: Segher Boessenkool @ 2026-07-23 15:14 UTC (permalink / raw)
To: Andi Kleen
Cc: Markus Elfring, acpica-devel, bpf, coresight, kvm, linux-acpi,
linux-arm-kernel, linux-gpio, linux-kselftest, linux-mm,
linux-perf-users, linux-pm, linux-trace-kernel, linuxppc-dev,
mptcp, netdev, platform-driver-x86, virtualization, Adrian Hunter,
Alex Mastro, Alex Williamson, Alexander Shishkin,
Alexei Starovoitov, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Ankur Arora, Arnaldo Carvalho de Melo, Bartosz Golaszewski,
Bobby Eshleman, Christian Bornträger, Christophe Leroy,
Chun-Tse Shao, Claudio Imbrenda, Costa Shulyupin, Crystal Wood,
Daniel Borkmann, Daniel Lezcano, David Hildenbrand, David Matlack,
David S. Miller, Donald Hunter, Eduard Zingerman, Emil Tsalapatis,
Eric Dumazet, Gabriele Monaco, Geliang Tang, Ian Rogers,
Ingo Molnar, Ivan Pravdin, Jakub Kicinski, James Clark,
Janosch Frank, Jason Xing, Jiri Olsa, Joe Damato, John Garry,
Josh Poimboeuf, Kaushlendra Kumar, Kent Gibson,
Kumar Kartikeya Dwivedi, Len Brown, Leo Yan, Lukasz Luba,
Madhavan Srinivasan, Mark Rutland, Martin KaFai Lau,
Masami Hiramatsu, Mat Martineau, Matthieu Baerts,
Michael Ellerman, Mike Leach, Mina Almasry, Mykyta Yatsenko,
Nam Cao, Namhyung Kim, Nicholas Piggin, Paolo Abeni,
Paolo Bonzini, Pawel Chmielewski, Peter Zijlstra, Quentin Monnet,
Rafael J. Wysocki, Raghavendra Rao Ananta, Saket Dumbre,
Shuah Khan, Simon Horman, Song Liu, Srinivas Pandruvada,
Stanislav Fomichev, Stefano Garzarella, Steven Rostedt,
Suzuki K Poulose, Swapnil Sapkal, Thomas Weißschuh,
Tomas Glozar, Vipin Sharma, Wander Lairson Costa, Will Deacon,
Willem de Bruijn, Willy Tarreau, Yonghong Song, Zhang Chujun,
Zhang Rui, LKML, kernel-janitors
In-Reply-To: <amIbVOIsOeZgcBZF@tassilo>
On Thu, Jul 23, 2026 at 06:47:00AM -0700, Andi Kleen wrote:
> On Thu, Jul 23, 2026 at 12:25:25PM +0200, Markus Elfring wrote:
> > From: Markus Elfring <elfring@users.sourceforge.net>
> > Date: Thu, 23 Jul 2026 11:13:30 +0200
> > Subject: [PATCH] tools: Use fputc() calls in some functions
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > Single characters should occasionally be transferred into the output.
> > Thus use the corresponding function “fputc” (or even “putchar”).
> >
> > The source code was transformed by using the Coccinelle software.
>
> gcc already does this transformation automatically.
Yup. And the printf() form is much more regular, expected, readable.
The primary consumers of source code are human beings, all programs
should be written with that in mind!
Segher
^ permalink raw reply
* Re: [PATCH] tools: Use fputc() calls in some functions
From: bot+bpf-ci @ 2026-07-23 15:07 UTC (permalink / raw)
To: Markus.Elfring, acpica-devel, bpf, coresight, kvm, linux-acpi,
linux-arm-kernel, linux-gpio, linux-kselftest, linux-mm,
linux-perf-users, linux-pm, linux-trace-kernel, linuxppc-dev,
mptcp, netdev, platform-driver-x86, virtualization, adrian.hunter,
amastro, alex, alexander.shishkin, ast, ak, andrew+netdev, akpm,
andrii, ankur.a.arora, acme, brgl, bobbyeshleman, borntraeger,
chleroy, ctshao, imbrenda, costa.shul, crwood, daniel,
daniel.lezcano, david, dmatlack, davem
Cc: linux-kernel, kernel-janitors, ast, andrii, daniel, martin.lau,
eddyz87, yonghong.song, clm, ihor.solodrai
In-Reply-To: <48f2d61f-d468-49b7-881e-f00777db073f@web.de>
[-- Attachment #1: Type: text/plain, Size: 28649 bytes --]
> diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
> --- a/tools/bpf/bpftool/common.c
> +++ b/tools/bpf/bpftool/common.c
> @@ -53,7 +53,7 @@ void p_err(const char *fmt, ...)
> } else {
> fprintf(stderr, "Error: ");
> vfprintf(stderr, fmt, ap);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
^^^^^^
The fputc() arguments are reversed. The C standard signature is
`int fputc(int c, FILE *stream)` - the character comes first, the stream
second. This passes the FILE* stderr as the int c parameter and '\n' as
the FILE* stream parameter.
Correct form is `fputc('\n', stderr)`.
This constraint violation triggers a -Werror=int-conversion diagnostic and
breaks the build. If it compiled, at runtime fputc would dereference
(FILE*)10, crashing the program.
The same argument reversal occurs in 67 conversions across this patch.
> @@ -67,7 +67,7 @@ void p_info(const char *fmt, ...)
>
> va_start(ap, fmt);
> vfprintf(stderr, fmt, ap);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
^^^^^^
Same reversed-argument bug.
> diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
> --- a/tools/bpf/bpftool/main.c
> +++ b/tools/bpf/bpftool/main.c
> @@ -273,7 +273,7 @@ void fprint_hex(FILE *f, void *arg, unsigned int n, const char *sep)
> if (!i)
> /* nothing */;
> else if (!(i % 16))
> - fprintf(f, "\n");
> + fputc(f, '\n');
Arguments reversed: `fputc(f, '\n')` should be `fputc('\n', f)`.
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -1266,7 +1266,7 @@ static void hex_print(void *data, unsigned int size, FILE *f)
> fprintf(f, "%c%s", c, j == i + 7 ? " " : "");
> }
>
> - fprintf(f, "\n");
> + fputc(f, '\n');
Arguments reversed.
> diff --git a/tools/include/nolibc/err.h b/tools/include/nolibc/err.h
> --- a/tools/include/nolibc/err.h
> +++ b/tools/include/nolibc/err.h
> @@ -27,7 +27,7 @@ void vwarnx(const char *fmt, va_list args)
> {
> fprintf(stderr, "%s: ", program_invocation_short_name);
> vfprintf(stderr, fmt, args);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed. This is in a nolibc header - nolibc's fputc
(tools/include/nolibc/stdio.h) does `write(fileno(stream), &ch, 1)`.
With stream==(FILE*)10 fileno dereferences a bogus pointer and crashes.
> diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
> --- a/tools/mm/page_owner_sort.c
> +++ b/tools/mm/page_owner_sort.c
> @@ -867,7 +867,7 @@ struct option longopts[] = {
> }
> if (cull & CULL_STACKTRACE)
> fprintf(fout, ":\n%s", list[i].stacktrace);
> - fprintf(fout, "\n");
> + fputc(fout, '\n');
Arguments reversed: should be `fputc('\n', fout)`.
> diff --git a/tools/net/ynl/ynltool/main.c b/tools/net/ynl/ynltool/main.c
> --- a/tools/net/ynl/ynltool/main.c
> +++ b/tools/net/ynl/ynltool/main.c
> @@ -156,7 +156,7 @@ void p_err(const char *fmt, ...)
> } else {
> fprintf(stderr, "Error: ");
> vfprintf(stderr, fmt, ap);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> }
> va_end(ap);
> }
> @@ -170,7 +170,7 @@ void p_info(const char *fmt, ...)
>
> va_start(ap, fmt);
> vfprintf(stderr, fmt, ap);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Both conversions reverse the arguments.
> diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
> --- a/tools/objtool/builtin-check.c
> +++ b/tools/objtool/builtin-check.c
> @@ -297,7 +297,7 @@ int make_backup(void)
> fprintf(stderr, " %s", arg);
> }
>
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed. objtool runs during the kernel build, so a build
failure or crash here breaks the build.
> diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
> --- a/tools/objtool/disas.c
> +++ b/tools/objtool/disas.c
> @@ -537,7 +537,7 @@ void disas_print_insn(FILE *stream, struct disas_context *dctx,
> return;
>
> if (strcmp(format, "\n") == 0) {
> - fprintf(stream, "\n");
> + fputc(stream, '\n');
Arguments reversed.
> diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
> --- a/tools/perf/builtin-c2c.c
> +++ b/tools/perf/builtin-c2c.c
> @@ -2511,7 +2511,7 @@ struct perf_hpp hpp = {
> hists__fprintf_headers(&c2c_hists->hists, out);
> once = true;
> } else {
> - fprintf(out, "\n");
> + fputc(out, '\n');
> }
[ ... ]
> @@ -2590,15 +2590,15 @@ static void perf_c2c__hists_fprintf(FILE *out, struct perf_session *session)
> setup_pager();
>
> print_c2c__display_stats(out);
> - fprintf(out, "\n");
> + fputc(out, '\n');
> print_shared_cacheline_info(out);
> - fprintf(out, "\n");
> + fputc(out, '\n');
> print_c2c_info(out, session);
>
> if (c2c.stats_only)
> return;
>
> - fprintf(out, "\n");
> + fputc(out, '\n');
[ ... ]
> @@ -2606,7 +2606,7 @@ static void perf_c2c__hists_fprintf(FILE *out, struct perf_session *session)
>
> hists__fprintf(&c2c.hists.hists, true, 0, 0, 0, stdout, true);
>
> - fprintf(out, "\n");
> + fputc(out, '\n');
All four conversions reverse the arguments.
> diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
> --- a/tools/perf/builtin-daemon.c
> +++ b/tools/perf/builtin-daemon.c
> @@ -691,7 +691,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
> /* session up time */
> csv_sep, (uint64_t)((curr - daemon->start) / 60));
>
> - fprintf(out, "\n");
> + fputc(out, '\n');
> } else {
[ ... ]
> @@ -730,7 +730,7 @@
> /* session up time */
> csv_sep, (uint64_t)((curr - session->start) / 60));
>
> - fprintf(out, "\n");
> + fputc(out, '\n');
Both conversions reverse the arguments.
> diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
> --- a/tools/perf/builtin-lock.c
> +++ b/tools/perf/builtin-lock.c
> @@ -1319,9 +1319,9 @@ static void print_result(void)
>
> list_for_each_entry(key, &lock_keys, list) {
> key->print(key, st);
> - fprintf(lock_output, " ");
> + fputc(lock_output, ' ');
> }
> - fprintf(lock_output, "\n");
> + fputc(lock_output, '\n');
Both `fputc(lock_output, ' ')` and `fputc(lock_output, '\n')` reverse the
arguments. Correct form: `fputc(' ', lock_output)` and
`fputc('\n', lock_output)`.
> @@ -1598,7 +1598,7 @@ static void print_header_csv(const char *sep)
> fprintf(lock_output, "%s%s %s", "type", sep, "caller");
> if (verbose > 0)
> fprintf(lock_output, "%s %s", sep, "stacktrace");
> - fprintf(lock_output, "\n");
> + fputc(lock_output, '\n');
[ ... ]
> @@ -1629,7 +1629,7 @@ static void print_lock_stat_stdio(struct lock_contention *con, struct lock_stat *st)
>
> list_for_each_entry(key, &lock_keys, list) {
> key->print(key, st);
> - fprintf(lock_output, " ");
> + fputc(lock_output, ' ');
> }
[ ... ]
> @@ -1687,7 +1687,7 @@ static void print_lock_stat_csv(struct lock_contention *con, struct lock_stat *st,
> case LOCK_AGGR_CALLER:
> fprintf(lock_output, "%s%s %s", get_type_flags_name(st->flags), sep, st->name);
> if (verbose <= 0)
> - fprintf(lock_output, "\n");
> + fputc(lock_output, '\n');
[ ... ]
> @@ -1721,7 +1721,7 @@ static void print_lock_stat_csv(struct lock_contention *con, struct lock_stat *st,
> get_symbol_name_offset(kmap, sym, ip, buf, sizeof(buf));
> fprintf(lock_output, "%s %#lx %s", i ? ":" : sep, (unsigned long) ip, buf);
> }
> - fprintf(lock_output, "\n");
> + fputc(lock_output, '\n');
> }
> }
[ ... ]
> @@ -1781,7 +1781,7 @@ static void print_footer_csv(int total, int bad, struct lock_contention_fails *fails,
> for (i = 0; i < BROKEN_MAX; i++)
> fprintf(lock_output, "%s bad_%s=%d", sep, name[i], bad_hist[i]);
> }
> - fprintf(lock_output, "\n");
> + fputc(lock_output, '\n');
All six conversions in builtin-lock.c reverse the arguments.
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -2561,7 +2561,7 @@
>
> perf_sample__fprintf_ipc(sample, evsel, fp);
>
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
[ ... ]
> @@ -2815,7 +2815,7 @@
> cursor, symbol_conf.bt_stop_list, fp);
> }
>
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
[ ... ]
> @@ -3271,11 +3271,11 @@ static int list_available_languages_cb(struct scripting_ops *ops, const char *spec)
>
> static void list_available_languages(void)
> {
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> fprintf(stderr, "Scripting language extensions (used in "
> "perf script -s [spec:]script.[spec]):\n\n");
> script_spec__for_each(&list_available_languages_cb);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
All four conversions reverse the arguments.
> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
> --- a/tools/perf/ui/gtk/util.c
> +++ b/tools/perf/ui/gtk/util.c
> @@ -37,7 +37,7 @@ static int perf_gtk__error(const char *format, va_list args)
> vasprintf(&msg, format, args) < 0) {
> fprintf(stderr, "Error:\n");
> vfprintf(stderr, format, args);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> return -1;
> }
>
> @@ -62,7 +62,7 @@ static int perf_gtk__warning_info_bar(const char *format, va_list args)
> vasprintf(&msg, format, args) < 0) {
> fprintf(stderr, "Warning:\n");
> vfprintf(stderr, format, args);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> return -1;
> }
>
> @@ -83,7 +83,7 @@ static int perf_gtk__warning_statusbar(const char *format, va_list args)
> vasprintf(&msg, format, args) < 0) {
> fprintf(stderr, "Warning:\n");
> vfprintf(stderr, format, args);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
All three conversions reverse the arguments.
> diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
> --- a/tools/perf/ui/stdio/hist.c
> +++ b/tools/perf/ui/stdio/hist.c
> @@ -691,7 +691,7 @@
> }
>
> next_line:
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
> }
[ ... ]
> @@ -783,7 +783,7 @@ hists__fprintf_standard_headers(struct hists *hists,
> if (line)
> fprintf(fp, "# ");
> fprintf_line(hists, hpp, line, fp);
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
> }
[ ... ]
> @@ -806,10 +806,10 @@ hists__fprintf_standard_headers(struct hists *hists,
>
> width = fmt->width(fmt, hpp, hists);
> for (i = 0; i < width; i++)
> - fprintf(fp, ".");
> + fputc(fp, '.');
> }
>
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
All four conversions reverse the arguments.
> diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
> --- a/tools/perf/util/debug.c
> +++ b/tools/perf/util/debug.c
> @@ -350,7 +350,7 @@ void __dump_stack(FILE *file, void **stackdump, size_t stackdump_size)
> fprintf(file, " #%zd %p ", i, stackdump[i]);
>
> map__fprintf_srcline(al.map, al.addr, "", file);
> - fprintf(file, "\n");
> + fputc(file, '\n');
Arguments reversed.
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -2454,7 +2454,7 @@ static void __print_pmu_caps(FILE *fp, int nr_caps, char **caps, char *pmu_name)
> delimiter = ", ";
> }
>
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
> }
[ ... ]
> @@ -2513,7 +2513,7 @@ static void print_pmu_mappings(struct feat_fd *ff, FILE *fp)
> pmu_num--;
> }
>
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
[ ... ]
> @@ -4349,7 +4349,7 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
> fprintf(fp, "%s ", feat_ops[bit].name);
> }
>
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
All three conversions reverse the arguments.
> diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-log.c b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
> --- a/tools/perf/util/intel-pt-decoder/intel-pt-log.c
> +++ b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
> @@ -70,14 +70,14 @@ static void intel_pt_print_data(const unsigned char *buf, int len, uint64_t pos,
> int i;
>
> for (i = 0; i < indent; i++)
> - fprintf(f, " ");
> + fputc(f, ' ');
>
> fprintf(f, " %08" PRIx64 ": ", pos);
> for (i = 0; i < len; i++)
> fprintf(f, " %02x", buf[i]);
> for (; i < 16; i++)
> fprintf(f, " ");
> - fprintf(f, " ");
> + fputc(f, ' ');
> }
>
> static void intel_pt_print_no_data(uint64_t pos, int indent)
> @@ -85,12 +85,12 @@ static void intel_pt_print_no_data(uint64_t pos, int indent)
> int i;
>
> for (i = 0; i < indent; i++)
> - fprintf(f, " ");
> + fputc(f, ' ');
>
> fprintf(f, " %08" PRIx64 ": ", pos);
> for (i = 0; i < 16; i++)
> fprintf(f, " ");
> - fprintf(f, " ");
> + fputc(f, ' ');
> }
All four conversions reverse the arguments.
> diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
> --- a/tools/perf/util/libbfd.c
> +++ b/tools/perf/util/libbfd.c
> @@ -603,7 +603,7 @@
> } else
> srcline = NULL;
>
> - fprintf(s, "\n");
> + fputc(s, '\n');
Arguments reversed. The variable s is an open_memstream() stream.
> diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
> --- a/tools/perf/util/stat-display.c
> +++ b/tools/perf/util/stat-display.c
> @@ -529,7 +529,7 @@ static void print_metric_json(struct perf_stat_config *config __maybe_unused,
> }
> }
> if (!config->metric_only)
> - fprintf(out, "}");
> + fputc(out, '}');
> }
[ ... ]
> @@ -1316,7 +1316,7 @@ static void print_header_std(struct perf_stat_config *config,
> FILE *output = config->output;
> int i;
>
> - fprintf(output, "\n");
> + fputc(output, '\n');
> fprintf(output, " Performance counter stats for ");
[ ... ]
> @@ -1333,7 +1333,7 @@ static void print_header_std(struct perf_stat_config *config,
> else
> fprintf(output, "thread id \'%s", _target->tid);
>
> - fprintf(output, "\'");
> + fputc(output, '\'');
[ ... ]
> @@ -1406,9 +1406,9 @@ static void print_table(struct perf_stat_config *config, FILE *output, double avg)
> fprintf(output, " %17.9f (%+.9f) ", run, run - avg);
>
> for (h = 0; h < n; h++)
> - fprintf(output, "#");
> + fputc(output, '#');
>
> - fprintf(output, "\n");
> + fputc(output, '\n');
> }
[ ... ]
> @@ -1428,7 +1428,7 @@ static void print_footer(struct perf_stat_config *config)
> return;
>
> if (!config->null_run)
> - fprintf(output, "\n");
> + fputc(output, '\n');
All six conversions reverse the arguments.
> diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
> --- a/tools/perf/util/values.c
> +++ b/tools/perf/util/values.c
> @@ -217,7 +217,7 @@ static void perf_read_values__display_pretty(FILE *fp,
> fprintf(fp, "# %*s %*s", pidwidth, "PID", tidwidth, "TID");
> for (j = 0; j < values->num_counters; j++)
> fprintf(fp, " %*s", counterwidth[j], evsel__name(values->counters[j]));
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
[ ... ]
> @@ -225,7 +225,7 @@ static void perf_read_values__display_pretty(FILE *fp,
> for (j = 0; j < values->num_counters; j++)
> fprintf(fp, " %*" PRIu64,
> counterwidth[j], values->value[i][j]);
> - fprintf(fp, "\n");
> + fputc(fp, '\n');
Both conversions reverse the arguments.
> diff --git a/tools/power/acpi/tools/acpidump/apdump.c b/tools/power/acpi/tools/acpidump/apdump.c
> --- a/tools/power/acpi/tools/acpidump/apdump.c
> +++ b/tools/power/acpi/tools/acpidump/apdump.c
> @@ -171,7 +171,7 @@ ap_dump_table_buffer(struct acpi_table_header *table,
> acpi_ut_dump_buffer_to_file(gbl_output_file,
> ACPI_CAST_PTR(u8, table), table_length,
> DB_BYTE_DISPLAY, 0);
> - fprintf(gbl_output_file, "\n");
> + fputc(gbl_output_file, '\n');
Arguments reversed.
> diff --git a/tools/power/x86/intel-speed-select/isst-display.c b/tools/power/x86/intel-speed-select/isst-display.c
> --- a/tools/power/x86/intel-speed-select/isst-display.c
> +++ b/tools/power/x86/intel-speed-select/isst-display.c
> @@ -122,7 +122,7 @@ static void format_and_print(FILE *outf, int level, char *header, char *value)
>
> if (level == 0) {
> if (header)
> - fprintf(outf, "{");
> + fputc(outf, '{');
[ ... ]
> @@ -138,7 +138,7 @@ static void format_and_print(FILE *outf, int level, char *header, char *value)
>
> if (value) {
> if (last_level != level)
> - fprintf(outf, "\n");
> + fputc(outf, '\n');
[ ... ]
> @@ -156,7 +156,7 @@ static void format_and_print(FILE *outf, int level, char *header, char *value)
> fprintf(outf, "\n%s}", delimiters);
> }
> if (abs(last_level - level) < 3)
> - fprintf(outf, "\n");
> + fputc(outf, '\n');
All three conversions reverse the arguments.
> diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
> --- a/tools/power/x86/turbostat/turbostat.c
> +++ b/tools/power/x86/turbostat/turbostat.c
> @@ -9099,7 +9099,7 @@ void dump_cpuid_hypervisor(void)
> dump_word_chars(ebx);
> dump_word_chars(ecx);
> dump_word_chars(edx);
> - fprintf(outf, "\n");
> + fputc(outf, '\n');
> }
[ ... ]
> @@ -9749,7 +9749,7 @@
> fprintf(outf, " siblings");
> for (ht_id = 0; ht_id <= MAX_HT_ID; ++ht_id)
> fprintf(outf, " %d", cpus[i].ht_sibling_cpu_id[ht_id]);
> - fprintf(outf, "\n");
> + fputc(outf, '\n');
Both conversions reverse the arguments.
> diff --git a/tools/testing/selftests/bpf/benchs/bench_htab_mem.c b/tools/testing/selftests/bpf/benchs/bench_htab_mem.c
> --- a/tools/testing/selftests/bpf/benchs/bench_htab_mem.c
> +++ b/tools/testing/selftests/bpf/benchs/bench_htab_mem.c
> @@ -148,7 +148,7 @@ static const struct htab_mem_use_case *htab_mem_find_use_case_or_exit(const char *name)
> fprintf(stderr, "available use case:");
> for (i = 0; i < ARRAY_SIZE(use_cases); i++)
> fprintf(stderr, " %s", use_cases[i].name);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/testing/selftests/bpf/jit_disasm_helpers.c b/tools/testing/selftests/bpf/jit_disasm_helpers.c
> --- a/tools/testing/selftests/bpf/jit_disasm_helpers.c
> +++ b/tools/testing/selftests/bpf/jit_disasm_helpers.c
> @@ -228,7 +228,7 @@ struct bpf_prog_info info = {};
> for (pc = 0, i = 0; i < jited_funcs; ++i) {
> fprintf(text_out, "func #%d:\n", i);
> disasm_one_func(text_out, image + pc, func_lens[i]);
> - fprintf(text_out, "\n");
> + fputc(text_out, '\n');
Arguments reversed.
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -383,7 +383,7 @@ static void print_test_result(const struct prog_test_def *test, const struct test_state *test_state)
> else
> fprintf(env.stdout_saved, "OK (SKIP: %d/%d)", skipped_cnt, subtests_cnt);
>
> - fprintf(env.stdout_saved, "\n");
> + fputc(env.stdout_saved, '\n');
> }
[ ... ]
> @@ -391,7 +391,7 @@ static void print_test_log(char *log_buf, size_t log_cnt)
> log_buf[log_cnt] = '\0';
> fprintf(env.stdout_saved, "%s", log_buf);
> if (log_buf[log_cnt - 1] != '\n')
> - fprintf(env.stdout_saved, "\n");
> + fputc(env.stdout_saved, '\n');
> }
[ ... ]
> @@ -409,7 +409,7 @@ static void print_subtest_name(int test_num, int subtest_num,
> if (result)
> fprintf(env.stdout_saved, ":%s", result);
>
> - fprintf(env.stdout_saved, "\n");
> + fputc(env.stdout_saved, '\n');
All three conversions reverse the arguments.
> diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
> --- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c
> +++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
> @@ -135,7 +135,7 @@ static void pr_err(const char *fmt, ...)
>
> if (errno != 0)
> fprintf(stderr, ": %s", strerror(errno));
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/testing/selftests/kvm/s390/keyop.c b/tools/testing/selftests/kvm/s390/keyop.c
> --- a/tools/testing/selftests/kvm/s390/keyop.c
> +++ b/tools/testing/selftests/kvm/s390/keyop.c
> @@ -117,7 +117,7 @@ static void dump_sk(const unsigned char skeys[], const char *descr)
> fprintf(stderr, "# %3d: ", i);
> for (j = 0; j < 32; j++)
> fprintf(stderr, "%02x ", skeys[i + j]);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> --- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> +++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> @@ -208,7 +208,7 @@
> }
> if (server_side)
> fprintf(stderr, ",server_side:1");
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/testing/selftests/net/psock_tpacket.c b/tools/testing/selftests/net/psock_tpacket.c
> --- a/tools/testing/selftests/net/psock_tpacket.c
> +++ b/tools/testing/selftests/net/psock_tpacket.c
> @@ -116,7 +116,7 @@ static int pfsocket(int ver)
> static void status_bar_update(void)
> {
> if (total_packets % 10 == 0) {
> - fprintf(stderr, ".");
> + fputc(stderr, '.');
[ ... ]
> @@ -825,7 +825,7 @@ static int test_tpacket(int version, int type)
> unmap_ring(sock, &ring);
> close(sock);
>
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Both conversions reverse the arguments.
> diff --git a/tools/testing/selftests/net/txtimestamp.c b/tools/testing/selftests/net/txtimestamp.c
> --- a/tools/testing/selftests/net/txtimestamp.c
> +++ b/tools/testing/selftests/net/txtimestamp.c
> @@ -200,10 +200,10 @@ static void __print_timestamp(const char *name, struct timespec *cur,
> ts_delta = timespec_to_ns64(cur) - timespec_to_ns64(&ts_usr);
> fprintf(stderr, " (USR +");
> __print_ts_delta_formatted(ts_delta);
> - fprintf(stderr, ")");
> + fputc(stderr, ')');
> }
>
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> }
[ ... ]
> @@ -254,7 +254,7 @@ static void print_timing_event(char *name, struct timing_event *te)
> __print_ts_delta_formatted(te->min);
> fprintf(stderr, ", max=");
> __print_ts_delta_formatted(te->max);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> }
[ ... ]
> @@ -271,7 +271,7 @@ static void print_payload(char *data, int len)
> fprintf(stderr, "payload: ");
> for (i = 0; i < len; i++)
> fprintf(stderr, "%02hhx ", data[i]);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> }
[ ... ]
> @@ -933,7 +933,7 @@ int main(int argc, char **argv)
> fprintf(stderr, "protocol: %s\n", sock_names[cfg_proto]);
> fprintf(stderr, "payload: %u\n", cfg_payload_len);
> fprintf(stderr, "server port: %u\n", dest_port);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
All four conversions reverse the arguments.
> diff --git a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
> --- a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
> +++ b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
> @@ -363,7 +363,7 @@
> goto err3;
> fprintf(stderr, "%02x ", tmp[i]);
> if (i == 5)
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/assert.h b/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
> --- a/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
> +++ b/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
> @@ -10,7 +10,7 @@ #define SELFTESTS_VFIO_LIB_INCLUDE_LIBVFIO_ASSERT_H
>
> #define VFIO_LOG_AND_EXIT(...) do { \
> fprintf(stderr, " " __VA_ARGS__); \
> - fprintf(stderr, "\n"); \
> + fputc(stderr, '\n'); \
Arguments reversed in a widely-used macro. Every expansion of this macro
is broken.
> diff --git a/tools/testing/selftests/vfio/lib/libvfio.c b/tools/testing/selftests/vfio/lib/libvfio.c
> --- a/tools/testing/selftests/vfio/lib/libvfio.c
> +++ b/tools/testing/selftests/vfio/lib/libvfio.c
> @@ -60,16 +60,16 @@ char **vfio_selftests_get_bdfs(int *argc, char *argv[], int *nr_bdfs)
> }
>
> fprintf(stderr, "Unable to determine which device(s) to use, skipping test.\n");
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> fprintf(stderr, "To pass the device address via environment variable:\n");
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> fprintf(stderr, " export VFIO_SELFTESTS_BDF=\"segment:bus:device.function\"\n");
> fprintf(stderr, " %s [options]\n", argv[0]);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> fprintf(stderr, "To pass the device address(es) via argv:\n");
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
> fprintf(stderr, " %s [options] segment:bus:device.function ...\n", argv[0]);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
All five conversions reverse the arguments.
> diff --git a/tools/testing/vsock/vsock_diag_test.c b/tools/testing/vsock/vsock_diag_test.c
> --- a/tools/testing/vsock/vsock_diag_test.c
> +++ b/tools/testing/vsock/vsock_diag_test.c
> @@ -86,7 +86,7 @@ static void print_vsock_addr(FILE *fp, unsigned int cid, unsigned int port)
> fprintf(fp, "%u:", cid);
>
> if (port == VMADDR_PORT_ANY)
> - fprintf(fp, "*");
> + fputc(fp, '*');
[ ... ]
> @@ -94,7 +94,7 @@ static void print_vsock_addr(FILE *fp, unsigned int cid, unsigned int port)
> static void print_vsock_stat(FILE *fp, struct vsock_stat *st)
> {
> print_vsock_addr(fp, st->msg.vdiag_src_cid, st->msg.vdiag_src_port);
> - fprintf(fp, " ");
> + fputc(fp, ' ');
Both conversions reverse the arguments.
> diff --git a/tools/thermal/tmon/sysfs.c b/tools/thermal/tmon/sysfs.c
> --- a/tools/thermal/tmon/sysfs.c
> +++ b/tools/thermal/tmon/sysfs.c
> @@ -522,7 +522,7 @@ int update_thermal_data()
> }
>
> if (tmon_log) {
> - fprintf(tmon_log, "\n");
> + fputc(tmon_log, '\n');
Arguments reversed.
> diff --git a/tools/thermal/tmon/tmon.c b/tools/thermal/tmon/tmon.c
> --- a/tools/thermal/tmon/tmon.c
> +++ b/tools/thermal/tmon/tmon.c
> @@ -199,7 +199,7 @@
> fprintf(tmon_log, "%s%d ", ptdata.cdi[i].type,
> ptdata.cdi[i].instance);
>
> - fprintf(tmon_log, "\n");
> + fputc(tmon_log, '\n');
Arguments reversed.
> diff --git a/tools/tracing/latency/latency-collector.c b/tools/tracing/latency/latency-collector.c
> --- a/tools/tracing/latency/latency-collector.c
> +++ b/tools/tracing/latency/latency-collector.c
> @@ -1999,7 +1999,7 @@
> "been enabled. Random sleep is intended for the following tracers:\n");
> for (i = 0; random_tracers[i]; i++)
> fprintf(stderr, "%s\n", random_tracers[i]);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
> --- a/tools/tracing/rtla/src/common.c
> +++ b/tools/tracing/rtla/src/common.c
> @@ -449,7 +449,7 @@ void common_usage(const char *tool, const char *mode,
> fprintf(stderr, "%s [-h] ", mode);
>
> print_msg_array(start_msgs);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
> --- a/tools/tracing/rtla/src/utils.c
> +++ b/tools/tracing/rtla/src/utils.c
> @@ -67,7 +67,7 @@ void fatal(const char *fmt, ...)
> va_start(ap, fmt);
> vfprintf(stderr, fmt, ap);
> va_end(ap);
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
> --- a/tools/usb/ffs-test.c
> +++ b/tools/usb/ffs-test.c
> @@ -562,7 +562,7 @@ empty_out_buf(struct thread *ignore, const void *buf, size_t nbytes)
> fprintf(stderr, "%4zd:", len);
> fprintf(stderr, " %02x", *p);
> if (31 == (len % 32))
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
> diff --git a/tools/verification/rv/src/in_kernel.c b/tools/verification/rv/src/in_kernel.c
> --- a/tools/verification/rv/src/in_kernel.c
> +++ b/tools/verification/rv/src/in_kernel.c
> @@ -667,7 +667,7 @@ static void ikm_usage_print_reactors(void)
> end = strstr(start, "\n");
> }
>
> - fprintf(stderr, "\n");
> + fputc(stderr, '\n');
Arguments reversed.
Looking at the lore discussion by Andi Kleen and Steven Rostedt, they
raised a concern that GCC already performs this fprintf->fputc optimization
automatically, making the patch unnecessary code churn. The author has not
responded to this feedback.
Beyond the optimization question, the technical implementation has a
systematic bug: every fputc() conversion in this patch reverses the
argument order.
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30016526959
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-23 14:55 UTC (permalink / raw)
To: Andrey Drobyshev, mst
Cc: linux-kernel, kvm, virtualization, netdev, stefanha, dongli.zhang,
maciej.szmigiero, bchaney, mark.kanda, ptikhomirov, den
In-Reply-To: <c631dabe-ebde-4b4d-b930-d78db4cbabc4@virtuozzo.com>
On Thu, 23 Jul 2026 at 16:50, Andrey Drobyshev
<andrey.drobyshev@virtuozzo.com> wrote:
>
> On 7/23/26 5:39 PM, Stefano Garzarella wrote:
> > On Thu, Jul 23, 2026 at 05:29:12PM +0300, Andrey Drobyshev wrote:
> >> On 7/23/26 5:03 PM, Stefano Garzarella wrote:
> >>> On Thu, Jul 23, 2026 at 04:57:47PM +0300, Andrey Drobyshev wrote:
> >>>> On 7/22/26 12:43 PM, Stefano Garzarella wrote:
> >>>>> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
> >>>>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
> >>>>>> vq->worker and queues work on it. vhost_workers_free() however clears
> >>>>>> the vq->worker pointers and immediately frees the workers, without
> >>>>>> waiting for a grace period. A caller that fetched the worker right
> >>>>>> before the pointer was cleared can therefore still be queueing work on
> >>>>>> it while it is freed. And even when the queueing itself wins the race,
> >>>>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
> >>>>>> future attempts to queue it are silently skipped.
> >>>>>>
> >>>>>> None of the current callers can actually hit this: net and scsi stop
> >>>>>> their virtqueues before the workers are freed, and vsock unhashes the
> >>>>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
> >>>>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
> >>>>>> in vhost-vsock keeps the device hashed while its workers are freed, so
> >>>>>> the lockless send/cancel paths become able to race with the teardown.
> >>>>>>
> >>>>>> Fix this by clearing the vq->worker pointers, waiting for a grace
> >>>>>> period, and then flushing the workers so any work the last readers
> >>>>>> queued runs before the workers are freed.
> >>>>>>
> >>>>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
> >>>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
> >>>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> >>>>>> ---
> >>>>>> drivers/vhost/vhost.c | 11 +++++++++++
> >>>>>> 1 file changed, 11 insertions(+)
> >>>>>
> >>>>> Sashiko reported some potential issues here:
> >>>>> https://sashiko.dev/#/patchset/20260720102241.371610-1-andrey.drobyshev@virtuozzo.com?part=4
> >>>>>
> >>>>> IMO the first one is pre-existing, but not really sure it is a real
> >>>>> issue since happening when the worker/vmm is going to be killed.
> >>>>>
> >>>>
> >>>> Sashiko claims:
> >>>>
> >>>>> Will this leave the queued work unexecuted and permanently break the
> >>>> virtqueue by leaving VHOST_WORK_QUEUED set?
> >>>>
> >>>> I agree this issue is pre-existing and doesn't have much to do with our
> >>>> series here. It looks real, but in reality should be harmless since we
> >>>> may only hit it while the device already dying. Means there's no VQ
> >>>> state to be saved. The only potentially observable artifact I guess is
> >>>> a warning here:
> >>>>
> >>>> vhost_workers_free()
> >>>> vhost_worker_destroy()
> >>>> WARN_ON(!llist_empty())
> >>>>
> >>>> So more of a cosmetic noise on a dying device. Again, not relevant to
> >>>> this series. But one optional way to make it go away would be to clear
> >>>> vq->worker under vq->mutex in vhost_workers_free() (mirroring
> >>>> vhost_worker_killed()).
> >>>>> The second one also not sure if it's an issue since the sender is not
> >>>>> lockless IIUC.
> >>>>>
> >>>>
> >>>> The term 'sender' is confusing here:
> >>>>
> >>>> * vhost_transport_send_pkt() is the .send_pkt() method of struct
> >>>> virtio_transport. It only stores skbs into the queue, doesn't process
> >>>> them. It indeed is lockless as it doesn't take vq->mutex.
> >>>>
> >>>> * vhost_transport_send_pkt_work() is the .fn() method of send_pkt_work.
> >>>> It's called by the worker thread to process skbs in the queue, calls
> >>>> vhost_transport_do_send_pkt() which does in turn take vq->mutex.
> >>>>
> >>>> I think sashiko points out to the former. Still, I don't think it's an
> >>>> actual bug. Look:
> >>>>
> >>>> 1) By invoking flush, we wake the worker thread:
> >>>> vhost_dev_flush()
> >>>> __vhost_worker_flush()
> >>>> vhost_worker_queue(flush.work)
> >>>> worker->ops->wakeup()
> >>>>
> >>>> 2) Then woken worker does:
> >>>> vhost_run_work_list()
> >>>> llist_for_each_entry_safe(work) {
> >>>> clear_bit(VHOST_WORK_QUEUED, &work->flags)
> >>>> work->fn(work) // for send_pkt_work = vhost_transport_send_pkt_work
> >>>> }
> >>>>
> >>>> 3) And then in work->fn() (vhost_transport_send_pkt_work):
> >>>> vhost_transport_send_pkt_work()
> >>>> vhost_transport_do_send_pkt()
> >>>> if (!vhost_vq_get_backend(vq))
> >>>> goto out;
> >>>>
> >>>> As you can see, we return early in case backend was unset.
> >>>>
> >>>> So after doing this flush, we have: 1) QUEUED bit is unset; that makes
> >>>> send_pkt_work re-queueable. 2) But the queue doesn't actually get
> >>>> processed, and VQ state isn't actually touched here - so I guess
> >>>> Sashiko's conclusion is incorrect and there's no actual bug.
> >>>>
> >>>>> But, please can you double check them?
> >>>>>
> >>>>
> >>>> In general I'd leave this patch as-is as Sashiko's complaints aren't
> >>>> very convincing so far. If you want I can add another patch which wraps
> >>>> NULLifying workers in vhost_workers_free() in vq->mutex.
> >>>>
> >>>> WDYT?
> >>>
> >>> Yeah, I agree, about the other patch, up to you, but I'll eventually
> >>> send it separately.
> >>>
> >>> Thanks,
> >>> Stefano
> >>>
> >>
> >> Alright, then let me resend it once more along with this 6th patch, so
> >> that we don't have it uncovered.
> >
> > But why it should be part of this series?
> >
> > If there is no strong reason, I'd send it as a separate patch.
> >
> > E.g. even this patch in theory may be a separate one, but this is
> > related to this series, so makes sense to have this included.
> >
>
> No particular reason, just while we're at it. Alright, no respin, will
> send it separately. Then I'll consider this series reviewed - unless,
> maybe, Michael will want to take a look as well?
Yep, I mean this should go with Michael's tree, so ... :-)
I sent my R-b to all patches, so I'm fine with this version.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-23 14:49 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <amInL2hPEG9ejUd0@sgarzare-redhat>
On 7/23/26 5:39 PM, Stefano Garzarella wrote:
> On Thu, Jul 23, 2026 at 05:29:12PM +0300, Andrey Drobyshev wrote:
>> On 7/23/26 5:03 PM, Stefano Garzarella wrote:
>>> On Thu, Jul 23, 2026 at 04:57:47PM +0300, Andrey Drobyshev wrote:
>>>> On 7/22/26 12:43 PM, Stefano Garzarella wrote:
>>>>> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>>>>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>>>>>> vq->worker and queues work on it. vhost_workers_free() however clears
>>>>>> the vq->worker pointers and immediately frees the workers, without
>>>>>> waiting for a grace period. A caller that fetched the worker right
>>>>>> before the pointer was cleared can therefore still be queueing work on
>>>>>> it while it is freed. And even when the queueing itself wins the race,
>>>>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>>>>>> future attempts to queue it are silently skipped.
>>>>>>
>>>>>> None of the current callers can actually hit this: net and scsi stop
>>>>>> their virtqueues before the workers are freed, and vsock unhashes the
>>>>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>>>>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>>>>>> in vhost-vsock keeps the device hashed while its workers are freed, so
>>>>>> the lockless send/cancel paths become able to race with the teardown.
>>>>>>
>>>>>> Fix this by clearing the vq->worker pointers, waiting for a grace
>>>>>> period, and then flushing the workers so any work the last readers
>>>>>> queued runs before the workers are freed.
>>>>>>
>>>>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>>>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>>>> ---
>>>>>> drivers/vhost/vhost.c | 11 +++++++++++
>>>>>> 1 file changed, 11 insertions(+)
>>>>>
>>>>> Sashiko reported some potential issues here:
>>>>> https://sashiko.dev/#/patchset/20260720102241.371610-1-andrey.drobyshev@virtuozzo.com?part=4
>>>>>
>>>>> IMO the first one is pre-existing, but not really sure it is a real
>>>>> issue since happening when the worker/vmm is going to be killed.
>>>>>
>>>>
>>>> Sashiko claims:
>>>>
>>>>> Will this leave the queued work unexecuted and permanently break the
>>>> virtqueue by leaving VHOST_WORK_QUEUED set?
>>>>
>>>> I agree this issue is pre-existing and doesn't have much to do with our
>>>> series here. It looks real, but in reality should be harmless since we
>>>> may only hit it while the device already dying. Means there's no VQ
>>>> state to be saved. The only potentially observable artifact I guess is
>>>> a warning here:
>>>>
>>>> vhost_workers_free()
>>>> vhost_worker_destroy()
>>>> WARN_ON(!llist_empty())
>>>>
>>>> So more of a cosmetic noise on a dying device. Again, not relevant to
>>>> this series. But one optional way to make it go away would be to clear
>>>> vq->worker under vq->mutex in vhost_workers_free() (mirroring
>>>> vhost_worker_killed()).
>>>>> The second one also not sure if it's an issue since the sender is not
>>>>> lockless IIUC.
>>>>>
>>>>
>>>> The term 'sender' is confusing here:
>>>>
>>>> * vhost_transport_send_pkt() is the .send_pkt() method of struct
>>>> virtio_transport. It only stores skbs into the queue, doesn't process
>>>> them. It indeed is lockless as it doesn't take vq->mutex.
>>>>
>>>> * vhost_transport_send_pkt_work() is the .fn() method of send_pkt_work.
>>>> It's called by the worker thread to process skbs in the queue, calls
>>>> vhost_transport_do_send_pkt() which does in turn take vq->mutex.
>>>>
>>>> I think sashiko points out to the former. Still, I don't think it's an
>>>> actual bug. Look:
>>>>
>>>> 1) By invoking flush, we wake the worker thread:
>>>> vhost_dev_flush()
>>>> __vhost_worker_flush()
>>>> vhost_worker_queue(flush.work)
>>>> worker->ops->wakeup()
>>>>
>>>> 2) Then woken worker does:
>>>> vhost_run_work_list()
>>>> llist_for_each_entry_safe(work) {
>>>> clear_bit(VHOST_WORK_QUEUED, &work->flags)
>>>> work->fn(work) // for send_pkt_work = vhost_transport_send_pkt_work
>>>> }
>>>>
>>>> 3) And then in work->fn() (vhost_transport_send_pkt_work):
>>>> vhost_transport_send_pkt_work()
>>>> vhost_transport_do_send_pkt()
>>>> if (!vhost_vq_get_backend(vq))
>>>> goto out;
>>>>
>>>> As you can see, we return early in case backend was unset.
>>>>
>>>> So after doing this flush, we have: 1) QUEUED bit is unset; that makes
>>>> send_pkt_work re-queueable. 2) But the queue doesn't actually get
>>>> processed, and VQ state isn't actually touched here - so I guess
>>>> Sashiko's conclusion is incorrect and there's no actual bug.
>>>>
>>>>> But, please can you double check them?
>>>>>
>>>>
>>>> In general I'd leave this patch as-is as Sashiko's complaints aren't
>>>> very convincing so far. If you want I can add another patch which wraps
>>>> NULLifying workers in vhost_workers_free() in vq->mutex.
>>>>
>>>> WDYT?
>>>
>>> Yeah, I agree, about the other patch, up to you, but I'll eventually
>>> send it separately.
>>>
>>> Thanks,
>>> Stefano
>>>
>>
>> Alright, then let me resend it once more along with this 6th patch, so
>> that we don't have it uncovered.
>
> But why it should be part of this series?
>
> If there is no strong reason, I'd send it as a separate patch.
>
> E.g. even this patch in theory may be a separate one, but this is
> related to this series, so makes sense to have this included.
>
No particular reason, just while we're at it. Alright, no respin, will
send it separately. Then I'll consider this series reviewed - unless,
maybe, Michael will want to take a look as well?
Thanks,
Andrey
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-23 14:39 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <f21667c3-64f1-40c6-9941-9453af173eda@virtuozzo.com>
On Thu, Jul 23, 2026 at 05:29:12PM +0300, Andrey Drobyshev wrote:
>On 7/23/26 5:03 PM, Stefano Garzarella wrote:
>> On Thu, Jul 23, 2026 at 04:57:47PM +0300, Andrey Drobyshev wrote:
>>> On 7/22/26 12:43 PM, Stefano Garzarella wrote:
>>>> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>>>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>>>>> vq->worker and queues work on it. vhost_workers_free() however clears
>>>>> the vq->worker pointers and immediately frees the workers, without
>>>>> waiting for a grace period. A caller that fetched the worker right
>>>>> before the pointer was cleared can therefore still be queueing work on
>>>>> it while it is freed. And even when the queueing itself wins the race,
>>>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>>>>> future attempts to queue it are silently skipped.
>>>>>
>>>>> None of the current callers can actually hit this: net and scsi stop
>>>>> their virtqueues before the workers are freed, and vsock unhashes the
>>>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>>>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>>>>> in vhost-vsock keeps the device hashed while its workers are freed, so
>>>>> the lockless send/cancel paths become able to race with the teardown.
>>>>>
>>>>> Fix this by clearing the vq->worker pointers, waiting for a grace
>>>>> period, and then flushing the workers so any work the last readers
>>>>> queued runs before the workers are freed.
>>>>>
>>>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>>> ---
>>>>> drivers/vhost/vhost.c | 11 +++++++++++
>>>>> 1 file changed, 11 insertions(+)
>>>>
>>>> Sashiko reported some potential issues here:
>>>> https://sashiko.dev/#/patchset/20260720102241.371610-1-andrey.drobyshev@virtuozzo.com?part=4
>>>>
>>>> IMO the first one is pre-existing, but not really sure it is a real
>>>> issue since happening when the worker/vmm is going to be killed.
>>>>
>>>
>>> Sashiko claims:
>>>
>>>> Will this leave the queued work unexecuted and permanently break the
>>> virtqueue by leaving VHOST_WORK_QUEUED set?
>>>
>>> I agree this issue is pre-existing and doesn't have much to do with our
>>> series here. It looks real, but in reality should be harmless since we
>>> may only hit it while the device already dying. Means there's no VQ
>>> state to be saved. The only potentially observable artifact I guess is
>>> a warning here:
>>>
>>> vhost_workers_free()
>>> vhost_worker_destroy()
>>> WARN_ON(!llist_empty())
>>>
>>> So more of a cosmetic noise on a dying device. Again, not relevant to
>>> this series. But one optional way to make it go away would be to clear
>>> vq->worker under vq->mutex in vhost_workers_free() (mirroring
>>> vhost_worker_killed()).
>>>> The second one also not sure if it's an issue since the sender is not
>>>> lockless IIUC.
>>>>
>>>
>>> The term 'sender' is confusing here:
>>>
>>> * vhost_transport_send_pkt() is the .send_pkt() method of struct
>>> virtio_transport. It only stores skbs into the queue, doesn't process
>>> them. It indeed is lockless as it doesn't take vq->mutex.
>>>
>>> * vhost_transport_send_pkt_work() is the .fn() method of send_pkt_work.
>>> It's called by the worker thread to process skbs in the queue, calls
>>> vhost_transport_do_send_pkt() which does in turn take vq->mutex.
>>>
>>> I think sashiko points out to the former. Still, I don't think it's an
>>> actual bug. Look:
>>>
>>> 1) By invoking flush, we wake the worker thread:
>>> vhost_dev_flush()
>>> __vhost_worker_flush()
>>> vhost_worker_queue(flush.work)
>>> worker->ops->wakeup()
>>>
>>> 2) Then woken worker does:
>>> vhost_run_work_list()
>>> llist_for_each_entry_safe(work) {
>>> clear_bit(VHOST_WORK_QUEUED, &work->flags)
>>> work->fn(work) // for send_pkt_work = vhost_transport_send_pkt_work
>>> }
>>>
>>> 3) And then in work->fn() (vhost_transport_send_pkt_work):
>>> vhost_transport_send_pkt_work()
>>> vhost_transport_do_send_pkt()
>>> if (!vhost_vq_get_backend(vq))
>>> goto out;
>>>
>>> As you can see, we return early in case backend was unset.
>>>
>>> So after doing this flush, we have: 1) QUEUED bit is unset; that makes
>>> send_pkt_work re-queueable. 2) But the queue doesn't actually get
>>> processed, and VQ state isn't actually touched here - so I guess
>>> Sashiko's conclusion is incorrect and there's no actual bug.
>>>
>>>> But, please can you double check them?
>>>>
>>>
>>> In general I'd leave this patch as-is as Sashiko's complaints aren't
>>> very convincing so far. If you want I can add another patch which wraps
>>> NULLifying workers in vhost_workers_free() in vq->mutex.
>>>
>>> WDYT?
>>
>> Yeah, I agree, about the other patch, up to you, but I'll eventually
>> send it separately.
>>
>> Thanks,
>> Stefano
>>
>
>Alright, then let me resend it once more along with this 6th patch, so
>that we don't have it uncovered.
But why it should be part of this series?
If there is no strong reason, I'd send it as a separate patch.
E.g. even this patch in theory may be a separate one, but this is
related to this series, so makes sense to have this included.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH] tools: Use fputc() calls in some functions
From: Steven Rostedt @ 2026-07-23 14:33 UTC (permalink / raw)
To: Andi Kleen
Cc: Markus Elfring, acpica-devel, bpf, coresight, kvm, linux-acpi,
linux-arm-kernel, linux-gpio, linux-kselftest, linux-mm,
linux-perf-users, linux-pm, linux-trace-kernel, linuxppc-dev,
mptcp, netdev, platform-driver-x86, virtualization, Adrian Hunter,
Alex Mastro, Alex Williamson, Alexander Shishkin,
Alexei Starovoitov, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Ankur Arora, Arnaldo Carvalho de Melo, Bartosz Golaszewski,
Bobby Eshleman, Christian Bornträger, Christophe Leroy,
Chun-Tse Shao, Claudio Imbrenda, Costa Shulyupin, Crystal Wood,
Daniel Borkmann, Daniel Lezcano, David Hildenbrand, David Matlack,
David S. Miller, Donald Hunter, Eduard Zingerman, Emil Tsalapatis,
Eric Dumazet, Gabriele Monaco, Geliang Tang, Ian Rogers,
Ingo Molnar, Ivan Pravdin, Jakub Kicinski, James Clark,
Janosch Frank, Jason Xing, Jiri Olsa, Joe Damato, John Garry,
Josh Poimboeuf, Kaushlendra Kumar, Kent Gibson,
Kumar Kartikeya Dwivedi, Len Brown, Leo Yan, Lukasz Luba,
Madhavan Srinivasan, Mark Rutland, Martin KaFai Lau,
Masami Hiramatsu, Mat Martineau, Matthieu Baerts,
Michael Ellerman, Mike Leach, Mina Almasry, Mykyta Yatsenko,
Nam Cao, Namhyung Kim, Nicholas Piggin, Paolo Abeni,
Paolo Bonzini, Pawel Chmielewski, Peter Zijlstra, Quentin Monnet,
Rafael J. Wysocki, Raghavendra Rao Ananta, Saket Dumbre,
Shuah Khan, Simon Horman, Song Liu, Srinivas Pandruvada,
Stanislav Fomichev, Stefano Garzarella, Suzuki K Poulose,
Swapnil Sapkal, Thomas Weißschuh, Tomas Glozar, Vipin Sharma,
Wander Lairson Costa, Will Deacon, Willem de Bruijn,
Willy Tarreau, Yonghong Song, Zhang Chujun, Zhang Rui, LKML,
kernel-janitors
In-Reply-To: <amIbVOIsOeZgcBZF@tassilo>
On Thu, 23 Jul 2026 06:47:00 -0700
Andi Kleen <ak@linux.intel.com> wrote:
> > The source code was transformed by using the Coccinelle software.
>
> gcc already does this transformation automatically.
Yep, I showed this in my talk[1] that looked at what printf("Hello World!")
does. Doing an objdump of:
#include <stdio.h>
int main(void) {
printf("Hello World!\n");
return 0;
}
has:
0000000000001139 <main>:
1139: 55 push %rbp
113a: 48 89 e5 mov %rsp,%rbp
113d: 48 8d 05 c0 0e 00 00 lea 0xec0(%rip),%rax # 2004 <_IO_stdin_used+0x4>
1144: 48 89 c7 mov %rax,%rdi
1147: e8 e4 fe ff ff call 1030 <puts@plt> <<<--- puts and not printf
114c: b8 00 00 00 00 mov $0x0,%eax
1151: 5d pop %rbp
1152: c3 ret
-- Steve
[1] https://www.youtube.com/watch?v=JRyrhsx-L5Y
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Denis V. Lunev @ 2026-07-23 14:31 UTC (permalink / raw)
To: Andrey Drobyshev, Stefano Garzarella
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <f21667c3-64f1-40c6-9941-9453af173eda@virtuozzo.com>
On 7/23/26 16:29, Andrey Drobyshev wrote:
> On 7/23/26 5:03 PM, Stefano Garzarella wrote:
>> On Thu, Jul 23, 2026 at 04:57:47PM +0300, Andrey Drobyshev wrote:
>>> On 7/22/26 12:43 PM, Stefano Garzarella wrote:
>>>> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>>>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>>>>> vq->worker and queues work on it. vhost_workers_free() however clears
>>>>> the vq->worker pointers and immediately frees the workers, without
>>>>> waiting for a grace period. A caller that fetched the worker right
>>>>> before the pointer was cleared can therefore still be queueing work on
>>>>> it while it is freed. And even when the queueing itself wins the race,
>>>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>>>>> future attempts to queue it are silently skipped.
>>>>>
>>>>> None of the current callers can actually hit this: net and scsi stop
>>>>> their virtqueues before the workers are freed, and vsock unhashes the
>>>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>>>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>>>>> in vhost-vsock keeps the device hashed while its workers are freed, so
>>>>> the lockless send/cancel paths become able to race with the teardown.
>>>>>
>>>>> Fix this by clearing the vq->worker pointers, waiting for a grace
>>>>> period, and then flushing the workers so any work the last readers
>>>>> queued runs before the workers are freed.
>>>>>
>>>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>>> ---
>>>>> drivers/vhost/vhost.c | 11 +++++++++++
>>>>> 1 file changed, 11 insertions(+)
>>>> Sashiko reported some potential issues here:
>>>> https://sashiko.dev/#/patchset/20260720102241.371610-1-andrey.drobyshev@virtuozzo.com?part=4
>>>>
>>>> IMO the first one is pre-existing, but not really sure it is a real
>>>> issue since happening when the worker/vmm is going to be killed.
>>>>
>>> Sashiko claims:
>>>
>>>> Will this leave the queued work unexecuted and permanently break the
>>> virtqueue by leaving VHOST_WORK_QUEUED set?
>>>
>>> I agree this issue is pre-existing and doesn't have much to do with our
>>> series here. It looks real, but in reality should be harmless since we
>>> may only hit it while the device already dying. Means there's no VQ
>>> state to be saved. The only potentially observable artifact I guess is
>>> a warning here:
>>>
>>> vhost_workers_free()
>>> vhost_worker_destroy()
>>> WARN_ON(!llist_empty())
>>>
>>> So more of a cosmetic noise on a dying device. Again, not relevant to
>>> this series. But one optional way to make it go away would be to clear
>>> vq->worker under vq->mutex in vhost_workers_free() (mirroring
>>> vhost_worker_killed()).
>>>> The second one also not sure if it's an issue since the sender is not
>>>> lockless IIUC.
>>>>
>>> The term 'sender' is confusing here:
>>>
>>> * vhost_transport_send_pkt() is the .send_pkt() method of struct
>>> virtio_transport. It only stores skbs into the queue, doesn't process
>>> them. It indeed is lockless as it doesn't take vq->mutex.
>>>
>>> * vhost_transport_send_pkt_work() is the .fn() method of send_pkt_work.
>>> It's called by the worker thread to process skbs in the queue, calls
>>> vhost_transport_do_send_pkt() which does in turn take vq->mutex.
>>>
>>> I think sashiko points out to the former. Still, I don't think it's an
>>> actual bug. Look:
>>>
>>> 1) By invoking flush, we wake the worker thread:
>>> vhost_dev_flush()
>>> __vhost_worker_flush()
>>> vhost_worker_queue(flush.work)
>>> worker->ops->wakeup()
>>>
>>> 2) Then woken worker does:
>>> vhost_run_work_list()
>>> llist_for_each_entry_safe(work) {
>>> clear_bit(VHOST_WORK_QUEUED, &work->flags)
>>> work->fn(work) // for send_pkt_work = vhost_transport_send_pkt_work
>>> }
>>>
>>> 3) And then in work->fn() (vhost_transport_send_pkt_work):
>>> vhost_transport_send_pkt_work()
>>> vhost_transport_do_send_pkt()
>>> if (!vhost_vq_get_backend(vq))
>>> goto out;
>>>
>>> As you can see, we return early in case backend was unset.
>>>
>>> So after doing this flush, we have: 1) QUEUED bit is unset; that makes
>>> send_pkt_work re-queueable. 2) But the queue doesn't actually get
>>> processed, and VQ state isn't actually touched here - so I guess
>>> Sashiko's conclusion is incorrect and there's no actual bug.
>>>
>>>> But, please can you double check them?
>>>>
>>> In general I'd leave this patch as-is as Sashiko's complaints aren't
>>> very convincing so far. If you want I can add another patch which wraps
>>> NULLifying workers in vhost_workers_free() in vq->mutex.
>>>
>>> WDYT?
>> Yeah, I agree, about the other patch, up to you, but I'll eventually
>> send it separately.
>>
>> Thanks,
>> Stefano
>>
> Alright, then let me resend it once more along with this 6th patch, so
> that we don't have it uncovered.
>
> Andrey
I am sending 6/5 patch as followup in such a case :-) and respin
only if rejected.
Den
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-23 14:29 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <amIe79X3j7GxDBdO@sgarzare-redhat>
On 7/23/26 5:03 PM, Stefano Garzarella wrote:
> On Thu, Jul 23, 2026 at 04:57:47PM +0300, Andrey Drobyshev wrote:
>> On 7/22/26 12:43 PM, Stefano Garzarella wrote:
>>> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>>>> vq->worker and queues work on it. vhost_workers_free() however clears
>>>> the vq->worker pointers and immediately frees the workers, without
>>>> waiting for a grace period. A caller that fetched the worker right
>>>> before the pointer was cleared can therefore still be queueing work on
>>>> it while it is freed. And even when the queueing itself wins the race,
>>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>>>> future attempts to queue it are silently skipped.
>>>>
>>>> None of the current callers can actually hit this: net and scsi stop
>>>> their virtqueues before the workers are freed, and vsock unhashes the
>>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>>>> in vhost-vsock keeps the device hashed while its workers are freed, so
>>>> the lockless send/cancel paths become able to race with the teardown.
>>>>
>>>> Fix this by clearing the vq->worker pointers, waiting for a grace
>>>> period, and then flushing the workers so any work the last readers
>>>> queued runs before the workers are freed.
>>>>
>>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>>> ---
>>>> drivers/vhost/vhost.c | 11 +++++++++++
>>>> 1 file changed, 11 insertions(+)
>>>
>>> Sashiko reported some potential issues here:
>>> https://sashiko.dev/#/patchset/20260720102241.371610-1-andrey.drobyshev@virtuozzo.com?part=4
>>>
>>> IMO the first one is pre-existing, but not really sure it is a real
>>> issue since happening when the worker/vmm is going to be killed.
>>>
>>
>> Sashiko claims:
>>
>>> Will this leave the queued work unexecuted and permanently break the
>> virtqueue by leaving VHOST_WORK_QUEUED set?
>>
>> I agree this issue is pre-existing and doesn't have much to do with our
>> series here. It looks real, but in reality should be harmless since we
>> may only hit it while the device already dying. Means there's no VQ
>> state to be saved. The only potentially observable artifact I guess is
>> a warning here:
>>
>> vhost_workers_free()
>> vhost_worker_destroy()
>> WARN_ON(!llist_empty())
>>
>> So more of a cosmetic noise on a dying device. Again, not relevant to
>> this series. But one optional way to make it go away would be to clear
>> vq->worker under vq->mutex in vhost_workers_free() (mirroring
>> vhost_worker_killed()).
>>> The second one also not sure if it's an issue since the sender is not
>>> lockless IIUC.
>>>
>>
>> The term 'sender' is confusing here:
>>
>> * vhost_transport_send_pkt() is the .send_pkt() method of struct
>> virtio_transport. It only stores skbs into the queue, doesn't process
>> them. It indeed is lockless as it doesn't take vq->mutex.
>>
>> * vhost_transport_send_pkt_work() is the .fn() method of send_pkt_work.
>> It's called by the worker thread to process skbs in the queue, calls
>> vhost_transport_do_send_pkt() which does in turn take vq->mutex.
>>
>> I think sashiko points out to the former. Still, I don't think it's an
>> actual bug. Look:
>>
>> 1) By invoking flush, we wake the worker thread:
>> vhost_dev_flush()
>> __vhost_worker_flush()
>> vhost_worker_queue(flush.work)
>> worker->ops->wakeup()
>>
>> 2) Then woken worker does:
>> vhost_run_work_list()
>> llist_for_each_entry_safe(work) {
>> clear_bit(VHOST_WORK_QUEUED, &work->flags)
>> work->fn(work) // for send_pkt_work = vhost_transport_send_pkt_work
>> }
>>
>> 3) And then in work->fn() (vhost_transport_send_pkt_work):
>> vhost_transport_send_pkt_work()
>> vhost_transport_do_send_pkt()
>> if (!vhost_vq_get_backend(vq))
>> goto out;
>>
>> As you can see, we return early in case backend was unset.
>>
>> So after doing this flush, we have: 1) QUEUED bit is unset; that makes
>> send_pkt_work re-queueable. 2) But the queue doesn't actually get
>> processed, and VQ state isn't actually touched here - so I guess
>> Sashiko's conclusion is incorrect and there's no actual bug.
>>
>>> But, please can you double check them?
>>>
>>
>> In general I'd leave this patch as-is as Sashiko's complaints aren't
>> very convincing so far. If you want I can add another patch which wraps
>> NULLifying workers in vhost_workers_free() in vq->mutex.
>>
>> WDYT?
>
> Yeah, I agree, about the other patch, up to you, but I'll eventually
> send it separately.
>
> Thanks,
> Stefano
>
Alright, then let me resend it once more along with this 6th patch, so
that we don't have it uncovered.
Andrey
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-23 14:03 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <20260720102241.371610-5-andrey.drobyshev@virtuozzo.com>
On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>vq->worker and queues work on it. vhost_workers_free() however clears
>the vq->worker pointers and immediately frees the workers, without
>waiting for a grace period. A caller that fetched the worker right
>before the pointer was cleared can therefore still be queueing work on
>it while it is freed. And even when the queueing itself wins the race,
>the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>future attempts to queue it are silently skipped.
>
>None of the current callers can actually hit this: net and scsi stop
>their virtqueues before the workers are freed, and vsock unhashes the
>device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>before the workers go away. But the upcoming VHOST_RESET_OWNER support
>in vhost-vsock keeps the device hashed while its workers are freed, so
>the lockless send/cancel paths become able to race with the teardown.
>
>Fix this by clearing the vq->worker pointers, waiting for a grace
>period, and then flushing the workers so any work the last readers
>queued runs before the workers are freed.
>
>Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>---
> drivers/vhost/vhost.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Stefano Garzarella @ 2026-07-23 14:03 UTC (permalink / raw)
To: Andrey Drobyshev
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <82066fcb-150f-47ef-86a7-0df0f9eb41c5@virtuozzo.com>
On Thu, Jul 23, 2026 at 04:57:47PM +0300, Andrey Drobyshev wrote:
>On 7/22/26 12:43 PM, Stefano Garzarella wrote:
>> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>>> vq->worker and queues work on it. vhost_workers_free() however clears
>>> the vq->worker pointers and immediately frees the workers, without
>>> waiting for a grace period. A caller that fetched the worker right
>>> before the pointer was cleared can therefore still be queueing work on
>>> it while it is freed. And even when the queueing itself wins the race,
>>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>>> future attempts to queue it are silently skipped.
>>>
>>> None of the current callers can actually hit this: net and scsi stop
>>> their virtqueues before the workers are freed, and vsock unhashes the
>>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>>> in vhost-vsock keeps the device hashed while its workers are freed, so
>>> the lockless send/cancel paths become able to race with the teardown.
>>>
>>> Fix this by clearing the vq->worker pointers, waiting for a grace
>>> period, and then flushing the workers so any work the last readers
>>> queued runs before the workers are freed.
>>>
>>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>>> ---
>>> drivers/vhost/vhost.c | 11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>
>> Sashiko reported some potential issues here:
>> https://sashiko.dev/#/patchset/20260720102241.371610-1-andrey.drobyshev@virtuozzo.com?part=4
>>
>> IMO the first one is pre-existing, but not really sure it is a real
>> issue since happening when the worker/vmm is going to be killed.
>>
>
>Sashiko claims:
>
>> Will this leave the queued work unexecuted and permanently break the
>virtqueue by leaving VHOST_WORK_QUEUED set?
>
>I agree this issue is pre-existing and doesn't have much to do with our
>series here. It looks real, but in reality should be harmless since we
>may only hit it while the device already dying. Means there's no VQ
>state to be saved. The only potentially observable artifact I guess is
>a warning here:
>
>vhost_workers_free()
> vhost_worker_destroy()
> WARN_ON(!llist_empty())
>
>So more of a cosmetic noise on a dying device. Again, not relevant to
>this series. But one optional way to make it go away would be to clear
>vq->worker under vq->mutex in vhost_workers_free() (mirroring
>vhost_worker_killed()).
>> The second one also not sure if it's an issue since the sender is not
>> lockless IIUC.
>>
>
>The term 'sender' is confusing here:
>
>* vhost_transport_send_pkt() is the .send_pkt() method of struct
>virtio_transport. It only stores skbs into the queue, doesn't process
>them. It indeed is lockless as it doesn't take vq->mutex.
>
>* vhost_transport_send_pkt_work() is the .fn() method of send_pkt_work.
>It's called by the worker thread to process skbs in the queue, calls
>vhost_transport_do_send_pkt() which does in turn take vq->mutex.
>
>I think sashiko points out to the former. Still, I don't think it's an
>actual bug. Look:
>
>1) By invoking flush, we wake the worker thread:
>vhost_dev_flush()
> __vhost_worker_flush()
> vhost_worker_queue(flush.work)
> worker->ops->wakeup()
>
>2) Then woken worker does:
>vhost_run_work_list()
> llist_for_each_entry_safe(work) {
> clear_bit(VHOST_WORK_QUEUED, &work->flags)
> work->fn(work) // for send_pkt_work = vhost_transport_send_pkt_work
> }
>
>3) And then in work->fn() (vhost_transport_send_pkt_work):
>vhost_transport_send_pkt_work()
> vhost_transport_do_send_pkt()
> if (!vhost_vq_get_backend(vq))
> goto out;
>
>As you can see, we return early in case backend was unset.
>
>So after doing this flush, we have: 1) QUEUED bit is unset; that makes
>send_pkt_work re-queueable. 2) But the queue doesn't actually get
>processed, and VQ state isn't actually touched here - so I guess
>Sashiko's conclusion is incorrect and there's no actual bug.
>
>> But, please can you double check them?
>>
>
>In general I'd leave this patch as-is as Sashiko's complaints aren't
>very convincing so far. If you want I can add another patch which wraps
>NULLifying workers in vhost_workers_free() in vq->mutex.
>
>WDYT?
Yeah, I agree, about the other patch, up to you, but I'll eventually
send it separately.
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v5 4/5] vhost: synchronize with RCU readers when freeing workers
From: Andrey Drobyshev @ 2026-07-23 13:57 UTC (permalink / raw)
To: Stefano Garzarella
Cc: linux-kernel, kvm, virtualization, netdev, mst, stefanha,
dongli.zhang, maciej.szmigiero, bchaney, mark.kanda, ptikhomirov,
den
In-Reply-To: <amCPLOrnpYmr5n0p@sgarzare-redhat>
On 7/22/26 12:43 PM, Stefano Garzarella wrote:
> On Mon, Jul 20, 2026 at 01:22:40PM +0300, Andrey Drobyshev wrote:
>> vhost_vq_work_queue() only holds the RCU read lock while it dereferences
>> vq->worker and queues work on it. vhost_workers_free() however clears
>> the vq->worker pointers and immediately frees the workers, without
>> waiting for a grace period. A caller that fetched the worker right
>> before the pointer was cleared can therefore still be queueing work on
>> it while it is freed. And even when the queueing itself wins the race,
>> the work is never run, so its VHOST_WORK_QUEUED bit stays set and all
>> future attempts to queue it are silently skipped.
>>
>> None of the current callers can actually hit this: net and scsi stop
>> their virtqueues before the workers are freed, and vsock unhashes the
>> device and does synchronize_rcu() of its own in vhost_vsock_dev_release()
>> before the workers go away. But the upcoming VHOST_RESET_OWNER support
>> in vhost-vsock keeps the device hashed while its workers are freed, so
>> the lockless send/cancel paths become able to race with the teardown.
>>
>> Fix this by clearing the vq->worker pointers, waiting for a grace
>> period, and then flushing the workers so any work the last readers
>> queued runs before the workers are freed.
>>
>> Fixes: 228a27cf78af ("vhost: Allow worker switching while work is queueing")
>> Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>> Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>> ---
>> drivers/vhost/vhost.c | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>
> Sashiko reported some potential issues here:
> https://sashiko.dev/#/patchset/20260720102241.371610-1-andrey.drobyshev@virtuozzo.com?part=4
>
> IMO the first one is pre-existing, but not really sure it is a real
> issue since happening when the worker/vmm is going to be killed.
>
Sashiko claims:
> Will this leave the queued work unexecuted and permanently break the
virtqueue by leaving VHOST_WORK_QUEUED set?
I agree this issue is pre-existing and doesn't have much to do with our
series here. It looks real, but in reality should be harmless since we
may only hit it while the device already dying. Means there's no VQ
state to be saved. The only potentially observable artifact I guess is
a warning here:
vhost_workers_free()
vhost_worker_destroy()
WARN_ON(!llist_empty())
So more of a cosmetic noise on a dying device. Again, not relevant to
this series. But one optional way to make it go away would be to clear
vq->worker under vq->mutex in vhost_workers_free() (mirroring
vhost_worker_killed()).
> The second one also not sure if it's an issue since the sender is not
> lockless IIUC.
>
The term 'sender' is confusing here:
* vhost_transport_send_pkt() is the .send_pkt() method of struct
virtio_transport. It only stores skbs into the queue, doesn't process
them. It indeed is lockless as it doesn't take vq->mutex.
* vhost_transport_send_pkt_work() is the .fn() method of send_pkt_work.
It's called by the worker thread to process skbs in the queue, calls
vhost_transport_do_send_pkt() which does in turn take vq->mutex.
I think sashiko points out to the former. Still, I don't think it's an
actual bug. Look:
1) By invoking flush, we wake the worker thread:
vhost_dev_flush()
__vhost_worker_flush()
vhost_worker_queue(flush.work)
worker->ops->wakeup()
2) Then woken worker does:
vhost_run_work_list()
llist_for_each_entry_safe(work) {
clear_bit(VHOST_WORK_QUEUED, &work->flags)
work->fn(work) // for send_pkt_work = vhost_transport_send_pkt_work
}
3) And then in work->fn() (vhost_transport_send_pkt_work):
vhost_transport_send_pkt_work()
vhost_transport_do_send_pkt()
if (!vhost_vq_get_backend(vq))
goto out;
As you can see, we return early in case backend was unset.
So after doing this flush, we have: 1) QUEUED bit is unset; that makes
send_pkt_work re-queueable. 2) But the queue doesn't actually get
processed, and VQ state isn't actually touched here - so I guess
Sashiko's conclusion is incorrect and there's no actual bug.
> But, please can you double check them?
>
In general I'd leave this patch as-is as Sashiko's complaints aren't
very convincing so far. If you want I can add another patch which wraps
NULLifying workers in vhost_workers_free() in vq->mutex.
WDYT?
Andrey
> Thanks,
> Stefano
>
> [...]
^ permalink raw reply
* Re: [PATCH] tools: Use fputc() calls in some functions
From: Andi Kleen @ 2026-07-23 13:47 UTC (permalink / raw)
To: Markus Elfring
Cc: acpica-devel, bpf, coresight, kvm, linux-acpi, linux-arm-kernel,
linux-gpio, linux-kselftest, linux-mm, linux-perf-users, linux-pm,
linux-trace-kernel, linuxppc-dev, mptcp, netdev,
platform-driver-x86, virtualization, Adrian Hunter, Alex Mastro,
Alex Williamson, Alexander Shishkin, Alexei Starovoitov,
Andrew Lunn, Andrew Morton, Andrii Nakryiko, Ankur Arora,
Arnaldo Carvalho de Melo, Bartosz Golaszewski, Bobby Eshleman,
Christian Bornträger, Christophe Leroy, Chun-Tse Shao,
Claudio Imbrenda, Costa Shulyupin, Crystal Wood, Daniel Borkmann,
Daniel Lezcano, David Hildenbrand, David Matlack, David S. Miller,
Donald Hunter, Eduard Zingerman, Emil Tsalapatis, Eric Dumazet,
Gabriele Monaco, Geliang Tang, Ian Rogers, Ingo Molnar,
Ivan Pravdin, Jakub Kicinski, James Clark, Janosch Frank,
Jason Xing, Jiri Olsa, Joe Damato, John Garry, Josh Poimboeuf,
Kaushlendra Kumar, Kent Gibson, Kumar Kartikeya Dwivedi,
Len Brown, Leo Yan, Lukasz Luba, Madhavan Srinivasan,
Mark Rutland, Martin KaFai Lau, Masami Hiramatsu, Mat Martineau,
Matthieu Baerts, Michael Ellerman, Mike Leach, Mina Almasry,
Mykyta Yatsenko, Nam Cao, Namhyung Kim, Nicholas Piggin,
Paolo Abeni, Paolo Bonzini, Pawel Chmielewski, Peter Zijlstra,
Quentin Monnet, Rafael J. Wysocki, Raghavendra Rao Ananta,
Saket Dumbre, Shuah Khan, Simon Horman, Song Liu,
Srinivas Pandruvada, Stanislav Fomichev, Stefano Garzarella,
Steven Rostedt, Suzuki K Poulose, Swapnil Sapkal,
Thomas Weißschuh, Tomas Glozar, Vipin Sharma,
Wander Lairson Costa, Will Deacon, Willem de Bruijn,
Willy Tarreau, Yonghong Song, Zhang Chujun, Zhang Rui, LKML,
kernel-janitors
In-Reply-To: <48f2d61f-d468-49b7-881e-f00777db073f@web.de>
On Thu, Jul 23, 2026 at 12:25:25PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 23 Jul 2026 11:13:30 +0200
> Subject: [PATCH] tools: Use fputc() calls in some functions
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Single characters should occasionally be transferred into the output.
> Thus use the corresponding function “fputc” (or even “putchar”).
>
> The source code was transformed by using the Coccinelle software.
gcc already does this transformation automatically.
^ permalink raw reply
* Re: [PATCH v2 3/3] iommu/virtio: Reject short event buffers
From: Will Deacon @ 2026-07-23 11:44 UTC (permalink / raw)
To: Weimin Xiong
Cc: Jean-Philippe Brucker, Joerg Roedel, Xiong Weimin, Robin Murphy,
virtualization, iommu, linux-kernel
In-Reply-To: <20260723095103.1-xiongweimin@kylinos.cn>
On Thu, Jul 23, 2026 at 09:54:09AM +0800, Weimin Xiong wrote:
> From: Xiong Weimin <xiongweimin@kylinos.cn>
>
> From: Xiong Weimin <xiongweimin@kylinos.cn>
>
> viommu_event_handler() only rejects event buffers that are larger than
> struct viommu_event. A short buffer is also invalid, but the handler
> would still read evt->head and, for fault events, the rest of evt->fault.
>
> Require the used length to match the event buffer size before looking at
> the event contents.
>
> Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
> ---
> drivers/iommu/virtio-iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
I don't know why you keep sending this without first responding to the
feedback you already received:
https://lore.kernel.org/all/20260709095518.GA1752964@myrica/
Will
^ permalink raw reply
* Re: [PATCH] vhost-scsi: flush backend after device ioctls
From: Jia Jia @ 2026-07-23 10:54 UTC (permalink / raw)
To: michael.christie, mst, jasowangio
Cc: pbonzoni, stefanha, eperezma, virtualization, kvm, netdev
In-Reply-To: <3d2dfa19-42f8-4d1f-a411-c72669f4c4bc@oracle.com>
The changelog is too long. I wrote it incrementally while investigating
the stale response-HVA case. English is not my first language, so some parts
came out wrong. In particular, saying that the ioctl waits for pre-update
commands may have suggested a full stop-new -> flush -> swap -> start-new
sequence. That was not what I meant; sorry about the confusion. If the patch
is otherwise acceptable, I will shorten the changelog in v2.
I wrote the patch myself. I used AI assistance only for notes and wording
help; it did not author or submit the code.
For this specific stale-response-HVA issue, I believe the patch is correct.
The patch is a return barrier: vhost_dev_ioctl() publishes the new memory
table, and vhost_scsi_flush() switches each vhost virtqueue to a new
inflight generation, flushes the vhost work, and waits for the old
generation's references. The completion path copies the response through
cmd->tvc_resp_iovs before releasing the old-generation reference. There is
no separate stop-new phase or full quiesce.
A command can arrive after vhost_dev_ioctl() returns and before
vhost_scsi_flush() switches that virtqueue's generation. It is assigned to the
old generation and is included in the flush, but vhost_set_memory() has
already updated that virtqueue's memory table, so its response iov uses the
new table. It therefore does not introduce another stale-HVA case. This is
why stopping new commands is not needed here.
An old-generation command may complete while this ioctl is still running in
the kernel, including while it is blocked in vhost_scsi_flush(); that is
expected. The same userspace thread cannot perform the remap and follow-up
TUR before this ioctl returns, because the thread is still blocked inside the
ioctl. The owner must keep the old mappings valid until the ioctl returns.
Remapping or dropping them from another userspace thread before then is
outside the lifetime assumption of this transition barrier.
^ permalink raw reply
* [PATCH] tools: Use fputc() calls in some functions
From: Markus Elfring @ 2026-07-23 10:25 UTC (permalink / raw)
To: acpica-devel, bpf, coresight, kvm, linux-acpi, linux-arm-kernel,
linux-gpio, linux-kselftest, linux-mm, linux-perf-users, linux-pm,
linux-trace-kernel, linuxppc-dev, mptcp, netdev,
platform-driver-x86, virtualization, Adrian Hunter, Alex Mastro,
Alex Williamson, Alexander Shishkin, Alexei Starovoitov,
Andi Kleen, Andrew Lunn, Andrew Morton, Andrii Nakryiko,
Ankur Arora, Arnaldo Carvalho de Melo, Bartosz Golaszewski,
Bobby Eshleman, Christian Bornträger, Christophe Leroy,
Chun-Tse Shao, Claudio Imbrenda, Costa Shulyupin, Crystal Wood,
Daniel Borkmann, Daniel Lezcano, David Hildenbrand, David Matlack,
David S. Miller, Donald Hunter, Eduard Zingerman, Emil Tsalapatis,
Eric Dumazet, Gabriele Monaco, Geliang Tang, Ian Rogers,
Ingo Molnar, Ivan Pravdin, Jakub Kicinski, James Clark,
Janosch Frank, Jason Xing, Jiri Olsa, Joe Damato, John Garry,
Josh Poimboeuf, Kaushlendra Kumar, Kent Gibson,
Kumar Kartikeya Dwivedi, Len Brown, Leo Yan, Lukasz Luba,
Madhavan Srinivasan, Mark Rutland, Martin KaFai Lau,
Masami Hiramatsu, Mat Martineau, Matthieu Baerts,
Michael Ellerman, Mike Leach, Mina Almasry, Mykyta Yatsenko,
Nam Cao, Namhyung Kim, Nicholas Piggin, Paolo Abeni,
Paolo Bonzini, Pawel Chmielewski, Peter Zijlstra, Quentin Monnet,
Rafael J. Wysocki, Raghavendra Rao Ananta, Saket Dumbre,
Shuah Khan, Simon Horman, Song Liu, Srinivas Pandruvada,
Stanislav Fomichev, Stefano Garzarella, Steven Rostedt,
Suzuki K Poulose, Swapnil Sapkal, Thomas Weißschuh,
Tomas Glozar, Vipin Sharma, Wander Lairson Costa, Will Deacon,
Willem de Bruijn, Willy Tarreau, Yonghong Song, Zhang Chujun,
Zhang Rui
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 23 Jul 2026 11:13:30 +0200
Subject: [PATCH] tools: Use fputc() calls in some functions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Single characters should occasionally be transferred into the output.
Thus use the corresponding function “fputc” (or even “putchar”).
The source code was transformed by using the Coccinelle software.
See also:
Improving output for single characters (with SmPL)?
https://lore.kernel.org/kernel-janitors/202dfb21-f5a2-4550-8a1f-aa07d84db345@web.de/
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
tools/bpf/bpftool/common.c | 4 ++--
tools/bpf/bpftool/main.c | 2 +-
tools/bpf/bpftool/netlink_dumper.h | 6 +++---
tools/bpf/bpftool/prog.c | 2 +-
tools/gpio/gpio-event-mon.c | 2 +-
tools/gpio/gpio-hammer.c | 4 ++--
tools/gpio/lsgpio.c | 4 ++--
tools/include/nolibc/err.h | 2 +-
tools/mm/page_owner_sort.c | 2 +-
tools/net/ynl/ynltool/main.c | 4 ++--
tools/objtool/builtin-check.c | 2 +-
tools/objtool/disas.c | 2 +-
tools/perf/builtin-c2c.c | 10 +++++-----
tools/perf/builtin-daemon.c | 4 ++--
tools/perf/builtin-lock.c | 14 +++++++-------
tools/perf/builtin-script.c | 8 ++++----
tools/perf/ui/gtk/util.c | 6 +++---
tools/perf/ui/stdio/hist.c | 8 ++++----
tools/perf/util/cs-etm.c | 2 +-
tools/perf/util/debug.c | 2 +-
| 6 +++---
tools/perf/util/intel-pt-decoder/intel-pt-log.c | 8 ++++----
tools/perf/util/libbfd.c | 2 +-
tools/perf/util/session.c | 4 ++--
tools/perf/util/stat-display.c | 12 ++++++------
tools/perf/util/values.c | 4 ++--
tools/power/acpi/tools/acpidump/apdump.c | 2 +-
tools/power/x86/intel-speed-select/isst-display.c | 6 +++---
tools/power/x86/turbostat/turbostat.c | 4 ++--
.../testing/selftests/bpf/benchs/bench_htab_mem.c | 2 +-
tools/testing/selftests/bpf/jit_disasm_helpers.c | 2 +-
tools/testing/selftests/bpf/test_progs.c | 12 ++++++------
tools/testing/selftests/bpf/veristat.c | 2 +-
tools/testing/selftests/drivers/net/hw/ncdevmem.c | 2 +-
tools/testing/selftests/kvm/s390/keyop.c | 2 +-
tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 2 +-
tools/testing/selftests/net/psock_tpacket.c | 4 ++--
tools/testing/selftests/net/txtimestamp.c | 10 +++++-----
.../testing/selftests/powerpc/nx-gzip/gunz_test.c | 2 +-
.../selftests/vfio/lib/include/libvfio/assert.h | 2 +-
tools/testing/selftests/vfio/lib/libvfio.c | 10 +++++-----
tools/testing/vsock/vsock_diag_test.c | 4 ++--
tools/thermal/tmon/sysfs.c | 2 +-
tools/thermal/tmon/tmon.c | 2 +-
tools/tracing/latency/latency-collector.c | 2 +-
tools/tracing/rtla/src/common.c | 2 +-
tools/tracing/rtla/src/utils.c | 2 +-
tools/usb/ffs-test.c | 2 +-
tools/verification/rv/src/in_kernel.c | 2 +-
49 files changed, 104 insertions(+), 104 deletions(-)
diff --git a/tools/bpf/bpftool/common.c b/tools/bpf/bpftool/common.c
index ef366ccc9650..3133058cfe40 100644
--- a/tools/bpf/bpftool/common.c
+++ b/tools/bpf/bpftool/common.c
@@ -53,7 +53,7 @@ void p_err(const char *fmt, ...)
} else {
fprintf(stderr, "Error: ");
vfprintf(stderr, fmt, ap);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
va_end(ap);
}
@@ -67,7 +67,7 @@ void p_info(const char *fmt, ...)
va_start(ap, fmt);
vfprintf(stderr, fmt, ap);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
va_end(ap);
}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index c91e1a6e1a1e..aecfc778625f 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -273,7 +273,7 @@ void fprint_hex(FILE *f, void *arg, unsigned int n, const char *sep)
if (!i)
/* nothing */;
else if (!(i % 16))
- fprintf(f, "\n");
+ fputc(f, '\n');
else if (!(i % 8))
fprintf(f, " ");
else
diff --git a/tools/bpf/bpftool/netlink_dumper.h b/tools/bpf/bpftool/netlink_dumper.h
index 96318106fb49..bf0919b4f52a 100644
--- a/tools/bpf/bpftool/netlink_dumper.h
+++ b/tools/bpf/bpftool/netlink_dumper.h
@@ -25,7 +25,7 @@
if (json_output) \
jsonw_start_object(json_wtr); \
else \
- fprintf(stdout, "{"); \
+ putchar('{'); \
}
#define NET_END_OBJECT_NESTED \
@@ -33,7 +33,7 @@
if (json_output) \
jsonw_end_object(json_wtr); \
else \
- fprintf(stdout, "}"); \
+ putchar('}'); \
}
#define NET_END_OBJECT \
@@ -47,7 +47,7 @@
if (json_output) \
jsonw_end_object(json_wtr); \
else \
- fprintf(stdout, "\n"); \
+ putchar('\n'); \
}
#define NET_START_ARRAY(name, fmt_str) \
diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
index a9f730d407a9..d27cb9d49860 100644
--- a/tools/bpf/bpftool/prog.c
+++ b/tools/bpf/bpftool/prog.c
@@ -1266,7 +1266,7 @@ static void hex_print(void *data, unsigned int size, FILE *f)
fprintf(f, "%c%s", c, j == i + 7 ? " " : "");
}
- fprintf(f, "\n");
+ fputc(f, '\n');
}
}
diff --git a/tools/gpio/gpio-event-mon.c b/tools/gpio/gpio-event-mon.c
index b70813b0bf8e..e12e14780c0e 100644
--- a/tools/gpio/gpio-event-mon.c
+++ b/tools/gpio/gpio-event-mon.c
@@ -121,7 +121,7 @@ int monitor_device(const char *device_name,
default:
fprintf(stdout, "unknown event");
}
- fprintf(stdout, "\n");
+ putchar('\n');
i++;
if (i == loops)
diff --git a/tools/gpio/gpio-hammer.c b/tools/gpio/gpio-hammer.c
index ba0866eb3581..b76c94fee04d 100644
--- a/tools/gpio/gpio-hammer.c
+++ b/tools/gpio/gpio-hammer.c
@@ -87,7 +87,7 @@ int hammer_device(const char *device_name, unsigned int *lines, int num_lines,
if (j == sizeof(swirr) - 1)
j = 0;
- fprintf(stdout, "[");
+ putchar('[');
for (i = 0; i < num_lines; i++) {
fprintf(stdout, "%u: %d", lines[i],
gpiotools_test_bit(values.bits, i));
@@ -101,7 +101,7 @@ int hammer_device(const char *device_name, unsigned int *lines, int num_lines,
if (loops && iteration == loops)
break;
}
- fprintf(stdout, "\n");
+ putchar('\n');
ret = 0;
exit_close_error:
diff --git a/tools/gpio/lsgpio.c b/tools/gpio/lsgpio.c
index 52a0be45410c..abb2c6fbc3d7 100644
--- a/tools/gpio/lsgpio.c
+++ b/tools/gpio/lsgpio.c
@@ -152,9 +152,9 @@ int list_device(const char *device_name)
if (linfo.flags) {
fprintf(stdout, " [");
print_attributes(&linfo);
- fprintf(stdout, "]");
+ putchar(']');
}
- fprintf(stdout, "\n");
+ putchar('\n');
}
diff --git a/tools/include/nolibc/err.h b/tools/include/nolibc/err.h
index e22ae87a7289..89c9a3d3bc77 100644
--- a/tools/include/nolibc/err.h
+++ b/tools/include/nolibc/err.h
@@ -27,7 +27,7 @@ void vwarnx(const char *fmt, va_list args)
{
fprintf(stderr, "%s: ", program_invocation_short_name);
vfprintf(stderr, fmt, args);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
static __attribute__((unused))
diff --git a/tools/mm/page_owner_sort.c b/tools/mm/page_owner_sort.c
index 35d3d254941c..0293a838211e 100644
--- a/tools/mm/page_owner_sort.c
+++ b/tools/mm/page_owner_sort.c
@@ -917,7 +917,7 @@ int main(int argc, char **argv)
}
if (cull & CULL_STACKTRACE)
fprintf(fout, ":\n%s", list[i].stacktrace);
- fprintf(fout, "\n");
+ fputc(fout, '\n');
}
}
diff --git a/tools/net/ynl/ynltool/main.c b/tools/net/ynl/ynltool/main.c
index 5d0f428eed0a..245d7f58c8fb 100644
--- a/tools/net/ynl/ynltool/main.c
+++ b/tools/net/ynl/ynltool/main.c
@@ -156,7 +156,7 @@ void p_err(const char *fmt, ...)
} else {
fprintf(stderr, "Error: ");
vfprintf(stderr, fmt, ap);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
va_end(ap);
}
@@ -170,7 +170,7 @@ void p_info(const char *fmt, ...)
va_start(ap, fmt);
vfprintf(stderr, fmt, ap);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
va_end(ap);
}
diff --git a/tools/objtool/builtin-check.c b/tools/objtool/builtin-check.c
index 118c3de2f293..b6d27d326b8a 100644
--- a/tools/objtool/builtin-check.c
+++ b/tools/objtool/builtin-check.c
@@ -297,7 +297,7 @@ int make_backup(void)
fprintf(stderr, " %s", arg);
}
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
return 0;
}
diff --git a/tools/objtool/disas.c b/tools/objtool/disas.c
index e6a54a83605c..339d1d5690c4 100644
--- a/tools/objtool/disas.c
+++ b/tools/objtool/disas.c
@@ -537,7 +537,7 @@ void disas_print_insn(FILE *stream, struct disas_context *dctx,
return;
if (strcmp(format, "\n") == 0) {
- fprintf(stream, "\n");
+ fputc(stream, '\n');
return;
}
diff --git a/tools/perf/builtin-c2c.c b/tools/perf/builtin-c2c.c
index c9584dbedf77..fc9b6b18eded 100644
--- a/tools/perf/builtin-c2c.c
+++ b/tools/perf/builtin-c2c.c
@@ -2511,7 +2511,7 @@ static void print_cacheline(struct c2c_hists *c2c_hists,
hists__fprintf_headers(&c2c_hists->hists, out);
once = true;
} else {
- fprintf(out, "\n");
+ fputc(out, '\n');
}
fprintf(out, " ----------------------------------------------------------------------\n");
@@ -2590,15 +2590,15 @@ static void perf_c2c__hists_fprintf(FILE *out, struct perf_session *session)
setup_pager();
print_c2c__display_stats(out);
- fprintf(out, "\n");
+ fputc(out, '\n');
print_shared_cacheline_info(out);
- fprintf(out, "\n");
+ fputc(out, '\n');
print_c2c_info(out, session);
if (c2c.stats_only)
return;
- fprintf(out, "\n");
+ fputc(out, '\n');
fprintf(out, "=================================================\n");
fprintf(out, " Shared Data Cache Line Table \n");
fprintf(out, "=================================================\n");
@@ -2606,7 +2606,7 @@ static void perf_c2c__hists_fprintf(FILE *out, struct perf_session *session)
hists__fprintf(&c2c.hists.hists, true, 0, 0, 0, stdout, true);
- fprintf(out, "\n");
+ fputc(out, '\n');
fprintf(out, "=================================================\n");
fprintf(out, " Shared Cache Line Distribution Pareto \n");
fprintf(out, "=================================================\n");
diff --git a/tools/perf/builtin-daemon.c b/tools/perf/builtin-daemon.c
index c4632577d129..7cb20b3a03e4 100644
--- a/tools/perf/builtin-daemon.c
+++ b/tools/perf/builtin-daemon.c
@@ -691,7 +691,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
/* session up time */
csv_sep, (uint64_t)((curr - daemon->start) / 60));
- fprintf(out, "\n");
+ fputc(out, '\n');
} else {
fprintf(out, "[%d:daemon] base: %s\n", getpid(), daemon->base);
if (cmd->list.verbose) {
@@ -730,7 +730,7 @@ static int cmd_session_list(struct daemon *daemon, union cmd *cmd, FILE *out)
/* session up time */
csv_sep, (uint64_t)((curr - session->start) / 60));
- fprintf(out, "\n");
+ fputc(out, '\n');
} else {
fprintf(out, "[%d:%s] perf record %s\n",
session->pid, session->name, session->run);
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index d5c0d55cb82d..3706aff25883 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -1319,9 +1319,9 @@ static void print_result(void)
list_for_each_entry(key, &lock_keys, list) {
key->print(key, st);
- fprintf(lock_output, " ");
+ fputc(lock_output, ' ');
}
- fprintf(lock_output, "\n");
+ fputc(lock_output, '\n');
if (++printed >= print_nr_entries)
break;
@@ -1598,7 +1598,7 @@ static void print_header_csv(const char *sep)
fprintf(lock_output, "%s%s %s", "type", sep, "caller");
if (verbose > 0)
fprintf(lock_output, "%s %s", sep, "stacktrace");
- fprintf(lock_output, "\n");
+ fputc(lock_output, '\n');
break;
case LOCK_AGGR_ADDR:
fprintf(lock_output, "%s%s %s%s %s\n", "address", sep, "symbol", sep, "type");
@@ -1629,7 +1629,7 @@ static void print_lock_stat_stdio(struct lock_contention *con, struct lock_stat
list_for_each_entry(key, &lock_keys, list) {
key->print(key, st);
- fprintf(lock_output, " ");
+ fputc(lock_output, ' ');
}
switch (aggr_mode) {
@@ -1687,7 +1687,7 @@ static void print_lock_stat_csv(struct lock_contention *con, struct lock_stat *s
case LOCK_AGGR_CALLER:
fprintf(lock_output, "%s%s %s", get_type_flags_name(st->flags), sep, st->name);
if (verbose <= 0)
- fprintf(lock_output, "\n");
+ fputc(lock_output, '\n');
break;
case LOCK_AGGR_TASK:
pid = st->addr;
@@ -1721,7 +1721,7 @@ static void print_lock_stat_csv(struct lock_contention *con, struct lock_stat *s
get_symbol_name_offset(kmap, sym, ip, buf, sizeof(buf));
fprintf(lock_output, "%s %#lx %s", i ? ":" : sep, (unsigned long) ip, buf);
}
- fprintf(lock_output, "\n");
+ fputc(lock_output, '\n');
}
}
@@ -1781,7 +1781,7 @@ static void print_footer_csv(int total, int bad, struct lock_contention_fails *f
for (i = 0; i < BROKEN_MAX; i++)
fprintf(lock_output, "%s bad_%s=%d", sep, name[i], bad_hist[i]);
}
- fprintf(lock_output, "\n");
+ fputc(lock_output, '\n');
}
static void print_footer(int total, int bad, struct lock_contention_fails *fails)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index f91d8b1fbd01..2b202018ba59 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -2563,7 +2563,7 @@ static void process_event(struct perf_script *script,
perf_sample__fprintf_ipc(sample, evsel, fp);
- fprintf(fp, "\n");
+ fputc(fp, '\n');
if (PRINT_FIELD(SRCCODE)) {
if (map__fprintf_srccode(al->map, al->addr, stdout,
@@ -2817,7 +2817,7 @@ static int process_deferred_sample_event(const struct perf_tool *tool,
cursor, symbol_conf.bt_stop_list, fp);
}
- fprintf(fp, "\n");
+ fputc(fp, '\n');
if (verbose > 0)
fflush(fp);
@@ -3273,11 +3273,11 @@ static int list_available_languages_cb(struct scripting_ops *ops, const char *sp
static void list_available_languages(void)
{
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
fprintf(stderr, "Scripting language extensions (used in "
"perf script -s [spec:]script.[spec]):\n\n");
script_spec__for_each(&list_available_languages_cb);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
/* Find script file relative to current directory or exec path */
diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
index c47f5c387838..037ebc47f7dc 100644
--- a/tools/perf/ui/gtk/util.c
+++ b/tools/perf/ui/gtk/util.c
@@ -37,7 +37,7 @@ static int perf_gtk__error(const char *format, va_list args)
vasprintf(&msg, format, args) < 0) {
fprintf(stderr, "Error:\n");
vfprintf(stderr, format, args);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
return -1;
}
@@ -62,7 +62,7 @@ static int perf_gtk__warning_info_bar(const char *format, va_list args)
vasprintf(&msg, format, args) < 0) {
fprintf(stderr, "Warning:\n");
vfprintf(stderr, format, args);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
return -1;
}
@@ -83,7 +83,7 @@ static int perf_gtk__warning_statusbar(const char *format, va_list args)
vasprintf(&msg, format, args) < 0) {
fprintf(stderr, "Warning:\n");
vfprintf(stderr, format, args);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
return -1;
}
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 8c4c8925df2c..044d2aa3b312 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -691,7 +691,7 @@ static int hists__fprintf_hierarchy_headers(struct hists *hists,
}
next_line:
- fprintf(fp, "\n");
+ fputc(fp, '\n');
}
fprintf(fp, "# ");
@@ -783,7 +783,7 @@ hists__fprintf_standard_headers(struct hists *hists,
if (line)
fprintf(fp, "# ");
fprintf_line(hists, hpp, line, fp);
- fprintf(fp, "\n");
+ fputc(fp, '\n');
}
if (sep)
@@ -806,10 +806,10 @@ hists__fprintf_standard_headers(struct hists *hists,
width = fmt->width(fmt, hpp, hists);
for (i = 0; i < width; i++)
- fprintf(fp, ".");
+ fputc(fp, '.');
}
- fprintf(fp, "\n");
+ fputc(fp, '\n');
fprintf(fp, "#\n");
return hpp_list->nr_header_lines + 2;
}
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 95e3ec1171ac..d4534b489252 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -912,7 +912,7 @@ static void cs_etm__dump_event(struct cs_etm_queue *etmq,
const char *color = PERF_COLOR_BLUE;
size_t buffer_used = 0;
- fprintf(stdout, "\n");
+ putchar('\n');
color_fprintf(stdout, color,
". ... CoreSight %s Trace data: size %#zx bytes\n",
cs_etm_decoder__get_name(etmq->decoder), buffer->size);
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 6b5ffe81f141..5429984ab49c 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -350,7 +350,7 @@ void __dump_stack(FILE *file, void **stackdump, size_t stackdump_size)
fprintf(file, " #%zd %p ", i, stackdump[i]);
map__fprintf_srcline(al.map, al.addr, "", file);
- fprintf(file, "\n");
+ fputc(file, '\n');
addr_location__exit(&al);
}
thread__put(thread);
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index e90e541f546b..e3a2a6e52ef0 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2454,7 +2454,7 @@ static void __print_pmu_caps(FILE *fp, int nr_caps, char **caps, char *pmu_name)
delimiter = ", ";
}
- fprintf(fp, "\n");
+ fputc(fp, '\n');
}
static void print_cpu_pmu_caps(struct feat_fd *ff, FILE *fp)
@@ -2513,7 +2513,7 @@ static void print_pmu_mappings(struct feat_fd *ff, FILE *fp)
pmu_num--;
}
- fprintf(fp, "\n");
+ fputc(fp, '\n');
if (!pmu_num)
return;
@@ -4349,7 +4349,7 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
fprintf(fp, "%s ", feat_ops[bit].name);
}
- fprintf(fp, "\n");
+ fputc(fp, '\n');
return 0;
}
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-log.c b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
index ef55d6232cf0..0a35493b854a 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-log.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-log.c
@@ -70,14 +70,14 @@ static void intel_pt_print_data(const unsigned char *buf, int len, uint64_t pos,
int i;
for (i = 0; i < indent; i++)
- fprintf(f, " ");
+ fputc(f, ' ');
fprintf(f, " %08" PRIx64 ": ", pos);
for (i = 0; i < len; i++)
fprintf(f, " %02x", buf[i]);
for (; i < 16; i++)
fprintf(f, " ");
- fprintf(f, " ");
+ fputc(f, ' ');
}
static void intel_pt_print_no_data(uint64_t pos, int indent)
@@ -85,12 +85,12 @@ static void intel_pt_print_no_data(uint64_t pos, int indent)
int i;
for (i = 0; i < indent; i++)
- fprintf(f, " ");
+ fputc(f, ' ');
fprintf(f, " %08" PRIx64 ": ", pos);
for (i = 0; i < 16; i++)
fprintf(f, " ");
- fprintf(f, " ");
+ fputc(f, ' ');
}
static ssize_t log_buf__write(void *cookie, const char *buf, size_t size)
diff --git a/tools/perf/util/libbfd.c b/tools/perf/util/libbfd.c
index d8241c7caac5..efbfdad8b330 100644
--- a/tools/perf/util/libbfd.c
+++ b/tools/perf/util/libbfd.c
@@ -603,7 +603,7 @@ int symbol__disassemble_bpf_libbfd(struct symbol *sym __maybe_unused,
} else
srcline = NULL;
- fprintf(s, "\n");
+ fputc(s, '\n');
prev_buf_size = buf_size;
fflush(s);
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 3237870a1a34..ef2b3226187c 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1257,7 +1257,7 @@ int perf_event__process_finished_round(const struct perf_tool *tool __maybe_unus
struct ordered_events *oe)
{
if (dump_trace)
- fprintf(stdout, "\n");
+ putchar('\n');
return ordered_events__flush(oe, OE_FLUSH__ROUND);
}
@@ -4015,7 +4015,7 @@ int perf_event__process_id_index(const struct perf_tool *tool __maybe_unused,
fprintf(stdout, " machine_pid: %"PRI_ld64, e2->machine_pid);
fprintf(stdout, " vcpu: %"PRI_lu64"\n", e2->vcpu);
} else {
- fprintf(stdout, "\n");
+ putchar('\n');
}
}
diff --git a/tools/perf/util/stat-display.c b/tools/perf/util/stat-display.c
index f94f1324d24a..b90e3be95549 100644
--- a/tools/perf/util/stat-display.c
+++ b/tools/perf/util/stat-display.c
@@ -529,7 +529,7 @@ static void print_metric_json(struct perf_stat_config *config __maybe_unused,
}
}
if (!config->metric_only)
- fprintf(out, "}");
+ fputc(out, '}');
}
static void new_line_json(struct perf_stat_config *config, void *ctx)
@@ -1316,7 +1316,7 @@ static void print_header_std(struct perf_stat_config *config,
FILE *output = config->output;
int i;
- fprintf(output, "\n");
+ fputc(output, '\n');
fprintf(output, " Performance counter stats for ");
if (_target->bpf_str)
fprintf(output, "\'BPF program(s) %s", _target->bpf_str);
@@ -1333,7 +1333,7 @@ static void print_header_std(struct perf_stat_config *config,
else
fprintf(output, "thread id \'%s", _target->tid);
- fprintf(output, "\'");
+ fputc(output, '\'');
if (config->run_count > 1)
fprintf(output, " (%d runs)", config->run_count);
fprintf(output, ":\n\n");
@@ -1406,9 +1406,9 @@ static void print_table(struct perf_stat_config *config, FILE *output, double av
fprintf(output, " %17.9f (%+.9f) ", run, run - avg);
for (h = 0; h < n; h++)
- fprintf(output, "#");
+ fputc(output, '#');
- fprintf(output, "\n");
+ fputc(output, '\n');
}
fprintf(output, "\n%*s# Final result:\n", indent, "");
@@ -1428,7 +1428,7 @@ static void print_footer(struct perf_stat_config *config)
return;
if (!config->null_run)
- fprintf(output, "\n");
+ fputc(output, '\n');
if (config->run_count == 1) {
fprintf(output, " %17.9f seconds time elapsed", avg);
diff --git a/tools/perf/util/values.c b/tools/perf/util/values.c
index 6eaddfcf833e..2c5340a928dd 100644
--- a/tools/perf/util/values.c
+++ b/tools/perf/util/values.c
@@ -217,7 +217,7 @@ static void perf_read_values__display_pretty(FILE *fp,
fprintf(fp, "# %*s %*s", pidwidth, "PID", tidwidth, "TID");
for (j = 0; j < values->num_counters; j++)
fprintf(fp, " %*s", counterwidth[j], evsel__name(values->counters[j]));
- fprintf(fp, "\n");
+ fputc(fp, '\n');
for (i = 0; i < values->threads; i++) {
fprintf(fp, " %*d %*d", pidwidth, values->pid[i],
@@ -225,7 +225,7 @@ static void perf_read_values__display_pretty(FILE *fp,
for (j = 0; j < values->num_counters; j++)
fprintf(fp, " %*" PRIu64,
counterwidth[j], values->value[i][j]);
- fprintf(fp, "\n");
+ fputc(fp, '\n');
}
free(counterwidth);
}
diff --git a/tools/power/acpi/tools/acpidump/apdump.c b/tools/power/acpi/tools/acpidump/apdump.c
index 72ad7915b04f..600d5b6e04a8 100644
--- a/tools/power/acpi/tools/acpidump/apdump.c
+++ b/tools/power/acpi/tools/acpidump/apdump.c
@@ -171,7 +171,7 @@ ap_dump_table_buffer(struct acpi_table_header *table,
acpi_ut_dump_buffer_to_file(gbl_output_file,
ACPI_CAST_PTR(u8, table), table_length,
DB_BYTE_DISPLAY, 0);
- fprintf(gbl_output_file, "\n");
+ fputc(gbl_output_file, '\n');
return (0);
}
diff --git a/tools/power/x86/intel-speed-select/isst-display.c b/tools/power/x86/intel-speed-select/isst-display.c
index e4884eb02837..5d492780aad9 100644
--- a/tools/power/x86/intel-speed-select/isst-display.c
+++ b/tools/power/x86/intel-speed-select/isst-display.c
@@ -122,7 +122,7 @@ static void format_and_print(FILE *outf, int level, char *header, char *value)
if (level == 0) {
if (header)
- fprintf(outf, "{");
+ fputc(outf, '{');
else
fprintf(outf, "\n}\n");
@@ -138,7 +138,7 @@ static void format_and_print(FILE *outf, int level, char *header, char *value)
if (value) {
if (last_level != level)
- fprintf(outf, "\n");
+ fputc(outf, '\n');
fprintf(outf, "%s\"%s\": ", delimiters, header);
fprintf(outf, "\"%s\"", value);
@@ -156,7 +156,7 @@ static void format_and_print(FILE *outf, int level, char *header, char *value)
fprintf(outf, "\n%s}", delimiters);
}
if (abs(last_level - level) < 3)
- fprintf(outf, "\n");
+ fputc(outf, '\n');
if (header)
fprintf(outf, "%s\"%s\": {", delimiters,
header);
diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c
index 4ad7cb1df5c5..bfa38199275f 100644
--- a/tools/power/x86/turbostat/turbostat.c
+++ b/tools/power/x86/turbostat/turbostat.c
@@ -9022,7 +9022,7 @@ void dump_cpuid_hypervisor(void)
dump_word_chars(ebx);
dump_word_chars(ecx);
dump_word_chars(edx);
- fprintf(outf, "\n");
+ fputc(outf, '\n');
}
void process_cpuid()
@@ -9692,7 +9692,7 @@ void topology_probe(bool startup)
fprintf(outf, " siblings");
for (ht_id = 0; ht_id <= MAX_HT_ID; ++ht_id)
fprintf(outf, " %d", cpus[i].ht_sibling_cpu_id[ht_id]);
- fprintf(outf, "\n");
+ fputc(outf, '\n');
}
}
diff --git a/tools/testing/selftests/bpf/benchs/bench_htab_mem.c b/tools/testing/selftests/bpf/benchs/bench_htab_mem.c
index 1ee217d97434..64b97a41af45 100644
--- a/tools/testing/selftests/bpf/benchs/bench_htab_mem.c
+++ b/tools/testing/selftests/bpf/benchs/bench_htab_mem.c
@@ -148,7 +148,7 @@ static const struct htab_mem_use_case *htab_mem_find_use_case_or_exit(const char
fprintf(stderr, "available use case:");
for (i = 0; i < ARRAY_SIZE(use_cases); i++)
fprintf(stderr, " %s", use_cases[i].name);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
exit(1);
}
diff --git a/tools/testing/selftests/bpf/jit_disasm_helpers.c b/tools/testing/selftests/bpf/jit_disasm_helpers.c
index 3558fe10e28c..58ea2a2ec13e 100644
--- a/tools/testing/selftests/bpf/jit_disasm_helpers.c
+++ b/tools/testing/selftests/bpf/jit_disasm_helpers.c
@@ -228,7 +228,7 @@ int get_jited_program_text(int fd, char *text, size_t text_sz)
for (pc = 0, i = 0; i < jited_funcs; ++i) {
fprintf(text_out, "func #%d:\n", i);
disasm_one_func(text_out, image + pc, func_lens[i]);
- fprintf(text_out, "\n");
+ fputc(text_out, '\n');
pc += func_lens[i];
}
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 7ba82974ee78..b520be1bf7aa 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -383,7 +383,7 @@ static void print_test_result(const struct prog_test_def *test, const struct tes
else
fprintf(env.stdout_saved, "OK (SKIP: %d/%d)", skipped_cnt, subtests_cnt);
- fprintf(env.stdout_saved, "\n");
+ fputc(env.stdout_saved, '\n');
}
static void print_test_log(char *log_buf, size_t log_cnt)
@@ -391,7 +391,7 @@ static void print_test_log(char *log_buf, size_t log_cnt)
log_buf[log_cnt] = '\0';
fprintf(env.stdout_saved, "%s", log_buf);
if (log_buf[log_cnt - 1] != '\n')
- fprintf(env.stdout_saved, "\n");
+ fputc(env.stdout_saved, '\n');
}
static void print_subtest_name(int test_num, int subtest_num,
@@ -409,7 +409,7 @@ static void print_subtest_name(int test_num, int subtest_num,
if (result)
fprintf(env.stdout_saved, ":%s", result);
- fprintf(env.stdout_saved, "\n");
+ fputc(env.stdout_saved, '\n');
}
static void jsonw_write_log_message(json_writer_t *w, char *log_buf, size_t log_cnt)
@@ -1333,14 +1333,14 @@ void hexdump(const char *prefix, const void *buf, size_t len)
for (int i = 0; i < len; i++) {
if (!(i % 16)) {
if (i)
- fprintf(stdout, "\n");
+ putchar('\n');
fprintf(stdout, "%s", prefix);
}
if (i && !(i % 8) && (i % 16))
- fprintf(stdout, "\t");
+ putchar('\t');
fprintf(stdout, "%02X ", ((uint8_t *)(buf))[i]);
}
- fprintf(stdout, "\n");
+ putchar('\n');
}
static void sigint_handler(int signum)
diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index c9c257784ee3..61ea29bb35d7 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -1628,7 +1628,7 @@ static void dump(__u32 prog_id, enum dump_mode mode, const char *file_name, cons
printf("DUMP (%s) %s/%s:\n", mode == DUMP_JITED ? "JITED" : "XLATED", file_name, prog_name);
while (fgets(buf, sizeof(buf), fp))
fputs(buf, stdout);
- fprintf(stdout, "\n");
+ putchar('\n');
if (ferror(fp))
fprintf(stderr, "Failed to dump BPF prog with error: %d\n", errno);
diff --git a/tools/testing/selftests/drivers/net/hw/ncdevmem.c b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
index d96e8a3b5a65..7bdcc4114dba 100644
--- a/tools/testing/selftests/drivers/net/hw/ncdevmem.c
+++ b/tools/testing/selftests/drivers/net/hw/ncdevmem.c
@@ -135,7 +135,7 @@ static void pr_err(const char *fmt, ...)
if (errno != 0)
fprintf(stderr, ": %s", strerror(errno));
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
static struct memory_buffer *udmabuf_alloc(size_t size)
diff --git a/tools/testing/selftests/kvm/s390/keyop.c b/tools/testing/selftests/kvm/s390/keyop.c
index c7805e87d12c..120834f8ba0d 100644
--- a/tools/testing/selftests/kvm/s390/keyop.c
+++ b/tools/testing/selftests/kvm/s390/keyop.c
@@ -117,7 +117,7 @@ static void dump_sk(const unsigned char skeys[], const char *descr)
fprintf(stderr, "# %3d: ", i);
for (j = 0; j < 32; j++)
fprintf(stderr, "%02x ", skeys[i + j]);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
}
diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 78180da1efcc..40467e86bfd4 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -208,7 +208,7 @@ static int capture_events(int fd, int event_group)
}
if (server_side)
fprintf(stderr, ",server_side:1");
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
} while (1);
return 0;
diff --git a/tools/testing/selftests/net/psock_tpacket.c b/tools/testing/selftests/net/psock_tpacket.c
index 7caf3135448d..724da9222ad9 100644
--- a/tools/testing/selftests/net/psock_tpacket.c
+++ b/tools/testing/selftests/net/psock_tpacket.c
@@ -116,7 +116,7 @@ static int pfsocket(int ver)
static void status_bar_update(void)
{
if (total_packets % 10 == 0) {
- fprintf(stderr, ".");
+ fputc(stderr, '.');
fflush(stderr);
}
}
@@ -825,7 +825,7 @@ static int test_tpacket(int version, int type)
unmap_ring(sock, &ring);
close(sock);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
return 0;
}
diff --git a/tools/testing/selftests/net/txtimestamp.c b/tools/testing/selftests/net/txtimestamp.c
index 170be192f5c7..86c8d4945d73 100644
--- a/tools/testing/selftests/net/txtimestamp.c
+++ b/tools/testing/selftests/net/txtimestamp.c
@@ -200,10 +200,10 @@ static void __print_timestamp(const char *name, struct timespec *cur,
ts_delta = timespec_to_ns64(cur) - timespec_to_ns64(&ts_usr);
fprintf(stderr, " (USR +");
__print_ts_delta_formatted(ts_delta);
- fprintf(stderr, ")");
+ fputc(stderr, ')');
}
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
static void record_timestamp_usr(void)
@@ -254,7 +254,7 @@ static void print_timing_event(char *name, struct timing_event *te)
__print_ts_delta_formatted(te->min);
fprintf(stderr, ", max=");
__print_ts_delta_formatted(te->max);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
/* TODO: convert to check_and_print payload once API is stable */
@@ -271,7 +271,7 @@ static void print_payload(char *data, int len)
fprintf(stderr, "payload: ");
for (i = 0; i < len; i++)
fprintf(stderr, "%02hhx ", data[i]);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
static void print_pktinfo(int family, int ifindex, void *saddr, void *daddr)
@@ -933,7 +933,7 @@ int main(int argc, char **argv)
fprintf(stderr, "protocol: %s\n", sock_names[cfg_proto]);
fprintf(stderr, "payload: %u\n", cfg_payload_len);
fprintf(stderr, "server port: %u\n", dest_port);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
if (do_ipv4) {
if (cfg_do_listen)
diff --git a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
index 7c23d3dd7d6d..412defe98271 100644
--- a/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
+++ b/tools/testing/selftests/powerpc/nx-gzip/gunz_test.c
@@ -363,7 +363,7 @@ int decompress_file(int argc, char **argv, void *devhandle)
goto err3;
fprintf(stderr, "%02x ", tmp[i]);
if (i == 5)
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
fprintf(stderr, "gzHeader MTIME, XFL, OS ignored\n");
diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/assert.h b/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
index 9fff88f6e4e1..682267bb71e3 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/assert.h
@@ -11,7 +11,7 @@
#define VFIO_LOG_AND_EXIT(...) do { \
fprintf(stderr, " " __VA_ARGS__); \
- fprintf(stderr, "\n"); \
+ fputc(stderr, '\n'); \
exit(KSFT_FAIL); \
} while (0)
diff --git a/tools/testing/selftests/vfio/lib/libvfio.c b/tools/testing/selftests/vfio/lib/libvfio.c
index 3a3d1ed635c1..80a312f149a6 100644
--- a/tools/testing/selftests/vfio/lib/libvfio.c
+++ b/tools/testing/selftests/vfio/lib/libvfio.c
@@ -60,16 +60,16 @@ char **vfio_selftests_get_bdfs(int *argc, char *argv[], int *nr_bdfs)
}
fprintf(stderr, "Unable to determine which device(s) to use, skipping test.\n");
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
fprintf(stderr, "To pass the device address via environment variable:\n");
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
fprintf(stderr, " export VFIO_SELFTESTS_BDF=\"segment:bus:device.function\"\n");
fprintf(stderr, " %s [options]\n", argv[0]);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
fprintf(stderr, "To pass the device address(es) via argv:\n");
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
fprintf(stderr, " %s [options] segment:bus:device.function ...\n", argv[0]);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
exit(KSFT_SKIP);
}
diff --git a/tools/testing/vsock/vsock_diag_test.c b/tools/testing/vsock/vsock_diag_test.c
index 081e045f4696..cc903cdb45d9 100644
--- a/tools/testing/vsock/vsock_diag_test.c
+++ b/tools/testing/vsock/vsock_diag_test.c
@@ -86,7 +86,7 @@ static void print_vsock_addr(FILE *fp, unsigned int cid, unsigned int port)
fprintf(fp, "%u:", cid);
if (port == VMADDR_PORT_ANY)
- fprintf(fp, "*");
+ fputc(fp, '*');
else
fprintf(fp, "%u", port);
}
@@ -94,7 +94,7 @@ static void print_vsock_addr(FILE *fp, unsigned int cid, unsigned int port)
static void print_vsock_stat(FILE *fp, struct vsock_stat *st)
{
print_vsock_addr(fp, st->msg.vdiag_src_cid, st->msg.vdiag_src_port);
- fprintf(fp, " ");
+ fputc(fp, ' ');
print_vsock_addr(fp, st->msg.vdiag_dst_cid, st->msg.vdiag_dst_port);
fprintf(fp, " %s %s %s %u\n",
sock_type_str(st->msg.vdiag_type),
diff --git a/tools/thermal/tmon/sysfs.c b/tools/thermal/tmon/sysfs.c
index cb1108bc9249..ebdb95819a76 100644
--- a/tools/thermal/tmon/sysfs.c
+++ b/tools/thermal/tmon/sysfs.c
@@ -522,7 +522,7 @@ int update_thermal_data()
}
if (tmon_log) {
- fprintf(tmon_log, "\n");
+ fputc(tmon_log, '\n');
fflush(tmon_log);
}
diff --git a/tools/thermal/tmon/tmon.c b/tools/thermal/tmon/tmon.c
index 7eb3216a27f4..9a972f73a2c0 100644
--- a/tools/thermal/tmon/tmon.c
+++ b/tools/thermal/tmon/tmon.c
@@ -199,7 +199,7 @@ static void prepare_logging(void)
fprintf(tmon_log, "%s%d ", ptdata.cdi[i].type,
ptdata.cdi[i].instance);
- fprintf(tmon_log, "\n");
+ fputc(tmon_log, '\n');
}
static struct option opts[] = {
diff --git a/tools/tracing/latency/latency-collector.c b/tools/tracing/latency/latency-collector.c
index ef97916e3873..3dc6fb9df926 100644
--- a/tools/tracing/latency/latency-collector.c
+++ b/tools/tracing/latency/latency-collector.c
@@ -1999,7 +1999,7 @@ static void scan_arguments(int argc, char *argv[])
"been enabled. Random sleep is intended for the following tracers:\n");
for (i = 0; random_tracers[i]; i++)
fprintf(stderr, "%s\n", random_tracers[i]);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
}
diff --git a/tools/tracing/rtla/src/common.c b/tools/tracing/rtla/src/common.c
index 8c7f5e75b2ec..e81e676d51d2 100644
--- a/tools/tracing/rtla/src/common.c
+++ b/tools/tracing/rtla/src/common.c
@@ -449,7 +449,7 @@ void common_usage(const char *tool, const char *mode,
fprintf(stderr, "%s [-h] ", mode);
print_msg_array(start_msgs);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
print_msg_array(common_options);
print_msg_array(opt_msgs);
diff --git a/tools/tracing/rtla/src/utils.c b/tools/tracing/rtla/src/utils.c
index cb187e7d48d1..3532da2f2296 100644
--- a/tools/tracing/rtla/src/utils.c
+++ b/tools/tracing/rtla/src/utils.c
@@ -67,7 +67,7 @@ void fatal(const char *fmt, ...)
va_start(ap, fmt);
vfprintf(stderr, fmt, ap);
va_end(ap);
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
exit(ERROR);
}
diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
index 22b938fbdfb7..e993736ba0d6 100644
--- a/tools/usb/ffs-test.c
+++ b/tools/usb/ffs-test.c
@@ -562,7 +562,7 @@ empty_out_buf(struct thread *ignore, const void *buf, size_t nbytes)
fprintf(stderr, "%4zd:", len);
fprintf(stderr, " %02x", *p);
if (31 == (len % 32))
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
fflush(stderr);
errno = EILSEQ;
diff --git a/tools/verification/rv/src/in_kernel.c b/tools/verification/rv/src/in_kernel.c
index e6dea4040f8f..26132171f600 100644
--- a/tools/verification/rv/src/in_kernel.c
+++ b/tools/verification/rv/src/in_kernel.c
@@ -667,7 +667,7 @@ static void ikm_usage_print_reactors(void)
end = strstr(start, "\n");
}
- fprintf(stderr, "\n");
+ fputc(stderr, '\n');
}
/*
* ikm_usage - print usage
--
2.55.0
^ permalink raw reply related
* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
From: Nguyen Dinh Phi [SG] @ 2026-07-23 10:26 UTC (permalink / raw)
To: Stefano Garzarella, Michal Luczaj
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, syzbot+1b2c9c4a0f8708082678, virtualization, netdev,
linux-kernel
In-Reply-To: <amHN9mOGj7oMak4-@sgarzare-redhat>
On 23/7/26 16:24, Stefano Garzarella wrote:
> On Thu, Jul 23, 2026 at 08:00:31AM +0200, Michal Luczaj wrote:
>> On 7/23/26 06:14, Nguyen Dinh Phi [SG] wrote:
>>> On 22/7/26 15:55, Stefano Garzarella wrote:
>>>> On Tue, Jul 21, 2026 at 01:34:03AM +0800, Phi Nguyen wrote:
>>>>> On 7/20/2026 4:17 PM, Stefano Garzarella wrote:
>>>>>> On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>>>>>>> After vsock_connect() exits the wait loop due to sk->sk_err being
>>>>>>> set, the error was read but not cleared. This left sk->sk_err set
>>>>>>> for subsequent operations.
>>>>>>
>>>>>> So, is this a fix? If yes, we should put a Fixes tag.
>>>>>>
>>>>>> Also, can you describe how to trigger the issue?
>>>>>>
>>>>>> Because I see this in vsock_connect(), so I thought it was in some
>>>>>> way already handled:
>>>>>>
>>>>>> /* sk_err might have been set as a result of an earlier
>>>>>> * (failed) connect attempt.
>>>>>> */
>>>>>> sk->sk_err = 0;
>>>>>>
>>>>> This only handles the case where the function following the failed
>>>>> connect is another connect() call.
>>>>
>>>> So, can we remove that with this patch, or better to leave as defensive
>>>> action?
>>>>
>>>
>>> I prefer to keep it here as defensive action
>>
>> Is changing how vsock_poll() behaves intended?
>
I think connect() already delivered the error directly to the
caller when it returned -- there's no reason for poll() to keep
reporting that same, already-handled error afterward.
> Good point, but IIUC __inet_stream_connect() is also using consuming the
> error with sock_error().
>
>>
>> I.e. if sk_err should be kept after a failed connect(), what about
>> `sk->sk_err = 0;` in vsock_listen() instead?
>
> Yeah, maybe this is a bit less invasive.
>
yes, __inet_stream_connect() does it, and actually, other protocols like
Bluetooth, TIPC, x25... are also consume sk_err with sock_error() after
their wait loop.
I think it is an established pattern, so I'd rather keep sock_error() in
vsock_connect() than move it to vsock_listen(). vsock_listen() would
only protect the listen() path; I see other places in af_vsock.c use
this sk_err.
>>
>>>> ...
>>>> Yeah, we need to handle that part better, I think it's a leftover when
>>>> we generalized AF_VSOCK to support more transport than vmci.
>>
>> Speaking of leftovers, I have trouble understanding where does vsock set
>> sk_err on listener sockets anyway. If it doesn't, why vsock_accept()
>> checks
>> for it?
>
> I can't also see where it can be set TBH. Should we remove it ?
I couldn't find it for listener side too.
Thanks,
Phi.
^ permalink raw reply
* Re: [PATCH v3] mm/page_reporting: use system_freezable_wq to fix UAF during suspend
From: Michael S. Tsirkin @ 2026-07-23 9:15 UTC (permalink / raw)
To: Link Lin
Cc: Andrew Morton, Vlastimil Babka, David Hildenbrand, virtualization,
linux-mm, linux-kernel, prasin, rientjes, duenwen, jasowang,
xuanzhuo, Ammar Faizi, jiaqiyan, ahwilkins, Greg Thelen,
Alexander Duyck, jthoughton, stable
In-Reply-To: <CALUx4KSfUUud86k4SLwETJAciRc8kZ9QocvBj39KmzXDiQ7J1g@mail.gmail.com>
On Wed, Jul 22, 2026 at 09:07:04PM -0700, Link Lin wrote:
> On Tue, Jul 21, 2026 at 5:36 PM Andrew Morton <akpm@linux-foundation.org> wrote:
> > hm, now where did that come from. I can find no such commit and that's
> > the second time this very unusual thing has happened in 30 minutes! I
> > wonder what's going on.
> >
> > I'll use
> > 36e66c554b5c ("mm: introduce Reported pages")
> > OK?
>
> Yes, that Fixes tag is perfectly OK. I pulled the previous hash from
> an internal downstream tree by mistake. Thank you for catching that
> and correcting it!
>
> > The bug is very old so I won't fast-track this fix into 7.2-rcX.
>
> Completely understood and agreed.
>
> > AI review pointed at a possible pre-existing use-after-free issue,
> > related to virtio-balloon. But I think this is a rephrasing of the
> > issue it flagged against your v2 patch.
>
> I actually looked closely at Sashiko's flag, and to my surprise, it is not a
> rephrasing—it caught a completely separate, valid edge case.
>
> My patch fixes the UAF on the PM suspend/teardown path. However, Sashiko noticed
> a UAF on the PM *restore* error path. If virtballoon_restore() fails during
> init_vqs(), it aborts without unregistering the page reporting worker. When
> system_freezable_wq thaws, the reporting worker wakes up and dereferences the
> now-dangling vb->reporting_vq pointer.
>
> Since it's a driver-specific lifecycle bug rather than a core MM workqueue
> issue, I will write up a separate follow-up patch to address it in
> virtio_balloon.c
> shortly.
>
> Thank you again for shepherding this fix into -mm!
>
> Best,
> Link
I suspect rest of work items we have, e.g. update_balloon_stats_work/update_balloon_size_work
all have issues around freeze/restore and error handling.
--
MST
^ permalink raw reply
* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
From: Stefano Garzarella @ 2026-07-23 8:24 UTC (permalink / raw)
To: Michal Luczaj
Cc: Nguyen Dinh Phi [SG], David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman,
syzbot+1b2c9c4a0f8708082678, virtualization, netdev, linux-kernel
In-Reply-To: <95cd0d4e-58c9-44ab-b94f-fcf57b88583b@rbox.co>
On Thu, Jul 23, 2026 at 08:00:31AM +0200, Michal Luczaj wrote:
>On 7/23/26 06:14, Nguyen Dinh Phi [SG] wrote:
>> On 22/7/26 15:55, Stefano Garzarella wrote:
>>> On Tue, Jul 21, 2026 at 01:34:03AM +0800, Phi Nguyen wrote:
>>>> On 7/20/2026 4:17 PM, Stefano Garzarella wrote:
>>>>> On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>>>>>> After vsock_connect() exits the wait loop due to sk->sk_err being
>>>>>> set, the error was read but not cleared. This left sk->sk_err set
>>>>>> for subsequent operations.
>>>>>
>>>>> So, is this a fix? If yes, we should put a Fixes tag.
>>>>>
>>>>> Also, can you describe how to trigger the issue?
>>>>>
>>>>> Because I see this in vsock_connect(), so I thought it was in some
>>>>> way already handled:
>>>>>
>>>>> /* sk_err might have been set as a result of an earlier
>>>>> * (failed) connect attempt.
>>>>> */
>>>>> sk->sk_err = 0;
>>>>>
>>>> This only handles the case where the function following the failed
>>>> connect is another connect() call.
>>>
>>> So, can we remove that with this patch, or better to leave as defensive
>>> action?
>>>
>>
>> I prefer to keep it here as defensive action
>
>Is changing how vsock_poll() behaves intended?
Good point, but IIUC __inet_stream_connect() is also using consuming the
error with sock_error().
>
>I.e. if sk_err should be kept after a failed connect(), what about
>`sk->sk_err = 0;` in vsock_listen() instead?
Yeah, maybe this is a bit less invasive.
>
>>> ...
>>> Yeah, we need to handle that part better, I think it's a leftover when
>>> we generalized AF_VSOCK to support more transport than vmci.
>
>Speaking of leftovers, I have trouble understanding where does vsock set
>sk_err on listener sockets anyway. If it doesn't, why vsock_accept() checks
>for it?
I can't also see where it can be set TBH. Should we remove it ?
Thanks,
Stefano
^ permalink raw reply
* Re: [PATCH v2] drm/virtio: fix deadlock in display_info_cb by removing hotplug from dequeue worker
From: Mikko Rapeli @ 2026-07-23 7:14 UTC (permalink / raw)
To: Dmitry Osipenko, stable
Cc: Ryosuke Yasuoka, David Airlie, Gerd Hoffmann, Gurchetan Singh,
Chia-I Wu, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
Simona Vetter, Dmitry Baryshkov, Javier Martinez Canillas,
dri-devel, virtualization, linux-kernel
In-Reply-To: <ba8a2de8-8ac7-490d-b48e-767d1e7893a9@collabora.com>
Hi, adding stable@vger.kernel.org
In Yocto distro, this change released in v7.2-rc4 seems to fix quite severe qemu
boot hangs seen with 6.18 stable kernels. Details in
https://bugzilla.yoctoproject.org/show_bug.cgi?id=16217
Please apply this to to 6.18 and other stable trees.
Cheers,
-Mikko
On Mon, Jul 13, 2026 at 07:31:14PM +0300, Dmitry Osipenko wrote:
> On 7/13/26 16:01, Ryosuke Yasuoka wrote:
> > A probe-time deadlock can occur between the dequeue worker and
> > drm_client_register(). During probe, drm_client_register() holds
> > clientlist_mutex and calls the fbdev hotplug callback, which triggers an
> > atomic commit that ends up sleeping in virtio_gpu_queue_ctrl_sgs()
> > waiting for virtqueue space. The dequeue worker that would free that
> > space calls virtio_gpu_cmd_get_display_info_cb(), which invokes
> > drm_kms_helper_hotplug_event() -> drm_client_dev_hotplug(), attempting
> > to acquire the same clientlist_mutex. Since wake_up() is only called
> > after the resp_cb loop, the probe thread is never woken and both threads
> > deadlock.
> >
> > Fix this by removing the hotplug notification from
> > virtio_gpu_cmd_get_display_info_cb(). The display data (outputs[i].info)
> > is still updated synchronously in the callback.
> >
> > For the init path, drm_client_register() already fires an initial
> > hotplug when the client is registered, which picks up the connector
> > state updated by display_info_cb.
> >
> > For the runtime config_changed path, add a wait_event_timeout() in
> > config_changed_work_func() so that display_info_cb updates the connector
> > data before the hotplug notification is sent. Also replace
> > drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event() since
> > virtio-gpu never calls drm_kms_helper_poll_init() and thus
> > drm_helper_hpd_irq_event() always returns false without doing anything.
> >
> > Fixes: 27655b9bb9f0 ("drm/client: Send hotplug event after registering a client")
> > Closes: https://syzkaller.appspot.com/bug?id=d6dd6f86d3aaf7eebe7406e45c1c6e549453f224
> > Closes: https://syzkaller.appspot.com/bug?id=908bd910da5dd79b88de4cf7baf376cc873a922e
> > Suggested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
> > ---
> > I checked whether drm_helper_hpd_irq_event() is needed in
> > virtio_gpu_init(), as Dmitry suggested. AFAIS, it is not needed because:
> >
> > 1. drm_helper_hpd_irq_event() is always a no-op in virtio-gpu.
> > It returns false immediately probe_helper.c:1088 because
> > dev->mode_config.poll_enabled is false — virtio-gpu never calls
> > drm_kms_helper_poll_init(). Even if it passed that gate, no
> > virtio-gpu connectors set DRM_CONNECTOR_POLL_HPD.
> >
> > 1082 bool drm_helper_hpd_irq_event(struct drm_device *dev)
> > 1083 {
> > ...
> > 1088 if (!dev->mode_config.poll_enabled)
> > 1089 return false;
> >
> > 2. virtio_gpu_init() runs before drm_dev_register() and
> > drm_client_setup(), so no DRM clients are registered yet.
> > drm_kms_helper_hotplug_event() would iterate an empty client list.
> > The initial hotplug is handled by drm_client_register(), which fires
> > a hotplug callback to the newly registered client. By that time,
> > display_info_cb has already updated the connector data.
> >
> > For the same reason, drm_helper_hpd_irq_event() in
> > config_changed_work_func() was also a no-op. The actual runtime hotplug
> > notification was always delivered by display_info_cb's call to
> > drm_kms_helper_hotplug_event(). This patch replaces it with a direct
> > drm_kms_helper_hotplug_event() call after waiting for the display info
> > response.
> > ---
> > Changes in v2:
> > - Dropped the work_struct approach from v1.
> > - Instead, removed the hotplug calls from display_info_cb entirely, as
> > suggested by Dmitry.
> > - Added wait_event_timeout() in config_changed_work_func() so that the
> > display info response is received before sending the hotplug
> > notification.
> > - Replaced drm_helper_hpd_irq_event() with drm_kms_helper_hotplug_event()
> > in config_changed_work_func() since drm_helper_hpd_irq_event() is
> > always a no-op in virtio-gpu (poll_enabled is never set).
> > - No changes to virtio_gpu_init() — drm_client_register() already
> > handles the initial hotplug and hotplug event does nothing before DRM
> > device/client has been registered.
> > - Link to v1: https://lore.kernel.org/r/20260630-virtiogpu_syzbot-v1-1-0aa06630750e@redhat.com
> > ---
> > drivers/gpu/drm/virtio/virtgpu_kms.c | 5 ++++-
> > drivers/gpu/drm/virtio/virtgpu_vq.c | 3 ---
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > index cfde9f573df6..b4329f28e976 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> > @@ -49,7 +49,10 @@ static void virtio_gpu_config_changed_work_func(struct work_struct *work)
> > virtio_gpu_cmd_get_edids(vgdev);
> > virtio_gpu_cmd_get_display_info(vgdev);
> > virtio_gpu_notify(vgdev);
> > - drm_helper_hpd_irq_event(vgdev->ddev);
> > + wait_event_timeout(vgdev->resp_wq,
> > + !vgdev->display_info_pending,
> > + 5 * HZ);
> > + drm_kms_helper_hotplug_event(vgdev->ddev);
> > }
> > events_clear |= VIRTIO_GPU_EVENT_DISPLAY;
> > }
> > diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> > index c8b9475a7472..e5e1af8b8e8a 100644
> > --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> > +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> > @@ -840,9 +840,6 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
> > vgdev->display_info_pending = false;
> > spin_unlock(&vgdev->display_info_lock);
> > wake_up(&vgdev->resp_wq);
> > -
> > - if (!drm_helper_hpd_irq_event(vgdev->ddev))
> > - drm_kms_helper_hotplug_event(vgdev->ddev);
> > }
> >
> > static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
> >
> > ---
> > base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
> > change-id: 20260619-virtiogpu_syzbot-bdab508ffcd5
> >
> > Best regards,
>
> Appled to misc-fixes, thanks!
>
> --
> Best regards,
> Dmitry
^ permalink raw reply
* Re: [PATCH] vsock: use sock_error() to consume sk_err after connect timeout
From: Michal Luczaj @ 2026-07-23 6:00 UTC (permalink / raw)
To: Nguyen Dinh Phi [SG], Stefano Garzarella
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, syzbot+1b2c9c4a0f8708082678, virtualization, netdev,
linux-kernel
In-Reply-To: <b81bfea4-6b19-4cc8-95c0-dcd4ad42476e@gmail.com>
On 7/23/26 06:14, Nguyen Dinh Phi [SG] wrote:
> On 22/7/26 15:55, Stefano Garzarella wrote:
>> On Tue, Jul 21, 2026 at 01:34:03AM +0800, Phi Nguyen wrote:
>>> On 7/20/2026 4:17 PM, Stefano Garzarella wrote:
>>>> On Mon, Jul 20, 2026 at 05:57:47AM +0800, Nguyen Dinh Phi wrote:
>>>>> After vsock_connect() exits the wait loop due to sk->sk_err being
>>>>> set, the error was read but not cleared. This left sk->sk_err set
>>>>> for subsequent operations.
>>>>
>>>> So, is this a fix? If yes, we should put a Fixes tag.
>>>>
>>>> Also, can you describe how to trigger the issue?
>>>>
>>>> Because I see this in vsock_connect(), so I thought it was in some
>>>> way already handled:
>>>>
>>>> /* sk_err might have been set as a result of an earlier
>>>> * (failed) connect attempt.
>>>> */
>>>> sk->sk_err = 0;
>>>>
>>> This only handles the case where the function following the failed
>>> connect is another connect() call.
>>
>> So, can we remove that with this patch, or better to leave as defensive
>> action?
>>
>
> I prefer to keep it here as defensive action
Is changing how vsock_poll() behaves intended?
I.e. if sk_err should be kept after a failed connect(), what about
`sk->sk_err = 0;` in vsock_listen() instead?
>> ...
>> Yeah, we need to handle that part better, I think it's a leftover when
>> we generalized AF_VSOCK to support more transport than vmci.
Speaking of leftovers, I have trouble understanding where does vsock set
sk_err on listener sockets anyway. If it doesn't, why vsock_accept() checks
for it?
thanks,
Michal
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox