2011/3/24 Steve French <smfrench@xxxxxxxxx>:
> On Thu, Mar 24, 2011 at 9:39 AM, Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
>> Base of approach for splitting all calls that uses cifs_invalidate_mapping
>> (or cifs_revalidate_dentry, cifs_revalidate_file) into two groups:
>> 1) aware about -EBUSY error code and report it back (cifs_d_revalidate,
>> cifs_strict_fsync, cifs_file_strict_mmap);
>> 2) don't do it (cifs_getattrs, cifs_lseek).
>>
>> diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
>> index adb6324..d3abfc5 100644
>> --- a/fs/cifs/inode.c
>> +++ b/fs/cifs/inode.c
>> @@ -1683,27 +1683,25 @@ cifs_inode_needs_reval(struct inode *inode)
>> /*
>> * Zap the cache. Called when invalid_mapping flag is set.
>> */
>> -void
>> +int
>> cifs_invalidate_mapping(struct inode *inode)
>> {
>> - int rc;
>> + int rc = 0;
>> struct cifsInodeInfo *cifs_i = CIFS_I(inode);
>>
>> - cifs_i->invalid_mapping = false;
>> -
>> if (inode->i_mapping && inode->i_mapping->nrpages != 0) {
>> /* write back any cached data */
>> rc = filemap_write_and_wait(inode->i_mapping);
>> mapping_set_error(inode->i_mapping, rc);
>> rc = invalidate_inode_pages2(inode->i_mapping);
>> - if (rc) {
>> - cERROR(1, "%s: could not invalidate inode %p", __func__,
>> - inode);
>> - cifs_i->invalid_mapping = true;
>> - }
>> + if (rc)
>> + cFYI(1, "%s: could not invalidate inode %p", __func__,
>> + inode);
>> + cifs_i->invalid_mapping = (rc != 0) ? true : false;
>> }
>
> aren't you setting invalid_mapping twice (the first one is redundant)?
I think I set it only here in cifs_invalidate_mapping - can you
clarify what you mean?
>
>
>> int cifs_revalidate_file(struct file *filp)
>> @@ -1722,7 +1720,7 @@ int cifs_revalidate_file(struct file *filp)
>>
>> check_inval:
>> if (CIFS_I(inode)->invalid_mapping)
>> - cifs_invalidate_mapping(inode);
>> + rc = cifs_invalidate_mapping(inode);
>>
>> return rc;
>> }
>> @@ -1764,7 +1762,7 @@ int cifs_revalidate_dentry(struct dentry *dentry)
>>
>> check_inval:
>> if (CIFS_I(inode)->invalid_mapping)
>> - cifs_invalidate_mapping(inode);
>> + rc = cifs_invalidate_mapping(inode);
>>
>> kfree(full_path);
>> FreeXid(xid);
>> @@ -1776,7 +1774,15 @@ int cifs_getattr(struct vfsmount *mnt, struct dentry *dentry,
>> {
>> struct cifs_sb_info *cifs_sb = CIFS_SB(dentry->d_sb);
>> struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb);
>> - int err = cifs_revalidate_dentry(dentry);
>> + int err;
>> +
>> + err = cifs_revalidate_dentry(dentry);
>> + /*
>> + * We only need to get file attributes and don't need to
>> + * aware about busy pages (-EBUSY error code).
>> + */
>> + if (err == -EBUSY)
>> + err = 0;
>>
>> if (!err) {
>> generic_fillattr(dentry->d_inode, stat);
>
> With this change couldn't we accidently return a temporary write error
> (ENOSPC for example or EACCESS) when revalidating a file - which the
> caller is not expecting (getattr should only return errors if it can't
> get the attributes)
Now I don't return error from filemap_write_and_wait - so, there are
no errors from write part of revalidation.
--
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
