Re: [PATCH 1/6] CIFS: Make cifsFileInfo_put work with strict cache mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2010/11/19 Pavel Shilovsky <piastryyy@xxxxxxxxx>:
> 2010/11/19 Pavel Shilovsky <piastryyy@xxxxxxxxx>:
>> 2010/11/15 Jeff Layton <jlayton@xxxxxxxxxx>:
>>>
>>> Not directly related to your patch, but what exactly is the purpose of
>>> the "open_inode_helper"? It looks like "pile o' random junk that we do
>>
>> I agree that we can replace it with simply calling get_query_info from
>> cifs_open.
>>
>>> for non-posix opens". Maybe it would be best to eliminate that function
>>> altogether and further unify the posix and non-posix open code. Steve,
>>> care to comment?
>>>
>>> Now that I'm done ranting, I don't think the invalidate mapping call
>>> here is unnecessary. We will likely have already invalidated the cache
>>> during the d_revalidate phase, and you're going to do it again below in
>>> cifs_new_fileinfo.
>>>
>>
>> I investigated it - you was right about unnecessary calling
>> cifs_invalidate_mapping from open_inode_helper and cifs_new_fileinfo.
>> But I found the following problem:
>>
>> When we execute script close2_problem.py (see in attachment) several
>> times (e.x. for i in `seq 1 20`; do ./close2_problem.py; done) with
>> (!) cifsFYI switched off it shows 'a' instead of 'x' (about 1-2 times
>> for 20 runs). If we switch cifsFYI on it is almost always 'x' (true) -
>> I have caught 'a' in this case one or two times for many-many runs.
>> What do you think about it?
>
> I continued the investigation and discovered that if we use
> invalidate_inode_pages2 instead of invalidate_remote_inode - the
> problem disappears! So, this is the patch we should apply to solve it
> (I don't post it as a separate patch? because I think it needs more
> discussing):
>
> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
> index 53cce8c..f20f200 100644
> --- a/fs/cifs/inode.c
> +++ b/fs/cifs/inode.c
> @@ -1591,7 +1591,10 @@ cifs_invalidate_mapping(struct inode *inode)
>                if (rc)
>                        cifs_i->write_behind_rc = rc;
>        }
> -       invalidate_remote_inode(inode);
> +
> +       if (inode->i_mapping)
> +               invalidate_inode_pages2(inode->i_mapping);
> +
>        cifs_fscache_reset_inode_cookie(inode);
>  }
>

I have just noticed that Linux kernel NFS client also uses
invalidate_inode_pages2 in nfs_revalidate_mapping:
http://git.kernel.org/?p=linux/kernel/git/sfrench/cifs-2.6.git;a=blob;f=fs/nfs/inode.c;h=314f57164602eda0762c0a226db44aa19be461f5;hb=362d31297fafb150676f4d564ecc7f7f3e3b7fd4#l839

-- 
Best regards,
Pavel Shilovsky.
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux