Re: Checking page_count(page) in invalidate_complete_page

[Chuck Lever <chuck.lever@oracle.com>]

Trond Myklebust wrote:
> On Tue, 2006-10-03 at 14:50 -0400, Chuck Lever wrote:
> 
>>>> diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
>>>> index 377839b..fe69c39 100644
>>>> --- a/fs/nfs/direct.c
>>>> +++ b/fs/nfs/direct.c
>>>> @@ -823,7 +823,7 @@ ssize_t nfs_file_direct_write(struct kio
>>>>  	 *      occur before the writes complete.  Kind of racey.
>>>>  	 */
>>>>  	if (mapping->nrpages)
>>>> -		invalidate_inode_pages2(mapping);
>>>> +		nfs_invalidate_mapping(mapping->host, mapping);
>>> This looks wrong. Why are we bumping the NFSIOS_DATAINVALIDATE counter
>>> on a direct write? We're not registering a cache consistency problem
>>> here.
>> We're looking for potential races between direct I/O and cache 
>> invalidation, among others.  Is your concern that this may report false 
>> positives?
> 
> No. I simply don't see what the use case is for this statistic. AFAICS
> it is purely a debugging tool for _developers_. That would have
> absolutely no place at all in /proc/self/mountstats, which is supposed
> to provide useful statistics for _administrators_.

Don't you think admins need to know when the client has potentially 
caused data corruption?

>> I'm not sure this invalidation is useful in any event.  Direct writes 
>> are treated like some other client has modified the file, so cached 
>> pages will get invalidated eventually anyway.  Maybe we should just 
>> remove this one?
> 
> That would break the principle that if one process modifies the file,
> then all processes on the same client will immediately see those
> changes.

We could achieve that simply by setting the NFS_INO_INVALID_DATA flag in 
nfs_file_direct_write() and/or nfs_direct_write_complete().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>