On Thu, Apr 01, 2021 at 12:40:34PM +0200, Christian Brauner wrote: > My suggestion was to look at all the places were we currently open-code > this in drivers/: > > drivers/android/binder.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/char/tpm/tpm_vtpm_proxy.c: fd = get_unused_fd_flags(O_RDWR); > drivers/dma-buf/dma-buf.c: fd = get_unused_fd_flags(flags); > drivers/dma-buf/sw_sync.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/dma-buf/sync_file.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > drivers/gpio/gpiolib-cdev.c: fd = get_unused_fd_flags(O_RDONLY | O_CLOEXEC); > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c: fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/drm_atomic_uapi.c: fence_state->fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/drm_lease.c: fd = get_unused_fd_flags(cl->flags & (O_CLOEXEC | O_NONBLOCK)); > drivers/gpu/drm/drm_syncobj.c: fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/drm_syncobj.c: int fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/msm/msm_gem_submit.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/virtio/virtgpu_ioctl.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c: out_fence_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/infiniband/core/rdma_core.c: new_fd = get_unused_fd_flags(O_CLOEXEC); > drivers/media/mc/mc-request.c: fd = get_unused_fd_flags(O_CLOEXEC); > drivers/misc/cxl/api.c: rc = get_unused_fd_flags(flags); > drivers/scsi/cxlflash/ocxl_hw.c: rc = get_unused_fd_flags(flags); > drivers/scsi/cxlflash/ocxl_hw.c: dev_err(dev, "%s: get_unused_fd_flags failed rc=%d\n", > drivers/tty/pty.c: fd = get_unused_fd_flags(flags); > drivers/vfio/vfio.c: ret = get_unused_fd_flags(O_CLOEXEC); > drivers/virt/nitro_enclaves/ne_misc_dev.c: enclave_fd = get_unused_fd_flags(O_CLOEXEC); > > and see whether all of them can be switched to simply using > receive_fd(). I did a completely untested rough sketch to illustrate > what I meant by using binder and devpts Xie seems to have just picked > those two. But the change is obviously only worth it if all or nearly > all callers can be switched over without risk of regression. > It would most likely simplify quite a few codepaths though especially in > the error paths since we can get rid of put_unused_fd() cleanup. > > But it requires buy in from others obviously. Opening a file can have non-trivial side effects; reserving a descriptor can't. Moreover, if you look at the second hit in your list, you'll see that we do *NOT* want that combined thing there - fd_install() is completely irreversible, so we can't do that until we made sure the reply (struct vtpm_proxy_new_dev) has been successfully copied to userland. No, your receive_fd_user() does not cover that. There's a bunch of other cases like that - the next ones on your list are drivers/dma-buf/sw_sync.c and drivers/dma-buf/sync_file.c, etc. So I would consider receive_fd() as an attractive nuisance if it is ever suggested (and available) for wide use... _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
