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);
> }
>
Sorry, "invalidate" patch above was based on v2.6.36 git tree. Here
are the version from cifs master branch:
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index ff7d299..66d3ba3 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -1679,12 +1679,16 @@ cifs_invalidate_mapping(struct inode *inode)
cifs_i->invalid_mapping = false;
- /* write back any cached data */
- if (inode->i_mapping && inode->i_mapping->nrpages != 0) {
- rc = filemap_write_and_wait(inode->i_mapping);
- mapping_set_error(inode->i_mapping, rc);
+ if (inode->i_mapping) {
+ /* write back any cached data */
+ if (inode->i_mapping->nrpages != 0) {
+ rc = filemap_write_and_wait(inode->i_mapping);
+ mapping_set_error(inode->i_mapping, rc);
+ }
+
+ invalidate_inode_pages2(inode->i_mapping);
}
- invalidate_remote_inode(inode);
+
cifs_fscache_reset_inode_cookie(inode);
}
If everything is ok with it I will post it as a separate patch in right format.
--
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
