On 2019-10-06 00:44, Ming Lei wrote:
> +struct scsi_host_mq_in_flight {
> +     int cnt;
> +};

Is this structure useful? Have you considered to use the 'int' datatype
directly and to leave out struct scsi_host_mq_in_flight?

>  /**
>   * scsi_host_busy - Return the host busy counter
>   * @shost:   Pointer to Scsi_Host to inc.
>   **/
>  int scsi_host_busy(struct Scsi_Host *shost)
>  {
> -     return atomic_read(&shost->host_busy);
> +     struct scsi_host_mq_in_flight in_flight = {
> +             .cnt = 0,
> +     };

In case struct scsi_host_mq_in_flight would be retained, have you
considered to use "{ }" to initialize "in_flight"?

> -static void scsi_dec_host_busy(struct Scsi_Host *shost)
> +static void scsi_dec_host_busy(struct Scsi_Host *shost, struct scsi_cmnd 
> *cmd)
>  {
>       unsigned long flags;
>  
>       rcu_read_lock();
> -     atomic_dec(&shost->host_busy);
> +     clear_bit(SCMD_STATE_INFLIGHT, &cmd->state);

If a new state variable would be introduced for SCSI commands, would it
be possible to use non-atomic operations to set and clear
SCMD_STATE_INFLIGHT? In other words, are any of the functions that
modify this bit ever called concurrently?

Thanks,

Bart.

Reply via email to