From mboxrd@z Thu Jan  1 00:00:00 1970
From: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Subject: Re: lookup intent patch
Date: Mon, 30 Mar 2009 10:57:32 -0500
Message-ID: <4a4634330903300857g70f1f91cl98cb6dcfae77d21e@mail.gmail.com>
References: <4a4634330902271134h3f334febke42b67ceab7e16eb@mail.gmail.com>
	 <4a4634330903191336n54758971r2ff809ba31a80791@mail.gmail.com>
	 <4a4634330903270815j415947f6ja190b884583de055@mail.gmail.com>
	 <20090327141322.6089f72f@tleilax.poochiereds.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: Steve French <smfrench@gmail.com>,
	Suresh Jayaraman <sjayaraman@novell.com>,
	"linux-cifs-client@lists.samba.org"
	<linux-cifs-client@lists.samba.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>
To: Jeff Layton <jlayton@redhat.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from yx-out-2324.google.com ([74.125.44.29]:9132 "EHLO
	yx-out-2324.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751323AbZC3P5f convert rfc822-to-8bit (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Mon, 30 Mar 2009 11:57:35 -0400
Received: by yx-out-2324.google.com with SMTP id 31so2029181yxl.1
        for <linux-fsdevel@vger.kernel.org>; Mon, 30 Mar 2009 08:57:32 -0700 (PDT)
In-Reply-To: <20090327141322.6089f72f@tleilax.poochiereds.net>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Fri, Mar 27, 2009 at 1:13 PM, Jeff Layton <jlayton@redhat.com> wrote=
:
> On Fri, 27 Mar 2009 10:15:21 -0500
> Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote:
>
> Please run this through checkpatch.pl, there is a lot of bad whitespa=
ce
> in this patch:
>
>> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
>> index f9b6f68..011d0ac 100644
>> --- a/fs/cifs/dir.c
>> +++ b/fs/cifs/dir.c
>> @@ -129,12 +129,67 @@ cifs_bp_rename_retry:
>> =A0 =A0 =A0 return full_path;
>> =A0}
>>
>> +static void
>> +cifs_fill_fileinfo(struct inode *newinode, __u16 fileHandle,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct cifsTconInfo *tcon,=
 bool write_only)
>> +{
>> + =A0 =A0 int oplock =3D 0;
>> + =A0 =A0 struct cifsFileInfo *pCifsFile;
>> + =A0 =A0 struct cifsInodeInfo *pCifsInode;
>> +
>> + =A0 =A0 cERROR(1, ("cifs_fill_fileinfo enter\n"));
>> +
>> + =A0 =A0 pCifsFile =3D kzalloc(sizeof(struct cifsFileInfo), GFP_KER=
NEL);
>> +
>> + =A0 =A0 if (pCifsFile =3D=3D NULL)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return;
>> +
>> + =A0 =A0 if (oplockEnabled)
>> + =A0 =A0 =A0 =A0 =A0 =A0 oplock =3D REQ_OPLOCK;
>> +
>> + =A0 =A0 pCifsFile->netfid =3D fileHandle;
>> + =A0 =A0 pCifsFile->pid =3D current->tgid;
>> + =A0 =A0 pCifsFile->pInode =3D newinode;
>> + =A0 =A0 pCifsFile->invalidHandle =3D false;
>> + =A0 =A0 pCifsFile->closePend =A0 =A0 =3D false;
>> + =A0 =A0 init_MUTEX(&pCifsFile->fh_sem);
> =A0 =A0 =A0 =A0 =A0 =A0^^^^
> Can you convert this to a mutex while you're at it? Might not hurt to=
 make
> that a separate patch though.
>
>> + =A0 =A0 mutex_init(&pCifsFile->lock_mutex);
>> + =A0 =A0 INIT_LIST_HEAD(&pCifsFile->llist);
>> + =A0 =A0 atomic_set(&pCifsFile->wrtPending, 0);
>> +
>> + =A0 =A0 /* set the following in open now
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile->pfile =3D file;=
 */
>> + =A0 =A0 write_lock(&GlobalSMBSeslock);
>> + =A0 =A0 list_add(&pCifsFile->tlist, &tcon->openFileList);
>> + =A0 =A0 pCifsInode =3D CIFS_I(newinode);
>> + =A0 =A0 if (pCifsInode) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 /* if readable file instance put first in =
list*/
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (write_only) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 list_add_tail(&pCifsFile->=
flist,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 &pCifsInode->openFileList);
>> + =A0 =A0 =A0 =A0 =A0 =A0 } else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 list_add(&pCifsFile->flist=
,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0&pCifsI=
node->openFileList);
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0 =A0 =A0 if ((oplock & 0xF) =3D=3D OPLOCK_EXCLUSIVE=
) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode->clientCanCache=
All =3D true;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode->clientCanCache=
Read =3D true;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cFYI(1, ("Exclusive Oplock=
 inode %p",
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 newinode))=
;
>> + =A0 =A0 =A0 =A0 =A0 =A0 } else if ((oplock & 0xF) =3D=3D OPLOCK_RE=
AD)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode->clientCanCache=
Read =3D true;
>> + =A0 =A0 }
>> + =A0 =A0 write_unlock(&GlobalSMBSeslock);
>> + =A0 =A0 cERROR(1, ("cifs_fill_fileinfo exit\n"));
>> +}
>> +
>> =A0int cifs_posix_open(char *full_path, struct inode **pinode,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct super_block *sb, int mode=
, int oflags,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 int *poplock, __u16 *pnetfid, in=
t xid)
>> =A0{
>> =A0 =A0 =A0 int rc;
>> =A0 =A0 =A0 __u32 oplock;
>> + =A0 =A0 bool write_only =3D false;
>> =A0 =A0 =A0 FILE_UNIX_BASIC_INFO *presp_data;
>> =A0 =A0 =A0 __u32 posix_flags =3D 0;
>> =A0 =A0 =A0 struct cifs_sb_info *cifs_sb =3D CIFS_SB(sb);
>> @@ -172,6 +227,8 @@ int cifs_posix_open(char *full_path, struct inod=
e **pinode,
>> =A0 =A0 =A0 if (oflags & O_DIRECT)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 posix_flags |=3D SMB_O_DIRECT;
>>
>> + =A0 =A0 =A0 =A0if (!(oflags & FMODE_READ))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0write_only =3D true;
>>
>> =A0 =A0 =A0 rc =3D CIFSPOSIXCreate(xid, cifs_sb->tcon, posix_flags, =
mode,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pnetfid, presp_data, &op=
lock, full_path,
>> @@ -198,6 +255,8 @@ int cifs_posix_open(char *full_path, struct inod=
e **pinode,
>>
>> =A0 =A0 =A0 posix_fill_in_inode(*pinode, presp_data, 1);
>>
>> + =A0 =A0 cifs_fill_fileinfo(*pinode, *pnetfid, cifs_sb->tcon, write=
_only);
>> +
>> =A0posix_open_ret:
>> =A0 =A0 =A0 kfree(presp_data);
>> =A0 =A0 =A0 return rc;
>> @@ -239,7 +298,6 @@ cifs_create(struct inode *inode, struct dentry *=
direntry, int mode,
>> =A0 =A0 =A0 char *full_path =3D NULL;
>> =A0 =A0 =A0 FILE_ALL_INFO *buf =3D NULL;
>> =A0 =A0 =A0 struct inode *newinode =3D NULL;
>> - =A0 =A0 struct cifsInodeInfo *pCifsInode;
>> =A0 =A0 =A0 int disposition =3D FILE_OVERWRITE_IF;
>> =A0 =A0 =A0 bool write_only =3D false;
>>
>> @@ -410,44 +468,8 @@ cifs_create_set_dentry:
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* mknod case - do not leave file open *=
/
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 CIFSSMBClose(xid, tcon, fileHandle);
>> =A0 =A0 =A0 } else if (newinode) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 struct cifsFileInfo *pCifsFile =3D
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 kzalloc(sizeof(struct cifs=
=46ileInfo), GFP_KERNEL);
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (pCifsFile =3D=3D NULL)
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto cifs_create_out;
>> - =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile->netfid =3D fileHandle;
>> - =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile->pid =3D current->tgid;
>> - =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile->pInode =3D newinode;
>> - =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile->invalidHandle =3D false;
>> - =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile->closePend =A0 =A0 =3D false;
>> - =A0 =A0 =A0 =A0 =A0 =A0 init_MUTEX(&pCifsFile->fh_sem);
>> - =A0 =A0 =A0 =A0 =A0 =A0 mutex_init(&pCifsFile->lock_mutex);
>> - =A0 =A0 =A0 =A0 =A0 =A0 INIT_LIST_HEAD(&pCifsFile->llist);
>> - =A0 =A0 =A0 =A0 =A0 =A0 atomic_set(&pCifsFile->wrtPending, 0);
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* set the following in open now
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile-=
>pfile =3D file; */
>> - =A0 =A0 =A0 =A0 =A0 =A0 write_lock(&GlobalSMBSeslock);
>> - =A0 =A0 =A0 =A0 =A0 =A0 list_add(&pCifsFile->tlist, &tcon->openFil=
eList);
>> - =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode =3D CIFS_I(newinode);
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (pCifsInode) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* if readable file instan=
ce put first in list*/
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (write_only) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 list_add_t=
ail(&pCifsFile->flist,
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 &pCifsInode->openFileList);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 list_add(&=
pCifsFile->flist,
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0&pCifsInode->openFileList);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((oplock & 0xF) =3D=3D =
OPLOCK_EXCLUSIVE) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode=
->clientCanCacheAll =3D true;
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode=
->clientCanCacheRead =3D true;
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cFYI(1, ("=
Exclusive Oplock inode %p",
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 newinode));
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else if ((oplock & 0xF) =
=3D=3D OPLOCK_READ)
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode=
->clientCanCacheRead =3D true;
>> - =A0 =A0 =A0 =A0 =A0 =A0 }
>> - =A0 =A0 =A0 =A0 =A0 =A0 write_unlock(&GlobalSMBSeslock);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cifs_fill_fileinfo(newinod=
e, fileHandle,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 cifs_sb->tcon, write_only);
>> =A0 =A0 =A0 }
>> =A0cifs_create_out:
>> =A0 =A0 =A0 kfree(buf);
>> @@ -580,13 +602,16 @@ int cifs_mknod(struct inode *inode, struct den=
try *direntry, int mode,
>> =A0 =A0 =A0 return rc;
>> =A0}
>>
>> -
>> =A0struct dentry *
>> =A0cifs_lookup(struct inode *parent_dir_inode, struct dentry *dirent=
ry,
>> =A0 =A0 =A0 =A0 =A0 struct nameidata *nd)
>> =A0{
>> =A0 =A0 =A0 int xid;
>> =A0 =A0 =A0 int rc =3D 0; /* to get around spurious gcc warning, set=
 to zero here */
>> + =A0 =A0 int oplock =3D 0;
>> + =A0 =A0 int mode;
>> + =A0 =A0 __u16 fileHandle =3D 0;
>> + =A0 =A0 bool posix_open =3D false;
>> =A0 =A0 =A0 struct cifs_sb_info *cifs_sb;
>> =A0 =A0 =A0 struct cifsTconInfo *pTcon;
>> =A0 =A0 =A0 struct inode *newInode =3D NULL;
>> @@ -632,12 +657,26 @@ cifs_lookup(struct inode *parent_dir_inode, st=
ruct dentry *direntry,
>> =A0 =A0 =A0 }
>> =A0 =A0 =A0 cFYI(1, ("Full path: %s inode =3D 0x%p", full_path, dire=
ntry->d_inode));
>>
>> - =A0 =A0 if (pTcon->unix_ext)
>> - =A0 =A0 =A0 =A0 =A0 =A0 rc =3D cifs_get_inode_info_unix(&newInode,=
 full_path,
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 parent_dir_inode->i_sb, xid);
>> - =A0 =A0 else
>> + =A0 =A0 if (pTcon->unix_ext) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_=
DIRECTORY)) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (nd->flags=
 & LOOKUP_OPEN)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!((nd->intent.open.fla=
gs & O_CREAT ) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 (nd->intent.open.flags & O_EXCL))) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode =3D n=
d->intent.open.create_mode &
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 ~current->fs->umask;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D cif=
s_posix_open(full_path, &newInode,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 parent_dir_inode->i_sb, mode,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 nd->intent.open.flags, &oplock,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 &fileHandle, xid);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 posix_open=
 =3D true;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (!posix_open || ((rc =3D=3D -EINVAL) ||=
 (rc =3D=3D -EOPNOTSUPP)))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D cifs_get_inode_info=
_unix(&newInode, full_path,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0 parent_dir_inode->i_sb, xid);
>> + =A0 =A0 } else
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D cifs_get_inode_info(&newInode, fu=
ll_path, NULL,
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0parent_dir_inode->i_sb, xid, NULL);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 parent_dir=
_inode->i_sb, xid, NULL);
>>
>
> Ok, so you're opening the file on lookup now. Aren't you supposed to =
use
> lookup_instantiate_filp() to pass an instantiated file pointer back t=
o the
> caller? If you don't do this, then I think you're susceptible to leak=
ing
> this open file if an error occurs between the lookup and the open.
>
> nfs4 does something very similar to what you're trying to do here. Yo=
u
> might want to look carefully at it as an example. nfs4_atomic_open()
> is a good place to start.
>
>> =A0 =A0 =A0 if ((rc =3D=3D 0) && (newInode !=3D NULL)) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (pTcon->nocase)
>> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
>> index 81747ac..4a7c886 100644
>> --- a/fs/cifs/file.c
>> +++ b/fs/cifs/file.c
>> @@ -284,35 +284,34 @@ int cifs_open(struct inode *inode, struct file=
 *file)
>> =A0 =A0 =A0 cifs_sb =3D CIFS_SB(inode->i_sb);
>> =A0 =A0 =A0 tcon =3D cifs_sb->tcon;
>>
>> - =A0 =A0 if (file->f_flags & O_CREAT) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 /* search inode for this file and fill in =
file->private_data */
>> - =A0 =A0 =A0 =A0 =A0 =A0 pCifsInode =3D CIFS_I(file->f_path.dentry-=
>d_inode);
>> - =A0 =A0 =A0 =A0 =A0 =A0 read_lock(&GlobalSMBSeslock);
>> - =A0 =A0 =A0 =A0 =A0 =A0 list_for_each(tmp, &pCifsInode->openFileLi=
st) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile =3D list_entry(t=
mp, struct cifsFileInfo,
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0 =A0 =A0flist);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if ((pCifsFile->pfile =3D=3D=
 NULL) &&
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (pCifsFile->pid =3D=
=3D current->tgid)) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* mode se=
t in cifs_create */
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* needed =
for writepage */
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile-=
>pfile =3D file;
>> -
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 file->priv=
ate_data =3D pCifsFile;
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> - =A0 =A0 =A0 =A0 =A0 =A0 }
>> - =A0 =A0 =A0 =A0 =A0 =A0 read_unlock(&GlobalSMBSeslock);
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (file->private_data !=3D NULL) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 rc =3D 0;
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 FreeXid(xid);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return rc;
>> - =A0 =A0 =A0 =A0 =A0 =A0 } else {
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (file->f_flags & O_EXCL=
)
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cERROR(1, =
("could not find file instance for "
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 =A0"new file %p", file));
>> + =A0 =A0 /* search inode for this file and fill in file->private_da=
ta */
>> + =A0 =A0 pCifsInode =3D CIFS_I(file->f_path.dentry->d_inode);
>> + =A0 =A0 read_lock(&GlobalSMBSeslock);
>> + =A0 =A0 list_for_each(tmp, &pCifsInode->openFileList) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile =3D list_entry(tmp, struct cifsF=
ileInfo,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0flist);
>> + =A0 =A0 =A0 =A0 =A0 =A0 if ((pCifsFile->pfile =3D=3D NULL) &&
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (pCifsFile->pid =3D=3D current->tg=
id)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* mode set in cifs_create=
 */
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* needed for writepage */
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 pCifsFile->pfile =3D file;
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 file->private_data =3D pCi=
fsFile;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
>> =A0 =A0 =A0 }
>> + =A0 =A0 read_unlock(&GlobalSMBSeslock);
>> +
>> + =A0 =A0 if (file->private_data !=3D NULL) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 rc =3D 0;
>> + =A0 =A0 =A0 =A0 =A0 =A0 FreeXid(xid);
>> + =A0 =A0 =A0 =A0 =A0 =A0 return rc;
>> + =A0 =A0 } else {
>> + =A0 =A0 =A0 =A0 =A0 =A0 if ((file->f_flags & O_CREAT) && (file->f_=
flags & O_EXCL))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 cERROR(1, ("could not find=
 file instance for "
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"ne=
w file %p", file));
>> + =A0 =A0 }
>>
>> =A0 =A0 =A0 full_path =3D build_path_from_dentry(file->f_path.dentry=
);
>> =A0 =A0 =A0 if (full_path =3D=3D NULL) {
>
>
> --
> Jeff Layton <jlayton@redhat.com>
>

Jeff,

Thanks.  Looking into it. I am trying to figure out the need/necessity
for cifs_lookup to call lookup_instanitate_flip.
lookup_instantiate_filp does call dentry_open and if cifs_lookup does
not call lookup_instantiate_flip,
nameidata_to_filp will call dentry_open.
So I am not sure what we loose if dentry_open does not get called
between lookup_hash and nameidata_to_flip
because of an error between those two calls, specifically how will the
cause of open file getting closed on the
server will be served if there was an in-betwen error by calling
lookup_instantiate_filp.

Regards,

Shirish
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel=
" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html