Instead of using the FD open counter to check if there is a user opened on the device, check if there is a valid user context.
Use the new lazy context creation mutex to protect against multiple calls to change the frequency. This is in preparation for removing the FD open counter. Signed-off-by: Oded Gabbay <[email protected]> --- drivers/misc/habanalabs/context.c | 9 +++++ drivers/misc/habanalabs/device.c | 40 +++++++++-------------- drivers/misc/habanalabs/goya/goya_hwmgr.c | 11 +++---- drivers/misc/habanalabs/habanalabs.h | 4 +-- drivers/misc/habanalabs/habanalabs_drv.c | 34 ++++++++----------- 5 files changed, 45 insertions(+), 53 deletions(-) diff --git a/drivers/misc/habanalabs/context.c b/drivers/misc/habanalabs/context.c index 3c1f7c29e908..a4cd47ced94d 100644 --- a/drivers/misc/habanalabs/context.c +++ b/drivers/misc/habanalabs/context.c @@ -220,9 +220,18 @@ bool hl_ctx_is_valid(struct hl_fpriv *hpriv) if (rc) { dev_err(hdev->dev, "Failed to create context %d\n", rc); valid = false; + goto unlock_mutex; } + + /* Device is IDLE at this point so it is legal to change PLLs. + * There is no need to check anything because if the PLL is + * already HIGH, the set function will return without doing + * anything + */ + hl_device_set_frequency(hdev, PLL_HIGH); } +unlock_mutex: mutex_unlock(&hdev->lazy_ctx_creation_lock); return valid; diff --git a/drivers/misc/habanalabs/device.c b/drivers/misc/habanalabs/device.c index b63beb73ec76..a791a1b58215 100644 --- a/drivers/misc/habanalabs/device.c +++ b/drivers/misc/habanalabs/device.c @@ -289,9 +289,13 @@ static void set_freq_to_low_job(struct work_struct *work) struct hl_device *hdev = container_of(work, struct hl_device, work_freq.work); - if (atomic_read(&hdev->fd_open_cnt) == 0) + mutex_lock(&hdev->lazy_ctx_creation_lock); + + if (!hdev->user_ctx) hl_device_set_frequency(hdev, PLL_LOW); + mutex_unlock(&hdev->lazy_ctx_creation_lock); + schedule_delayed_work(&hdev->work_freq, usecs_to_jiffies(HL_PLL_LOW_JOB_FREQ_USEC)); } @@ -341,7 +345,7 @@ static int device_late_init(struct hl_device *hdev) hdev->high_pll = hdev->asic_prop.high_pll; /* force setting to low frequency */ - atomic_set(&hdev->curr_pll_profile, PLL_LOW); + hdev->curr_pll_profile = PLL_LOW; if (hdev->pm_mng_profile == PM_AUTO) hdev->asic_funcs->set_pll_profile(hdev, PLL_LOW); @@ -390,38 +394,26 @@ static void device_late_fini(struct hl_device *hdev) * @hdev: pointer to habanalabs device structure * @freq: the new frequency value * - * Change the frequency if needed. - * We allose to set PLL to low only if there is no user process - * Returns 0 if no change was done, otherwise returns 1; + * Change the frequency if needed. This function has no protection against + * concurrency, therefore it is assumed that the calling function has protected + * itself against the case of calling this function from multiple threads with + * different values + * + * Returns 0 if no change was done, otherwise returns 1 */ int hl_device_set_frequency(struct hl_device *hdev, enum hl_pll_frequency freq) { - enum hl_pll_frequency old_freq = - (freq == PLL_HIGH) ? PLL_LOW : PLL_HIGH; - int ret; - - if (hdev->pm_mng_profile == PM_MANUAL) - return 0; - - ret = atomic_cmpxchg(&hdev->curr_pll_profile, old_freq, freq); - if (ret == freq) + if ((hdev->pm_mng_profile == PM_MANUAL) || + (hdev->curr_pll_profile == freq)) return 0; - /* - * in case we want to lower frequency, check if device is not - * opened. We must have a check here to workaround race condition with - * hl_device_open - */ - if ((freq == PLL_LOW) && (atomic_read(&hdev->fd_open_cnt) > 0)) { - atomic_set(&hdev->curr_pll_profile, PLL_HIGH); - return 0; - } - dev_dbg(hdev->dev, "Changing device frequency to %s\n", freq == PLL_HIGH ? "high" : "low"); hdev->asic_funcs->set_pll_profile(hdev, freq); + hdev->curr_pll_profile = freq; + return 1; } diff --git a/drivers/misc/habanalabs/goya/goya_hwmgr.c b/drivers/misc/habanalabs/goya/goya_hwmgr.c index a51d836542a1..8522c6e0a25e 100644 --- a/drivers/misc/habanalabs/goya/goya_hwmgr.c +++ b/drivers/misc/habanalabs/goya/goya_hwmgr.c @@ -254,11 +254,11 @@ static ssize_t pm_mng_profile_store(struct device *dev, goto out; } - mutex_lock(&hdev->fd_open_cnt_lock); + mutex_lock(&hdev->lazy_ctx_creation_lock); - if (atomic_read(&hdev->fd_open_cnt) > 0) { + if (hdev->user_ctx) { dev_err(hdev->dev, - "Can't change PM profile while user process is opened on the device\n"); + "Can't change PM profile while user context is opened on the device\n"); count = -EPERM; goto unlock_mutex; } @@ -266,7 +266,7 @@ static ssize_t pm_mng_profile_store(struct device *dev, if (strncmp("auto", buf, strlen("auto")) == 0) { /* Make sure we are in LOW PLL when changing modes */ if (hdev->pm_mng_profile == PM_MANUAL) { - atomic_set(&hdev->curr_pll_profile, PLL_HIGH); + hdev->curr_pll_profile = PLL_HIGH; hl_device_set_frequency(hdev, PLL_LOW); hdev->pm_mng_profile = PM_AUTO; } @@ -279,11 +279,10 @@ static ssize_t pm_mng_profile_store(struct device *dev, } else { dev_err(hdev->dev, "value should be auto or manual\n"); count = -EINVAL; - goto unlock_mutex; } unlock_mutex: - mutex_unlock(&hdev->fd_open_cnt_lock); + mutex_unlock(&hdev->lazy_ctx_creation_lock); out: return count; } diff --git a/drivers/misc/habanalabs/habanalabs.h b/drivers/misc/habanalabs/habanalabs.h index d0e55839d673..6354fc88ef8a 100644 --- a/drivers/misc/habanalabs/habanalabs.h +++ b/drivers/misc/habanalabs/habanalabs.h @@ -1190,10 +1190,10 @@ struct hl_device_reset_work { * value is saved so in case of hard-reset, KMD will restore this * value and update the F/W after the re-initialization * @in_reset: is device in reset flow. - * @curr_pll_profile: current PLL profile. * @fd_open_cnt: number of open user processes. * @cs_active_cnt: number of active command submissions on this device (active * means already in H/W queues) + * @curr_pll_profile: current PLL profile. * @major: habanalabs KMD major. * @high_pll: high PLL profile frequency. * @soft_reset_cnt: number of soft reset since KMD loading. @@ -1268,9 +1268,9 @@ struct hl_device { u64 timeout_jiffies; u64 max_power; atomic_t in_reset; - atomic_t curr_pll_profile; atomic_t fd_open_cnt; atomic_t cs_active_cnt; + enum hl_pll_frequency curr_pll_profile; u32 major; u32 high_pll; u32 soft_reset_cnt; diff --git a/drivers/misc/habanalabs/habanalabs_drv.c b/drivers/misc/habanalabs/habanalabs_drv.c index b2c52e46e130..a781aa936160 100644 --- a/drivers/misc/habanalabs/habanalabs_drv.c +++ b/drivers/misc/habanalabs/habanalabs_drv.c @@ -95,42 +95,40 @@ int hl_device_open(struct inode *inode, struct file *filp) return -ENXIO; } + hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL); + if (!hpriv) + return -ENOMEM; + mutex_lock(&hdev->fd_open_cnt_lock); if (hl_device_disabled_or_in_reset(hdev)) { dev_err_ratelimited(hdev->dev, "Can't open %s because it is disabled or in reset\n", dev_name(hdev->dev)); - mutex_unlock(&hdev->fd_open_cnt_lock); - return -EPERM; + rc = -EPERM; + goto out_err; } if (hdev->in_debug) { dev_err_ratelimited(hdev->dev, "Can't open %s because it is being debugged by another user\n", dev_name(hdev->dev)); - mutex_unlock(&hdev->fd_open_cnt_lock); - return -EPERM; + rc = -EPERM; + goto out_err; } if (atomic_read(&hdev->fd_open_cnt)) { dev_info_ratelimited(hdev->dev, "Can't open %s because another user is working on it\n", dev_name(hdev->dev)); - mutex_unlock(&hdev->fd_open_cnt_lock); - return -EBUSY; + rc = -EBUSY; + goto out_err; } atomic_inc(&hdev->fd_open_cnt); mutex_unlock(&hdev->fd_open_cnt_lock); - hpriv = kzalloc(sizeof(*hpriv), GFP_KERNEL); - if (!hpriv) { - rc = -ENOMEM; - goto close_device; - } - hpriv->hdev = hdev; filp->private_data = hpriv; hpriv->filp = filp; @@ -143,19 +141,13 @@ int hl_device_open(struct inode *inode, struct file *filp) hpriv->taskpid = find_get_pid(current->pid); - /* - * Device is IDLE at this point so it is legal to change PLLs. There - * is no need to check anything because if the PLL is already HIGH, the - * set function will return without doing anything - */ - hl_device_set_frequency(hdev, PLL_HIGH); - hl_debugfs_add_file(hpriv); return 0; -close_device: - atomic_dec(&hdev->fd_open_cnt); +out_err: + mutex_unlock(&hdev->fd_open_cnt_lock); + kfree(hpriv); return rc; } -- 2.17.1
