[PATCH] bcache: use (REQ_META|REQ_PRIO) to indicate bio for metadata

[PATCH] bcache: use (REQ_META|REQ_PRIO) to indicate bio for metadata

[Coly Li]

In 'commit 752f66a75aba ("bcache: use REQ_PRIO to indicate bio for
metadata")' REQ_META is replaced by REQ_PRIO to indicate metadata bio.
This assumption is not always correct, e.g. XFS uses REQ_META to mark
metadata bio other than REQ_PRIO. This is why Nix noticed that bcache
does not cache metadata for XFS after the above commit.

Thanks to Dave Chinner, he explains the difference between REQ_META and
REQ_PRIO from view of file system developer. Here I quote part of his
explanation from mailing list,
   REQ_META is used for metadata. REQ_PRIO is used to communicate to
   the lower layers that the submitter considers this IO to be more
   important that non REQ_PRIO IO and so dispatch should be expedited.

   IOWs, if the filesystem considers metadata IO to be more important
   that user data IO, then it will use REQ_PRIO | REQ_META rather than
   just REQ_META.

Then it seems bios with REQ_META or REQ_PRIO should both be cached for
performance optimation, because they are all probably low I/O latency
demand by upper layer (e.g. file system).

So in this patch, when we want to decide whether to bypass the cache,
REQ_META and REQ_PRIO are both checked. Then both metadata and
high priority I/O requests will be handled properly.

(Coly Li: backport the original patch for Linux-stable v4.19)

Reported-by: Nix <nix@esperi.org.uk>
Signed-off-by: Coly Li <colyli@suse.de>
Reviewed-by: Andre Noll <maan@tuebingen.mpg.de>
Tested-by: Nix <nix@esperi.org.uk>
Cc: stable@vger.kernel.org
Cc: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
 drivers/md/bcache/request.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
index 22944aa7d8e5..4ca3e3d3f9c7 100644
--- a/drivers/md/bcache/request.c
+++ b/drivers/md/bcache/request.c
@@ -392,10 +392,11 @@ static bool check_should_bypass(struct cached_dev *dc, struct bio *bio)
 
 	/*
 	 * Flag for bypass if the IO is for read-ahead or background,
-	 * unless the read-ahead request is for metadata (eg, for gfs2).
+	 * unless the read-ahead request is for metadata
+	 * (eg, for gfs2 or xfs).
 	 */
 	if (bio->bi_opf & (REQ_RAHEAD|REQ_BACKGROUND) &&
-	    !(bio->bi_opf & REQ_META))
+	    !(bio->bi_opf & (REQ_META|REQ_PRIO)))
 		goto skip;
 
 	if (bio->bi_iter.bi_sector & (c->sb.block_size - 1) ||
@@ -877,7 +878,7 @@ static int cached_dev_cache_miss(struct btree *b, struct search *s,
 	}
 
 	if (!(bio->bi_opf & REQ_RAHEAD) &&
-	    !(bio->bi_opf & REQ_META) &&
+	    !(bio->bi_opf & (REQ_META|REQ_PRIO)) &&
 	    s->iop.c->gc_stats.in_use < CUTOFF_CACHE_READA)
 		reada = min_t(sector_t, dc->readahead >> 9,
 			      get_capacity(bio->bi_disk) - bio_end_sector(bio));
-- 
2.16.4

Re: [PATCH] bcache: use (REQ_META|REQ_PRIO) to indicate bio for metadata

[Greg KH]

On Fri, Mar 22, 2019 at 03:48:44PM +0800, Coly Li wrote:
> In 'commit 752f66a75aba ("bcache: use REQ_PRIO to indicate bio for
> metadata")' REQ_META is replaced by REQ_PRIO to indicate metadata bio.
> This assumption is not always correct, e.g. XFS uses REQ_META to mark
> metadata bio other than REQ_PRIO. This is why Nix noticed that bcache
> does not cache metadata for XFS after the above commit.
> 
> Thanks to Dave Chinner, he explains the difference between REQ_META and
> REQ_PRIO from view of file system developer. Here I quote part of his
> explanation from mailing list,
>    REQ_META is used for metadata. REQ_PRIO is used to communicate to
>    the lower layers that the submitter considers this IO to be more
>    important that non REQ_PRIO IO and so dispatch should be expedited.
> 
>    IOWs, if the filesystem considers metadata IO to be more important
>    that user data IO, then it will use REQ_PRIO | REQ_META rather than
>    just REQ_META.
> 
> Then it seems bios with REQ_META or REQ_PRIO should both be cached for
> performance optimation, because they are all probably low I/O latency
> demand by upper layer (e.g. file system).
> 
> So in this patch, when we want to decide whether to bypass the cache,
> REQ_META and REQ_PRIO are both checked. Then both metadata and
> high priority I/O requests will be handled properly.
> 
> (Coly Li: backport the original patch for Linux-stable v4.19)

Now queued up, thanks.

greg k-h