[PATCH] NULL pointer in raid1_make_request passed to bio_trim when adding md as bcache caching dev

[PATCH] NULL pointer in raid1_make_request passed to bio_trim when adding md as bcache caching dev

[Eric Wheeler]

On Sun, 10 Apr 2016 20:01:30 +0200, Sebastian Roesner wrote:
> Am 01.04.2016 um 20:14 schrieb Eric Wheeler:
> > On Mon, 28 Mar 2016, Shaohua Li wrote:
> > > On Sat, Mar 26, 2016 at 05:46:16PM +0100, Sebastian Roesner wrote:
> > > > Hello Ming, Eric,
> > > > 
> > > > Am 26.03.2016 um 16:40 schrieb Ming Lei:
> > > > I was able to reproduce it on a non-productive system, but only after
> > > > copying the bcache superblocks/partition starts from the original system,
> > > > with new created ones it worked fine.
> > >
> > > 320 bvecs exceeds what bio-clone_set can handle. Could you please try below patch?
> > >
> > Hey Sebastian,
> > 
> > Have you had a chance to test the patch below from Shaohua Li?
> 
> I just tried the patch and it did not crash anymore.
 
Hello all,

It doesn't look like this made it into v4.8-rc1 and it is known to fix at 
least Sebastian's issue.

Can this get into v4.8?

--
Eric Wheeler


commit 92761dad7ff6e1bf25de247e0064dd398e797599
Author: Shaohua Li <shli@fb.com>
Date:   Mon Mar 28 10:54:35 2016 -0700

    block: don't make BLK_DEF_MAX_SECTORS too big
    
    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 fixes commit 30e2bc08b2bb7c069. We probably should make the bvec_slabs
    hold bigger bvecs if bigger bio size is required.
    
    Signed-off-by: Shaohua Li <shli@fb.com>

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 7e5d7e0..da64325 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -1153,7 +1153,11 @@ extern int blk_verify_command(unsigned char *cmd, fmode_t has_write_perm);
 enum blk_default_limits {
 	BLK_MAX_SEGMENTS	= 128,
 	BLK_SAFE_MAX_SECTORS	= 255,
-	BLK_DEF_MAX_SECTORS	= 2560,
+	/*
+	 * if you change this, please also change bvec_alloc and BIO_MAX_PAGES.
+	 * Otherwise bio_alloc_bioset will break.
+	 */
+	BLK_DEF_MAX_SECTORS	= BIO_MAX_SECTORS,
 	BLK_MAX_SEGMENT_SIZE	= 65536,
 	BLK_SEG_BOUNDARY_MASK	= 0xFFFFFFFFUL,
 };
--
To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] NULL pointer in raid1_make_request passed to bio_trim when adding md as bcache caching dev

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1649 bytes --]

Hi Eric,

[auto build test ERROR on linus/master]
[also build test ERROR on v4.8-rc1 next-20160811]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Eric-Wheeler/NULL-pointer-in-raid1_make_request-passed-to-bio_trim-when-adding-md-as-bcache-caching-dev/20160811-115046
config: i386-randconfig-s0-201632 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Eric-Wheeler/NULL-pointer-in-raid1_make_request-passed-to-bio_trim-when-adding-md-as-bcache-caching-dev/20160811-115046 HEAD 84e331216ae8d3359dd0e7bf2cf4e11e30ca73e5 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   In file included from fs/btrfs/super.c:19:0:
>> include/linux/blkdev.h:1179:24: error: 'BIO_MAX_SECTORS' undeclared here (not in a function)
     BLK_DEF_MAX_SECTORS = BIO_MAX_SECTORS,
                           ^~~~~~~~~~~~~~~

vim +/BIO_MAX_SECTORS +1179 include/linux/blkdev.h

  1173		BLK_MAX_SEGMENTS	= 128,
  1174		BLK_SAFE_MAX_SECTORS	= 255,
  1175		/*
  1176		 * if you change this, please also change bvec_alloc and BIO_MAX_PAGES.
  1177		 * Otherwise bio_alloc_bioset will break.
  1178		 */
> 1179		BLK_DEF_MAX_SECTORS	= BIO_MAX_SECTORS,
  1180		BLK_MAX_SEGMENT_SIZE	= 65536,
  1181		BLK_SEG_BOUNDARY_MASK	= 0xFFFFFFFFUL,
  1182	};

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 34099 bytes --]

Re: [PATCH] NULL pointer in raid1_make_request passed to bio_trim when adding md as bcache caching dev

[Eric Wheeler]

On Wed, 10 Aug 2016, Eric Wheeler wrote:

> On Sun, 10 Apr 2016 20:01:30 +0200, Sebastian Roesner wrote:
> > Am 01.04.2016 um 20:14 schrieb Eric Wheeler:
> > > On Mon, 28 Mar 2016, Shaohua Li wrote:
> > > > On Sat, Mar 26, 2016 at 05:46:16PM +0100, Sebastian Roesner wrote:
> > > > > Hello Ming, Eric,
> > > > > 
> > > > > Am 26.03.2016 um 16:40 schrieb Ming Lei:
> > > > > I was able to reproduce it on a non-productive system, but only after
> > > > > copying the bcache superblocks/partition starts from the original system,
> > > > > with new created ones it worked fine.
> > > >
> > > > 320 bvecs exceeds what bio-clone_set can handle. Could you please try below patch?
> > > >
> > > Hey Sebastian,
> > > 
> > > Have you had a chance to test the patch below from Shaohua Li?
> > 
> > I just tried the patch and it did not crash anymore.
>  
> It doesn't look like this made it into v4.8-rc1 and it is known to fix at 
> least Sebastian's issue.
> 
> commit 92761dad7ff6e1bf25de247e0064dd398e797599
> Author: Shaohua Li <shli@fb.com>
> Date:   Mon Mar 28 10:54:35 2016 -0700
> 
>     block: don't make BLK_DEF_MAX_SECTORS too big
>     
>     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 fixes commit 30e2bc08b2bb7c069. We probably should make the bvec_slabs
>     hold bigger bvecs if bigger bio size is required.
>     
>     Signed-off-by: Shaohua Li <shli@fb.com>
> 
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 7e5d7e0..da64325 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -1153,7 +1153,11 @@ extern int blk_verify_command(unsigned char *cmd, fmode_t has_write_perm);
>  enum blk_default_limits {
>  	BLK_MAX_SEGMENTS	= 128,
>  	BLK_SAFE_MAX_SECTORS	= 255,
> -	BLK_DEF_MAX_SECTORS	= 2560,
> +	/*
> +	 * if you change this, please also change bvec_alloc and BIO_MAX_PAGES.
> +	 * Otherwise bio_alloc_bioset will break.
> +	 */
> +	BLK_DEF_MAX_SECTORS	= BIO_MAX_SECTORS,


It seems that I spoke too soon, as BIO_MAX_SECTORS was removed for v4.8.

What should BLK_DEF_MAX_SECTORS be defined as?  

Something like this?
	-	BLK_DEF_MAX_SECTORS     = 2560,
	+	BLK_DEF_MAX_SECTORS = (BIO_MAX_PAGES << (PAGE_SHIFT-9)

Or was this issue patched by Shaohua Li solved in some other way?

It appears that BLK_DEF_MAX_SECTORS is still used:

$ git grep BLK_DEF_MAX_SECTORS
block/blk-settings.c:   max_sectors = min_t(unsigned int, max_sectors, BLK_DEF_MAX_SECTORS);
drivers/block/aoe/aoeblk.c:     blk_queue_max_hw_sectors(q, BLK_DEF_MAX_SECTORS);
drivers/scsi/sd.c:              rw_max = BLK_DEF_MAX_SECTORS;


--
Eric Wheeler


>  	BLK_MAX_SEGMENT_SIZE	= 65536,
>  	BLK_SEG_BOUNDARY_MASK	= 0xFFFFFFFFUL,
>  };
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>