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 <[email protected]>
---
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
On Aug 20, 2015, at 9:59 PM, Trond Myklebust <[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Chuck Lever
On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <[email protected]> wrote:
>
> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <[email protected]> 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
On Aug 25, 2015, at 1:04 PM, Trond Myklebust <[email protected]> wrote:
> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <[email protected]> wrote:
>>
>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <[email protected]> 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
On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <[email protected]> wrote:
>
> On Aug 25, 2015, at 1:04 PM, Trond Myklebust <[email protected]> wrote:
>
>> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <[email protected]> wrote:
>>>
>>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <[email protected]> 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.
On Aug 25, 2015, at 2:41 PM, Trond Myklebust <[email protected]> wrote:
> On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <[email protected]> wrote:
>>
>> On Aug 25, 2015, at 1:04 PM, Trond Myklebust <[email protected]> wrote:
>>
>>> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <[email protected]> wrote:
>>>>
>>>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <[email protected]> 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
On Tue, Aug 25, 2015 at 2:46 PM, Chuck Lever <[email protected]> wrote:
>
> On Aug 25, 2015, at 2:41 PM, Trond Myklebust <[email protected]> wrote:
>
>> On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <[email protected]> wrote:
>>>
>>> On Aug 25, 2015, at 1:04 PM, Trond Myklebust <[email protected]> wrote:
>>>
>>>> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <[email protected]> wrote:
>>>>>
>>>>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <[email protected]> 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...
On Aug 25, 2015, at 2:53 PM, Trond Myklebust <[email protected]> wrote:
> On Tue, Aug 25, 2015 at 2:46 PM, Chuck Lever <[email protected]> wrote:
>>
>> On Aug 25, 2015, at 2:41 PM, Trond Myklebust <[email protected]> wrote:
>>
>>> On Tue, Aug 25, 2015 at 2:18 PM, Chuck Lever <[email protected]> wrote:
>>>>
>>>> On Aug 25, 2015, at 1:04 PM, Trond Myklebust <[email protected]> wrote:
>>>>
>>>>> On Tue, Aug 25, 2015 at 12:31 PM, Chuck Lever <[email protected]> wrote:
>>>>>>
>>>>>> On Aug 20, 2015, at 9:59 PM, Trond Myklebust <[email protected]> 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