video: tegra: host: Fix channel refcount issues
Arto Merilainen [Mon, 14 Jul 2014 13:43:37 +0000 (16:43 +0300)]
This patch fixes few possible races in channel initialisation and
deinitialisation:
- If two channel initialisations were running concurrently for
the same device and the first initialisation failed, we potentially
gave an uninitialised channel to the second requester
- If putchannel() triggered uninitialisation, we were still able
to give the channel in getchannel().

Bug 200013323

Change-Id: I99fa726db99fbb98401d5703cc2572131907f726
(cherry picked from commit 4fb20fef4a9eaf222c3f6d4c83fcb495f207c478)
Signed-off-by: Arto Merilainen <amerilainen@nvidia.com>
Reviewed-on: http://git-master/r/498509
Reviewed-on: http://git-master/r/538725
Reviewed-by: Automatic_Commit_Validation_User
GVS: Gerrit_Virtual_Submit
Reviewed-by: Shreshtha Sahu <ssahu@nvidia.com>
Tested-by: Shreshtha Sahu <ssahu@nvidia.com>
Reviewed-by: Venkat Moganty <vmoganty@nvidia.com>
Reviewed-by: Shridhar Rasal <srasal@nvidia.com>

drivers/video/tegra/host/host1x/host1x_channel.c
drivers/video/tegra/host/nvhost_channel.c
drivers/video/tegra/host/nvhost_channel.h

index 5b2b499..011b523 100644 (file)
@@ -526,7 +526,6 @@ static inline int hwctx_handler_init(struct nvhost_channel *ch)
 static int host1x_channel_init(struct nvhost_channel *ch,
        struct nvhost_master *dev)
 {
-       mutex_init(&ch->reflock);
        mutex_init(&ch->submitlock);
 
        ch->aperture = host1x_channel_aperture(dev->aperture, ch->chid);
index 9f0ca9e..05a31d8 100644 (file)
@@ -130,7 +130,7 @@ int nvhost_channel_release(struct nvhost_device_data *pdata)
        return 0;
 }
 /* Unmap channel from device and free all resources, deinit device */
-int nvhost_channel_unmap(struct nvhost_channel *ch)
+static int nvhost_channel_unmap_locked(struct nvhost_channel *ch)
 {
        struct nvhost_device_data *pdata;
        struct nvhost_master *host;
@@ -145,12 +145,10 @@ int nvhost_channel_unmap(struct nvhost_channel *ch)
        pdata = platform_get_drvdata(ch->dev);
        host = nvhost_get_host(pdata->pdev);
 
-       mutex_lock(&host->chlist_mutex);
        max_channels = host->info.nb_channels;
 
        if (ch->chid == NVHOST_INVALID_CHANNEL) {
                dev_err(&host->dev->dev, "Freeing un-mapped channel\n");
-               mutex_unlock(&host->chlist_mutex);
                return 0;
        }
        if (ch->error_notifier_ref)
@@ -195,10 +193,9 @@ int nvhost_channel_unmap(struct nvhost_channel *ch)
        ch->ctxhandler = NULL;
        ch->cur_ctx = NULL;
        ch->aperture = NULL;
+       ch->refcount = 0;
        pdata->channels[ch->dev_chid] = NULL;
 
-       mutex_unlock(&host->chlist_mutex);
-
        return 0;
 }
 
@@ -230,7 +227,7 @@ int nvhost_channel_map(struct nvhost_device_data *pdata,
                }
                ch = nvhost_check_channel(pdata);
                if (ch)
-                       nvhost_getchannel(ch);
+                       ch->refcount++;
                mutex_unlock(&host->chlist_mutex);
                *channel = ch;
                return 0;
@@ -254,32 +251,27 @@ int nvhost_channel_map(struct nvhost_device_data *pdata,
 
        /* Get channel from list and map to device */
        ch = host->chlist[index];
-       if (!ch) {
-               dev_err(&host->dev->dev, "%s: No channel is free\n", __func__);
-               mutex_unlock(&host->chlist_mutex);
-               return -EBUSY;
-       }
-       if (ch->chid == NVHOST_INVALID_CHANNEL) {
-               ch->dev = pdata->pdev;
-               ch->chid = index;
-               nvhost_channel_assign(pdata, ch);
-               nvhost_set_chanops(ch);
-       } else {
+       if (!ch || (ch->chid != NVHOST_INVALID_CHANNEL)) {
                dev_err(&host->dev->dev, "%s: wrong channel map\n", __func__);
                mutex_unlock(&host->chlist_mutex);
                return -EINVAL;
        }
 
+       ch->dev = pdata->pdev;
+       ch->chid = index;
+       nvhost_channel_assign(pdata, ch);
+       nvhost_set_chanops(ch);
+       set_bit(ch->chid, &host->allocated_channels);
+       ch->refcount = 1;
+
        /* Initialize channel */
        err = nvhost_channel_init(ch, host);
        if (err) {
                dev_err(&ch->dev->dev, "%s: channel init failed\n", __func__);
+               nvhost_channel_unmap_locked(ch);
                mutex_unlock(&host->chlist_mutex);
-               nvhost_channel_unmap(ch);
                return err;
        }
-       nvhost_getchannel(ch);
-       set_bit(ch->chid, &host->allocated_channels);
 
        /* set next free channel */
        if (index >= (max_channels - 1))
@@ -291,8 +283,8 @@ int nvhost_channel_map(struct nvhost_device_data *pdata,
                err = pdata->init(ch->dev);
                if (err) {
                        dev_err(&ch->dev->dev, "device init failed\n");
+                       nvhost_channel_unmap_locked(ch);
                        mutex_unlock(&host->chlist_mutex);
-                       nvhost_channel_unmap(ch);
                        return err;
                }
        }
@@ -374,20 +366,28 @@ int nvhost_channel_submit(struct nvhost_job *job)
 
 void nvhost_getchannel(struct nvhost_channel *ch)
 {
-       atomic_inc(&ch->refcount);
+       struct nvhost_device_data *pdata = platform_get_drvdata(ch->dev);
+       struct nvhost_master *host = nvhost_get_host(pdata->pdev);
+
+       mutex_lock(&host->chlist_mutex);
+       ch->refcount++;
+       mutex_unlock(&host->chlist_mutex);
 }
 
 void nvhost_putchannel(struct nvhost_channel *ch, int cnt)
 {
-       int ref;
+       struct nvhost_device_data *pdata = platform_get_drvdata(ch->dev);
+       struct nvhost_master *host = nvhost_get_host(pdata->pdev);
 
-       ref = atomic_sub_return(cnt, &ch->refcount);
+       mutex_lock(&host->chlist_mutex);
+       ch->refcount -= cnt;
 
        /* WARN on negative reference, with zero reference unmap channel*/
-       if (!ref)
-               nvhost_channel_unmap(ch);
-       else if (ref < 0)
+       if (!ch->refcount)
+               nvhost_channel_unmap_locked(ch);
+       else if (ch->refcount < 0)
                WARN_ON(1);
+       mutex_unlock(&host->chlist_mutex);
 }
 
 int nvhost_channel_suspend(struct nvhost_channel *ch)
index 84643a2..026e756 100644 (file)
@@ -47,10 +47,9 @@ struct nvhost_channel_ops {
 
 struct nvhost_channel {
        struct nvhost_channel_ops ops;
-       atomic_t refcount;
+       int refcount;
        int chid;
        int dev_chid;
-       struct mutex reflock;
        struct mutex submitlock;
        void __iomem *aperture;
        struct nvhost_hwctx *cur_ctx;