Hi David, > The following commit included in 3.10, and copied to some table-series > kernels: > > commit: c2b93e0699723700f886ce17bb65ffd771195a6d > cifs: only set ops for inodes in I_NEW state > > ... causes a regression in mfsymlink handling under some circumstances. > > Specifically, this patch modified cifs_fattr_to_inode() to only invoke > cifs_set_ops() if that inode still had the I_NEW flag set --- preventing > an inode's ops-set from from being changed after it had been > initialized, preventing an oops if another thread of execution was > already chasing that pointer. > > However, mfsymlinks are also affected by this commit. In the cold-cache > case, a user invoking stat() on an mfsymlink will still see the correct > results. But, if the dentry cache is instead populated via > cifs_readdir/filldir, then inode property population operates in a two > stage mode: > > - First, the inode is initialized as a regular file, with the > CIFS_FATTR_NEED_REVAL flag set. > > - Later, when the file is stat()'d, then correct operation depends on > the (re)validation steps from being able to update the inode's > properties --- including the set of file operations permitted. > > (A comment explains: "trying to get the type and mode can be slow, so > just call those regular files for now, and mark for reval") > > With the above commit, this second revalidation step never updates the > operations field, resulting in symlinks not having the readlink(), etc. > functions correctly hooked up. > A naïve fix-up is to modify the conditional to also permit (re)setting > ops on S_IFLNK-mode files; however, I suspect a correct fix lies elsewhere? I guess 'if (inode->i_state & I_NEW)' should be changed to 'if ((inode->i_state & I_NEW) || (fattr->cf_flags & CIFS_FATTR_NEED_REVAL))' Would that solve the problem? Jeff/Steve, any comment on this? metze
Attachment:
signature.asc
Description: OpenPGP digital signature