Re: [RFC PATCH] vfs: shutdown lease notifications on file close

[Dan Williams]

On Fri, Oct 13, 2017 at 10:43 AM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> On Fri, Oct 13, 2017 at 10:01 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>> On Fri, Oct 13, 2017 at 08:56:10AM -0700, Dan Williams wrote:
>>> While implementing MAP_DIRECT, an mmap flag that arranges for an
>>> FL_LAYOUT lease to be established, Al noted:
>>>
>>>     You are not even guaranteed that descriptor will remain be still
>>>     open by the time you pass it down to your helper, nevermind the
>>>     moment when event actually happens...
>>>
>>> The first problem can be solved with an fd{get,put} at mmap
>>> {entry,exit}.
>>
>> Huh?  fdget() does *NOT* guarantee that descriptor won't get closed.  What
>> it does is guarantee that struct file won't get closed under you, which
>> is nowhere near the same thing.  And while we are at it, it certainly
>> _is_ called by mmap()...
>>
>>> The second problem appears to be a general issue.
>>>
>>> Leases follow the lifetime of the inode, so it is possible for a lease
>>> to be broken after the file is closed. When that happens userspace may
>>> get a notification on a stale fd. Of course it is not recommended that a
>>> process close a file descriptor with an active lease, but if it does we
>>> should assume that the notification is not needed either. Walk leases at
>>> close time and invalidate any pending fasync instances.
>>
>> What the hell is special about close(2) and not, e.g. dup2(2)?  Or execve(2)
>> triggering close-on-exec, etc...  Besides, you are changing a user-visible
>> behaviour here.  Suppose your process forks and the child closes all
>> descriptors; should that stop SIGIO delivery to the parent?
>>
>> Let's step back for a minute; could you describe how the userland is supposed
>> to use that thing?
>
> MAP_DIRECT is a meant as a way to safely pass DAX mappings of a file
> to the RDMA sub-system...

Al, before you spend any more time thinking about this let me close
with the RDMA folks on a notification scheme that works for them.