From: mlin@kernel.org (Ming Lin)
Subject: [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same}
Date: Wed, 21 Oct 2015 10:18:22 -0700 [thread overview]
Message-ID: <1445447902.26847.3.camel@ssi> (raw)
In-Reply-To: <20151021153351.GA4984@redhat.com>
On Wed, 2015-10-21@11:33 -0400, Mike Snitzer wrote:
> On Wed, Oct 21 2015 at 11:01am -0400,
> Ming Lin <mlin@kernel.org> wrote:
>
> > On Wed, 2015-10-21@09:39 -0400, Jeff Moyer wrote:
> > > Christoph Hellwig <hch at infradead.org> writes:
> > >
> > > > Jens, Ming:
> > > >
> > > > are you fine with the one liner change to get back to the old I/O
> > > > pattern? While it looks like the cards fault I'd like to avoid this
> > > > annoying regression.
> > >
> > > I'm not Jens or Ming, but your patch looks fine to me, though you'll
> > > want to remove the MAX_BIO_SECTORS definition since it's now unused.
> > > It's not clear to me why the limit was lowered in the first place.
> >
> > UINT_MAX >> 9 is not power of 2 and it causes dm-thinp discard fails.
> >
> > At the lengthy discussion:
> > [PATCH v5 01/11] block: make generic_make_request handle arbitrarily sized bios
> > We agreed to cap discard to 2G as an interim solution for 4.3 until the
> > dm-thinp discard code is rewritten.
>
> But did Jens ever commit that change to cap at 2G? I don't recall
> seeing it.
Yes, commit b49a0871
>
> > Hi Mike,
> >
> > Will the dm-thinp discard rewritten ready for 4.4?
>
> No. I'm not clear what needs changing in dm-thinp. I'll have to
> revisit the thread to refresh my memory.
>
> BTW, DM thinp can easily handle discards that aren't a power-of-2 so
> long as the requested discard is a factor of the thinp blocksize.
You are right. It's not about power-of-2.
Copy my old post here about why dm-thinp discard may fail with "UINT_MAX
>> 9".
4G: 8388608 sectors
UINT_MAX: 8388607 sectors
dm-thinp block size = default discard granularity = 128 sectors
blkdev_issue_discard(sector=0, nr_sectors=8388608)
[start_sector, end_sector]
[0, 8388607]
[0, 8388606], then dm-thinp splits it to 2 bios
[0, 8388479]
[8388480, 8388606] ---> this has problem in process_discard_bio(),
because the discard size(7 sectors) covers less than a block(128 sectors)
[8388607, 8388607] ---> same problem
WARNING: multiple messages have this Message-ID (diff)
From: Ming Lin <mlin@kernel.org>
To: Mike Snitzer <snitzer@redhat.com>
Cc: Jeff Moyer <jmoyer@redhat.com>,
Christoph Hellwig <hch@infradead.org>,
lkml <linux-kernel@vger.kernel.org>, Jens Axboe <axboe@kernel.dk>,
Kent Overstreet <kent.overstreet@gmail.com>,
Dongsu Park <dpark@posteo.net>,
"Martin K. Petersen" <martin.petersen@oracle.com>,
Ming Lin <ming.l@ssi.samsung.com>,
linux-nvme@lists.infradead.org
Subject: Re: [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same}
Date: Wed, 21 Oct 2015 10:18:22 -0700 [thread overview]
Message-ID: <1445447902.26847.3.camel@ssi> (raw)
In-Reply-To: <20151021153351.GA4984@redhat.com>
On Wed, 2015-10-21 at 11:33 -0400, Mike Snitzer wrote:
> On Wed, Oct 21 2015 at 11:01am -0400,
> Ming Lin <mlin@kernel.org> wrote:
>
> > On Wed, 2015-10-21 at 09:39 -0400, Jeff Moyer wrote:
> > > Christoph Hellwig <hch@infradead.org> writes:
> > >
> > > > Jens, Ming:
> > > >
> > > > are you fine with the one liner change to get back to the old I/O
> > > > pattern? While it looks like the cards fault I'd like to avoid this
> > > > annoying regression.
> > >
> > > I'm not Jens or Ming, but your patch looks fine to me, though you'll
> > > want to remove the MAX_BIO_SECTORS definition since it's now unused.
> > > It's not clear to me why the limit was lowered in the first place.
> >
> > UINT_MAX >> 9 is not power of 2 and it causes dm-thinp discard fails.
> >
> > At the lengthy discussion:
> > [PATCH v5 01/11] block: make generic_make_request handle arbitrarily sized bios
> > We agreed to cap discard to 2G as an interim solution for 4.3 until the
> > dm-thinp discard code is rewritten.
>
> But did Jens ever commit that change to cap at 2G? I don't recall
> seeing it.
Yes, commit b49a0871
>
> > Hi Mike,
> >
> > Will the dm-thinp discard rewritten ready for 4.4?
>
> No. I'm not clear what needs changing in dm-thinp. I'll have to
> revisit the thread to refresh my memory.
>
> BTW, DM thinp can easily handle discards that aren't a power-of-2 so
> long as the requested discard is a factor of the thinp blocksize.
You are right. It's not about power-of-2.
Copy my old post here about why dm-thinp discard may fail with "UINT_MAX
>> 9".
4G: 8388608 sectors
UINT_MAX: 8388607 sectors
dm-thinp block size = default discard granularity = 128 sectors
blkdev_issue_discard(sector=0, nr_sectors=8388608)
[start_sector, end_sector]
[0, 8388607]
[0, 8388606], then dm-thinp splits it to 2 bios
[0, 8388479]
[8388480, 8388606] ---> this has problem in process_discard_bio(),
because the discard size(7 sectors) covers less than a block(128 sectors)
[8388607, 8388607] ---> same problem
next prev parent reply other threads:[~2015-10-21 17:18 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-12 7:07 [PATCH v6 00/11] simplify block layer based on immutable biovecs Ming Lin
[not found] ` <1439363241-31772-1-git-send-email-mlin-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2015-08-12 7:07 ` [PATCH v6 01/11] block: make generic_make_request handle arbitrarily sized bios Ming Lin
2015-08-12 7:07 ` Ming Lin
2015-08-12 7:07 ` [Drbd-dev] " Ming Lin
2015-08-12 7:07 ` [PATCH v6 02/11] block: simplify bio_add_page() Ming Lin
2015-08-12 7:07 ` [PATCH v6 03/11] bcache: remove driver private bio splitting code Ming Lin
2016-01-08 1:53 ` Eric Wheeler
2016-01-13 2:00 ` Eric Wheeler
2016-01-13 5:54 ` Vojtech Pavlik
2016-01-13 23:03 ` Eric Wheeler
2015-08-12 7:07 ` [PATCH v6 04/11] btrfs: remove bio splitting and merge_bvec_fn() calls Ming Lin
2015-08-12 7:07 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same} Ming Lin
2015-10-13 11:50 ` Christoph Hellwig
2015-10-13 11:50 ` Christoph Hellwig
2015-10-13 17:44 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard, write_same} Ming Lin
2015-10-13 17:44 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same} Ming Lin
2015-10-14 13:27 ` Christoph Hellwig
2015-10-14 13:27 ` Christoph Hellwig
2015-10-14 16:38 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same}B Keith Busch
2015-10-14 16:38 ` Keith Busch
2015-10-14 16:50 ` Christoph Hellwig
2015-10-14 16:50 ` Christoph Hellwig
2015-10-21 16:02 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same} Mike Snitzer
2015-10-21 16:02 ` Mike Snitzer
2015-10-21 16:19 ` Mike Snitzer
2015-10-21 16:19 ` Mike Snitzer
2015-10-21 16:33 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard, write_same} Martin K. Petersen
2015-10-21 16:33 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same} Martin K. Petersen
2015-10-21 17:33 ` Ming Lin
2015-10-21 17:33 ` Ming Lin
2015-10-21 18:18 ` Mike Snitzer
2015-10-21 18:18 ` Mike Snitzer
2015-10-21 20:13 ` Ming Lin
2015-10-21 20:13 ` Ming Lin
2015-10-22 10:24 ` Christoph Hellwig
2015-10-22 10:24 ` Christoph Hellwig
2015-10-22 11:22 ` Christoph Hellwig
2015-10-22 11:22 ` Christoph Hellwig
2015-10-21 7:21 ` Christoph Hellwig
2015-10-21 7:21 ` Christoph Hellwig
2015-10-21 13:39 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard, write_same} Jeff Moyer
2015-10-21 13:39 ` [PATCH v6 05/11] block: remove split code in blkdev_issue_{discard,write_same} Jeff Moyer
2015-10-21 15:01 ` Ming Lin
2015-10-21 15:01 ` Ming Lin
2015-10-21 15:33 ` Mike Snitzer
2015-10-21 15:33 ` Mike Snitzer
2015-10-21 17:18 ` Ming Lin [this message]
2015-10-21 17:18 ` Ming Lin
2015-08-12 7:07 ` [PATCH v6 06/11] md/raid5: split bio for chunk_aligned_read Ming Lin
2015-08-12 7:07 ` [PATCH v6 07/11] md/raid5: get rid of bio_fits_rdev() Ming Lin
2015-08-12 7:07 ` [PATCH v6 08/11] block: kill merge_bvec_fn() completely Ming Lin
2015-08-12 7:07 ` [Drbd-dev] " Ming Lin
2015-08-12 7:07 ` [PATCH v6 09/11] fs: use helper bio_add_page() instead of open coding on bi_io_vec Ming Lin
2015-08-12 7:07 ` [PATCH v6 10/11] block: remove bio_get_nr_vecs() Ming Lin
2015-08-12 7:07 ` [PATCH v6 11/11] Documentation: update notes in biovecs about arbitrarily sized bios Ming Lin
2015-08-13 16:51 ` [PATCH v6 00/11] simplify block layer based on immutable biovecs Jens Axboe
2015-08-13 17:03 ` Ming Lin
2015-08-13 17:07 ` Jens Axboe
2015-08-13 17:36 ` Ming Lin
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=1445447902.26847.3.camel@ssi \
--to=mlin@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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.