Re: [LKP] Re: [io_uring] 584b0180f0: phoronix-test-suite.fio.SequentialWrite.IO_uring.Yes.Yes.1MB.DefaultTestDirectory.mb_s -10.2% regression

[Yin Fengwei <fengwei.yin@intel.com>]

Hi Jens,

On 7/21/2022 2:13 AM, Jens Axboe wrote:
> Can you try this? It's against 5.19-rc7.
> 
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index a01ea49f3017..34758e95990a 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -2015,6 +2015,64 @@ static inline void io_arm_ltimeout(struct io_kiocb *req)
>  		__io_arm_ltimeout(req);
>  }
>  
> +static bool io_bdev_nowait(struct block_device *bdev)
> +{
> +	return !bdev || blk_queue_nowait(bdev_get_queue(bdev));
> +}
> +
> +/*
> + * If we tracked the file through the SCM inflight mechanism, we could support
> + * any file. For now, just ensure that anything potentially problematic is done
> + * inline.
> + */
> +static bool __io_file_supports_nowait(struct file *file, umode_t mode)
> +{
> +	if (S_ISBLK(mode)) {
> +		if (IS_ENABLED(CONFIG_BLOCK) &&
> +		    io_bdev_nowait(I_BDEV(file->f_mapping->host)))
> +			return true;
> +		return false;
> +	}
> +	if (S_ISSOCK(mode))
> +		return true;
> +	if (S_ISREG(mode)) {
> +		if (IS_ENABLED(CONFIG_BLOCK) &&
> +		    io_bdev_nowait(file->f_inode->i_sb->s_bdev) &&
> +		    file->f_op != &io_uring_fops)
> +			return true;
> +		return false;
> +	}
> +
> +	/* any ->read/write should understand O_NONBLOCK */
> +	if (file->f_flags & O_NONBLOCK)
> +		return true;
> +	return file->f_mode & FMODE_NOWAIT;
> +}
> +
> +static inline bool io_file_supports_nowait(struct io_kiocb *req)
> +{
> +	return req->flags & REQ_F_SUPPORT_NOWAIT;
> +}
> +
> +/*
> + * If we tracked the file through the SCM inflight mechanism, we could support
> + * any file. For now, just ensure that anything potentially problematic is done
> + * inline.
> + */
> +static unsigned int io_file_get_flags(struct file *file)
> +{
> +	umode_t mode = file_inode(file)->i_mode;
> +	unsigned int res = 0;
> +
> +	if (S_ISREG(mode))
> +		res |= FFS_ISREG;
> +	if (__io_file_supports_nowait(file, mode))
> +		res |= FFS_NOWAIT;
> +	if (io_file_need_scm(file))
> +		res |= FFS_SCM;
> +	return res;
> +}
> +
>  static void io_prep_async_work(struct io_kiocb *req)
>  {
>  	const struct io_op_def *def = &io_op_defs[req->opcode];
> @@ -2031,6 +2089,9 @@ static void io_prep_async_work(struct io_kiocb *req)
>  	if (req->flags & REQ_F_FORCE_ASYNC)
>  		req->work.flags |= IO_WQ_WORK_CONCURRENT;
>  
> +	if (req->file && !io_req_ffs_set(req))
> +		req->flags |= io_file_get_flags(req->file) << REQ_F_SUPPORT_NOWAIT_BIT;
> +
>  	if (req->flags & REQ_F_ISREG) {
>  		if (def->hash_reg_file || (ctx->flags & IORING_SETUP_IOPOLL))
>  			io_wq_hash_work(&req->work, file_inode(req->file));
> @@ -3556,64 +3617,6 @@ static void io_iopoll_req_issued(struct io_kiocb *req, unsigned int issue_flags)
>  	}
>  }
>  
> -static bool io_bdev_nowait(struct block_device *bdev)
> -{
> -	return !bdev || blk_queue_nowait(bdev_get_queue(bdev));
> -}
> -
> -/*
> - * If we tracked the file through the SCM inflight mechanism, we could support
> - * any file. For now, just ensure that anything potentially problematic is done
> - * inline.
> - */
> -static bool __io_file_supports_nowait(struct file *file, umode_t mode)
> -{
> -	if (S_ISBLK(mode)) {
> -		if (IS_ENABLED(CONFIG_BLOCK) &&
> -		    io_bdev_nowait(I_BDEV(file->f_mapping->host)))
> -			return true;
> -		return false;
> -	}
> -	if (S_ISSOCK(mode))
> -		return true;
> -	if (S_ISREG(mode)) {
> -		if (IS_ENABLED(CONFIG_BLOCK) &&
> -		    io_bdev_nowait(file->f_inode->i_sb->s_bdev) &&
> -		    file->f_op != &io_uring_fops)
> -			return true;
> -		return false;
> -	}
> -
> -	/* any ->read/write should understand O_NONBLOCK */
> -	if (file->f_flags & O_NONBLOCK)
> -		return true;
> -	return file->f_mode & FMODE_NOWAIT;
> -}
> -
> -/*
> - * If we tracked the file through the SCM inflight mechanism, we could support
> - * any file. For now, just ensure that anything potentially problematic is done
> - * inline.
> - */
> -static unsigned int io_file_get_flags(struct file *file)
> -{
> -	umode_t mode = file_inode(file)->i_mode;
> -	unsigned int res = 0;
> -
> -	if (S_ISREG(mode))
> -		res |= FFS_ISREG;
> -	if (__io_file_supports_nowait(file, mode))
> -		res |= FFS_NOWAIT;
> -	if (io_file_need_scm(file))
> -		res |= FFS_SCM;
> -	return res;
> -}
> -
> -static inline bool io_file_supports_nowait(struct io_kiocb *req)
> -{
> -	return req->flags & REQ_F_SUPPORT_NOWAIT;
> -}
> -
>  static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  {
>  	struct kiocb *kiocb = &req->rw.kiocb;
> 
> -- Jens Axboe
This change could make regression gone. The test result is as following:
28d3a5662d44077aa6eb42bfcfa is your patch


584b0180f0f4d67d                   v5.19-rc7 28d3a5662d44077aa6eb42bfcfa
---------------- --------------------------- ---------------------------
       fail:runs  %reproduction    fail:runs  %reproduction    fail:runs
           |             |             |             |             |
        503:3         9297%         782:3          178%         509:3     dmesg.timestamp:last
          3:3            0%           3:3            0%           3:3     pmeter.pmeter.fail
           :3          100%           3:3          100%           3:3     kmsg.I/O_error,dev_loop#,sector#op#:(READ)flags#phys_seg#prio_class
           :3         3755%         112:3         4016%         120:3     kmsg.timestamp:I/O_error,dev_loop#,sector#op#:(READ)flags#phys_seg#prio_class
        465:3         9221%         742:3          235%         473:3     kmsg.timestamp:last
         %stddev     %change         %stddev     %change         %stddev
             \          |                \          |                \
    972.00            -0.3%     968.67           +11.4%       1082        phoronix-test-suite.fio.SequentialWrite.IO_uring.Yes.Yes.1MB.DefaultTestDirectory.iops
    975.00            -0.3%     972.33           +11.5%       1086        phoronix-test-suite.fio.SequentialWrite.IO_uring.Yes.Yes.1MB.DefaultTestDirectory.mb_s

Comparing to v5.19-rc7 and 584b0180f0f4d67d, it could bring 11% regression back.
Thanks.


Regards
Yin, Fengwei