[Intel-gfx] [PATCH 1/4] drm/i915/guc: Fix lockdep due to log relay channel handling under struct_mutex

[Sagar Arun Kamble]

On 1/22/2018 3:46 PM, Chris Wilson wrote:
> Quoting Sagar Arun Kamble (2018-01-22 08:26:01)
>> +int intel_guc_log_relay_create(struct intel_guc *guc)
>> +{
>> +       struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +       struct rchan *guc_log_relay_chan;
>> +       size_t n_subbufs, subbuf_size;
>> +       int ret;
>> +
>> +       if (!i915_modparams.guc_log_level)
>> +               return 0;
>> +
>> +       GEM_BUG_ON(guc_log_has_relay(guc));
>> +
>>           /* Keep the size of sub buffers same as shared log buffer */
>> -       subbuf_size = guc->log.vma->obj->base.size;
>> +       subbuf_size = GUC_LOG_SIZE;
>>   
>>          /* Store up to 8 snapshots, which is large enough to buffer sufficient
>>           * boot time logs and provides enough leeway to User, in terms of
>> @@ -407,33 +442,31 @@ static int guc_log_runtime_create(struct intel_guc *guc)
>>                  DRM_ERROR("Couldn't create relay chan for GuC logging\n");
>>   
>>                  ret = -ENOMEM;
>> -               goto err_vaddr;
>> +               goto err;
>>          }
>>   
>>          GEM_BUG_ON(guc_log_relay_chan->subbuf_size < subbuf_size);
>>          guc->log.runtime.relay_chan = guc_log_relay_chan;
>>   
>> -       INIT_WORK(&guc->log.runtime.flush_work, capture_logs_work);
>>          return 0;
>>   
>> -err_vaddr:
>> -       i915_gem_object_unpin_map(guc->log.vma->obj);
>> -       guc->log.runtime.buf_addr = NULL;
>> +err:
>> +       /* logging will be off */
>> +       i915_modparams.guc_log_level = 0;
>>          return ret;
>>   }
>>   
>> -static void guc_log_runtime_destroy(struct intel_guc *guc)
>> +void intel_guc_log_relay_destroy(struct intel_guc *guc)
>>   {
> Now exposed to multiple users, we need to document what the locking
> requirements are here. Or add some local locking.
Do you mean locking between relay_create and relay_destroy?
>   Looks like at the
> moment, _create is using struct_mutex,
relay_create and relay_destroy are now to be done outside of struct_mutex.
I will add this documentation to the functions.
> which as we are only handling the
> relay may be overkill (but since this is one-off init, fair enough).
> -Chris