On Wed, Oct 14, 2020 at 07:44:16PM -0700, Linus Torvalds wrote: > On Wed, Oct 14, 2020 at 6:48 PM Qian Cai <cai@xxxxxx> wrote: > > > > While on this topic, I just want to bring up a bug report that we are chasing an > > issue that a process is stuck in the loop of wait_on_page_bit_common() for more > > than 10 minutes before I gave up. > > Judging by call trace, that looks like a deadlock rather than a missed wakeup. > > The trace isn't reliable, but I find it suspicious that the call trace > just before the fault contains that > "iov_iter_copy_from_user_atomic()". > > IOW, I think you're in fuse_fill_write_pages(), which has allocated > the page, locked it, and then it takes a page fault. > > And the page fault waits on a page that is locked. > > This is a classic deadlock. > > The *intent* is that iov_iter_copy_from_user_atomic() returns zero, > and you retry without the page lock held. > > HOWEVER. > > That's not what fuse actually does. Fuse will do multiple pages, and > it will unlock only the _last_ page. It keeps the other pages locked, > and puts them in an array: > > ap->pages[ap->num_pages] = page; > > And after the iov_iter_copy_from_user_atomic() fails, it does that > "unlock" and repeat. > > But while the _last_ page was unlocked, the *previous* pages are still > locked in that array. Deadlock. > > I really don't think this has anything at all to do with page locking, > and everything to do with fuse_fill_write_pages() having a deadlock if > the source of data is a mmap of one of the pages it is trying to write > to (just with an offset, so that it's not the last page). > > See a similar code sequence in generic_perform_write(), but notice how > that code only has *one* page that it locks, and never holds an array > of pages around over that iov_iter_fault_in_readable() thing. Indeed. This is a deadlock in fuse. Thanks for the analysis. I can now trivially reproduce it with following program. Thanks Vivek #include <stdio.h> #include <stdlib.h> #include <errno.h> #include <string.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <unistd.h> #include <sys/mman.h> #include <sys/uio.h> int main(int argc, char *argv[]) { int fd, ret; void *map_addr; size_t map_length = 2 * 4096; char *buf_out = "Hello World"; struct iovec iov[2]; if (argc != 2 ) { printf("Usage:%s <file-to-write> \n", argv[0]); exit(1); } fd = open(argv[1], O_RDWR | O_TRUNC); if (fd == -1) { fprintf(stderr, "Failed to open file %s:%s, errorno=%d\n", argv[1], strerror(errno), errno); exit(1); } map_addr = mmap(NULL, map_length, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (map_addr == MAP_FAILED) { fprintf(stderr, "mmap failed %s, errorno=%d\n", strerror(errno), errno); exit(1); } /* Write first page and second page */ pwrite(fd, buf_out, strlen(buf_out), 0); pwrite(fd, buf_out, strlen(buf_out), 4096); /* Copy from first page and then second page */ iov[0].iov_base = map_addr; iov[0].iov_len = strlen(buf_out) + 1; iov[1].iov_base = map_addr + 4096; iov[1].iov_len = strlen(buf_out) + 1; /* * Write second page offset (4K - 12), reading from first page and * then second page. In first iteration we should fault in first * page and lock second page. And in second iteration we should * try fault in second page which is locked. Deadlock. */ ret = pwritev(fd, iov, 2, 4096 + 4096 - 12); printf("write() returned=%d\n", ret); munmap(map_addr, map_length); close(fd); }