Skip to contentSkip to conversation

media: vivid: work around high stack usage with clang

PATCH In review Arnd Bergmann 2 participants 2 messages Jul 18, 2019 Media (V4L/DVB)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 media: vivid: work around high stack usage with clang

Building a KASAN-enabled kernel with clang ends up in a case where too much is inlined into vivid_thread_vid_cap() and the stack usage grows a lot, possibly when the register allocation fails to produce efficient code and spills a lot of temporaries to the stack. This uses more than twice the amount of stack than the sum of the individual functions when they are not inlined:

drivers/media/platform/vivid/vivid-kthread-cap.c:766:12: error: stack frame size of 2208 bytes in function 'vivid_thread_vid_cap' [-Werror,-Wframe-larger-than=]

Marking two of the key functions in here as 'noinline_for_stack' avoids the pathological case in clang without any apparent downside for gcc.

---
Not sure how much effort we want to put into fixing clang to not get into this case. I could open an llvm bug report if something thinks this has a chance of getting fixed there.

---
 drivers/media/platform/vivid/vivid-kthread-cap.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/media/platform/vivid/vivid-kthread-cap.c +4 -3
index 6cf495a7d5cc..4f94897e6303 100644
@@ -232,8 +232,8 @@ static void *plane_vaddr(struct tpg_data *tpg, struct vivid_buffer *buf,
 	return vbuf;
 }
 
-static int vivid_copy_buffer(struct vivid_dev *dev, unsigned p, u8 *vcapbuf,
-		struct vivid_buffer *vid_cap_buf)
+static noinline_for_stack int vivid_copy_buffer(struct vivid_dev *dev, unsigned p,
+		u8 *vcapbuf, struct vivid_buffer *vid_cap_buf)
 {
 	bool blank = dev->must_blank[vid_cap_buf->vb.vb2_buf.index];
 	struct tpg_data *tpg = &dev->tpg;
@@ -670,7 +670,8 @@ static void vivid_cap_update_frame_period(struct vivid_dev *dev)
 	dev->cap_frame_period = f_period;
 }
 
-static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs)
+static noinline_for_stack void vivid_thread_vid_cap_tick(struct vivid_dev *dev,
+							 int dropped_bufs)
 {
 	struct vivid_buffer *vid_cap_buf = NULL;
 	struct vivid_buffer *vbi_cap_buf = NULL;
Signature · 1 line
2.20.0
Acked-by on v1 1/1
#1 lore raw
Quoted 8 lines · Arnd Bergmann

Building a KASAN-enabled kernel with clang ends up in a case where too much is inlined into vivid_thread_vid_cap() and the stack usage grows a lot, possibly when the register allocation fails to produce efficient code and spills a lot of temporaries to the stack. This uses more than twice the amount of stack than the sum of the individual functions when they are not inlined:

Quoted 4 lines

drivers/media/platform/vivid/vivid-kthread-cap.c:766:12: error: stack frame size of 2208 bytes in function 'vivid_thread_vid_cap' [-Werror,-Wframe-larger-than=]

Marking two of the key functions in here as 'noinline_for_stack' avoids the pathological case in clang without any apparent downside for gcc.

Thanks for the patch, should be one less warning for CI.

Quoted 42 lines

---
Not sure how much effort we want to put into fixing clang to not get into this case. I could open an llvm bug report if something thinks this has a chance of getting fixed there.

---
 drivers/media/platform/vivid/vivid-kthread-cap.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/media/platform/vivid/vivid-kthread-cap.c +4 -4
index 6cf495a7d5cc..4f94897e6303 100644
@@ -232,8 +232,8 @@ static void *plane_vaddr(struct tpg_data *tpg, struct vivid_buffer *buf,
        return vbuf;
 }
 
-static int vivid_copy_buffer(struct vivid_dev *dev, unsigned p, u8 *vcapbuf,
-               struct vivid_buffer *vid_cap_buf)
+static noinline_for_stack int vivid_copy_buffer(struct vivid_dev *dev, unsigned p,
+               u8 *vcapbuf, struct vivid_buffer *vid_cap_buf)
 {
        bool blank = dev->must_blank[vid_cap_buf->vb.vb2_buf.index];
        struct tpg_data *tpg = &dev->tpg;
@@ -670,7 +670,8 @@ static void vivid_cap_update_frame_period(struct vivid_dev *dev)
        dev->cap_frame_period = f_period;
 }
 
-static void vivid_thread_vid_cap_tick(struct vivid_dev *dev, int dropped_bufs)
+static noinline_for_stack void vivid_thread_vid_cap_tick(struct vivid_dev *dev,
+                                                        int dropped_bufs)
 {
        struct vivid_buffer *vid_cap_buf = NULL;
        struct vivid_buffer *vbi_cap_buf = NULL;
--

2.20.0

--
You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190718141652.3323402-1-arnd%40arndb.de.

Signature · 2 lines
Thanks,
~Nick Desaulniers