On Fri, Sep 9, 2016 at 9:26 PM, Timmons C. Player <
[email protected]> wrote:
> There is a potential lock order inversion issue here. Closing
> the file from the network side requires taking the file lock
> after taking the socket lock. Conversely, polling a socket from
> epoll takes the file lock and then takes the socket lock.
>
> Bumping the reference count here ensures that the networking
> side won't attempt to close the socket while we're polling, and
> hence won't need to take the file lock.
>
I still need to think about this issue, but something is strange for me:
Commit 38e361ff64c2c4328d8a90a48e329063305a379d suggests that:
The normal lock order is the socket lock, then file::f_lock, but
socket_file::epoll_add() inverts that order.
So I wonder why does polling a socket from epoll do (according to what you
said) the opposite of what is described here as the "normal lock order",
and why wasn't this noticed in the above commit?
>
> Signed-off-by: Timmons C. Player <[email protected]>
> ---
> core/epoll.cc | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/core/epoll.cc b/core/epoll.cc
> index f425c4a..9d7b890 100644
> --- a/core/epoll.cc
> +++ b/core/epoll.cc
> @@ -187,7 +187,9 @@ public:
> epoll_event& evt = found->second;
> int active = 0;
> if (evt.events) {
> + fhold(key._file);
> active = key._file->poll(events_epoll_
> to_poll(evt.events));
> + fdrop(key._file);
> }
> active = events_poll_to_epoll(active);
> if (!active || (evt.events & EPOLLET)) {
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
>
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
