On Mon, Mar 30, 2009 at 12:45 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote: > On Mon, 30 Mar 2009 10:57:32 -0500 > Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx> wrote: >> >> 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. >> > > I'm not certain since I haven't tested your patch, but you may end up > with an inode refcount leak (aka Busy inodes after umount...). You're > doing an open on the file in the lookup and I think that increases the > refcount of the inode (i_count). Eventually, that inode gets "put" when > you close the file. In the error situation described above though, that > put will never occur. As far as the VFS is concerned, the file was > never actually opened, so it doesn't need to issue a fput(). We would still be in do_flip_open and so if there is an error, while exiting release_open_intent would get called which would so the cleanup i.e. call fput(). Let me introduce an error in between to verify whether the data structures are cleaned up, such as i_count of an inode. > > Properly cleaning up the references is the main reason to make sure > that you pass the filp back to the caller here. Closing the open file > on the server is also a nice side benefit since that could block the > granting of oplocks and such. > I think caller is oblivious to the speed-up mechanism that cifs is attempting by taking advantage of lookup intents to reduce network traffic. > -- > Jeff Layton <jlayton@xxxxxxxxxx> > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
