All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/5] CIFS: Invalidate inode pages on the last close
@ 2010-10-31 19:42 Pavel Shilovsky
       [not found] ` <AANLkTi=ZzzjuH-yzgkCWCpm3g7UoBXM8rLgWdUVzj2VO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Shilovsky @ 2010-10-31 19:42 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

When we close the last file handle of the inode we should invalidate it
to prevent data coherency problem when we open it again but it has been
modified by other clients.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/file.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index d7c212a..6edeb5b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -290,6 +290,8 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
 			cifs_file->dentry->d_inode);
 		cifsi->clientCanCacheRead = false;
 		cifsi->clientCanCacheAll  = false;
+		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO)
+			invalidate_remote_inode(inode);
 	}
 	spin_unlock(&cifs_file_list_lock);

-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/5] CIFS: Invalidate inode pages on the last close
       [not found] ` <AANLkTi=ZzzjuH-yzgkCWCpm3g7UoBXM8rLgWdUVzj2VO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-11-01 13:41   ` Jeff Layton
  0 siblings, 0 replies; 6+ messages in thread
From: Jeff Layton @ 2010-11-01 13:41 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Sun, 31 Oct 2010 22:42:40 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> When we close the last file handle of the inode we should invalidate it
> to prevent data coherency problem when we open it again but it has been
> modified by other clients.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/file.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index d7c212a..6edeb5b 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -290,6 +290,8 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
>  			cifs_file->dentry->d_inode);
>  		cifsi->clientCanCacheRead = false;
>  		cifsi->clientCanCacheAll  = false;
> +		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO)
> +			invalidate_remote_inode(inode);
>  	}
>  	spin_unlock(&cifs_file_list_lock);
> 

NAK. I don't think it's safe to call invalidate_remote_inode under a
spinlock like this.

-- 
Jeff Layton <jlayton-vpEMnDpepFuMZCB2o+C8xQ@public.gmane.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/5] CIFS: Invalidate inode pages on the last close
       [not found]         ` <AANLkTika0teNKgPBqWi23X-iAXvfjuxdOz=p0HJ6Fo-3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-11-02 19:42           ` Pavel Shilovsky
  0 siblings, 0 replies; 6+ messages in thread
From: Pavel Shilovsky @ 2010-11-02 19:42 UTC (permalink / raw)
  To: Steve French; +Cc: Jeff Layton, linux-cifs-u79uwXL29TY76Z2rM5mHXA

2010/11/2 Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Jeff's point looks correct.  Also note when you fix this up - the
> local variable for cifs_sb from the previous patch (which caused a
> build warning for unused variable) needs to be readded back to this
> patch.
>
Yes, I agree - it's my bug. I will post right variant further.

-- 
Best regards,
Pavel Shilovsky.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/5] CIFS: Invalidate inode pages on the last close
       [not found]     ` <20101102111932.7377e2bb-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2010-11-02 19:26       ` Steve French
       [not found]         ` <AANLkTika0teNKgPBqWi23X-iAXvfjuxdOz=p0HJ6Fo-3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Steve French @ 2010-11-02 19:26 UTC (permalink / raw)
  To: Jeff Layton; +Cc: Pavel Shilovsky, linux-cifs-u79uwXL29TY76Z2rM5mHXA

Jeff's point looks correct.  Also note when you fix this up - the
local variable for cifs_sb from the previous patch (which caused a
build warning for unused variable) needs to be readded back to this
patch.

On Tue, Nov 2, 2010 at 10:19 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, 2 Nov 2010 12:01:17 +0300
> Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On strict cache mode when we close the last file handle of the inode we
>> should invalidate this inode to prevent data coherency problem when we
>> open it again but it has been modified by other clients.
>>
>> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  fs/cifs/cifs_fs_sb.h |    1 +
>>  fs/cifs/file.c       |    3 +++
>>  2 files changed, 4 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
>> index 525ba59..3690c50 100644
>> --- a/fs/cifs/cifs_fs_sb.h
>> +++ b/fs/cifs/cifs_fs_sb.h
>> @@ -40,6 +40,7 @@
>>  #define CIFS_MOUNT_FSCACHE   0x8000 /* local caching enabled */
>>  #define CIFS_MOUNT_MF_SYMLINKS       0x10000 /* Minshall+French Symlinks enabled */
>>  #define CIFS_MOUNT_MULTIUSER 0x20000 /* multiuser mount */
>> +#define CIFS_MOUNT_STRICT_IO 0x40000 /* strict cache mode */
>>
>>  struct cifs_sb_info {
>>       struct radix_tree_root tlink_tree;
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index a566f15..3970e68 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -284,6 +284,9 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
>>       }
>>       spin_unlock(&cifs_file_list_lock);
>>
>> +     if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO)
>> +             invalidate_remote_inode(inode);
>> +
>>       if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
>>               int xid, rc;
>>
>
> Suppose I have this file open twice and have r/o oplocks on both
> filehandles. I close one of those file descriptors and the cache is
> invalidated. Is that really what we want here?
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Thanks,

Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/5] CIFS: Invalidate inode pages on the last close
       [not found] ` <AANLkTikE-QfKh8AYe=NZtQOKPhJ_LGQ+K5xcGb6wBfbw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2010-11-02 15:19   ` Jeff Layton
       [not found]     ` <20101102111932.7377e2bb-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Jeff Layton @ 2010-11-02 15:19 UTC (permalink / raw)
  To: Pavel Shilovsky; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On Tue, 2 Nov 2010 12:01:17 +0300
Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> On strict cache mode when we close the last file handle of the inode we
> should invalidate this inode to prevent data coherency problem when we
> open it again but it has been modified by other clients.
> 
> Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  fs/cifs/cifs_fs_sb.h |    1 +
>  fs/cifs/file.c       |    3 +++
>  2 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
> index 525ba59..3690c50 100644
> --- a/fs/cifs/cifs_fs_sb.h
> +++ b/fs/cifs/cifs_fs_sb.h
> @@ -40,6 +40,7 @@
>  #define CIFS_MOUNT_FSCACHE	0x8000 /* local caching enabled */
>  #define CIFS_MOUNT_MF_SYMLINKS	0x10000 /* Minshall+French Symlinks enabled */
>  #define CIFS_MOUNT_MULTIUSER	0x20000 /* multiuser mount */
> +#define CIFS_MOUNT_STRICT_IO	0x40000 /* strict cache mode */
> 
>  struct cifs_sb_info {
>  	struct radix_tree_root tlink_tree;
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index a566f15..3970e68 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -284,6 +284,9 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
>  	}
>  	spin_unlock(&cifs_file_list_lock);
> 
> +	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO)
> +		invalidate_remote_inode(inode);
> +
>  	if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
>  		int xid, rc;
> 

Suppose I have this file open twice and have r/o oplocks on both
filehandles. I close one of those file descriptors and the cache is
invalidated. Is that really what we want here?

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 2/5] CIFS: Invalidate inode pages on the last close
@ 2010-11-02  9:01 Pavel Shilovsky
       [not found] ` <AANLkTikE-QfKh8AYe=NZtQOKPhJ_LGQ+K5xcGb6wBfbw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: Pavel Shilovsky @ 2010-11-02  9:01 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA

On strict cache mode when we close the last file handle of the inode we
should invalidate this inode to prevent data coherency problem when we
open it again but it has been modified by other clients.

Signed-off-by: Pavel Shilovsky <piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/cifs/cifs_fs_sb.h |    1 +
 fs/cifs/file.c       |    3 +++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index 525ba59..3690c50 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -40,6 +40,7 @@
 #define CIFS_MOUNT_FSCACHE	0x8000 /* local caching enabled */
 #define CIFS_MOUNT_MF_SYMLINKS	0x10000 /* Minshall+French Symlinks enabled */
 #define CIFS_MOUNT_MULTIUSER	0x20000 /* multiuser mount */
+#define CIFS_MOUNT_STRICT_IO	0x40000 /* strict cache mode */

 struct cifs_sb_info {
 	struct radix_tree_root tlink_tree;
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index a566f15..3970e68 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -284,6 +284,9 @@ void cifsFileInfo_put(struct cifsFileInfo *cifs_file)
 	}
 	spin_unlock(&cifs_file_list_lock);

+	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_STRICT_IO)
+		invalidate_remote_inode(inode);
+
 	if (!tcon->need_reconnect && !cifs_file->invalidHandle) {
 		int xid, rc;

-- 
1.7.2.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-11-02 19:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-31 19:42 [PATCH 2/5] CIFS: Invalidate inode pages on the last close Pavel Shilovsky
     [not found] ` <AANLkTi=ZzzjuH-yzgkCWCpm3g7UoBXM8rLgWdUVzj2VO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-11-01 13:41   ` Jeff Layton
2010-11-02  9:01 Pavel Shilovsky
     [not found] ` <AANLkTikE-QfKh8AYe=NZtQOKPhJ_LGQ+K5xcGb6wBfbw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-11-02 15:19   ` Jeff Layton
     [not found]     ` <20101102111932.7377e2bb-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2010-11-02 19:26       ` Steve French
     [not found]         ` <AANLkTika0teNKgPBqWi23X-iAXvfjuxdOz=p0HJ6Fo-3-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-11-02 19:42           ` Pavel Shilovsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.