[Top] [All Lists]

Re: [PATCH 2/3] simplify xfs_setattr

To: Timothy Shimmin <tes@xxxxxxx>
Subject: Re: [PATCH 2/3] simplify xfs_setattr
From: Christoph Hellwig <hch@xxxxxx>
Date: Wed, 9 Jul 2008 18:29:14 +0200
Cc: xfs@xxxxxxxxxxx
In-reply-to: <48747DAD.7060501@xxxxxxx>
References: <20080627154557.GB31476@xxxxxx> <20080705172021.GA7177@xxxxxx> <48747DAD.7060501@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.3.28i
On Wed, Jul 09, 2008 at 06:58:21PM +1000, Timothy Shimmin wrote:
> > -   if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET))
> > -           flags |= ATTR_UTIME;
> > -#ifdef ATTR_NO_BLOCK
> > -   if ((ia_valid & ATTR_NO_BLOCK))
> > -           flags |= ATTR_NONBLOCK;
> > -#endif
> > -
> So this code looks different.
> We are now dropping the flags. Why is that?
> Presumably because we were mapping ia_valid's:
> But now we pass ATTR_?TIME_SET and ATTR_NO_BLOCK straight thru.
> So previously we didn't map them onto va_mask bits but as separate flags
> instead.

Yeah, not that ATTR_NO_BLOCK doesn't actually exist in any tree I have
access to, and thus it's not actually handled in the new xfs_setattr.

> So we get rid of the test for XFS_AT_NOSET.
> where:
> I can't see anywhere we set any of these.
> Presumably out of the xattr calls.
> Some left over from IRIX I guess.

Probably.  Note that linux uses the ATTR_ flags only for ->setattr so
there are per defintion none that can't be set.

> #define XFS_AT_UPDATIME         0x00010000
> #define XFS_AT_UPDMTIME         0x00020000
> #define XFS_AT_UPDCTIME         0x00040000
> 3 more not supported by vfs ATTR_* macros.
> I can't see where we set any of these.
> So no loss there I guess.
> Presumably they were just for IRIX.

It's an IRIX leftover.  I will submit a patch to introduce something
similar to Linux for 2.6.27, that's why I'd like these patches in for
2.6.26 so that I have a clean base to start from.

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