Re: [PATCH v2] vfs: Add support to check max and min inode times

[Arnd Bergmann <arnd@arndb.de>]

On Thursday 03 March 2016 21:08:25 Deepa Dinamani wrote:
> On Thu, Mar 3, 2016 at 7:40 PM, Arnd Bergmann <arnd@arndb.de> 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