From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030824AbXAZIvS (ORCPT ); Fri, 26 Jan 2007 03:51:18 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1030830AbXAZIvS (ORCPT ); Fri, 26 Jan 2007 03:51:18 -0500 Received: from amsfep17-int.chello.nl ([213.46.243.15]:45045 "EHLO amsfep12-int.chello.nl" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1030824AbXAZIvS (ORCPT ); Fri, 26 Jan 2007 03:51:18 -0500 Subject: Re: [PATCH] nfs: fix congestion control -v4 From: Peter Zijlstra To: Andrew Morton Cc: Trond Myklebust , Christoph Lameter , linux-kernel@vger.kernel.org, linux-mm@kvack.org, pj@sgi.com In-Reply-To: <1169798412.6189.79.camel@twins> References: <20070116054743.15358.77287.sendpatchset@schroedinger.engr.sgi.com> <20070116135325.3441f62b.akpm@osdl.org> <1168985323.5975.53.camel@lappy> <1169070763.5975.70.camel@lappy> <1169070886.6523.8.camel@lade.trondhjem.org> <1169126868.6197.55.camel@twins> <1169135375.6105.15.camel@lade.trondhjem.org> <1169199234.6197.129.camel@twins> <1169212022.6197.148.camel@twins> <1169229461.6197.154.camel@twins> <1169231212.5775.29.camel@lade.trondhjem.org> <1169276500.6197.159.camel@twins> <1169482343.6083.7.camel@lade.trondhjem.org> <1169739148.6189.68.camel@twins> <20070125210245.3fb0e30e.akpm@osdl.org> <1169798412.6189.79.camel@twins> Content-Type: text/plain Date: Fri, 26 Jan 2007 09:50:34 +0100 Message-Id: <1169801434.6189.85.camel@twins> Mime-Version: 1.0 X-Mailer: Evolution 2.8.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2007-01-26 at 09:00 +0100, Peter Zijlstra wrote: > On Thu, 2007-01-25 at 21:02 -0800, Andrew Morton wrote: > > On Thu, 25 Jan 2007 16:32:28 +0100 > > Peter Zijlstra wrote: > > > > > +long congestion_wait_interruptible(int rw, long timeout) > > > +{ > > > + long ret; > > > + DEFINE_WAIT(wait); > > > + wait_queue_head_t *wqh = &congestion_wqh[rw]; > > > + > > > + prepare_to_wait(wqh, &wait, TASK_INTERRUPTIBLE); > > > + if (signal_pending(current)) > > > + ret = -ERESTARTSYS; > > > + else > > > + ret = io_schedule_timeout(timeout); > > > + finish_wait(wqh, &wait); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL(congestion_wait_interruptible); > > > > I think this can share code with congestion_wait()? > > > > static long __congestion_wait(int rw, long timeout, int state) > > { > > long ret; > > DEFINE_WAIT(wait); > > wait_queue_head_t *wqh = &congestion_wqh[rw]; > > > > prepare_to_wait(wqh, &wait, state); > > ret = io_schedule_timeout(timeout); > > finish_wait(wqh, &wait); > > return ret; > > } > > > > long congestion_wait_interruptible(int rw, long timeout) > > { > > long ret = __congestion_wait(rw, timeout); > > > > if (signal_pending(current)) > > ret = -ERESTARTSYS; > > return ret; > > } > > > > it's only infinitesimally less efficient.. > > All the other _interruptible functions check signal_pending before > calling schedule. Which seems to make sense since its called in a loop > anyway, and if the loop condition turns false when interrupted you might > as well just finish up instead of bailing out. > > However if you'd rather see your version, who am I to object ;-) ok, first wake up, then reply to emails :-) How about this: long congestion_wait_interruptible(int rw, long timeout) { long ret; if (signal_pending(current)) ret = -ERESTARTSYS; else ret = congestion_wait(rw, timeout); return ret; } EXPORT_SYMBOL(congestion_wait_interruptible); From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: Re: [PATCH] nfs: fix congestion control -v4 From: Peter Zijlstra In-Reply-To: <1169798412.6189.79.camel@twins> References: <20070116054743.15358.77287.sendpatchset@schroedinger.engr.sgi.com> <20070116135325.3441f62b.akpm@osdl.org> <1168985323.5975.53.camel@lappy> <1169070763.5975.70.camel@lappy> <1169070886.6523.8.camel@lade.trondhjem.org> <1169126868.6197.55.camel@twins> <1169135375.6105.15.camel@lade.trondhjem.org> <1169199234.6197.129.camel@twins> <1169212022.6197.148.camel@twins> <1169229461.6197.154.camel@twins> <1169231212.5775.29.camel@lade.trondhjem.org> <1169276500.6197.159.camel@twins> <1169482343.6083.7.camel@lade.trondhjem.org> <1169739148.6189.68.camel@twins> <20070125210245.3fb0e30e.akpm@osdl.org> <1169798412.6189.79.camel@twins> Content-Type: text/plain Date: Fri, 26 Jan 2007 09:50:34 +0100 Message-Id: <1169801434.6189.85.camel@twins> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org Return-Path: To: Andrew Morton Cc: Trond Myklebust , Christoph Lameter , linux-kernel@vger.kernel.org, linux-mm@kvack.org, pj@sgi.com List-ID: On Fri, 2007-01-26 at 09:00 +0100, Peter Zijlstra wrote: > On Thu, 2007-01-25 at 21:02 -0800, Andrew Morton wrote: > > On Thu, 25 Jan 2007 16:32:28 +0100 > > Peter Zijlstra wrote: > > > > > +long congestion_wait_interruptible(int rw, long timeout) > > > +{ > > > + long ret; > > > + DEFINE_WAIT(wait); > > > + wait_queue_head_t *wqh = &congestion_wqh[rw]; > > > + > > > + prepare_to_wait(wqh, &wait, TASK_INTERRUPTIBLE); > > > + if (signal_pending(current)) > > > + ret = -ERESTARTSYS; > > > + else > > > + ret = io_schedule_timeout(timeout); > > > + finish_wait(wqh, &wait); > > > + return ret; > > > +} > > > +EXPORT_SYMBOL(congestion_wait_interruptible); > > > > I think this can share code with congestion_wait()? > > > > static long __congestion_wait(int rw, long timeout, int state) > > { > > long ret; > > DEFINE_WAIT(wait); > > wait_queue_head_t *wqh = &congestion_wqh[rw]; > > > > prepare_to_wait(wqh, &wait, state); > > ret = io_schedule_timeout(timeout); > > finish_wait(wqh, &wait); > > return ret; > > } > > > > long congestion_wait_interruptible(int rw, long timeout) > > { > > long ret = __congestion_wait(rw, timeout); > > > > if (signal_pending(current)) > > ret = -ERESTARTSYS; > > return ret; > > } > > > > it's only infinitesimally less efficient.. > > All the other _interruptible functions check signal_pending before > calling schedule. Which seems to make sense since its called in a loop > anyway, and if the loop condition turns false when interrupted you might > as well just finish up instead of bailing out. > > However if you'd rather see your version, who am I to object ;-) ok, first wake up, then reply to emails :-) How about this: long congestion_wait_interruptible(int rw, long timeout) { long ret; if (signal_pending(current)) ret = -ERESTARTSYS; else ret = congestion_wait(rw, timeout); return ret; } EXPORT_SYMBOL(congestion_wait_interruptible); -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org