Thread (11 messages) 11 messages, 2 authors, 2021-10-14

Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()

From: Greg KH <gregkh@linuxfoundation.org>
Date: 2021-10-09 11:58:48
Also in: lkml, virtualization

On Sat, Oct 09, 2021 at 07:48:28PM +0800, Xianting Tian wrote:
quoted hunk ↗ jump to hunk
--- a/drivers/tty/hvc/hvc_console.h
+++ b/drivers/tty/hvc/hvc_console.h
@@ -32,13 +32,21 @@
  */
 #define HVC_ALLOC_TTY_ADAPTERS	8
 
+/*
+ * These sizes are most efficient for vio, because they are the
+ * native transfer size. We could make them selectable in the
+ * future to better deal with backends that want other buffer sizes.
+ */
+#define N_OUTBUF	16
+#define N_INBUF		16
+
+#define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
Does this conflict with what is in hvcs.c?
quoted hunk ↗ jump to hunk
+
 struct hvc_struct {
 	struct tty_port port;
 	spinlock_t lock;
 	int index;
 	int do_wakeup;
-	char *outbuf;
-	int outbuf_size;
 	int n_outbuf;
 	uint32_t vtermno;
 	const struct hv_ops *ops;
@@ -48,6 +56,18 @@ struct hvc_struct {
 	struct work_struct tty_resize;
 	struct list_head next;
 	unsigned long flags;
+
+	/* the buf is used in hvc console api for putting chars */
+	char cons_outbuf[N_OUTBUF] __ALIGNED__;
+	spinlock_t cons_outbuf_lock;
Did you look at the placement using pahole as to how this structure now
looks?
+
+	/* the buf is for putting single char to tty */
+	char outchar;
+	spinlock_t outchar_lock;
So you have a lock for a character and a different one for a longer
string?  Why can they not just use the same lock?  Why are 2 needed at
all, can't you just use the first character of cons_outbuf[] instead?
Surely you do not have 2 sends happening at the same time, right?
+
+	/* the buf is for putting chars to tty */
+	int outbuf_size;
+	char outbuf[0] __ALIGNED__;
I thought we were not allowing [0] anymore in kernel structures?

thanks,

greg k-h
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help