xfs
[Top] [All Lists]

Re: [PATCH] always set a/c/mtime through ->setattr

To: Miklos Szeredi <miklos@xxxxxxxxxx>
Subject: Re: [PATCH] always set a/c/mtime through ->setattr
From: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date: Sat, 31 May 2008 14:20:48 +0100
Cc: hch@xxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, Artem.Bityutskiy@xxxxxxxxx
In-reply-to: <E1JzFFa-0008VR-9B@pomaz-ex.szeredi.hu>
References: <20080520060838.GA6436@lst.de> <E1JyMSu-0001au-96@pomaz-ex.szeredi.hu> <20080520083351.GA14826@lst.de> <E1JzFFa-0008VR-9B@pomaz-ex.szeredi.hu>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.17 (2007-11-01)
On Thu, May 22, 2008 at 08:10:50PM +0200, Miklos Szeredi wrote:

> But there are quite a few others which don't call inode_setattr (which
> means that the unchanged time optimization is lost), or which do
> something possibly slow in their ->setattr():
> 
>  adfs, 9p, afs, coda, gfs2 ...
> 
> just to name a few at the start of the alphabet.
> 
> So it looks to me as this could cause some unintended performance
> regressions in these filesystems.

Actually, there's worse one: ext3.  It *does* call inode_setattr(),
all right, but then it proceeds to call ext3_orphan_del().  Which
will
        lock_super(inode->i_sb);
        if (list_empty(&ei->i_orphan)) {
                unlock_super(inode->i_sb);
                return 0;
        }
and bugger off, but...
        * it's going to cost us
        * code in _caller_ is bogus - we call that sucker regardless of
whether we had ATTR_SIZE in ia_valid

And there's one more problem, promising very ugly code review: locking
rules for notify_change() had suddenly changed - you are calling it
without i_mutex now.  And ext3_setattr() is not happy - especially due
to this blind call of ext3_orphan_del() in there.  We can easily fix
that one, but you'll need to audit the rest of instances...


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