From: Lars Ellenberg <lars.ellenberg@linbit.com>
To: Alasdair G Kergon <agk@redhat.com>
Cc: device-mapper development <dm-devel@redhat.com>,
Mikulas Patocka <mpatocka@redhat.com>,
linux-kernel@vger.kernel.org, jens.axboe@oracle.com
Subject: [PATCH] dm: max_segments=1 if merge_bvec_fn is not supported
Date: Sat, 6 Mar 2010 22:10:13 +0100 [thread overview]
Message-ID: <20100306211012.GA9689@racke> (raw)
If the lower device exposes a merge_bvec_fn,
dm_set_device_limits() restricts max_sectors
to PAGE_SIZE "just to be safe".
This is not sufficient, however.
If someone uses bio_add_page() to add 8 disjunct 512 byte partial
pages to a bio, it would succeed, but could still cross a border
of whatever restrictions are below us (e.g. raid10 stripe boundary).
An attempted bio_split() would not succeed, because bi_vcnt is 8.
One example that triggered this frequently is the xen io layer.
raid10_make_request bug: can't convert block across chunks or bigger than 64k 209265151 1
Signed-off-by: Lars <lars.ellenberg@linbit.com>
---
Neil: you may want to double check linear.c and raid*.c for similar logic,
even though it is unlikely that someone puts md raid6 on top of something
exposing a merge_bvec_fn.
This is not the first time this has been patched, btw.
See https://bugzilla.redhat.com/show_bug.cgi?id=440093
and the patch by Mikulas:
https://bugzilla.redhat.com/attachment.cgi?id=342638&action=diff
---
drivers/md/dm-table.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 4b22feb..c686ff4 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -515,14 +515,22 @@ int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
/*
* Check if merge fn is supported.
- * If not we'll force DM to use PAGE_SIZE or
+ * If not we'll force DM to use single bio_vec of PAGE_SIZE or
* smaller I/O, just to be safe.
*/
- if (q->merge_bvec_fn && !ti->type->merge)
+ if (q->merge_bvec_fn && !ti->type->merge) {
limits->max_sectors =
min_not_zero(limits->max_sectors,
(unsigned int) (PAGE_SIZE >> 9));
+ /* Restricting max_sectors is not enough.
+ * If someone uses bio_add_page to add 8 disjunct 512 byte
+ * partial pages to a bio, it would succeed,
+ * but could still cross a border of whatever restrictions
+ * are below us (e.g. raid0 stripe boundary). An attempted
+ * bio_split() would not succeed, because bi_vcnt is 8. */
+ limits->max_segments = 1;
+ }
return 0;
}
EXPORT_SYMBOL_GPL(dm_set_device_limits);
--
1.6.3.3
WARNING: multiple messages have this Message-ID (diff)
From: Lars Ellenberg <lars.ellenberg@linbit.com>
To: Alasdair G Kergon <agk@redhat.com>
Cc: device-mapper development <dm-devel@redhat.com>,
Mikulas Patocka <mpatocka@redhat.com>, Neil Brown <neilb@suse.de>,
jens.axboe@oracle.com, linux-kernel@vger.kernel.org
Subject: [PATCH] dm: max_segments=1 if merge_bvec_fn is not supported
Date: Sat, 6 Mar 2010 22:10:13 +0100 [thread overview]
Message-ID: <20100306211012.GA9689@racke> (raw)
If the lower device exposes a merge_bvec_fn,
dm_set_device_limits() restricts max_sectors
to PAGE_SIZE "just to be safe".
This is not sufficient, however.
If someone uses bio_add_page() to add 8 disjunct 512 byte partial
pages to a bio, it would succeed, but could still cross a border
of whatever restrictions are below us (e.g. raid10 stripe boundary).
An attempted bio_split() would not succeed, because bi_vcnt is 8.
One example that triggered this frequently is the xen io layer.
raid10_make_request bug: can't convert block across chunks or bigger than 64k 209265151 1
Signed-off-by: Lars <lars.ellenberg@linbit.com>
---
Neil: you may want to double check linear.c and raid*.c for similar logic,
even though it is unlikely that someone puts md raid6 on top of something
exposing a merge_bvec_fn.
This is not the first time this has been patched, btw.
See https://bugzilla.redhat.com/show_bug.cgi?id=440093
and the patch by Mikulas:
https://bugzilla.redhat.com/attachment.cgi?id=342638&action=diff
---
drivers/md/dm-table.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
index 4b22feb..c686ff4 100644
--- a/drivers/md/dm-table.c
+++ b/drivers/md/dm-table.c
@@ -515,14 +515,22 @@ int dm_set_device_limits(struct dm_target *ti, struct dm_dev *dev,
/*
* Check if merge fn is supported.
- * If not we'll force DM to use PAGE_SIZE or
+ * If not we'll force DM to use single bio_vec of PAGE_SIZE or
* smaller I/O, just to be safe.
*/
- if (q->merge_bvec_fn && !ti->type->merge)
+ if (q->merge_bvec_fn && !ti->type->merge) {
limits->max_sectors =
min_not_zero(limits->max_sectors,
(unsigned int) (PAGE_SIZE >> 9));
+ /* Restricting max_sectors is not enough.
+ * If someone uses bio_add_page to add 8 disjunct 512 byte
+ * partial pages to a bio, it would succeed,
+ * but could still cross a border of whatever restrictions
+ * are below us (e.g. raid0 stripe boundary). An attempted
+ * bio_split() would not succeed, because bi_vcnt is 8. */
+ limits->max_segments = 1;
+ }
return 0;
}
EXPORT_SYMBOL_GPL(dm_set_device_limits);
--
1.6.3.3
next reply other threads:[~2010-03-06 21:10 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-06 21:10 Lars Ellenberg [this message]
2010-03-06 21:10 ` [PATCH] dm: max_segments=1 if merge_bvec_fn is not supported Lars Ellenberg
2010-03-08 5:33 ` Neil Brown
2010-03-08 8:35 ` Mikulas Patocka
2010-03-08 13:14 ` Lars Ellenberg
2010-03-18 18:48 ` Andrew Morton
2010-03-18 21:48 ` Neil Brown
2010-12-04 6:43 ` [PATCH] dm: check max_sectors in dm_merge_bvec (was: Re: dm: max_segments=1 if merge_bvec_fn is not supported) Mike Snitzer
2010-12-04 16:03 ` Lars Ellenberg
2010-12-04 19:21 ` 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=20100306211012.GA9689@racke \
--to=lars.ellenberg@linbit.com \
--cc=agk@redhat.com \
--cc=dm-devel@redhat.com \
--cc=jens.axboe@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpatocka@redhat.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.