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

Chris Wilson chris at chris-wilson.co.uk
Tue Jan 23 15:46:46 UTC 2018


Quoting Sagar Arun Kamble (2018-01-23 15:42:17)
>  static void *guc_get_write_buffer(struct intel_guc *guc)
>  {
> -       if (!guc->log.runtime.relay_chan)
> +       if (!guc_log_has_relay(guc))
>                 return NULL;
>  
>         /* Just get the base address of a new sub buffer and copy data into it
> @@ -266,7 +276,9 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
>         log_buf_state = src_data = guc->log.runtime.buf_addr;
>  
>         /* Get the pointer to local buffer to store the logs */
> +       mutex_lock(&guc->log.runtime.relay_lock);
>         log_buf_snapshot_state = dst_data = guc_get_write_buffer(guc);
> +       mutex_unlock(&guc->log.runtime.relay_lock);

Since dst_data/log_buf_snapshot_state are pointing into the relay_chan
they are only valid underneath the relay_lock (we don't have any
references or whatnot).

>         /* Actual logs are present from the 2nd page */
>         src_data += PAGE_SIZE;
> @@ -335,6 +347,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
>                 dst_data += buffer_size;
>         }
>  
> +       mutex_lock(&guc->log.runtime.relay_lock);
>         if (log_buf_snapshot_state)
>                 guc_move_to_next_buf(guc);
>         else {
> @@ -344,6 +357,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
>                 DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n");
>                 guc->log.capture_miss_count++;
>         }
> +       mutex_unlock(&guc->log.runtime.relay_lock);

I.e. it's no good just reacquiring the lock as the state may have
perished in between.
-Chris


More information about the Intel-gfx mailing list