login
Search
Incorporated all feedback received: - Explicitly allow people to dereference sb->buf and sb->len, and include comments about the potential problems with doing so. Document that alloc is probably better left unreferenced. Remove sb_len() and sb_string(). - Ensure that we grow by at least 1.5x, even if we're using slob. - Start by allocating 128 bytes - Pass a gfp_t to sb_printk - Drop the bogus change to snd_ac97_proc_regs_write - Add stringbuf.o to obj-y instead of lib-y, otherwise a fully modular kernel might not pull in 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." -
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 ...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 ...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 | 8 ++- 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 ...
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 | 7 ++++++-
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, 73 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, ...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. Cheers, Kyle Moffett -
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."
-
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. To fix it, you have 2 options. 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 Cheers, Kyle Moffett -
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 ...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 ...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);
-
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);
-
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 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." -
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. -
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.
-
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. -
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] != ...Hi Rusty,
FWIW I like this patch better.
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
-
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 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. -
Hi Rusty,
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.
Yeah, makes sense. Thanks.
Pekka
-
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."
-
sb_printf As it doesn't actually output anything but extends a buffer pointer in a struct, perhaps it's better to rename sb_printf to sb_sprintf. I believe the common use will be GFP_ATOMIC. Perhaps a default wrapper define and a renamed function? void sb_sprintf_gfp(struct stringbuf *sb, gfp_t gfp, const char *fmt, ...) #define sb_sprintf(sb, fmt, arg...) \ sb_sprintf_gfp(sb, GFP_ATOMIC, fmt, ##arg) -
printf prints to stdout. fprintf prints to a file. sprintf prints to An interesting belief. Of the three users I've converted, one uses If you look at the patches, you'll see that they basically all have a wrapper around sb_printf that I was able to insert the GFP argument into, so I don't see this as a win. I hadn't bothered with one for ISDN, and that one involved much more editing. -- 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." -
I was hoping that each module/subsystem would not need a separate wrapper and could use the plain sb_printf function. -
Then they should carefully think about whether they need GFP_ATOMIC or GFP_KERNEL. Just like we don't have a kalloc() that uses GFP_ATOMIC to spare people from having to think about it. ISDN and sound had done that thinking for me -- one knew they needed GFP_ATOMIC, the other needed GFP_KERNEL. I was able to work out that GFP_KERNEL made sense for the partition code. -- 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." -
