All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Mc Guire <der.herr@hofr.at>
To: Wolfram Sang <wsa@the-dreams.de>
Cc: Nicholas Mc Guire <hofrat@osadl.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Wolfram Sang <wsa+renesas@sang-engineering.com>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: ks7010: fix wait_for_completion_interruptible_timeout return handling
Date: Tue, 26 Jul 2016 06:43:34 +0000	[thread overview]
Message-ID: <20160726064334.GA30333@osadl.at> (raw)
In-Reply-To: <20160725205402.GC1698@katana>

On Mon, Jul 25, 2016 at 10:54:03PM +0200, Wolfram Sang wrote:
> On Mon, Jul 25, 2016 at 09:21:50PM +0200, Nicholas Mc Guire wrote:
> > wait_for_completion_interruptible_timeout return 0 on timeout and 
> > -ERESTARTSYS if interrupted. The check for 
> > !wait_for_completion_interruptible_timeout() would report an interrupt
> > as timeout. Further, while HZ/50 will work most of the time it could 
> 
> Wouldn't it interpret -ERESTARTSYS as *no timeout*?
>

yup - actually the current code just treats the -ERESTARTSYS 
case as success.
 
> Anyway, the plain !0 comparison for me clearly shows that
> 'interruptible' was more copy&pasted then really planned or supported.
> If it was, it would need to cancel something. Also, 20ms is pretty hard
> to cancel for a user ;) Given all that and the troubles we had with
> 'interruptible' in the I2C subsystem, I'd much vote for dropping
> interruptible here.
> 
> > fail for HZ < 50, so this is switched to msecs_to_jiffies(20).
> 
> Rest looks good, thanks!
> 

thx!
hofrat

      reply	other threads:[~2016-07-26  6:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-25 19:21 [PATCH] staging: ks7010: fix wait_for_completion_interruptible_timeout return handling Nicholas Mc Guire
2016-07-25 20:54 ` Wolfram Sang
2016-07-26  6:43   ` Nicholas Mc Guire [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160726064334.GA30333@osadl.at \
    --to=der.herr@hofr.at \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hofrat@osadl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wsa+renesas@sang-engineering.com \
    --cc=wsa@the-dreams.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.