Re: [GIT PULL] Block changes for 4.18-rc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 3, 2018 at 5:42 PM Jens Axboe <axboe@xxxxxxxxx> wrote:
>
>  drivers/md/md.c                                 |  61 +--
>  drivers/md/md.h                                 |   4 +-

So I've pulled this, but I get a new warning:

  drivers/md/md.c: In function ‘mddev_put’:
  drivers/md/md.c:543:1: warning: the frame size of 2112 bytes is
larger than 2048 bytes [-Wframe-larger-than=]

which seems to be due to commit afeee514ce7f ("md: convert to
bioset_init()/mempool_init()").

As of that commit, mddev_put() now allocates *two* "struct bio_set"
structures on the stack, and those really aren't small. Yes, part of
it is that I do my test-builds with all that debug stuff, but those
structures have several embedded mempool_t members, and they really
are pretty sizable.

And I _really_ do not think it's acceptable to have that kind of stack
usage in there. I'm not sure you can trigger mddev_put() in the IO
paths, but even without that I don't think it's acceptable.

Also, the code simply looks like complete and utter garbage. It does

                bs = mddev->bio_set;
                sync_bs = mddev->sync_set;

to _copy_ those structures, and then does bioset_exit() on them. WTF?

Why the hell doesn't it just do biset_exit() on the originals instead,
before freeing the mddev?

I've pulled this, but this needs to be fixed. That is just broken
garbage right now. No way in hell is it acceptable to waste 2kB of
stack space on something idiotic like this.

                 Linus




[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux