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 <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>

diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index ec9f9c1..62fd5bd 100644 (file)
--- 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 __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,
 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 7342bfb..c7674f5 100644 (file)
--- 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);
 }
 
        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)
 {
 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) {
 
        /* 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;
                }
                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;
        }
 
        inode = *pinode;


@@ -465,11 +498,8 @@ int cifs_get_inode_info(struct inode **pinode,
 
        /* get new inode */
        if (*pinode == NULL) {
 
        /* 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
                /* 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;
 
                if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) {
                        int rc1 = 0;

-                       __u64 inode_num;

 
                        rc1 = CIFSGetSrvInodeNumber(xid, pTcon,
                                        full_path, &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? */
                        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);
        }
        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;
 
                        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;
                        }
 
                        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;
                        newinode->i_nlink = 2;

-
-                       insert_inode_hash(newinode);

                        d_instantiate(direntry, newinode);
 
                        /* we already checked in POSIXCreate whether
                        d_instantiate(direntry, newinode);
 
                        /* we already checked in POSIXCreate whether

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 9f51f9b..02a2022 100644 (file)
--- 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 */
 
 }
 #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 */
 /* 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));
        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) {
 
        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;
                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) {
                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 (*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) {
        } 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;
                }
 
                        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;
                        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 (*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;
        }
 
                rc = 2;
        }
 


@@ -842,9 +840,7 @@ static int cifs_get_name_from_search_buf(struct qstr *pqst,
                        len = strnlen(filename, PATH_MAX);
                }
 
                        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;
        } 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;
 
        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 ((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 */
        /* 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 */