Subject: Re: drm drivers for NetBSD
To: Yorick Hardy <yhardy@uj.ac.za>
From: Yorick Hardy <yhardy@uj.ac.za>
List: tech-x11
Date: 03/22/2007 09:39:26
This is a multi-part message in MIME format.
--------------000304030403000106050001
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
This time with the patch ;-)
Yorick Hardy wrote:
> Matthias,
>
> Is it possible to integrate the following patch?
>
> Because drm_close is only called once we end
> up with a lot of references to processes that
> may no longer exist.
>
> This is not a fix, but nearly eliminates the problem.
>
> I think I should let Eric Anholt know what we are doing.
> I suspect it could be useful to keep FreeBSD drm
> and NetBSD drm relatively consistent.
>
> Matthias Drochner wrote:
>> macallan@netbsd.org said:
>>> Please commit
>>
>> OK -- there are some minor issues which need reviev and/or cleanup
>> which I wantet to do first, but this will not cause big changes and
>> can be done later.
>> I'll start committing in a minute.
>>
>> best regards
>> Matthias
>>
>>
>
>
--
Kind regards,
Yorick Hardy
--------------000304030403000106050001
Content-Type: text/plain;
name="drm.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="drm.patch"
diff -ur drmP.h.orig drmP.h
--- drmP.h.orig 2007-03-17 23:18:51.000000000 +0200
+++ drmP.h 2007-03-21 18:35:41.000000000 +0200
@@ -906,6 +906,8 @@
dev_type_mmap(drm_mmap);
#endif
+int drm_close_pid(drm_device_t *dev, drm_file_t *priv, pid_t pid);
+
/* File operations helpers (drm_fops.c) */
#ifdef __FreeBSD__
extern int drm_open_helper(struct cdev *kdev, int flags, int fmt,
diff -ur drm_drv.c.orig drm_drv.c
--- drm_drv.c.orig 2007-03-17 23:18:51.000000000 +0200
+++ drm_drv.c 2007-03-21 18:35:41.000000000 +0200
@@ -768,27 +768,10 @@
return retcode;
}
-int drm_close(DRM_CDEV kdev, int flags, int fmt, DRM_STRUCTCDEVPROC *p)
+int drm_close_pid(drm_device_t *dev, drm_file_t *priv, pid_t pid)
{
- drm_file_t *priv;
- DRM_DEVICE;
int retcode = 0;
- DRMFILE filp = (void *)(uintptr_t)(DRM_CURRENTPID);
-
- DRM_DEBUG( "open_count = %d\n", dev->open_count );
-
- DRM_LOCK();
-
-#ifdef __FreeBSD__
- priv = drm_find_file_by_proc(dev, p);
-#elif defined(__NetBSD__)
- priv = drm_find_file_by_proc(dev, p->l_proc);
-#endif
- if (!priv) {
- DRM_UNLOCK();
- DRM_ERROR("can't find authenticator\n");
- return EINVAL;
- }
+ DRMFILE filp = (void *)(uintptr_t)(pid);
if (dev->driver.preclose != NULL)
dev->driver.preclose(dev, filp);
@@ -857,19 +840,6 @@
if (dev->driver.use_dma && !dev->driver.reclaim_buffers_locked)
drm_reclaim_buffers(dev, filp);
-#if defined (__FreeBSD__) && (__FreeBSD_version >= 500000)
- funsetown(&dev->buf_sigio);
-#elif defined(__FreeBSD__)
- funsetown(dev->buf_sigio);
-#elif defined(__NetBSD__) || defined(__OpenBSD__)
- dev->buf_pgid = 0;
-#endif /* __NetBSD__ || __OpenBSD__ */
-
-#ifdef __NetBSD__
- /* On NetBSD, close will only be called once */
- DRM_DEBUG("setting priv->refs %d to 1\n", (int)priv->refs);
- priv->refs = 1;
-#endif
if (--priv->refs == 0) {
if (dev->driver.postclose != NULL)
dev->driver.postclose(dev, priv);
@@ -877,19 +847,52 @@
free(priv, M_DRM);
}
- /* ========================================================
- * End inline drm_release
- */
+ return retcode;
+}
+
+int drm_close(DRM_CDEV kdev, int flags, int fmt, DRM_STRUCTCDEVPROC *p)
+{
+ drm_file_t *priv;
+ DRM_DEVICE;
+ int retcode = 0;
+
+ DRM_DEBUG( "open_count = %d\n", dev->open_count );
+
+ DRM_LOCK();
- atomic_inc( &dev->counts[_DRM_STAT_CLOSES] );
#ifdef __FreeBSD__
- device_unbusy(dev->device);
+ priv = drm_find_file_by_proc(dev, p);
+#elif defined(__NetBSD__)
+ priv = drm_find_file_by_proc(dev, p->l_proc);
#endif
+ if (!priv) {
+ DRM_UNLOCK();
+ DRM_ERROR("can't find authenticator\n");
+ return EINVAL;
+ }
+
#ifdef __NetBSD__
/* On NetBSD, close will only be called once */
+ DRM_DEBUG("setting priv->refs %d to 1\n", (int)priv->refs);
+ priv->refs = 1;
DRM_DEBUG("setting open_count %d to 1\n", (int)dev->open_count);
dev->open_count = 1;
#endif
+
+ retcode = drm_close_pid(dev, priv, DRM_CURRENTPID);
+
+#if defined (__FreeBSD__) && (__FreeBSD_version >= 500000)
+ funsetown(&dev->buf_sigio);
+#elif defined(__FreeBSD__)
+ funsetown(dev->buf_sigio);
+#elif defined(__NetBSD__) || defined(__OpenBSD__)
+ dev->buf_pgid = 0;
+#endif /* __NetBSD__ || __OpenBSD__ */
+
+ atomic_inc( &dev->counts[_DRM_STAT_CLOSES] );
+#ifdef __FreeBSD__
+ device_unbusy(dev->device);
+#endif
if (--dev->open_count == 0) {
retcode = drm_lastclose(dev);
}
diff -ur drm_fops.c.orig drm_fops.c
--- drm_fops.c.orig 2007-03-17 23:18:51.000000000 +0200
+++ drm_fops.c 2007-03-21 18:35:41.000000000 +0200
@@ -41,6 +41,7 @@
drm_file_t *drm_find_file_by_proc(drm_device_t *dev, DRM_STRUCTPROC *p)
{
+ int restart = 1;
#if __FreeBSD_version >= 500021
uid_t uid = p->td_ucred->cr_svuid;
pid_t pid = p->td_proc->p_pid;
@@ -55,9 +56,25 @@
DRM_SPINLOCK_ASSERT(&dev->dev_lock);
- TAILQ_FOREACH(priv, &dev->files, link)
- if (priv->pid == pid && priv->uid == uid)
- return priv;
+ while(restart) {
+ restart = 0;
+ TAILQ_FOREACH(priv, &dev->files, link) {
+#ifdef __NetBSD__
+ /* if the process disappeared, free the resources
+ * NetBSD only calls drm_close once, so this frees
+ * resources earlier.
+ */
+ if (pfind(priv->pid) == NULL) {
+ drm_close_pid(dev, priv, priv->pid);
+ restart = 1;
+ break;
+ }
+ else
+#endif
+ if (priv->pid == pid && priv->uid == uid)
+ return priv;
+ }
+ }
return NULL;
}
--------------000304030403000106050001--