[Intel-gfx] [RFC v2 1/8] drm: provide management functions for drm_file
From: David Herrmann Rather than doing drm_file allocation/destruction right in the fops, lets provide separate helpers. This decouples drm_file management from the still-mandatory drm-fops. It prepares for use of drm_file without the fops, both by possible separate fops implementations and APIs (not that I am aware of any such plans), and more importantly from in-kernel use where no real file is available. Signed-off-by: David Herrmann [rebased] Signed-off-by: Noralf Trønnes --- drivers/gpu/drm/drm_file.c | 309 +++-- drivers/gpu/drm/drm_internal.h | 2 + 2 files changed, 179 insertions(+), 132 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index b3c6e997ccdb..d208faade27e 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -101,6 +101,179 @@ DEFINE_MUTEX(drm_global_mutex); static int drm_open_helper(struct file *filp, struct drm_minor *minor); +/** + * drm_file_alloc - allocate file context + * @minor: minor to allocate on + * + * This allocates a new DRM file context. It is not linked into any context and + * can be used by the caller freely. Note that the context keeps a pointer to + * @minor, so it must be freed before @minor is. + * + * The legacy paths might require the drm_global_mutex to be held. + * + * RETURNS: + * Pointer to newly allocated context, ERR_PTR on failure. + */ +struct drm_file *drm_file_alloc(struct drm_minor *minor) +{ + struct drm_device *dev = minor->dev; + struct drm_file *file; + int ret; + + file = kzalloc(sizeof(*file), GFP_KERNEL); + if (!file) + return ERR_PTR(-ENOMEM); + + file->pid = get_pid(task_pid(current)); + file->minor = minor; + + /* for compatibility root is always authenticated */ + file->authenticated = capable(CAP_SYS_ADMIN); + file->lock_count = 0; + + INIT_LIST_HEAD(&file->lhead); + INIT_LIST_HEAD(&file->fbs); + mutex_init(&file->fbs_lock); + INIT_LIST_HEAD(&file->blobs); + INIT_LIST_HEAD(&file->pending_event_list); + INIT_LIST_HEAD(&file->event_list); + init_waitqueue_head(&file->event_wait); + file->event_space = 4096; /* set aside 4k for event buffer */ + + mutex_init(&file->event_read_lock); + + if (drm_core_check_feature(dev, DRIVER_GEM)) + drm_gem_open(dev, file); + + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + drm_syncobj_open(file); + + if (drm_core_check_feature(dev, DRIVER_PRIME)) + drm_prime_init_file_private(&file->prime); + + if (dev->driver->open) { + ret = dev->driver->open(dev, file); + if (ret < 0) + goto out_prime_destroy; + } + + if (drm_is_primary_client(file)) { + ret = drm_master_open(file); + if (ret) + goto out_close; + } + + return file; + +out_close: + if (dev->driver->postclose) + dev->driver->postclose(dev, file); +out_prime_destroy: + if (drm_core_check_feature(dev, DRIVER_PRIME)) + drm_prime_destroy_file_private(&file->prime); + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ)) + drm_syncobj_release(file); + if (drm_core_check_feature(dev, DRIVER_GEM)) + drm_gem_release(dev, file); + put_pid(file->pid); + kfree(file); + + return ERR_PTR(ret); +} + +static void drm_events_release(struct drm_file *file_priv) +{ + struct drm_device *dev = file_priv->minor->dev; + struct drm_pending_event *e, *et; + unsigned long flags; + + spin_lock_irqsave(&dev->event_lock, flags); + + /* Unlink pending events */ + list_for_each_entry_safe(e, et, &file_priv->pending_event_list, +pending_link) { + list_del(&e->pending_link); + e->file_priv = NULL; + } + + /* Remove unconsumed events */ + list_for_each_entry_safe(e, et, &file_priv->event_list, link) { + list_del(&e->link); + kfree(e); + } + + spin_unlock_irqrestore(&dev->event_lock, flags); +} + +/** + * drm_file_free - free file context + * @file: context to free, or NULL + * + * This destroys and deallocates a DRM file context previously allocated via + * drm_file_alloc(). The caller must make sure to unlink it from any contexts + * before calling this. + * + * The legacy paths might require the drm_global_mutex to be held. + * + * If NULL is passed, this is a no-op. + * + * RETURNS: + * 0 on success, or error code on failure. + */ +void drm_file_free(struct drm_file *file) +{ + struct drm_device *dev; + + if (!file) + return; + + dev = file->minor->dev; + + DRM_DEBUG("pid = %d, device = 0x%lx, open_count = %d\n", + task_pid_nr(current), + (long)old_en
Re: [Intel-gfx] [RFC v2 1/8] drm: provide management functions for drm_file
On Wed, Jan 03, 2018 at 11:21:03PM +0100, Noralf Trønnes wrote: > From: David Herrmann > > Rather than doing drm_file allocation/destruction right in the fops, lets > provide separate helpers. This decouples drm_file management from the > still-mandatory drm-fops. It prepares for use of drm_file without the > fops, both by possible separate fops implementations and APIs (not that I > am aware of any such plans), and more importantly from in-kernel use where > no real file is available. > > Signed-off-by: David Herrmann > [rebased] > Signed-off-by: Noralf Trønnes > --- > drivers/gpu/drm/drm_file.c | 309 > +++-- > drivers/gpu/drm/drm_internal.h | 2 + > 2 files changed, 179 insertions(+), 132 deletions(-) > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index b3c6e997ccdb..d208faade27e 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -101,6 +101,179 @@ DEFINE_MUTEX(drm_global_mutex); > > static int drm_open_helper(struct file *filp, struct drm_minor *minor); > > +/** > + * drm_file_alloc - allocate file context > + * @minor: minor to allocate on > + * > + * This allocates a new DRM file context. It is not linked into any context > and > + * can be used by the caller freely. Note that the context keeps a pointer to > + * @minor, so it must be freed before @minor is. > + * > + * The legacy paths might require the drm_global_mutex to be held. I'd remove this line, since it's only relevant to close the driver load vs. file open race for drivers still using the deprecated ->load callback. This only confuses and doesn't help anyone writing a new/modern driver. > + * > + * RETURNS: > + * Pointer to newly allocated context, ERR_PTR on failure. > + */ > +struct drm_file *drm_file_alloc(struct drm_minor *minor) > +{ > + struct drm_device *dev = minor->dev; > + struct drm_file *file; > + int ret; > + > + file = kzalloc(sizeof(*file), GFP_KERNEL); > + if (!file) > + return ERR_PTR(-ENOMEM); > + > + file->pid = get_pid(task_pid(current)); > + file->minor = minor; > + > + /* for compatibility root is always authenticated */ > + file->authenticated = capable(CAP_SYS_ADMIN); > + file->lock_count = 0; > + > + INIT_LIST_HEAD(&file->lhead); > + INIT_LIST_HEAD(&file->fbs); > + mutex_init(&file->fbs_lock); > + INIT_LIST_HEAD(&file->blobs); > + INIT_LIST_HEAD(&file->pending_event_list); > + INIT_LIST_HEAD(&file->event_list); > + init_waitqueue_head(&file->event_wait); > + file->event_space = 4096; /* set aside 4k for event buffer */ > + > + mutex_init(&file->event_read_lock); > + > + if (drm_core_check_feature(dev, DRIVER_GEM)) > + drm_gem_open(dev, file); > + > + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ)) > + drm_syncobj_open(file); > + > + if (drm_core_check_feature(dev, DRIVER_PRIME)) > + drm_prime_init_file_private(&file->prime); > + > + if (dev->driver->open) { > + ret = dev->driver->open(dev, file); > + if (ret < 0) > + goto out_prime_destroy; > + } > + > + if (drm_is_primary_client(file)) { > + ret = drm_master_open(file); > + if (ret) > + goto out_close; > + } > + > + return file; > + > +out_close: > + if (dev->driver->postclose) > + dev->driver->postclose(dev, file); > +out_prime_destroy: > + if (drm_core_check_feature(dev, DRIVER_PRIME)) > + drm_prime_destroy_file_private(&file->prime); > + if (drm_core_check_feature(dev, DRIVER_SYNCOBJ)) > + drm_syncobj_release(file); > + if (drm_core_check_feature(dev, DRIVER_GEM)) > + drm_gem_release(dev, file); > + put_pid(file->pid); > + kfree(file); > + > + return ERR_PTR(ret); > +} > + > +static void drm_events_release(struct drm_file *file_priv) > +{ > + struct drm_device *dev = file_priv->minor->dev; > + struct drm_pending_event *e, *et; > + unsigned long flags; > + > + spin_lock_irqsave(&dev->event_lock, flags); > + > + /* Unlink pending events */ > + list_for_each_entry_safe(e, et, &file_priv->pending_event_list, > + pending_link) { > + list_del(&e->pending_link); > + e->file_priv = NULL; > + } > + > + /* Remove unconsumed events */ > + list_for_each_entry_safe(e, et, &file_priv->event_list, link) { > + list_del(&e->link); > + kfree(e); > + } > + > + spin_unlock_irqrestore(&dev->event_lock, flags); > +} > + > +/** > + * drm_file_free - free file context > + * @file: context to free, or NULL > + * > + * This destroys and deallocates a DRM file context previously allocated via > + * drm_file_alloc(). The caller must make sure to unlink it from any contexts > + * before calling this. > + * > + * The legacy paths might requir
Re: [Intel-gfx] [RFC v2 1/8] drm: provide management functions for drm_file
Hi On Tue, Jan 9, 2018 at 11:20 AM, Daniel Vetter wrote: > On Wed, Jan 03, 2018 at 11:21:03PM +0100, Noralf Trønnes wrote: >> From: David Herrmann >> >> Rather than doing drm_file allocation/destruction right in the fops, lets >> provide separate helpers. This decouples drm_file management from the >> still-mandatory drm-fops. It prepares for use of drm_file without the >> fops, both by possible separate fops implementations and APIs (not that I >> am aware of any such plans), and more importantly from in-kernel use where >> no real file is available. >> >> Signed-off-by: David Herrmann >> [rebased] >> Signed-off-by: Noralf Trønnes >> --- For completeness: Still fine with me! Thanks for picking it up. Thanks David ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx