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

[Rusty Russell <rusty@rustcorp.com.au>]

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.