Messages in this thread | | | Subject | Re: [PATCH] Fix for vma merging refcounting bug | From | "Stephen C. Tweedie" <> | Date | 11 May 2003 21:04:06 +0100 |
| |
Hi,
On Sat, 2003-05-10 at 17:33, Andrea Arcangeli wrote: > On Fri, May 09, 2003 at 01:34:21PM +0100, Stephen C. Tweedie wrote: > > When a new vma can be merged simultaneously with its two immediate > > neighbours in both directions, vma_merge() extends the predecessor vma > > and deletes the successor. However, if the vma maps a file, it fails to > > fput() when doing the delete, leaving the file's refcount inconsistent.
> great catch! nobody could notice it in practice
Yep --- I only noticed it because I was running a quick-and-dirty vma merging test and wanted to test on a shmfs file, and noticed that the temporary shmfs filesystem became unmountable afterwards. Test attached, in case anybody is interested (it's the third test, mapping a file page by page in two interleaved passes, which triggers this case.)
> I'm attaching for review what I'm applying to my -aa tree, to fix the > above and the other issue with the non-ram vma merging fixed in 2.5.
Looks OK.
Cheers, Stephen
#include <stdio.h> #include <stdlib.h> #include <string.h> #include <errno.h>
#include <unistd.h> #include <fcntl.h> #include <sys/mman.h> #include <sys/ipc.h> #include <sys/shm.h>
static char *testfile = "/tmp/vma-test.dat";
#define TEST_PAGES 1024 int pagesize; int filesize;
char *map_addr;
void DIE(char *) __attribute__ ((noreturn));
void DIE(char *why) { perror(why); exit(1); }
#define plural(n) (((n) == 1) ? "" : "s") void test_maps(char *which) { int fd; int rc; int count = 0; char buffer[256]; char filename[128]; FILE *mapfile;
fd = open("/proc/self/maps", O_RDONLY); if (fd < 0) DIE("open(/proc/self/maps");
mapfile = fopen("/proc/self/maps", "r");
while (1) { if (!fgets(buffer, 256, mapfile)) break; rc = sscanf(buffer, "%*x-%*x %*4s %*x %*5s %*d %127s\n", filename); if (!rc) continue; if (!strcmp(testfile, filename)) count++; } printf("Testing %s: found %d map%s\n", which, count, plural(count)); }
#define clear_maps() \ err = munmap(map_addr, filesize); \ if (err) \ DIE("munmap"); \
static void map_page(int fd, int i) { char *ptr;
ptr = mmap(map_addr + i * pagesize, pagesize, PROT_READ, MAP_SHARED | MAP_FIXED, fd, i * pagesize); if (ptr == MAP_FAILED) DIE("mmap"); if (ptr != map_addr + i * pagesize) { fprintf(stderr, "mmap returned unexpected address\n"); exit(1); } }
int main(int argc, char *argv[]) { int fd; int err; int i;
if (argc > 1) testfile = argv[1]; pagesize = getpagesize(); filesize = TEST_PAGES * pagesize;
fd = open(testfile, O_CREAT|O_TRUNC|O_RDWR, 0666); if (fd < 0) DIE("open");
err = ftruncate(fd, filesize); if (err) DIE("ftuncate");
/* Find a suitable mmap address for the entire file */ map_addr = mmap(0, filesize, PROT_READ, MAP_SHARED, fd, 0); if (map_addr == MAP_FAILED) DIE("mmap"); clear_maps();
/* Now map it in piece by piece */ for (i = 0; i < TEST_PAGES; i++) map_page(fd, i); test_maps("backwards merging"); clear_maps();
/* Next, map it in backwards */ for (i = TEST_PAGES; i-- > 0; ) map_page(fd, i); test_maps("forwards merging"); clear_maps();
/* Finally, map it in in two interleaved passes */ for (i = 0; i < TEST_PAGES; i+=2) map_page(fd, i); for (i = 1; i < TEST_PAGES; i+=2) map_page(fd, i); test_maps("interleaved merging");
close(fd); unlink(testfile); return 0; }
| |