Davide Libenzi wrote: > On Thu, 25 Jun 2009, Rusty Russell wrote: > > >> On Thu, 25 Jun 2009 08:15:11 am Davide Libenzi wrote: >> >>> Some components would like to know if userspace dropped the fd, and take >>> proper action accordingly (release resources, drop module instances, >>> etc...). >>> >> Like to know? Possibly. Need to know? Not anything I've seen so far. >> >> If userspace creates the fd, component grab a ref and if userspace wants that >> fd completely freed must close the fd *and* tell component. Simple, race free >> and explicit. All wins. >> >> As this discussion shows, doing some kind of implies non-reference is hard, >> complex and racy. >> > > Easier, we agree. Not doing anything is always easier, provided the > userspace interface allows for it. > Cleaner, I'm not sure. Again, it depends from the userspace interface, but > usually when you close(2) something, you expect the kernel to react > accordingly, and not on relying on userspace issuing extra calls in order > to proper cleanup the kernel context. > This is even more true when the eventfd is the sole handle to the visisble > userspace interface. > In such cases, not taking proper action on close(2) and requiring extra > calls, would lead to designing interface with the close-no-really-i-mean-it > patterns. > Avi, Davide's argument is compelling in favor of considering the latest irqfd fixes I pushed. You could run into weirdness if we tried to revert to the old requirement on explicit DEASSIGN ioctl once we start doing things like handing the fd to other apps/components. Its cleaner IMO to retain the implicit-deassign-on-close behavior. Since v5 also restores optional explicit DEASSIGN support, the only downside to accepting my patches (vs reverting POLLHUP completely) is the complexity required to deal with POLLHUP. But this has been substantially reduced and clarified since yesterdays review, and I am reasonably confident the protocol is sound. Please take a close look at it and consider for merging, if you would. Thanks, -Greg
Attachment:
signature.asc
Description: OpenPGP digital signature
