xfs
[Top] [All Lists]

Re: [xfs-masters] [patch 2/2] fs/xfs_super: replace schedule_timeout() w

To: Nishanth Aravamudan <nacc@xxxxxxxxxx>
Subject: Re: [xfs-masters] [patch 2/2] fs/xfs_super: replace schedule_timeout() with msleep_interruptible()
From: Nathan Scott <nathans@xxxxxxx>
Date: Tue, 8 Mar 2005 11:45:26 +1100
Cc: domen@xxxxxxxxxxxx, linux-xfs@xxxxxxxxxxx
In-reply-to: <20050308002031.GG2778@us.ibm.com>; from nacc@us.ibm.com on Mon, Mar 07, 2005 at 04:20:31PM -0800
References: <20050306104000.A71391F204@trashy.coderock.org> <20050308000114.GG720@frodo> <20050308002031.GG2778@us.ibm.com>
Sender: linux-xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.2.5i
On Mon, Mar 07, 2005 at 04:20:31PM -0800, Nishanth Aravamudan wrote:
> On Tue, Mar 08, 2005 at 11:01:14AM +1100, Nathan Scott wrote:
> > On Sun, Mar 06, 2005 at 11:40:00AM +0100, domen@xxxxxxxxxxxx wrote:
> > > 
> > > Use msleep_interruptible() instead of schedule_timeout(). The
> > > current code is not wrong; however a change to msleep_interruptible() has 
> > > two
> > > major benefits: 1) consistency across the kernel and 2) uses 
> > > human-sensible time
> > > units (msecs). Change the units of timeleft appropriately to msecs.
> > > ...
> > > - timeleft = (xfs_syncd_centisecs * HZ) / 100;
> > > + timeleft = xfs_syncd_centisecs * 10;
> > 
> > Hmm... can you explain that a bit more?  These are already in
> > "human-sensible" units - centisecs (ala. pdflush), your patch
> > seems to break this, and changes a user-visible interface too.
> 
> While youre interface may be in human-sensible units, the internal timer
> subsystem is not (jiffies only exist in the kernel).
> msleep_interruptible() changes this, clearly. My patch doesn't really
> change anything. It really shouldn't really result in any different
> behavior as far as I can tell. msleep_interruptible() takes a timeout
> value in milliseconds, which is 10 times the number of centiseconds
> specified.
> 
> Does that clear things up?

Certainly does, the patches look fine then; I'll test them for awhile
then get them merged in.

thanks.

-- 
Nathan


<Prev in Thread] Current Thread [Next in Thread>