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)?
> 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)
--
Thanks,
Steve
--
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
