Re: [PATCH v5 05/11] block: mq-deadline: Clean up deadline_check_fifo()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/17/23 07:33, Bart Van Assche wrote:
> Change the return type of deadline_check_fifo() from 'int' into 'bool'.
> Use time_is_before_eq_jiffies() instead of time_after_eq(). No
> functionality has been changed.
> 
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Cc: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Cc: Ming Lei <ming.lei@xxxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
>  block/mq-deadline.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> index 5839a027e0f0..a016cafa54b3 100644
> --- a/block/mq-deadline.c
> +++ b/block/mq-deadline.c
> @@ -272,21 +272,15 @@ static u32 dd_queued(struct deadline_data *dd, enum dd_prio prio)
>  }
>  
>  /*
> - * deadline_check_fifo returns 0 if there are no expired requests on the fifo,
> - * 1 otherwise. Requires !list_empty(&dd->fifo_list[data_dir])
> + * deadline_check_fifo returns true if and only if there are expired requests
> + * in the FIFO list. Requires !list_empty(&dd->fifo_list[data_dir]).
>   */
> -static inline int deadline_check_fifo(struct dd_per_prio *per_prio,
> -				      enum dd_data_dir data_dir)
> +static inline bool deadline_check_fifo(struct dd_per_prio *per_prio,
> +				       enum dd_data_dir data_dir)
>  {
>  	struct request *rq = rq_entry_fifo(per_prio->fifo_list[data_dir].next);
>  
> -	/*
> -	 * rq is expired!
> -	 */
> -	if (time_after_eq(jiffies, (unsigned long)rq->fifo_time))
> -		return 1;
> -
> -	return 0;
> +	return time_is_before_eq_jiffies((unsigned long)rq->fifo_time);

This looks wrong: isn't this reversing the return value ?
Shouldn't this be:

	return time_after_eq(jiffies, (unsigned long)rq->fifo_time));

To return true if the first request in fifo list *expired* as indicated by the
function kdoc comment.

>  }
>  
>  /*

-- 
Damien Le Moal
Western Digital Research




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux