2010/12/8 Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>: > On Wed, Dec 8, 2010 at 8:28 AM, Pavel Shilovsky <piastryyy@xxxxxxxxx> wrote: >> Delete cifs_open_inode_helper and move non-posix open related things >> to cifs_nt_open function. >> >> Signed-off-by: Pavel Shilovsky <piastryyy@xxxxxxxxx> >> --- >> fs/cifs/file.c | 189 ++++++++++++++++++++++---------------------------------- >> 1 files changed, 73 insertions(+), 116 deletions(-) >> >> diff --git a/fs/cifs/file.c b/fs/cifs/file.c >> index b857ce5..8f6a084 100644 >> --- a/fs/cifs/file.c >> +++ b/fs/cifs/file.c >> @@ -104,53 +104,6 @@ static inline int cifs_get_disposition(unsigned int flags) >> return FILE_OPEN; >> } >> >> -static inline int cifs_open_inode_helper(struct inode *inode, >> - struct cifsTconInfo *pTcon, __u32 oplock, FILE_ALL_INFO *buf, >> - char *full_path, int xid) >> -{ >> - struct cifsInodeInfo *pCifsInode = CIFS_I(inode); >> - struct timespec temp; >> - int rc; >> - >> - if (pCifsInode->clientCanCacheRead) { >> - /* we have the inode open somewhere else >> - no need to discard cache data */ >> - goto client_can_cache; >> - } >> - >> - /* BB need same check in cifs_create too? */ >> - /* if not oplocked, invalidate inode pages if mtime or file >> - size changed */ >> - temp = cifs_NTtimeToUnix(buf->LastWriteTime); >> - if (timespec_equal(&inode->i_mtime, &temp) && >> - (inode->i_size == >> - (loff_t)le64_to_cpu(buf->EndOfFile))) { >> - cFYI(1, "inode unchanged on server"); >> - } else { >> - if (inode->i_mapping) { >> - /* BB no need to lock inode until after invalidate >> - since namei code should already have it locked? */ >> - rc = filemap_write_and_wait(inode->i_mapping); >> - mapping_set_error(inode->i_mapping, rc); >> - } >> - cFYI(1, "invalidating remote inode since open detected it " >> - "changed"); >> - invalidate_remote_inode(inode); >> - } >> - >> -client_can_cache: >> - if (pTcon->unix_ext) >> - rc = cifs_get_inode_info_unix(&inode, full_path, inode->i_sb, >> - xid); >> - else >> - rc = cifs_get_inode_info(&inode, full_path, buf, inode->i_sb, >> - xid, NULL); >> - >> - cifs_set_oplock_level(pCifsInode, oplock); >> - >> - return rc; >> -} >> - >> int cifs_posix_open(char *full_path, struct inode **pinode, >> struct super_block *sb, int mode, unsigned int f_flags, >> __u32 *poplock, __u16 *pnetfid, int xid) >> @@ -213,6 +166,76 @@ posix_open_ret: >> return rc; >> } >> >> +static int >> +cifs_nt_open(char *full_path, struct inode *inode, struct cifs_sb_info *cifs_sb, >> + struct cifsTconInfo *tcon, unsigned int f_flags, __u32 *poplock, >> + __u16 *pnetfid, int xid) >> +{ >> + int rc; >> + int desiredAccess; >> + int disposition; >> + FILE_ALL_INFO *buf; >> + >> + desiredAccess = cifs_convert_flags(f_flags); >> + >> +/********************************************************************* >> + * open flag mapping table: >> + * >> + * POSIX Flag CIFS Disposition >> + * ---------- ---------------- >> + * O_CREAT FILE_OPEN_IF >> + * O_CREAT | O_EXCL FILE_CREATE >> + * O_CREAT | O_TRUNC FILE_OVERWRITE_IF >> + * O_TRUNC FILE_OVERWRITE >> + * none of the above FILE_OPEN >> + * >> + * Note that there is not a direct match between disposition >> + * FILE_SUPERSEDE (ie create whether or not file exists although >> + * O_CREAT | O_TRUNC is similar but truncates the existing >> + * file rather than creating a new file as FILE_SUPERSEDE does >> + * (which uses the attributes / metadata passed in on open call) >> + *? >> + *? O_SYNC is a reasonable match to CIFS writethrough flag >> + *? and the read write flags match reasonably. O_LARGEFILE >> + *? is irrelevant because largefile support is always used >> + *? by this client. Flags O_APPEND, O_DIRECT, O_DIRECTORY, >> + * O_FASYNC, O_NOFOLLOW, O_NONBLOCK need further investigation >> + *********************************************************************/ >> + >> + disposition = cifs_get_disposition(f_flags); >> + >> + /* BB pass O_SYNC flag through on file attributes .. BB */ >> + >> + buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL); >> + if (!buf) >> + return -ENOMEM; >> + >> + if (tcon->ses->capabilities & CAP_NT_SMBS) >> + rc = CIFSSMBOpen(xid, tcon, full_path, disposition, >> + desiredAccess, CREATE_NOT_DIR, pnetfid, poplock, buf, >> + cifs_sb->local_nls, cifs_sb->mnt_cifs_flags >> + & CIFS_MOUNT_MAP_SPECIAL_CHR); >> + else >> + rc = SMBLegacyOpen(xid, tcon, full_path, disposition, >> + desiredAccess, CREATE_NOT_DIR, pnetfid, poplock, buf, >> + cifs_sb->local_nls, cifs_sb->mnt_cifs_flags >> + & CIFS_MOUNT_MAP_SPECIAL_CHR); >> + >> + if (rc) >> + goto out; >> + >> + if (tcon->unix_ext) >> + rc = cifs_get_inode_info_unix(&inode, full_path, inode->i_sb, >> + xid); >> + else >> + rc = cifs_get_inode_info(&inode, full_path, buf, inode->i_sb, >> + xid, pnetfid); >> + > > Just a nit-picking, we probably do not expect a non-posix function > calling a function for unix! > If the server broke posix open but still supports unix extensions we should use unix specific function, I think. -- 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