Re: [PATCH 1/2] staging: greybus: loopback: use gb_loopback_async_wait_all don't spin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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).

> +
> +			/* 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.

> +					"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.

>  			}
>  			mutex_unlock(&gb->mutex);
>  			continue;

Johan
_______________________________________________
greybus-dev mailing list
greybus-dev@xxxxxxxxxxxxxxxx
https://lists.linaro.org/mailman/listinfo/greybus-dev




[Index of Archives]     [Asterisk App Development]     [PJ SIP]     [Gnu Gatekeeper]     [IETF Sipping]     [Info Cyrus]     [ALSA User]     [Fedora Linux Users]     [Linux SCTP]     [DCCP]     [Gimp]     [Yosemite News]     [Deep Creek Hot Springs]     [Yosemite Campsites]     [ISDN Cause Codes]     [Asterisk Books]

  Powered by Linux