search for: try_module_get

...t; > > > > int ret; > > > > > > > > > > - if (unlikely(!parent->ops->open)) > > > > > + if (unlikely(!ops->open)) > > > > > return -EINVAL; > > > > > > > > > > if (!try_module_get(THIS_MODULE)) > > > > > return -ENODEV; > > > > > > > RJM>] My understanding lately is that this call to > > try_module_get(THIS_MODULE) is no longer needed as is considered as a > > latent bug. > > Quote from > > htt...

...t; > > > > int ret; > > > > > > > > > > - if (unlikely(!parent->ops->open)) > > > > > + if (unlikely(!ops->open)) > > > > > return -EINVAL; > > > > > > > > > > if (!try_module_get(THIS_MODULE)) > > > > > return -ENODEV; > > > > > > > RJM>] My understanding lately is that this call to > > try_module_get(THIS_MODULE) is no longer needed as is considered as a > > latent bug. > > Quote from > > htt...

...; > mdev_get_dev_ops(mdev); > > > > int ret; > > > > > > > > - if (unlikely(!parent->ops->open)) > > > > + if (unlikely(!ops->open)) > > > > return -EINVAL; > > > > > > > > if (!try_module_get(THIS_MODULE)) > > > > return -ENODEV; > > > > RJM>] My understanding lately is that this call to > try_module_get(THIS_MODULE) is no longer needed as is considered as a > latent bug. > Quote from > https://stackoverflow.com/questions/1741415...

...ret; > > > > > > > > > > > > - if (unlikely(!parent->ops->open)) > > > > > > + if (unlikely(!ops->open)) > > > > > > return -EINVAL; > > > > > > > > > > > > if (!try_module_get(THIS_MODULE)) > > > > > > return -ENODEV; > > > > > > > > > > RJM>] My understanding lately is that this call to > > > try_module_get(THIS_MODULE) is no longer needed as is considered as a > > > latent b...

...truct vfio_mdev_device_ops *ops = > > mdev_get_dev_ops(mdev); > > > int ret; > > > > > > - if (unlikely(!parent->ops->open)) > > > + if (unlikely(!ops->open)) > > > return -EINVAL; > > > > > > if (!try_module_get(THIS_MODULE)) > > > return -ENODEV; > RJM>] My understanding lately is that this call to try_module_get(THIS_MODULE) is no longer needed as is considered as a latent bug. Quote from https://stackoverflow.com/questions/1741415/linux-kernel-modules-when-to-use-try-module-g...

...truct vfio_mdev_device_ops *ops = > > mdev_get_dev_ops(mdev); > > > int ret; > > > > > > - if (unlikely(!parent->ops->open)) > > > + if (unlikely(!ops->open)) > > > return -EINVAL; > > > > > > if (!try_module_get(THIS_MODULE)) > > > return -ENODEV; > RJM>] My understanding lately is that this call to try_module_get(THIS_MODULE) is no longer needed as is considered as a latent bug. Quote from https://stackoverflow.com/questions/1741415/linux-kernel-modules-when-to-use-try-module-g...

...e been misled by the kerneldoc into thinking that the references obtained by get_driver() do somehow pin the driver structure in memory. This simply isn''t true; all it pins is the associated private structure. Code that needs to pin a driver must do it some other way (probably by calling try_module_get()). In short, these routines don''t do anything useful and they can actively mislead people. Removing them won''t introduce any bugs that aren''t already present. There is no reason to keep them. Alan Stern -- To unsubscribe from this list: send the line "unsubsc...

...> and vsock_loopback_cancel_pkt() with module exit. There is no other > > reason for using a pointer. > > > > It's cleaner to implement the synchronization once in af_vsock.c (or > > virtio_transport_common.c) instead of making each transport do it. > > Maybe try_module_get() and related APIs provide the necessary semantics > > so that core vsock code can hold the transport module while it's being > > used to send/cancel a packet. > > Right, the module cannot be unloaded until open sockets, so here the > synchronization is not needed. > &...

...> and vsock_loopback_cancel_pkt() with module exit. There is no other > > reason for using a pointer. > > > > It's cleaner to implement the synchronization once in af_vsock.c (or > > virtio_transport_common.c) instead of making each transport do it. > > Maybe try_module_get() and related APIs provide the necessary semantics > > so that core vsock code can hold the transport module while it's being > > used to send/cancel a packet. > > Right, the module cannot be unloaded until open sockets, so here the > synchronization is not needed. > &...

...> Just to clarify, a new type of mdev driver but provides dummy > vfio_device_ops for VFIO to make container DMA ioctl work. I see. Thanks! IIUC, you mean we can provide a very tiny VFIO device driver in drivers/vhost/mdev.c, e.g.: static int vfio_vhost_mdev_open(void *device_data) { if (!try_module_get(THIS_MODULE)) return -ENODEV; return 0; } static void vfio_vhost_mdev_release(void *device_data) { module_put(THIS_MODULE); } static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = { .name = "vfio-vhost-mdev", .open = vfio_vhost_mdev_open, .release = vfio_vhost_mdev_rele...

...> Just to clarify, a new type of mdev driver but provides dummy > vfio_device_ops for VFIO to make container DMA ioctl work. I see. Thanks! IIUC, you mean we can provide a very tiny VFIO device driver in drivers/vhost/mdev.c, e.g.: static int vfio_vhost_mdev_open(void *device_data) { if (!try_module_get(THIS_MODULE)) return -ENODEV; return 0; } static void vfio_vhost_mdev_release(void *device_data) { module_put(THIS_MODULE); } static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = { .name = "vfio-vhost-mdev", .open = vfio_vhost_mdev_open, .release = vfio_vhost_mdev_rele...

...s really a way to synchronize vsock_loopback_send_pkt() and vsock_loopback_cancel_pkt() with module exit. There is no other reason for using a pointer. It's cleaner to implement the synchronization once in af_vsock.c (or virtio_transport_common.c) instead of making each transport do it. Maybe try_module_get() and related APIs provide the necessary semantics so that core vsock code can hold the transport module while it's being used to send/cancel a packet. > +MODULE_ALIAS_NETPROTO(PF_VSOCK); Why does this module define the alias for PF_VSOCK? Doesn't another module already define this al...

...s really a way to synchronize vsock_loopback_send_pkt() and vsock_loopback_cancel_pkt() with module exit. There is no other reason for using a pointer. It's cleaner to implement the synchronization once in af_vsock.c (or virtio_transport_common.c) instead of making each transport do it. Maybe try_module_get() and related APIs provide the necessary semantics so that core vsock code can hold the transport module while it's being used to send/cancel a packet. > +MODULE_ALIAS_NETPROTO(PF_VSOCK); Why does this module define the alias for PF_VSOCK? Doesn't another module already define this al...

...x 5f85b1b..96f99c7 100644 --- a/drivers/pci/hotplug/pci_hotplug_core.c +++ b/drivers/pci/hotplug/pci_hotplug_core.c @@ -102,13 +102,13 @@ static int get_##name (struct hotplug_slot *slot, type *value) \ { \ struct hotplug_slot_ops *ops = slot->ops; \ int retval = 0; \ - if (try_module_get(ops->owner)) { \ - if (ops->get_##name) \ - retval = ops->get_##name(slot, value); \ - else \ - *value = slot->info->name; \ - module_put(ops->owner); \ - } \ + if (!try_module_get(ops->owner)) \ + return -ENODEV; \ + if (ops->get_#...

...x 5f85b1b..96f99c7 100644 --- a/drivers/pci/hotplug/pci_hotplug_core.c +++ b/drivers/pci/hotplug/pci_hotplug_core.c @@ -102,13 +102,13 @@ static int get_##name (struct hotplug_slot *slot, type *value) \ { \ struct hotplug_slot_ops *ops = slot->ops; \ int retval = 0; \ - if (try_module_get(ops->owner)) { \ - if (ops->get_##name) \ - retval = ops->get_##name(slot, value); \ - else \ - *value = slot->info->name; \ - module_put(ops->owner); \ - } \ + if (!try_module_get(ops->owner)) \ + return -ENODEV; \ + if (ops->get_#...

...x 5f85b1b..96f99c7 100644 --- a/drivers/pci/hotplug/pci_hotplug_core.c +++ b/drivers/pci/hotplug/pci_hotplug_core.c @@ -102,13 +102,13 @@ static int get_##name (struct hotplug_slot *slot, type *value) \ { \ struct hotplug_slot_ops *ops = slot->ops; \ int retval = 0; \ - if (try_module_get(ops->owner)) { \ - if (ops->get_##name) \ - retval = ops->get_##name(slot, value); \ - else \ - *value = slot->info->name; \ - module_put(ops->owner); \ - } \ + if (!try_module_get(ops->owner)) \ + return -ENODEV; \ + if (ops->get_#...

...gt; > > vfio_device_ops for VFIO to make container DMA ioctl work. > > I see. Thanks! IIUC, you mean we can provide a very tiny > > VFIO device driver in drivers/vhost/mdev.c, e.g.: > > > > static int vfio_vhost_mdev_open(void *device_data) > > { > > if (!try_module_get(THIS_MODULE)) > > return -ENODEV; > > return 0; > > } > > > > static void vfio_vhost_mdev_release(void *device_data) > > { > > module_put(THIS_MODULE); > > } > > > > static const struct vfio_device_ops vfio_vhost_mdev_dev_ops = { &gt...

...l.h> #include <linux/slab.h> #include <linux/vfio.h> #include <linux/mdev.h> #include "mdev_private.h" /* XXX: we need a proper way to include below vhost header. */ #include "../../vhost/vhost.h" static int vfio_vhost_mdev_open(void *device_data) { if (!try_module_get(THIS_MODULE)) return -ENODEV; /* ... */ vhost_dev_init(...); return 0; } static void vfio_vhost_mdev_release(void *device_data) { /* ... */ module_put(THIS_MODULE); } static long vfio_vhost_mdev_unlocked_ioctl(void *device_data, unsigned int cmd, unsigned long arg) { struct mdev...

...l.h> #include <linux/slab.h> #include <linux/vfio.h> #include <linux/mdev.h> #include "mdev_private.h" /* XXX: we need a proper way to include below vhost header. */ #include "../../vhost/vhost.h" static int vfio_vhost_mdev_open(void *device_data) { if (!try_module_get(THIS_MODULE)) return -ENODEV; /* ... */ vhost_dev_init(...); return 0; } static void vfio_vhost_mdev_release(void *device_data) { /* ... */ module_put(THIS_MODULE); } static long vfio_vhost_mdev_unlocked_ioctl(void *device_data, unsigned int cmd, unsigned long arg) { struct mdev...

...v.h> > > > > #include "mdev_private.h" > > > > /* XXX: we need a proper way to include below vhost header. */ > > #include "../../vhost/vhost.h" > > > > static int vfio_vhost_mdev_open(void *device_data) > > { > > if (!try_module_get(THIS_MODULE)) > > return -ENODEV; > > > > /* ... */ > > vhost_dev_init(...); > > > > return 0; > > } > > > > static void vfio_vhost_mdev_release(void *device_data) > > { > > /* ... */ > > module_put(THIS_MODULE); >...