From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: Regression with wait_event_timeout in next-20140226 Date: Wed, 9 Apr 2014 12:05:14 -0700 Message-ID: <20140409120514.da292ccfd5530a995090228d@linux-foundation.org> References: <530E17C7.1030504@free-electrons.com> <20140226165043.GA22802@laptop.programming.kicks-ass.net> <20140226142534.3b8c6eb1652d000206948ae0@linux-foundation.org> <20140409111638.GS11096@twins.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Return-path: Received: from mail.linuxfoundation.org ([140.211.169.12]:37375 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933480AbaDITFP (ORCPT ); Wed, 9 Apr 2014 15:05:15 -0400 In-Reply-To: <20140409111638.GS11096@twins.programming.kicks-ass.net> Sender: linux-next-owner@vger.kernel.org List-ID: To: Peter Zijlstra Cc: Gregory CLEMENT , "linux-kernel@vger.kernel.org" , linux-next@vger.kernel.org, Johannes Berg , Steven Rostedt On Wed, 9 Apr 2014 13:16:38 +0200 Peter Zijlstra wrote: > On Wed, Feb 26, 2014 at 02:25:34PM -0800, Andrew Morton wrote: > > Is there anything we can do to make all this clearer? Simply using a > > distinctive variable name ("__wait_var__"?) in place of __ret (and > > documenting it) would help a lot. > > > > Some __ret's are long and some are int. Maybe that's a glitch, maybe > > it's because some __ret's are used for inter-macro communications and > > some are not, which just makes things worse. > > > > I started to do a patch, got all confused and gave up. We've made > > quite a tangly mess in there, alas. > > Something like so? > > --- > Subject: wait: Explain the shadowing and type inconsistencies > From: Peter Zijlstra > Date: Wed Apr 9 12:50:34 CEST 2014 > > Stick in a comment before someone else tries to fix the sparse warning > this generates. > > Requested-by: Andrew Morton > Signed-off-by: Peter Zijlstra > Link: http://lkml.kernel.org/n/tip-o2ro6f3vkxklni0bc8f7m68s@git.kernel.org > --- > include/linux/wait.h | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > --- a/include/linux/wait.h > +++ b/include/linux/wait.h > @@ -191,11 +191,23 @@ wait_queue_head_t *bit_waitqueue(void *, > (!__builtin_constant_p(state) || \ > state == TASK_INTERRUPTIBLE || state == TASK_KILLABLE) \ > > +/* > + * The below macro ___wait_event() has an explicit shadow of the __ret > + * variable when used from the wait_event_*() macros. > + * > + * This is so that both can use the ___wait_cond_timeout() construct > + * to wrap the condition. > + * > + * The type inconsistency of the wait_event_*() __ret variable is also > + * on purpose; we use long where we can return timeout values and int > + * otherwise. > + */ > + > #define ___wait_event(wq, condition, state, exclusive, ret, cmd) \ > ({ \ > __label__ __out; \ > wait_queue_t __wait; \ > - long __ret = ret; \ > + long __ret = ret; /* explicit shadow */ \ > \ > INIT_LIST_HEAD(&__wait.task_list); \ > if (exclusive) \ Looks nice to me, thanks.