Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-30 Thread Denys Vlasenko
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-28 Thread Pavel Machek
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-28 Thread Matt Mackall
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-28 Thread Rusty Russell
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 25 bytes of memcpy. Ouch! In other words, the system was compiled for size optimization and that's what happened. The question is:

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-27 Thread Rusty Russell
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-27 Thread Pekka Enberg
Hi Rusty, On 10/26/07, Rusty Russell [EMAIL PROTECTED] 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-27 Thread Rusty Russell
On Saturday 27 October 2007 21:47:09 Pekka Enberg wrote: Hi Rusty, Hi Pekka, On 10/26/07, Rusty Russell [EMAIL PROTECTED] wrote: How about this? It's as simple as I could make it... FWIW I like this patch better. Thanks. + kfree(oldsb); + *sb = (struct

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-27 Thread Pekka Enberg
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 [EMAIL PROTECTED] wrote:

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-27 Thread Matthew Wilcox
On Sat, Oct 27, 2007 at 07:34:47PM +0300, Pekka Enberg wrote: On 10/27/07, Rusty Russell [EMAIL PROTECTED] 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-26 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-26 Thread Pekka Enberg
Hi, On 10/24/07, Matthew Wilcox [EMAIL PROTECTED] 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 =

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-26 Thread Matt Mackall
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-25 Thread Rusty Russell
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-25 Thread Joe Perches
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-25 Thread Joe Perches
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) sblen =

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread Kyle Moffett
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread Florian Weimer
* 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). - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread Matt Mackall
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread 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?

[PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread Kyle Moffett
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-24 Thread 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

[PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread Matt Mackall
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread Linus Torvalds
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.

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread Eric St-Laurent
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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread Andrew Morton
On Tue, 23 Oct 2007 20:30:35 -0600 Matthew Wilcox [EMAIL PROTECTED] 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

Re: [PATCH 1/4] stringbuf: A string buffer implementation

2007-10-23 Thread Eric St-Laurent
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