On Thu, 27 Jun 2019 10:19:14 +0200 Cornelia Huck <coh...@redhat.com> wrote:
> On Wed, 26 Jun 2019 08:27:58 -0600 > Alex Williamson <alex.william...@redhat.com> wrote: > > > This allows udev to trigger rules when a parent device is registered > > or unregistered from mdev. > > > > Signed-off-by: Alex Williamson <alex.william...@redhat.com> > > --- > > drivers/vfio/mdev/mdev_core.c | 10 ++++++++-- > > 1 file changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c > > index ae23151442cb..ecec2a3b13cb 100644 > > --- a/drivers/vfio/mdev/mdev_core.c > > +++ b/drivers/vfio/mdev/mdev_core.c > > @@ -146,6 +146,8 @@ int mdev_register_device(struct device *dev, const > > struct mdev_parent_ops *ops) > > { > > int ret; > > struct mdev_parent *parent; > > + char *env_string = "MDEV_STATE=registered"; > > This string is probably reasonable enough. > > > + char *envp[] = { env_string, NULL }; > > > > /* check for mandatory ops */ > > if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups) > > @@ -196,7 +198,8 @@ int mdev_register_device(struct device *dev, const > > struct mdev_parent_ops *ops) > > list_add(&parent->next, &parent_list); > > mutex_unlock(&parent_list_lock); > > > > - dev_info(dev, "MDEV: Registered\n"); > > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp); > > I also agree with the positioning here. > > > + > > return 0; > > > > add_dev_err: > > @@ -220,6 +223,8 @@ EXPORT_SYMBOL(mdev_register_device); > > void mdev_unregister_device(struct device *dev) > > { > > struct mdev_parent *parent; > > + char *env_string = "MDEV_STATE=unregistered"; > > Ok. > > > + char *envp[] = { env_string, NULL }; > > > > mutex_lock(&parent_list_lock); > > parent = __find_parent_device(dev); > > @@ -228,7 +233,6 @@ void mdev_unregister_device(struct device *dev) > > mutex_unlock(&parent_list_lock); > > return; > > } > > - dev_info(dev, "MDEV: Unregistering\n"); > > > > list_del(&parent->next); > > mutex_unlock(&parent_list_lock); > > @@ -243,6 +247,8 @@ void mdev_unregister_device(struct device *dev) > > up_write(&parent->unreg_sem); > > > > mdev_put_parent(parent); > > + > > + kobject_uevent_env(&dev->kobj, KOBJ_CHANGE, envp); > > I'm wondering whether we should indicate this uevent earlier: Once we > have detached from the parent list, we're basically done for all > practical purposes. So maybe move this right before we grab the > unreg_sem? That would make it a "this thing is about to go away" (ie. "unregistering") rather than "this thing is gone" ("unregistered"). I was aiming for the latter as the former just seems like it might make userspace race to remove devices. Note that I don't actually make use of this event in mdevctl currently, so we could maybe save it for later, but the symmetry seemed preferable. Thanks, Alex