From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756376AbZDAAvs (ORCPT ); Tue, 31 Mar 2009 20:51:48 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752559AbZDAAvg (ORCPT ); Tue, 31 Mar 2009 20:51:36 -0400 Received: from mx2.redhat.com ([66.187.237.31]:41422 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751416AbZDAAvf (ORCPT ); Tue, 31 Mar 2009 20:51:35 -0400 Date: Tue, 31 Mar 2009 20:50:18 -0400 From: Jeff Layton To: Andrew Morton Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH] writeback: guard against jiffies wraparound on inode->dirtied_when checks (try #2) Message-ID: <20090331205018.228ea5f8@tleilax.poochiereds.net> In-Reply-To: <20090331172031.b2971f1e.akpm@linux-foundation.org> References: <1238544239-31882-1-git-send-email-jlayton@redhat.com> <20090331172031.b2971f1e.akpm@linux-foundation.org> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 31 Mar 2009 17:20:31 -0700 Andrew Morton wrote: > On Tue, 31 Mar 2009 20:03:59 -0400 > Jeff Layton wrote: > > > + * It's not sufficient to just do a time_after() check on > > + * dirtied_when. That assumes that dirtied_when will always > > + * change within a period of jiffies that encompasses half the > > + * machine word size (2^31 jiffies on 32-bit arch). That's not > > + * necessarily the case if an inode is being constantly > > + * redirtied. Since dirtied_when can never be in the future, > > + * we can assume that if it appears to be so then it is > > + * actually in the distant past. > > so this really is a 32-bit-only thing. > > I guess that isn't worth optimising for though. > Yeah, it's pretty much impossible to hit this on a 64-bit machine. > otoh, given that all three comparisons are the same: > > + time_after(inode->dirtied_when, *older_than_this) && > + time_before_eq(inode->dirtied_when, jiffies)) > > (although one is inverted (i think?)), it might end up nicer if this was all done > in a little helper function? > > That way we only need to comment what's going on at a single site, and > we could omit the additional test if !CONFIG_64BIT. Ok, that seems reasonable. At one point I had a macro similar to time_in_range(), but dropped it primarily because time_after_but_before_eq() wasn't easy on the eyes. Thoughts on better names? -- Jeff Layton