loop: are parallel requests serialized by the single workqueue?

* loop: are parallel requests serialized by the single workqueue?
@ 2022-03-17  2:26 Eric Wheeler
  2022-03-17 10:20 ` Chaitanya Kulkarni
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Wheeler @ 2022-03-17  2:26 UTC (permalink / raw)
  To: Ming Lei; +Cc: linux-block

Hi Ming,

I was studying the loop.c DIO & AIO changes you made back in 2015 that 
increased loop performance and reduced the memory footprint 
(bc07c10a3603a5ab3ef01ba42b3d41f9ac63d1b6).  

I have a few questions if you are able to comment, here is a quick 
summary:

The direct IO path starts by queuing the work:

  .queue_rq       = loop_queue_rq:

	-> loop_queue_work(lo, cmd);
	-> INIT_WORK(&worker->work, loop_workfn);
		... queue_work(lo->workqueue, work);

Then from within the workqueue:

	-> loop_workfn()
	-> loop_process_work(worker, &worker->cmd_list, worker->lo);
	-> loop_handle_cmd(cmd);
	-> do_req_filebacked(lo, blk_mq_rq_from_pdu(cmd) );
	-> lo_rw_aio(lo, cmd, pos, READ) // (or WRITE)

From here the kiocb is setup and this is the 5.17-rc8 code at the 
bottom of lo_rw_aio() when it sets up the dispatch to the filesystem:

	cmd->iocb.ki_pos = pos;
	cmd->iocb.ki_filp = file;
	cmd->iocb.ki_complete = lo_rw_aio_complete;
	cmd->iocb.ki_flags = IOCB_DIRECT;
	cmd->iocb.ki_ioprio = IOPRIO_PRIO_VALUE(IOPRIO_CLASS_NONE, 0);

	if (rw == WRITE)
		ret = call_write_iter(file, &cmd->iocb, &iter);
	else
		ret = call_read_iter(file, &cmd->iocb, &iter);

	lo_rw_aio_do_completion(cmd);

	if (ret != -EIOCBQUEUED)
		lo_rw_aio_complete(&cmd->iocb, ret);

After having called `call_read_iter` it is in the filesystem's
handler.

Since ki_complete is defined, does that mean the filesystem will _always_ 
take these in and always queue these internally and return -EIOCBQUEUED 
from call_read_iter()?  Another way to ask: if ki_complete!=NULL, can a 
filesystem ever behave synchronously?  (Is there documentation about this 
somewhere?  I couldn't find anything definitive.)

About the cleanup after dispatch at the bottom of lo_rw_aio() from this 
code (also shown above):

	lo_rw_aio_do_completion(cmd);

	if (ret != -EIOCBQUEUED)
		lo_rw_aio_complete(&cmd->iocb, ret);

* It appears that lo_rw_aio_do_completion() will `kfree(cmd->bvec)`.  If 
  the filesystem queued the cmd->iocb for internal use, would it have made 
  a copy of cmd->bvec so that this is safe?

* If ret != -EIOCBQUEUED, then lo_rw_aio_complete() is called which calls 
  lo_rw_aio_do_completion() a second time.  Now lo_rw_aio_do_completion 
  does do this ref check, so it _is_ safe:

	if (!atomic_dec_and_test(&cmd->ref))
		return;

For my own understanding, is this equivalent?

-	lo_rw_aio_do_completion(cmd);

	if (ret != -EIOCBQUEUED)
		lo_rw_aio_complete(&cmd->iocb, ret);
+	else
+		lo_rw_aio_do_completion(cmd);

Related questions: 

* For REQ_OP_FLUSH, lo_req_flush() is called: it does not appear that 
  lo_req_flush() does anything to make sure ki_complete has been called 
  for pending work, it just calls vfs_fsync().

  Is this a consistency problem?

  For example, if loop has queued async kiocb's to the filesystem via
  .read_iter/.write_iter, is it the filesystem's responsibility to 
  complete that work before returning from vfs_sync() or is it possible 
  that the vfs_sync() completes before ->ki_complete() is called?

* Would there be any benefit to parallelizing the do_req_filebackend()  
  calls with (for example) multiple work queues?

* If so, then besides flushing the parallel work and doing vfs_fsync (from 
  lo_req_flush), are there any other consistency issues to consider on 
  REQ_OP_FLUSH?  
     * Can READs and WRITEs be out-of-order between flushes?

Thanks for your help!

-Eric

--
Eric Wheeler

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