[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA93t1o0Rupuz0c+8DiCXMNMcNLUEQkuAR5LKiz6omcq9_R55Q@mail.gmail.com>
Date: Wed, 18 Nov 2015 12:28:38 -0800
From: Rajat Jain <rajatxjain@...il.com>
To: Al Viro <viro@...iv.linux.org.uk>
Cc: linux-fsdevel@...r.kernel.org,
Davide Libenzi <davidel@...ilserver.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Anonymous inode cleanup?
Hi Al,
Thanks a lot! That was very helpful. I have one follow up question though.
On Thu, Nov 12, 2015 at 10:31 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> On Thu, Nov 12, 2015 at 09:43:00PM -0800, Rajat Jain wrote:
>> Hello,
>>
>> I'm writing a module that wants to get anonymous fd [using
>> anon_inode_getfd()] and my code looks like this:
>>
>> fd = anon_inode_getfd(...)
>> if (fd < 0)
>> return -EINVAL;
>>
>> if (foobar_fail()) {
>> /* undo everything */
>> return -EINVAL;
>> }
>>
>> My question is that in case of a failure after the anon_inode_getfd(),
>> I want to cleanup and undo whatever needs to be done w.r.t. anodnymous
>> fd I just allocated. (May be put a reference, or return the fd to the
>> free pool or whatever). Can some one please let me know what cleanup
>> needs to be done?
>>
>> However neither I see a cleanup function, nor I see any of the drivers
>> attempting
>> to free the fd in case of failure.
>
> You can't. As soon as it's in descriptor table, you'd better be *done*
> with it. No "I need more setup done", no "I just need to do one final
> check" - the moment it hits the descriptor table, another thread might
> be issuing syscalls on it. Including dup2(), so there's no way to take
> it back. Moreover, another thread might've done dup2() over your
> descriptor, so you can't even decide to close the one you'd just installed.
> Yes, even in cases when the failed action would be to report the resulting
> descriptor to userland. Generally you should try to return descriptors to
> userland only via the syscall return value.
>
> _If_ you are returning them via a sucky API, the right sequence is
> reserve the descriptor(s)
> set the file(s) up
> fill whatever structure you'll be using to report descriptors to
> userland and copy it to userland memory
If this step fails, what is the cleanup needed for "set the files up"
anon_inode_getfile() step? is it fput()?
Thanks,
Rajat
> use fd_install() to put files into descriptor table.
>
> See e.g. fs/pipe.c and look for pipe2 in there for example of dealing with
> such APIs.
>
> "Set the file up" primitive in case of anon_inode is anon_inode_getfile();
> grep and you shall see...
>
> Again, fd_install() is the equivalent of hitting "send" - there's no way
> to make what you've published disappear. It's the point of no return.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists