| Message ID | 1440122378-30452-1-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Aug 20, 2015, at 9:59 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > If the ctime or mtime or change attribute have changed because > of an operation we initiated, we should make sure that we force > an attribute update. However we do not want to mark the page cache > for revalidation. I've tested your linux-next branch (tip is aebbe9d73169 ("NFS41/flexfiles: zero out DS write wcc") against Solaris 12 with write delegation enabled (over RDMA, even!). I was not able to reproduce the write append failures I saw before. > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > fs/nfs/inode.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c > index 2744d48bbbfe..e2cc0031decb 100644 > --- a/fs/nfs/inode.c > +++ b/fs/nfs/inode.c > @@ -1477,6 +1477,13 @@ static int nfs_post_op_update_inode_locked(struct inode *inode, struct nfs_fattr > { > unsigned long invalid = NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE; > > + /* > + * Don't revalidate the pagecache if we hold a delegation, but do > + * force an attribute update > + */ > + if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) > + invalid = NFS_INO_INVALID_ATTR|NFS_INO_REVAL_FORCED; > + > if (S_ISDIR(inode->i_mode)) > invalid |= NFS_INO_INVALID_DATA; > nfs_set_cache_invalid(inode, invalid); > -- > 2.4.3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Aug 20, 2015, at 9:59 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > >> If the ctime or mtime or change attribute have changed because >> of an operation we initiated, we should make sure that we force >> an attribute update. However we do not want to mark the page cache >> for revalidation. > > I've tested your linux-next branch (tip is aebbe9d73169 > ("NFS41/flexfiles: zero out DS write wcc") against Solaris 12 > with write delegation enabled (over RDMA, even!). > > I was not able to reproduce the write append failures I saw > before. > Perfect. Thanks for testing! Trond -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 25, 2015, at 1:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >> >> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >> >>> If the ctime or mtime or change attribute have changed because >>> of an operation we initiated, we should make sure that we force >>> an attribute update. However we do not want to mark the page cache >>> for revalidation. >> >> I've tested your linux-next branch (tip is aebbe9d73169 >> ("NFS41/flexfiles: zero out DS write wcc") against Solaris 12 >> with write delegation enabled (over RDMA, even!). >> >> I was not able to reproduce the write append failures I saw >> before. >> > > Perfect. Thanks for testing! Would it be possible to label some of these for stable? -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Aug 25, 2015, at 1:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > >> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>> >>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>> >>>> If the ctime or mtime or change attribute have changed because >>>> of an operation we initiated, we should make sure that we force >>>> an attribute update. However we do not want to mark the page cache >>>> for revalidation. >>> >>> I've tested your linux-next branch (tip is aebbe9d73169 >>> ("NFS41/flexfiles: zero out DS write wcc") against Solaris 12 >>> with write delegation enabled (over RDMA, even!). >>> >>> I was not able to reproduce the write append failures I saw >>> before. >>> >> >> Perfect. Thanks for testing! > > Would it be possible to label some of these for stable? > I think this one is the only one that is missing such a label. I've reworded the commit message and pushed out a revised patch. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 25, 2015, at 2:41 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >> >> On Aug 25, 2015, at 1:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >> >>> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>>> >>>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>>> >>>>> If the ctime or mtime or change attribute have changed because >>>>> of an operation we initiated, we should make sure that we force >>>>> an attribute update. However we do not want to mark the page cache >>>>> for revalidation. >>>> >>>> I've tested your linux-next branch (tip is aebbe9d73169 >>>> ("NFS41/flexfiles: zero out DS write wcc") against Solaris 12 >>>> with write delegation enabled (over RDMA, even!). >>>> >>>> I was not able to reproduce the write append failures I saw >>>> before. >>>> >>> >>> Perfect. Thanks for testing! >> >> Would it be possible to label some of these for stable? >> > > I think this one is the only one that is missing such a label. I've > reworded the commit message and pushed out a revised patch. Not related to the write append problem, but I've also seen missing opens on delegated files during reboot recovery in 4.0 kernels. Olga reported it before I got to it. Is that one also appropriate for stable? -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 25, 2015 at 2:46 PM, Chuck Lever <chuck.lever@oracle.com> wrote: > > On Aug 25, 2015, at 2:41 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > >> On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>> >>> On Aug 25, 2015, at 1:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>> >>>> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>>>> >>>>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>>>> >>>>>> If the ctime or mtime or change attribute have changed because >>>>>> of an operation we initiated, we should make sure that we force >>>>>> an attribute update. However we do not want to mark the page cache >>>>>> for revalidation. >>>>> >>>>> I've tested your linux-next branch (tip is aebbe9d73169 >>>>> ("NFS41/flexfiles: zero out DS write wcc") against Solaris 12 >>>>> with write delegation enabled (over RDMA, even!). >>>>> >>>>> I was not able to reproduce the write append failures I saw >>>>> before. >>>>> >>>> >>>> Perfect. Thanks for testing! >>> >>> Would it be possible to label some of these for stable? >>> >> >> I think this one is the only one that is missing such a label. I've >> reworded the commit message and pushed out a revised patch. > > Not related to the write append problem, but I've also seen > missing opens on delegated files during reboot recovery in > 4.0 kernels. Olga reported it before I got to it. > > Is that one also appropriate for stable? > I 've queued the revert of the patch that broke things for stable, but I didn't want to queue the new attempt at ensuring that we cache opens during a reboot reclaim... -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Aug 25, 2015, at 2:53 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: > On Tue, Aug 25, 2015 at 2:46 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >> >> On Aug 25, 2015, at 2:41 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >> >>> On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>>> >>>> On Aug 25, 2015, at 1:04 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>>> >>>>> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <chuck.lever@oracle.com> wrote: >>>>>> >>>>>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <trond.myklebust@primarydata.com> wrote: >>>>>> >>>>>>> If the ctime or mtime or change attribute have changed because >>>>>>> of an operation we initiated, we should make sure that we force >>>>>>> an attribute update. However we do not want to mark the page cache >>>>>>> for revalidation. >>>>>> >>>>>> I've tested your linux-next branch (tip is aebbe9d73169 >>>>>> ("NFS41/flexfiles: zero out DS write wcc") against Solaris 12 >>>>>> with write delegation enabled (over RDMA, even!). >>>>>> >>>>>> I was not able to reproduce the write append failures I saw >>>>>> before. >>>>>> >>>>> >>>>> Perfect. Thanks for testing! >>>> >>>> Would it be possible to label some of these for stable? >>>> >>> >>> I think this one is the only one that is missing such a label. I've >>> reworded the commit message and pushed out a revised patch. >> >> Not related to the write append problem, but I've also seen >> missing opens on delegated files during reboot recovery in >> 4.0 kernels. Olga reported it before I got to it. >> >> Is that one also appropriate for stable? >> > > I 've queued the revert of the patch that broke things for stable, but > I didn't want to queue the new attempt at ensuring that we cache opens > during a reboot reclaim... Understood, thank you! -- Chuck Lever -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/inode.c b/fs/nfs/inode.c index 2744d48bbbfe..e2cc0031decb 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -1477,6 +1477,13 @@ static int nfs_post_op_update_inode_locked(struct inode *inode, struct nfs_fattr { unsigned long invalid = NFS_INO_INVALID_ATTR|NFS_INO_REVAL_PAGECACHE; + /* + * Don't revalidate the pagecache if we hold a delegation, but do + * force an attribute update + */ + if (NFS_PROTO(inode)->have_delegation(inode, FMODE_READ)) + invalid = NFS_INO_INVALID_ATTR|NFS_INO_REVAL_FORCED; + if (S_ISDIR(inode->i_mode)) invalid |= NFS_INO_INVALID_DATA; nfs_set_cache_invalid(inode, invalid);
If the ctime or mtime or change attribute have changed because of an operation we initiated, we should make sure that we force an attribute update. However we do not want to mark the page cache for revalidation. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- fs/nfs/inode.c | 7 +++++++ 1 file changed, 7 insertions(+)