Hello, On Thu, 2022-11-24 at 15:02 -0800, Eric Biggers wrote: > On Thu, Nov 24, 2022 at 06:57:41PM +0100, Paolo Abeni wrote: > > To reduce the contention this patch introduces explicit reference counting > > for the eventpoll struct. Each registered event acquires a reference, > > and references are released at ep_remove() time. ep_free() doesn't touch > > anymore the event RB tree, it just unregisters the existing callbacks > > and drops a reference to the ep struct. The struct itself is freed when > > the reference count reaches 0. The reference count updates are protected > > by the mtx mutex so no additional atomic operations are needed. > > So, the behavior before this patch is that closing an epoll file frees all > resources associated with it. This behavior is documented in the man page > epoll_create(2): "When all file descriptors referring to an epoll instance have > been closed, the kernel destroys the instance and releases the associated > resources for reuse." > > The behavior after this patch is that the resources aren't freed until the epoll > file *and* all files that were added to it have been closed. > > Is that okay? This is actually the question that I intended to raise here. I should have probably make it explicit. Also thank you for pointing out the man page info, at very least this patch would require updating it - or possibly that is a reason to shot this patch completelly. I would love to ear more opinions ;) > I suppose in most cases it is, since the usual use case for epoll > is to have a long-lived epoll instance and shorter lived file descriptors that > are polled using that long-lived epoll instance. > > But probably some users do things the other way around. I.e., they have a > long-lived file descriptor that is repeatedly polled using different epoll > instances that have a shorter lifetime. > > In that case, the number of 'struct eventpoll' and 'struct epitem' in kernel > memory will keep growing until 'max_user_watches' is hit, at which point > EPOLL_CTL_ADD will start failing with ENOSPC. > > Are you sure that is fine? I did not think about such use-case, thank you for pointing that out! Even if it looks like quite a corner-case, it also looks like quite a deal breaker to me. Again other opinions more then welcome! ;) Please allow me a question: do you think that solving the contention problem reported here inside the kernel is worthy? Or should we encourage (or enforce) the user-space to always do EPOLL_CTL_DEL for better scalability? Thanks, Paolo
