Re: [PATCH v2 0/5] rework access to /proc/net/rpc

["David Härdeman" <david@hardeman.nu>]

On 2014-12-09 09:42, Timo Teras wrote:
> On Tue, 09 Dec 2014 09:16:59 +0100
> David Härdeman <david@hardeman.nu> wrote:
> 
>> it seems that the "rework access to /proc/net/rpc" patchset removed
>> dynamic buffers in favour of static, fixed size, buffers. That seems
>> like a step backwards to me?
> 
> Depends a bit on your view. On read() side, readline() like
> functionality is removed yes. Though, my understanding is so that this
> is not needed with the kernel API. Maybe someone can correct me if I'm
> wrong. The removal simplifies memory management, overall code size. As
> probably has a positive impact on speed too (probably not too big, but
> this communication is used all overall, so it might be useful).

And it makes the buffer size static, introducing an arbitrary limitation 
(although a rather large one...32KB allocated on the stack IIRC)

> On write() side the old code was completely wrong. It did several
> assumptions on how FILE buffering works, most of them being incorrect
> in general, but also glibc. It only worked because no large messages
> have been sent to kernel.

I didn't really check the write() side, it was just the readline() that 
I was interested in actually...

> 
>> At least the readline() function could be implemented using
>> read/write (instead of fread/fwrite) and a dynamic buffer...no?
> 
> It's extra complexity. I'd rather not add it unless it's required. My
> understanding about the communication mechanism with kernel is that
> it's not required. Why have code that would never be used?

I agree that it depends on your view. I tend to be very sceptical of 
arbitrary limitations unless they have a very good reason (like 
measurable and relevant performance impact), I doubt that's the case 
here.

It's up to the maintainer though, I just wanted to point it out :)

Regards,
David