[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[RFC PATCH] xen/gntdev: reduce stack usage by dynamically allocating gntdev_copy_batch



While building the kernel with LLVM, a warning was reported due to
excessive stack usage in `gntdev_ioctl`:

        drivers/xen/gntdev.c:991: warning: stack frame size (1160) exceeds 
limit (1024) in function 'gntdev_ioctl'

Further analysis revealed that the large stack frame was caused by
`struct gntdev_copy_batch`, which was declared on the stack inside
`gntdev_ioctl_grant_copy()`. Since this function was inlined into
`gntdev_ioctl`, the stack usage was attributed to the latter.

This patch fixes the issue by dynamically allocating `gntdev_copy_batch`
using `kmalloc()`, which significantly reduces the stack footprint and
eliminates the warning.

This approach is consistent with similar fixes upstream, such as:

commit fa26198d30f3 ("iommu/io-pgtable-arm: dynamically allocate selftest 
device struct")

Fixes: a4cdb556cae0 ("xen/gntdev: add ioctl for grant copy")
Signed-off-by: Abinash Singh <abinashsinghlalotra@xxxxxxxxx>
---
The stack usage was confirmed using the `-fstack-usage`  flag and mannual 
analysis, which showed:

  drivers/xen/gntdev.c:953: gntdev_ioctl_grant_copy.isra   1048 bytes
  drivers/xen/gntdev.c:826: gntdev_copy                     56 bytes

Since `gntdev_ioctl` was calling the inlined `gntdev_ioctl_grant_copy`, the 
total
frame size exceeded 1024 bytes, triggering the warning.

This patch addresses the warning and keeps stack usage within acceptable limits.
Is this patch fine or kmalloc may affect performance ?
---
 drivers/xen/gntdev.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
index 61faea1f0663..9811f3d7c87c 100644
--- a/drivers/xen/gntdev.c
+++ b/drivers/xen/gntdev.c
@@ -953,15 +953,19 @@ static int gntdev_grant_copy_seg(struct gntdev_copy_batch 
*batch,
 static long gntdev_ioctl_grant_copy(struct gntdev_priv *priv, void __user *u)
 {
        struct ioctl_gntdev_grant_copy copy;
-       struct gntdev_copy_batch batch;
+       struct gntdev_copy_batch *batch;
        unsigned int i;
        int ret = 0;
 
        if (copy_from_user(&copy, u, sizeof(copy)))
                return -EFAULT;
-
-       batch.nr_ops = 0;
-       batch.nr_pages = 0;
+
+       batch = kmalloc(sizeof(*batch), GFP_KERNEL);
+       if (!batch)
+               return -ENOMEM;
+
+       batch->nr_ops = 0;
+       batch->nr_pages = 0;
 
        for (i = 0; i < copy.count; i++) {
                struct gntdev_grant_copy_segment seg;
@@ -971,18 +975,20 @@ static long gntdev_ioctl_grant_copy(struct gntdev_priv 
*priv, void __user *u)
                        goto out;
                }
 
-               ret = gntdev_grant_copy_seg(&batch, &seg, 
&copy.segments[i].status);
+               ret = gntdev_grant_copy_seg(batch, &seg, 
&copy.segments[i].status);
                if (ret < 0)
                        goto out;
 
                cond_resched();
        }
-       if (batch.nr_ops)
-               ret = gntdev_copy(&batch);
-       return ret;
+       if (batch->nr_ops)
+               ret = gntdev_copy(batch);
+       goto free_batch;
 
   out:
-       gntdev_put_pages(&batch);
+       gntdev_put_pages(batch);
+  free_batch:
+       kfree(batch);
        return ret;
 }
 
-- 
2.43.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.