Sounds good! Thanks for the guidance and v3) —Alexey > On May 21, 2021, at 7:29 AM, Theodore Y. Ts'o <tytso@xxxxxxx> wrote: > > On Fri, May 21, 2021 at 12:43:46AM -0700, Alexey Makhalov wrote: >> Hi Ted, >> >> Good point! This paragraph can be just dropped as the next one >> describes the issue with superblock re-read. Will send v2 shortly. > > Thanks; for what it's worth, I'm going to be editing the commit > description anyway; it's really helpful during the patch review to > understand how you found the problem, and how to reproduce it. > However, the commit description when it lands upstream will include a > link to this mail thread on lore.kernel.org, and so including a long > stack trace isn't really necessary. > > I'm going to cut it down to something like this: > > ------------------------------ > ext4: fix memory leak in ext4_fill_super > > Buffer head references must be released before calling kill_bdev(); > otherwise the buffer head (and its page referenced by b_data) will not > be freed by kill_bdev, and subsequently that bh will be leaked. > > If blocksizes differ, sb_set_blocksize() will kill current buffers and > page cache by using kill_bdev(). And then super block will be reread > again but using correct blocksize this time. sb_set_blocksize() didn't > fully free superblock page and buffer head, and being busy, they were > not freed and instead leaked. > > This can easily be reproduced by calling an infinite loop of: > > systemctl start <ext4_on_lvm>.mount, and > systemctl stop <ext4_on_lvm>.mount > > ... since systemd creates a cgroup for each slice which it mounts, and > the bh leak get amplified by a dying memory cgroup that also never > gets freed, and memory consumption is much more easily noticed. > > Signed-off-by: Alexey Makhalov <amakhalov@xxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Link: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fr%2F459B4724-842E-4B47-B2E7-D29805431E69%40vmware.com&data=04%7C01%7Camakhalov%40vmware.com%7Ce5ae2270f1134d9a3cce08d91c64cb26%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C1%7C637572041508854286%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=%2Fa%2FqTcBfL1tYkIq0byM46DXmpxTFOraAly2Ib9sbghE%3D&reserved=0 > Fixes: ce40733ce93d ("ext4: Check for return value from sb_set_blocksize") > Fixes: ac27a0ec112a ("ext4: initial copy of files from ext3") > Signed-off-by: "Theodore Ts'o" <tytso@xxxxxxx> > Cc: Andreas Dilger <adilger.kernel@xxxxxxxxx> > ------------------------------ > > The commit description above is 18 lines (exclusive of trailers and > headers) versus 71 lines, and is much more to the point for someone > who might be doing code archeology via "git log". > > When submitting this as a patch, you can either use a separate cover > letter (git format-patch --cover-letter) or including the explanation > after the --- in the diff, so that it disappears before the commit is > added via "git am". But it's not that hard for me to rework a commit > description, so I'll take care of it for this patch; no need to send a > V3. > > Cheers, > > - Ted
Attachment:
signature.asc
Description: Message signed with OpenPGP
