public inbox for [email protected]
 help / color / mirror / Atom feed
* [Question] about async buffered reads feature
@ 2020-09-25  9:18 Hao_Xu
  2020-09-25 21:23 ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Hao_Xu @ 2020-09-25  9:18 UTC (permalink / raw)
  To: axboe; +Cc: io-uring

Hi Jens,
I'm doing tests about this feature: [PATCHSET RFC 0/11] Add support for 
async buffered reads
But currently with fio testing, I found the code doesn't go to the 
essential places in the function generic_file_buffered_read:

           if (iocb->ki_flags & IOCB_WAITQ) {
                   if (written) {
                           put_page(page);
                           goto out;
                   }
                   error = wait_on_page_locked_async(page,
                                                   iocb->ki_waitq);
           } else {

and

   page_not_up_to_date:
          /* Get exclusive access to the page ... */
          if (iocb->ki_flags & IOCB_WAITQ)
                  error = lock_page_async(page, iocb->ki_waitq);
          else


could you give me a copy of your test program which you mentioned in the 
RFC?
My testing environment:
fio version: 3.10
kernel version: the mainline kernel, latest commit is 
805c6d3c19210c90c109107d189744e960eae025


Thanks,
Hao


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Question] about async buffered reads feature
  2020-09-25  9:18 [Question] about async buffered reads feature Hao_Xu
@ 2020-09-25 21:23 ` Jens Axboe
  2020-09-25 21:39   ` Jens Axboe
  0 siblings, 1 reply; 3+ messages in thread
From: Jens Axboe @ 2020-09-25 21:23 UTC (permalink / raw)
  To: Hao_Xu; +Cc: io-uring

On 9/25/20 3:18 AM, Hao_Xu wrote:
> Hi Jens,
> I'm doing tests about this feature: [PATCHSET RFC 0/11] Add support for 
> async buffered reads
> But currently with fio testing, I found the code doesn't go to the 
> essential places in the function generic_file_buffered_read:
> 
>            if (iocb->ki_flags & IOCB_WAITQ) {
>                    if (written) {
>                            put_page(page);
>                            goto out;
>                    }
>                    error = wait_on_page_locked_async(page,
>                                                    iocb->ki_waitq);
>            } else {
> 
> and
> 
>    page_not_up_to_date:
>           /* Get exclusive access to the page ... */
>           if (iocb->ki_flags & IOCB_WAITQ)
>                   error = lock_page_async(page, iocb->ki_waitq);
>           else

Can you try with this added? Looks like a regression got introduced...


diff --git a/fs/io_uring.c b/fs/io_uring.c
index 40670ad4446c..99b842ac2dc0 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3339,10 +3339,7 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
 			goto done;
 		/* some cases will consume bytes even on error returns */
 		iov_iter_revert(iter, iov_count - iov_iter_count(iter));
-		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
-		if (ret)
-			goto out_free;
-		return -EAGAIN;
+		goto copy_iov;
 	} else if (ret < 0) {
 		/* make sure -ERESTARTSYS -> -EINTR is done */
 		goto done;

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Question] about async buffered reads feature
  2020-09-25 21:23 ` Jens Axboe
@ 2020-09-25 21:39   ` Jens Axboe
  0 siblings, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2020-09-25 21:39 UTC (permalink / raw)
  To: Hao_Xu; +Cc: io-uring

On 9/25/20 3:23 PM, Jens Axboe wrote:
> On 9/25/20 3:18 AM, Hao_Xu wrote:
>> Hi Jens,
>> I'm doing tests about this feature: [PATCHSET RFC 0/11] Add support for 
>> async buffered reads
>> But currently with fio testing, I found the code doesn't go to the 
>> essential places in the function generic_file_buffered_read:
>>
>>            if (iocb->ki_flags & IOCB_WAITQ) {
>>                    if (written) {
>>                            put_page(page);
>>                            goto out;
>>                    }
>>                    error = wait_on_page_locked_async(page,
>>                                                    iocb->ki_waitq);
>>            } else {
>>
>> and
>>
>>    page_not_up_to_date:
>>           /* Get exclusive access to the page ... */
>>           if (iocb->ki_flags & IOCB_WAITQ)
>>                   error = lock_page_async(page, iocb->ki_waitq);
>>           else
> 
> Can you try with this added? Looks like a regression got introduced...

Oops, needs 'ret' cleared too. This should be better!

diff --git a/fs/io_uring.c b/fs/io_uring.c
index ad828fa19af4..11b8e428300d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3172,10 +3172,8 @@ static int io_read(struct io_kiocb *req, bool force_nonblock,
 			goto done;
 		/* some cases will consume bytes even on error returns */
 		iov_iter_revert(iter, iov_count - iov_iter_count(iter));
-		ret = io_setup_async_rw(req, iovec, inline_vecs, iter, false);
-		if (ret)
-			goto out_free;
-		return -EAGAIN;
+		ret = 0;
+		goto copy_iov;
 	} else if (ret < 0) {
 		/* make sure -ERESTARTSYS -> -EINTR is done */
 		goto done;

-- 
Jens Axboe


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-09-25 21:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-25  9:18 [Question] about async buffered reads feature Hao_Xu
2020-09-25 21:23 ` Jens Axboe
2020-09-25 21:39   ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox