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&data=05%7C01%7Cfeliu%40nvidia.com%7C6cd34740c4674c1892f608db2263b300%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638141583834629671%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IA0QCCKjHnYiEk2vPlZ5WjlXs1CMXDphyyqTYnbqQqo%3D&reserved=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(&vp_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 = &vp_dev->vdev; > > > if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED) > > > && !is_power_of_2(num)){ > > > dev_warn(&vp_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(&create->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(&vp_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 = &vp_dev->vdev; if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED) && !is_power_of_2(num)){ dev_warn(&vp_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&data=05%7C01%7Cfeliu%40nvidia.com%7C6cd34740c4674c1892f608db2263b300%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638141583834629671%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=IA0QCCKjHnYiEk2vPlZ5WjlXs1CMXDphyyqTYnbqQqo%3D&reserved=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&data=05%7C01%7Cfeliu%40nvidia.com%7C08831607a6fb4f58881408db213f8638%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638140328946332918%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FnD4GINUds2HLLo47aY5Ps%2B9nKWPW2XRI35z1Hp0yx4%3D&reserved=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(&vp_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 = &vp_dev->vdev; > if (!virtio_has_feature(vdev, VIRTIO_F_RING_PACKED) > && !is_power_of_2(num)){ > dev_warn(&vp_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(&create->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