linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Coly Li <colyli@suse.de>
Cc: Andre Noll <maan@tuebingen.mpg.de>, Nix <nix@esperi.org.uk>,
	linux-bcache@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-kernel@vger.kernel.org, Christoph Hellwig <hch@lst.de>,
	axboe@kernel.dk
Subject: Re: bcache on XFS: metadata I/O (dirent I/O?) not getting cached at all?
Date: Thu, 7 Feb 2019 14:10:08 +1100	[thread overview]
Message-ID: <20190207031008.GJ14116@dastard> (raw)
In-Reply-To: <438851ef-ef77-b5f2-d46d-05762b6330b2@suse.de>

On Thu, Feb 07, 2019 at 10:38:58AM +0800, Coly Li wrote:
> On 2019/2/7 10:26 上午, Dave Chinner wrote:
> > On Thu, Feb 07, 2019 at 01:24:25AM +0100, Andre Noll wrote:
> >> On Thu, Feb 07, 10:43, Dave Chinner wrote
> >>> File data readahead: REQ_RAHEAD
> >>> Metadata readahead: REQ_META | REQ_RAHEAD
> >>>
> >>> drivers/md/bcache/request.c::check_should_bypass():
> >>>
> >>>         /*
> >>>          * Flag for bypass if the IO is for read-ahead or background,
> >>>          * unless the read-ahead request is for metadata (eg, for gfs2).
> >>>          */
> >>>         if (bio->bi_opf & (REQ_RAHEAD|REQ_BACKGROUND) &&
> >>>             !(bio->bi_opf & REQ_PRIO))
> >>>                 goto skip;
> >>>
> >>> bcache needs fixing - it thinks REQ_PRIO means metadata IO. That's
> >>> wrong - REQ_META means it's metadata IO, and so this is a bcache
> >>> bug.
> >>
> >> Do you think 752f66a75abad is bad (ha!) and should be reverted?
> > 
> > Yes, that change is just broken. From include/linux/blk_types.h:
> > 
> > 	__REQ_META,             /* metadata io request */
> > 	__REQ_PRIO,             /* boost priority in cfq */
> > 
> > 
> 
> Hi Dave,
> 
> > i.e. REQ_META means that it is a metadata request, REQ_PRIO means it
> > is a "high priority" request. Two completely different things, often
> > combined, but not interchangeable.
> 
> I found in file system metadata IO, most of time REQ_META and REQ_PRIO
> are tagged together for bio, but XFS seems not use REQ_PRIO.

Yes, that's correct, because we don't specifically prioritise
metadata IO over data IO.

> Is there any basic principle for when should these tags to be used or
> not ?

Yes.

> e.g. If REQ_META is enough for meta data I/O, why REQ_PRIO is used
> too.

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.

Historically speaking, REQ_PRIO was a hack for CFQ to get it to
dispatch journal IO from a different thread without waiting for a
time slice to expire. In the XFs world, we just said "don't use CFQ,
it's fundametnally broken for highly concurrent applications" and
didn't bother trying to hack around the limitations of CFQ.

These days REQ_PRIO is only used by the block layer writeback
throttle, but unlike bcache it considers both REQ_META and REQ_PRIO
to mean the same thing.

REQ_META, OTOH, is used by BFQ and blk-cgroup to detect metadata
IO and don't look at REQ_PRIO at all. So, really, REQ_META is for
metadata, not REQ_PRIO. REQ_PRIO looks like it should just go away.

> And if REQ_PRIO is necessary, why it is not used in fs/xfs/ code ?

It's not necessary, it's just an /optimisation/ that some
filesystems make and some IO schedulers used to pay attention to. It
looks completely redundant now.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2019-02-07  3:10 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-06 22:11 bcache on XFS: metadata I/O (dirent I/O?) not getting cached at all? Nix
2019-02-06 23:43 ` Dave Chinner
2019-02-07  0:24   ` Andre Noll
2019-02-07  2:26     ` Dave Chinner
2019-02-07  2:38       ` Coly Li
2019-02-07  3:10         ` Dave Chinner [this message]
2019-02-07  8:18           ` Coly Li
2019-02-07 13:10         ` Nix
2019-02-07  2:27     ` Coly Li
2019-02-07  9:28       ` Andre Noll
2019-02-07  8:16 ` Coly Li
2019-02-07  9:41   ` Andre Noll
2019-02-07 10:23     ` Coly Li
2019-02-07 20:51   ` Nix

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190207031008.GJ14116@dastard \
    --to=david@fromorbit.com \
    --cc=axboe@kernel.dk \
    --cc=colyli@suse.de \
    --cc=hch@lst.de \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=maan@tuebingen.mpg.de \
    --cc=nix@esperi.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).