Linux-Block Archive on lore.kernel.org
 help / Atom feed
* [patch] block: clarify documentation for blk_{start|finish}_plug
@ 2019-01-08 21:57 Jeff Moyer
  2019-01-08 22:09 ` Darrick J. Wong
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jeff Moyer @ 2019-01-08 21:57 UTC (permalink / raw)
  To: axboe; +Cc: Darrick J. Wong, linux-block

There was some confusion about what these functions did.  Make it clear
that this is a hint for upper layers to pass to the block layer, and
that it does not guarantee that I/O will not be submitted between a
start and finish plug.

Reported-by: "Darrick J. Wong" <darrick.wong@oracle.com>
Signed-off-by: Jeff Moyer <jmoyer@redhat.com>

diff --git a/block/blk-core.c b/block/blk-core.c
index c78042975737..f2732f106a2e 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1683,6 +1683,15 @@ EXPORT_SYMBOL(kblockd_mod_delayed_work_on);
  * @plug:	The &struct blk_plug that needs to be initialized
  *
  * Description:
+ *   blk_start_plug() indicates to the block layer an intent by the caller
+ *   to submit multiple I/O requests in a batch.  The block layer may use
+ *   this hint to defer submitting I/Os from the caller until blk_finish_plug()
+ *   is called.  However, the block layer may choose to submit requests
+ *   before a call to blk_finish_plug() if the number of queued I/Os
+ *   exceeds %BLK_MAX_REQUEST_COUNT, or if the size of the I/O is larger than
+ *   %BLK_PLUG_FLUSH_SIZE.  The queued I/Os may also be submitted early if
+ *   the task schedules (see below).
+ *
  *   Tracking blk_plug inside the task_struct will help with auto-flushing the
  *   pending I/O should the task end up blocking between blk_start_plug() and
  *   blk_finish_plug(). This is important from a performance perspective, but
@@ -1765,6 +1774,16 @@ void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule)
 		blk_mq_flush_plug_list(plug, from_schedule);
 }
 
+/**
+ * blk_finish_plug - mark the end of a batch of submitted I/O
+ * @plug:	The &struct blk_plug passed to blk_start_plug()
+ *
+ * Description:
+ * Indicate that a batch of I/O submissions is complete.  This function
+ * must be paired with an initial call to blk_start_plug().  The intent
+ * is to allow the block layer to optimize I/O submission.  See the
+ * documentation for blk_start_plug() for more information.
+ */
 void blk_finish_plug(struct blk_plug *plug)
 {
 	if (plug != current->plug)

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

* Re: [patch] block: clarify documentation for blk_{start|finish}_plug
  2019-01-08 21:57 [patch] block: clarify documentation for blk_{start|finish}_plug Jeff Moyer
@ 2019-01-08 22:09 ` Darrick J. Wong
  2019-01-09  3:07 ` Ming Lei
  2019-01-09  3:50 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Darrick J. Wong @ 2019-01-08 22:09 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: axboe, linux-block

On Tue, Jan 08, 2019 at 04:57:34PM -0500, Jeff Moyer wrote:
> There was some confusion about what these functions did.  Make it clear
> that this is a hint for upper layers to pass to the block layer, and
> that it does not guarantee that I/O will not be submitted between a
> start and finish plug.
> 
> Reported-by: "Darrick J. Wong" <darrick.wong@oracle.com>
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> 
> diff --git a/block/blk-core.c b/block/blk-core.c
> index c78042975737..f2732f106a2e 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1683,6 +1683,15 @@ EXPORT_SYMBOL(kblockd_mod_delayed_work_on);
>   * @plug:	The &struct blk_plug that needs to be initialized
>   *
>   * Description:
> + *   blk_start_plug() indicates to the block layer an intent by the caller
> + *   to submit multiple I/O requests in a batch.  The block layer may use
> + *   this hint to defer submitting I/Os from the caller until blk_finish_plug()
> + *   is called.  However, the block layer may choose to submit requests
> + *   before a call to blk_finish_plug() if the number of queued I/Os
> + *   exceeds %BLK_MAX_REQUEST_COUNT, or if the size of the I/O is larger than
> + *   %BLK_PLUG_FLUSH_SIZE.  The queued I/Os may also be submitted early if
> + *   the task schedules (see below).

Makes sense to my poor brain,

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

>   *   Tracking blk_plug inside the task_struct will help with auto-flushing the
>   *   pending I/O should the task end up blocking between blk_start_plug() and
>   *   blk_finish_plug(). This is important from a performance perspective, but
> @@ -1765,6 +1774,16 @@ void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule)
>  		blk_mq_flush_plug_list(plug, from_schedule);
>  }
>  
> +/**
> + * blk_finish_plug - mark the end of a batch of submitted I/O
> + * @plug:	The &struct blk_plug passed to blk_start_plug()
> + *
> + * Description:
> + * Indicate that a batch of I/O submissions is complete.  This function
> + * must be paired with an initial call to blk_start_plug().  The intent
> + * is to allow the block layer to optimize I/O submission.  See the
> + * documentation for blk_start_plug() for more information.
> + */
>  void blk_finish_plug(struct blk_plug *plug)
>  {
>  	if (plug != current->plug)

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

* Re: [patch] block: clarify documentation for blk_{start|finish}_plug
  2019-01-08 21:57 [patch] block: clarify documentation for blk_{start|finish}_plug Jeff Moyer
  2019-01-08 22:09 ` Darrick J. Wong
@ 2019-01-09  3:07 ` Ming Lei
  2019-01-09  3:50 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Ming Lei @ 2019-01-09  3:07 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: Jens Axboe, Darrick J. Wong, linux-block

On Wed, Jan 9, 2019 at 5:59 AM Jeff Moyer <jmoyer@redhat.com> wrote:
>
> There was some confusion about what these functions did.  Make it clear
> that this is a hint for upper layers to pass to the block layer, and
> that it does not guarantee that I/O will not be submitted between a
> start and finish plug.
>
> Reported-by: "Darrick J. Wong" <darrick.wong@oracle.com>
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index c78042975737..f2732f106a2e 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1683,6 +1683,15 @@ EXPORT_SYMBOL(kblockd_mod_delayed_work_on);
>   * @plug:      The &struct blk_plug that needs to be initialized
>   *
>   * Description:
> + *   blk_start_plug() indicates to the block layer an intent by the caller
> + *   to submit multiple I/O requests in a batch.  The block layer may use
> + *   this hint to defer submitting I/Os from the caller until blk_finish_plug()
> + *   is called.  However, the block layer may choose to submit requests
> + *   before a call to blk_finish_plug() if the number of queued I/Os
> + *   exceeds %BLK_MAX_REQUEST_COUNT, or if the size of the I/O is larger than
> + *   %BLK_PLUG_FLUSH_SIZE.  The queued I/Os may also be submitted early if
> + *   the task schedules (see below).
> + *
>   *   Tracking blk_plug inside the task_struct will help with auto-flushing the
>   *   pending I/O should the task end up blocking between blk_start_plug() and
>   *   blk_finish_plug(). This is important from a performance perspective, but
> @@ -1765,6 +1774,16 @@ void blk_flush_plug_list(struct blk_plug *plug, bool from_schedule)
>                 blk_mq_flush_plug_list(plug, from_schedule);
>  }
>
> +/**
> + * blk_finish_plug - mark the end of a batch of submitted I/O
> + * @plug:      The &struct blk_plug passed to blk_start_plug()
> + *
> + * Description:
> + * Indicate that a batch of I/O submissions is complete.  This function
> + * must be paired with an initial call to blk_start_plug().  The intent
> + * is to allow the block layer to optimize I/O submission.  See the
> + * documentation for blk_start_plug() for more information.
> + */
>  void blk_finish_plug(struct blk_plug *plug)
>  {
>         if (plug != current->plug)

Reviewed-by: Ming Lei <ming.lei@redhat.com>

Thanks,
Ming Lei

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

* Re: [patch] block: clarify documentation for blk_{start|finish}_plug
  2019-01-08 21:57 [patch] block: clarify documentation for blk_{start|finish}_plug Jeff Moyer
  2019-01-08 22:09 ` Darrick J. Wong
  2019-01-09  3:07 ` Ming Lei
@ 2019-01-09  3:50 ` Jens Axboe
  2 siblings, 0 replies; 4+ messages in thread
From: Jens Axboe @ 2019-01-09  3:50 UTC (permalink / raw)
  To: Jeff Moyer; +Cc: Darrick J. Wong, linux-block

On 1/8/19 2:57 PM, Jeff Moyer wrote:
> There was some confusion about what these functions did.  Make it clear
> that this is a hint for upper layers to pass to the block layer, and
> that it does not guarantee that I/O will not be submitted between a
> start and finish plug.

Thanks Jeff, that's definitely an improvement, I like how it's phrased
as intent, there are no guarantees and we can change things down the
line as well (the auto-unplug threshold is a good example of that).

Applied for 5.0.

-- 
Jens Axboe


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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-08 21:57 [patch] block: clarify documentation for blk_{start|finish}_plug Jeff Moyer
2019-01-08 22:09 ` Darrick J. Wong
2019-01-09  3:07 ` Ming Lei
2019-01-09  3:50 ` Jens Axboe

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org linux-block@archiver.kernel.org
	public-inbox-index linux-block


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/ public-inbox