On Tue, 9 Jul 2013 17:19:12 -0400 "J. Bruce Fields" <bfields@xxxxxxxxxx> wrote: > On Tue, Jul 09, 2013 at 04:51:01PM -0400, J. Bruce Fields wrote: > > On Tue, Jul 09, 2013 at 09:30:47AM -0400, Jeff Layton wrote: > > > On Wed, 3 Jul 2013 16:12:36 -0400 > > > "J. Bruce Fields" <bfields@xxxxxxxxxx> wrote: > > > > > > > From: "J. Bruce Fields" <bfields@xxxxxxxxxx> > > > > > > > > NFSv4 uses leases to guarantee that clients can cache metadata as well > > > > as data. > > > > > ... > > > Isn't it possible we'll need to break a delegation on truncate()? > > > > In the truncate case, the caller called break_lease, and in the > > ftruncate case it's called with a write open, and the open already broke > > any leases or delegations. > > > > Might need a comment--could I get away with just this?: > > > > mutex_lock(&dentry->d_inode->i_mutex); > > + /* NULL is safe: any delegations have already been broken: */ > > ret = notify_change(dentry, &newattrs, NULL); > > mutex_unlock(&dentry->d_inode->i_mutex); > > return ret; > > > > I also added something to the notify_change kerneldoc: "passing NULL is > > fine for callers holding the file open for write, as there can be no > > conflicting delegation in that case." > > Another question is whether it's really worth dropping locks and > retrying in this case. > > We could instead do the following. > > --b. > > commit 40a4fd613034cd3f242ec11e5ecd44f9a83ab39d > Author: J. Bruce Fields <bfields@xxxxxxxxxx> > Date: Tue Sep 20 17:19:26 2011 -0400 > > locks: break delegations on any attribute modification > > NFSv4 uses leases to guarantee that clients can cache metadata as well > as data. > > Note unlike link, unlink, and rename, we don't bother dropping locks and > retrying. In the other cases we're holding a directory mutex, hence > blocking operations (even lookups) on the same directory. In this case > we're holding only the i_mutex on this file, so the impact of an > unresponsive client is limited to this file. > > Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx> > > diff --git a/fs/attr.c b/fs/attr.c > index 1449adb..a2c1d04 100644 > --- a/fs/attr.c > +++ b/fs/attr.c > @@ -243,6 +243,9 @@ int notify_change(struct dentry * dentry, struct iattr * attr) > error = security_inode_setattr(dentry, attr); > if (error) > return error; > + error = break_deleg_wait(inode); > + if (error) > + return error; > > if (inode->i_op->setattr) > error = inode->i_op->setattr(dentry, attr); I guess the question is whether there are operations that require the i_mutex but that don't require the delegation recall to have finished. -- Jeff Layton <jlayton@xxxxxxxxxx> -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
