linux-kernel - Re: [PATCH] spi subsystem: destroy the spi_bitbang workqueue only after the spi master is unregistered

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <200703082219.33296.david-b@pacbell.net>
Date:	Thu, 8 Mar 2007 22:19:32 -0800
From:	David Brownell <david-b@...bell.net>
To:	Chris Lesiak <chris.lesiak@...or.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi subsystem: destroy the spi_bitbang workqueue only after the spi master is unregistered

On Wednesday 07 March 2007 3:48 pm, Chris Lesiak wrote:
> From: Chris Lesiak <chris.lesiak@...or.com>
> 
> This patch fixes a bug in the cleanup of an spi_bitbang bus. 

It's nearly right, but see below.


> @@ -505,28 +499,10 @@ EXPORT_SYMBOL_GPL(spi_bitbang_start);
>   */
>  int spi_bitbang_stop(struct spi_bitbang *bitbang)
>  {
> -	unsigned	limit = 500;
> -
> -	spin_lock_irq(&bitbang->lock);
> -	bitbang->shutdown = 0;
> -	while (!list_empty(&bitbang->queue) && limit--) {
> -		spin_unlock_irq(&bitbang->lock);
> -
> -		dev_dbg(bitbang->master->cdev.dev, "wait for queue\n");
> -		msleep(10);
> -
> -		spin_lock_irq(&bitbang->lock);
> -	}

You completely removed an odious busy-wait, which is good ...

> -	spin_unlock_irq(&bitbang->lock);
> -	if (!list_empty(&bitbang->queue)) {
> -		dev_err(bitbang->master->cdev.dev, "queue didn't empty\n");
> -		return -EBUSY;
> -	}
> +	spi_unregister_master(bitbang->master);

... but right here there should be a WARN_ON(!list_empty(...)) to
flag the corresponding bogosity:  that somehow a request never
completed.

>  
>  	destroy_workqueue(bitbang->workqueue);
>  
> -	spi_unregister_master(bitbang->master);
> -
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(spi_bitbang_stop);
> 
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/