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

[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 <rusty@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/