On 7 March 2017 at 02:32, David Gwynne <[email protected]> wrote: > >> On 2 Mar 2017, at 21:19, Mike Belopuhov <[email protected]> wrote: >> >> On Thu, Mar 02, 2017 at 10:11 +0100, Martin Pieuchot wrote: >>> On 02/03/17(Thu) 01:16, Mike Belopuhov wrote: >>>> On 2 March 2017 at 00:56, David Gwynne <[email protected]> wrote: >>>>> >>>>>> On 2 Mar 2017, at 06:43, Mike Belopuhov <[email protected]> wrote: >>>>>> >>>>>> This convers hand rolled lists into exactly the same mbuf_lists. >>>>>> I need this because of the next diff that uses the ml_len packet >>>>>> counter that mbuf_lists have. Otherwise there's no functional >>>>>> change. >>>>> >>>>> i didnt use mbuf lists here because they have an extra counter >>>>> that isnt, or wasnt, needed. >>>>> >>>>> im not sure you need to know how long a list is in your later >>>>> diff, you just need to know if it is not empty. you can do that >>>>> by checking if the head is NULL. >>>>> >>>> >>>> true, i was thinking about clarifying this, but i like the >>>> "declarativeness" of the length check. i don't think that an >>>> extra counter is a big deal. also mbuf lists make this code look >>>> simpler which is a good thing, imo. >>> >>> I agree. I find the code much easier to understand with mikeb@'s diff. >>> >>> IMHO we should avoid hand-rolled lists. >>> >>> ok mpi@ >>> >> >> Here's an updated diff with suggestions from bluhm@ and it's >> even slimmer now but I don't mind either way. If David isn't >> comfortable with this change, we don't have to do it. > > im really not keen, but i guess i will submit to populism if > everyone else feels strongly about this. >
I've pondered it a bit more and while I didn't think I care too much, while rebasing I've realised that I'd be doing a disservice for everyone coming later to this code. What tipped the scales was how you used priq_deq_commit in the last diff for the lack of better API and I thought hell, there is a better API: mbuf_lists. It's just much more fitting. You've even invented it yourself! And besides I'm of opinion that priq_deq_begin and priq_deq_commit existence is purely accidental and they should be merged and drivers using them are doing it wrong from the QoS point of view (albeit not so much from the priq PoV).
