LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/4] stringbuf: A string buffer implementation
@ 2007-10-23 21:12 Matthew Wilcox
  2007-10-23 21:12 ` [PATCH 2/4] isdn: Use stringbuf Matthew Wilcox
                   ` (7 more replies)
  0 siblings, 8 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-23 21:12 UTC (permalink / raw)
  To: torvalds, akpm, linux-kernel; +Cc: Matthew Wilcox, Matthew Wilcox

Consecutive calls to printk are non-atomic, which leads to various
implementations for accumulating strings which can be printed in one call.
This is a generic string buffer which can also be used for non-printk
purposes.  There is no sb_scanf implementation yet as I haven't identified
a user for it.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 include/linux/stringbuf.h |   85 +++++++++++++++++++++++++++++++++++++++++++++
 lib/Makefile              |    2 +-
 lib/stringbuf.c           |   79 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 165 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/stringbuf.h
 create mode 100644 lib/stringbuf.c

diff --git a/include/linux/stringbuf.h b/include/linux/stringbuf.h
new file mode 100644
index 0000000..f9200fe
--- /dev/null
+++ b/include/linux/stringbuf.h
@@ -0,0 +1,85 @@
+/*
+ * An auto-expanding string buffer.  There's no sb_alloc because you really
+ * want to allocate it on the stack or embed it in another structure.
+ *
+ * Please don't access the elements directly; that will give us the chance
+ * to change the implementation later if necessary.
+ *
+ * No locking is performed, although memory allocation is done with
+ * GFP_ATOMIC to allow it to be called when you're holding a lock.
+ */
+
+#include <stdarg.h>
+#include <linux/string.h>
+
+struct stringbuf {
+	char *s;
+	int alloc;
+	int len;
+};
+
+/*
+ * Returns the current length of the string.  Note that more memory
+ * may have been allocated for the string than this.
+ */
+static inline int sb_len(struct stringbuf *sb)
+{
+	return sb->len;
+}
+
+/*
+ * Returns a negative errno if an error has been found with this stringbuf,
+ * or 0 if no error has occurred.  If an error has occurred, sb_string()
+ * returns a suitable description of the error
+ */
+static inline int sb_error(struct stringbuf *sb)
+{
+	return sb->alloc < 0 ? sb->alloc : 0;
+}
+
+/*
+ * Initialise a newly-allocated stringbuf
+ */
+static inline void sb_init(struct stringbuf *sb)
+{
+	memset(sb, 0, sizeof(*sb));
+}
+
+/*
+ * Free the contents of a stringbuf, not the stringbuf itself.  The
+ * stringbuf is then reinitialised so it can be reused, or sb_free can
+ * be called on it multiple times.
+ */
+static inline void sb_free(struct stringbuf *sb)
+{
+	if (sb->alloc > 0)
+		kfree(sb->s);
+	sb_init(sb);
+}
+
+extern void sb_printf(struct stringbuf *sb, const char *fmt, ...)
+	__attribute__((format(printf, 2, 3)));
+#if 0
+/* Waiting for a user to show up */
+extern void sb_vprintf(struct stringbuf *sb, const char *fmt, va_list args)
+	__attribute__((format(printf, 2, 0)));
+#endif
+
+/* Get a pointer to the string. */
+static inline char *sb_string(struct stringbuf *sb)
+{
+	return sb->s;
+}
+
+/*
+ * Convert the stringbuf to a string.  It is the caller's responsibility
+ * to free the string.  The stringbuf is then ready to be reused.
+ */
+static inline char *sb_to_string(struct stringbuf *sb)
+{
+	char *s = sb->s;
+	if (sb_error(sb))
+		s = kstrdup(s, GFP_ATOMIC);
+	sb_init(sb);
+	return s;
+}
diff --git a/lib/Makefile b/lib/Makefile
index 3a0983b..0c33ff6 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -2,7 +2,7 @@
 # Makefile for some libs needed in the kernel.
 #
 
-lib-y := ctype.o string.o vsprintf.o cmdline.o \
+lib-y := ctype.o string.o stringbuf.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o dump_stack.o \
 	 idr.o int_sqrt.o bitmap.o extable.o prio_tree.o \
 	 sha1.o irq_regs.o reciprocal_div.o argv_split.o \
diff --git a/lib/stringbuf.c b/lib/stringbuf.c
new file mode 100644
index 0000000..0853062
--- /dev/null
+++ b/lib/stringbuf.c
@@ -0,0 +1,79 @@
+/*
+ * stringbuf.c
+ *
+ * Copyright (c) 2007 Intel Corporation
+ * By Matthew Wilcox <willy@linux.intel.com>
+ *
+ * Released under the terms of the GNU GPL, version 2.
+ *
+ * A simple automatically expanding string.  I'm not opposed to adding
+ * more functions to this (eg sb_scanf, sb_insert, sb_delete, sb_strchr, etc),
+ * but I have no need for them right now, so I haven't added them.
+ */
+
+#include <stdarg.h>
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/stringbuf.h>
+
+#define INITIAL_SIZE 32
+#define ERR_STRING "out of memory"
+
+/*
+ * Not currently used outside this file, but separated from sb_printf
+ * for when someone needs it.
+ */
+static void sb_vprintf(struct stringbuf *sb, const char *format, va_list args)
+{
+	char *s;
+	int size;
+
+	if (sb->alloc == -ENOMEM)
+		return;
+	if (sb->alloc == 0) {
+		sb->s = kmalloc(INITIAL_SIZE, GFP_ATOMIC);
+		if (!sb->s) 
+			goto nomem;
+		sb->alloc = INITIAL_SIZE;
+	}
+
+	s = sb->s + sb->len;
+	size = vsnprintf(s, sb->alloc - sb->len, format, args);
+
+	sb->len += size;
+	if (likely(sb->len < sb->alloc))
+		return;
+
+	s = krealloc(sb->s, sb->len + 1, GFP_ATOMIC);
+	if (!s)
+		goto nomem;
+	sb->s = s;
+	sb->alloc = ksize(s);
+
+	/* Point to the end of the old string since we already updated ->len */
+	s += sb->len - size;
+	vsprintf(s, format, args);
+	return;
+
+ nomem:
+	kfree(sb->s);
+	sb->s = ERR_STRING;
+	sb->len = sizeof(ERR_STRING);
+	sb->alloc = -ENOMEM;
+}
+/* When we get our first modular user, delete this comment
+EXPORT_SYMBOL(sb_vprintf);
+*/
+
+void sb_printf(struct stringbuf *sb, const char *format, ...)
+{
+	va_list args;
+
+	va_start(args, format);
+	sb_vprintf(sb, format, args);
+	va_end(args);
+}
+EXPORT_SYMBOL(sb_printf);
-- 
1.5.3.4


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH 2/4] isdn: Use stringbuf
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
@ 2007-10-23 21:12 ` Matthew Wilcox
  2007-10-23 21:12   ` [PATCH 3/4] sound: " Matthew Wilcox
  2007-10-23 22:11 ` [PATCH 1/4] stringbuf: A string buffer implementation Matt Mackall
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-23 21:12 UTC (permalink / raw)
  To: torvalds, akpm, linux-kernel; +Cc: Matthew Wilcox, Matthew Wilcox

Get rid of the _cdebbuf structure that was used to accumulate strings
for a debug printk and use the stringbuf instead.  Allocate the stringbuf
on the stack instead of with kmalloc.  Return a char * to the callers
rather than a stringbuf.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 drivers/isdn/capi/capidrv.c   |   18 ++--
 drivers/isdn/capi/capiutil.c  |  220 ++++++++++-------------------------------
 drivers/isdn/capi/kcapi.c     |   35 +++----
 include/linux/isdn/capiutil.h |   16 +---
 4 files changed, 83 insertions(+), 206 deletions(-)

diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index 476012b..ba6645c 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -995,7 +995,7 @@ static void handle_plci(_cmsg * cmsg)
 	capidrv_contr *card = findcontrbynumber(cmsg->adr.adrController & 0x7f);
 	capidrv_plci *plcip;
 	isdn_ctrl cmd;
-	_cdebbuf *cdb;
+	char *s;
 
 	if (!card) {
 		printk(KERN_ERR "capidrv: %s from unknown controller 0x%x\n",
@@ -1128,11 +1128,11 @@ static void handle_plci(_cmsg * cmsg)
 				break;
 			}
 		}
-		cdb = capi_cmsg2str(cmsg);
-		if (cdb) {
+		s = capi_cmsg2str(cmsg);
+		if (s) {
 			printk(KERN_WARNING "capidrv-%d: %s\n",
-				card->contrnr, cdb->buf);
-			cdebbuf_free(cdb);
+				card->contrnr, s);
+			kfree(s);
 		} else
 			printk(KERN_WARNING "capidrv-%d: CAPI_INFO_IND InfoNumber %x not handled\n",
 				card->contrnr, cmsg->InfoNumber);
@@ -1385,12 +1385,12 @@ static void capidrv_recv_message(struct capi20_appl *ap, struct sk_buff *skb)
 {
 	capi_message2cmsg(&s_cmsg, skb->data);
 	if (debugmode > 3) {
-		_cdebbuf *cdb = capi_cmsg2str(&s_cmsg);
+		char *s = capi_cmsg2str(&s_cmsg);
 
-		if (cdb) {
+		if (s) {
 			printk(KERN_DEBUG "%s: applid=%d %s\n", __FUNCTION__,
-				ap->applid, cdb->buf);
-			cdebbuf_free(cdb);
+				ap->applid, s);
+			kfree(s);
 		} else
 			printk(KERN_DEBUG "%s: applid=%d %s not traced\n",
 				__FUNCTION__, ap->applid,
diff --git a/drivers/isdn/capi/capiutil.c b/drivers/isdn/capi/capiutil.c
index 22379b9..c8df3f0 100644
--- a/drivers/isdn/capi/capiutil.c
+++ b/drivers/isdn/capi/capiutil.c
@@ -14,6 +14,7 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/stddef.h>
+#include <linux/stringbuf.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/init.h>
@@ -705,75 +706,28 @@ static char *pnames[] =
     /*2f */ "Useruserdata"
 };
 
-
-
-#include <stdarg.h>
-
-/*-------------------------------------------------------*/
-static _cdebbuf *bufprint(_cdebbuf *cdb, char *fmt,...)
-{
-	va_list f;
-	size_t n,r;
-
-	if (!cdb)
-		return NULL;
-	va_start(f, fmt);
-	r = cdb->size - cdb->pos;
-	n = vsnprintf(cdb->p, r, fmt, f);
-	va_end(f);
-	if (n >= r) {
-		/* truncated, need bigger buffer */
-		size_t ns = 2 * cdb->size;
-		u_char *nb;
-
-		while ((ns - cdb->pos) <= n)
-			ns *= 2;
-		nb = kmalloc(ns, GFP_ATOMIC);
-		if (!nb) {
-			cdebbuf_free(cdb);
-			return NULL;
-		}
-		memcpy(nb, cdb->buf, cdb->pos);
-		kfree(cdb->buf);
-		nb[cdb->pos] = 0;
-		cdb->buf = nb;
-		cdb->p = cdb->buf + cdb->pos;
-		cdb->size = ns;
-		va_start(f, fmt);
-		r = cdb->size - cdb->pos;
-		n = vsnprintf(cdb->p, r, fmt, f);
-		va_end(f);
-	}
-	cdb->p += n;
-	cdb->pos += n;
-	return cdb;
-}
-
-static _cdebbuf *printstructlen(_cdebbuf *cdb, u8 * m, unsigned len)
+static void printstructlen(struct stringbuf *sb, u8 *m, unsigned len)
 {
 	unsigned hex = 0;
 
-	if (!cdb)
-		return NULL;
 	for (; len; len--, m++)
 		if (isalnum(*m) || *m == ' ') {
 			if (hex)
-				cdb = bufprint(cdb, ">");
-			cdb = bufprint(cdb, "%c", *m);
+				sb_printf(sb, ">");
+			sb_printf(sb, "%c", *m);
 			hex = 0;
 		} else {
 			if (!hex)
-				cdb = bufprint(cdb, "<%02x", *m);
+				sb_printf(sb, "<%02x", *m);
 			else
-				cdb = bufprint(cdb, " %02x", *m);
+				sb_printf(sb, " %02x", *m);
 			hex = 1;
 		}
 	if (hex)
-		cdb = bufprint(cdb, ">");
-	return cdb;
+		sb_printf(sb, ">");
 }
 
-static _cdebbuf *printstruct(_cdebbuf *cdb, u8 * m)
+static void printstruct(struct stringbuf *sb, u8 *m)
 {
 	unsigned len;
 
@@ -784,45 +738,42 @@ static _cdebbuf *printstruct(_cdebbuf *cdb, u8 * m)
 		len = ((u16 *) (m + 1))[0];
 		m += 3;
 	}
-	cdb = printstructlen(cdb, m, len);
-	return cdb;
+	printstructlen(sb, m, len);
 }
 
 /*-------------------------------------------------------*/
 #define NAME (pnames[cmsg->par[cmsg->p]])
 
-static _cdebbuf *protocol_message_2_pars(_cdebbuf *cdb, _cmsg *cmsg, int level)
+static void protocol_message_2_pars(struct stringbuf *sb, _cmsg *cmsg, int level)
 {
 	for (; TYP != _CEND; cmsg->p++) {
 		int slen = 29 + 3 - level;
 		int i;
 
-		if (!cdb)
-			return NULL;
-		cdb = bufprint(cdb, "  ");
+		sb_printf(sb, "  ");
 		for (i = 0; i < level - 1; i++)
-			cdb = bufprint(cdb, " ");
+			sb_printf(sb, " ");
 
 		switch (TYP) {
 		case _CBYTE:
-			cdb = bufprint(cdb, "%-*s = 0x%x\n", slen, NAME, *(u8 *) (cmsg->m + cmsg->l));
+			sb_printf(sb, "%-*s = 0x%x\n", slen, NAME, *(u8 *) (cmsg->m + cmsg->l));
 			cmsg->l++;
 			break;
 		case _CWORD:
-			cdb = bufprint(cdb, "%-*s = 0x%x\n", slen, NAME, *(u16 *) (cmsg->m + cmsg->l));
+			sb_printf(sb, "%-*s = 0x%x\n", slen, NAME, *(u16 *) (cmsg->m + cmsg->l));
 			cmsg->l += 2;
 			break;
 		case _CDWORD:
-			cdb = bufprint(cdb, "%-*s = 0x%lx\n", slen, NAME, *(u32 *) (cmsg->m + cmsg->l));
+			sb_printf(sb, "%-*s = 0x%x\n", slen, NAME, *(u32 *) (cmsg->m + cmsg->l));
 			cmsg->l += 4;
 			break;
 		case _CSTRUCT:
-			cdb = bufprint(cdb, "%-*s = ", slen, NAME);
+			sb_printf(sb, "%-*s = ", slen, NAME);
 			if (cmsg->m[cmsg->l] == '\0')
-				cdb = bufprint(cdb, "default");
+				sb_printf(sb, "default");
 			else
-				cdb = printstruct(cdb, cmsg->m + cmsg->l);
-			cdb = bufprint(cdb, "\n");
+				printstruct(sb, cmsg->m + cmsg->l);
+			sb_printf(sb, "\n");
 			if (cmsg->m[cmsg->l] != 0xff)
 				cmsg->l += 1 + cmsg->m[cmsg->l];
 			else
@@ -833,164 +784,102 @@ static _cdebbuf *protocol_message_2_pars(_cdebbuf *cdb, _cmsg *cmsg, int level)
 		case _CMSTRUCT:
 /*----- Metastruktur 0 -----*/
 			if (cmsg->m[cmsg->l] == '\0') {
-				cdb = bufprint(cdb, "%-*s = default\n", slen, NAME);
+				sb_printf(sb, "%-*s = default\n", slen, NAME);
 				cmsg->l++;
 				jumpcstruct(cmsg);
 			} else {
 				char *name = NAME;
 				unsigned _l = cmsg->l;
-				cdb = bufprint(cdb, "%-*s\n", slen, name);
+				sb_printf(sb, "%-*s\n", slen, name);
 				cmsg->l = (cmsg->m + _l)[0] == 255 ? cmsg->l + 3 : cmsg->l + 1;
 				cmsg->p++;
-				cdb = protocol_message_2_pars(cdb, cmsg, level + 1);
+				protocol_message_2_pars(sb, cmsg, level + 1);
 			}
 			break;
 		}
 	}
-	return cdb;
 }
 /*-------------------------------------------------------*/
 
-static _cdebbuf *g_debbuf;
-static u_long g_debbuf_lock;
+static unsigned long g_debbuf_lock;
 static _cmsg *g_cmsg;
 
-static _cdebbuf *cdebbuf_alloc(void)
+static _cmsg *cmsg_alloc(void)
 {
-	_cdebbuf *cdb;
-
-	if (likely(!test_and_set_bit(1, &g_debbuf_lock))) {
-		cdb = g_debbuf;
-		goto init;
-	} else
-		cdb = kmalloc(sizeof(_cdebbuf), GFP_ATOMIC);
-	if (!cdb)
-		return NULL;
-	cdb->buf = kmalloc(CDEBUG_SIZE, GFP_ATOMIC);
-	if (!cdb->buf) {
-		kfree(cdb);
-		return NULL;
-	}
-	cdb->size = CDEBUG_SIZE;
-init:
-	cdb->buf[0] = 0;
-	cdb->p = cdb->buf;
-	cdb->pos = 0;
-	return cdb;
+	if (likely(!test_and_set_bit(1, &g_debbuf_lock)))
+		return g_cmsg;
+	else
+		return kmalloc(sizeof(_cmsg), GFP_ATOMIC);
 }
 
-void cdebbuf_free(_cdebbuf *cdb)
+void cmsg_free(_cmsg *cmsg)
 {
-	if (likely(cdb == g_debbuf)) {
-		test_and_clear_bit(1, &g_debbuf_lock);
-		return;
-	}
-	if (likely(cdb))
-		kfree(cdb->buf);
-	kfree(cdb);
+	if (likely(cmsg == g_cmsg))
+		clear_bit(1, &g_debbuf_lock);
+	else
+		kfree(cmsg);
 }
 
-
-_cdebbuf *capi_message2str(u8 * msg)
+char *capi_message2str(u8 *msg)
 {
-	_cdebbuf *cdb;
-	_cmsg	*cmsg;
-
-	cdb = cdebbuf_alloc();
-	if (unlikely(!cdb))
+	char *s;
+	_cmsg *cmsg = cmsg_alloc();
+	if (unlikely(!cmsg))
 		return NULL;
-	if (likely(cdb == g_debbuf))
-		cmsg = g_cmsg;
-	else
-		cmsg = kmalloc(sizeof(_cmsg), GFP_ATOMIC);
-	if (unlikely(!cmsg)) {
-		cdebbuf_free(cdb);
-		return NULL;
-	}
+
 	cmsg->m = msg;
-	cmsg->l = 8;
-	cmsg->p = 0;
 	byteTRcpy(cmsg->m + 4, &cmsg->Command);
 	byteTRcpy(cmsg->m + 5, &cmsg->Subcommand);
 	cmsg->par = cpars[command_2_index(cmsg->Command, cmsg->Subcommand)];
 
-	cdb = bufprint(cdb, "%-26s ID=%03d #0x%04x LEN=%04d\n",
-		 mnames[command_2_index(cmsg->Command, cmsg->Subcommand)],
-		 ((unsigned short *) msg)[1],
-		 ((unsigned short *) msg)[3],
-		 ((unsigned short *) msg)[0]);
+	s = capi_cmsg2str(cmsg);
+	cmsg_free(cmsg);
 
-	cdb = protocol_message_2_pars(cdb, cmsg, 1);
-	if (unlikely(cmsg != g_cmsg))
-		kfree(cmsg);
-	return cdb;
+	return s;
 }
 
-_cdebbuf *capi_cmsg2str(_cmsg * cmsg)
+char *capi_cmsg2str(_cmsg *cmsg)
 {
-	_cdebbuf *cdb;
+	struct stringbuf sb;
+
+	sb_init(&sb);
 
-	cdb = cdebbuf_alloc();
-	if (!cdb)
-		return NULL;
 	cmsg->l = 8;
 	cmsg->p = 0;
-	cdb = bufprint(cdb, "%s ID=%03d #0x%04x LEN=%04d\n",
+	sb_printf(&sb, "%s ID=%03d #0x%04x LEN=%04d\n",
 		 mnames[command_2_index(cmsg->Command, cmsg->Subcommand)],
 		 ((u16 *) cmsg->m)[1],
 		 ((u16 *) cmsg->m)[3],
 		 ((u16 *) cmsg->m)[0]);
-	cdb = protocol_message_2_pars(cdb, cmsg, 1);
-	return cdb;
+	protocol_message_2_pars(&sb, cmsg, 1);
+	return sb_to_string(&sb);
 }
 
 int __init cdebug_init(void)
 {
-	g_cmsg= kmalloc(sizeof(_cmsg), GFP_KERNEL);
+	g_cmsg = kmalloc(sizeof(_cmsg), GFP_KERNEL);
 	if (!g_cmsg)
 		return ENOMEM;
-	g_debbuf = kmalloc(sizeof(_cdebbuf), GFP_KERNEL);
-	if (!g_debbuf) {
-		kfree(g_cmsg);
-		return ENOMEM;
-	}
-	g_debbuf->buf = kmalloc(CDEBUG_GSIZE, GFP_KERNEL);
-	if (!g_debbuf->buf) {
-		kfree(g_cmsg);
-		kfree(g_debbuf);
-		return ENOMEM;;
-	}
-	g_debbuf->size = CDEBUG_GSIZE;
-	g_debbuf->buf[0] = 0;
-	g_debbuf->p = g_debbuf->buf;
-	g_debbuf->pos = 0;
 	return 0;
 }
 
 void __exit cdebug_exit(void)
 {
-	if (g_debbuf)
-		kfree(g_debbuf->buf);
-	kfree(g_debbuf);
 	kfree(g_cmsg);
 }
 
 #else /* !CONFIG_CAPI_TRACE */
 
-static _cdebbuf g_debbuf = {"CONFIG_CAPI_TRACE not enabled", NULL, 0, 0};
-
-_cdebbuf *capi_message2str(u8 * msg)
-{
-	return &g_debbuf;
-}
+static const char *g_debbuf = "CONFIG_CAPI_TRACE not enabled";
 
-_cdebbuf *capi_cmsg2str(_cmsg * cmsg)
+char *capi_message2str(u8 * msg)
 {
-	return &g_debbuf;
+	return kstrdup(g_debbuf, GFP_ATOMIC);
 }
 
-void cdebbuf_free(_cdebbuf *cdb)
+char *capi_cmsg2str(_cmsg * cmsg)
 {
+	return kstrdup(g_debbuf, GFP_ATOMIC);
 }
 
 int __init cdebug_init(void)
@@ -1004,7 +893,6 @@ void __exit cdebug_exit(void)
 
 #endif
 
-EXPORT_SYMBOL(cdebbuf_free);
 EXPORT_SYMBOL(capi_cmsg2message);
 EXPORT_SYMBOL(capi_message2cmsg);
 EXPORT_SYMBOL(capi_cmsg_header);
diff --git a/drivers/isdn/capi/kcapi.c b/drivers/isdn/capi/kcapi.c
index f555318..3905331 100644
--- a/drivers/isdn/capi/kcapi.c
+++ b/drivers/isdn/capi/kcapi.c
@@ -276,14 +276,13 @@ void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *s
 	int showctl = 0;
 	u8 cmd, subcmd;
 	unsigned long flags;
-	_cdebbuf *cdb;
 
 	if (card->cardstate != CARD_RUNNING) {
-		cdb = capi_message2str(skb->data);
-		if (cdb) {
+		char *s = capi_message2str(skb->data);
+		if (s) {
 			printk(KERN_INFO "kcapi: controller [%03d] not active, got: %s",
-				card->cnr, cdb->buf);
-			cdebbuf_free(cdb);
+				card->cnr, s);
+			kfree(s);
 		} else
 			printk(KERN_INFO "kcapi: controller [%03d] not active, cannot trace\n",
 				card->cnr);
@@ -307,11 +306,11 @@ void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *s
 			       capi_cmd2str(cmd, subcmd),
 			       CAPIMSG_LEN(skb->data));
 		} else {
-			cdb = capi_message2str(skb->data);
-			if (cdb) {
+			char *s = capi_message2str(skb->data);
+			if (s) {
 				printk(KERN_DEBUG "kcapi: got [%03d] %s\n",
-					card->cnr, cdb->buf);
-				cdebbuf_free(cdb);
+					card->cnr, s);
+				kfree(s);
 			} else
 				printk(KERN_DEBUG "kcapi: got [%03d] id#%d %s len=%u, cannot trace\n",
 					card->cnr, CAPIMSG_APPID(skb->data),
@@ -324,12 +323,13 @@ void capi_ctr_handle_message(struct capi_ctr * card, u16 appl, struct sk_buff *s
 	read_lock_irqsave(&application_lock, flags);
 	ap = get_capi_appl_by_nr(CAPIMSG_APPID(skb->data));
 	if ((!ap) || (ap->release_in_progress)) {
+		char *s;
 		read_unlock_irqrestore(&application_lock, flags);
-		cdb = capi_message2str(skb->data);
-		if (cdb) {
+		s = capi_message2str(skb->data);
+		if (s) {
 			printk(KERN_ERR "kcapi: handle_message: applid %d state released (%s)\n",
-			CAPIMSG_APPID(skb->data), cdb->buf);
-			cdebbuf_free(cdb);
+			CAPIMSG_APPID(skb->data), s);
+			kfree(s);
 		} else
 			printk(KERN_ERR "kcapi: handle_message: applid %d state released (%s) cannot trace\n",
 				CAPIMSG_APPID(skb->data),
@@ -649,12 +649,11 @@ u16 capi20_put_message(struct capi20_appl *ap, struct sk_buff *skb)
 			       capi_cmd2str(cmd, subcmd),
 			       CAPIMSG_LEN(skb->data));
 		} else {
-			_cdebbuf *cdb = capi_message2str(skb->data);
-			if (cdb) {
+			char *s = capi_message2str(skb->data);
+			if (s) {
 				printk(KERN_DEBUG "kcapi: put [%03d] %s\n",
-					CAPIMSG_CONTROLLER(skb->data),
-					cdb->buf);
-				cdebbuf_free(cdb);
+					CAPIMSG_CONTROLLER(skb->data), s);
+				kfree(s);
 			} else
 				printk(KERN_DEBUG "kcapi: put [%03d] id#%d %s len=%u cannot trace\n",
 					CAPIMSG_CONTROLLER(skb->data),
diff --git a/include/linux/isdn/capiutil.h b/include/linux/isdn/capiutil.h
index 5a52f2c..bd48c51 100644
--- a/include/linux/isdn/capiutil.h
+++ b/include/linux/isdn/capiutil.h
@@ -177,22 +177,12 @@ char *capi_info2str(__u16 reason);
 
 char *capi_cmd2str(__u8 cmd, __u8 subcmd);
 
-typedef struct {
-	u_char	*buf;
-	u_char	*p;
-	size_t	size;
-	size_t	pos;
-} _cdebbuf;
-
-#define	CDEBUG_SIZE	1024
-#define	CDEBUG_GSIZE	4096
-
-void cdebbuf_free(_cdebbuf *cdb);
 int cdebug_init(void);
 void cdebug_exit(void);
 
-_cdebbuf *capi_cmsg2str(_cmsg *cmsg);
-_cdebbuf *capi_message2str(__u8 *msg);
+/* Callers must kfree the strings returned from these functions */
+char *capi_cmsg2str(_cmsg *cmsg);
+char *capi_message2str(__u8 *msg);
 
 /*-----------------------------------------------------------------------*/
 
-- 
1.5.3.4


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH 3/4] sound: Use stringbuf
  2007-10-23 21:12 ` [PATCH 2/4] isdn: Use stringbuf Matthew Wilcox
@ 2007-10-23 21:12   ` Matthew Wilcox
  2007-10-23 21:12     ` [PATCH 4/4] partitions: Fix non-atomic printk Matthew Wilcox
  2007-10-24 14:18     ` [PATCH 3/4] sound: Use stringbuf Takashi Iwai
  0 siblings, 2 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-23 21:12 UTC (permalink / raw)
  To: torvalds, akpm, linux-kernel; +Cc: Matthew Wilcox, Matthew Wilcox

sound/ had its own snd_info_buffer for doing proc reads, which can be
profitably replaced with stringbuf.  It actually finds a bug since ->read
and ->write now have a different signature.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 include/sound/info.h                    |    7 ++-
 sound/core/hwdep.c                      |    2 +-
 sound/core/info.c                       |   81 +++++--------------------------
 sound/core/info_oss.c                   |    6 +-
 sound/core/init.c                       |    8 ++--
 sound/core/oss/mixer_oss.c              |    2 +-
 sound/core/oss/pcm_oss.c                |    2 +-
 sound/core/pcm.c                        |   16 +++---
 sound/core/pcm_memory.c                 |    4 +-
 sound/core/rawmidi.c                    |    2 +-
 sound/core/seq/oss/seq_oss.c            |    2 +-
 sound/core/seq/oss/seq_oss_device.h     |    8 ++--
 sound/core/seq/oss/seq_oss_init.c       |    2 +-
 sound/core/seq/oss/seq_oss_midi.c       |    2 +-
 sound/core/seq/oss/seq_oss_readq.c      |    2 +-
 sound/core/seq/oss/seq_oss_synth.c      |    2 +-
 sound/core/seq/seq_clientmgr.c          |    8 ++--
 sound/core/seq/seq_device.c             |    2 +-
 sound/core/seq/seq_info.c               |    2 +-
 sound/core/seq/seq_info.h               |    6 +-
 sound/core/seq/seq_memory.c             |    2 +-
 sound/core/seq/seq_queue.c              |    2 +-
 sound/core/seq/seq_timer.c              |    2 +-
 sound/core/sound.c                      |    2 +-
 sound/core/sound_oss.c                  |    2 +-
 sound/core/timer.c                      |    2 +-
 sound/drivers/vx/vx_core.c              |    2 +-
 sound/i2c/l3/uda1341.c                  |    4 +-
 sound/isa/ad1848/ad1848_lib.c           |    2 +-
 sound/isa/gus/gus_irq.c                 |    2 +-
 sound/isa/gus/gus_mem.c                 |    4 +-
 sound/isa/opti9xx/miro.c                |    2 +-
 sound/isa/sb/sb16_csp.c                 |    4 +-
 sound/pci/ac97/ac97_proc.c              |   12 ++--
 sound/pci/ac97/ak4531_codec.c           |    2 +-
 sound/pci/ad1889.c                      |    2 +-
 sound/pci/ali5451/ali5451.c             |    2 +-
 sound/pci/atiixp.c                      |    2 +-
 sound/pci/atiixp_modem.c                |    2 +-
 sound/pci/ca0106/ca0106_proc.c          |   17 +++----
 sound/pci/cmipci.c                      |    2 +-
 sound/pci/cs4281.c                      |    2 +-
 sound/pci/cs46xx/dsp_spos.c             |   14 +++---
 sound/pci/cs46xx/dsp_spos_scb_lib.c     |    4 +-
 sound/pci/emu10k1/emu10k1x.c            |    2 +-
 sound/pci/emu10k1/emuproc.c             |   28 +++++-----
 sound/pci/ens1370.c                     |    2 +-
 sound/pci/hda/hda_proc.c                |   16 +++---
 sound/pci/ice1712/ice1712.c             |    2 +-
 sound/pci/ice1712/ice1724.c             |    2 +-
 sound/pci/ice1712/pontis.c              |    4 +-
 sound/pci/intel8x0.c                    |    2 +-
 sound/pci/intel8x0m.c                   |    2 +-
 sound/pci/korg1212/korg1212.c           |    2 +-
 sound/pci/mixart/mixart.c               |    2 +-
 sound/pci/pcxhr/pcxhr.c                 |    4 +-
 sound/pci/riptide/riptide.c             |    2 +-
 sound/pci/rme32.c                       |    2 +-
 sound/pci/rme96.c                       |    2 +-
 sound/pci/rme9652/hdsp.c                |    2 +-
 sound/pci/rme9652/hdspm.c               |    6 +-
 sound/pci/rme9652/rme9652.c             |    2 +-
 sound/pci/sonicvibes.c                  |    2 +-
 sound/pci/trident/trident_main.c        |    2 +-
 sound/pci/via82xx.c                     |    2 +-
 sound/pci/via82xx_modem.c               |    2 +-
 sound/pci/ymfpci/ymfpci_main.c          |    2 +-
 sound/pcmcia/pdaudiocf/pdaudiocf_core.c |    2 +-
 sound/sparc/dbri.c                      |    4 +-
 sound/synth/emux/emux_proc.c            |    2 +-
 sound/usb/usbaudio.c                    |   10 ++--
 sound/usb/usbmixer.c                    |    2 +-
 72 files changed, 158 insertions(+), 215 deletions(-)

diff --git a/include/sound/info.h b/include/sound/info.h
index fecbb1f..4066355 100644
--- a/include/sound/info.h
+++ b/include/sound/info.h
@@ -23,6 +23,7 @@
  */
 
 #include <linux/poll.h>
+#include <linux/stringbuf.h>
 
 /* buffer for information */
 struct snd_info_buffer {
@@ -40,7 +41,7 @@ struct snd_info_buffer {
 struct snd_info_entry;
 
 struct snd_info_entry_text {
-	void (*read) (struct snd_info_entry *entry, struct snd_info_buffer *buffer);
+	void (*read) (struct snd_info_entry *entry, struct stringbuf *buffer);
 	void (*write) (struct snd_info_entry *entry, struct snd_info_buffer *buffer);
 };
 
@@ -104,7 +105,7 @@ extern struct snd_info_entry *snd_oss_root;
 #define snd_oss_root NULL
 #endif
 
-int snd_iprintf(struct snd_info_buffer * buffer, char *fmt,...) __attribute__ ((format (printf, 2, 3)));
+#define snd_iprintf(buffer, fmt, args...) sb_printf(buffer, fmt , ## args)
 int snd_info_init(void);
 int snd_info_done(void);
 
@@ -131,7 +132,7 @@ int snd_card_proc_new(struct snd_card *card, const char *name, struct snd_info_e
 
 static inline void snd_info_set_text_ops(struct snd_info_entry *entry, 
 					 void *private_data,
-					 void (*read)(struct snd_info_entry *, struct snd_info_buffer *))
+					 void (*read)(struct snd_info_entry *, struct stringbuf *))
 {
 	entry->private_data = private_data;
 	entry->c.text.read = read;
diff --git a/sound/core/hwdep.c b/sound/core/hwdep.c
index bfd9d18..908e6ca 100644
--- a/sound/core/hwdep.c
+++ b/sound/core/hwdep.c
@@ -463,7 +463,7 @@ static int snd_hwdep_dev_disconnect(struct snd_device *device)
  */
 
 static void snd_hwdep_proc_read(struct snd_info_entry *entry,
-				struct snd_info_buffer *buffer)
+				struct stringbuf *buffer)
 {
 	struct snd_hwdep *hwdep;
 
diff --git a/sound/core/info.c b/sound/core/info.c
index 1ffd29b..f3562e8 100644
--- a/sound/core/info.c
+++ b/sound/core/info.c
@@ -70,7 +70,7 @@ int snd_info_check_reserved_words(const char *str)
 static DEFINE_MUTEX(info_mutex);
 
 struct snd_info_private_data {
-	struct snd_info_buffer *rbuffer;
+	struct stringbuf rbuffer;
 	struct snd_info_buffer *wbuffer;
 	struct snd_info_entry *entry;
 	void *file_private_data;
@@ -99,49 +99,6 @@ static int resize_info_buffer(struct snd_info_buffer *buffer,
 	return 0;
 }
 
-/**
- * snd_iprintf - printf on the procfs buffer
- * @buffer: the procfs buffer
- * @fmt: the printf format
- *
- * Outputs the string on the procfs buffer just like printf().
- *
- * Returns the size of output string.
- */
-int snd_iprintf(struct snd_info_buffer *buffer, char *fmt,...)
-{
-	va_list args;
-	int len, res;
-	int err = 0;
-
-	might_sleep();
-	if (buffer->stop || buffer->error)
-		return 0;
-	len = buffer->len - buffer->size;
-	va_start(args, fmt);
-	for (;;) {
-		va_list ap;
-		va_copy(ap, args);
-		res = vsnprintf(buffer->buffer + buffer->curr, len, fmt, ap);
-		va_end(ap);
-		if (res < len)
-			break;
-		err = resize_info_buffer(buffer, buffer->len + PAGE_SIZE);
-		if (err < 0)
-			break;
-		len = buffer->len - buffer->size;
-	}
-	va_end(args);
-
-	if (err < 0)
-		return err;
-	buffer->curr += res;
-	buffer->size += res;
-	return res;
-}
-
-EXPORT_SYMBOL(snd_iprintf);
-
 /*
 
  */
@@ -212,7 +169,7 @@ static ssize_t snd_info_entry_read(struct file *file, char __user *buffer,
 {
 	struct snd_info_private_data *data;
 	struct snd_info_entry *entry;
-	struct snd_info_buffer *buf;
+	struct stringbuf *buf;
 	size_t size = 0;
 	loff_t pos;
 
@@ -226,14 +183,13 @@ static ssize_t snd_info_entry_read(struct file *file, char __user *buffer,
 	entry = data->entry;
 	switch (entry->content) {
 	case SNDRV_INFO_CONTENT_TEXT:
-		buf = data->rbuffer;
-		if (buf == NULL)
-			return -EIO;
-		if (pos >= buf->size)
+		buf = &data->rbuffer;
+		size = sb_len(buf);
+		if (pos >= size)
 			return 0;
-		size = buf->size - pos;
+		size -= pos;
 		size = min(count, size);
-		if (copy_to_user(buffer, buf->buffer + pos, size))
+		if (copy_to_user(buffer, sb_string(buf) + pos, size))
 			return -EFAULT;
 		break;
 	case SNDRV_INFO_CONTENT_DATA:
@@ -340,14 +296,7 @@ static int snd_info_entry_open(struct inode *inode, struct file *file)
 	switch (entry->content) {
 	case SNDRV_INFO_CONTENT_TEXT:
 		if (mode == O_RDONLY || mode == O_RDWR) {
-			buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
-			if (buffer == NULL)
-				goto __nomem;
-			data->rbuffer = buffer;
-			buffer->len = PAGE_SIZE;
-			buffer->buffer = kmalloc(buffer->len, GFP_KERNEL);
-			if (buffer->buffer == NULL)
-				goto __nomem;
+			sb_init(&data->rbuffer);
 		}
 		if (mode == O_WRONLY || mode == O_RDWR) {
 			buffer = kzalloc(sizeof(*buffer), GFP_KERNEL);
@@ -376,17 +325,14 @@ static int snd_info_entry_open(struct inode *inode, struct file *file)
 	    (mode == O_RDONLY || mode == O_RDWR)) {
 		if (entry->c.text.read) {
 			mutex_lock(&entry->access);
-			entry->c.text.read(entry, data->rbuffer);
+			entry->c.text.read(entry, &data->rbuffer);
 			mutex_unlock(&entry->access);
 		}
 	}
 	return 0;
 
  __nomem:
-	if (data->rbuffer) {
-		kfree(data->rbuffer->buffer);
-		kfree(data->rbuffer);
-	}
+	sb_free(&data->rbuffer);
 	if (data->wbuffer) {
 		kfree(data->wbuffer->buffer);
 		kfree(data->wbuffer);
@@ -411,10 +357,7 @@ static int snd_info_entry_release(struct inode *inode, struct file *file)
 	entry = data->entry;
 	switch (entry->content) {
 	case SNDRV_INFO_CONTENT_TEXT:
-		if (data->rbuffer) {
-			kfree(data->rbuffer->buffer);
-			kfree(data->rbuffer);
-		}
+		sb_free(&data->rbuffer);
 		if (data->wbuffer) {
 			if (entry->c.text.write) {
 				entry->c.text.write(entry, data->wbuffer);
@@ -975,7 +918,7 @@ EXPORT_SYMBOL(snd_info_register);
 
 static struct snd_info_entry *snd_info_version_entry;
 
-static void snd_info_version_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void snd_info_version_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	snd_iprintf(buffer,
 		    "Advanced Linux Sound Architecture Driver Version "
diff --git a/sound/core/info_oss.c b/sound/core/info_oss.c
index 435c939..15d3f43 100644
--- a/sound/core/info_oss.c
+++ b/sound/core/info_oss.c
@@ -66,9 +66,9 @@ int snd_oss_info_register(int dev, int num, char *string)
 
 EXPORT_SYMBOL(snd_oss_info_register);
 
-extern void snd_card_info_read_oss(struct snd_info_buffer *buffer);
+extern void snd_card_info_read_oss(struct stringbuf *buffer);
 
-static int snd_sndstat_show_strings(struct snd_info_buffer *buf, char *id, int dev)
+static int snd_sndstat_show_strings(struct stringbuf *buf, char *id, int dev)
 {
 	int idx, ok = -1;
 	char *str;
@@ -92,7 +92,7 @@ static int snd_sndstat_show_strings(struct snd_info_buffer *buf, char *id, int d
 }
 
 static void snd_sndstat_proc_read(struct snd_info_entry *entry,
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	snd_iprintf(buffer, "Sound Driver:3.8.1a-980706 (ALSA v" CONFIG_SND_VERSION " emulation code)\n");
 	snd_iprintf(buffer, "Kernel: %s %s %s %s %s\n",
diff --git a/sound/core/init.c b/sound/core/init.c
index 2cb7099..241bb94 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -50,7 +50,7 @@ EXPORT_SYMBOL(snd_mixer_oss_notify_callback);
 
 #ifdef CONFIG_PROC_FS
 static void snd_card_id_read(struct snd_info_entry *entry,
-			     struct snd_info_buffer *buffer)
+			     struct stringbuf *buffer)
 {
 	snd_iprintf(buffer, "%s\n", entry->card->id);
 }
@@ -538,7 +538,7 @@ EXPORT_SYMBOL(snd_card_register);
 static struct snd_info_entry *snd_card_info_entry;
 
 static void snd_card_info_read(struct snd_info_entry *entry,
-			       struct snd_info_buffer *buffer)
+			       struct stringbuf *buffer)
 {
 	int idx, count;
 	struct snd_card *card;
@@ -563,7 +563,7 @@ static void snd_card_info_read(struct snd_info_entry *entry,
 
 #ifdef CONFIG_SND_OSSEMUL
 
-void snd_card_info_read_oss(struct snd_info_buffer *buffer)
+void snd_card_info_read_oss(struct stringbuf *buffer)
 {
 	int idx, count;
 	struct snd_card *card;
@@ -586,7 +586,7 @@ void snd_card_info_read_oss(struct snd_info_buffer *buffer)
 #ifdef MODULE
 static struct snd_info_entry *snd_card_module_info_entry;
 static void snd_card_module_info_read(struct snd_info_entry *entry,
-				      struct snd_info_buffer *buffer)
+				      struct stringbuf *buffer)
 {
 	int idx;
 	struct snd_card *card;
diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index 3ace4a5..2510eab 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -1088,7 +1088,7 @@ static char *oss_mixer_names[SNDRV_OSS_MAX_MIXERS] = {
  */
 
 static void snd_mixer_oss_proc_read(struct snd_info_entry *entry,
-				    struct snd_info_buffer *buffer)
+				    struct stringbuf *buffer)
 {
 	struct snd_mixer_oss *mixer = entry->private_data;
 	int i;
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index d0c4ceb..50644ee 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2768,7 +2768,7 @@ static int snd_pcm_oss_mmap(struct file *file, struct vm_area_struct *area)
  */
 
 static void snd_pcm_oss_proc_read(struct snd_info_entry *entry,
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct snd_pcm_str *pstr = entry->private_data;
 	struct snd_pcm_oss_setup *setup = pstr->oss.setup_list;
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index cf9b949..ab19818 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -291,7 +291,7 @@ static const char *snd_pcm_oss_format_name(int format)
 #endif
 
 static void snd_pcm_proc_info_read(struct snd_pcm_substream *substream,
-				   struct snd_info_buffer *buffer)
+				   struct stringbuf *buffer)
 {
 	struct snd_pcm_info *info;
 	int err;
@@ -326,21 +326,21 @@ static void snd_pcm_proc_info_read(struct snd_pcm_substream *substream,
 }
 
 static void snd_pcm_stream_proc_info_read(struct snd_info_entry *entry,
-					  struct snd_info_buffer *buffer)
+					  struct stringbuf *buffer)
 {
 	snd_pcm_proc_info_read(((struct snd_pcm_str *)entry->private_data)->substream,
 			       buffer);
 }
 
 static void snd_pcm_substream_proc_info_read(struct snd_info_entry *entry,
-					     struct snd_info_buffer *buffer)
+					     struct stringbuf *buffer)
 {
 	snd_pcm_proc_info_read((struct snd_pcm_substream *)entry->private_data,
 			       buffer);
 }
 
 static void snd_pcm_substream_proc_hw_params_read(struct snd_info_entry *entry,
-						  struct snd_info_buffer *buffer)
+						  struct stringbuf *buffer)
 {
 	struct snd_pcm_substream *substream = entry->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -373,7 +373,7 @@ static void snd_pcm_substream_proc_hw_params_read(struct snd_info_entry *entry,
 }
 
 static void snd_pcm_substream_proc_sw_params_read(struct snd_info_entry *entry,
-						  struct snd_info_buffer *buffer)
+						  struct stringbuf *buffer)
 {
 	struct snd_pcm_substream *substream = entry->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -398,7 +398,7 @@ static void snd_pcm_substream_proc_sw_params_read(struct snd_info_entry *entry,
 }
 
 static void snd_pcm_substream_proc_status_read(struct snd_info_entry *entry,
-					       struct snd_info_buffer *buffer)
+					       struct stringbuf *buffer)
 {
 	struct snd_pcm_substream *substream = entry->private_data;
 	struct snd_pcm_runtime *runtime = substream->runtime;
@@ -429,7 +429,7 @@ static void snd_pcm_substream_proc_status_read(struct snd_info_entry *entry,
 
 #ifdef CONFIG_SND_PCM_XRUN_DEBUG
 static void snd_pcm_xrun_debug_read(struct snd_info_entry *entry,
-				    struct snd_info_buffer *buffer)
+				    struct stringbuf *buffer)
 {
 	struct snd_pcm_str *pstr = entry->private_data;
 	snd_iprintf(buffer, "%d\n", pstr->xrun_debug);
@@ -1058,7 +1058,7 @@ EXPORT_SYMBOL(snd_pcm_notify);
  */
 
 static void snd_pcm_proc_read(struct snd_info_entry *entry,
-			      struct snd_info_buffer *buffer)
+			      struct stringbuf *buffer)
 {
 	struct snd_pcm *pcm;
 
diff --git a/sound/core/pcm_memory.c b/sound/core/pcm_memory.c
index a13e38c..754720f 100644
--- a/sound/core/pcm_memory.c
+++ b/sound/core/pcm_memory.c
@@ -137,7 +137,7 @@ EXPORT_SYMBOL(snd_pcm_lib_preallocate_free_for_all);
  * prints the current allocated size in kB.
  */
 static void snd_pcm_lib_preallocate_proc_read(struct snd_info_entry *entry,
-					      struct snd_info_buffer *buffer)
+					      struct stringbuf *buffer)
 {
 	struct snd_pcm_substream *substream = entry->private_data;
 	snd_iprintf(buffer, "%lu\n", (unsigned long) substream->dma_buffer.bytes / 1024);
@@ -149,7 +149,7 @@ static void snd_pcm_lib_preallocate_proc_read(struct snd_info_entry *entry,
  * prints the maximum allowed size in kB.
  */
 static void snd_pcm_lib_preallocate_max_proc_read(struct snd_info_entry *entry,
-						  struct snd_info_buffer *buffer)
+						  struct stringbuf *buffer)
 {
 	struct snd_pcm_substream *substream = entry->private_data;
 	snd_iprintf(buffer, "%lu\n", (unsigned long) substream->dma_max / 1024);
diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c
index b8e700b..2bc574f 100644
--- a/sound/core/rawmidi.c
+++ b/sound/core/rawmidi.c
@@ -1299,7 +1299,7 @@ static unsigned int snd_rawmidi_poll(struct file *file, poll_table * wait)
  */
 
 static void snd_rawmidi_proc_info_read(struct snd_info_entry *entry,
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct snd_rawmidi *rmidi;
 	struct snd_rawmidi_substream *substream;
diff --git a/sound/core/seq/oss/seq_oss.c b/sound/core/seq/oss/seq_oss.c
index bc09923..00b357b 100644
--- a/sound/core/seq/oss/seq_oss.c
+++ b/sound/core/seq/oss/seq_oss.c
@@ -268,7 +268,7 @@ unregister_device(void)
 static struct snd_info_entry *info_entry;
 
 static void
-info_read(struct snd_info_entry *entry, struct snd_info_buffer *buf)
+info_read(struct snd_info_entry *entry, struct stringbuf *buf)
 {
 	mutex_lock(&register_mutex);
 	snd_iprintf(buf, "OSS sequencer emulation version %s\n", SNDRV_SEQ_OSS_VERSION_STR);
diff --git a/sound/core/seq/oss/seq_oss_device.h b/sound/core/seq/oss/seq_oss_device.h
index 9a8567c..d1c2939 100644
--- a/sound/core/seq/oss/seq_oss_device.h
+++ b/sound/core/seq/oss/seq_oss_device.h
@@ -139,10 +139,10 @@ void snd_seq_oss_process_queue(struct seq_oss_devinfo *dp, abstime_t time);
 
 
 /* proc interface */
-void snd_seq_oss_system_info_read(struct snd_info_buffer *buf);
-void snd_seq_oss_midi_info_read(struct snd_info_buffer *buf);
-void snd_seq_oss_synth_info_read(struct snd_info_buffer *buf);
-void snd_seq_oss_readq_info_read(struct seq_oss_readq *q, struct snd_info_buffer *buf);
+void snd_seq_oss_system_info_read(struct stringbuf *buf);
+void snd_seq_oss_midi_info_read(struct stringbuf *buf);
+void snd_seq_oss_synth_info_read(struct stringbuf *buf);
+void snd_seq_oss_readq_info_read(struct seq_oss_readq *q, struct stringbuf *buf);
 
 /* file mode macros */
 #define is_read_mode(mode)	((mode) & SNDRV_SEQ_OSS_FILE_READ)
diff --git a/sound/core/seq/oss/seq_oss_init.c b/sound/core/seq/oss/seq_oss_init.c
index d0d721c..b32af27 100644
--- a/sound/core/seq/oss/seq_oss_init.c
+++ b/sound/core/seq/oss/seq_oss_init.c
@@ -515,7 +515,7 @@ filemode_str(int val)
  * proc interface
  */
 void
-snd_seq_oss_system_info_read(struct snd_info_buffer *buf)
+snd_seq_oss_system_info_read(struct stringbuf *buf)
 {
 	int i;
 	struct seq_oss_devinfo *dp;
diff --git a/sound/core/seq/oss/seq_oss_midi.c b/sound/core/seq/oss/seq_oss_midi.c
index 0a711d2..959e88e 100644
--- a/sound/core/seq/oss/seq_oss_midi.c
+++ b/sound/core/seq/oss/seq_oss_midi.c
@@ -687,7 +687,7 @@ capmode_str(int val)
 }
 
 void
-snd_seq_oss_midi_info_read(struct snd_info_buffer *buf)
+snd_seq_oss_midi_info_read(struct stringbuf *buf)
 {
 	int i;
 	struct seq_oss_midi *mdev;
diff --git a/sound/core/seq/oss/seq_oss_readq.c b/sound/core/seq/oss/seq_oss_readq.c
index f5de79f..6fcd892 100644
--- a/sound/core/seq/oss/seq_oss_readq.c
+++ b/sound/core/seq/oss/seq_oss_readq.c
@@ -227,7 +227,7 @@ snd_seq_oss_readq_put_timestamp(struct seq_oss_readq *q, unsigned long curt, int
  * proc interface
  */
 void
-snd_seq_oss_readq_info_read(struct seq_oss_readq *q, struct snd_info_buffer *buf)
+snd_seq_oss_readq_info_read(struct seq_oss_readq *q, struct stringbuf *buf)
 {
 	snd_iprintf(buf, "  read queue [%s] length = %d : tick = %ld\n",
 		    (waitqueue_active(&q->midi_sleep) ? "sleeping":"running"),
diff --git a/sound/core/seq/oss/seq_oss_synth.c b/sound/core/seq/oss/seq_oss_synth.c
index ab570a0..b9f216f 100644
--- a/sound/core/seq/oss/seq_oss_synth.c
+++ b/sound/core/seq/oss/seq_oss_synth.c
@@ -626,7 +626,7 @@ snd_seq_oss_synth_make_info(struct seq_oss_devinfo *dp, int dev, struct synth_in
  * proc interface
  */
 void
-snd_seq_oss_synth_info_read(struct snd_info_buffer *buf)
+snd_seq_oss_synth_info_read(struct stringbuf *buf)
 {
 	int i;
 	struct seq_oss_synth *rec;
diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c
index 2e3fa25..5283017 100644
--- a/sound/core/seq/seq_clientmgr.c
+++ b/sound/core/seq/seq_clientmgr.c
@@ -2427,7 +2427,7 @@ EXPORT_SYMBOL(snd_seq_kernel_client_write_poll);
 /*
  *  /proc interface
  */
-static void snd_seq_info_dump_subscribers(struct snd_info_buffer *buffer,
+static void snd_seq_info_dump_subscribers(struct stringbuf *buffer,
 					  struct snd_seq_port_subs_info *group,
 					  int is_src, char *msg)
 {
@@ -2466,7 +2466,7 @@ static void snd_seq_info_dump_subscribers(struct snd_info_buffer *buffer,
 
 #define FLAG_PERM_DUPLEX(perm) ((perm) & SNDRV_SEQ_PORT_CAP_DUPLEX ? 'X' : '-')
 
-static void snd_seq_info_dump_ports(struct snd_info_buffer *buffer,
+static void snd_seq_info_dump_ports(struct stringbuf *buffer,
 				    struct snd_seq_client *client)
 {
 	struct snd_seq_client_port *p;
@@ -2486,12 +2486,12 @@ static void snd_seq_info_dump_ports(struct snd_info_buffer *buffer,
 }
 
 
-void snd_seq_info_pool(struct snd_info_buffer *buffer,
+void snd_seq_info_pool(struct stringbuf *buffer,
 		       struct snd_seq_pool *pool, char *space);
 
 /* exported to seq_info.c */
 void snd_seq_info_clients_read(struct snd_info_entry *entry, 
-			       struct snd_info_buffer *buffer)
+			       struct stringbuf *buffer)
 {
 	int c;
 	struct snd_seq_client *client;
diff --git a/sound/core/seq/seq_device.c b/sound/core/seq/seq_device.c
index 37852cd..193ce08 100644
--- a/sound/core/seq/seq_device.c
+++ b/sound/core/seq/seq_device.c
@@ -104,7 +104,7 @@ static void remove_drivers(void);
 
 #ifdef CONFIG_PROC_FS
 static void snd_seq_device_info(struct snd_info_entry *entry,
-				struct snd_info_buffer *buffer)
+				struct stringbuf *buffer)
 {
 	struct ops_list *ops;
 
diff --git a/sound/core/seq/seq_info.c b/sound/core/seq/seq_info.c
index 8a7fe5c..2e490b3 100644
--- a/sound/core/seq/seq_info.c
+++ b/sound/core/seq/seq_info.c
@@ -35,7 +35,7 @@ static struct snd_info_entry *timer_entry;
 
 static struct snd_info_entry * __init
 create_info_entry(char *name, void (*read)(struct snd_info_entry *,
-					   struct snd_info_buffer *))
+					   struct stringbuf *))
 {
 	struct snd_info_entry *entry;
 
diff --git a/sound/core/seq/seq_info.h b/sound/core/seq/seq_info.h
index 4892a7f..79bce50 100644
--- a/sound/core/seq/seq_info.h
+++ b/sound/core/seq/seq_info.h
@@ -24,9 +24,9 @@
 #include <sound/info.h>
 #include <sound/seq_kernel.h>
 
-void snd_seq_info_clients_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer);
-void snd_seq_info_timer_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer);
-void snd_seq_info_queues_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer);
+void snd_seq_info_clients_read(struct snd_info_entry *entry, struct stringbuf *buffer);
+void snd_seq_info_timer_read(struct snd_info_entry *entry, struct stringbuf *buffer);
+void snd_seq_info_queues_read(struct snd_info_entry *entry, struct stringbuf *buffer);
 
 
 #ifdef CONFIG_PROC_FS
diff --git a/sound/core/seq/seq_memory.c b/sound/core/seq/seq_memory.c
index a72a194..4a7e7ba 100644
--- a/sound/core/seq/seq_memory.c
+++ b/sound/core/seq/seq_memory.c
@@ -504,7 +504,7 @@ void __exit snd_sequencer_memory_done(void)
 
 
 /* exported to seq_clientmgr.c */
-void snd_seq_info_pool(struct snd_info_buffer *buffer,
+void snd_seq_info_pool(struct stringbuf *buffer,
 		       struct snd_seq_pool *pool, char *space)
 {
 	if (pool == NULL)
diff --git a/sound/core/seq/seq_queue.c b/sound/core/seq/seq_queue.c
index 9b87bb0..47dee4f 100644
--- a/sound/core/seq/seq_queue.c
+++ b/sound/core/seq/seq_queue.c
@@ -759,7 +759,7 @@ int snd_seq_control_queue(struct snd_seq_event *ev, int atomic, int hop)
 #ifdef CONFIG_PROC_FS
 /* exported to seq_info.c */
 void snd_seq_info_queues_read(struct snd_info_entry *entry, 
-			      struct snd_info_buffer *buffer)
+			      struct stringbuf *buffer)
 {
 	int i, bpm;
 	struct snd_seq_queue *q;
diff --git a/sound/core/seq/seq_timer.c b/sound/core/seq/seq_timer.c
index 8716352..eeff0d3 100644
--- a/sound/core/seq/seq_timer.c
+++ b/sound/core/seq/seq_timer.c
@@ -428,7 +428,7 @@ snd_seq_tick_time_t snd_seq_timer_get_cur_tick(struct snd_seq_timer *tmr)
 #ifdef CONFIG_PROC_FS
 /* exported to seq_info.c */
 void snd_seq_info_timer_read(struct snd_info_entry *entry,
-			     struct snd_info_buffer *buffer)
+			     struct stringbuf *buffer)
 {
 	int idx;
 	struct snd_seq_queue *q;
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 7b486c4..b48b686 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -379,7 +379,7 @@ static const char *snd_device_type_name(int type)
 	}
 }
 
-static void snd_minor_info_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void snd_minor_info_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	int minor;
 	struct snd_minor *mptr;
diff --git a/sound/core/sound_oss.c b/sound/core/sound_oss.c
index dc73313..96d503e 100644
--- a/sound/core/sound_oss.c
+++ b/sound/core/sound_oss.c
@@ -229,7 +229,7 @@ static const char *snd_oss_device_type_name(int type)
 }
 
 static void snd_minor_info_oss_read(struct snd_info_entry *entry,
-				    struct snd_info_buffer *buffer)
+				    struct stringbuf *buffer)
 {
 	int minor;
 	struct snd_minor *mptr;
diff --git a/sound/core/timer.c b/sound/core/timer.c
index e7dc56c..05bca6f 100644
--- a/sound/core/timer.c
+++ b/sound/core/timer.c
@@ -1026,7 +1026,7 @@ static int snd_timer_register_system(void)
  */
 
 static void snd_timer_proc_read(struct snd_info_entry *entry,
-				struct snd_info_buffer *buffer)
+				struct stringbuf *buffer)
 {
 	struct snd_timer *timer;
 	struct snd_timer_instance *ti;
diff --git a/sound/drivers/vx/vx_core.c b/sound/drivers/vx/vx_core.c
index ed19bc1..433a288 100644
--- a/sound/drivers/vx/vx_core.c
+++ b/sound/drivers/vx/vx_core.c
@@ -592,7 +592,7 @@ static void vx_reset_board(struct vx_core *chip, int cold_reset)
  * proc interface
  */
 
-static void vx_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void vx_proc_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct vx_core *chip = entry->private_data;
 	static char *audio_src_vxp[] = { "Line", "Mic", "Digital" };
diff --git a/sound/i2c/l3/uda1341.c b/sound/i2c/l3/uda1341.c
index b074fdd..83f16a3 100644
--- a/sound/i2c/l3/uda1341.c
+++ b/sound/i2c/l3/uda1341.c
@@ -428,7 +428,7 @@ static const char *peak_value[] = {
 };
 
 static void snd_uda1341_proc_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct l3_client *clnt = entry->private_data;
 	struct uda1341 *uda = clnt->driver_data;
@@ -493,7 +493,7 @@ static void snd_uda1341_proc_read(struct snd_info_entry *entry,
 }
 
 static void snd_uda1341_proc_regs_read(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct l3_client *clnt = entry->private_data;
 	struct uda1341 *uda = clnt->driver_data;		
diff --git a/sound/isa/ad1848/ad1848_lib.c b/sound/isa/ad1848/ad1848_lib.c
index a901cd1..9a64035 100644
--- a/sound/isa/ad1848/ad1848_lib.c
+++ b/sound/isa/ad1848/ad1848_lib.c
@@ -213,7 +213,7 @@ static void snd_ad1848_mce_down(struct snd_ad1848 *chip)
 	for (timeout = 12000; timeout > 0 && (inb(AD1848P(chip, REGSEL)) & AD1848_INIT); timeout--)
 		udelay(100);
 
-	snd_printdd("(1) timeout = %d\n", timeout);
+	snd_printdd("(1) timeout = %ld\n", timeout);
 
 #ifdef CONFIG_SND_DEBUG
 	if (inb(AD1848P(chip, REGSEL)) & AD1848_INIT)
diff --git a/sound/isa/gus/gus_irq.c b/sound/isa/gus/gus_irq.c
index cd9a6f1..035d9fc 100644
--- a/sound/isa/gus/gus_irq.c
+++ b/sound/isa/gus/gus_irq.c
@@ -114,7 +114,7 @@ __again:
 
 #ifdef CONFIG_SND_DEBUG
 static void snd_gus_irq_info_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct snd_gus_card *gus;
 	struct snd_gus_voice *pvoice;
diff --git a/sound/isa/gus/gus_mem.c b/sound/isa/gus/gus_mem.c
index bcf4656..d47b2d0 100644
--- a/sound/isa/gus/gus_mem.c
+++ b/sound/isa/gus/gus_mem.c
@@ -28,7 +28,7 @@
 
 #ifdef CONFIG_SND_DEBUG
 static void snd_gf1_mem_info_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer);
+				  struct stringbuf *buffer);
 #endif
 
 void snd_gf1_mem_lock(struct snd_gf1_mem * alloc, int xup)
@@ -286,7 +286,7 @@ int snd_gf1_mem_done(struct snd_gus_card * gus)
 
 #ifdef CONFIG_SND_DEBUG
 static void snd_gf1_mem_info_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct snd_gus_card *gus;
 	struct snd_gf1_mem *alloc;
diff --git a/sound/isa/opti9xx/miro.c b/sound/isa/opti9xx/miro.c
index d295936..dd91018 100644
--- a/sound/isa/opti9xx/miro.c
+++ b/sound/isa/opti9xx/miro.c
@@ -842,7 +842,7 @@ static void snd_miro_write(struct snd_miro *chip, unsigned char reg,
  */
 
 static void snd_miro_proc_read(struct snd_info_entry * entry, 
-			       struct snd_info_buffer *buffer)
+			       struct stringbuf *buffer)
 {
 	struct snd_miro *miro = (struct snd_miro *) entry->private_data;
 	char* model = "unknown";
diff --git a/sound/isa/sb/sb16_csp.c b/sound/isa/sb/sb16_csp.c
index 3682059..5dfe4b9 100644
--- a/sound/isa/sb/sb16_csp.c
+++ b/sound/isa/sb/sb16_csp.c
@@ -110,7 +110,7 @@ static void snd_sb_qsound_destroy(struct snd_sb_csp * p);
 static int snd_sb_csp_qsound_transfer(struct snd_sb_csp * p);
 
 static int init_proc_entry(struct snd_sb_csp * p, int device);
-static void info_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer);
+static void info_read(struct snd_info_entry *entry, struct stringbuf *buffer);
 
 /*
  * Detect CSP chip and create a new instance
@@ -1149,7 +1149,7 @@ static int init_proc_entry(struct snd_sb_csp * p, int device)
 	return 0;
 }
 
-static void info_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void info_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_sb_csp *p = entry->private_data;
 
diff --git a/sound/pci/ac97/ac97_proc.c b/sound/pci/ac97/ac97_proc.c
index fed4a2c..5bd3cd5 100644
--- a/sound/pci/ac97/ac97_proc.c
+++ b/sound/pci/ac97/ac97_proc.c
@@ -36,7 +36,7 @@
  * proc interface
  */
 
-static void snd_ac97_proc_read_functions(struct snd_ac97 *ac97, struct snd_info_buffer *buffer)
+static void snd_ac97_proc_read_functions(struct snd_ac97 *ac97, struct stringbuf *buffer)
 {
 	int header = 0, function;
 	unsigned short info, sense_info;
@@ -106,7 +106,7 @@ static const char *snd_ac97_stereo_enhancements[] =
   /*  31 */ "Reserved 31"
 };
 
-static void snd_ac97_proc_read_main(struct snd_ac97 *ac97, struct snd_info_buffer *buffer, int subidx)
+static void snd_ac97_proc_read_main(struct snd_ac97 *ac97, struct stringbuf *buffer, int subidx)
 {
 	char name[64];
 	unsigned short val, tmp, ext, mext;
@@ -340,7 +340,7 @@ static void snd_ac97_proc_read_main(struct snd_ac97 *ac97, struct snd_info_buffe
 	}
 }
 
-static void snd_ac97_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void snd_ac97_proc_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_ac97 *ac97 = entry->private_data;
 	
@@ -375,7 +375,7 @@ static void snd_ac97_proc_read(struct snd_info_entry *entry, struct snd_info_buf
 
 #ifdef CONFIG_SND_DEBUG
 /* direct register write for debugging */
-static void snd_ac97_proc_regs_write(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void snd_ac97_proc_regs_write(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_ac97 *ac97 = entry->private_data;
 	char line[64];
@@ -392,7 +392,7 @@ static void snd_ac97_proc_regs_write(struct snd_info_entry *entry, struct snd_in
 }
 #endif
 
-static void snd_ac97_proc_regs_read_main(struct snd_ac97 *ac97, struct snd_info_buffer *buffer, int subidx)
+static void snd_ac97_proc_regs_read_main(struct snd_ac97 *ac97, struct stringbuf *buffer, int subidx)
 {
 	int reg, val;
 
@@ -403,7 +403,7 @@ static void snd_ac97_proc_regs_read_main(struct snd_ac97 *ac97, struct snd_info_
 }
 
 static void snd_ac97_proc_regs_read(struct snd_info_entry *entry, 
-				    struct snd_info_buffer *buffer)
+				    struct stringbuf *buffer)
 {
 	struct snd_ac97 *ac97 = entry->private_data;
 
diff --git a/sound/pci/ac97/ak4531_codec.c b/sound/pci/ac97/ak4531_codec.c
index 722de45..52275b0 100644
--- a/sound/pci/ac97/ak4531_codec.c
+++ b/sound/pci/ac97/ak4531_codec.c
@@ -466,7 +466,7 @@ void snd_ak4531_resume(struct snd_ak4531 *ak4531)
  */
 
 static void snd_ak4531_proc_read(struct snd_info_entry *entry, 
-				 struct snd_info_buffer *buffer)
+				 struct stringbuf *buffer)
 {
 	struct snd_ak4531 *ak4531 = entry->private_data;
 
diff --git a/sound/pci/ad1889.c b/sound/pci/ad1889.c
index 98970d4..8a01490 100644
--- a/sound/pci/ad1889.c
+++ b/sound/pci/ad1889.c
@@ -665,7 +665,7 @@ snd_ad1889_pcm_init(struct snd_ad1889 *chip, int device, struct snd_pcm **rpcm)
 }
 
 static void
-snd_ad1889_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+snd_ad1889_proc_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_ad1889 *chip = entry->private_data;
 	u16 reg;
diff --git a/sound/pci/ali5451/ali5451.c b/sound/pci/ali5451/ali5451.c
index 4c2bd7a..a986ea8 100644
--- a/sound/pci/ali5451/ali5451.c
+++ b/sound/pci/ali5451/ali5451.c
@@ -2126,7 +2126,7 @@ static int snd_ali_chip_init(struct snd_ali *codec)
 
 /* proc for register dump */
 static void snd_ali_proc_read(struct snd_info_entry *entry,
-			      struct snd_info_buffer *buf)
+			      struct stringbuf *buf)
 {
 	struct snd_ali *codec = entry->private_data;
 	int i;
diff --git a/sound/pci/atiixp.c b/sound/pci/atiixp.c
index 89184a4..737d31e 100644
--- a/sound/pci/atiixp.c
+++ b/sound/pci/atiixp.c
@@ -1524,7 +1524,7 @@ static int snd_atiixp_resume(struct pci_dev *pci)
  */
 
 static void snd_atiixp_proc_read(struct snd_info_entry *entry,
-				 struct snd_info_buffer *buffer)
+				 struct stringbuf *buffer)
 {
 	struct atiixp *chip = entry->private_data;
 	int i;
diff --git a/sound/pci/atiixp_modem.c b/sound/pci/atiixp_modem.c
index ce752f8..e273674 100644
--- a/sound/pci/atiixp_modem.c
+++ b/sound/pci/atiixp_modem.c
@@ -1168,7 +1168,7 @@ static int snd_atiixp_resume(struct pci_dev *pci)
  */
 
 static void snd_atiixp_proc_read(struct snd_info_entry *entry,
-				 struct snd_info_buffer *buffer)
+				 struct stringbuf *buffer)
 {
 	struct atiixp_modem *chip = entry->private_data;
 	int i;
diff --git a/sound/pci/ca0106/ca0106_proc.c b/sound/pci/ca0106/ca0106_proc.c
index ae80f51..2e8be6a 100644
--- a/sound/pci/ca0106/ca0106_proc.c
+++ b/sound/pci/ca0106/ca0106_proc.c
@@ -99,7 +99,7 @@ static struct snd_ca0106_category_str snd_ca0106_con_category[] = {
 };
 
 
-static void snd_ca0106_proc_dump_iec958( struct snd_info_buffer *buffer, u32 value)
+static void snd_ca0106_proc_dump_iec958(struct stringbuf *buffer, u32 value)
 {
 	int i;
 	u32 status[4];
@@ -274,7 +274,7 @@ static void snd_ca0106_proc_dump_iec958( struct snd_info_buffer *buffer, u32 val
 }
 
 static void snd_ca0106_proc_iec958(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct snd_ca0106 *emu = entry->private_data;
 	u32 value;
@@ -314,7 +314,7 @@ static void snd_ca0106_proc_reg_write32(struct snd_info_entry *entry,
 }
 
 static void snd_ca0106_proc_reg_read32(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct snd_ca0106 *emu = entry->private_data;
 	unsigned long value;
@@ -330,7 +330,7 @@ static void snd_ca0106_proc_reg_read32(struct snd_info_entry *entry,
 }
 
 static void snd_ca0106_proc_reg_read16(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct snd_ca0106 *emu = entry->private_data;
         unsigned int value;
@@ -346,7 +346,7 @@ static void snd_ca0106_proc_reg_read16(struct snd_info_entry *entry,
 }
 
 static void snd_ca0106_proc_reg_read8(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct snd_ca0106 *emu = entry->private_data;
 	unsigned int value;
@@ -362,7 +362,7 @@ static void snd_ca0106_proc_reg_read8(struct snd_info_entry *entry,
 }
 
 static void snd_ca0106_proc_reg_read1(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct snd_ca0106 *emu = entry->private_data;
 	unsigned long value;
@@ -380,7 +380,7 @@ static void snd_ca0106_proc_reg_read1(struct snd_info_entry *entry,
 }
 
 static void snd_ca0106_proc_reg_read2(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct snd_ca0106 *emu = entry->private_data;
 	unsigned long value;
@@ -448,10 +448,9 @@ int __devinit snd_ca0106_proc_init(struct snd_ca0106 * emu)
 //		entry->private_data = emu;
 	}
 	if(! snd_card_proc_new(emu->card, "ca0106_i2c", &entry)) {
-		snd_info_set_text_ops(entry, emu, snd_ca0106_proc_i2c_write);
+		entry->private_data = emu;
 		entry->c.text.write = snd_ca0106_proc_i2c_write;
 		entry->mode |= S_IWUSR;
-//		entry->private_data = emu;
 	}
 	if(! snd_card_proc_new(emu->card, "ca0106_regs2", &entry)) 
 		snd_info_set_text_ops(entry, emu, snd_ca0106_proc_reg_read2);
diff --git a/sound/pci/cmipci.c b/sound/pci/cmipci.c
index 6832649..4abef9e 100644
--- a/sound/pci/cmipci.c
+++ b/sound/pci/cmipci.c
@@ -2706,7 +2706,7 @@ static int __devinit snd_cmipci_mixer_new(struct cmipci *cm, int pcm_spdif_devic
 
 #ifdef CONFIG_PROC_FS
 static void snd_cmipci_proc_read(struct snd_info_entry *entry, 
-				 struct snd_info_buffer *buffer)
+				 struct stringbuf *buffer)
 {
 	struct cmipci *cm = entry->private_data;
 	int i, v;
diff --git a/sound/pci/cs4281.c b/sound/pci/cs4281.c
index 9a55f4a..a60649b 100644
--- a/sound/pci/cs4281.c
+++ b/sound/pci/cs4281.c
@@ -1127,7 +1127,7 @@ static int __devinit snd_cs4281_mixer(struct cs4281 * chip)
  */
 
 static void snd_cs4281_proc_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct cs4281 *chip = entry->private_data;
 
diff --git a/sound/pci/cs46xx/dsp_spos.c b/sound/pci/cs46xx/dsp_spos.c
index 590b35d..ea9367c 100644
--- a/sound/pci/cs46xx/dsp_spos.c
+++ b/sound/pci/cs46xx/dsp_spos.c
@@ -484,8 +484,8 @@ cs46xx_dsp_lookup_symbol_addr (struct snd_cs46xx * chip, u32 address, int symbol
 }
 
 
-static void cs46xx_dsp_proc_symbol_table_read (struct snd_info_entry *entry,
-					       struct snd_info_buffer *buffer)
+static void cs46xx_dsp_proc_symbol_table_read(struct snd_info_entry *entry,
+					      struct stringbuf *buffer)
 {
 	struct snd_cs46xx *chip = entry->private_data;
 	struct dsp_spos_instance * ins = chip->dsp_spos_instance;
@@ -513,7 +513,7 @@ static void cs46xx_dsp_proc_symbol_table_read (struct snd_info_entry *entry,
 
 
 static void cs46xx_dsp_proc_modules_read (struct snd_info_entry *entry,
-					  struct snd_info_buffer *buffer)
+					  struct stringbuf *buffer)
 {
 	struct snd_cs46xx *chip = entry->private_data;
 	struct dsp_spos_instance * ins = chip->dsp_spos_instance;
@@ -536,7 +536,7 @@ static void cs46xx_dsp_proc_modules_read (struct snd_info_entry *entry,
 }
 
 static void cs46xx_dsp_proc_task_tree_read (struct snd_info_entry *entry,
-					    struct snd_info_buffer *buffer)
+					    struct stringbuf *buffer)
 {
 	struct snd_cs46xx *chip = entry->private_data;
 	struct dsp_spos_instance * ins = chip->dsp_spos_instance;
@@ -564,7 +564,7 @@ static void cs46xx_dsp_proc_task_tree_read (struct snd_info_entry *entry,
 }
 
 static void cs46xx_dsp_proc_scb_read (struct snd_info_entry *entry,
-				      struct snd_info_buffer *buffer)
+				      struct stringbuf *buffer)
 {
 	struct snd_cs46xx *chip = entry->private_data;
 	struct dsp_spos_instance * ins = chip->dsp_spos_instance;
@@ -597,7 +597,7 @@ static void cs46xx_dsp_proc_scb_read (struct snd_info_entry *entry,
 }
 
 static void cs46xx_dsp_proc_parameter_dump_read (struct snd_info_entry *entry,
-						 struct snd_info_buffer *buffer)
+						 struct stringbuf *buffer)
 {
 	struct snd_cs46xx *chip = entry->private_data;
 	/*struct dsp_spos_instance * ins = chip->dsp_spos_instance; */
@@ -625,7 +625,7 @@ static void cs46xx_dsp_proc_parameter_dump_read (struct snd_info_entry *entry,
 }
 
 static void cs46xx_dsp_proc_sample_dump_read (struct snd_info_entry *entry,
-					      struct snd_info_buffer *buffer)
+					      struct stringbuf *buffer)
 {
 	struct snd_cs46xx *chip = entry->private_data;
 	int i,col = 0;
diff --git a/sound/pci/cs46xx/dsp_spos_scb_lib.c b/sound/pci/cs46xx/dsp_spos_scb_lib.c
index eded4df..8efe52c 100644
--- a/sound/pci/cs46xx/dsp_spos_scb_lib.c
+++ b/sound/pci/cs46xx/dsp_spos_scb_lib.c
@@ -66,8 +66,8 @@ static void remove_symbol (struct snd_cs46xx * chip, struct dsp_symbol_entry * s
 }
 
 #ifdef CONFIG_PROC_FS
-static void cs46xx_dsp_proc_scb_info_read (struct snd_info_entry *entry,
-					   struct snd_info_buffer *buffer)
+static void cs46xx_dsp_proc_scb_info_read(struct snd_info_entry *entry,
+					  struct stringbuf *buffer)
 {
 	struct proc_scb_info * scb_info  = entry->private_data;
 	struct dsp_scb_descriptor * scb = scb_info->scb_desc;
diff --git a/sound/pci/emu10k1/emu10k1x.c b/sound/pci/emu10k1/emu10k1x.c
index 1ec7eba..faf0c7a 100644
--- a/sound/pci/emu10k1/emu10k1x.c
+++ b/sound/pci/emu10k1/emu10k1x.c
@@ -1004,7 +1004,7 @@ static int __devinit snd_emu10k1x_create(struct snd_card *card,
 }
 
 static void snd_emu10k1x_proc_reg_read(struct snd_info_entry *entry, 
-				       struct snd_info_buffer *buffer)
+				       struct stringbuf *buffer)
 {
 	struct emu10k1x *emu = entry->private_data;
 	unsigned long value,value1,value2;
diff --git a/sound/pci/emu10k1/emuproc.c b/sound/pci/emu10k1/emuproc.c
index c3fb10e..53b7b11 100644
--- a/sound/pci/emu10k1/emuproc.c
+++ b/sound/pci/emu10k1/emuproc.c
@@ -37,7 +37,7 @@
 
 #ifdef CONFIG_PROC_FS
 static void snd_emu10k1_proc_spdif_status(struct snd_emu10k1 * emu,
-					  struct snd_info_buffer *buffer,
+					  struct stringbuf *buffer,
 					  char *title,
 					  int status_reg,
 					  int rate_reg)
@@ -80,7 +80,7 @@ static void snd_emu10k1_proc_spdif_status(struct snd_emu10k1 * emu,
 }
 
 static void snd_emu10k1_proc_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	/* FIXME - output names are in emufx.c too */
 	static char *creative_outs[32] = {
@@ -237,7 +237,7 @@ static void snd_emu10k1_proc_read(struct snd_info_entry *entry,
 }
 
 static void snd_emu10k1_proc_spdif_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct snd_emu10k1 *emu = entry->private_data;
 	u32 value;
@@ -285,7 +285,7 @@ static void snd_emu10k1_proc_spdif_read(struct snd_info_entry *entry,
 }
 
 static void snd_emu10k1_proc_rates_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	static int samplerate[8] = { 44100, 48000, 96000, 192000, 4, 5, 6, 7 };
 	struct snd_emu10k1 *emu = entry->private_data;
@@ -300,7 +300,7 @@ static void snd_emu10k1_proc_rates_read(struct snd_info_entry *entry,
 }
 
 static void snd_emu10k1_proc_acode_read(struct snd_info_entry *entry, 
-				        struct snd_info_buffer *buffer)
+				        struct stringbuf *buffer)
 {
 	u32 pc;
 	struct snd_emu10k1 *emu = entry->private_data;
@@ -389,7 +389,7 @@ static long snd_emu10k1_fx8010_read(struct snd_info_entry *entry,
 }
 
 static void snd_emu10k1_proc_voices_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct snd_emu10k1 *emu = entry->private_data;
 	struct snd_emu10k1_voice *voice;
@@ -410,7 +410,7 @@ static void snd_emu10k1_proc_voices_read(struct snd_info_entry *entry,
 
 #ifdef CONFIG_SND_DEBUG
 static void snd_emu_proc_emu1010_reg_read(struct snd_info_entry *entry,
-				     struct snd_info_buffer *buffer)
+				     struct stringbuf *buffer)
 {
 	struct snd_emu10k1 *emu = entry->private_data;
 	int value;
@@ -427,7 +427,7 @@ static void snd_emu_proc_emu1010_reg_read(struct snd_info_entry *entry,
 }
 
 static void snd_emu_proc_io_reg_read(struct snd_info_entry *entry,
-				     struct snd_info_buffer *buffer)
+				     struct stringbuf *buffer)
 {
 	struct snd_emu10k1 *emu = entry->private_data;
 	unsigned long value;
@@ -496,7 +496,7 @@ static void snd_ptr_write(struct snd_emu10k1 *emu,
 
 
 static void snd_emu_proc_ptr_reg_read(struct snd_info_entry *entry,
-				      struct snd_info_buffer *buffer, int iobase, int offset, int length, int voices)
+				      struct stringbuf *buffer, int iobase, int offset, int length, int voices)
 {
 	struct snd_emu10k1 *emu = entry->private_data;
 	unsigned long value;
@@ -547,31 +547,31 @@ static void snd_emu_proc_ptr_reg_write20(struct snd_info_entry *entry,
 	
 
 static void snd_emu_proc_ptr_reg_read00a(struct snd_info_entry *entry,
-					 struct snd_info_buffer *buffer)
+					 struct stringbuf *buffer)
 {
 	snd_emu_proc_ptr_reg_read(entry, buffer, 0, 0, 0x40, 64);
 }
 
 static void snd_emu_proc_ptr_reg_read00b(struct snd_info_entry *entry,
-					 struct snd_info_buffer *buffer)
+					 struct stringbuf *buffer)
 {
 	snd_emu_proc_ptr_reg_read(entry, buffer, 0, 0x40, 0x40, 64);
 }
 
 static void snd_emu_proc_ptr_reg_read20a(struct snd_info_entry *entry,
-					 struct snd_info_buffer *buffer)
+					 struct stringbuf *buffer)
 {
 	snd_emu_proc_ptr_reg_read(entry, buffer, 0x20, 0, 0x40, 4);
 }
 
 static void snd_emu_proc_ptr_reg_read20b(struct snd_info_entry *entry,
-					 struct snd_info_buffer *buffer)
+					 struct stringbuf *buffer)
 {
 	snd_emu_proc_ptr_reg_read(entry, buffer, 0x20, 0x40, 0x40, 4);
 }
 
 static void snd_emu_proc_ptr_reg_read20c(struct snd_info_entry *entry,
-					 struct snd_info_buffer * buffer)
+					 struct stringbuf * buffer)
 {
 	snd_emu_proc_ptr_reg_read(entry, buffer, 0x20, 0x80, 0x20, 4);
 }
diff --git a/sound/pci/ens1370.c b/sound/pci/ens1370.c
index b958f86..41b1349 100644
--- a/sound/pci/ens1370.c
+++ b/sound/pci/ens1370.c
@@ -1865,7 +1865,7 @@ static inline void snd_ensoniq_free_gameport(struct ensoniq *ensoniq) { }
  */
 
 static void snd_ensoniq_proc_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct ensoniq *ensoniq = entry->private_data;
 
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c
index e94944f..55532c3 100644
--- a/sound/pci/hda/hda_proc.c
+++ b/sound/pci/hda/hda_proc.c
@@ -47,7 +47,7 @@ static const char *get_wid_type_name(unsigned int wid_value)
 		return "UNKNOWN Widget";
 }
 
-static void print_amp_caps(struct snd_info_buffer *buffer,
+static void print_amp_caps(struct stringbuf *buffer,
 			   struct hda_codec *codec, hda_nid_t nid, int dir)
 {
 	unsigned int caps;
@@ -66,7 +66,7 @@ static void print_amp_caps(struct snd_info_buffer *buffer,
 		    (caps & AC_AMPCAP_MUTE) >> AC_AMPCAP_MUTE_SHIFT);
 }
 
-static void print_amp_vals(struct snd_info_buffer *buffer,
+static void print_amp_vals(struct stringbuf *buffer,
 			   struct hda_codec *codec, hda_nid_t nid,
 			   int dir, int stereo, int indices)
 {
@@ -90,7 +90,7 @@ static void print_amp_vals(struct snd_info_buffer *buffer,
 	snd_iprintf(buffer, "\n");
 }
 
-static void print_pcm_rates(struct snd_info_buffer *buffer, unsigned int pcm)
+static void print_pcm_rates(struct stringbuf *buffer, unsigned int pcm)
 {
 	static unsigned int rates[] = {
 		8000, 11025, 16000, 22050, 32000, 44100, 48000, 88200,
@@ -106,7 +106,7 @@ static void print_pcm_rates(struct snd_info_buffer *buffer, unsigned int pcm)
 	snd_iprintf(buffer, "\n");
 }
 
-static void print_pcm_bits(struct snd_info_buffer *buffer, unsigned int pcm)
+static void print_pcm_bits(struct stringbuf *buffer, unsigned int pcm)
 {
 	static unsigned int bits[] = { 8, 16, 20, 24, 32 };
 	int i;
@@ -119,7 +119,7 @@ static void print_pcm_bits(struct snd_info_buffer *buffer, unsigned int pcm)
 	snd_iprintf(buffer, "\n");
 }
 
-static void print_pcm_formats(struct snd_info_buffer *buffer,
+static void print_pcm_formats(struct stringbuf *buffer,
 			      unsigned int streams)
 {
 	snd_iprintf(buffer, "    formats [0x%x]:", streams & 0xf);
@@ -132,7 +132,7 @@ static void print_pcm_formats(struct snd_info_buffer *buffer,
 	snd_iprintf(buffer, "\n");
 }
 
-static void print_pcm_caps(struct snd_info_buffer *buffer,
+static void print_pcm_caps(struct stringbuf *buffer,
 			   struct hda_codec *codec, hda_nid_t nid)
 {
 	unsigned int pcm = snd_hda_param_read(codec, nid, AC_PAR_PCM);
@@ -202,7 +202,7 @@ static const char *get_jack_color(u32 cfg)
 		return "UNKNOWN";
 }
 
-static void print_pin_caps(struct snd_info_buffer *buffer,
+static void print_pin_caps(struct stringbuf *buffer,
 			   struct hda_codec *codec, hda_nid_t nid)
 {
 	static char *jack_conns[4] = { "Jack", "N/A", "Fixed", "Both" };
@@ -241,7 +241,7 @@ static void print_pin_caps(struct snd_info_buffer *buffer,
 
 
 static void print_codec_info(struct snd_info_entry *entry,
-			     struct snd_info_buffer *buffer)
+			     struct stringbuf *buffer)
 {
 	struct hda_codec *codec = entry->private_data;
 	char buf[32];
diff --git a/sound/pci/ice1712/ice1712.c b/sound/pci/ice1712/ice1712.c
index 052fc3c..24752c2 100644
--- a/sound/pci/ice1712/ice1712.c
+++ b/sound/pci/ice1712/ice1712.c
@@ -1543,7 +1543,7 @@ static inline unsigned int eeprom_double(struct snd_ice1712 *ice, int idx)
 }
 
 static void snd_ice1712_proc_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct snd_ice1712 *ice = entry->private_data;
 	unsigned int idx;
diff --git a/sound/pci/ice1712/ice1724.c b/sound/pci/ice1712/ice1724.c
index 0b0bbb0..6350e2d 100644
--- a/sound/pci/ice1712/ice1724.c
+++ b/sound/pci/ice1712/ice1724.c
@@ -1248,7 +1248,7 @@ static inline unsigned int eeprom_triple(struct snd_ice1712 *ice, int idx)
 }
 
 static void snd_vt1724_proc_read(struct snd_info_entry *entry, 
-				 struct snd_info_buffer *buffer)
+				 struct stringbuf *buffer)
 {
 	struct snd_ice1712 *ice = entry->private_data;
 	unsigned int idx;
diff --git a/sound/pci/ice1712/pontis.c b/sound/pci/ice1712/pontis.c
index faefd52..e4b857c 100644
--- a/sound/pci/ice1712/pontis.c
+++ b/sound/pci/ice1712/pontis.c
@@ -651,7 +651,7 @@ static void wm_proc_regs_write(struct snd_info_entry *entry, struct snd_info_buf
 	mutex_unlock(&ice->gpio_mutex);
 }
 
-static void wm_proc_regs_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void wm_proc_regs_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_ice1712 *ice = (struct snd_ice1712 *)entry->private_data;
 	int reg, val;
@@ -674,7 +674,7 @@ static void wm_proc_init(struct snd_ice1712 *ice)
 	}
 }
 
-static void cs_proc_regs_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void cs_proc_regs_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_ice1712 *ice = (struct snd_ice1712 *)entry->private_data;
 	int reg, val;
diff --git a/sound/pci/intel8x0.c b/sound/pci/intel8x0.c
index b4a38a3..0114f98 100644
--- a/sound/pci/intel8x0.c
+++ b/sound/pci/intel8x0.c
@@ -2655,7 +2655,7 @@ static void __devinit intel8x0_measure_ac97_clock(struct intel8x0 *chip)
 
 #ifdef CONFIG_PROC_FS
 static void snd_intel8x0_proc_read(struct snd_info_entry * entry,
-				   struct snd_info_buffer *buffer)
+				   struct stringbuf *buffer)
 {
 	struct intel8x0 *chip = entry->private_data;
 	unsigned int tmp;
diff --git a/sound/pci/intel8x0m.c b/sound/pci/intel8x0m.c
index fad806e..9f6bdd1 100644
--- a/sound/pci/intel8x0m.c
+++ b/sound/pci/intel8x0m.c
@@ -1060,7 +1060,7 @@ static int intel8x0m_resume(struct pci_dev *pci)
 
 #ifdef CONFIG_PROC_FS
 static void snd_intel8x0m_proc_read(struct snd_info_entry * entry,
-				   struct snd_info_buffer *buffer)
+				   struct stringbuf *buffer)
 {
 	struct intel8x0m *chip = entry->private_data;
 	unsigned int tmp;
diff --git a/sound/pci/korg1212/korg1212.c b/sound/pci/korg1212/korg1212.c
index c4af57f..983b04b 100644
--- a/sound/pci/korg1212/korg1212.c
+++ b/sound/pci/korg1212/korg1212.c
@@ -2051,7 +2051,7 @@ static struct snd_kcontrol_new snd_korg1212_controls[] = {
  */
 
 static void snd_korg1212_proc_read(struct snd_info_entry *entry,
-				   struct snd_info_buffer *buffer)
+				   struct stringbuf *buffer)
 {
 	int n;
 	struct snd_korg1212 *korg1212 = entry->private_data;
diff --git a/sound/pci/mixart/mixart.c b/sound/pci/mixart/mixart.c
index 880b824..194b765 100644
--- a/sound/pci/mixart/mixart.c
+++ b/sound/pci/mixart/mixart.c
@@ -1202,7 +1202,7 @@ static struct snd_info_entry_ops snd_mixart_proc_ops_BA1 = {
 
 
 static void snd_mixart_proc_read(struct snd_info_entry *entry, 
-                                 struct snd_info_buffer *buffer)
+                                 struct stringbuf *buffer)
 {
 	struct snd_mixart *chip = entry->private_data;        
 	u32 ref; 
diff --git a/sound/pci/pcxhr/pcxhr.c b/sound/pci/pcxhr/pcxhr.c
index 2d618bd..7677969 100644
--- a/sound/pci/pcxhr/pcxhr.c
+++ b/sound/pci/pcxhr/pcxhr.c
@@ -1059,7 +1059,7 @@ static int __devinit pcxhr_create(struct pcxhr_mgr *mgr, struct snd_card *card,
 }
 
 /* proc interface */
-static void pcxhr_proc_info(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void pcxhr_proc_info(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_pcxhr *chip = entry->private_data;
 	struct pcxhr_mgr *mgr = chip->mgr;
@@ -1120,7 +1120,7 @@ static void pcxhr_proc_info(struct snd_info_entry *entry, struct snd_info_buffer
 		snd_iprintf(buffer, "no firmware loaded\n");
 	snd_iprintf(buffer, "\n");
 }
-static void pcxhr_proc_sync(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void pcxhr_proc_sync(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_pcxhr *chip = entry->private_data;
 	struct pcxhr_mgr *mgr = chip->mgr;
diff --git a/sound/pci/riptide/riptide.c b/sound/pci/riptide/riptide.c
index 8e54104..0074e97 100644
--- a/sound/pci/riptide/riptide.c
+++ b/sound/pci/riptide/riptide.c
@@ -1928,7 +1928,7 @@ snd_riptide_create(struct snd_card *card, struct pci_dev *pci,
 
 static void
 snd_riptide_proc_read(struct snd_info_entry *entry,
-		      struct snd_info_buffer *buffer)
+		      struct stringbuf *buffer)
 {
 	struct snd_riptide *chip = entry->private_data;
 	struct pcmhw *data;
diff --git a/sound/pci/rme32.c b/sound/pci/rme32.c
index 1475912..fef422a 100644
--- a/sound/pci/rme32.c
+++ b/sound/pci/rme32.c
@@ -1463,7 +1463,7 @@ static int __devinit snd_rme32_create(struct rme32 * rme32)
  */
 
 static void
-snd_rme32_proc_read(struct snd_info_entry * entry, struct snd_info_buffer *buffer)
+snd_rme32_proc_read(struct snd_info_entry * entry, struct stringbuf *buffer)
 {
 	int n;
 	struct rme32 *rme32 = (struct rme32 *) entry->private_data;
diff --git a/sound/pci/rme96.c b/sound/pci/rme96.c
index 0b3c532..3ce41db 100644
--- a/sound/pci/rme96.c
+++ b/sound/pci/rme96.c
@@ -1663,7 +1663,7 @@ snd_rme96_create(struct rme96 *rme96)
  */
 
 static void 
-snd_rme96_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+snd_rme96_proc_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	int n;
 	struct rme96 *rme96 = (struct rme96 *)entry->private_data;
diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index ff26a36..c6de4b7 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -3242,7 +3242,7 @@ static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
  ------------------------------------------------------------*/
 
 static void
-snd_hdsp_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+snd_hdsp_proc_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct hdsp *hdsp = (struct hdsp *) entry->private_data;
 	unsigned int status;
diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
index f1bdda6..93310d9 100644
--- a/sound/pci/rme9652/hdspm.c
+++ b/sound/pci/rme9652/hdspm.c
@@ -2938,7 +2938,7 @@ static int snd_hdspm_create_controls(struct snd_card *card, struct hdspm * hdspm
 
 static void
 snd_hdspm_proc_read_madi(struct snd_info_entry * entry,
-			 struct snd_info_buffer *buffer)
+			 struct stringbuf *buffer)
 {
 	struct hdspm *hdspm = entry->private_data;
 	unsigned int status;
@@ -3133,7 +3133,7 @@ snd_hdspm_proc_read_madi(struct snd_info_entry * entry,
 
 static void
 snd_hdspm_proc_read_aes32(struct snd_info_entry * entry,
-			  struct snd_info_buffer *buffer)
+			  struct stringbuf *buffer)
 {
 	struct hdspm *hdspm = entry->private_data;
 	unsigned int status;
@@ -3305,7 +3305,7 @@ snd_hdspm_proc_read_aes32(struct snd_info_entry * entry,
 #ifdef CONFIG_SND_DEBUG
 static void
 snd_hdspm_proc_read_debug(struct snd_info_entry * entry,
-			  struct snd_info_buffer *buffer)
+			  struct stringbuf *buffer)
 {
 	struct hdspm *hdspm = entry->private_data;
 
diff --git a/sound/pci/rme9652/rme9652.c b/sound/pci/rme9652/rme9652.c
index 34f96f1..2a0b7da 100644
--- a/sound/pci/rme9652/rme9652.c
+++ b/sound/pci/rme9652/rme9652.c
@@ -1589,7 +1589,7 @@ static int snd_rme9652_create_controls(struct snd_card *card, struct snd_rme9652
  ------------------------------------------------------------*/
 
 static void
-snd_rme9652_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+snd_rme9652_proc_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_rme9652 *rme9652 = (struct snd_rme9652 *) entry->private_data;
 	u32 thru_bits = rme9652->thru_bits;
diff --git a/sound/pci/sonicvibes.c b/sound/pci/sonicvibes.c
index 44a7f5f..09b09b7 100644
--- a/sound/pci/sonicvibes.c
+++ b/sound/pci/sonicvibes.c
@@ -1110,7 +1110,7 @@ static int __devinit snd_sonicvibes_mixer(struct sonicvibes * sonic)
  */
 
 static void snd_sonicvibes_proc_read(struct snd_info_entry *entry, 
-				     struct snd_info_buffer *buffer)
+				     struct stringbuf *buffer)
 {
 	struct sonicvibes *sonic = entry->private_data;
 	unsigned char tmp;
diff --git a/sound/pci/trident/trident_main.c b/sound/pci/trident/trident_main.c
index a235e03..7013581 100644
--- a/sound/pci/trident/trident_main.c
+++ b/sound/pci/trident/trident_main.c
@@ -3281,7 +3281,7 @@ static int snd_trident_sis_reset(struct snd_trident *trident)
  */
 
 static void snd_trident_proc_read(struct snd_info_entry *entry, 
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct snd_trident *trident = entry->private_data;
 	char *s;
diff --git a/sound/pci/via82xx.c b/sound/pci/via82xx.c
index cf62d2a..2b96eca 100644
--- a/sound/pci/via82xx.c
+++ b/sound/pci/via82xx.c
@@ -2017,7 +2017,7 @@ static int __devinit snd_via686_init_misc(struct via82xx *chip)
  * proc interface
  */
 static void snd_via82xx_proc_read(struct snd_info_entry *entry,
-				  struct snd_info_buffer *buffer)
+				  struct stringbuf *buffer)
 {
 	struct via82xx *chip = entry->private_data;
 	int i;
diff --git a/sound/pci/via82xx_modem.c b/sound/pci/via82xx_modem.c
index 57fb9ae..ca2c73b 100644
--- a/sound/pci/via82xx_modem.c
+++ b/sound/pci/via82xx_modem.c
@@ -913,7 +913,7 @@ static int __devinit snd_via82xx_mixer_new(struct via82xx_modem *chip)
 /*
  * proc interface
  */
-static void snd_via82xx_proc_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void snd_via82xx_proc_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct via82xx_modem *chip = entry->private_data;
 	int i;
diff --git a/sound/pci/ymfpci/ymfpci_main.c b/sound/pci/ymfpci/ymfpci_main.c
index 1fe39ed..731d3a1 100644
--- a/sound/pci/ymfpci/ymfpci_main.c
+++ b/sound/pci/ymfpci/ymfpci_main.c
@@ -1929,7 +1929,7 @@ int __devinit snd_ymfpci_timer(struct snd_ymfpci *chip, int device)
  */
 
 static void snd_ymfpci_proc_read(struct snd_info_entry *entry, 
-				 struct snd_info_buffer *buffer)
+				 struct stringbuf *buffer)
 {
 	struct snd_ymfpci *chip = entry->private_data;
 	int i;
diff --git a/sound/pcmcia/pdaudiocf/pdaudiocf_core.c b/sound/pcmcia/pdaudiocf/pdaudiocf_core.c
index 484c8f9..e533beb 100644
--- a/sound/pcmcia/pdaudiocf/pdaudiocf_core.c
+++ b/sound/pcmcia/pdaudiocf/pdaudiocf_core.c
@@ -128,7 +128,7 @@ void pdacf_reinit(struct snd_pdacf *chip, int resume)
 }
 
 static void pdacf_proc_read(struct snd_info_entry * entry,
-                            struct snd_info_buffer *buffer)
+                            struct stringbuf *buffer)
 {
 	struct snd_pdacf *chip = entry->private_data;
 	u16 tmp;
diff --git a/sound/sparc/dbri.c b/sound/sparc/dbri.c
index 376b986..339f22c 100644
--- a/sound/sparc/dbri.c
+++ b/sound/sparc/dbri.c
@@ -2446,7 +2446,7 @@ static int __devinit snd_dbri_mixer(struct snd_card *card)
 			/proc interface
 ****************************************************************************/
 static void dbri_regs_read(struct snd_info_entry *entry,
-			   struct snd_info_buffer *buffer)
+			   struct stringbuf *buffer)
 {
 	struct snd_dbri *dbri = entry->private_data;
 
@@ -2458,7 +2458,7 @@ static void dbri_regs_read(struct snd_info_entry *entry,
 
 #ifdef DBRI_DEBUG
 static void dbri_debug_read(struct snd_info_entry *entry,
-			    struct snd_info_buffer *buffer)
+			    struct stringbuf *buffer)
 {
 	struct snd_dbri *dbri = entry->private_data;
 	int pipe;
diff --git a/sound/synth/emux/emux_proc.c b/sound/synth/emux/emux_proc.c
index 680f2b7..06e0783 100644
--- a/sound/synth/emux/emux_proc.c
+++ b/sound/synth/emux/emux_proc.c
@@ -30,7 +30,7 @@
 
 static void
 snd_emux_proc_info_read(struct snd_info_entry *entry, 
-			struct snd_info_buffer *buf)
+			struct stringbuf *buf)
 {
 	struct snd_emux *emu;
 	int i;
diff --git a/sound/usb/usbaudio.c b/sound/usb/usbaudio.c
index 967b823..bb1e0ac 100644
--- a/sound/usb/usbaudio.c
+++ b/sound/usb/usbaudio.c
@@ -2101,7 +2101,7 @@ static struct usb_driver usb_audio_driver = {
 /*
  * proc interface for list the supported pcm formats
  */
-static void proc_dump_substream_formats(struct snd_usb_substream *subs, struct snd_info_buffer *buffer)
+static void proc_dump_substream_formats(struct snd_usb_substream *subs, struct stringbuf *buffer)
 {
 	struct list_head *p;
 	static char *sync_types[4] = {
@@ -2137,7 +2137,7 @@ static void proc_dump_substream_formats(struct snd_usb_substream *subs, struct s
 	}
 }
 
-static void proc_dump_substream_status(struct snd_usb_substream *subs, struct snd_info_buffer *buffer)
+static void proc_dump_substream_status(struct snd_usb_substream *subs, struct stringbuf *buffer)
 {
 	if (subs->running) {
 		unsigned int i;
@@ -2159,7 +2159,7 @@ static void proc_dump_substream_status(struct snd_usb_substream *subs, struct sn
 	}
 }
 
-static void proc_pcm_format_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void proc_pcm_format_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_usb_stream *stream = entry->private_data;
 
@@ -3327,14 +3327,14 @@ static int snd_usb_create_quirk(struct snd_usb_audio *chip,
 /*
  * common proc files to show the usb device info
  */
-static void proc_audio_usbbus_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void proc_audio_usbbus_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_usb_audio *chip = entry->private_data;
 	if (! chip->shutdown)
 		snd_iprintf(buffer, "%03d/%03d\n", chip->dev->bus->busnum, chip->dev->devnum);
 }
 
-static void proc_audio_usbid_read(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
+static void proc_audio_usbid_read(struct snd_info_entry *entry, struct stringbuf *buffer)
 {
 	struct snd_usb_audio *chip = entry->private_data;
 	if (! chip->shutdown)
diff --git a/sound/usb/usbmixer.c b/sound/usb/usbmixer.c
index 5e32969..05a152f 100644
--- a/sound/usb/usbmixer.c
+++ b/sound/usb/usbmixer.c
@@ -1961,7 +1961,7 @@ static int snd_audigy2nx_controls_create(struct usb_mixer_interface *mixer)
 }
 
 static void snd_audigy2nx_proc_read(struct snd_info_entry *entry,
-				    struct snd_info_buffer *buffer)
+				    struct stringbuf *buffer)
 {
 	static const struct {
 		int unitid;
-- 
1.5.3.4


^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH 4/4] partitions: Fix non-atomic printk
  2007-10-23 21:12   ` [PATCH 3/4] sound: " Matthew Wilcox
@ 2007-10-23 21:12     ` Matthew Wilcox
  2007-10-24 14:18     ` [PATCH 3/4] sound: Use stringbuf Takashi Iwai
  1 sibling, 0 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-23 21:12 UTC (permalink / raw)
  To: torvalds, akpm, linux-kernel; +Cc: Matthew Wilcox, Matthew Wilcox

With asynchronous disk scanning, it's quite common for the partition
report to be interrupted by reports of other discs being discovered.
Use a new function, print_partition, to accumulate the partitions into
a stringbuf.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 fs/partitions/acorn.c  |   24 ++++++++++--------------
 fs/partitions/amiga.c  |    9 ++++-----
 fs/partitions/atari.c  |   20 +++++++++-----------
 fs/partitions/check.c  |   19 ++++++++++++++-----
 fs/partitions/check.h  |    6 +++++-
 fs/partitions/efi.c    |    1 -
 fs/partitions/ibm.c    |   11 +++++------
 fs/partitions/karma.c  |    1 -
 fs/partitions/ldm.c    |   11 +++++------
 fs/partitions/mac.c    |    3 +--
 fs/partitions/msdos.c  |   36 +++++++++++++++++-------------------
 fs/partitions/osf.c    |    1 -
 fs/partitions/sgi.c    |    1 -
 fs/partitions/sun.c    |    1 -
 fs/partitions/sysv68.c |    5 ++---
 fs/partitions/ultrix.c |    1 -
 16 files changed, 72 insertions(+), 78 deletions(-)

diff --git a/fs/partitions/acorn.c b/fs/partitions/acorn.c
index 3d3e166..7ece065 100644
--- a/fs/partitions/acorn.c
+++ b/fs/partitions/acorn.c
@@ -46,7 +46,7 @@ adfs_partition(struct parsed_partitions *state, char *name, char *data,
 		   (le32_to_cpu(dr->disc_size) >> 9);
 
 	if (name)
-		printk(" [%s]", name);
+		print_partition(state, " [%s]", name);
 	put_partition(state, slot, first_sector, nr_sects);
 	return dr;
 }
@@ -81,14 +81,14 @@ riscix_partition(struct parsed_partitions *state, struct block_device *bdev,
 	if (!rr)
 		return -1;
 
-	printk(" [RISCiX]");
+	print_partition(state, " [RISCiX]");
 
 
 	if (rr->magic == RISCIX_MAGIC) {
 		unsigned long size = nr_sects > 2 ? 2 : nr_sects;
 		int part;
 
-		printk(" <");
+		print_partition(state, " <");
 
 		put_partition(state, slot++, first_sect, size);
 		for (part = 0; part < 8; part++) {
@@ -97,11 +97,11 @@ riscix_partition(struct parsed_partitions *state, struct block_device *bdev,
 				put_partition(state, slot++,
 					le32_to_cpu(rr->part[part].start),
 					le32_to_cpu(rr->part[part].length));
-				printk("(%s)", rr->part[part].name);
+				print_partition(state, "(%s)", rr->part[part].name);
 			}
 		}
 
-		printk(" >\n");
+		print_partition(state, " >");
 	} else {
 		put_partition(state, slot++, first_sect, nr_sects);
 	}
@@ -131,7 +131,7 @@ linux_partition(struct parsed_partitions *state, struct block_device *bdev,
 	struct linux_part *linuxp;
 	unsigned long size = nr_sects > 2 ? 2 : nr_sects;
 
-	printk(" [Linux]");
+	print_partition(state, " [Linux]");
 
 	put_partition(state, slot++, first_sect, size);
 
@@ -139,7 +139,7 @@ linux_partition(struct parsed_partitions *state, struct block_device *bdev,
 	if (!linuxp)
 		return -1;
 
-	printk(" <");
+	print_partition(state, " <");
 	while (linuxp->magic == cpu_to_le32(LINUX_NATIVE_MAGIC) ||
 	       linuxp->magic == cpu_to_le32(LINUX_SWAP_MAGIC)) {
 		if (slot == state->limit)
@@ -149,7 +149,7 @@ linux_partition(struct parsed_partitions *state, struct block_device *bdev,
 				 le32_to_cpu(linuxp->nr_sects));
 		linuxp ++;
 	}
-	printk(" >");
+	print_partition(state, " >");
 
 	put_dev_sector(sect);
 	return slot;
@@ -306,7 +306,6 @@ adfspart_check_ADFS(struct parsed_partitions *state, struct block_device *bdev)
 			break;
 		}
 	}
-	printk("\n");
 	return 1;
 }
 #endif
@@ -379,7 +378,7 @@ adfspart_check_ICS(struct parsed_partitions *state, struct block_device *bdev)
 		return 0;
 	}
 
-	printk(" [ICS]");
+	print_partition(state, " [ICS]");
 
 	for (slot = 1, p = (const struct ics_part *)data; p->size; p++) {
 		u32 start = le32_to_cpu(p->start);
@@ -413,7 +412,6 @@ adfspart_check_ICS(struct parsed_partitions *state, struct block_device *bdev)
 	}
 
 	put_dev_sector(sect);
-	printk("\n");
 	return 1;
 }
 #endif
@@ -474,7 +472,7 @@ adfspart_check_POWERTEC(struct parsed_partitions *state, struct block_device *bd
 		return 0;
 	}
 
-	printk(" [POWERTEC]");
+	print_partition(state, " [POWERTEC]");
 
 	for (i = 0, p = (const struct ptec_part *)data; i < 12; i++, p++) {
 		u32 start = le32_to_cpu(p->start);
@@ -485,7 +483,6 @@ adfspart_check_POWERTEC(struct parsed_partitions *state, struct block_device *bd
 	}
 
 	put_dev_sector(sect);
-	printk("\n");
 	return 1;
 }
 #endif
@@ -557,7 +554,6 @@ adfspart_check_EESOX(struct parsed_partitions *state, struct block_device *bdev)
 
 		size = get_capacity(bdev->bd_disk);
 		put_partition(state, slot++, start, size - start);
-		printk("\n");
 	}
 
 	return i ? 1 : 0;
diff --git a/fs/partitions/amiga.c b/fs/partitions/amiga.c
index 9917a8c..9853dee 100644
--- a/fs/partitions/amiga.c
+++ b/fs/partitions/amiga.c
@@ -70,7 +70,7 @@ amiga_partition(struct parsed_partitions *state, struct block_device *bdev)
 	/* blksize is blocks per 512 byte standard block */
 	blksize = be32_to_cpu( rdb->rdb_BlockBytes ) / 512;
 
-	printk(" RDSK (%d)", blksize * 512);	/* Be more informative */
+	print_partition(state, " RDSK (%d)", blksize * 512);	/* Be more informative */
 	blk = be32_to_cpu(rdb->rdb_PartitionList);
 	put_dev_sector(sect);
 	for (part = 1; blk>0 && part<=16; part++, put_dev_sector(sect)) {
@@ -110,20 +110,19 @@ amiga_partition(struct parsed_partitions *state, struct block_device *bdev)
 			__be32 *dt = (__be32 *)dostype;
 			*dt = pb->pb_Environment[16];
 			if (dostype[3] < ' ')
-				printk(" (%c%c%c^%c)",
+				print_partition(state, " (%c%c%c^%c)",
 					dostype[0], dostype[1],
 					dostype[2], dostype[3] + '@' );
 			else
-				printk(" (%c%c%c%c)",
+				print_partition(state, " (%c%c%c%c)",
 					dostype[0], dostype[1],
 					dostype[2], dostype[3]);
-			printk("(res %d spb %d)",
+			print_partition(state, "(res %d spb %d)",
 				be32_to_cpu(pb->pb_Environment[6]),
 				be32_to_cpu(pb->pb_Environment[4]));
 		}
 		res = 1;
 	}
-	printk("\n");
 
 rdb_done:
 	return res;
diff --git a/fs/partitions/atari.c b/fs/partitions/atari.c
index 1f3572d..b696b52 100644
--- a/fs/partitions/atari.c
+++ b/fs/partitions/atari.c
@@ -62,7 +62,7 @@ int atari_partition(struct parsed_partitions *state, struct block_device *bdev)
 	}
 
 	pi = &rs->part[0];
-	printk (" AHDI");
+	print_partition(state, " AHDI");
 	for (slot = 1; pi < &rs->part[4] && slot < state->limit; slot++, pi++) {
 		struct rootsector *xrs;
 		Sector sect2;
@@ -81,19 +81,19 @@ int atari_partition(struct parsed_partitions *state, struct block_device *bdev)
 #ifdef ICD_PARTS
 		part_fmt = 1;
 #endif
-		printk(" XGM<");
+		print_partition(state, " XGM<");
 		partsect = extensect = be32_to_cpu(pi->st);
 		while (1) {
 			xrs = (struct rootsector *)read_dev_sector(bdev, partsect, &sect2);
 			if (!xrs) {
-				printk (" block %ld read failed\n", partsect);
+				print_partition(state, " block %ld read failed\n", partsect);
 				put_dev_sector(sect);
 				return -1;
 			}
 
 			/* ++roman: sanity check: bit 0 of flg field must be set */
 			if (!(xrs->part[0].flg & 1)) {
-				printk( "\nFirst sub-partition in extended partition is not valid!\n" );
+				printk("First sub-partition in extended partition is not valid!\n");
 				put_dev_sector(sect2);
 				break;
 			}
@@ -108,7 +108,7 @@ int atari_partition(struct parsed_partitions *state, struct block_device *bdev)
 				break;
 			}
 			if (memcmp( xrs->part[1].id, "XGM", 3 ) != 0) {
-				printk("\nID of extended partition is not XGM!\n");
+				printk("ID of extended partition is not XGM!\n");
 				put_dev_sector(sect2);
 				break;
 			}
@@ -116,18 +116,18 @@ int atari_partition(struct parsed_partitions *state, struct block_device *bdev)
 			partsect = be32_to_cpu(xrs->part[1].st) + extensect;
 			put_dev_sector(sect2);
 			if (++slot == state->limit) {
-				printk( "\nMaximum number of partitions reached!\n" );
+				printk("Maximum number of partitions reached!\n");
 				break;
 			}
 		}
-		printk(" >");
+		print_partition(state, " >");
 	}
 #ifdef ICD_PARTS
 	if ( part_fmt!=1 ) { /* no extended partitions -> test ICD-format */
 		pi = &rs->icdpart[0];
 		/* sanity check: no ICD format if first partition invalid */
 		if (OK_id(pi->id)) {
-			printk(" ICD<");
+			print_partition(state, " ICD<");
 			for (; pi < &rs->icdpart[8] && slot < state->limit; slot++, pi++) {
 				/* accept only GEM,BGM,RAW,LNX,SWP partitions */
 				if (!((pi->flg & 1) && OK_id(pi->id)))
@@ -137,13 +137,11 @@ int atari_partition(struct parsed_partitions *state, struct block_device *bdev)
 						be32_to_cpu(pi->st),
 						be32_to_cpu(pi->siz));
 			}
-			printk(" >");
+			print_partition(state, " >");
 		}
 	}
 #endif
 	put_dev_sector(sect);
 
-	printk ("\n");
-
 	return 1;
 }
diff --git a/fs/partitions/check.c b/fs/partitions/check.c
index 722e12e..a3dcbf6 100644
--- a/fs/partitions/check.c
+++ b/fs/partitions/check.c
@@ -162,9 +162,10 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 	state = kmalloc(sizeof(struct parsed_partitions), GFP_KERNEL);
 	if (!state)
 		return NULL;
+	sb_init(&state->sb);
 
 	disk_name(hd, 0, state->name);
-	printk(KERN_INFO " %s:", state->name);
+	print_partition(state, "%s:", state->name);
 	if (isdigit(state->name[strlen(state->name)-1]))
 		sprintf(state->name, "p");
 
@@ -182,15 +183,23 @@ check_partition(struct gendisk *hd, struct block_device *bdev)
 		}
 
 	}
-	if (res > 0)
+
+	if (res > 0) {
+		printk(KERN_INFO " %s\n", sb_string(&state->sb));
+		sb_free(&state->sb);
 		return state;
-	if (err)
+	}
+
 	/* The partition is unrecognized. So report I/O errors if there were any */
+	if (err)
 		res = err;
 	if (!res)
-		printk(" unknown partition table\n");
+		print_partition(state, " unknown partition table");
 	else if (warn_no_part)
-		printk(" unable to read partition table\n");
+		print_partition(state, " unable to read partition table");
+
+	printk(KERN_INFO " %s\n", sb_string(&state->sb));
+	sb_free(&state->sb);
 	kfree(state);
 	return ERR_PTR(res);
 }
diff --git a/fs/partitions/check.h b/fs/partitions/check.h
index 17ae8ec..84be0df 100644
--- a/fs/partitions/check.h
+++ b/fs/partitions/check.h
@@ -1,5 +1,6 @@
 #include <linux/pagemap.h>
 #include <linux/blkdev.h>
+#include <linux/stringbuf.h>
 
 /*
  * add_gd_partition adds a partitions details to the devices partition
@@ -16,15 +17,18 @@ struct parsed_partitions {
 	} parts[MAX_PART];
 	int next;
 	int limit;
+	struct stringbuf sb;
 };
 
+#define print_partition(p, fmt, args...) sb_printf(&p->sb, fmt , ## args)
+
 static inline void
 put_partition(struct parsed_partitions *p, int n, sector_t from, sector_t size)
 {
 	if (n < p->limit) {
 		p->parts[n].from = from;
 		p->parts[n].size = size;
-		printk(" %s%d", p->name, n);
+		sb_printf(&p->sb, " %s%d", p->name, n);
 	}
 }
 
diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
index e7b0700..9631304 100644
--- a/fs/partitions/efi.c
+++ b/fs/partitions/efi.c
@@ -634,6 +634,5 @@ efi_partition(struct parsed_partitions *state, struct block_device *bdev)
 	}
 	kfree(ptes);
 	kfree(gpt);
-	printk("\n");
 	return 1;
 }
diff --git a/fs/partitions/ibm.c b/fs/partitions/ibm.c
index 1e064c4..53766a7 100644
--- a/fs/partitions/ibm.c
+++ b/fs/partitions/ibm.c
@@ -111,14 +111,14 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev)
 			 * VM style CMS1 labeled disk
 			 */
 			if (label->cms.disk_offset != 0) {
-				printk("CMS1/%8s(MDSK):", name);
+				print_partition(state, "CMS1/%8s(MDSK):", name);
 				/* disk is reserved minidisk */
 				blocksize = label->cms.block_size;
 				offset = label->cms.disk_offset;
 				size = (label->cms.block_count - 1)
 					* (blocksize >> 9);
 			} else {
-				printk("CMS1/%8s:", name);
+				print_partition(state, "CMS1/%8s:", name);
 				offset = (info->label_block + 1);
 				size = i_size >> 9;
 			}
@@ -127,9 +127,9 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev)
 			 * Old style LNX1 or unlabeled disk
 			 */
 			if (strncmp(type, "LNX1", 4) == 0)
-				printk ("LNX1/%8s:", name);
+				print_partition(state, "LNX1/%8s:", name);
 			else
-				printk("(nonl)");
+				print_partition(state, "(nonl)");
 			offset = (info->label_block + 1);
 			size = i_size >> 9;
 		}
@@ -147,7 +147,7 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev)
 		 * if not, something is wrong, skipping partition detection
 		 */
 		if (strncmp(type, "VOL1",  4) == 0) {
-			printk("VOL1/%8s:", name);
+			print_partition(state, "VOL1/%8s:", name);
 			/*
 			 * get block number and read then go through format1
 			 * labels
@@ -203,7 +203,6 @@ ibm_partition(struct parsed_partitions *state, struct block_device *bdev)
 
 	}
 
-	printk("\n");
 	goto out_freeall;
 
 
diff --git a/fs/partitions/karma.c b/fs/partitions/karma.c
index 176d89b..e85c430 100644
--- a/fs/partitions/karma.c
+++ b/fs/partitions/karma.c
@@ -50,7 +50,6 @@ int karma_partition(struct parsed_partitions *state, struct block_device *bdev)
 		}
 		slot++;
 	}
-	printk("\n");
 	put_dev_sector(sect);
 	return 1;
 }
diff --git a/fs/partitions/ldm.c b/fs/partitions/ldm.c
index e7dd1d4..ae057d0 100644
--- a/fs/partitions/ldm.c
+++ b/fs/partitions/ldm.c
@@ -611,7 +611,7 @@ static struct vblk * ldm_get_disk_objid (const struct ldmdb *ldb)
 
 /**
  * ldm_create_data_partitions - Create data partitions for this device
- * @pp:   List of the partitions parsed so far
+ * @state:   List of the partitions parsed so far
  * @ldb:  Cache of the database structures
  *
  * The database contains ALL the partitions for ALL disk groups, so we need to
@@ -626,7 +626,7 @@ static struct vblk * ldm_get_disk_objid (const struct ldmdb *ldb)
  * Return:  'true'   Partition created
  *          'false'  Error, probably a range checking problem
  */
-static bool ldm_create_data_partitions (struct parsed_partitions *pp,
+static bool ldm_create_data_partitions (struct parsed_partitions *state,
 					const struct ldmdb *ldb)
 {
 	struct list_head *item;
@@ -635,7 +635,7 @@ static bool ldm_create_data_partitions (struct parsed_partitions *pp,
 	struct vblk_part *part;
 	int part_num = 1;
 
-	BUG_ON (!pp || !ldb);
+	BUG_ON (!state || !ldb);
 
 	disk = ldm_get_disk_objid (ldb);
 	if (!disk) {
@@ -643,7 +643,7 @@ static bool ldm_create_data_partitions (struct parsed_partitions *pp,
 		return false;
 	}
 
-	printk (" [LDM]");
+	print_partition(state, " [LDM]");
 
 	/* Create the data partitions */
 	list_for_each (item, &ldb->v_part) {
@@ -653,12 +653,11 @@ static bool ldm_create_data_partitions (struct parsed_partitions *pp,
 		if (part->disk_id != disk->obj_id)
 			continue;
 
-		put_partition (pp, part_num, ldb->ph.logical_disk_start +
+		put_partition(state, part_num, ldb->ph.logical_disk_start +
 				part->start, part->size);
 		part_num++;
 	}
 
-	printk ("\n");
 	return true;
 }
 
diff --git a/fs/partitions/mac.c b/fs/partitions/mac.c
index d4a0fad..44fee83 100644
--- a/fs/partitions/mac.c
+++ b/fs/partitions/mac.c
@@ -59,7 +59,7 @@ int mac_partition(struct parsed_partitions *state, struct block_device *bdev)
 		put_dev_sector(sect);
 		return 0;		/* not a MacOS disk */
 	}
-	printk(" [mac]");
+	print_partition(state, " [mac]");
 	blocks_in_map = be32_to_cpu(part->map_count);
 	for (blk = 1; blk <= blocks_in_map; ++blk) {
 		int pos = blk * secsize;
@@ -127,6 +127,5 @@ int mac_partition(struct parsed_partitions *state, struct block_device *bdev)
 #endif
 
 	put_dev_sector(sect);
-	printk("\n");
 	return 1;
 }
diff --git a/fs/partitions/msdos.c b/fs/partitions/msdos.c
index 5567ec0..63a0085 100644
--- a/fs/partitions/msdos.c
+++ b/fs/partitions/msdos.c
@@ -212,9 +212,9 @@ parse_solaris_x86(struct parsed_partitions *state, struct block_device *bdev,
 		put_dev_sector(sect);
 		return;
 	}
-	printk(" %s%d: <solaris:", state->name, origin);
+	print_partition(state, " %s%d: <solaris:", state->name, origin);
 	if (le32_to_cpu(v->v_version) != 1) {
-		printk("  cannot handle version %d vtoc>\n",
+		print_partition(state, "  cannot handle version %d vtoc>\n",
 			le32_to_cpu(v->v_version));
 		put_dev_sector(sect);
 		return;
@@ -225,7 +225,7 @@ parse_solaris_x86(struct parsed_partitions *state, struct block_device *bdev,
 		struct solaris_x86_slice *s = &v->v_slice[i];
 		if (s->s_size == 0)
 			continue;
-		printk(" [s%d]", i);
+		print_partition(state, " [s%d]", i);
 		/* solaris partitions are relative to current MS-DOS
 		 * one; must add the offset of the current partition */
 		put_partition(state, state->next++,
@@ -233,7 +233,7 @@ parse_solaris_x86(struct parsed_partitions *state, struct block_device *bdev,
 				 le32_to_cpu(s->s_size));
 	}
 	put_dev_sector(sect);
-	printk(" >\n");
+	print_partition(state, " >");
 #endif
 }
 
@@ -258,7 +258,7 @@ parse_bsd(struct parsed_partitions *state, struct block_device *bdev,
 		put_dev_sector(sect);
 		return;
 	}
-	printk(" %s%d: <%s:", state->name, origin, flavour);
+	print_partition(state, " %s%d: <%s:", state->name, origin, flavour);
 
 	if (le16_to_cpu(l->d_npartitions) < max_partitions)
 		max_partitions = le16_to_cpu(l->d_npartitions);
@@ -275,16 +275,16 @@ parse_bsd(struct parsed_partitions *state, struct block_device *bdev,
 			/* full parent partition, we have it already */
 			continue;
 		if (offset > bsd_start || offset+size < bsd_start+bsd_size) {
-			printk("bad subpartition - ignored\n");
+			printk("bad subpartition %d - ignored\n", p - l->d_partitions);
 			continue;
 		}
 		put_partition(state, state->next++, bsd_start, bsd_size);
 	}
 	put_dev_sector(sect);
 	if (le16_to_cpu(l->d_npartitions) > max_partitions)
-		printk(" (ignored %d more)",
+		print_partition(state, " (ignored %d more)",
 		       le16_to_cpu(l->d_npartitions) - max_partitions);
-	printk(" >\n");
+	print_partition(state, " >");
 }
 #endif
 
@@ -339,7 +339,7 @@ parse_unixware(struct parsed_partitions *state, struct block_device *bdev,
 		put_dev_sector(sect);
 		return;
 	}
-	printk(" %s%d: <unixware:", state->name, origin);
+	print_partition(state, " %s%d: <unixware:", state->name, origin);
 	p = &l->vtoc.v_slice[1];
 	/* I omit the 0th slice as it is the same as whole disk. */
 	while (p - &l->vtoc.v_slice[0] < UNIXWARE_NUMSLICE) {
@@ -352,7 +352,7 @@ parse_unixware(struct parsed_partitions *state, struct block_device *bdev,
 		p++;
 	}
 	put_dev_sector(sect);
-	printk(" >\n");
+	print_partition(state, " >");
 #endif
 }
 
@@ -383,7 +383,7 @@ parse_minix(struct parsed_partitions *state, struct block_device *bdev,
 	if (msdos_magic_present (data + 510) &&
 	    SYS_IND(p) == MINIX_PARTITION) { /* subpartition table present */
 
-		printk(" %s%d: <minix:", state->name, origin);
+		print_partition(state, " %s%d: <minix:", state->name, origin);
 		for (i = 0; i < MINIX_NR_SUBPARTITIONS; i++, p++) {
 			if (state->next == state->limit)
 				break;
@@ -392,7 +392,7 @@ parse_minix(struct parsed_partitions *state, struct block_device *bdev,
 				put_partition(state, state->next++,
 					      START_SECT(p), NR_SECTS(p));
 		}
-		printk(" >\n");
+		print_partition(state, " >");
 	}
 	put_dev_sector(sect);
 #endif /* CONFIG_MINIX_SUBPARTITION */
@@ -431,7 +431,7 @@ int msdos_partition(struct parsed_partitions *state, struct block_device *bdev)
 
 	if (aix_magic_present(data, bdev)) {
 		put_dev_sector(sect);
-		printk( " [AIX]");
+		print_partition(state,  " [AIX]");
 		return 0;
 	}
 
@@ -477,22 +477,20 @@ int msdos_partition(struct parsed_partitions *state, struct block_device *bdev)
 			/* prevent someone doing mkfs or mkswap on an
 			   extended partition, but leave room for LILO */
 			put_partition(state, slot, start, size == 1 ? 1 : 2);
-			printk(" <");
+			print_partition(state, " <");
 			parse_extended(state, bdev, start, size);
-			printk(" >");
+			print_partition(state, " >");
 			continue;
 		}
 		put_partition(state, slot, start, size);
 		if (SYS_IND(p) == LINUX_RAID_PARTITION)
 			state->parts[slot].flags = 1;
 		if (SYS_IND(p) == DM6_PARTITION)
-			printk("[DM]");
+			print_partition(state, "[DM]");
 		if (SYS_IND(p) == EZD_PARTITION)
-			printk("[EZD]");
+			print_partition(state, "[EZD]");
 	}
 
-	printk("\n");
-
 	/* second pass - output for each on a separate line */
 	p = (struct partition *) (0x1be + data);
 	for (slot = 1 ; slot <= 4 ; slot++, p++) {
diff --git a/fs/partitions/osf.c b/fs/partitions/osf.c
index c05c17b..634229d 100644
--- a/fs/partitions/osf.c
+++ b/fs/partitions/osf.c
@@ -72,7 +72,6 @@ int osf_partition(struct parsed_partitions *state, struct block_device *bdev)
 				le32_to_cpu(partition->p_size));
 		slot++;
 	}
-	printk("\n");
 	put_dev_sector(sect);
 	return 1;
 }
diff --git a/fs/partitions/sgi.c b/fs/partitions/sgi.c
index ed5ac83..54b9850 100644
--- a/fs/partitions/sgi.c
+++ b/fs/partitions/sgi.c
@@ -76,7 +76,6 @@ int sgi_partition(struct parsed_partitions *state, struct block_device *bdev)
 		}
 		slot++;
 	}
-	printk("\n");
 	put_dev_sector(sect);
 	return 1;
 }
diff --git a/fs/partitions/sun.c b/fs/partitions/sun.c
index c95e6a6..dafe00d 100644
--- a/fs/partitions/sun.c
+++ b/fs/partitions/sun.c
@@ -116,7 +116,6 @@ int sun_partition(struct parsed_partitions *state, struct block_device *bdev)
 		}
 		slot++;
 	}
-	printk("\n");
 	put_dev_sector(sect);
 	return 1;
 }
diff --git a/fs/partitions/sysv68.c b/fs/partitions/sysv68.c
index 4eba27b..5ab913b 100644
--- a/fs/partitions/sysv68.c
+++ b/fs/partitions/sysv68.c
@@ -73,7 +73,7 @@ int sysv68_partition(struct parsed_partitions *state, struct block_device *bdev)
 		return -1;
 
 	slices -= 1; /* last slice is the whole disk */
-	printk("sysV68: %s(s%u)", state->name, slices);
+	print_partition(state, "sysV68: %s(s%u)", state->name, slices);
 	slice = (struct slice *)data;
 	for (i = 0; i < slices; i++, slice++) {
 		if (slot == state->limit)
@@ -82,11 +82,10 @@ int sysv68_partition(struct parsed_partitions *state, struct block_device *bdev)
 			put_partition(state, slot,
 				be32_to_cpu(slice->blkoff),
 				be32_to_cpu(slice->nblocks));
-			printk("(s%u)", i);
+			print_partition(state, "(s%u)", i);
 		}
 		slot++;
 	}
-	printk("\n");
 	put_dev_sector(sect);
 	return 1;
 }
diff --git a/fs/partitions/ultrix.c b/fs/partitions/ultrix.c
index ec852c1..5b01971 100644
--- a/fs/partitions/ultrix.c
+++ b/fs/partitions/ultrix.c
@@ -39,7 +39,6 @@ int ultrix_partition(struct parsed_partitions *state, struct block_device *bdev)
 					      label->pt_part[i].pi_blkoff,
 					      label->pt_part[i].pi_nblocks);
 		put_dev_sector(sect);
-		printk ("\n");
 		return 1;
 	} else {
 		put_dev_sector(sect);
-- 
1.5.3.4


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
  2007-10-23 21:12 ` [PATCH 2/4] isdn: Use stringbuf Matthew Wilcox
@ 2007-10-23 22:11 ` Matt Mackall
  2007-10-24  1:49   ` Matthew Wilcox
  2007-10-23 23:43 ` Linus Torvalds
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Matt Mackall @ 2007-10-23 22:11 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Tue, Oct 23, 2007 at 05:12:43PM -0400, Matthew Wilcox wrote:
> Consecutive calls to printk are non-atomic, which leads to various
> implementations for accumulating strings which can be printed in one call.
> This is a generic string buffer which can also be used for non-printk
> purposes.  There is no sb_scanf implementation yet as I haven't identified
> a user for it.

You might want to consider growing the buffer by no less than a small
constant factor like 1.3x. This will keep things that do short concats
in a loop from degrading to O(n^2) performance due to realloc and
memcpy.

> + * No locking is performed, although memory allocation is done with
> + * GFP_ATOMIC to allow it to be called when you're holding a lock.

Should probably just bite the bullet and pass a flag.

> +#define INITIAL_SIZE 32

Too small. That will guarantee that most users end up doing a realloc.
Can we have 128 instead?

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
  2007-10-23 21:12 ` [PATCH 2/4] isdn: Use stringbuf Matthew Wilcox
  2007-10-23 22:11 ` [PATCH 1/4] stringbuf: A string buffer implementation Matt Mackall
@ 2007-10-23 23:43 ` Linus Torvalds
  2007-10-24  2:30   ` Matthew Wilcox
  2007-10-24  2:19 ` Eric St-Laurent
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Linus Torvalds @ 2007-10-23 23:43 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: akpm, linux-kernel, Matthew Wilcox



On Tue, 23 Oct 2007, Matthew Wilcox wrote:
>
> This is a generic string buffer which can also be used for non-printk
> purposes.  There is no sb_scanf implementation yet as I haven't identified
> a user for it.

Hmm. Have you looked at the git "strbuf" code? 

And stuff like "sb_string()" are just evil. The string is there. Just call 
it something better than "s", and let people just use "sb->buf" or 
something. Why make the interface harder to use than necessary?

			Linus

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 22:11 ` [PATCH 1/4] stringbuf: A string buffer implementation Matt Mackall
@ 2007-10-24  1:49   ` Matthew Wilcox
  2007-10-24 15:20     ` Matt Mackall
  0 siblings, 1 reply; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24  1:49 UTC (permalink / raw)
  To: Matt Mackall; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Tue, Oct 23, 2007 at 05:11:16PM -0500, Matt Mackall wrote:
> You might want to consider growing the buffer by no less than a small
> constant factor like 1.3x. This will keep things that do short concats
> in a loop from degrading to O(n^2) performance due to realloc and
> memcpy.

I looked at slab and slub, and would grow the buffer by no less than
1.5x each time, thanks to the buckets.  I'd initially implemented 2x,
but switched to allocating size+1 and calling ksize() as being a more
efficient implementation.

I presume slob is different?  Actually, slob doesn't seem to
provide krealloc, so I think stringbuf won't work on slob.  Will you
have time to fix this?

> Should probably just bite the bullet and pass a flag.

Hrm.

extern void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *fmt, ...)
        __attribute__((format(printf, 3, 4)));

?  Any objections?

> > +#define INITIAL_SIZE 32
> 
> Too small. That will guarantee that most users end up doing a realloc.
> Can we have 128 instead?

I don't care.  Sure!

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
                   ` (2 preceding siblings ...)
  2007-10-23 23:43 ` Linus Torvalds
@ 2007-10-24  2:19 ` Eric St-Laurent
  2007-10-24  2:35   ` Matthew Wilcox
  2007-10-24 13:21 ` Florian Weimer
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Eric St-Laurent @ 2007-10-24  2:19 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox


On Tue, 2007-10-23 at 17:12 -0400, Matthew Wilcox wrote:
> Consecutive calls to printk are non-atomic, which leads to various
> implementations for accumulating strings which can be printed in one call.
> This is a generic string buffer which can also be used for non-printk
> purposes.  There is no sb_scanf implementation yet as I haven't identified
> a user for it.
> 
> +
> +struct stringbuf {
> +	char *s;
> +	int alloc;
> +	int len;
> +};
> +

I don't know if copy-on-write semantics are really useful for current
in-kernel uses, but I've coded and used a C++ string class like this in
the past:

struct string_data
{
	int nrefs;
	unsigned len;
	unsigned capacity;
	//char data[capacity];	/* allocated along string_data */
};

struct string	/* or typedef in C... */
{
	struct string *data;
};

[ struct string_data is a hidden implementation detail, only struct
string is exposed ]

Multiple string objects can share the same data, by increasing the nrefs
count, a new data is allocated if the string is modified and nrefs > 1.

Not having to iterate over the string to calculate it's length,
allocating a larger buffer to eliminate re-allocation and copy-on-write
semantics make a string like this a vast performance improvement over a
normal C string for a minimal (about 3 ints per data buffer) memory
cost.

By using it correctly it can prevents buffer overflows.

You still always null terminate the string stored in data to directly
use it a normal C string.

You also statically allocate an empty string which is shared by all
"uninitialized" or empty strings.


Even without copy-on-write semantics and reference counting, I think
this approach is better because it uses 1 less "object" and allocation:

struct string - "handle" (pointer really) to string data
struct string_data - string data

versus:

struct stringbuf *sb - pointer to string object
struct stringbuf - string object
char *s (member of stringbuf) - string data



Best regards,

- Eric



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 23:43 ` Linus Torvalds
@ 2007-10-24  2:30   ` Matthew Wilcox
  2007-10-24  2:45     ` Andrew Morton
  0 siblings, 1 reply; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24  2:30 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: akpm, linux-kernel, Matthew Wilcox

On Tue, Oct 23, 2007 at 04:43:53PM -0700, Linus Torvalds wrote:
> On Tue, 23 Oct 2007, Matthew Wilcox wrote:
> > This is a generic string buffer which can also be used for non-printk
> > purposes.  There is no sb_scanf implementation yet as I haven't identified
> > a user for it.
> 
> Hmm. Have you looked at the git "strbuf" code? 

I hadn't.  It's quite spookily similar in some ways.  There's a lot of
things you've added because you presumably have use for them in git that
could easily be added once we had users for them in the kernel.  There's
a few things stringbufs do that strbufs don't need to because they're in
userspace.

I can easily change the API at this point, so if renaming things will
make you happier, just say so.

> And stuff like "sb_string()" are just evil. The string is there. Just call 
> it something better than "s", and let people just use "sb->buf" or 
> something. Why make the interface harder to use than necessary?

It's a matter of taste ... some people prefer to use accessors for
everything, other people prefer to expose the underlying structure
directly.  I have to confess to being influenced by Java in 1998-99,
so possibly I tend to go for accessors too often.  Or possibly it's
just my experience of restructuring things in Linux where diffs would
be smaller if we only had used accessors for something.  In any case,
I don't care to fight about this; if you want it to be called buf,
then buf it shall be called, and I'll get rid of sb_string.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24  2:19 ` Eric St-Laurent
@ 2007-10-24  2:35   ` Matthew Wilcox
  2007-10-24  2:48     ` Eric St-Laurent
  0 siblings, 1 reply; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24  2:35 UTC (permalink / raw)
  To: Eric St-Laurent; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Tue, Oct 23, 2007 at 10:19:06PM -0400, Eric St-Laurent wrote:
> I don't know if copy-on-write semantics are really useful for current
> in-kernel uses, but I've coded and used a C++ string class like this in
> the past:

CoW isn't in the slightest bit helpful.  The point of these is to
provide an accumulator, therefore the majority of accesses to these
stringbufs are writes.

> Multiple string objects can share the same data, by increasing the nrefs
> count, a new data is allocated if the string is modified and nrefs > 1.

If we were trying to get rid of char * throughout the kernel, that might
make some sense; stringbufs have a more limited target though.

> Even without copy-on-write semantics and reference counting, I think
> this approach is better because it uses 1 less "object" and allocation:
> 
> struct string - "handle" (pointer really) to string data
> struct string_data - string data
> 
> versus:
> 
> struct stringbuf *sb - pointer to string object
> struct stringbuf - string object
> char *s (member of stringbuf) - string data

I think you missed the part of the patch where I said that the stringbuf
is normally allocated on the stack or as part of some other structure.
The only thing that should be allocated is the string itself.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24  2:30   ` Matthew Wilcox
@ 2007-10-24  2:45     ` Andrew Morton
  0 siblings, 0 replies; 39+ messages in thread
From: Andrew Morton @ 2007-10-24  2:45 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: Linus Torvalds, linux-kernel, Matthew Wilcox

On Tue, 23 Oct 2007 20:30:35 -0600 Matthew Wilcox <matthew@wil.cx> wrote:

> It's a matter of taste ... some people prefer to use accessors for
> everything, other people prefer to expose the underlying structure
> directly.

Experience tells us that when you use accessors you end up thanking
yourself later on.  Especially when diddling struct page ;)

Although one really should make a best-effort guess at "how likely is
this to change in the future".  If the implementation-to-be-copied
has been out there for a while then "not very" is a good answer.

But hey, don't listen to me - I like C++, and approve of Java.


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24  2:35   ` Matthew Wilcox
@ 2007-10-24  2:48     ` Eric St-Laurent
  0 siblings, 0 replies; 39+ messages in thread
From: Eric St-Laurent @ 2007-10-24  2:48 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox


On Tue, 2007-10-23 at 20:35 -0600, Matthew Wilcox wrote:

[...]

> > Multiple string objects can share the same data, by increasing the nrefs
> > count, a new data is allocated if the string is modified and nrefs > 1.
> 
> If we were trying to get rid of char * throughout the kernel, that might
> make some sense; stringbufs have a more limited target though.
> 

[...]

No contest, my suggestions only make sense for a general uses string
library.

I suspect most in-kernel string manipulations are limited to prepare
buffers to be copied to (and read from) user-space.


- Eric



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
                   ` (3 preceding siblings ...)
  2007-10-24  2:19 ` Eric St-Laurent
@ 2007-10-24 13:21 ` Florian Weimer
  2007-10-24 14:02   ` Matthew Wilcox
  2007-10-26 12:05 ` Pekka Enberg
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 39+ messages in thread
From: Florian Weimer @ 2007-10-24 13:21 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

* Matthew Wilcox:

> +struct stringbuf {
> +	char *s;
> +	int alloc;
> +	int len;
> +};

I think alloc and len should be unsigned (including some return values
in the remaining patch).

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24 13:21 ` Florian Weimer
@ 2007-10-24 14:02   ` Matthew Wilcox
  0 siblings, 0 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24 14:02 UTC (permalink / raw)
  To: Florian Weimer; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Wed, Oct 24, 2007 at 03:21:06PM +0200, Florian Weimer wrote:
> > +struct stringbuf {
> > +	char *s;
> > +	int alloc;
> > +	int len;
> > +};
> 
> I think alloc and len should be unsigned (including some return values
> in the remaining patch).

I don't.  Strings should never be as long as 2GB.  To put this in
perspective, the *entire* Encyclopaedia Britannica (all 32 volumes)
is estimated at being 1GB of text.

While it would be a fair criticism that I haven't put a check for
overrunning 2GB in the code, the implementation relies on a single
continuous buffer from kmalloc, and that's currently limited to 33554432
bytes (32MB).  I don't foresee kmalloc's maximum size going up by 7
orders of magnitude -- and if it did, fragmentation would prevent you
from ever getting it.

So, I might consider a change to set -E2BIG instead of -ENOMEM if we
pass KMALLOC_MAX_SIZE, but I do think this criticism is rather straining
at gnats.

Also, 'alloc' can be an errno, and that is signalled by a negative number.
Yes, we could do something like if (sb->alloc > (unsigned)-4095) like
the mmap code does, but given the points above, it's just not worth doing.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/4] sound: Use stringbuf
  2007-10-23 21:12   ` [PATCH 3/4] sound: " Matthew Wilcox
  2007-10-23 21:12     ` [PATCH 4/4] partitions: Fix non-atomic printk Matthew Wilcox
@ 2007-10-24 14:18     ` Takashi Iwai
  2007-10-24 16:01       ` Matthew Wilcox
  1 sibling, 1 reply; 39+ messages in thread
From: Takashi Iwai @ 2007-10-24 14:18 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

At Tue, 23 Oct 2007 17:12:45 -0400,
Matthew Wilcox wrote:
> 
> sound/ had its own snd_info_buffer for doing proc reads, which can be
> profitably replaced with stringbuf.  It actually finds a bug since ->read
> and ->write now have a different signature.

Do you mean the assignment of snd_ca0106_proc_reg_write?
I also found it out today coindentally :)

Any other bugs I missed?


> @@ -375,7 +375,7 @@ static void snd_ac97_proc_read(struct snd_info_entry *entry, struct snd_info_buf
>  
>  #ifdef CONFIG_SND_DEBUG
>  /* direct register write for debugging */
> -static void snd_ac97_proc_regs_write(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
> +static void snd_ac97_proc_regs_write(struct snd_info_entry *entry, struct stringbuf *buffer)
>  {
>  	struct snd_ac97 *ac97 = entry->private_data;
>  	char line[64];

This shouldn't be changed.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/4] sound: Use stringbuf
  2007-10-24 16:01       ` Matthew Wilcox
@ 2007-10-24 14:50         ` Takashi Iwai
  0 siblings, 0 replies; 39+ messages in thread
From: Takashi Iwai @ 2007-10-24 14:50 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

At Wed, 24 Oct 2007 10:01:05 -0600,
Matthew Wilcox wrote:
> 
> > Any other bugs I missed?
> 
> Let me try to find them ... there was this warning fix:
> 
> diff --git a/sound/isa/ad1848/ad1848_lib.c b/sound/isa/ad1848/ad1848_lib.c
> index a901cd1..9a64035 100644
> --- a/sound/isa/ad1848/ad1848_lib.c
> +++ b/sound/isa/ad1848/ad1848_lib.c
> @@ -213,7 +213,7 @@ static void snd_ad1848_mce_down(struct snd_ad1848 *chip)
>         for (timeout = 12000; timeout > 0 && (inb(AD1848P(chip, REGSEL)) & AD184
> 8_INIT); timeout--)
>                 udelay(100);
>  
> -       snd_printdd("(1) timeout = %d\n", timeout);
> +       snd_printdd("(1) timeout = %ld\n", timeout);
>  
>  #ifdef CONFIG_SND_DEBUG
>         if (inb(AD1848P(chip, REGSEL)) & AD1848_INIT)
> 
> but I think that's the only one.

OK, thanks.  I fixed it on ALSA tree now.


Takashi

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24  1:49   ` Matthew Wilcox
@ 2007-10-24 15:20     ` Matt Mackall
  2007-10-24 15:30       ` Matthew Wilcox
  0 siblings, 1 reply; 39+ messages in thread
From: Matt Mackall @ 2007-10-24 15:20 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Tue, Oct 23, 2007 at 07:49:20PM -0600, Matthew Wilcox wrote:
> On Tue, Oct 23, 2007 at 05:11:16PM -0500, Matt Mackall wrote:
> > You might want to consider growing the buffer by no less than a small
> > constant factor like 1.3x. This will keep things that do short concats
> > in a loop from degrading to O(n^2) performance due to realloc and
> > memcpy.
> 
> I looked at slab and slub, and would grow the buffer by no less than
> 1.5x each time, thanks to the buckets.  I'd initially implemented 2x,
> but switched to allocating size+1 and calling ksize() as being a more
> efficient implementation.

Fair enough.
 
> I presume slob is different?  Actually, slob doesn't seem to
> provide krealloc, so I think stringbuf won't work on slob.  Will you
> have time to fix this?

http://lxr.linux.no/source/mm/slob.c#L207

Yep, slob is different, it has no kmalloc buckets.

> > Should probably just bite the bullet and pass a flag.
> 
> Hrm.
> 
> extern void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *fmt, ...)
>         __attribute__((format(printf, 3, 4)));
> 
> ?  Any objections?

Fine by me.
 
> > > +#define INITIAL_SIZE 32
> > 
> > Too small. That will guarantee that most users end up doing a realloc.
> > Can we have 128 instead?
> 
> I don't care.  Sure!

Most of these objects will have very short lifetimes, so there's very
little downside.

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24 15:20     ` Matt Mackall
@ 2007-10-24 15:30       ` Matthew Wilcox
  0 siblings, 0 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24 15:30 UTC (permalink / raw)
  To: Matt Mackall; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Wed, Oct 24, 2007 at 10:20:36AM -0500, Matt Mackall wrote:
> On Tue, Oct 23, 2007 at 07:49:20PM -0600, Matthew Wilcox wrote:
> > I presume slob is different?  Actually, slob doesn't seem to
> > provide krealloc, so I think stringbuf won't work on slob.  Will you
> > have time to fix this?
> 
> http://lxr.linux.no/source/mm/slob.c#L207

That's 2.6.22.6; I was looking at 2.6.23, where clameter had moved
krealloc into mm/util.c.  So that explains why I thought slob didn't
provide it.  Apologies for the confusion.

> Yep, slob is different, it has no kmalloc buckets.

OK.  I have no problem with making stringbuf allocate max(oldsize * 1.5,
newsize+1).

> > extern void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *fmt, ...)
> >         __attribute__((format(printf, 3, 4)));
> > 
> > ?  Any objections?
> 
> Fine by me.

OK, I'll do that for the next round of patches.

> Most of these objects will have very short lifetimes, so there's very
> little downside.

I agree.  It was good for testing though, when I inadvertently wrote to
the wrong bit of the string on a realloc ;-)

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 3/4] sound: Use stringbuf
  2007-10-24 14:18     ` [PATCH 3/4] sound: Use stringbuf Takashi Iwai
@ 2007-10-24 16:01       ` Matthew Wilcox
  2007-10-24 14:50         ` Takashi Iwai
  0 siblings, 1 reply; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24 16:01 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Wed, Oct 24, 2007 at 04:18:40PM +0200, Takashi Iwai wrote:
> At Tue, 23 Oct 2007 17:12:45 -0400,
> Matthew Wilcox wrote:
> > 
> > sound/ had its own snd_info_buffer for doing proc reads, which can be
> > profitably replaced with stringbuf.  It actually finds a bug since ->read
> > and ->write now have a different signature.
> 
> Do you mean the assignment of snd_ca0106_proc_reg_write?
> I also found it out today coindentally :)

I was referring to this hunk, so yes:

@@ -448,10 +448,9 @@ int __devinit snd_ca0106_proc_init(struct snd_ca0106 * emu)
 //             entry->private_data = emu;
        }
        if(! snd_card_proc_new(emu->card, "ca0106_i2c", &entry)) {
-               snd_info_set_text_ops(entry, emu, snd_ca0106_proc_i2c_write);
+               entry->private_data = emu;
                entry->c.text.write = snd_ca0106_proc_i2c_write;
                entry->mode |= S_IWUSR;
-//             entry->private_data = emu;
        }
        if(! snd_card_proc_new(emu->card, "ca0106_regs2", &entry)) 
                snd_info_set_text_ops(entry, emu, snd_ca0106_proc_reg_read2);


> Any other bugs I missed?

Let me try to find them ... there was this warning fix:

diff --git a/sound/isa/ad1848/ad1848_lib.c b/sound/isa/ad1848/ad1848_lib.c
index a901cd1..9a64035 100644
--- a/sound/isa/ad1848/ad1848_lib.c
+++ b/sound/isa/ad1848/ad1848_lib.c
@@ -213,7 +213,7 @@ static void snd_ad1848_mce_down(struct snd_ad1848 *chip)
        for (timeout = 12000; timeout > 0 && (inb(AD1848P(chip, REGSEL)) & AD184
8_INIT); timeout--)
                udelay(100);
 
-       snd_printdd("(1) timeout = %d\n", timeout);
+       snd_printdd("(1) timeout = %ld\n", timeout);
 
 #ifdef CONFIG_SND_DEBUG
        if (inb(AD1848P(chip, REGSEL)) & AD1848_INIT)

but I think that's the only one.

> > -static void snd_ac97_proc_regs_write(struct snd_info_entry *entry, struct snd_info_buffer *buffer)
> > +static void snd_ac97_proc_regs_write(struct snd_info_entry *entry, struct stringbuf *buffer)
> 
> This shouldn't be changed.

Quite right, sorry about that, will omit it from the next round.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
                   ` (4 preceding siblings ...)
  2007-10-24 13:21 ` Florian Weimer
@ 2007-10-26 12:05 ` Pekka Enberg
  2007-10-27  7:31 ` Pavel Machek
  2007-10-30 15:26 ` Denys Vlasenko
  7 siblings, 0 replies; 39+ messages in thread
From: Pekka Enberg @ 2007-10-26 12:05 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

Hi,

On 10/24/07, Matthew Wilcox <matthew@wil.cx> wrote:
> +static void sb_vprintf(struct stringbuf *sb, const char *format, va_list args)
> +{
> +       char *s;
> +       int size;
> +
> +       if (sb->alloc == -ENOMEM)
> +               return;
> +       if (sb->alloc == 0) {
> +               sb->s = kmalloc(INITIAL_SIZE, GFP_ATOMIC);

How about putting ->gfp_flags to struct stringbuf and initializing
that in sb_init() instead of hard-coding to GFP_ATOMIC here?

Btw, the "sb" abbrevation is already used for "superblock". Perhaps we
could use "str" for these?

                                             Pekka

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
                   ` (5 preceding siblings ...)
  2007-10-26 12:05 ` Pekka Enberg
@ 2007-10-27  7:31 ` Pavel Machek
  2007-10-30 15:26 ` Denys Vlasenko
  7 siblings, 0 replies; 39+ messages in thread
From: Pavel Machek @ 2007-10-27  7:31 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Tue 2007-10-23 17:12:43, Matthew Wilcox wrote:
> Consecutive calls to printk are non-atomic, which leads to various
> implementations for accumulating strings which can be printed in one call.
> This is a generic string buffer which can also be used for non-printk
> purposes.  There is no sb_scanf implementation yet as I haven't identified
> a user for it.

GFP_ATOMIC alloca5tion there is bad news. It can randomly fail.....
without good method of handling that in caller.
							Pavel
> +/*
> + * Convert the stringbuf to a string.  It is the caller's responsibility
> + * to free the string.  The stringbuf is then ready to be reused.
> + */
> +static inline char *sb_to_string(struct stringbuf *sb)
> +{
> +	char *s = sb->s;
> +	if (sb_error(sb))
> +		s = kstrdup(s, GFP_ATOMIC);
> +	sb_init(sb);
> +	return s;
> +}

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
                   ` (6 preceding siblings ...)
  2007-10-27  7:31 ` Pavel Machek
@ 2007-10-30 15:26 ` Denys Vlasenko
  7 siblings, 0 replies; 39+ messages in thread
From: Denys Vlasenko @ 2007-10-30 15:26 UTC (permalink / raw)
  To: Matthew Wilcox; +Cc: torvalds, akpm, linux-kernel, Matthew Wilcox

On Tuesday 23 October 2007 22:12, Matthew Wilcox wrote:
> Consecutive calls to printk are non-atomic, which leads to various
> implementations for accumulating strings which can be printed in one call.
> This is a generic string buffer which can also be used for non-printk
> purposes.  There is no sb_scanf implementation yet as I haven't identified
> a user for it.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

> +static inline void sb_free(struct stringbuf *sb)
> +{
> +	if (sb->alloc > 0)
> +		kfree(sb->s);
> +	sb_init(sb);
> +}

> +static inline char *sb_to_string(struct stringbuf *sb)
> +{
> +	char *s = sb->s;
> +	if (sb_error(sb))
> +		s = kstrdup(s, GFP_ATOMIC);
> +	sb_init(sb);
> +	return s;
> +}

I am not sure inlining these functions is a win.
--
vda

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-29  3:03           ` Matt Mackall
@ 2007-10-29  5:38             ` Rusty Russell
  0 siblings, 0 replies; 39+ messages in thread
From: Rusty Russell @ 2007-10-29  5:38 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

On Monday 29 October 2007 14:03:52 Matt Mackall wrote:
> And on SLOB, which doesn't have those bloaty power-of-2 constraints?
> Looks like ~500 reallocs, including 250000 bytes of memcpy. Ouch!

In other words, the system was compiled for size optimization and that's
what happened.

The question is: how bad is it?  Let's look at those numbers for SLOB (32-bit
x86).  First we find that it does 1000 reallocs to build the string, so it
really is worst case: we do a memcpy every time, so that's 500k of copying.

Yet SLOB comes in at 423 ns per call.  Remember that the other allocators got
around 1491 ns?  I went back and turned slub debugging off, and that number
hardly changed.

Deeper probing didn't show any convincing cause for the speedup.  Perhaps
slob is recycling memory for this case far better than the others,
outweighing the overhead.

Here's the patch I'm using, analysis welcome.
Rusty.
===
diff --git a/drivers/lguest/core.c b/drivers/lguest/core.c
index cb4c670..a7b4033 100644
--- a/drivers/lguest/core.c
+++ b/drivers/lguest/core.c
@@ -239,6 +239,121 @@ int run_guest(struct lguest *lg, unsigned long __user *user)
 	return -ENOENT;
 }
 
+#include <linux/slab.h>
+#include <linux/stringbuf.h>
+#include <linux/ktime.h>
+
+static inline void *realloc_no_copy(const void *p, size_t new_size, gfp_t flags)
+{
+	void *ret;
+	size_t ks = 0;
+
+	if (unlikely(!new_size)) {
+		kfree(p);
+		return ZERO_SIZE_PTR;
+	}
+
+	if (p)
+		ks = ksize(p);
+
+	if (ks >= new_size)
+		return (void *)p;
+
+	ret = kmalloc_track_caller(new_size, flags);
+	if (ret)
+		kfree(p);
+	return ret;
+}
+
+static void test_stringbuf(void)
+{
+	unsigned int i, j;
+	ktime_t start, end;
+	unsigned int sb_alloc_count = 0;
+	char c[5];
+
+	start = ktime_get();
+	for (i = 0; i < 1000; i++) {
+		struct stringbuf *oldsb = NULL, *sb = NULL;
+
+		for (j = 0; j < 1000; j++) {
+			unsigned int k;
+			sb_printf_append(&sb, GFP_KERNEL, "a");
+			sb_alloc_count += (sb != oldsb);
+			oldsb = sb;
+#if 0
+			if (sb->buf[j+1] != '\0') {
+				printk("Final sb->buf[%i] = %i\n",
+				       j+1, sb->buf[j+1]);
+				break;
+			}
+			for (k = 0; k < j; k++) {
+				if (sb->buf[k] != 'a') {
+					printk("sb->buf[%i] = %i\n",
+					       k, sb->buf[k]);
+					break;
+				}
+			}
+#endif
+		}
+		sb_free(sb);
+	}
+	end = ktime_get();
+
+	printk("1000 x 1000 sb_printf_append == %lli ns %u allocs\n",
+	       ktime_to_ns(ktime_sub(end, start)), sb_alloc_count);
+
+	start = ktime_get();
+	for (i = 0; i < 1000; i++) {
+		struct stringbuf *oldsb = NULL, *sb = NULL;
+
+		for (j = 0; j < 1000; j++) {
+			sprintf(c, "a", &sb);
+			sb_alloc_count += (sb != oldsb);
+			oldsb = sb;
+		}
+		sb_free(sb);
+	}
+	end = ktime_get();
+
+	printk("1000 x 1000 sprintf == %lli ns\n",
+	       ktime_to_ns(ktime_sub(end, start)));
+
+	sb_alloc_count = 0;
+	start = ktime_get();
+	for (i = 0; i < 1000; i++) {
+		struct stringbuf *oldsb = NULL, *sb = NULL;
+
+		for (j = 0; j < 1000; j++) {
+			sb = realloc_no_copy(oldsb, j + 1, GFP_KERNEL);
+			sb_alloc_count += (sb != oldsb);
+			oldsb = sb;
+		}
+		kfree(sb);
+	}
+	end = ktime_get();
+
+	printk("1000 x 1000 realloc_no_copy == %lli ns %u allocs\n",
+	       ktime_to_ns(ktime_sub(end, start)), sb_alloc_count);
+
+	sb_alloc_count = 0;
+	start = ktime_get();
+	for (i = 0; i < 1000; i++) {
+		struct stringbuf *oldsb = NULL, *sb = NULL;
+
+		for (j = 0; j < 1000; j++) {
+			sb = krealloc(oldsb, j + 1, GFP_KERNEL);
+			sb_alloc_count += (sb != oldsb);
+			oldsb = sb;
+		}
+		kfree(sb);
+	}
+	end = ktime_get();
+
+	printk("1000 x 1000 krealloc == %lli ns %u allocs\n",
+	       ktime_to_ns(ktime_sub(end, start)), sb_alloc_count);
+}
+
 /*H:000
  * Welcome to the Host!
  *
@@ -251,6 +366,8 @@ static int __init init(void)
 {
 	int err;
 
+	test_stringbuf();
+
 	/* Lguest can't run under Xen, VMI or itself.  It does Tricky Stuff. */
 	if (paravirt_enabled()) {
 		printk("lguest is afraid of %s\n", pv_info.name);
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
diff --git a/include/linux/stringbuf.h b/include/linux/stringbuf.h
new file mode 100644
index 0000000..70b21c6
--- /dev/null
+++ b/include/linux/stringbuf.h
@@ -0,0 +1,30 @@
+#ifndef _LINUX_STRINGBUF_H
+#define _LINUX_STRINGBUF_H
+#include <linux/slab.h>
+
+/* This starts NULL and gets krealloc'ed as it grows. */
+struct stringbuf {
+	char buf[0];
+};
+
+/* Your stringbuf will point to this if we run out of memory. */
+extern char enomem_string[];
+
+/* Tack some stuff on the stringbuf. */
+extern void sb_printf_append(struct stringbuf **sb,
+			     gfp_t gfp, const char *fmt, ...)
+	__attribute__((format(printf, 3, 4)));
+
+/**
+ * sb_free - free a stringbuf used by sb_printf_append.
+ * @sb: the stringbuf pointer
+ *
+ * Handles the NULL and OOM cases, so no checking needed.
+ */
+static inline void sb_free(struct stringbuf *sb)
+{
+	if (sb->buf != enomem_string)
+		kfree(sb);
+}
+
+#endif /* _LINUX_STRINGBUF_H */
diff --git a/kernel/module.c b/kernel/module.c
diff --git a/lib/Makefile b/lib/Makefile
index 3a0983b..f075389 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -14,7 +14,7 @@ lib-$(CONFIG_SMP) += cpumask.o
 lib-y	+= kobject.o kref.o klist.o
 
 obj-y += div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
-	 bust_spinlocks.o hexdump.o kasprintf.o
+	 bust_spinlocks.o hexdump.o kasprintf.o stringbuf.o
 
 ifeq ($(CONFIG_DEBUG_KOBJECT),y)
 CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/stringbuf.c b/lib/stringbuf.c
new file mode 100644
index 0000000..e3c51be
--- /dev/null
+++ b/lib/stringbuf.c
@@ -0,0 +1,48 @@
+#include <stdarg.h>
+#include <linux/stringbuf.h>
+#include <linux/module.h>
+
+char enomem_string[] __attribute__((aligned(__alignof__(struct stringbuf))))
+	= "stringbuf: out of memory";
+EXPORT_SYMBOL(enomem_string);
+
+/**
+ * sb_printf_append - append to a stringbuf
+ * @sb: a pointer to the stringbuf ptr (which starts NULL)
+ * @gfp: flags for allocation
+ * @fmt: printf-style format
+ *
+ * Reallocates *@sb and appends to it.  Sets *sb to a explanatory string if
+ * out of memory.
+ */
+void sb_printf_append(struct stringbuf **sb, gfp_t gfp, const char *fmt, ...)
+{
+	unsigned int fmtlen, len;
+	va_list args;
+	struct stringbuf *oldsb = *sb;
+
+	if (oldsb->buf == enomem_string)
+		return;
+
+	va_start(args, fmt);
+	fmtlen = vsnprintf(NULL, 0, fmt, args);
+	va_end(args);
+
+	if (oldsb) {
+		len = strlen(oldsb->buf);
+		*sb = krealloc(oldsb, fmtlen + len + 1, gfp);
+	} else {
+		len = 0;
+		*sb = kmalloc(fmtlen + 1, gfp);
+	}
+
+	if (!*sb) {
+		kfree(oldsb);
+		*sb = (struct stringbuf *)enomem_string;
+	} else {
+		va_start(args, fmt);
+		vsprintf((*sb)->buf + len, fmt, args);
+		va_end(args);
+	}
+}
+EXPORT_SYMBOL(sb_printf_append);
diff --git a/mm/slub.c b/mm/slub.c
index aac1dd3..0760285 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3075,6 +3075,7 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, void *caller)
 
 	return slab_alloc(s, gfpflags, -1, caller);
 }
+EXPORT_SYMBOL(__kmalloc_track_caller);
 
 void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
 					int node, void *caller)

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-27 10:09         ` Rusty Russell
@ 2007-10-29  3:03           ` Matt Mackall
  2007-10-29  5:38             ` Rusty Russell
  0 siblings, 1 reply; 39+ messages in thread
From: Matt Mackall @ 2007-10-29  3:03 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

On Sat, Oct 27, 2007 at 08:09:30PM +1000, Rusty Russell wrote:
> On Saturday 27 October 2007 06:57:14 Matt Mackall wrote:
> > Well I expect once you start letting people easily build strings by
> > concatenation, you'll very shortly afterwards have people using them
> > in loops.  And having hidden O(n^2) behavior in there is a little sad, 
> > even though n will tend to be small and well-bounded. If we can do
> > something simple to avoid it, we should.
> 
> Hi Matt,
> 
>         I avoid typing even a single character of optimization until it's 
> justified.  This is partially a reaction against the machoptimization 
> tendencies of many kernel programmers, but it's mainly a concern at the 
> kernel's complexity creep.
> 
> Meanwhile, of course, I've now spent far too long analyzing this :)
> 
> Building a 1000 byte string 1 byte at a time involves 6 reallocs (SLAB) or 10 
> reallocs (SLUB).  Frankly, that's good enough without an explicit alloc 
> length field (better in some ways).

And on SLOB, which doesn't have those bloaty power-of-2 constraints?
Looks like ~500 reallocs, including 250000 bytes of memcpy. Ouch!

SLAB and SLUB (accidentally) internalize the optimization that I'm
proposing: grow the string by a constant factor at each step. Failing
to do that makes the behavior dismal.

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-27 16:34         ` Pekka Enberg
@ 2007-10-27 16:48           ` Matthew Wilcox
  0 siblings, 0 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-27 16:48 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Rusty Russell, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

On Sat, Oct 27, 2007 at 07:34:47PM +0300, Pekka Enberg wrote:
> On 10/27/07, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > On Saturday 27 October 2007 21:47:09 Pekka Enberg wrote:
> > > Why don't we just return -ENOMEM here just like all other APIs in the
> > > kernel?
> > I think Willy did it because this is for printk.  It makes more sense than
> > everyone opencoding an -ENOMEM handler, which will have to be replaced by
> > some mildly amusing string like "I want to printk but I have no memory!".
> > Next think you know 70% of the kernel will be bad limericks as everyone tries
> > to one-up each other.
> 
> My point was, of course, that the caller gets to decide what to do on
> OOM. For the most part, you probably want to ignore it but for things
> like pseudo filesystems where you generate a human-readable string,
> returning -ENOMEM from read(2) is preferable.

That was why I had an sb_error() as part of the API so a caller who
cared could check, and a caller who didn't care would never know --
they'd just print the out-of-memory string and continue happily.

It's not hard to add an sb_error to Rusty's rewrite.  It looks something
like:

static inline int sb_error(struct stringbuf *sb)
{
	if (sb == sb_enomem_string)
		return -ENOMEM;
	return 0;
}

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-27 12:50       ` Rusty Russell
@ 2007-10-27 16:34         ` Pekka Enberg
  2007-10-27 16:48           ` Matthew Wilcox
  0 siblings, 1 reply; 39+ messages in thread
From: Pekka Enberg @ 2007-10-27 16:34 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

Hi Rusty,

On Saturday 27 October 2007 21:47:09 Pekka Enberg wrote:
> > > +               kfree(oldsb);
> > > +               *sb = (struct stringbuf *)enomem_string;
> >
> > Why don't we just return -ENOMEM here just like all other APIs in the
> > kernel?

On 10/27/07, Rusty Russell <rusty@rustcorp.com.au> wrote:
> I think Willy did it because this is for printk.  It makes more sense than
> everyone opencoding an -ENOMEM handler, which will have to be replaced by
> some mildly amusing string like "I want to printk but I have no memory!".
> Next think you know 70% of the kernel will be bad limericks as everyone tries
> to one-up each other.

My point was, of course, that the caller gets to decide what to do on
OOM. For the most part, you probably want to ignore it but for things
like pseudo filesystems where you generate a human-readable string,
returning -ENOMEM from read(2) is preferable.

On Saturday 27 October 2007 21:47:09 Pekka Enberg wrote:
> > And I wonder if it makes more sense to store gfp_flags in
> > struct stringbuf and always use that? I mean, why would you want to
> > sometimes do GFP_ATOMIC and GFP_KERNEL allocations for the same
> > buffer?

On 10/27/07, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Firstly we don't have a buffer on first call (NULL), though we could introduce
> an sb_init() for that.  Secondly, since the purpose of this code is because
> they can't do the printk all at once: who's to say that isn't because they
> need to grab a lock for some of it?  Finally, we generally choose to expose
> the alloc flags to the caller to make them think about whether they really
> want to do allocation at this point.

Yeah, makes sense. Thanks.

                                   Pekka

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-27 11:47     ` Pekka Enberg
@ 2007-10-27 12:50       ` Rusty Russell
  2007-10-27 16:34         ` Pekka Enberg
  0 siblings, 1 reply; 39+ messages in thread
From: Rusty Russell @ 2007-10-27 12:50 UTC (permalink / raw)
  To: Pekka Enberg
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

On Saturday 27 October 2007 21:47:09 Pekka Enberg wrote:
> Hi Rusty,

Hi Pekka,

> On 10/26/07, Rusty Russell <rusty@rustcorp.com.au> wrote:
> > How about this?  It's as simple as I could make it...
>
> FWIW I like this patch better.

Thanks.

> > +               kfree(oldsb);
> > +               *sb = (struct stringbuf *)enomem_string;
>
> Why don't we just return -ENOMEM here just like all other APIs in the
> kernel?

I think Willy did it because this is for printk.  It makes more sense than 
everyone opencoding an -ENOMEM handler, which will have to be replaced by 
some mildly amusing string like "I want to printk but I have no memory!".  
Next think you know 70% of the kernel will be bad limericks as everyone tries 
to one-up each other.

> And I wonder if it makes more sense to store gfp_flags in 
> struct stringbuf and always use that? I mean, why would you want to
> sometimes do GFP_ATOMIC and GFP_KERNEL allocations for the same
> buffer?

Firstly we don't have a buffer on first call (NULL), though we could introduce 
an sb_init() for that.  Secondly, since the purpose of this code is because 
they can't do the printk all at once: who's to say that isn't because they 
need to grab a lock for some of it?  Finally, we generally choose to expose 
the alloc flags to the caller to make them think about whether they really 
want to do allocation at this point.

Cheers,
Rusty.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-26  2:11   ` Rusty Russell
  2007-10-26  3:41     ` Joe Perches
  2007-10-26 11:57     ` Matthew Wilcox
@ 2007-10-27 11:47     ` Pekka Enberg
  2007-10-27 12:50       ` Rusty Russell
  2 siblings, 1 reply; 39+ messages in thread
From: Pekka Enberg @ 2007-10-27 11:47 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

Hi Rusty,

On 10/26/07, Rusty Russell <rusty@rustcorp.com.au> wrote:
>        This just seems like more optimization and complexity that we need.  Interfaces
> using vsnprintf don't seem like good candidates for optimization.
>
> How about this?  It's as simple as I could make it...

FWIW I like this patch better.

On 10/26/07, Rusty Russell <rusty@rustcorp.com.au> wrote:
> +void sb_printf_append(struct stringbuf **sb, gfp_t gfp, const char *fmt, ...)
> +{
> +       unsigned int fmtlen, len;
> +       va_list args;
> +       struct stringbuf *oldsb = *sb;
> +
> +       if (oldsb->buf == enomem_string)
> +               return;
> +
> +       va_start(args, fmt);
> +       fmtlen = vsnprintf(NULL, 0, fmt, args);
> +       va_end(args);
> +
> +       len = oldsb ? strlen(oldsb->buf) : 0;
> +       *sb = krealloc(oldsb, len + fmtlen + 1, gfp);
> +       if (!*sb) {
> +               kfree(oldsb);
> +               *sb = (struct stringbuf *)enomem_string;

Why don't we just return -ENOMEM here just like all other APIs in the
kernel? And I wonder if it makes more sense to store gfp_flags in
struct stringbuf and always use that? I mean, why would you want to
sometimes do GFP_ATOMIC and GFP_KERNEL allocations for the same
buffer?

                                  Pekka

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-26 20:57       ` Matt Mackall
@ 2007-10-27 10:09         ` Rusty Russell
  2007-10-29  3:03           ` Matt Mackall
  0 siblings, 1 reply; 39+ messages in thread
From: Rusty Russell @ 2007-10-27 10:09 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

On Saturday 27 October 2007 06:57:14 Matt Mackall wrote:
> Well I expect once you start letting people easily build strings by
> concatenation, you'll very shortly afterwards have people using them
> in loops.  And having hidden O(n^2) behavior in there is a little sad, 
> even though n will tend to be small and well-bounded. If we can do
> something simple to avoid it, we should.

Hi Matt,

        I avoid typing even a single character of optimization until it's 
justified.  This is partially a reaction against the machoptimization 
tendencies of many kernel programmers, but it's mainly a concern at the 
kernel's complexity creep.

Meanwhile, of course, I've now spent far too long analyzing this :)

Building a 1000 byte string 1 byte at a time involves 6 reallocs (SLAB) or 10 
reallocs (SLUB).  Frankly, that's good enough without an explicit alloc 
length field (better in some ways).

As to keeping an explicit length vs strlen(): those 1000 calls on my test 
machine take 1491ns per call with an explicit length vs 1496ns per call with 
strlen().  That's not worth 4 bytes, let alone a single line of code, O(n^2) 
or no.

As the nail in the coffin, callers only use ->buf, so are insulated from any 
such optimizations if we decided to do them in future.

Hope that helps,
Rusty.
PS.  I don't think we should switch this to a simple char ** tho, as 
the "struct stringbuf" gives us some type safety and reminds people not to 
simply kfree it.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-26 11:57     ` Matthew Wilcox
@ 2007-10-26 20:57       ` Matt Mackall
  2007-10-27 10:09         ` Rusty Russell
  0 siblings, 1 reply; 39+ messages in thread
From: Matt Mackall @ 2007-10-26 20:57 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Rusty Russell, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

On Fri, Oct 26, 2007 at 05:57:27AM -0600, Matthew Wilcox wrote:
> On Fri, Oct 26, 2007 at 12:11:01PM +1000, Rusty Russell wrote:
> > 	This just seems like more optimization and complexity that we need.  Interfaces
> > using vsnprintf don't seem like good candidates for optimization.
> 
> That's a fair point, but I'm optimising for fewer trips into the
> slab(/slub/slob) allocator, and thus having less of an impact on the
> rest of the system.  Given that 'an alloc on every call' was one of the
> complaints Matt had about my v1 stringbuf patch, I can't imagine he'll
> be happy about this one either.

Well I expect once you start letting people easily build strings by
concatenation, you'll very shortly afterwards have people using them
in loops. And having hidden O(n^2) behavior in there is a little sad,
even though n will tend to be small and well-bounded. If we can do
something simple to avoid it, we should.

-- 
Mathematics is the supreme nostalgia of our time.

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-26  2:11   ` Rusty Russell
  2007-10-26  3:41     ` Joe Perches
@ 2007-10-26 11:57     ` Matthew Wilcox
  2007-10-26 20:57       ` Matt Mackall
  2007-10-27 11:47     ` Pekka Enberg
  2 siblings, 1 reply; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-26 11:57 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Linus Torvalds, Andrew Morton, linux-kernel, Matthew Wilcox

On Fri, Oct 26, 2007 at 12:11:01PM +1000, Rusty Russell wrote:
> 	This just seems like more optimization and complexity that we need.  Interfaces
> using vsnprintf don't seem like good candidates for optimization.

That's a fair point, but I'm optimising for fewer trips into the
slab(/slub/slob) allocator, and thus having less of an impact on the
rest of the system.  Given that 'an alloc on every call' was one of the
complaints Matt had about my v1 stringbuf patch, I can't imagine he'll
be happy about this one either.

> How about this?  It's as simple as I could make it...

Some very cute tricks in there; I particularly like passing a
double-pointer to ...printf so that the caller doesn't have to check
the return value.

Ultimately, I don't particularly care what version of stringbuf gets
merged, I just want something to make my dmesg non-ugly.  The fact that
my stringbuf implementation looked so damn similar to half a dozen things
that people had already invented elsewhere in the kernel made me think
that this was a good interface because it could be used to replace their
complex code too.

If you want to see some really complex stringbuffer-esque code that needs
replacing, take a look at pnp_info_buffer in drivers/pnp/interface.c ...

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-26  3:41     ` Joe Perches
@ 2007-10-26  5:05       ` Joe Perches
  0 siblings, 0 replies; 39+ messages in thread
From: Joe Perches @ 2007-10-26  5:05 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

Perhaps have an sb_alloc function and a failure mode that
uses printk when sb_alloc fails or sb_append is passed null?

Perhaps something like:

stringbuf *sb_alloc(char* level, gfp_t priority)
{
	stringbuf *sb = kmalloc(sizeof(*sb), priority);
	if (sb)
		sb>len = sprintf(sb->buf, "%s", level);
	else
		printk(level);
	return sb;
}
EXPORT_SYMBOL(sb_alloc);

/**
 * sb_append - append to a stringbuf
 * @sb: a pointer to the stringbuf
 * @fmt: printf-style format
 */
void sb_append(struct stringbuf *sb, const char *fmt, ...)
{
	va_list args;

	va_start(args, fmt);
	if (sb)
		sb->len += vscnprintf(&sb->buf[len], sizeof(sb->buf) - sb->len, fmt, args);
	else
		vprintk(fmt, args);
	va_end(args);
}
EXPORT_SYMBOL(sb_append);

void sb_printk(struct stringbuf *sb)
{
	if (sb && sb->len > 0) {
		printk(sb->buf);
		sb->len = 0;
	}
}
EXPORT_SYMBOL(sb_printk);

void sb_free(stringbuf *sb)
{
	kfree(sb);
}
EXPORT_SYMBOL(sb_free);



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-26  2:11   ` Rusty Russell
@ 2007-10-26  3:41     ` Joe Perches
  2007-10-26  5:05       ` Joe Perches
  2007-10-26 11:57     ` Matthew Wilcox
  2007-10-27 11:47     ` Pekka Enberg
  2 siblings, 1 reply; 39+ messages in thread
From: Joe Perches @ 2007-10-26  3:41 UTC (permalink / raw)
  To: Rusty Russell
  Cc: Matthew Wilcox, Linus Torvalds, Andrew Morton, linux-kernel,
	Matthew Wilcox

On Fri, 2007-10-26 at 12:11 +1000, Rusty Russell wrote:
> On Thursday 25 October 2007 05:59:49 Matthew Wilcox wrote:
> > Consecutive calls to printk are non-atomic, which leads to various
> > implementations for accumulating strings which can be printed in one call.
> > This is a generic string buffer which can also be used for non-printk
> > purposes.

> This just seems like more optimization and complexity that we need.
> Interfaces using vsnprintf don't seem like good candidates for
> optimization.

Perhaps just tie stringbuf to printk and make stringbuf->buf
the same 1K size as the printk buffer?

no reallocs, no in-place out-of-memory handling
normal kzalloc and kfree of stringbuf * or add sb_alloc & sb_free

struct stringbuf {
	int len;
	char buf[1024];
}

/**
 * sb_printf_append - append to a stringbuf
 * @sb: a pointer to the stringbuf
 * @fmt: printf-style format
 */
void sb_printf_append(struct stringbuf *sb, const char *fmt, ...)
{
	va_list args;

	va_start(args, fmt);
	sb->len += vscnprintf(&sb->buf[len], sizeof(sb->buf) - sb->len, fmt, args);
	va_end(args);
}
EXPORT_SYMBOL(sb_printf_append);



^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24 19:59 ` [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
  2007-10-24 20:59   ` Kyle Moffett
@ 2007-10-26  2:11   ` Rusty Russell
  2007-10-26  3:41     ` Joe Perches
                       ` (2 more replies)
  1 sibling, 3 replies; 39+ messages in thread
From: Rusty Russell @ 2007-10-26  2:11 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, Matthew Wilcox

On Thursday 25 October 2007 05:59:49 Matthew Wilcox wrote:
> Consecutive calls to printk are non-atomic, which leads to various
> implementations for accumulating strings which can be printed in one call.
> This is a generic string buffer which can also be used for non-printk
> purposes.  There is no sb_scanf implementation yet as I haven't identified
> a user for it.
>
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> ---
>  include/linux/stringbuf.h |   77 ++++++++++++++++++++++++++++++++++++++++
>  lib/Makefile              |    2 +-
>  lib/stringbuf.c           |   85 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 163 insertions(+), 1 deletions(-)

Hi Willy,

	This just seems like more optimization and complexity that we need.  Interfaces
using vsnprintf don't seem like good candidates for optimization.

How about this?  It's as simple as I could make it...

 include/linux/stringbuf.h |   30 ++++++++++++++++++++++++++++++
 lib/Makefile              |    2 +-
 lib/stringbuf.c           |   41 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/stringbuf.h
 create mode 100644 lib/stringbuf.c

diff --git a/include/linux/stringbuf.h b/include/linux/stringbuf.h
new file mode 100644
index 0000000..70b21c6
--- /dev/null
+++ b/include/linux/stringbuf.h
@@ -0,0 +1,30 @@
+#ifndef _LINUX_STRINGBUF_H
+#define _LINUX_STRINGBUF_H
+#include <linux/slab.h>
+
+/* This starts NULL and gets krealloc'ed as it grows. */
+struct stringbuf {
+	char buf[0];
+};
+
+/* Your stringbuf will point to this if we run out of memory. */
+extern char enomem_string[];
+
+/* Tack some stuff on the stringbuf. */
+extern void sb_printf_append(struct stringbuf **sb,
+			     gfp_t gfp, const char *fmt, ...)
+	__attribute__((format(printf, 3, 4)));
+
+/**
+ * sb_free - free a stringbuf used by sb_printf_append.
+ * @sb: the stringbuf pointer
+ *
+ * Handles the NULL and OOM cases, so no checking needed.
+ */
+static inline void sb_free(struct stringbuf *sb)
+{
+	if (sb->buf != enomem_string)
+		kfree(sb);
+}
+
+#endif /* _LINUX_STRINGBUF_H */
diff --git a/lib/Makefile b/lib/Makefile
index 3a0983b..f075389 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -14,7 +14,7 @@ lib-$(CONFIG_SMP) += cpumask.o
 lib-y	+= kobject.o kref.o klist.o
 
 obj-y += div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
-	 bust_spinlocks.o hexdump.o kasprintf.o
+	 bust_spinlocks.o hexdump.o kasprintf.o stringbuf.o
 
 ifeq ($(CONFIG_DEBUG_KOBJECT),y)
 CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/stringbuf.c b/lib/stringbuf.c
new file mode 100644
index 0000000..ffe977a
--- /dev/null
+++ b/lib/stringbuf.c
@@ -0,0 +1,41 @@
+#include <stdarg.h>
+#include <linux/stringbuf.h>
+#include <linux/module.h>
+
+char enomem_string[] __attribute__((aligned(__alignof__(struct stringbuf))))
+	= "stringbuf: out of memory";
+
+/**
+ * sb_printf_append - append to a stringbuf
+ * @sb: a pointer to the stringbuf ptr (which starts NULL)
+ * @gfp: flags for allocation
+ * @fmt: printf-style format
+ *
+ * Reallocates *@sb and appends to it.  Sets *sb to a explanatory string if
+ * out of memory.
+ */
+void sb_printf_append(struct stringbuf **sb, gfp_t gfp, const char *fmt, ...)
+{
+	unsigned int fmtlen, len;
+	va_list args;
+	struct stringbuf *oldsb = *sb;
+
+	if (oldsb->buf == enomem_string)
+		return;
+
+	va_start(args, fmt);
+	fmtlen = vsnprintf(NULL, 0, fmt, args);
+	va_end(args);
+
+	len = oldsb ? strlen(oldsb->buf) : 0;
+	*sb = krealloc(oldsb, len + fmtlen + 1, gfp);
+	if (!*sb) {
+		kfree(oldsb);
+		*sb = (struct stringbuf *)enomem_string;
+	} else {
+		va_start(args, fmt);
+		vsprintf((*sb)->buf + len, fmt, args);
+		va_end(args);
+	}
+}
+EXPORT_SYMBOL(sb_printf_append);

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-25  0:07       ` Kyle Moffett
@ 2007-10-25  3:23         ` Matthew Wilcox
  0 siblings, 0 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-25  3:23 UTC (permalink / raw)
  To: Kyle Moffett; +Cc: Linus Torvalds, Andrew Morton, linux-kernel, Matthew Wilcox

On Wed, Oct 24, 2007 at 08:07:23PM -0400, Kyle Moffett wrote:
> No, the problem is what happens when you don't have enough space  
> allocated:  you call "vsnprintf(s, len, format, args);" and then  
> later call "vsprintf(s, format, args);" with the *SAME* "args".   
> That's what's broken.

Ah, gotcha.  I don't really like the va_copy idea, and I don't like the idea of having sb_printf call sb_vprintf twice ... how about just doing away with sb_vprintf altogether:

diff --git a/include/linux/stringbuf.h b/include/linux/stringbuf.h
index c030bc6..ae5c02f 100644
--- a/include/linux/stringbuf.h
+++ b/include/linux/stringbuf.h
@@ -57,11 +57,6 @@ static inline void sb_free(struct stringbuf *sb)
 
 extern void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *fmt, ...)
 	__attribute__((format(printf, 3, 4)));
-#if 0
-/* Waiting for a user to show up */
-extern void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char *fmt,
-			va_list args) __attribute__((format(printf, 3, 0)));
-#endif
 
 /*
  * Convert the stringbuf to a string.  It is the caller's responsibility
diff --git a/lib/stringbuf.c b/lib/stringbuf.c
index b4e59f4..691fcd1 100644
--- a/lib/stringbuf.c
+++ b/lib/stringbuf.c
@@ -22,12 +22,9 @@
 #define INITIAL_SIZE 128
 #define ERR_STRING "out of memory"
 
-/*
- * Not currently used outside this file, but separated from sb_printf
- * for when someone needs it.
- */
-static void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char *format, va_list args)
+void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *format, ...)
 {
+	va_list args;
 	char *s;
 	int size, newlen;
 
@@ -41,7 +38,9 @@ static void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char *format, va_l
 	}
 
 	s = sb->buf + sb->len;
+	va_start(args, format);
 	size = vsnprintf(s, sb->alloc - sb->len, format, args);
+	va_end(args);
 
 	sb->len += size;
 	if (likely(sb->len < sb->alloc))
@@ -61,7 +60,9 @@ static void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char *format, va_l
 
 	/* Point to the end of the old string since we already updated ->len */
 	s += sb->len - size;
+	va_start(args, format);
 	vsprintf(s, format, args);
+	va_end(args);
 	return;
 
  nomem:
@@ -70,16 +71,4 @@ static void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char *format, va_l
 	sb->len = sizeof(ERR_STRING);
 	sb->alloc = -ENOMEM;
 }
-/* When we get our first modular user, delete this comment
-EXPORT_SYMBOL(sb_vprintf);
-*/
-
-void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *format, ...)
-{
-	va_list args;
-
-	va_start(args, format);
-	sb_vprintf(sb, gfp, format, args);
-	va_end(args);
-}
 EXPORT_SYMBOL(sb_printf);

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24 21:21     ` Matthew Wilcox
@ 2007-10-25  0:07       ` Kyle Moffett
  2007-10-25  3:23         ` Matthew Wilcox
  0 siblings, 1 reply; 39+ messages in thread
From: Kyle Moffett @ 2007-10-25  0:07 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, Matthew Wilcox

On Oct 24, 2007, at 17:21:10, Matthew Wilcox wrote:
> On Wed, Oct 24, 2007 at 04:59:48PM -0400, Kyle Moffett wrote:
>> This seems unlikely to work reliably as the various "v*printf"  
>> functions modify the va_list argument they are passed.  It may  
>> happen to work on your particular architecture depending on how  
>> that argument data is passed and stored, but you probably actually  
>> want to make a copy of the varargs list for the first vsnprintf call.
>
> I based what I did on how printk works:
>
>         va_start(args, fmt);
>         r = vprintk(fmt, args);
>         va_end(args);
>
> It doesn't call va_* anywhere else.  I don't claim to be a varargs  
> expert, but if I'm wrong, I'm at least wrong the same way that  
> printk is, so not in any way that's significant for any other  
> architecture Linux runs on.

No, the problem is what happens when you don't have enough space  
allocated:  you call "vsnprintf(s, len, format, args);" and then  
later call "vsprintf(s, format, args);" with the *SAME* "args".   
That's what's broken.

So this is wrong:
> va_list args;
> va_start(args, fmt);
> r1 = vprintk(fmt, args);
> r2 = vprintk(fmt, args);
> va_end(args);

To fix it, you have 2 options.

Option 1:
> va_list args;
> va_start(args, fmt);
> r1 = vprintk(fmt, args);
> va_end(args);
> va_start(args, fmt);
> r2 = vprintk(fmt, args);
> va_end(args);

Option 2:
> va_list args, argscopy;
> va_start(args, fmt);
> va_copy(argscopy, args);
> r1 = vprintk(fmt, argscopy);
> va_end(argscopy);
> r2 = vprintk(fmt, args);
> va_end(args);

Now in a function which *receives* a va_list from one of its callers,  
"Option 1" isn't an option because you don't have the original stack  
frame, so the result looks like this:

> void func1(const char *fmt, ...)
> {
> 	va_list ap;
> 	va_start(ap, fmt);
> 	func2(fmt, ap);
> 	va_end(ap);
> }
>
> void func2(const char *fmt, va_list ap)
> {
> 	va_list ap2;
> 	va_copy(ap2, ap);
> 	vprintk(fmt, ap2);
> 	va_end(ap2);
> 	vprintk(fmt, ap);
> }

Cheers,
Kyle Moffett


^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24 20:59   ` Kyle Moffett
@ 2007-10-24 21:21     ` Matthew Wilcox
  2007-10-25  0:07       ` Kyle Moffett
  0 siblings, 1 reply; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24 21:21 UTC (permalink / raw)
  To: Kyle Moffett; +Cc: Linus Torvalds, Andrew Morton, linux-kernel, Matthew Wilcox

On Wed, Oct 24, 2007 at 04:59:48PM -0400, Kyle Moffett wrote:
> This seems unlikely to work reliably as the various "v*printf"  
> functions modify the va_list argument they are passed.  It may happen  
> to work on your particular architecture depending on how that  
> argument data is passed and stored, but you probably actually want to  
> make a copy of the varargs list for the first vsnprintf call.   

I based what I did on how printk works:

        va_start(args, fmt);
        r = vprintk(fmt, args);
        va_end(args);

It doesn't call va_* anywhere else.  I don't claim to be a varargs expert,
but if I'm wrong, I'm at least wrong the same way that printk is, so not
in any way that's significant for any other architecture Linux runs on.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

^ permalink raw reply	[flat|nested] 39+ messages in thread

* Re: [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24 19:59 ` [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
@ 2007-10-24 20:59   ` Kyle Moffett
  2007-10-24 21:21     ` Matthew Wilcox
  2007-10-26  2:11   ` Rusty Russell
  1 sibling, 1 reply; 39+ messages in thread
From: Kyle Moffett @ 2007-10-24 20:59 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Linus Torvalds, Andrew Morton, linux-kernel, Matthew Wilcox

On Oct 24, 2007, at 15:59:49, Matthew Wilcox wrote:
> +static void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char  
> *format, va_list args)
> +{

[...]

> +	s = sb->buf + sb->len;
> +	size = vsnprintf(s, sb->alloc - sb->len, format, args);

[...]

> +	/* Point to the end of the old string since we already updated - 
> >len */
> +	s += sb->len - size;
> +	vsprintf(s, format, args);

[...]

> +void sb_printf(struct stringbuf *sb, gfp_t gfp, const char  
> *format, ...)
> +{
> +	va_list args;
> +
> +	va_start(args, format);
> +	sb_vprintf(sb, gfp, format, args);
> +	va_end(args);
> +}

This seems unlikely to work reliably as the various "v*printf"  
functions modify the va_list argument they are passed.  It may happen  
to work on your particular architecture depending on how that  
argument data is passed and stored, but you probably actually want to  
make a copy of the varargs list for the first vsnprintf call.   
Example below:

> va_list argscopy;
> va_copy(argscopy, args);
>
> [...]
>
> size = vsnprintf(s, sb->alloc - sb->len, format, argscopy)
>
> [...]
>
> s += sb->len - size;
> vsprintf(s, format, args);
>
> [...]
>
> va_end(argscopy);

Cheers,
Kyle Moffett

^ permalink raw reply	[flat|nested] 39+ messages in thread

* [PATCH 1/4] stringbuf: A string buffer implementation
  2007-10-24 19:58 Stringbuf, v2 Matthew Wilcox
@ 2007-10-24 19:59 ` Matthew Wilcox
  2007-10-24 20:59   ` Kyle Moffett
  2007-10-26  2:11   ` Rusty Russell
  0 siblings, 2 replies; 39+ messages in thread
From: Matthew Wilcox @ 2007-10-24 19:59 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, linux-kernel
  Cc: Matthew Wilcox, Matthew Wilcox

Consecutive calls to printk are non-atomic, which leads to various
implementations for accumulating strings which can be printed in one call.
This is a generic string buffer which can also be used for non-printk
purposes.  There is no sb_scanf implementation yet as I haven't identified
a user for it.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
---
 include/linux/stringbuf.h |   77 ++++++++++++++++++++++++++++++++++++++++
 lib/Makefile              |    2 +-
 lib/stringbuf.c           |   85 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/stringbuf.h
 create mode 100644 lib/stringbuf.c

diff --git a/include/linux/stringbuf.h b/include/linux/stringbuf.h
new file mode 100644
index 0000000..c030bc6
--- /dev/null
+++ b/include/linux/stringbuf.h
@@ -0,0 +1,77 @@
+/*
+ * An auto-expanding string buffer.  There's no sb_alloc because you really
+ * want to allocate it on the stack or embed it in another structure.
+ *
+ * No locking is performed for you; callers are expected to handle locking
+ * themselves if necessary.
+ */
+
+#include <stdarg.h>
+#include <linux/string.h>
+
+/*
+ * 'buf' is the string and may be accessed directly.
+ * 'len' is the number of bytes excluding the NUL terminator.  It may
+ *   also be accessed directly, but note that a freshly-initialised
+ *   stringbuf has a NULL buf, and a len of 0.
+ * 'alloc' is the number of bytes currently allocated to 'buf'.  It
+ *   probably shouldn't be accessed directly.  If you find yourself
+ *   having to, maybe you need to add more functionality to the
+ *   stringbuf code.
+ */
+struct stringbuf {
+	char *buf;
+	int len;
+	int alloc;
+};
+
+/*
+ * Returns a negative errno if an error has been found with this stringbuf,
+ * or 0 if no error has occurred.  If an error has occurred, sb->buf
+ * contains a suitable description of the error
+ */
+static inline int sb_error(struct stringbuf *sb)
+{
+	return sb->alloc < 0 ? sb->alloc : 0;
+}
+
+/*
+ * Initialise a newly-allocated stringbuf
+ */
+static inline void sb_init(struct stringbuf *sb)
+{
+	memset(sb, 0, sizeof(*sb));
+}
+
+/*
+ * Free the contents of a stringbuf, not the stringbuf itself.  The
+ * stringbuf is then reinitialised so it can be reused, or sb_free can
+ * be called on it multiple times.
+ */
+static inline void sb_free(struct stringbuf *sb)
+{
+	if (sb->alloc > 0)
+		kfree(sb->buf);
+	sb_init(sb);
+}
+
+extern void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *fmt, ...)
+	__attribute__((format(printf, 3, 4)));
+#if 0
+/* Waiting for a user to show up */
+extern void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char *fmt,
+			va_list args) __attribute__((format(printf, 3, 0)));
+#endif
+
+/*
+ * Convert the stringbuf to a string.  It is the caller's responsibility
+ * to free the string.  The stringbuf is then ready to be reused.
+ */
+static inline char *sb_to_string(struct stringbuf *sb)
+{
+	char *s = sb->buf;
+	if (sb_error(sb))
+		s = kstrdup(s, GFP_ATOMIC);
+	sb_init(sb);
+	return s;
+}
diff --git a/lib/Makefile b/lib/Makefile
index 3a0983b..f075389 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -14,7 +14,7 @@ lib-$(CONFIG_SMP) += cpumask.o
 lib-y	+= kobject.o kref.o klist.o
 
 obj-y += div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \
-	 bust_spinlocks.o hexdump.o kasprintf.o
+	 bust_spinlocks.o hexdump.o kasprintf.o stringbuf.o
 
 ifeq ($(CONFIG_DEBUG_KOBJECT),y)
 CFLAGS_kobject.o += -DDEBUG
diff --git a/lib/stringbuf.c b/lib/stringbuf.c
new file mode 100644
index 0000000..b4e59f4
--- /dev/null
+++ b/lib/stringbuf.c
@@ -0,0 +1,85 @@
+/*
+ * stringbuf.c
+ *
+ * Copyright (c) 2007 Intel Corporation
+ * By Matthew Wilcox <willy@linux.intel.com>
+ *
+ * Released under the terms of the GNU GPL, version 2.
+ *
+ * A simple automatically expanding string.  I'm not opposed to adding
+ * more functions to this (eg sb_scanf, sb_insert, sb_delete, sb_strchr, etc),
+ * but I have no need for them right now, so I haven't added them.
+ */
+
+#include <stdarg.h>
+#include <linux/compiler.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/stringbuf.h>
+
+#define INITIAL_SIZE 128
+#define ERR_STRING "out of memory"
+
+/*
+ * Not currently used outside this file, but separated from sb_printf
+ * for when someone needs it.
+ */
+static void sb_vprintf(struct stringbuf *sb, gfp_t gfp, const char *format, va_list args)
+{
+	char *s;
+	int size, newlen;
+
+	if (sb->alloc == -ENOMEM)
+		return;
+	if (sb->alloc == 0) {
+		sb->buf = kmalloc(INITIAL_SIZE, gfp);
+		if (!sb->buf) 
+			goto nomem;
+		sb->alloc = INITIAL_SIZE;
+	}
+
+	s = sb->buf + sb->len;
+	size = vsnprintf(s, sb->alloc - sb->len, format, args);
+
+	sb->len += size;
+	if (likely(sb->len < sb->alloc))
+		return;
+
+	/*
+	 * Ensure we always grow the buffer by at least 1.5x.  slab and
+	 * slub already enforce this, but slob will happily allocate
+	 * sizes down to two bytes in granularity
+	 */
+	newlen = max(sb->len + 1, sb->alloc + sb->alloc / 2);
+	s = krealloc(sb->buf, newlen, gfp);
+	if (!s)
+		goto nomem;
+	sb->buf = s;
+	sb->alloc = ksize(s);
+
+	/* Point to the end of the old string since we already updated ->len */
+	s += sb->len - size;
+	vsprintf(s, format, args);
+	return;
+
+ nomem:
+	kfree(sb->buf);
+	sb->buf = ERR_STRING;
+	sb->len = sizeof(ERR_STRING);
+	sb->alloc = -ENOMEM;
+}
+/* When we get our first modular user, delete this comment
+EXPORT_SYMBOL(sb_vprintf);
+*/
+
+void sb_printf(struct stringbuf *sb, gfp_t gfp, const char *format, ...)
+{
+	va_list args;
+
+	va_start(args, format);
+	sb_vprintf(sb, gfp, format, args);
+	va_end(args);
+}
+EXPORT_SYMBOL(sb_printf);
-- 
1.5.3.4


^ permalink raw reply	[flat|nested] 39+ messages in thread

end of thread, other threads:[~2007-10-30 15:26 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-23 21:12 [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
2007-10-23 21:12 ` [PATCH 2/4] isdn: Use stringbuf Matthew Wilcox
2007-10-23 21:12   ` [PATCH 3/4] sound: " Matthew Wilcox
2007-10-23 21:12     ` [PATCH 4/4] partitions: Fix non-atomic printk Matthew Wilcox
2007-10-24 14:18     ` [PATCH 3/4] sound: Use stringbuf Takashi Iwai
2007-10-24 16:01       ` Matthew Wilcox
2007-10-24 14:50         ` Takashi Iwai
2007-10-23 22:11 ` [PATCH 1/4] stringbuf: A string buffer implementation Matt Mackall
2007-10-24  1:49   ` Matthew Wilcox
2007-10-24 15:20     ` Matt Mackall
2007-10-24 15:30       ` Matthew Wilcox
2007-10-23 23:43 ` Linus Torvalds
2007-10-24  2:30   ` Matthew Wilcox
2007-10-24  2:45     ` Andrew Morton
2007-10-24  2:19 ` Eric St-Laurent
2007-10-24  2:35   ` Matthew Wilcox
2007-10-24  2:48     ` Eric St-Laurent
2007-10-24 13:21 ` Florian Weimer
2007-10-24 14:02   ` Matthew Wilcox
2007-10-26 12:05 ` Pekka Enberg
2007-10-27  7:31 ` Pavel Machek
2007-10-30 15:26 ` Denys Vlasenko
2007-10-24 19:58 Stringbuf, v2 Matthew Wilcox
2007-10-24 19:59 ` [PATCH 1/4] stringbuf: A string buffer implementation Matthew Wilcox
2007-10-24 20:59   ` Kyle Moffett
2007-10-24 21:21     ` Matthew Wilcox
2007-10-25  0:07       ` Kyle Moffett
2007-10-25  3:23         ` Matthew Wilcox
2007-10-26  2:11   ` Rusty Russell
2007-10-26  3:41     ` Joe Perches
2007-10-26  5:05       ` Joe Perches
2007-10-26 11:57     ` Matthew Wilcox
2007-10-26 20:57       ` Matt Mackall
2007-10-27 10:09         ` Rusty Russell
2007-10-29  3:03           ` Matt Mackall
2007-10-29  5:38             ` Rusty Russell
2007-10-27 11:47     ` Pekka Enberg
2007-10-27 12:50       ` Rusty Russell
2007-10-27 16:34         ` Pekka Enberg
2007-10-27 16:48           ` Matthew Wilcox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).