Linux-mtd Archive on lore.kernel.org
 help / color / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: Mike Snitzer <snitzer@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>,
	linux-block@vger.kernel.org, martin.petersen@oracle.com,
	Hans de Goede <hdegoede@redhat.com>, Song Liu <song@kernel.org>,
	Richard Weinberger <richard@nod.at>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-raid@vger.kernel.org, Minchan Kim <minchan@kernel.org>,
	dm-devel@redhat.com, linux-mtd@lists.infradead.org,
	linux-mm@kvack.org, drbd-dev@tron.linbit.com,
	cgroups@vger.kernel.org, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 06/14] block: lift setting the readahead size into the block layer
Date: Wed, 2 Sep 2020 17:11:44 +0200
Message-ID: <20200902151144.GA1738@lst.de> (raw)
In-Reply-To: <20200826220737.GA25613@redhat.com>

On Wed, Aug 26, 2020 at 06:07:38PM -0400, Mike Snitzer wrote:
> On Sun, Jul 26 2020 at 11:03am -0400,
> Christoph Hellwig <hch@lst.de> wrote:
> 
> > Drivers shouldn't really mess with the readahead size, as that is a VM
> > concept.  Instead set it based on the optimal I/O size by lifting the
> > algorithm from the md driver when registering the disk.  Also set
> > bdi->io_pages there as well by applying the same scheme based on
> > max_sectors.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  block/blk-settings.c         |  5 ++---
> >  block/blk-sysfs.c            |  1 -
> >  block/genhd.c                | 13 +++++++++++--
> >  drivers/block/aoe/aoeblk.c   |  2 --
> >  drivers/block/drbd/drbd_nl.c | 12 +-----------
> >  drivers/md/bcache/super.c    |  4 ----
> >  drivers/md/dm-table.c        |  3 ---
> >  drivers/md/raid0.c           | 16 ----------------
> >  drivers/md/raid10.c          | 24 +-----------------------
> >  drivers/md/raid5.c           | 13 +------------
> >  10 files changed, 16 insertions(+), 77 deletions(-)
> 
> 
> In general these changes need a solid audit relative to stacking
> drivers.  That is, the limits stacking methods (blk_stack_limits)
> vs lower level allocation methods (__device_add_disk).
> 
> You optimized for lowlevel __device_add_disk establishing the bdi's
> ra_pages and io_pages.  That is at the beginning of disk allocation,
> well before any build up of stacking driver's queue_io_opt() -- which
> was previously done in disk_stack_limits or driver specific methods
> (e.g. dm_table_set_restrictions) that are called _after_ all the limits
> stacking occurs.
> 
> By inverting the setting of the bdi's ra_pages and io_pages to be done
> so early in __device_add_disk it'll break properly setting these values
> for at least DM afaict.

ra_pages never got inherited by stacking drivers, check it by modifying
it on an underlying device and then creating a trivial dm or md one.
And I think that is a good thing - in general we shouldn't really mess
with this thing from drivers if we can avoid it.  I've kept the legacy
aoe and md parity raid cases, out of which the first looks pretty weird
and the md one at least remotely sensible.

->io_pages is still inherited in disk_stack_limits, just like before
so no change either.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

  reply index

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-26 15:03 bdi cleanups v3 Christoph Hellwig
2020-07-26 15:03 ` [PATCH 01/14] fs: remove the unused SB_I_MULTIROOT flag Christoph Hellwig
2020-07-26 15:03 ` [PATCH 02/14] drbd: remove dead code in device_to_statistics Christoph Hellwig
2020-07-26 15:03 ` [PATCH 03/14] drbd: remove RB_CONGESTED_REMOTE Christoph Hellwig
2020-07-26 15:03 ` [PATCH 04/14] bdi: initialize ->ra_pages in bdi_init Christoph Hellwig
2020-07-26 15:03 ` [PATCH 05/14] md: update the optimal I/O size on reshape Christoph Hellwig
2020-07-26 15:03 ` [PATCH 06/14] block: lift setting the readahead size into the block layer Christoph Hellwig
2020-08-26 22:07   ` Mike Snitzer
2020-09-02 15:11     ` Christoph Hellwig [this message]
2020-09-02 16:20       ` Mike Snitzer
2020-09-10  9:28         ` Christoph Hellwig
2020-09-10 17:15           ` Mike Snitzer
2020-09-15  7:05             ` Christoph Hellwig
2020-07-26 15:03 ` [PATCH 07/14] block: make QUEUE_SYSFS_BIT_FNS a little more useful Christoph Hellwig
2020-07-29  8:57   ` [block] 54529aac98: fsmark.files_per_sec -13.6% regression kernel test robot
2020-07-30  2:14   ` [block] 54529aac98: blktests.block.005.fail kernel test robot
2020-07-26 15:03 ` [PATCH 08/14] block: add helper macros for queue sysfs entries Christoph Hellwig
2020-07-26 15:03 ` [PATCH 09/14] bdi: remove BDI_CAP_CGROUP_WRITEBACK Christoph Hellwig
2020-07-26 15:03 ` [PATCH 10/14] bdi: remove BDI_CAP_SYNCHRONOUS_IO Christoph Hellwig
2020-07-26 19:06   ` Minchan Kim
2020-07-27  7:58     ` Christoph Hellwig
2020-07-26 15:03 ` [PATCH 11/14] mm: use SWP_SYNCHRONOUS_IO more intelligently Christoph Hellwig
2020-07-26 15:03 ` [PATCH 12/14] bdi: replace BDI_CAP_STABLE_WRITES with a queue and a sb flag Christoph Hellwig
2020-07-26 15:03 ` [PATCH 13/14] bdi: invert BDI_CAP_NO_ACCT_WB Christoph Hellwig
2020-07-26 15:03 ` [PATCH 14/14] bdi: replace BDI_CAP_NO_{WRITEBACK, ACCT_DIRTY} with a single flag Christoph Hellwig
2020-07-26 15:12 ` bdi cleanups v3 Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2020-07-24  7:32 Christoph Hellwig
2020-07-24  7:33 ` [PATCH 06/14] block: lift setting the readahead size into the block layer Christoph Hellwig
2020-07-28 12:23   ` Johannes Thumshirn
2020-07-22  6:25 bdi cleanups v2 Christoph Hellwig
2020-07-22  6:25 ` [PATCH 06/14] block: lift setting the readahead size into the block layer Christoph Hellwig
2020-07-22  7:13   ` Johannes Thumshirn
2020-07-22  7:18     ` Christoph Hellwig
2020-07-20  7:51 bdi cleanups Christoph Hellwig
2020-07-20  7:51 ` [PATCH 06/14] block: lift setting the readahead size into the block layer Christoph Hellwig

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=20200902151144.GA1738@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=cgroups@vger.kernel.org \
    --cc=dm-devel@redhat.com \
    --cc=drbd-dev@tron.linbit.com \
    --cc=hdegoede@redhat.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=minchan@kernel.org \
    --cc=richard@nod.at \
    --cc=snitzer@redhat.com \
    --cc=song@kernel.org \
    /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

Linux-mtd Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mtd/0 linux-mtd/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-mtd linux-mtd/ https://lore.kernel.org/linux-mtd \
		linux-mtd@lists.infradead.org
	public-inbox-index linux-mtd

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-mtd


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