Re: [PATCH] md/raid0: don't use line[200] to store zone info

[Guoqing Jiang <gqjiang@xxxxxxxx>]

On 4/11/19 5:44 AM, Song Liu wrote:
> On Wed, Apr 10, 2019 at 2:10 AM Guoqing Jiang <gqjiang@xxxxxxxx> wrote:
> > The line[200] may not big enough to create raid0 with
> > lots of members, then the calltrace appears.
> >
> > [   42.081732] WARNING: CPU: 1 PID: 282 at lib/vsprintf.c:2334 vsnprintf+0x485/0x520
> > [   42.082318] Modules linked in: raid0(E) md_mod(E) zram(E) configfs(E) dm_mod(E)
> > [   42.082318] CPU: 1 PID: 282 Comm: mdadm Tainted: G            E     5.1.0-rc4-5-default+ #30
> > [   42.082318] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014
> > [   42.082318] RIP: 0010:vsnprintf+0x485/0x520
> > [   42.082318] Code: 08 48 8d 42 08 49 89 44 24 08 e9 1a fe ff ff 83 f8 2f 77 49 89 c2 83 c0 08 49 03 54 24 10 41 89 04 24 48 63 12 e9 02 fe ff ff <0f> 0b e9 26 fc ff ff 83 f8 2f 77 17 89 c2 83 c0 08 49 03 54 24 10
> > [   42.082318] RSP: 0018:ffffc9000095fa88 EFLAGS: 00010292
> > [   42.082318] RAX: 0000000000000000 RBX: 00000000000000cc RCX: ffffc9000095fae0
> > [   42.082318] RDX: ffffffffa007f649 RSI: fffffffffffffffc RDI: ffffc9000095fc44
> > [   42.082318] RBP: ffffc9000095fb30 R08: ffffc9000095fb58 R09: 000000008e97e7f4
> > [   42.082318] R10: ffffc9000095fb78 R11: 0000000000000000 R12: 0000000000000000
> > [   42.082318] R13: ffffc9000095fb78 R14: 000000000000001f R15: 0000000000000000
> > [   42.090354] FS:  00007fc0b3cc4700(0000) GS:ffff88807b400000(0000) knlGS:0000000000000000
> > [   42.090354] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [   42.090354] CR2: 000000000121cd88 CR3: 000000007df96000 CR4: 00000000000006e0
> > [   42.090354] Call Trace:
> > [   42.090354]  snprintf+0x49/0x60
> > [   42.090354]  ? disk_name+0x8b/0xa0
> > [   42.090354]  dump_zones+0x104/0x220 [raid0]
> > [   42.090354]  ? validate_chain.isra.43+0x6ec/0xad0
> > [   42.090354]  raid0_run+0x197/0x240 [raid0]
> > [   42.090354]  md_run+0x45a/0xae0 [md_mod]
> > [   42.090354]  ? security_capable+0x38/0x50
> > [   42.090354]  do_md_run+0xf/0xc0 [md_mod]
> > [   42.090354]  md_ioctl+0x1169/0x1b00 [md_mod]
> > [   42.090354]  ? cyc2ns_read_begin+0x20/0x50
> > [   42.090354]  ? sched_clock+0x5/0x10
> > [   42.090354]  blkdev_ioctl+0x506/0xa30
> > [   42.090354]  ? selinux_file_ioctl+0x190/0x230
> > [   42.090354]  block_ioctl+0x3f/0x50
> > [   42.090354]  do_vfs_ioctl+0xa5/0x6e0
> > [   42.098359]  ksys_ioctl+0x60/0x90
> > [   42.098359]  __x64_sys_ioctl+0x16/0x20
> > [   42.098359]  do_syscall_64+0x60/0x210
> > [   42.098359]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> >
> > So revert it to use previous way.
> >
> > Fixes: 766038846e875 ("md/raid0: replace printk() with pr_*()")
> > Signed-off-by: Guoqing Jiang <gqjiang@xxxxxxxx>
> > ---
> >   drivers/md/raid0.c | 11 +++++------
> >   1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> > index f3fb5bb8c82a..edd17025faf4 100644
> > --- a/drivers/md/raid0.c
> > +++ b/drivers/md/raid0.c
> > @@ -63,14 +63,13 @@ static void dump_zones(struct mddev *mddev)
> >                   mdname(mddev),
> >                   conf->nr_strip_zones, conf->nr_strip_zones==1?"":"s");
> >          for (j = 0; j < conf->nr_strip_zones; j++) {
> > -               char line[200];
> > -               int len = 0;
> > +               printk(KERN_INFO "md: zone%d=[", j);
> >
> >                  for (k = 0; k < conf->strip_zone[j].nb_dev; k++)
> > -                       len += snprintf(line+len, 200-len, "%s%s", k?"/":"",
> > -                                       bdevname(conf->devlist[j*raid_disks
> > -                                                              + k]->bdev, b));
> > -               pr_debug("md: zone%d=[%s]\n", j, line);
> > +                       printk(KERN_CONT "%s%s", k?"/":"",
> > +                       bdevname(conf->devlist[j*raid_disks
> > +                                               + k]->bdev, b));
> > +               printk(KERN_CONT "]\n");
> I think it is not recommended to use KERN_CONT here. How about we
> trim the line like:
>
> diff --git i/drivers/md/raid0.c w/drivers/md/raid0.c
> index f3fb5bb8c82a..cb996aa9f02b 100644
> --- i/drivers/md/raid0.c
> +++ w/drivers/md/raid0.c
> @@ -66,7 +66,7 @@ static void dump_zones(struct mddev *mddev)
>                  char line[200];
>                  int len = 0;
>
> -               for (k = 0; k < conf->strip_zone[j].nb_dev; k++)
> +               for (k = 0; k < conf->strip_zone[j].nb_dev && len < 200; k++)
>                          len += snprintf(line+len, 200-len, "%s%s", k?"/":"",
>                                          bdevname(conf->devlist[j*raid_disks
>                                                                 + k]->bdev, b));

Then we can't get the all the member of zone with "pr_debug("md: 
zone%d=[%s]\n", j, line)",
eg, if create raid0 with zram0-zram31, the above only can show:

zone0=[zram0/zram1/zram10/zram11/zram12/zram13/zram14/zram15/zram16/zram17/zram18/zram19/zram2/zram20/zram21/zram22/zram23/zram24/zram25/zram26/zram27/zram28/zram29/zram3/zram30/zram31/zram4/zram5/zram6/zra]

Thanks,
Guoqing