2011/3/23 Steve French <smfrench@xxxxxxxxx>:
> Pavel,
> Jeff and I talked about invalidate_mapping and its uses this
> afternoon. A few comments:
> 1) Take a look at what happens on filemap_fdatawait call in
> invalidate_mapping to see if it makes sense to return errors through
> back to some of the callers of invalidate_mapping, in particular,
> strict_fsync. If we can't write out the file data (ENOSPC, or host
> down etc.), we want to make sure that the return code gets sent back
> on any calls that can reasonably expect such an error.
vfs_fsync_range has already done filemap_fdatawait call - so, in this
case there is no need to do it again in cifs_invalidate_mapping. The
only reason for calling this is to invalidate_inode_pages2 but any
error there shouldn't affect fsync behavior, as I think.
> 2) If invalidate_inode_pages2 fails (e.g. with EBUSY, because one of
> the pages couldn't be freed from the mapping because it just got
> redirtied right after we flushed it the line before) we set the
> mapping to invalid but don't check
> cifs_i->invalid_mapping
> in many places. Should we add checks for cifs_i->invalid_mapping in
> more places?
The one place where we should add such a check is read call, but it
needs cifs_revalidate_file instead (that I am going to provide next)
before generic_file_aio_read. In this case cifs_revalidate will check
for invalid_mapping and needs to return a error if we could not
invalidate all inode pages in cifs_invalidate_mapping (because it
returns wrong data to the read call). But as you noticed lseek
shouldn't think about this error and it uses cifs_revalidate_file too.
So, we may add extra check for -EBUSY error code in callers of
cifs_revalidate_{dentry,file} and cifs_invaliadate_mapping and
separate them into two groups:
1) that aware about -EBUSY error code and return a error the it's caller.
these are: cifs_d_revalidate, cifs_file_aio_read (future
implementation), cifs_file_strict_mmap.
2) that doesn't aware about it and return ok in this case.
these are: cifs_getattrs, cifs_lseek, cifs_fsync.
Thoughts?
--
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
