Re: [PATCH v2] SCSI and block: Simplify resume handling

[Bart Van Assche <bvanassche@acm.org>]

On 2020-07-01 11:37, Alan Stern wrote:
>  void blk_post_runtime_resume(struct request_queue *q, int err)
>  {
> -	if (!q->dev)
> -		return;
> -
> -	spin_lock_irq(&q->queue_lock);
>  	if (!err) {
> -		q->rpm_status = RPM_ACTIVE;
> -		pm_runtime_mark_last_busy(q->dev);
> -		pm_request_autosuspend(q->dev);
> -	} else {
> +		blk_set_runtime_active(q);
> +	} else if (q->dev) {
> +		spin_lock_irq(&q->queue_lock);
>  		q->rpm_status = RPM_SUSPENDED;
> +		spin_unlock_irq(&q->queue_lock);
>  	}
> -	spin_unlock_irq(&q->queue_lock);
> -
> -	if (!err)
> -		blk_clear_pm_only(q);
>  }
>  EXPORT_SYMBOL(blk_post_runtime_resume);

I'd like to keep the if (!q->dev) check at the start of the function instead
of moving it to the middle of the function to keep the symmetry with the
existing runtime power management functions in the same source file.

>  void blk_set_runtime_active(struct request_queue *q)
>  {
>  	if (q->dev) {
> +		int old_status;
> +
>  		spin_lock_irq(&q->queue_lock);
> +		old_status = q->rpm_status;
>  		q->rpm_status = RPM_ACTIVE;
>  		pm_runtime_mark_last_busy(q->dev);
>  		pm_request_autosuspend(q->dev);
>  		spin_unlock_irq(&q->queue_lock);
> +
> +		if (old_status != RPM_ACTIVE)
> +			blk_clear_pm_only(q);
>  	}
>  }

Since this function is being modified, please change the if (q->dev) into
if (!q->dev) return since returning early is the recommended kernel coding
style.

Thanks,

Bart.