Hi Denis, On Tue, 2011-03-01 at 21:47 -0600, Denis Kenzior wrote: > This is a great description, but right after reading it one should > realize that this patch is better broken down into two. The first patch > addressing the ringbuffer performance improvements and the second one > dealing with PPP/HDLC queuing. In fact, if there were two patches, then > ringbuffer changes could go in right away.
Fair enough ; I'll separate the changes. > Please try to avoid going over 80 characters a line. Sometimes it is > unavoidable, but lets try very hard to avoid it. Understood. > Is there a particular reason why you chose to use a ring buffer of ring > buffers? A simple GQueue might be much easier to understand. If you are > worried about 'infinite queuing' then a simple counter might help to > alleviate that. Well, this implementation opens the door to further optimizations, such as reusing cells (buffers) to avoid frequent allocation/deallocation. > You change BUFFER_SIZE from 2048 to 4096 and remove the multiplication > here. It is a good idea to send these types of changes in a separate > patch for two reasons: > - Logically they should be separate > - It is much easier to review for correctness outside the context of a > large patch Got it. By the way, why is it written this way? Is it to stress that we want space for two frames? > Please watch out for space / tab indentation mixups. OK. I'll fix that. > So I think we have to be a bit careful here. HDLC framing can in theory > (if you're maximally unlucky) result in doubling of the data size once > it is framed. This means that we might have enough space in the current > buffer according to this estimate, but still exceed it once the actual > framing is performed. If so, then we have to drop the frame. > > There are two possibilities: > - Retry again with a full buffer this time > - Always pick a buffer if we have less than 2x size available > Thanks for the insight. Picking a new buffer may be best. That will cause more buffer "run away" though, strengthening the case for better buffer recycling strategies. Regards, -- Patrick _______________________________________________ ofono mailing list [email protected] http://lists.ofono.org/listinfo/ofono
