On 12/09/2014 09:01 AM, David Härdeman wrote: > On 2014-12-09 09:42, Timo Teras wrote: >> On Tue, 09 Dec 2014 09:16:59 +0100 >> David Härdeman <david@xxxxxxxxxxx> 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. Your skeptical-ability of arbitrary limitations has become very clear in the last few hours... ;-) I guess I'm indifferent about it... From reading your gssd patch set, it is a bit more artful not to use fixed size buffers but again, I'm indifferent... That said... if patches appear removing these fixed buffers they definitely would be considered... > > It's up to the maintainer though, I just wanted to point it out :) My understanding these patches were needed to make nfs-utils compatible with the musl c-library. That is the case, correct? steved. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
