On 2017-06-07 15:44, Felix Fietkau wrote:
> On 2017-06-07 13:09, Alexandru Ardelean wrote:
>> It's not very often that the tx_queue is used,
>> to store backlog messages to send to a client.
>>
>> And for most cases, 32 backlog messages seems to be enough.
>> In fact, for most cases, I've seen ~1 entry in the queue
>> being used every now-n-then.
>>
>> The issue is more visible/present with the `ubus list` command.
>> I've traced the code to ubusd_handle_lookup() in:
>>
>> ```
>> if (!attr[UBUS_ATTR_OBJPATH]) {
>> avl_for_each_element(&path, obj, path)
>> ubusd_send_obj(cl, ub, obj);
>> return 0;
>> }
>> ```
>> The code-path eventually leads to `ubus_msg_send()`.
>> It seems that once the first element is queued, then
>> the condition check for `(!cl->tx_queue[cl->txq_cur])`
>> will queue all messages iterated in the above snippet,
>> without trying any writes.
>>
>> This can be solved, either by making the queue dynamic
>> and allow it to expand above the current fixed limit (1).
>>
>> Or, by forcing/allowing writes during the tx_queue-ing (2).
>>
>> This patch implements (1).
>>
>> Signed-off-by: Alexandru Ardelean <ardeleana...@gmail.com>
> If I remember correctly, I chose the backlog array because a message can
> be referenced multiple times (e.g. in the notify/subscribe case).
> This would break in your linked-list implementation if a message needs
> to be queued for multiple clients.
I just re-checked and it seems that multiple references are no longer
used, so this implementation would probably work. I will take a closer look.
- Felix
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
