On Fri, 30 Mar 2007, Andrew Morton wrote: > > <would anyone be interested in hearing my opinion on the DUMP_SEEK macro > again?>
Oooh, yes please. > diff -puN fs/binfmt_elf_fdpic.c~fix-page-leak-during-core-dump > fs/binfmt_elf_fdpic.c > --- a/fs/binfmt_elf_fdpic.c~fix-page-leak-during-core-dump > +++ a/fs/binfmt_elf_fdpic.c > @@ -1480,8 +1480,10 @@ static int elf_fdpic_dump_segments(struc > DUMP_SEEK(file->f_pos + PAGE_SIZE); > } > else if (page == ZERO_PAGE(addr)) { > - DUMP_SEEK(file->f_pos + PAGE_SIZE); > - page_cache_release(page); > + if (!dump_seek(file, file->f_pos + PAGE_SIZE)) { > + page_cache_release(page); > + return 0; > + } > } > else { > void *kaddr; > _ No, I think that's wrong: whereas the binfmt_elf one did its page_cache_release down below at the bottom of the block, this version does it in each subblock, so there you're removing the dump_seek success one. Can't we preserve that beauteous macro here and just do... --- a/fs/binfmt_elf_fdpic.c +++ b/fs/binfmt_elf_fdpic.c @@ -1480,8 +1480,8 @@ static int elf_fdpic_dump_segments(struc DUMP_SEEK(file->f_pos + PAGE_SIZE); } else if (page == ZERO_PAGE(addr)) { - DUMP_SEEK(file->f_pos + PAGE_SIZE); page_cache_release(page); + DUMP_SEEK(file->f_pos + PAGE_SIZE); } else { void *kaddr; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/