linux-kernel - Re: [PATCH v3] iio: adc: exynos5

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: <5140C816.3090008@metafoo.de>
Date:	Wed, 13 Mar 2013 19:40:22 +0100
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Doug Anderson <dianders@...omium.org>
CC:	Naveen Krishna Chatradhi <ch.naveen@...sung.com>,
	linux-iio <linux-iio@...r.kernel.org>, dan.carpenter@...cle.com,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-samsung-soc@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Naveen Krishna <naveenkrishna.ch@...il.com>
Subject: Re: [PATCH v3] iio: adc: exynos5_adc: fix compilation warnings

On 03/13/2013 07:35 PM, Lars-Peter Clausen wrote:
> On 03/13/2013 07:23 PM, Doug Anderson wrote:
>> Lars,
>>
>> On Wed, Mar 13, 2013 at 11:11 AM, Lars-Peter Clausen <lars@...afoo.de> wrote:
>>> Agreed. Adding the dependency on OF in Kconfig should be all that is needed.
>>
>> I think changing the timeout from 'unsigned long' to 'long' is also
>> legit (to match the actual type returned) and a good idea.
>>
>> -Doug
> 
> Yes, but that's a different issue and to be honest I didn't even realize
> that the patch was trying to fix this as well. In my opinion it's best to
> split this up into two patches one which fixes the OF dependency. The other
> fixing the timeout issue. Cause there is more to it than just changing the type.
> 
> wait_for_completion_interruptible_timeout() may return
> 	1) 0, if there was a timeout, waiting for the completion
> 	2) > 0, if the completion was completeted, the returned value
> 	   the  remaining time.
> 	3) < 0, If it was interrupt while waiting for the completion
> 
> The code currently only handles 1) and 2), but it also needs to handle 3).
> Since the completion has not been completed in case 3.
> 
> E.g. something like this should work:
> 
> 	if (timeout == 0)
> 		return -ETIMEDOUT;
> 	else if(timeout < 0)
> 		return timeout;
> 	return 0;
> 

I just saw, there is another issue related to this. The driver should call
INIT_COMPLETION(&info->completion) before starting the conversion. Otherwise
there may be a problem if we got interrupted while waiting for the
interrupt. E.g. imagine the following sequence.

1) Start conversion
2) Wait for completion
3) Abort waiting
4) Interrupt kicks in and marks the completion as done

Now if another conversion is started the completion will already be
completed and wait_for_completion_interruptible_timeout() will return right
away without waiting for the conversion to be finished.

- Lars
--
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/