Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1530904 > unrolled thread
| Started by | Konstantin Khlebnikov <khlebnikov@yandex-team.ru> |
|---|---|
| First post | 2016-11-27 17:40 +0100 |
| Last post | 2016-11-30 01:00 +0100 |
| Articles | 4 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] md/raid5: limit request size according to implementation limits Konstantin Khlebnikov <khlebnikov@yandex-team.ru> - 2016-11-27 17:40 +0100
Re: [PATCH] md/raid5: limit request size according to implementation limits Coly Li <colyli@suse.de> - 2016-11-28 05:50 +0100
Re: [PATCH] md/raid5: limit request size according to implementation limits Konstantin Khlebnikov <koct9i@gmail.com> - 2016-11-28 07:10 +0100
Re: [PATCH] md/raid5: limit request size according to implementation limits Shaohua Li <shli@kernel.org> - 2016-11-30 01:00 +0100
| From | Konstantin Khlebnikov <khlebnikov@yandex-team.ru> |
|---|---|
| Date | 2016-11-27 17:40 +0100 |
| Subject | [PATCH] md/raid5: limit request size according to implementation limits |
| Message-ID | <sIaEG-6Hs-13@gated-at.bofh.it> |
Current implementation employ 16bit counter of active stripes in lower bits of bio->bi_phys_segments. If request is big enough to overflow this counter bio will be completed and freed too early. Fortunately this not happens in default configuration because several other limits prevent that: stripe_cache_size * nr_disks effectively limits count of active stripes. And small max_sectors_kb at lower disks prevent that during normal read/write operations. Overflow easily happens in discard if it's enabled by module parameter "devices_handle_discard_safely" and stripe_cache_size is set big enough. This patch limits requests size with 256Mb - 8Kb to prevent overflows. Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> Cc: Shaohua Li <shli@kernel.org> Cc: Neil Brown <neilb@suse.com> Cc: stable@vger.kernel.org --- drivers/md/raid5.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c index 92ac251e91e6..cce6057b9aca 100644 --- a/drivers/md/raid5.c +++ b/drivers/md/raid5.c @@ -6984,6 +6984,15 @@ static int raid5_run(struct mddev *mddev) stripe = (stripe | (stripe-1)) + 1; mddev->queue->limits.discard_alignment = stripe; mddev->queue->limits.discard_granularity = stripe; + + /* + * We use 16-bit counter of active stripes in bi_phys_segments + * (minus one for over-loaded initialization) + */ + blk_queue_max_hw_sectors(mddev->queue, 0xfffe * STRIPE_SECTORS); + blk_queue_max_discard_sectors(mddev->queue, + 0xfffe * STRIPE_SECTORS); + /* * unaligned part of discard request will be ignored, so can't * guarantee discard_zeroes_data
[toc] | [next] | [standalone]
| From | Coly Li <colyli@suse.de> |
|---|---|
| Date | 2016-11-28 05:50 +0100 |
| Message-ID | <sIm37-5E5-5@gated-at.bofh.it> |
| In reply to | #1530904 |
On 2016/11/28 上午12:32, Konstantin Khlebnikov wrote: > Current implementation employ 16bit counter of active stripes in lower > bits of bio->bi_phys_segments. If request is big enough to overflow > this counter bio will be completed and freed too early. > > Fortunately this not happens in default configuration because several > other limits prevent that: stripe_cache_size * nr_disks effectively > limits count of active stripes. And small max_sectors_kb at lower > disks prevent that during normal read/write operations. > > Overflow easily happens in discard if it's enabled by module parameter > "devices_handle_discard_safely" and stripe_cache_size is set big enough. > > This patch limits requests size with 256Mb - 8Kb to prevent overflows. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> > Cc: Shaohua Li <shli@kernel.org> > Cc: Neil Brown <neilb@suse.com> > Cc: stable@vger.kernel.org > --- > drivers/md/raid5.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c > index 92ac251e91e6..cce6057b9aca 100644 > --- a/drivers/md/raid5.c > +++ b/drivers/md/raid5.c > @@ -6984,6 +6984,15 @@ static int raid5_run(struct mddev *mddev) > stripe = (stripe | (stripe-1)) + 1; > mddev->queue->limits.discard_alignment = stripe; > mddev->queue->limits.discard_granularity = stripe; > + > + /* > + * We use 16-bit counter of active stripes in bi_phys_segments > + * (minus one for over-loaded initialization) > + */ > + blk_queue_max_hw_sectors(mddev->queue, 0xfffe * STRIPE_SECTORS); > + blk_queue_max_discard_sectors(mddev->queue, > + 0xfffe * STRIPE_SECTORS); > + Could you please to explain why use 0xfffe * STRIPE_SECTORS here ? Thanks. Coly
[toc] | [prev] | [next] | [standalone]
| From | Konstantin Khlebnikov <koct9i@gmail.com> |
|---|---|
| Date | 2016-11-28 07:10 +0100 |
| Subject | Re: [PATCH] md/raid5: limit request size according to implementation limits |
| Message-ID | <sIniy-6HM-5@gated-at.bofh.it> |
| In reply to | #1531032 |
On Mon, Nov 28, 2016 at 7:40 AM, Coly Li <colyli@suse.de> wrote: > On 2016/11/28 上午12:32, Konstantin Khlebnikov wrote: >> Current implementation employ 16bit counter of active stripes in lower >> bits of bio->bi_phys_segments. If request is big enough to overflow >> this counter bio will be completed and freed too early. >> >> Fortunately this not happens in default configuration because several >> other limits prevent that: stripe_cache_size * nr_disks effectively >> limits count of active stripes. And small max_sectors_kb at lower >> disks prevent that during normal read/write operations. >> >> Overflow easily happens in discard if it's enabled by module parameter >> "devices_handle_discard_safely" and stripe_cache_size is set big enough. >> >> This patch limits requests size with 256Mb - 8Kb to prevent overflows. >> >> Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> >> Cc: Shaohua Li <shli@kernel.org> >> Cc: Neil Brown <neilb@suse.com> >> Cc: stable@vger.kernel.org >> --- >> drivers/md/raid5.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c >> index 92ac251e91e6..cce6057b9aca 100644 >> --- a/drivers/md/raid5.c >> +++ b/drivers/md/raid5.c >> @@ -6984,6 +6984,15 @@ static int raid5_run(struct mddev *mddev) >> stripe = (stripe | (stripe-1)) + 1; >> mddev->queue->limits.discard_alignment = stripe; >> mddev->queue->limits.discard_granularity = stripe; >> + >> + /* >> + * We use 16-bit counter of active stripes in bi_phys_segments >> + * (minus one for over-loaded initialization) >> + */ >> + blk_queue_max_hw_sectors(mddev->queue, 0xfffe * STRIPE_SECTORS); >> + blk_queue_max_discard_sectors(mddev->queue, >> + 0xfffe * STRIPE_SECTORS); >> + > > Could you please to explain why use 0xfffe * STRIPE_SECTORS here ? This code send individual bio to lower device for each STRIPE_SECTORS (8) and count them in 16-bit counter 0xffff max (you could find this constant above in this file) but counter initialized with 1 to prevent hitting zero during generation thus maximum is 0xfffe stripes which is 256Mb - 8Kb in bytes > > Thanks. > > Coly >
[toc] | [prev] | [next] | [standalone]
| From | Shaohua Li <shli@kernel.org> |
|---|---|
| Date | 2016-11-30 01:00 +0100 |
| Message-ID | <sJ0tz-6JF-5@gated-at.bofh.it> |
| In reply to | #1530904 |
On Sun, Nov 27, 2016 at 07:32:32PM +0300, Konstantin Khlebnikov wrote: > Current implementation employ 16bit counter of active stripes in lower > bits of bio->bi_phys_segments. If request is big enough to overflow > this counter bio will be completed and freed too early. > > Fortunately this not happens in default configuration because several > other limits prevent that: stripe_cache_size * nr_disks effectively > limits count of active stripes. And small max_sectors_kb at lower > disks prevent that during normal read/write operations. > > Overflow easily happens in discard if it's enabled by module parameter > "devices_handle_discard_safely" and stripe_cache_size is set big enough. > > This patch limits requests size with 256Mb - 8Kb to prevent overflows. > > Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> > Cc: Shaohua Li <shli@kernel.org> > Cc: Neil Brown <neilb@suse.com> > Cc: stable@vger.kernel.org > --- > drivers/md/raid5.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c > index 92ac251e91e6..cce6057b9aca 100644 > --- a/drivers/md/raid5.c > +++ b/drivers/md/raid5.c > @@ -6984,6 +6984,15 @@ static int raid5_run(struct mddev *mddev) > stripe = (stripe | (stripe-1)) + 1; > mddev->queue->limits.discard_alignment = stripe; > mddev->queue->limits.discard_granularity = stripe; > + > + /* > + * We use 16-bit counter of active stripes in bi_phys_segments > + * (minus one for over-loaded initialization) > + */ > + blk_queue_max_hw_sectors(mddev->queue, 0xfffe * STRIPE_SECTORS); > + blk_queue_max_discard_sectors(mddev->queue, > + 0xfffe * STRIPE_SECTORS); > + > /* > * unaligned part of discard request will be ignored, so can't > * guarantee discard_zeroes_data Thanks! I applied this one, which is easy for stable too. After Neil's patches to remove the limitation, we can remove this one. Thanks, Shaohua
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web