tty,vcs removing con_buf/conf_buf_mtx

seems there's no longer need for using con_buf/conf_buf_mtx as vcs_read/vcs_write buffer for user's data. The do_con_write function, that was the other user of this, is currently using its own kmalloc-ed buffer. Not sure when this got changed, as I was able to find this code in 2.6.9, but it's already gone as far as current git history goes - 2.6.12-rc2. AFAICS there's a behaviour change with the current change. The lseek is not completely mutually exclusive with the vcs_read/vcs_write - the file->f_pos might get updated via lseek callback during the vcs_read/vcs_write processing. I tried to find out if the prefered behaviour is to keep this in sync within read/write/lseek functions, but I did not find any pattern on different places. I guess if user end up calling write/lseek from different threads she should know what she's doing. If needed we could use dedicated fd mutex/buffer. Signed-off-by: Jiri Olsa <jolsa@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

tty,vcs removing con_buf/conf_buf_mtx
seems there's no longer need for using con_buf/conf_buf_mtx as vcs_read/vcs_write buffer for user's data. The do_con_write function, that was the other user of this, is currently using its own kmalloc-ed buffer. Not sure when this got changed, as I was able to find this code in 2.6.9, but it's already gone as far as current git history goes - 2.6.12-rc2. AFAICS there's a behaviour change with the current change. The lseek is not completely mutually exclusive with the vcs_read/vcs_write - the file->f_pos might get updated via lseek callback during the vcs_read/vcs_write processing. I tried to find out if the prefered behaviour is to keep this in sync within read/write/lseek functions, but I did not find any pattern on different places. I guess if user end up calling write/lseek from different threads she should know what she's doing. If needed we could use dedicated fd mutex/buffer. Signed-off-by: Jiri Olsa <jolsa@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Jiri Olsa · Greg Kroah-Hartman
1 parent dc1892c4bc
Showing 3 changed files with 52 additions and 66 deletions Side-by-side Diff
drivers/tty/vt/vc_screen.c
drivers/tty/vt/vt.c
include/linux/vt_kern.h
@@ -28,7 +28,6 @@
 #include <linux/interrupt.h>
 #include <linux/mm.h>
 #include <linux/init.h>
-#include <linux/mutex.h>
 #include <linux/vt_kern.h>
 #include <linux/selection.h>
 #include <linux/kbd_kern.h>
@@ -51,6 +50,8 @@
 #undef addr
 #define HEADER_SIZE	4
  
+#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE)
+
 struct vcs_poll_data {
 	struct notifier_block notifier;
 	unsigned int cons_num;
  
  
  
@@ -131,21 +132,45 @@
 	return poll;
 }
  
+/*
+ * Returns VC for inode.
+ * Must be called with console_lock.
+ */
+static struct vc_data*
+vcs_vc(struct inode *inode, int *viewed)
+{
+	unsigned int currcons = iminor(inode) & 127;
+
+	WARN_CONSOLE_UNLOCKED();
+
+	if (currcons == 0) {
+		currcons = fg_console;
+		if (viewed)
+			*viewed = 1;
+	} else {
+		currcons--;
+		if (viewed)
+			*viewed = 0;
+	}
+	return vc_cons[currcons].d;
+}
+
+/*
+ * Returns size for VC carried by inode.
+ * Must be called with console_lock.
+ */
 static int
 vcs_size(struct inode *inode)
 {
 	int size;
 	int minor = iminor(inode);
-	int currcons = minor & 127;
 	struct vc_data *vc;
  
-	if (currcons == 0)
-		currcons = fg_console;
-	else
-		currcons--;
-	if (!vc_cons_allocated(currcons))
+	WARN_CONSOLE_UNLOCKED();
+
+	vc = vcs_vc(inode, NULL);
+	if (!vc)
 		return -ENXIO;
-	vc = vc_cons[currcons].d;
  
 	size = vc->vc_rows * vc->vc_cols;
  
  
  
  
@@ -158,17 +183,13 @@
 {
 	int size;
  
-	mutex_lock(&con_buf_mtx);
 	console_lock();
 	size = vcs_size(file->f_path.dentry->d_inode);
 	console_unlock();
-	if (size < 0) {
-		mutex_unlock(&con_buf_mtx);
+	if (size < 0)
 		return size;
-	}
 	switch (orig) {
 		default:
-			mutex_unlock(&con_buf_mtx);
 			return -EINVAL;
 		case 2:
 			offset += size;
  
@@ -179,11 +200,9 @@
 			break;
 	}
 	if (offset < 0 || offset > size) {
-		mutex_unlock(&con_buf_mtx);
 		return -EINVAL;
 	}
 	file->f_pos = offset;
-	mutex_unlock(&con_buf_mtx);
 	return file->f_pos;
 }
  
  
  
@@ -196,12 +215,15 @@
 	struct vc_data *vc;
 	struct vcs_poll_data *poll;
 	long pos;
-	long viewed, attr, read;
-	int col, maxcol;
+	long attr, read;
+	int col, maxcol, viewed;
 	unsigned short *org = NULL;
 	ssize_t ret;
+	char *con_buf;
  
-	mutex_lock(&con_buf_mtx);
+	con_buf = (char *) __get_free_page(GFP_KERNEL);
+	if (!con_buf)
+		return -ENOMEM;
  
 	pos = *ppos;
  
  
  
@@ -211,18 +233,10 @@
 	console_lock();
  
 	attr = (currcons & 128);
-	currcons = (currcons & 127);
-	if (currcons == 0) {
-		currcons = fg_console;
-		viewed = 1;
-	} else {
-		currcons--;
-		viewed = 0;
-	}
 	ret = -ENXIO;
-	if (!vc_cons_allocated(currcons))
+	vc = vcs_vc(inode, &viewed);
+	if (!vc)
 		goto unlock_out;
-	vc = vc_cons[currcons].d;
  
 	ret = -EINVAL;
 	if (pos < 0)
@@ -367,7 +381,7 @@
 		ret = read;
 unlock_out:
 	console_unlock();
-	mutex_unlock(&con_buf_mtx);
+	free_page((unsigned long) con_buf);
 	return ret;
 }
  
  
  
  
@@ -378,13 +392,16 @@
 	unsigned int currcons = iminor(inode);
 	struct vc_data *vc;
 	long pos;
-	long viewed, attr, size, written;
+	long attr, size, written;
 	char *con_buf0;
-	int col, maxcol;
+	int col, maxcol, viewed;
 	u16 *org0 = NULL, *org = NULL;
 	size_t ret;
+	char *con_buf;
  
-	mutex_lock(&con_buf_mtx);
+	con_buf = (char *) __get_free_page(GFP_KERNEL);
+	if (!con_buf)
+		return -ENOMEM;
  
 	pos = *ppos;
  
  
  
@@ -394,19 +411,10 @@
 	console_lock();
  
 	attr = (currcons & 128);
-	currcons = (currcons & 127);
-
-	if (currcons == 0) {
-		currcons = fg_console;
-		viewed = 1;
-	} else {
-		currcons--;
-		viewed = 0;
-	}
 	ret = -ENXIO;
-	if (!vc_cons_allocated(currcons))
+	vc = vcs_vc(inode, &viewed);
+	if (!vc)
 		goto unlock_out;
-	vc = vc_cons[currcons].d;
  
 	size = vcs_size(inode);
 	ret = -EINVAL;
@@ -561,9 +569,7 @@
  
 unlock_out:
 	console_unlock();
-
-	mutex_unlock(&con_buf_mtx);
-
+	free_page((unsigned long) con_buf);
 	return ret;
 }
  
@@ -2068,18 +2068,6 @@
 	}
 }
  
-/* This is a temporary buffer used to prepare a tty console write
- * so that we can easily avoid touching user space while holding the
- * console spinlock.  It is allocated in con_init and is shared by
- * this code and the vc_screen read/write tty calls.
- *
- * We have to allocate this statically in the kernel data section
- * since console_init (and thus con_init) are called before any
- * kernel memory allocation is available.
- */
-char con_buf[CON_BUF_SIZE];
-DEFINE_MUTEX(con_buf_mtx);
-
 /* is_double_width() is based on the wcwidth() implementation by
  * Markus Kuhn -- 2007-05-26 (Unicode 5.0)
  * Latest version: http://www.cl.cam.ac.uk/~mgk25/ucs/wcwidth.c
@@ -142,14 +142,6 @@
 	return false;
 }
  
-/*
- * vc_screen.c shares this temporary buffer with the console write code so that
- * we can easily avoid touching user space while holding the console spinlock.
- */
-
-#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE)
-extern char con_buf[CON_BUF_SIZE];
-extern struct mutex con_buf_mtx;
 extern char vt_dont_switch;
 extern int default_utf8;
 extern int global_cursor_default;
...	...	@@ -28,7 +28,6 @@
28	28	#include <linux/interrupt.h>
29	29	#include <linux/mm.h>
30	30	#include <linux/init.h>
31		-#include <linux/mutex.h>
32	31	#include <linux/vt_kern.h>
33	32	#include <linux/selection.h>
34	33	#include <linux/kbd_kern.h>
...	...	@@ -51,6 +50,8 @@
51	50	#undef addr
52	51	#define HEADER_SIZE 4
53	52
	53	+#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE)
	54	+
54	55	struct vcs_poll_data {
55	56	struct notifier_block notifier;
56	57	unsigned int cons_num;
57	58
58	59
59	60
...	...	@@ -131,21 +132,45 @@
131	132	return poll;
132	133	}
133	134
	135	+/*
	136	+ * Returns VC for inode.
	137	+ * Must be called with console_lock.
	138	+ */
	139	+static struct vc_data*
	140	+vcs_vc(struct inode inode, int viewed)
	141	+{
	142	+ unsigned int currcons = iminor(inode) & 127;
	143	+
	144	+ WARN_CONSOLE_UNLOCKED();
	145	+
	146	+ if (currcons == 0) {
	147	+ currcons = fg_console;
	148	+ if (viewed)
	149	+ *viewed = 1;
	150	+ } else {
	151	+ currcons--;
	152	+ if (viewed)
	153	+ *viewed = 0;
	154	+ }
	155	+ return vc_cons[currcons].d;
	156	+}
	157	+
	158	+/*
	159	+ * Returns size for VC carried by inode.
	160	+ * Must be called with console_lock.
	161	+ */
134	162	static int
135	163	vcs_size(struct inode *inode)
136	164	{
137	165	int size;
138	166	int minor = iminor(inode);
139		- int currcons = minor & 127;
140	167	struct vc_data *vc;
141	168
142		- if (currcons == 0)
143		- currcons = fg_console;
144		- else
145		- currcons--;
146		- if (!vc_cons_allocated(currcons))
	169	+ WARN_CONSOLE_UNLOCKED();
	170	+
	171	+ vc = vcs_vc(inode, NULL);
	172	+ if (!vc)
147	173	return -ENXIO;
148		- vc = vc_cons[currcons].d;
149	174
150	175	size = vc->vc_rows * vc->vc_cols;
151	176
152	177
153	178
154	179
...	...	@@ -158,17 +183,13 @@
158	183	{
159	184	int size;
160	185
161		- mutex_lock(&con_buf_mtx);
162	186	console_lock();
163	187	size = vcs_size(file->f_path.dentry->d_inode);
164	188	console_unlock();
165		- if (size < 0) {
166		- mutex_unlock(&con_buf_mtx);
	189	+ if (size < 0)
167	190	return size;
168		- }
169	191	switch (orig) {
170	192	default:
171		- mutex_unlock(&con_buf_mtx);
172	193	return -EINVAL;
173	194	case 2:
174	195	offset += size;
175	196
...	...	@@ -179,11 +200,9 @@
179	200	break;
180	201	}
181	202	if (offset < 0 \|\| offset > size) {
182		- mutex_unlock(&con_buf_mtx);
183	203	return -EINVAL;
184	204	}
185	205	file->f_pos = offset;
186		- mutex_unlock(&con_buf_mtx);
187	206	return file->f_pos;
188	207	}
189	208
190	209
191	210
...	...	@@ -196,12 +215,15 @@
196	215	struct vc_data *vc;
197	216	struct vcs_poll_data *poll;
198	217	long pos;
199		- long viewed, attr, read;
200		- int col, maxcol;
	218	+ long attr, read;
	219	+ int col, maxcol, viewed;
201	220	unsigned short *org = NULL;
202	221	ssize_t ret;
	222	+ char *con_buf;
203	223
204		- mutex_lock(&con_buf_mtx);
	224	+ con_buf = (char *) __get_free_page(GFP_KERNEL);
	225	+ if (!con_buf)
	226	+ return -ENOMEM;
205	227
206	228	pos = *ppos;
207	229
208	230
209	231
...	...	@@ -211,18 +233,10 @@
211	233	console_lock();
212	234
213	235	attr = (currcons & 128);
214		- currcons = (currcons & 127);
215		- if (currcons == 0) {
216		- currcons = fg_console;
217		- viewed = 1;
218		- } else {
219		- currcons--;
220		- viewed = 0;
221		- }
222	236	ret = -ENXIO;
223		- if (!vc_cons_allocated(currcons))
	237	+ vc = vcs_vc(inode, &viewed);
	238	+ if (!vc)
224	239	goto unlock_out;
225		- vc = vc_cons[currcons].d;
226	240
227	241	ret = -EINVAL;
228	242	if (pos < 0)
...	...	@@ -367,7 +381,7 @@
367	381	ret = read;
368	382	unlock_out:
369	383	console_unlock();
370		- mutex_unlock(&con_buf_mtx);
	384	+ free_page((unsigned long) con_buf);
371	385	return ret;
372	386	}
373	387
374	388
375	389
376	390
...	...	@@ -378,13 +392,16 @@
378	392	unsigned int currcons = iminor(inode);
379	393	struct vc_data *vc;
380	394	long pos;
381		- long viewed, attr, size, written;
	395	+ long attr, size, written;
382	396	char *con_buf0;
383		- int col, maxcol;
	397	+ int col, maxcol, viewed;
384	398	u16 org0 = NULL, org = NULL;
385	399	size_t ret;
	400	+ char *con_buf;
386	401
387		- mutex_lock(&con_buf_mtx);
	402	+ con_buf = (char *) __get_free_page(GFP_KERNEL);
	403	+ if (!con_buf)
	404	+ return -ENOMEM;
388	405
389	406	pos = *ppos;
390	407
391	408
392	409
...	...	@@ -394,19 +411,10 @@
394	411	console_lock();
395	412
396	413	attr = (currcons & 128);
397		- currcons = (currcons & 127);
398		-
399		- if (currcons == 0) {
400		- currcons = fg_console;
401		- viewed = 1;
402		- } else {
403		- currcons--;
404		- viewed = 0;
405		- }
406	414	ret = -ENXIO;
407		- if (!vc_cons_allocated(currcons))
	415	+ vc = vcs_vc(inode, &viewed);
	416	+ if (!vc)
408	417	goto unlock_out;
409		- vc = vc_cons[currcons].d;
410	418
411	419	size = vcs_size(inode);
412	420	ret = -EINVAL;
...	...	@@ -561,9 +569,7 @@
561	569
562	570	unlock_out:
563	571	console_unlock();
564		-
565		- mutex_unlock(&con_buf_mtx);
566		-
	572	+ free_page((unsigned long) con_buf);
567	573	return ret;
568	574	}
569	575
...	...	@@ -2068,18 +2068,6 @@
2068	2068	}
2069	2069	}
2070	2070
2071		-/* This is a temporary buffer used to prepare a tty console write
2072		- * so that we can easily avoid touching user space while holding the
2073		- * console spinlock. It is allocated in con_init and is shared by
2074		- * this code and the vc_screen read/write tty calls.
2075		- *
2076		- * We have to allocate this statically in the kernel data section
2077		- * since console_init (and thus con_init) are called before any
2078		- * kernel memory allocation is available.
2079		- */
2080		-char con_buf[CON_BUF_SIZE];
2081		-DEFINE_MUTEX(con_buf_mtx);
2082		-
2083	2071	/* is_double_width() is based on the wcwidth() implementation by
2084	2072	* Markus Kuhn -- 2007-05-26 (Unicode 5.0)
2085	2073	* Latest version: http://www.cl.cam.ac.uk/~mgk25/ucs/wcwidth.c
...	...	@@ -142,14 +142,6 @@
142	142	return false;
143	143	}
144	144
145		-/*
146		- * vc_screen.c shares this temporary buffer with the console write code so that
147		- * we can easily avoid touching user space while holding the console spinlock.
148		- */
149		-
150		-#define CON_BUF_SIZE (CONFIG_BASE_SMALL ? 256 : PAGE_SIZE)
151		-extern char con_buf[CON_BUF_SIZE];
152		-extern struct mutex con_buf_mtx;
153	145	extern char vt_dont_switch;
154	146	extern int default_utf8;
155	147	extern int global_cursor_default;