On 20/12/16 10:28, Johan Hovold wrote:
> On Tue, Dec 20, 2016 at 01:12:08AM +0000, Bryan O'Donoghue wrote:
>> Currently the greybus-loopback thread logic spins around waiting for
>> send_count == iteration_max which on real hardware doesn't make a
>> difference to us but in simulation is excruciatingly slow, anti-social and
>> bad manners. Use the existing gb_loopback_async_wait_all() function to gate
>> continuing when the send_count == iteration_max and go to sleep until
>> there's something worthwhile to-do.
>>
>> Signed-off-by: Bryan O'Donoghue <pure.logic@xxxxxxxxxxxxxxxxx>
>
> You forgot to CC me and Alex.
>
>> ---
>> drivers/staging/greybus/loopback.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/staging/greybus/loopback.c b/drivers/staging/greybus/loopback.c
>> index 7882306..d6302ef 100644
>> --- a/drivers/staging/greybus/loopback.c
>> +++ b/drivers/staging/greybus/loopback.c
>> @@ -1008,11 +1008,23 @@ static int gb_loopback_fn(void *data)
>>
>> /* Optionally terminate */
>> if (gb->send_count == gb->iteration_max) {
>> + mutex_unlock(&gb->mutex);
>> +
>> + /* Wait for synchronous and asynchronus completion */
>> + gb_loopback_async_wait_all(gb);
>
> This introduces a non-interruptible wait here, which should be ok given
> that all outstanding operations will be cancelled by core as part of
> connection disable before stopping the kthread during disconnect.
>
> But this driver is full of such subtleties and really needs a clean up
> (or rewrite).
A rewrite is on the bucket list at some point.
>> +
>> + /* Mark complete unless user-space has poked us */
>> + mutex_lock(&gb->mutex);
>> if (gb->iteration_count == gb->iteration_max) {
>> gb->type = 0;
>> gb->send_count = 0;
>> sysfs_notify(&gb->dev->kobj, NULL,
>> "iteration_count");
>> + dev_dbg(&gb->connection->bundle->dev,
>
> You have a pointer to the bundle you should use here and below.
Will do.
>
>> + "load test complete\n");
>> + } else {
>> + dev_dbg(&gb->connection->bundle->dev,
>> + "continuing on with new test set\n");
>
> The fact that user-space can reset test counters and change parameters
> while a test is running is really another bug.
Agreed will be done on the rewrite.
---
bod
_______________________________________________
greybus-dev mailing list
greybus-dev@xxxxxxxxxxxxxxxx
https://lists.linaro.org/mailman/listinfo/greybus-dev
|