From: Jeff Layton <jlayton@redhat.com>
To: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
Steve French <smfrench@gmail.com>,
"linux-cifs-client@lists.samba.org"
<linux-cifs-client@lists.samba.org>
Subject: Re: [linux-cifs-client] Re: lookup intent patch
Date: Mon, 30 Mar 2009 13:45:09 -0400 [thread overview]
Message-ID: <20090330134509.565eb9e0@tleilax.poochiereds.net> (raw)
In-Reply-To: <4a4634330903300857g70f1f91cl98cb6dcfae77d21e@mail.gmail.com>
On Mon, 30 Mar 2009 10:57:32 -0500
Shirish Pargaonkar <shirishpargaonkar@gmail.com> 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().
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.
--
Jeff Layton <jlayton@redhat.com>
next prev parent reply other threads:[~2009-03-30 17:45 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <4a4634330902271134h3f334febke42b67ceab7e16eb@mail.gmail.com>
[not found] ` <4a4634330903191336n54758971r2ff809ba31a80791@mail.gmail.com>
2009-03-27 15:15 ` lookup intent patch Shirish Pargaonkar
2009-03-27 18:13 ` Jeff Layton
2009-03-30 15:57 ` Shirish Pargaonkar
2009-03-30 17:45 ` Jeff Layton [this message]
2009-03-30 20:07 ` [linux-cifs-client] " Shirish Pargaonkar
2009-03-31 0:29 ` Jeff Layton
2009-03-31 9:25 ` Shirish Pargaonkar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090330134509.565eb9e0@tleilax.poochiereds.net \
--to=jlayton@redhat.com \
--cc=linux-cifs-client@lists.samba.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=shirishpargaonkar@gmail.com \
--cc=smfrench@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).