Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754218AbcK1GG4 (ORCPT ); Mon, 28 Nov 2016 01:06:56 -0500 Received: from mail-lf0-f65.google.com ([209.85.215.65]:34649 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752038AbcK1GGs (ORCPT ); Mon, 28 Nov 2016 01:06:48 -0500 MIME-Version: 1.0 In-Reply-To: References: <148026435214.19980.7956943898609877817.stgit@buzz> From: Konstantin Khlebnikov Date: Mon, 28 Nov 2016 09:06:45 +0300 Message-ID: Subject: Re: [PATCH] md/raid5: limit request size according to implementation limits To: Coly Li Cc: Konstantin Khlebnikov , Shaohua Li , Neil Brown , linux-raid@vger.kernel.org, Linux Kernel Mailing List , Stable Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id uAS670rx002692 Content-Length: 2316 Lines: 55 On Mon, Nov 28, 2016 at 7:40 AM, Coly Li 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 >> Cc: Shaohua Li >> Cc: Neil Brown >> 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 >