From mboxrd@z Thu Jan  1 00:00:00 1970
From: Christoph Hellwig <hch@infradead.org>
Subject: Re: [PATCH 06/13] cifs: add new cifs_iget function and convert
	unix codepath to use it
Date: Mon, 11 May 2009 17:06:05 -0400
Message-ID: <20090511210605.GE5751@infradead.org>
References: <1242073472-7100-1-git-send-email-jlayton@redhat.com> <1242073472-7100-7-git-send-email-jlayton@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: linux-cifs-client@lists.samba.org, linux-fsdevel@vger.kernel.org
To: Jeff Layton <jlayton@redhat.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from bombadil.infradead.org ([18.85.46.34]:40270 "EHLO
	bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1756072AbZEKVGE (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Mon, 11 May 2009 17:06:04 -0400
Content-Disposition: inline
In-Reply-To: <1242073472-7100-7-git-send-email-jlayton@redhat.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Mon, May 11, 2009 at 04:24:25PM -0400, Jeff Layton wrote:
> Add a new cifs_iget function that uses iget5_locked to identify inodes.
> This will compare inodes based on the uniqueid value in a cifs_fattr
> struct.
> 
> Rather than filling out an already-created inode, have
> cifs_get_inode_info_unix instead fill out cifs_fattr and hand that off
> to cifs_iget. cifs_iget can then properly look for hardlinked inodes.
> 
> With this, we should then have proper hardlink detection and can
> eventually get rid of some nasty CIFS-specific hacks for handing them.
> 
> Fixing the unix readdir codepath will be done in a later patch.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>

> +	fattr = cifs_unix_basic_to_fattr(presp_data, cifs_sb);
> +	if (!fattr) {
> +		rc = -ENOMEM;
> +		goto posix_open_ret;
> +	}

The cifs_fattr is probably small enough to be stored on-stack in
many places.  And a calling convention where allocations if needed
are done by the caller and cifs_unix_basic_to_fattr just fills the
buffer is probably nicer either way.

>  /*
> + * Allocate a cifs_fattr struct and fill it with fake inode info.
> + *
> + * Needed to setup cifs_fattr data for the directory which is the
> + * junction to the new submount (ie to setup the fake directory
> + * which represents a DFS referral).
> + *
> + * Returns pointer to new cifs_fattr or NULL on error.
>   */

> +static struct cifs_fattr *
> +cifs_create_fake_fattr(struct super_block *sb)

Same comment about caling conventions applies here, too.

Also wou;dn't something like cifs_create_dfsmount_fattr be a better
name?