linux-kernel - Re: [PATCH 18/25] TMIO MMC: don't use [delayed_]work

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: <Pine.LNX.4.64.1212242314420.31888@axis700.grange>
Date:	Mon, 24 Dec 2012 23:31:05 +0100 (CET)
From:	Guennadi Liakhovetski <g.liakhovetski@....de>
To:	Tejun Heo <tj@...nel.org>
cc:	linux-kernel@...r.kernel.org, Ian Molton <ian@...menth.co.uk>,
	linux-mmc@...r.kernel.org
Subject: Re: [PATCH 18/25] TMIO MMC: don't use [delayed_]work_pending()

Hi Tejun

On Fri, 21 Dec 2012, Tejun Heo wrote:

> There's no need to test whether a (delayed) work item in pending
> before queueing, flushing or cancelling it.  Most uses are unnecessary
> and quite a few of them are buggy.
> 
> Remove unnecessary pending tests from tmio mmc.  Only compile tested.

Thanks for the patch. It looks good to me. What happens below is upon mmc 
card detection IRQs, of which there are typically several due to pin 
debouncing, a delayed work is scheduled to verify a possibly new card 
state (plugged in or removed). So, the code below checks, that after the 
first of those interrupts the work is already scheduled and doesn't 
schedule it again. IIUC, this is indeed unneeded, since 
queue_delayed_work_on() only has any effect if the work isn't queued yet. 
Ah, there's a nitpick - you can remove one pair of parenthesis;-) 
Otherwise, I guess, this is an

Acked-by: Guennadi Liakhovetski <g.liakhovetski@....de>

and Chris can take it (preferably, after the correction) via his queue. If 
you give me a bit more time I can also test this, but it really looks 
pretty obvious. Maybe Chris will prefer you to take this via your queue - 
that's up to you two to decide.

Thanks
Guennadi

> 
> Signed-off-by: Tejun Heo <tj@...nel.org>
> Cc: Guennadi Liakhovetski <g.liakhovetski@....de>
> Cc: Ian Molton <ian@...menth.co.uk>
> Cc: linux-mmc@...r.kernel.org
> ---
> Please let me know how this patch should be routed.  I can take it
> through the workqueue tree if necessary.
> 
> Thanks.
> 
>  drivers/mmc/host/tmio_mmc_pio.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 50bf495..f4f18b3 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -573,8 +573,7 @@ static bool __tmio_mmc_card_detect_irq(struct tmio_mmc_host *host,
>  		tmio_mmc_ack_mmc_irqs(host, TMIO_STAT_CARD_INSERT |
>  			TMIO_STAT_CARD_REMOVE);
>  		if ((((ireg & TMIO_STAT_CARD_REMOVE) && mmc->card) ||
> -		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)) &&
> -		    !work_pending(&mmc->detect.work))
> +		     ((ireg & TMIO_STAT_CARD_INSERT) && !mmc->card)))
>  			mmc_detect_change(host->mmc, msecs_to_jiffies(100));
>  		return true;
>  	}
> -- 
> 1.8.0.2
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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/