[linux-cifs-client,2/3] cifs: use iunique() in cifs_new_inode()
diff mbox

Message ID 1234357708-24624-2-git-send-email-jlayton@redhat.com
State New, archived
Headers show

Commit Message

Jeff Layton Feb. 11, 2009, 1:08 p.m. UTC
...to make sure that we get unique inode numbers when we're generating
them on the fly.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/inode.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

Comments

Steve French Feb. 11, 2009, 3:10 p.m. UTC | #1
smbfs and ncpfs and fat use iunique but a comment in the kernel source warns
about using it since it is slow.  Wonder if it is measurably slow?

On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <jlayton@redhat.com> wrote:

> ...to make sure that we get unique inode numbers when we're generating
> them on the fly.
>
>
Jeff Layton Feb. 11, 2009, 3:33 p.m. UTC | #2
On Wed, 11 Feb 2009 09:10:35 -0600
Steve French <smfrench@gmail.com> wrote:

> smbfs and ncpfs and fat use iunique but a comment in the kernel source warns
> about using it since it is slow.  Wonder if it is measurably slow?
> 
> On Wed, Feb 11, 2009 at 7:08 AM, Jeff Layton <jlayton@redhat.com> wrote:
> 
> > ...to make sure that we get unique inode numbers when we're generating
> > them on the fly.
> >
> >
> 
> 

I'm sure there will be some performance impact. If you really want to
bench it out correctly, you'll probably need to generate a ton of
inodes on a mount so that you can ensure that each hash bucket will
have a lot. Ideally you'd also want to make the counter in iunique
wrap too so that it has to look for a while for a free inode. 

In practice, I doubt it'll be noticable -- certainly it should still be
faster than a round trip to the server. Even if there is some
performance impact though, I think it's still the right thing to do.

Patch
diff mbox

diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 475115c..91d63f3 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -224,11 +224,12 @@  cifs_new_inode(struct super_block *sb, __u64 *inum)
 	/*
 	 * 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()?
+	 *     ensure that the server is really filling in that field?
 	 */
 	if (inum && (CIFS_SB(sb)->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM))
 		inode->i_ino = (unsigned long) *inum;
+	else
+		inode->i_ino = iunique(sb, 2);
 
 	/*
 	 * must set this here instead of cifs_alloc_inode since VFS will