Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci
On Sat, Mar 11, 2023 at 05:19:43PM -0500, Feng Liu wrote:
>
>
> On 2023-03-11 p.m.2:06, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
> > >
> > >
> > > On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
> > > > External email: Use caution opening links or attachments
> > > >
> > > >
> > > > On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
> > > > > This patch series performs a clean up of the code in virtio_ring and
> > > > > virtio_pci, modifying it to conform with the Linux kernel coding style
> > > > > guidance [1]. The modifications ensure the code easy to read and
> > > > > understand. This small series does few short cleanups in the code.
> > > > >
> > > > > Patch-1 Allow non power of 2 sizes for virtqueues
> > > > > Patch-2 Avoid using inline for small functions.
> > > > > Patch-3 Use const to annotate read-only pointer params.
> > > > >
> > > > > [1]
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease=05%7C01%7Cfeliu%40nvidia.com%7C6cd34740c4674c1892f608db2263b300%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638141583834629671%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=IA0QCCKjHnYiEk2vPlZ5WjlXs1CMXDphyyqTYnbqQqo%3D=0
> > > > >
> > > > > All of the patches have been verified based on the kernel code
> > > > > commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> > > >
> > > > verified how?
> > > >
> > > Hi Michael
> > > 1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
> > > install the kernel, and use iperf to test traffic
> > > 2. To validate this change, we tested various virtqueue sizes for packed
> > > rings, including 128, 256, 512, 100, 200, 500, and 1000, with
> > > CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
> > > passed successfully.
> >
> > Given split ring does not support non power of 2 how exactly
> > did you configure non power of 2?
> >
>
> Hi, Michael
> We can implement the test by modifying qemu; 1. force the
> VIRTIO_F_RING_PACKED feature bit to be set, 2. set
> VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE and VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE to the
> value of non power_of_2, 3. remove the check of is_power_of virtqueue, then
> qemu can create the required virtual device (non power_2 size , packed
> virtqueue device) ;In this way, any length and packed ring test can be
> performed;
> remove the modified code, I can test split vq, and can see that the size of
> power_of_2 can load the driver normally, and the size of non power_of_2 will
> give an warning and fail to load the driver
Sounds like a plan but what exactly did you do previously?
You indicated you tested non powers of 2.
> > >
> > > > > Feng Liu (3):
> > > > > virtio_pci_modern: Allow non power of 2 sizes for virtqueues
> > > > > virtio_ring: Avoid using inline for small functions
> > > > > virtio_ring: Use const to annotate read-only pointer params
> > > > >
> > > > >drivers/virtio/virtio_pci_modern.c | 5
> > > > >drivers/virtio/virtio_ring.c | 48
> > > > > +++---
> > > > >include/linux/virtio.h | 14 -
> > > > >3 files changed, 31 insertions(+), 36 deletions(-)
> > > > >
> > > > > --
> > > > > 2.34.1
> > > >
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues
On Sat, Mar 11, 2023 at 05:25:04PM -0500, Feng Liu wrote:
>
>
> On 2023-03-11 p.m.2:05, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote:
> > >
> > >
> > > On 2023-03-10 a.m.8:36, Parav Pandit wrote:
> > > >
> > > >
> > > > > From: Feng Liu
> > > > > Sent: Friday, March 10, 2023 12:34 AM
> > > >
> > > > >
> > > > > - if (!is_power_of_2(num)) {
> > > > > - dev_warn(_dev->pci_dev->dev, "bad queue size %u",
> > > > > num);
> > > > > - return ERR_PTR(-EINVAL);
> > > > > - }
> > > > > -
> > > >
> > > > The check is still valid for split q.
> > > > Maybe the right place for such a check is not the pci transport driver.
> > > > But layer below where split vs packed q knowledge resides and hence,
> > > > power of 2 check can be omitted for packed vq.
> > >
> > > Hi, Parav
> > > I think you are right, I checked the virtio spec, only packed
> > > virtqueue
> > > can use queue size which is not power_of_2; so, I think the check can be
> > > reserved only for split queue here, something like
> > >
> > > struct virtio_device *vdev = _dev->vdev;
> > > if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
> > > && !is_power_of_2(num)){
> > > dev_warn(_dev->pci_dev->dev, "bad queue size %u", num);
> > > return ERR_PTR(-EINVAL);
> > > }
> > >
> > > I will fix it in next version
> > >
> > > Hi, Michael and Jason
> > > Do you have any comments on this?
> > >
> >
> > Hmm good point. Which raises the question: so how did you test it then?
> >
> Hi Michael
>
> I will construct a non power of 2 size packed virtqueue device to test
> whether the driver is loaded successfully and whether the traffic is normal;
> at the same time, I will also construct a non power of 2 size split
> virtqueue device for testing to see if an warning is given and the driver is
> loaded fail
>
> The method of constructing the device, such as the reply steps in the
> previous email
Okay but previously you said you tested ring sizes 100 and 200 with
iperf. How did you construct these?
> >
> > --
> > MST
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 03/11] kthread: Pass in the thread's name during creation
On 3/11/23 10:11 AM, michael.chris...@oracle.com wrote:
> On 3/11/23 2:53 AM, Christian Brauner wrote:
>> On Fri, Mar 10, 2023 at 04:03:24PM -0600, Mike Christie wrote:
>>> This has us pass in the thread's name during creation in kernel_thread.
>>>
>>> Signed-off-by: Mike Christie
>>> ---
>>> kernel/kthread.c | 35 ++-
>>> 1 file changed, 14 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/kernel/kthread.c b/kernel/kthread.c
>>> index 63574cee925e..831a55b406d8 100644
>>> --- a/kernel/kthread.c
>>> +++ b/kernel/kthread.c
>>> @@ -38,6 +38,7 @@ struct task_struct *kthreadd_task;
>>> struct kthread_create_info
>>> {
>>> /* Information passed to kthread() from kthreadd. */
>>> + char *full_name;
>>> int (*threadfn)(void *data);
>>> void *data;
>>> int node;
>>> @@ -343,10 +344,15 @@ static int kthread(void *_create)
>>> /* Release the structure when caller killed by a fatal signal. */
>>> done = xchg(>done, NULL);
>>> if (!done) {
>>> + kfree(create->full_name);
>>> kfree(create);
>>> kthread_exit(-EINTR);
>>> }
>>>
>>> + if (strlen(create->full_name) >= TASK_COMM_LEN)
>>> + self->full_name = create->full_name;
>>> + else
>>> + kfree(create->full_name);
>>
>> This is monir but wwiw, this looks suspicious when reading it without
>> more context. It took me a while to see that kthread->full_name is
>> intended to store the untruncated name only if truncation actually needs
>> to happen. So either we should always initialize this or we should add a
>> comment. You can just send a tiny patch that I can fold into this one so
>> you don't have to resend the whole series...
Hey Christian, here is a patch you can fold into the original. Thanks
for your help.
>From ac82986ec4e7faae245ec48cb9213a4ca1c1d4d6 Mon Sep 17 00:00:00 2001
From: Mike Christie
Date: Sat, 11 Mar 2023 16:14:24 -0600
Subject: [PATCH] kthread: Always save the full_name
Simplify the kthread name handling by always using the full_name.
---
kernel/kthread.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 831a55b406d8..5596ec3f75cf 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -349,10 +349,7 @@ static int kthread(void *_create)
kthread_exit(-EINTR);
}
- if (strlen(create->full_name) >= TASK_COMM_LEN)
- self->full_name = create->full_name;
- else
- kfree(create->full_name);
+ self->full_name = create->full_name;
self->threadfn = threadfn;
self->data = data;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues
On 2023-03-11 p.m.2:05, Michael S. Tsirkin wrote:
External email: Use caution opening links or attachments
On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote:
On 2023-03-10 a.m.8:36, Parav Pandit wrote:
From: Feng Liu
Sent: Friday, March 10, 2023 12:34 AM
- if (!is_power_of_2(num)) {
- dev_warn(_dev->pci_dev->dev, "bad queue size %u",
num);
- return ERR_PTR(-EINVAL);
- }
-
The check is still valid for split q.
Maybe the right place for such a check is not the pci transport driver.
But layer below where split vs packed q knowledge resides and hence, power of 2
check can be omitted for packed vq.
Hi, Parav
I think you are right, I checked the virtio spec, only packed virtqueue
can use queue size which is not power_of_2; so, I think the check can be
reserved only for split queue here, something like
struct virtio_device *vdev = _dev->vdev;
if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
&& !is_power_of_2(num)){
dev_warn(_dev->pci_dev->dev, "bad queue size %u", num);
return ERR_PTR(-EINVAL);
}
I will fix it in next version
Hi, Michael and Jason
Do you have any comments on this?
Hmm good point. Which raises the question: so how did you test it then?
Hi Michael
I will construct a non power of 2 size packed virtqueue device to test
whether the driver is loaded successfully and whether the traffic is
normal; at the same time, I will also construct a non power of 2 size
split virtqueue device for testing to see if an warning is given and the
driver is loaded fail
The method of constructing the device, such as the reply steps in the
previous email
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci
On 2023-03-11 p.m.2:06, Michael S. Tsirkin wrote:
External email: Use caution opening links or attachments
On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
External email: Use caution opening links or attachments
On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
This patch series performs a clean up of the code in virtio_ring and
virtio_pci, modifying it to conform with the Linux kernel coding style
guidance [1]. The modifications ensure the code easy to read and
understand. This small series does few short cleanups in the code.
Patch-1 Allow non power of 2 sizes for virtqueues
Patch-2 Avoid using inline for small functions.
Patch-3 Use const to annotate read-only pointer params.
[1]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease=05%7C01%7Cfeliu%40nvidia.com%7C6cd34740c4674c1892f608db2263b300%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638141583834629671%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=IA0QCCKjHnYiEk2vPlZ5WjlXs1CMXDphyyqTYnbqQqo%3D=0
All of the patches have been verified based on the kernel code
commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
verified how?
Hi Michael
1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
install the kernel, and use iperf to test traffic
2. To validate this change, we tested various virtqueue sizes for packed
rings, including 128, 256, 512, 100, 200, 500, and 1000, with
CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
passed successfully.
Given split ring does not support non power of 2 how exactly
did you configure non power of 2?
Hi, Michael
We can implement the test by modifying qemu; 1. force the
VIRTIO_F_RING_PACKED feature bit to be set, 2. set
VIRTIO_NET_RX_QUEUE_DEFAULT_SIZE and VIRTIO_NET_TX_QUEUE_DEFAULT_SIZE to
the value of non power_of_2, 3. remove the check of is_power_of
virtqueue, then qemu can create the required virtual device (non power_2
size , packed virtqueue device) ;In this way, any length and packed ring
test can be performed;
remove the modified code, I can test split vq, and can see that the size
of power_of_2 can load the driver normally, and the size of non
power_of_2 will give an warning and fail to load the driver
Feng Liu (3):
virtio_pci_modern: Allow non power of 2 sizes for virtqueues
virtio_ring: Avoid using inline for small functions
virtio_ring: Use const to annotate read-only pointer params
drivers/virtio/virtio_pci_modern.c | 5
drivers/virtio/virtio_ring.c | 48 +++---
include/linux/virtio.h | 14 -
3 files changed, 31 insertions(+), 36 deletions(-)
--
2.34.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/11] Use copy_process in vhost layer
On Sat, Mar 11, 2023 at 09:21:14AM -0800, Linus Torvalds wrote: > On Fri, Mar 10, 2023 at 2:04 PM Mike Christie > wrote: > > > > The following patches were made over Linus's tree and apply over next. They > > allow the vhost layer to use copy_process instead of using > > workqueue_structs to create worker threads for VM's devices. > > Ok, all these patches looked fine to me from a quick scan - nothing > that I reacted to as objectionable, and several of them looked like > nice cleanups. > > The only one I went "Why do you do it that way" for was in 10/11 > (entirely internal to vhost, so I don't feel too strongly about this) > how you made "struct vhost_worker" be a pointer in "struct vhost_dev". > > It _looks_ to me like it could just have been an embedded structure > rather than a separate allocation. > > IOW, why do > >vhost_dev->worker > > instead of doing > > vhost_dev.worker > > and just having it all in the same allocation? > > Not a big deal. Maybe you wanted the 'test if worker pointer is NULL' > code to stay around, and basically use that pointer as a flag too. Or > maybe there is some other reason you want to keep that separate.. > >Linus I agree with Linus here, slightly better embedded, but no huge deal. Which tree is this going on? If not mine here's my ack: Acked-by: Michael S. Tsirkin -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 0/3] virtio_ring: Clean up code for virtio ring and pci
On Fri, Mar 10, 2023 at 08:21:31AM -0500, Feng Liu wrote:
>
>
> On 2023-03-10 a.m.3:06, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On Fri, Mar 10, 2023 at 07:34:25AM +0200, Feng Liu wrote:
> > > This patch series performs a clean up of the code in virtio_ring and
> > > virtio_pci, modifying it to conform with the Linux kernel coding style
> > > guidance [1]. The modifications ensure the code easy to read and
> > > understand. This small series does few short cleanups in the code.
> > >
> > > Patch-1 Allow non power of 2 sizes for virtqueues
> > > Patch-2 Avoid using inline for small functions.
> > > Patch-3 Use const to annotate read-only pointer params.
> > >
> > > [1]
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Fv6.2-rc3%2Fprocess%2Fcoding-style.html%23the-inline-disease=05%7C01%7Cfeliu%40nvidia.com%7C08831607a6fb4f58881408db213f8638%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638140328946332918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=FnD4GINUds2HLLo47aY5Ps%2B9nKWPW2XRI35z1Hp0yx4%3D=0
> > >
> > > All of the patches have been verified based on the kernel code
> > > commit 44889ba56cbb ("Merge tag 'net-6.3-rc2' of
> > > git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")
> >
> > verified how?
> >
> Hi Michael
> 1. Applied the patches on lastest kernel source(44889ba56cbb), compile and
> install the kernel, and use iperf to test traffic
> 2. To validate this change, we tested various virtqueue sizes for packed
> rings, including 128, 256, 512, 100, 200, 500, and 1000, with
> CONFIG_PAGE_POISONING enabled, and test by iperf& ping -f and all tests
> passed successfully.
Given split ring does not support non power of 2 how exactly
did you configure non power of 2?
>
> > > Feng Liu (3):
> > >virtio_pci_modern: Allow non power of 2 sizes for virtqueues
> > >virtio_ring: Avoid using inline for small functions
> > >virtio_ring: Use const to annotate read-only pointer params
> > >
> > > drivers/virtio/virtio_pci_modern.c | 5
> > > drivers/virtio/virtio_ring.c | 48 +++---
> > > include/linux/virtio.h | 14 -
> > > 3 files changed, 31 insertions(+), 36 deletions(-)
> > >
> > > --
> > > 2.34.1
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/3] virtio_pci_modern: Allow non power of 2 sizes for virtqueues
On Fri, Mar 10, 2023 at 10:23:16AM -0500, Feng Liu wrote:
>
>
> On 2023-03-10 a.m.8:36, Parav Pandit wrote:
> >
> >
> > > From: Feng Liu
> > > Sent: Friday, March 10, 2023 12:34 AM
> >
> > >
> > > - if (!is_power_of_2(num)) {
> > > - dev_warn(_dev->pci_dev->dev, "bad queue size %u",
> > > num);
> > > - return ERR_PTR(-EINVAL);
> > > - }
> > > -
> >
> > The check is still valid for split q.
> > Maybe the right place for such a check is not the pci transport driver.
> > But layer below where split vs packed q knowledge resides and hence, power
> > of 2 check can be omitted for packed vq.
>
> Hi, Parav
> I think you are right, I checked the virtio spec, only packed virtqueue
> can use queue size which is not power_of_2; so, I think the check can be
> reserved only for split queue here, something like
>
> struct virtio_device *vdev = _dev->vdev;
> if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED)
> && !is_power_of_2(num)){
> dev_warn(_dev->pci_dev->dev, "bad queue size %u", num);
> return ERR_PTR(-EINVAL);
> }
>
> I will fix it in next version
>
> Hi, Michael and Jason
> Do you have any comments on this?
>
Hmm good point. Which raises the question: so how did you test it then?
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/11] Use copy_process in vhost layer
On 3/11/23 11:21 AM, Linus Torvalds wrote: > On Fri, Mar 10, 2023 at 2:04 PM Mike Christie > wrote: >> >> The following patches were made over Linus's tree and apply over next. They >> allow the vhost layer to use copy_process instead of using >> workqueue_structs to create worker threads for VM's devices. > > Ok, all these patches looked fine to me from a quick scan - nothing > that I reacted to as objectionable, and several of them looked like > nice cleanups. > > The only one I went "Why do you do it that way" for was in 10/11 > (entirely internal to vhost, so I don't feel too strongly about this) > how you made "struct vhost_worker" be a pointer in "struct vhost_dev". > > It _looks_ to me like it could just have been an embedded structure > rather than a separate allocation. > > IOW, why do > >vhost_dev->worker > > instead of doing > > vhost_dev.worker > > and just having it all in the same allocation? > > Not a big deal. Maybe you wanted the 'test if worker pointer is NULL' > code to stay around, and basically use that pointer as a flag too. Or > maybe there is some other reason you want to keep that separate.. > There were 2 reasons: 1. Yeah, we needed a flag to indicate that the worker was not setup for the cases like where userspace just opens the dev then closes it without doing the IOCTL that does vhost_dev_set_owner. 2. I could have handled #1 by embedding the worker in the vhost_dev and then just testing worker.vtsk. However, I have a followup patchset that allows us to create multiple worker threads per device. For that patchset I then do: - if (vhost_dev->worker) + if (vhost_dev->workers) so I think it just saved me some typing. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/11] Use copy_process in vhost layer
On Fri, Mar 10, 2023 at 2:04 PM Mike Christie wrote: > > The following patches were made over Linus's tree and apply over next. They > allow the vhost layer to use copy_process instead of using > workqueue_structs to create worker threads for VM's devices. Ok, all these patches looked fine to me from a quick scan - nothing that I reacted to as objectionable, and several of them looked like nice cleanups. The only one I went "Why do you do it that way" for was in 10/11 (entirely internal to vhost, so I don't feel too strongly about this) how you made "struct vhost_worker" be a pointer in "struct vhost_dev". It _looks_ to me like it could just have been an embedded structure rather than a separate allocation. IOW, why do vhost_dev->worker instead of doing vhost_dev.worker and just having it all in the same allocation? Not a big deal. Maybe you wanted the 'test if worker pointer is NULL' code to stay around, and basically use that pointer as a flag too. Or maybe there is some other reason you want to keep that separate.. Linus ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 03/11] kthread: Pass in the thread's name during creation
On 3/11/23 2:53 AM, Christian Brauner wrote:
> On Fri, Mar 10, 2023 at 04:03:24PM -0600, Mike Christie wrote:
>> This has us pass in the thread's name during creation in kernel_thread.
>>
>> Signed-off-by: Mike Christie
>> ---
>> kernel/kthread.c | 35 ++-
>> 1 file changed, 14 insertions(+), 21 deletions(-)
>>
>> diff --git a/kernel/kthread.c b/kernel/kthread.c
>> index 63574cee925e..831a55b406d8 100644
>> --- a/kernel/kthread.c
>> +++ b/kernel/kthread.c
>> @@ -38,6 +38,7 @@ struct task_struct *kthreadd_task;
>> struct kthread_create_info
>> {
>> /* Information passed to kthread() from kthreadd. */
>> +char *full_name;
>> int (*threadfn)(void *data);
>> void *data;
>> int node;
>> @@ -343,10 +344,15 @@ static int kthread(void *_create)
>> /* Release the structure when caller killed by a fatal signal. */
>> done = xchg(>done, NULL);
>> if (!done) {
>> +kfree(create->full_name);
>> kfree(create);
>> kthread_exit(-EINTR);
>> }
>>
>> +if (strlen(create->full_name) >= TASK_COMM_LEN)
>> +self->full_name = create->full_name;
>> +else
>> +kfree(create->full_name);
>
> This is monir but wwiw, this looks suspicious when reading it without
> more context. It took me a while to see that kthread->full_name is
> intended to store the untruncated name only if truncation actually needs
> to happen. So either we should always initialize this or we should add a
> comment. You can just send a tiny patch that I can fold into this one so
> you don't have to resend the whole series...
Ok. Thanks. I think always initializing it would make the code a lot easier
to understand and be nicer.
I don't think it would be too much extra memory used, and we don't have
to worry about some random userspace app breaking because it wanted to
configure the thread but the name got truncated.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
