Skip to content

Commit 70045cf

Browse files
committed
xen/gntdev: remove struct gntdev_copy_batch from stack
When compiling the kernel with LLVM, the following warning was issued: drivers/xen/gntdev.c:991: warning: stack frame size (1160) exceeds limit (1024) in function 'gntdev_ioctl' The main reason is struct gntdev_copy_batch which is located on the stack and has a size of nearly 1kb. For performance reasons it shouldn't by just dynamically allocated instead, so allocate a new instance when needed and instead of freeing it put it into a list of free structs anchored in struct gntdev_priv. Fixes: a4cdb55 ("xen/gntdev: add ioctl for grant copy") Reported-by: Abinash Singh <[email protected]> Reviewed-by: Stefano Stabellini <[email protected]> Signed-off-by: Juergen Gross <[email protected]> Message-ID: <[email protected]>
1 parent 532c8b5 commit 70045cf

File tree

2 files changed

+54
-21
lines changed

2 files changed

+54
-21
lines changed

drivers/xen/gntdev-common.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ struct gntdev_priv {
2626
/* lock protects maps and freeable_maps. */
2727
struct mutex lock;
2828

29+
/* Free instances of struct gntdev_copy_batch. */
30+
struct gntdev_copy_batch *batch;
31+
struct mutex batch_lock;
32+
2933
#ifdef CONFIG_XEN_GRANT_DMA_ALLOC
3034
/* Device for which DMA memory is allocated. */
3135
struct device *dma_dev;

drivers/xen/gntdev.c

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,18 @@ MODULE_AUTHOR("Derek G. Murray <[email protected]>, "
5656
"Gerd Hoffmann <[email protected]>");
5757
MODULE_DESCRIPTION("User-space granted page access driver");
5858

59+
#define GNTDEV_COPY_BATCH 16
60+
61+
struct gntdev_copy_batch {
62+
struct gnttab_copy ops[GNTDEV_COPY_BATCH];
63+
struct page *pages[GNTDEV_COPY_BATCH];
64+
s16 __user *status[GNTDEV_COPY_BATCH];
65+
unsigned int nr_ops;
66+
unsigned int nr_pages;
67+
bool writeable;
68+
struct gntdev_copy_batch *next;
69+
};
70+
5971
static unsigned int limit = 64*1024;
6072
module_param(limit, uint, 0644);
6173
MODULE_PARM_DESC(limit,
@@ -584,6 +596,8 @@ static int gntdev_open(struct inode *inode, struct file *flip)
584596
INIT_LIST_HEAD(&priv->maps);
585597
mutex_init(&priv->lock);
586598

599+
mutex_init(&priv->batch_lock);
600+
587601
#ifdef CONFIG_XEN_GNTDEV_DMABUF
588602
priv->dmabuf_priv = gntdev_dmabuf_init(flip);
589603
if (IS_ERR(priv->dmabuf_priv)) {
@@ -608,6 +622,7 @@ static int gntdev_release(struct inode *inode, struct file *flip)
608622
{
609623
struct gntdev_priv *priv = flip->private_data;
610624
struct gntdev_grant_map *map;
625+
struct gntdev_copy_batch *batch;
611626

612627
pr_debug("priv %p\n", priv);
613628

@@ -620,6 +635,14 @@ static int gntdev_release(struct inode *inode, struct file *flip)
620635
}
621636
mutex_unlock(&priv->lock);
622637

638+
mutex_lock(&priv->batch_lock);
639+
while (priv->batch) {
640+
batch = priv->batch;
641+
priv->batch = batch->next;
642+
kfree(batch);
643+
}
644+
mutex_unlock(&priv->batch_lock);
645+
623646
#ifdef CONFIG_XEN_GNTDEV_DMABUF
624647
gntdev_dmabuf_fini(priv->dmabuf_priv);
625648
#endif
@@ -785,17 +808,6 @@ static long gntdev_ioctl_notify(struct gntdev_priv *priv, void __user *u)
785808
return rc;
786809
}
787810

788-
#define GNTDEV_COPY_BATCH 16
789-
790-
struct gntdev_copy_batch {
791-
struct gnttab_copy ops[GNTDEV_COPY_BATCH];
792-
struct page *pages[GNTDEV_COPY_BATCH];
793-
s16 __user *status[GNTDEV_COPY_BATCH];
794-
unsigned int nr_ops;
795-
unsigned int nr_pages;
796-
bool writeable;
797-
};
798-
799811
static int gntdev_get_page(struct gntdev_copy_batch *batch, void __user *virt,
800812
unsigned long *gfn)
801813
{
@@ -953,36 +965,53 @@ static int gntdev_grant_copy_seg(struct gntdev_copy_batch *batch,
953965
static long gntdev_ioctl_grant_copy(struct gntdev_priv *priv, void __user *u)
954966
{
955967
struct ioctl_gntdev_grant_copy copy;
956-
struct gntdev_copy_batch batch;
968+
struct gntdev_copy_batch *batch;
957969
unsigned int i;
958970
int ret = 0;
959971

960972
if (copy_from_user(&copy, u, sizeof(copy)))
961973
return -EFAULT;
962974

963-
batch.nr_ops = 0;
964-
batch.nr_pages = 0;
975+
mutex_lock(&priv->batch_lock);
976+
if (!priv->batch) {
977+
batch = kmalloc(sizeof(*batch), GFP_KERNEL);
978+
} else {
979+
batch = priv->batch;
980+
priv->batch = batch->next;
981+
}
982+
mutex_unlock(&priv->batch_lock);
983+
if (!batch)
984+
return -ENOMEM;
985+
986+
batch->nr_ops = 0;
987+
batch->nr_pages = 0;
965988

966989
for (i = 0; i < copy.count; i++) {
967990
struct gntdev_grant_copy_segment seg;
968991

969992
if (copy_from_user(&seg, &copy.segments[i], sizeof(seg))) {
970993
ret = -EFAULT;
994+
gntdev_put_pages(batch);
971995
goto out;
972996
}
973997

974-
ret = gntdev_grant_copy_seg(&batch, &seg, &copy.segments[i].status);
975-
if (ret < 0)
998+
ret = gntdev_grant_copy_seg(batch, &seg, &copy.segments[i].status);
999+
if (ret < 0) {
1000+
gntdev_put_pages(batch);
9761001
goto out;
1002+
}
9771003

9781004
cond_resched();
9791005
}
980-
if (batch.nr_ops)
981-
ret = gntdev_copy(&batch);
982-
return ret;
1006+
if (batch->nr_ops)
1007+
ret = gntdev_copy(batch);
1008+
1009+
out:
1010+
mutex_lock(&priv->batch_lock);
1011+
batch->next = priv->batch;
1012+
priv->batch = batch;
1013+
mutex_unlock(&priv->batch_lock);
9831014

984-
out:
985-
gntdev_put_pages(&batch);
9861015
return ret;
9871016
}
9881017

0 commit comments

Comments
 (0)