From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mout.kundenserver.de ([217.72.192.73]:54832 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754539AbcCCQDY (ORCPT ); Thu, 3 Mar 2016 11:03:24 -0500 From: Arnd Bergmann To: Deepa Dinamani Cc: Linux FS-devel Mailing List , y2038@lists.linaro.org, Alexander Viro , Thomas Gleixner , Dave Chinner Subject: Re: [PATCH v2] vfs: Add support to check max and min inode times Date: Thu, 03 Mar 2016 17:03:11 +0100 Message-ID: <27027369.MPMOSttCfe@wuerfel> In-Reply-To: References: <1456991401-27643-1-git-send-email-deepa.kernel@gmail.com> <7084215.crXNYPGr5j@wuerfel> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thursday 03 March 2016 21:08:25 Deepa Dinamani wrote: > On Thu, Mar 3, 2016 at 7:40 PM, Arnd Bergmann wrote: > > On Wednesday 02 March 2016 23:50:01 Deepa Dinamani wrote: > >> + /* > >> + * Assign a default empty range [MAX_VFS_TIME, MIN_VFS_TIME]. > >> + * This will help VFS detect filesystems that do not populate > >> + * these fields in the superblock. > >> + */ > >> + s->s_time_min = MAX_VFS_TIME; > >> + s->s_time_max = MIN_VFS_TIME; > >> s->cleancache_poolid = CLEANCACHE_NO_POOL; > > > > I think this is still less clear than it should be, even with the > > comment explaining it. > > > > As I just replied to Dave, how about initializing both to -1 instead? > > This was one of my first ideas. > The reason I did not choose this is because > 1. [-1, -1] is a valid range theoretically. > 2. -1 would now be treated as a value. > > It is true that no filesystem is using -1 as a limit. > But, I did not feel right excluding a perfectly good value. > Choosing [1,1] or any other number except probably 0 would work the same way. > > This is the reason that I decided an invalid range should be > represented by an invalid tuple and > not a single value. > My premise was that a range where min_time > max_time is invalid. Right. > My first choice here as in the first patch I shared with you privately > was [1,-1]. > But, the condition min_time < max_time can easily be achieved by > changing just one of the values. > For instance, [1, 2016]. But, this doesn't necessarily mean that the > filesystem explicitly set both the values. I see where you are coming from here, but I think we could also assume some minimum level of competence from the file system developers, so I would not have quite described it this way ;-) > For this reason, the invalid range I chose is [max_time, min_time]. > Of course [min_time, min_time] and [max_time, max_time] ranges are > also considered valid in this approach. > > This guarantees > 1. That both min and max values are assigned by each filesystem. > 2. The range is universally invalid. Ok, at least it clearly shows the intention that it cannot possibly be valid, unlike the '0' or '-1' defaults. Arnd