From: Mike Snitzer <snitzer@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: dm-devel@redhat.com, linux-kernel@vger.kernel.org,
martin.petersen@oracle.com
Subject: Re: block: fix alignment_offset math that assumes io_min is a power-of-2
Date: Wed, 8 Oct 2014 18:41:50 -0400 [thread overview]
Message-ID: <20141008224149.GB15590@redhat.com> (raw)
In-Reply-To: <5435BCD5.9050306@kernel.dk>
On Wed, Oct 08 2014 at 6:38pm -0400,
Jens Axboe <axboe@kernel.dk> wrote:
> On 10/08/2014 04:28 PM, Mike Snitzer wrote:
> > On Wed, Oct 08 2014 at 6:12pm -0400,
> > Jens Axboe <axboe@kernel.dk> wrote:
> >
> >> On 10/08/2014 04:05 PM, Mike Snitzer wrote:
> >>> The math in both blk_stack_limits() and queue_limit_alignment_offset()
> >>> assume that a block device's io_min (aka minimum_io_size) is always a
> >>> power-of-2. Fix the math such that it works for non-power-of-2 io_min.
> >>>
> >>> This issue (of alignment_offset != 0) became apparent when testing
> >>> dm-thinp with a thinp blocksize that matches a RAID6 stripesize of
> >>> 1280K. Commit fdfb4c8c1 ("dm thin: set minimum_io_size to pool's data
> >>> block size") unlocked the potential for alignment_offset != 0 due to
> >>> the dm-thin-pool's io_min possibly being a non-power-of-2.
> >>
> >> Well that sucks, AND with a mask is considerably cheaper than a MOD...
> >
> > Yeah, certainly does suck (please note v2 that I just sent). The MODs
> > shouldn't kill us, these functions aren't called in any real hot path.
> > A storm at boot maybe.. or SCSI rescan but...
>
> I had it mixed up with the recent blk_max_size_offset() - you are right,
> this is not in a hot path. For that case, I don't really care, it's fine.
>
> Is v2 runtime tested?
Yes.
Here is the DM stack for an lvm created dm-thin-pool (dm-5).
# lsblk /dev/skd0
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
skd0 252:0 0 745.3G 0 disk
├─bricks-mypool_tmeta 253:2 0 15.8G 0 lvm
│ └─bricks-mypool-tpool 253:4 0 512G 0 lvm
│ └─bricks-mypool 253:5 0 512G 0 lvm
└─bricks-mypool_tdata 253:3 0 512G 0 lvm
└─bricks-mypool-tpool 253:4 0 512G 0 lvm
└─bricks-mypool 253:5 0 512G 0 lvm
Before patch:
# cat /sys/block/dm-5/alignment_offset
1048576
After patch:
# cat /sys/block/dm-5/alignment_offset
0
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-kernel@vger.kernel.org, dm-devel@redhat.com,
martin.petersen@oracle.com
Subject: Re: block: fix alignment_offset math that assumes io_min is a power-of-2
Date: Wed, 8 Oct 2014 18:41:50 -0400 [thread overview]
Message-ID: <20141008224149.GB15590@redhat.com> (raw)
In-Reply-To: <5435BCD5.9050306@kernel.dk>
On Wed, Oct 08 2014 at 6:38pm -0400,
Jens Axboe <axboe@kernel.dk> wrote:
> On 10/08/2014 04:28 PM, Mike Snitzer wrote:
> > On Wed, Oct 08 2014 at 6:12pm -0400,
> > Jens Axboe <axboe@kernel.dk> wrote:
> >
> >> On 10/08/2014 04:05 PM, Mike Snitzer wrote:
> >>> The math in both blk_stack_limits() and queue_limit_alignment_offset()
> >>> assume that a block device's io_min (aka minimum_io_size) is always a
> >>> power-of-2. Fix the math such that it works for non-power-of-2 io_min.
> >>>
> >>> This issue (of alignment_offset != 0) became apparent when testing
> >>> dm-thinp with a thinp blocksize that matches a RAID6 stripesize of
> >>> 1280K. Commit fdfb4c8c1 ("dm thin: set minimum_io_size to pool's data
> >>> block size") unlocked the potential for alignment_offset != 0 due to
> >>> the dm-thin-pool's io_min possibly being a non-power-of-2.
> >>
> >> Well that sucks, AND with a mask is considerably cheaper than a MOD...
> >
> > Yeah, certainly does suck (please note v2 that I just sent). The MODs
> > shouldn't kill us, these functions aren't called in any real hot path.
> > A storm at boot maybe.. or SCSI rescan but...
>
> I had it mixed up with the recent blk_max_size_offset() - you are right,
> this is not in a hot path. For that case, I don't really care, it's fine.
>
> Is v2 runtime tested?
Yes.
Here is the DM stack for an lvm created dm-thin-pool (dm-5).
# lsblk /dev/skd0
NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
skd0 252:0 0 745.3G 0 disk
├─bricks-mypool_tmeta 253:2 0 15.8G 0 lvm
│ └─bricks-mypool-tpool 253:4 0 512G 0 lvm
│ └─bricks-mypool 253:5 0 512G 0 lvm
└─bricks-mypool_tdata 253:3 0 512G 0 lvm
└─bricks-mypool-tpool 253:4 0 512G 0 lvm
└─bricks-mypool 253:5 0 512G 0 lvm
Before patch:
# cat /sys/block/dm-5/alignment_offset
1048576
After patch:
# cat /sys/block/dm-5/alignment_offset
0
next prev parent reply other threads:[~2014-10-08 22:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-08 22:05 [PATCH] block: fix alignment_offset math that assumes io_min is a power-of-2 Mike Snitzer
2014-10-08 22:12 ` Jens Axboe
2014-10-08 22:28 ` Mike Snitzer
2014-10-08 22:38 ` Jens Axboe
2014-10-08 22:41 ` Mike Snitzer [this message]
2014-10-08 22:41 ` Mike Snitzer
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=20141008224149.GB15590@redhat.com \
--to=snitzer@redhat.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=martin.petersen@oracle.com \
/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.