Date: Tue, 29 Mar 2016 15:01:10 -0700
From: Shaohua Li <shli@fb.com>
To: Christoph Hellwig <hch@infradead.org>
CC: <linux-block@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <axboe@fb.com>, <Kernel-team@fb.com>, "4.2+" <stable@vger.kernel.org>,
        Ming Lei <ming.lei@canonical.com>
Subject: Re: [PATCH] block: don't make BLK_DEF_MAX_SECTORS too big
Message-ID: <20160329220053.GA1716708@devbig084.prn1.facebook.com>
References: <21cf85d32278bbe5acbc3def0a6db75db98a2670.1459269590.git.shli@fb.com>
 <20160329211833.GA18532@infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20160329211833.GA18532@infradead.org>
User-Agent: Mutt/1.5.20 (2009-12-10)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1185
Lines: 21

On Tue, Mar 29, 2016 at 02:18:33PM -0700, Christoph Hellwig wrote:
> On Tue, Mar 29, 2016 at 09:42:33AM -0700, Shaohua Li wrote:
> > bio_alloc_bioset() allocates bvecs from bvec_slabs which can only
> > allocate maximum 256 bvec (eg, 1M for 4k pages). We can't bump
> > BLK_DEF_MAX_SECTORS to exceed this value otherwise bio_alloc_bioset will
> > fail.
> 
> This might be true, but it's not a good enough reason.  Request based
> driver couldn't care less about the limits of bio_alloc_bioset.
> 
> It seems the bug is that somone (would be great to know whoe exactly)
> passes a too large value to bio_alloc_bioset. And given that we still
> have bio_add_page around for actually adding pages to a bio it seems
> like the proper fix would be to simply clamp down the actual allocation
> and segment limit inside bio_alloc_bioset.  Which would also help to
> eventually remove code doing just that in tons of callers.

The problem is bcache allocates a big bio (with bio_alloc). The bio is
split with blk_queue_split, but it isn't split to small size because
queue limit. the bio is cloned later in md, which uses bio_alloc_bioset.
bio_alloc_bioset itself can't allocate big size bio.