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

Rusty Russell
2007-10-27T12:50:11Z

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.