From 132ac7b77cc95a22d6118d327c96586759fbf006 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 10 Feb 2009 07:33:57 -0500 Subject: [PATCH] cifs: refactor new_inode() calls and inode initialization Move new inode creation into a separate routine and refactor the callers to take advantage of it. Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsproto.h | 2 + fs/cifs/inode.c | 96 ++++++++++++++++++++++++++++----------------- fs/cifs/readdir.c | 54 ++++++++++++------------- 3 files changed, 86 insertions(+), 66 deletions(-) diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index ec9f9c1c7d88..62fd5bd499f6 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -92,6 +92,8 @@ extern u64 cifs_UnixTimeToNT(struct timespec); extern __le64 cnvrtDosCifsTm(__u16 date, __u16 time); extern struct timespec cnvrtDosUnixTm(__u16 date, __u16 time); +extern struct inode *cifs_new_inode(struct super_block *sb, + unsigned long *inum); extern int cifs_get_inode_info(struct inode **pinode, const unsigned char *search_path, FILE_ALL_INFO *pfile_info, diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 7342bfb02ae0..c7674f595adb 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -199,6 +199,49 @@ static void fill_fake_finddataunix(FILE_UNIX_BASIC_INFO *pfnd_dat, pfnd_dat->Gid = cpu_to_le64(pinode->i_gid); } +/** + * cifs_new inode - create new inode, initialize, and hash it + * @sb - pointer to superblock + * @inum - if valid pointer and serverino is enabled, replace i_ino with val + * + * Create a new inode, initialize it for CIFS and hash it. Returns the new + * inode or NULL if one couldn't be allocated. + * + * If the share isn't mounted with "serverino" or inum is a NULL pointer then + * we'll just use the inode number assigned by new_inode(). Note that this can + * mean i_ino collisions since the i_ino assigned by new_inode is not + * guaranteed to be unique. + */ +struct inode * +cifs_new_inode(struct super_block *sb, unsigned long *inum) +{ + struct inode *inode; + + inode = new_inode(sb); + if (inode == NULL) + return NULL; + + /* + * BB: Is i_ino == 0 legal? Here, we assume that it is. If it isn't we + * stop passing inum as ptr. Are there sanity checks we can use to + * ensure that the server is really filling in that field? Also, + * if serverino is disabled, perhaps we should be using iunique()? + */ + if (inum && (CIFS_SB(sb)->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)) + inode->i_ino = *inum; + + /* + * must set this here instead of cifs_alloc_inode since VFS will + * clobber i_flags + */ + if (sb->s_flags & MS_NOATIME) + inode->i_flags |= S_NOATIME | S_NOCMTIME; + + insert_inode_hash(inode); + + return inode; +} + int cifs_get_inode_info_unix(struct inode **pinode, const unsigned char *full_path, struct super_block *sb, int xid) { @@ -233,22 +276,12 @@ int cifs_get_inode_info_unix(struct inode **pinode, /* get new inode */ if (*pinode == NULL) { - *pinode = new_inode(sb); + *pinode = cifs_new_inode(sb, (unsigned long *) + &find_data.UniqueId); if (*pinode == NULL) { rc = -ENOMEM; goto cgiiu_exit; } - /* Is an i_ino of zero legal? */ - /* note ino incremented to unique num in new_inode */ - /* Are there sanity checks we can use to ensure that - the server is really filling in that field? */ - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) - (*pinode)->i_ino = (unsigned long)find_data.UniqueId; - - if (sb->s_flags & MS_NOATIME) - (*pinode)->i_flags |= S_NOATIME | S_NOCMTIME; - - insert_inode_hash(*pinode); } inode = *pinode; @@ -465,11 +498,8 @@ int cifs_get_inode_info(struct inode **pinode, /* get new inode */ if (*pinode == NULL) { - *pinode = new_inode(sb); - if (*pinode == NULL) { - rc = -ENOMEM; - goto cgii_exit; - } + __u64 inode_num; + /* Is an i_ino of zero legal? Can we use that to check if the server supports returning inode numbers? Are there other sanity checks we can use to ensure that @@ -486,7 +516,6 @@ int cifs_get_inode_info(struct inode **pinode, if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) { int rc1 = 0; - __u64 inode_num; rc1 = CIFSGetSrvInodeNumber(xid, pTcon, full_path, &inode_num, @@ -496,12 +525,17 @@ int cifs_get_inode_info(struct inode **pinode, if (rc1) { cFYI(1, ("GetSrvInodeNum rc %d", rc1)); /* BB EOPNOSUPP disable SERVER_INUM? */ - } else /* do we need cast or hash to ino? */ - (*pinode)->i_ino = inode_num; - } /* else ino incremented to unique num in new_inode*/ - if (sb->s_flags & MS_NOATIME) - (*pinode)->i_flags |= S_NOATIME | S_NOCMTIME; - insert_inode_hash(*pinode); + } + *pinode = cifs_new_inode(sb, (unsigned long *) + &inode_num); + } else { + *pinode = cifs_new_inode(sb, NULL); + } + + if (*pinode == NULL) { + rc = -ENOMEM; + goto cgii_exit; + } } inode = *pinode; cifsInfo = CIFS_I(inode); @@ -1114,24 +1148,14 @@ int cifs_mkdir(struct inode *inode, struct dentry *direntry, int mode) else direntry->d_op = &cifs_dentry_ops; - newinode = new_inode(inode->i_sb); + newinode = cifs_new_inode(inode->i_sb, (unsigned long *) + &pInfo->UniqueId); if (newinode == NULL) { kfree(pInfo); goto mkdir_get_info; } - /* Is an i_ino of zero legal? */ - /* Are there sanity checks we can use to ensure that - the server is really filling in that field? */ - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) { - newinode->i_ino = - (unsigned long)pInfo->UniqueId; - } /* note ino incremented to unique num in new_inode */ - if (inode->i_sb->s_flags & MS_NOATIME) - newinode->i_flags |= S_NOATIME | S_NOCMTIME; newinode->i_nlink = 2; - - insert_inode_hash(newinode); d_instantiate(direntry, newinode); /* we already checked in POSIXCreate whether diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c index 9f51f9bf0292..02a20221e841 100644 --- a/fs/cifs/readdir.c +++ b/fs/cifs/readdir.c @@ -56,35 +56,34 @@ static inline void dump_cifs_file_struct(struct file *file, char *label) } #endif /* DEBUG2 */ -/* Returns one if new inode created (which therefore needs to be hashed) */ +/* Returns 1 if new inode created, 2 if both dentry and inode were */ /* Might check in the future if inode number changed so we can rehash inode */ -static int construct_dentry(struct qstr *qstring, struct file *file, - struct inode **ptmp_inode, struct dentry **pnew_dentry) +static int +construct_dentry(struct qstr *qstring, struct file *file, + struct inode **ptmp_inode, struct dentry **pnew_dentry, + unsigned long *inum) { - struct dentry *tmp_dentry; - struct cifs_sb_info *cifs_sb; - struct cifsTconInfo *pTcon; + struct dentry *tmp_dentry = NULL; + struct super_block *sb = file->f_path.dentry->d_sb; int rc = 0; cFYI(1, ("For %s", qstring->name)); - cifs_sb = CIFS_SB(file->f_path.dentry->d_sb); - pTcon = cifs_sb->tcon; qstring->hash = full_name_hash(qstring->name, qstring->len); tmp_dentry = d_lookup(file->f_path.dentry, qstring); if (tmp_dentry) { + /* BB: overwrite old name? i.e. tmp_dentry->d_name and + * tmp_dentry->d_name.len?? + */ cFYI(0, ("existing dentry with inode 0x%p", tmp_dentry->d_inode)); *ptmp_inode = tmp_dentry->d_inode; -/* BB overwrite old name? i.e. tmp_dentry->d_name and tmp_dentry->d_name.len??*/ if (*ptmp_inode == NULL) { - *ptmp_inode = new_inode(file->f_path.dentry->d_sb); + *ptmp_inode = cifs_new_inode(sb, inum); if (*ptmp_inode == NULL) return rc; rc = 1; } - if (file->f_path.dentry->d_sb->s_flags & MS_NOATIME) - (*ptmp_inode)->i_flags |= S_NOATIME | S_NOCMTIME; } else { tmp_dentry = d_alloc(file->f_path.dentry, qstring); if (tmp_dentry == NULL) { @@ -93,15 +92,14 @@ static int construct_dentry(struct qstr *qstring, struct file *file, return rc; } - *ptmp_inode = new_inode(file->f_path.dentry->d_sb); - if (pTcon->nocase) + if (CIFS_SB(sb)->tcon->nocase) tmp_dentry->d_op = &cifs_ci_dentry_ops; else tmp_dentry->d_op = &cifs_dentry_ops; + + *ptmp_inode = cifs_new_inode(sb, inum); if (*ptmp_inode == NULL) return rc; - if (file->f_path.dentry->d_sb->s_flags & MS_NOATIME) - (*ptmp_inode)->i_flags |= S_NOATIME | S_NOCMTIME; rc = 2; } @@ -842,9 +840,7 @@ static int cifs_get_name_from_search_buf(struct qstr *pqst, len = strnlen(filename, PATH_MAX); } - /* BB fixme - hash low and high 32 bits if not 64 bit arch BB */ - if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) - *pinum = pFindData->UniqueId; + *pinum = pFindData->UniqueId; } else if (level == SMB_FIND_FILE_DIRECTORY_INFO) { FILE_DIRECTORY_INFO *pFindData = (FILE_DIRECTORY_INFO *)current_entry; @@ -940,20 +936,18 @@ static int cifs_filldir(char *pfindEntry, struct file *file, if (rc) return rc; - rc = construct_dentry(&qstring, file, &tmp_inode, &tmp_dentry); + /* only these two infolevels return valid inode numbers */ + if (pCifsF->srch_inf.info_level == SMB_FIND_FILE_UNIX || + pCifsF->srch_inf.info_level == SMB_FIND_FILE_ID_FULL_DIR_INFO) + rc = construct_dentry(&qstring, file, &tmp_inode, &tmp_dentry, + &inum); + else + rc = construct_dentry(&qstring, file, &tmp_inode, &tmp_dentry, + NULL); + if ((tmp_inode == NULL) || (tmp_dentry == NULL)) return -ENOMEM; - if (rc) { - /* inode created, we need to hash it with right inode number */ - if (inum != 0) { - /* BB fixme - hash the 2 32 quantities bits together if - * necessary BB */ - tmp_inode->i_ino = inum; - } - insert_inode_hash(tmp_inode); - } - /* we pass in rc below, indicating whether it is a new inode, so we can figure out whether to invalidate the inode cached data if the file has changed */