On 28/03/18 22:16, Denis Kenzior wrote:
> Hi Jonas,
>
> On 03/28/2018 01:59 PM, Jonas Bonn wrote:
>> This brings in list.h from the Linux kernel and replaces the GQueues
>> used for enqueuing requests with struct list_heads.  These list
>> operations are both more efficient and generally easier to work with.
>
> In general I agree, but do they really have any significant advantages 
> for our use cases?  Its not like we maintain items on multiple lists...

By the end of the series we have requests on both the device list and on 
the service list.

The biggest advantage is when removing items from the lists that they 
are on.  Since the object on the list knows how it is attached to the 
list (via its list head), it can detach itself from the list without 
need access to the "list object" itself.  With glib lists, a request on 
both a device and service list would need to be able to access the 
device in order to get the device's list in order to be able to ask the 
list to delete it and then do the same for the service.  With the Linux 
lists you just do "detach me" (list_del(node)) and you're done... much 
better code

Aside from that, the Linux lists are really simple macros, not function 
calls; all the state they require is in the list items itself, no extra 
list entry holders required; they are cache friendly; etc.  Lots of 
advantages, though these are mostly moot in ofono where list performance 
isn't really an issue.

Anyway, please bear with these lists for this cleanup.  When the shared 
service cleanup is done, we can return to GQueues if necessary and then 
we'll have a clearer picture of what the difference is.  As things stand 
though, things are complicated enough that the GQueues are just in the way.

Trying to find a series of small patches through this cleanup hasn't 
been easy.  The first attempts I did ended up ballooning because when 
you start trying to make the change in patch 29 you pretty much 
automatically end up with most of the other changes in the preceding 28 
patches before this system is operational again... so the alternative to 
some of these (possibly) oddball patches was one _big_ patch that wasn't 
reviewable.

/Jonas

>
>> Since ofono and Linux share a common license, this code borrowing should
>> not be problematic in any way.
>
> While there's no problem with the license, I really don't want QMI to 
> look completely alien to the rest of the code base.  We use GLib style 
> lists everywhere, so I'm really hesitant to go this way.  I think I'd 
> rather we stuck to that (or switch to ell) for code maintenance & 
> readability reasons.
>
> Regards,
> -Denis