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);
--
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
