* [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages
@ 2021-10-13 0:13 Omar Sandoval
2021-10-13 3:41 ` Wang Yugui
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Omar Sandoval @ 2021-10-13 0:13 UTC (permalink / raw)
To: linux-btrfs; +Cc: kernel-team
From: Omar Sandoval <osandov@fb.com>
Attempting to defragment a Btrfs file containing a transparent huge page
immediately deadlocks with the following stack trace:
#0 context_switch (kernel/sched/core.c:4940:2)
#1 __schedule (kernel/sched/core.c:6287:8)
#2 schedule (kernel/sched/core.c:6366:3)
#3 io_schedule (kernel/sched/core.c:8389:2)
#4 wait_on_page_bit_common (mm/filemap.c:1356:4)
#5 __lock_page (mm/filemap.c:1648:2)
#6 lock_page (./include/linux/pagemap.h:625:3)
#7 pagecache_get_page (mm/filemap.c:1910:4)
#8 find_or_create_page (./include/linux/pagemap.h:420:9)
#9 defrag_prepare_one_page (fs/btrfs/ioctl.c:1068:9)
#10 defrag_one_range (fs/btrfs/ioctl.c:1326:14)
#11 defrag_one_cluster (fs/btrfs/ioctl.c:1421:9)
#12 btrfs_defrag_file (fs/btrfs/ioctl.c:1523:9)
#13 btrfs_ioctl_defrag (fs/btrfs/ioctl.c:3117:9)
#14 btrfs_ioctl (fs/btrfs/ioctl.c:4872:10)
#15 vfs_ioctl (fs/ioctl.c:51:10)
#16 __do_sys_ioctl (fs/ioctl.c:874:11)
#17 __se_sys_ioctl (fs/ioctl.c:860:1)
#18 __x64_sys_ioctl (fs/ioctl.c:860:1)
#19 do_syscall_x64 (arch/x86/entry/common.c:50:14)
#20 do_syscall_64 (arch/x86/entry/common.c:80:7)
#21 entry_SYSCALL_64+0x7c/0x15b (arch/x86/entry/entry_64.S:113)
A huge page is represented by a compound page, which consists of a
struct page for each PAGE_SIZE page within the huge page. The first
struct page is the "head page", and the remaining are "tail pages".
Defragmentation attempts to lock each page in the range. However,
lock_page() on a tail page actually locks the corresponding head page.
So, if defragmentation tries to lock more than one struct page in a
compound page, it tries to lock the same head page twice and deadlocks
with itself.
There are a few options for dealing with this:
1. Return an error if defrag is called for a transparent huge page.
2. Silently skip over defragging transparent huge pages.
3. Implement real support for defragging transparent huge pages.
This patch implements option 1. The error code (ETXTBUSY) is up for
bikeshedding. Option 2 seems reasonable as well. Option 3, however,
would require more effort. THP for filesystems is currently read-only,
so I suspect that a lot of code is not ready for using these pages for
I/O.
This can be reproduced with:
$ cat create_thp_file.c
#include <fcntl.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/mman.h>
static const char zeroes[1024 * 1024];
static const size_t FILE_SIZE = 2 * 1024 * 1024;
int main(int argc, char **argv)
{
if (argc != 2) {
fprintf(stderr, "usage: %s PATH\n", argv[0]);
return EXIT_FAILURE;
}
int fd = creat(argv[1], 0777);
if (fd == -1) {
perror("creat");
return EXIT_FAILURE;
}
size_t written = 0;
while (written < FILE_SIZE) {
ssize_t ret = write(fd, zeroes,
sizeof(zeroes) < FILE_SIZE - written ?
sizeof(zeroes) : FILE_SIZE - written);
if (ret < 0) {
perror("write");
return EXIT_FAILURE;
}
written += ret;
}
close(fd);
fd = open(argv[1], O_RDONLY);
if (fd == -1) {
perror("open");
return EXIT_FAILURE;
}
/*
* Reserve some address space so that we can align the file mapping to
* the huge page size.
*/
void *placeholder_map = mmap(NULL, FILE_SIZE * 2, PROT_NONE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
if (placeholder_map == MAP_FAILED) {
perror("mmap (placeholder)");
return EXIT_FAILURE;
}
void *aligned_address =
(void *)(((uintptr_t)placeholder_map + FILE_SIZE - 1) & ~(FILE_SIZE - 1));
void *map = mmap(aligned_address, FILE_SIZE, PROT_READ | PROT_EXEC,
MAP_SHARED | MAP_FIXED, fd, 0);
if (map == MAP_FAILED) {
perror("mmap");
return EXIT_FAILURE;
}
if (madvise(map, FILE_SIZE, MADV_HUGEPAGE) < 0) {
perror("madvise");
return EXIT_FAILURE;
}
char *line = NULL;
size_t line_capacity = 0;
FILE *smaps_file = fopen("/proc/self/smaps", "r");
if (!smaps_file) {
perror("fopen");
return EXIT_FAILURE;
}
for (;;) {
for (size_t off = 0; off < FILE_SIZE; off += 4096)
((volatile char *)map)[off];
ssize_t ret;
bool this_mapping = false;
while ((ret = getline(&line, &line_capacity, smaps_file)) > 0) {
unsigned long start, end, huge;
if (sscanf(line, "%lx-%lx", &start, &end) == 2) {
this_mapping = (start <= (uintptr_t)map &&
(uintptr_t)map < end);
} else if (this_mapping &&
sscanf(line, "FilePmdMapped: %ld", &huge) == 1 &&
huge > 0) {
return EXIT_SUCCESS;
}
}
sleep(6);
rewind(smaps_file);
fflush(smaps_file);
}
}
$ ./create_thp_file huge
$ btrfs fi defrag -czstd ./huge
Signed-off-by: Omar Sandoval <osandov@fb.com>
---
fs/btrfs/ioctl.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9eb0c1eb568e..47c1a72241da 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1069,7 +1069,10 @@ static struct page *defrag_prepare_one_page(struct btrfs_inode *inode,
if (!page)
return ERR_PTR(-ENOMEM);
- ret = set_page_extent_mapped(page);
+ if (PageCompound(page))
+ ret = -ETXTBSY;
+ else
+ ret = set_page_extent_mapped(page);
if (ret < 0) {
unlock_page(page);
put_page(page);
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages
2021-10-13 0:13 [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages Omar Sandoval
@ 2021-10-13 3:41 ` Wang Yugui
2021-10-13 17:20 ` Omar Sandoval
2021-10-13 10:20 ` Filipe Manana
2021-10-15 10:40 ` David Sterba
2 siblings, 1 reply; 6+ messages in thread
From: Wang Yugui @ 2021-10-13 3:41 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-btrfs, kernel-team
Hi,
I failed to use this reproducer to reproduce this problem.
kernel: 5.15.0-0.rc5
kernel config: CONFIG_TRANSPARENT_HUGEPAGE_MADVISE=y
mount option: rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/
btrfs souce: 5.15.0-0.rc5, misc-next
create_thp_file is a loop, could we add
char cmd[256] = {};
sprintf(cmd, "btrfs fi defrag -czstd %s", argv[1]);
(void)!system(cmd);
into it to make it more easy to reproduce ?
Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2021/10/13
> From: Omar Sandoval <osandov@fb.com>
>
> Attempting to defragment a Btrfs file containing a transparent huge page
> immediately deadlocks with the following stack trace:
>
> #0 context_switch (kernel/sched/core.c:4940:2)
> #1 __schedule (kernel/sched/core.c:6287:8)
> #2 schedule (kernel/sched/core.c:6366:3)
> #3 io_schedule (kernel/sched/core.c:8389:2)
> #4 wait_on_page_bit_common (mm/filemap.c:1356:4)
> #5 __lock_page (mm/filemap.c:1648:2)
> #6 lock_page (./include/linux/pagemap.h:625:3)
> #7 pagecache_get_page (mm/filemap.c:1910:4)
> #8 find_or_create_page (./include/linux/pagemap.h:420:9)
> #9 defrag_prepare_one_page (fs/btrfs/ioctl.c:1068:9)
> #10 defrag_one_range (fs/btrfs/ioctl.c:1326:14)
> #11 defrag_one_cluster (fs/btrfs/ioctl.c:1421:9)
> #12 btrfs_defrag_file (fs/btrfs/ioctl.c:1523:9)
> #13 btrfs_ioctl_defrag (fs/btrfs/ioctl.c:3117:9)
> #14 btrfs_ioctl (fs/btrfs/ioctl.c:4872:10)
> #15 vfs_ioctl (fs/ioctl.c:51:10)
> #16 __do_sys_ioctl (fs/ioctl.c:874:11)
> #17 __se_sys_ioctl (fs/ioctl.c:860:1)
> #18 __x64_sys_ioctl (fs/ioctl.c:860:1)
> #19 do_syscall_x64 (arch/x86/entry/common.c:50:14)
> #20 do_syscall_64 (arch/x86/entry/common.c:80:7)
> #21 entry_SYSCALL_64+0x7c/0x15b (arch/x86/entry/entry_64.S:113)
>
> A huge page is represented by a compound page, which consists of a
> struct page for each PAGE_SIZE page within the huge page. The first
> struct page is the "head page", and the remaining are "tail pages".
>
> Defragmentation attempts to lock each page in the range. However,
> lock_page() on a tail page actually locks the corresponding head page.
> So, if defragmentation tries to lock more than one struct page in a
> compound page, it tries to lock the same head page twice and deadlocks
> with itself.
>
> There are a few options for dealing with this:
>
> 1. Return an error if defrag is called for a transparent huge page.
> 2. Silently skip over defragging transparent huge pages.
> 3. Implement real support for defragging transparent huge pages.
>
> This patch implements option 1. The error code (ETXTBUSY) is up for
> bikeshedding. Option 2 seems reasonable as well. Option 3, however,
> would require more effort. THP for filesystems is currently read-only,
> so I suspect that a lot of code is not ready for using these pages for
> I/O.
>
> This can be reproduced with:
>
> $ cat create_thp_file.c
> #include <fcntl.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <stdint.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
>
> static const char zeroes[1024 * 1024];
> static const size_t FILE_SIZE = 2 * 1024 * 1024;
>
> int main(int argc, char **argv)
> {
> if (argc != 2) {
> fprintf(stderr, "usage: %s PATH\n", argv[0]);
> return EXIT_FAILURE;
> }
> int fd = creat(argv[1], 0777);
> if (fd == -1) {
> perror("creat");
> return EXIT_FAILURE;
> }
> size_t written = 0;
> while (written < FILE_SIZE) {
> ssize_t ret = write(fd, zeroes,
> sizeof(zeroes) < FILE_SIZE - written ?
> sizeof(zeroes) : FILE_SIZE - written);
> if (ret < 0) {
> perror("write");
> return EXIT_FAILURE;
> }
> written += ret;
> }
> close(fd);
> fd = open(argv[1], O_RDONLY);
> if (fd == -1) {
> perror("open");
> return EXIT_FAILURE;
> }
>
> /*
> * Reserve some address space so that we can align the file mapping to
> * the huge page size.
> */
> void *placeholder_map = mmap(NULL, FILE_SIZE * 2, PROT_NONE,
> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> if (placeholder_map == MAP_FAILED) {
> perror("mmap (placeholder)");
> return EXIT_FAILURE;
> }
>
> void *aligned_address =
> (void *)(((uintptr_t)placeholder_map + FILE_SIZE - 1) & ~(FILE_SIZE - 1));
>
> void *map = mmap(aligned_address, FILE_SIZE, PROT_READ | PROT_EXEC,
> MAP_SHARED | MAP_FIXED, fd, 0);
> if (map == MAP_FAILED) {
> perror("mmap");
> return EXIT_FAILURE;
> }
> if (madvise(map, FILE_SIZE, MADV_HUGEPAGE) < 0) {
> perror("madvise");
> return EXIT_FAILURE;
> }
>
> char *line = NULL;
> size_t line_capacity = 0;
> FILE *smaps_file = fopen("/proc/self/smaps", "r");
> if (!smaps_file) {
> perror("fopen");
> return EXIT_FAILURE;
> }
> for (;;) {
> for (size_t off = 0; off < FILE_SIZE; off += 4096)
> ((volatile char *)map)[off];
>
> ssize_t ret;
> bool this_mapping = false;
> while ((ret = getline(&line, &line_capacity, smaps_file)) > 0) {
> unsigned long start, end, huge;
> if (sscanf(line, "%lx-%lx", &start, &end) == 2) {
> this_mapping = (start <= (uintptr_t)map &&
> (uintptr_t)map < end);
> } else if (this_mapping &&
> sscanf(line, "FilePmdMapped: %ld", &huge) == 1 &&
> huge > 0) {
> return EXIT_SUCCESS;
> }
> }
>
> sleep(6);
> rewind(smaps_file);
> fflush(smaps_file);
> }
> }
> $ ./create_thp_file huge
> $ btrfs fi defrag -czstd ./huge
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages
2021-10-13 0:13 [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages Omar Sandoval
2021-10-13 3:41 ` Wang Yugui
@ 2021-10-13 10:20 ` Filipe Manana
2021-10-15 10:40 ` David Sterba
2 siblings, 0 replies; 6+ messages in thread
From: Filipe Manana @ 2021-10-13 10:20 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-btrfs, kernel-team
On Wed, Oct 13, 2021 at 1:15 AM Omar Sandoval <osandov@osandov.com> wrote:
>
> From: Omar Sandoval <osandov@fb.com>
>
> Attempting to defragment a Btrfs file containing a transparent huge page
> immediately deadlocks with the following stack trace:
>
> #0 context_switch (kernel/sched/core.c:4940:2)
> #1 __schedule (kernel/sched/core.c:6287:8)
> #2 schedule (kernel/sched/core.c:6366:3)
> #3 io_schedule (kernel/sched/core.c:8389:2)
> #4 wait_on_page_bit_common (mm/filemap.c:1356:4)
> #5 __lock_page (mm/filemap.c:1648:2)
> #6 lock_page (./include/linux/pagemap.h:625:3)
> #7 pagecache_get_page (mm/filemap.c:1910:4)
> #8 find_or_create_page (./include/linux/pagemap.h:420:9)
> #9 defrag_prepare_one_page (fs/btrfs/ioctl.c:1068:9)
> #10 defrag_one_range (fs/btrfs/ioctl.c:1326:14)
> #11 defrag_one_cluster (fs/btrfs/ioctl.c:1421:9)
> #12 btrfs_defrag_file (fs/btrfs/ioctl.c:1523:9)
> #13 btrfs_ioctl_defrag (fs/btrfs/ioctl.c:3117:9)
> #14 btrfs_ioctl (fs/btrfs/ioctl.c:4872:10)
> #15 vfs_ioctl (fs/ioctl.c:51:10)
> #16 __do_sys_ioctl (fs/ioctl.c:874:11)
> #17 __se_sys_ioctl (fs/ioctl.c:860:1)
> #18 __x64_sys_ioctl (fs/ioctl.c:860:1)
> #19 do_syscall_x64 (arch/x86/entry/common.c:50:14)
> #20 do_syscall_64 (arch/x86/entry/common.c:80:7)
> #21 entry_SYSCALL_64+0x7c/0x15b (arch/x86/entry/entry_64.S:113)
>
> A huge page is represented by a compound page, which consists of a
> struct page for each PAGE_SIZE page within the huge page. The first
> struct page is the "head page", and the remaining are "tail pages".
>
> Defragmentation attempts to lock each page in the range. However,
> lock_page() on a tail page actually locks the corresponding head page.
> So, if defragmentation tries to lock more than one struct page in a
> compound page, it tries to lock the same head page twice and deadlocks
> with itself.
>
> There are a few options for dealing with this:
>
> 1. Return an error if defrag is called for a transparent huge page.
> 2. Silently skip over defragging transparent huge pages.
> 3. Implement real support for defragging transparent huge pages.
>
> This patch implements option 1. The error code (ETXTBUSY) is up for
> bikeshedding. Option 2 seems reasonable as well. Option 3, however,
> would require more effort. THP for filesystems is currently read-only,
> so I suspect that a lot of code is not ready for using these pages for
> I/O.
Doing option 1 for now looks reasonable to me.
In the long term, having option 3 would be nice.
Looks good to me, thanks.
Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
> This can be reproduced with:
>
> $ cat create_thp_file.c
> #include <fcntl.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <stdint.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
>
> static const char zeroes[1024 * 1024];
> static const size_t FILE_SIZE = 2 * 1024 * 1024;
>
> int main(int argc, char **argv)
> {
> if (argc != 2) {
> fprintf(stderr, "usage: %s PATH\n", argv[0]);
> return EXIT_FAILURE;
> }
> int fd = creat(argv[1], 0777);
> if (fd == -1) {
> perror("creat");
> return EXIT_FAILURE;
> }
> size_t written = 0;
> while (written < FILE_SIZE) {
> ssize_t ret = write(fd, zeroes,
> sizeof(zeroes) < FILE_SIZE - written ?
> sizeof(zeroes) : FILE_SIZE - written);
> if (ret < 0) {
> perror("write");
> return EXIT_FAILURE;
> }
> written += ret;
> }
> close(fd);
> fd = open(argv[1], O_RDONLY);
> if (fd == -1) {
> perror("open");
> return EXIT_FAILURE;
> }
>
> /*
> * Reserve some address space so that we can align the file mapping to
> * the huge page size.
> */
> void *placeholder_map = mmap(NULL, FILE_SIZE * 2, PROT_NONE,
> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> if (placeholder_map == MAP_FAILED) {
> perror("mmap (placeholder)");
> return EXIT_FAILURE;
> }
>
> void *aligned_address =
> (void *)(((uintptr_t)placeholder_map + FILE_SIZE - 1) & ~(FILE_SIZE - 1));
>
> void *map = mmap(aligned_address, FILE_SIZE, PROT_READ | PROT_EXEC,
> MAP_SHARED | MAP_FIXED, fd, 0);
> if (map == MAP_FAILED) {
> perror("mmap");
> return EXIT_FAILURE;
> }
> if (madvise(map, FILE_SIZE, MADV_HUGEPAGE) < 0) {
> perror("madvise");
> return EXIT_FAILURE;
> }
>
> char *line = NULL;
> size_t line_capacity = 0;
> FILE *smaps_file = fopen("/proc/self/smaps", "r");
> if (!smaps_file) {
> perror("fopen");
> return EXIT_FAILURE;
> }
> for (;;) {
> for (size_t off = 0; off < FILE_SIZE; off += 4096)
> ((volatile char *)map)[off];
>
> ssize_t ret;
> bool this_mapping = false;
> while ((ret = getline(&line, &line_capacity, smaps_file)) > 0) {
> unsigned long start, end, huge;
> if (sscanf(line, "%lx-%lx", &start, &end) == 2) {
> this_mapping = (start <= (uintptr_t)map &&
> (uintptr_t)map < end);
> } else if (this_mapping &&
> sscanf(line, "FilePmdMapped: %ld", &huge) == 1 &&
> huge > 0) {
> return EXIT_SUCCESS;
> }
> }
>
> sleep(6);
> rewind(smaps_file);
> fflush(smaps_file);
> }
> }
> $ ./create_thp_file huge
> $ btrfs fi defrag -czstd ./huge
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> fs/btrfs/ioctl.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 9eb0c1eb568e..47c1a72241da 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1069,7 +1069,10 @@ static struct page *defrag_prepare_one_page(struct btrfs_inode *inode,
> if (!page)
> return ERR_PTR(-ENOMEM);
>
> - ret = set_page_extent_mapped(page);
> + if (PageCompound(page))
> + ret = -ETXTBSY;
> + else
> + ret = set_page_extent_mapped(page);
> if (ret < 0) {
> unlock_page(page);
> put_page(page);
> --
> 2.33.0
>
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages
2021-10-13 3:41 ` Wang Yugui
@ 2021-10-13 17:20 ` Omar Sandoval
2021-10-14 1:49 ` Wang Yugui
0 siblings, 1 reply; 6+ messages in thread
From: Omar Sandoval @ 2021-10-13 17:20 UTC (permalink / raw)
To: Wang Yugui; +Cc: linux-btrfs, kernel-team
On Wed, Oct 13, 2021 at 11:41:06AM +0800, Wang Yugui wrote:
> Hi,
>
> I failed to use this reproducer to reproduce this problem.
>
> kernel: 5.15.0-0.rc5
> kernel config: CONFIG_TRANSPARENT_HUGEPAGE_MADVISE=y
> mount option: rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/
> btrfs souce: 5.15.0-0.rc5, misc-next
You also need CONFIG_READ_ONLY_THP_FOR_FS=y.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages
2021-10-13 17:20 ` Omar Sandoval
@ 2021-10-14 1:49 ` Wang Yugui
0 siblings, 0 replies; 6+ messages in thread
From: Wang Yugui @ 2021-10-14 1:49 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-btrfs, kernel-team
Hi,
> On Wed, Oct 13, 2021 at 11:41:06AM +0800, Wang Yugui wrote:
> > Hi,
> >
> > I failed to use this reproducer to reproduce this problem.
> >
> > kernel: 5.15.0-0.rc5
> > kernel config: CONFIG_TRANSPARENT_HUGEPAGE_MADVISE=y
> > mount option: rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/
> > btrfs souce: 5.15.0-0.rc5, misc-next
>
> You also need CONFIG_READ_ONLY_THP_FOR_FS=y.
Thanks a lot.
This reproducer works well with CONFIG_READ_ONLY_THP_FOR_FS=y.
Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2021/10/14
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages
2021-10-13 0:13 [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages Omar Sandoval
2021-10-13 3:41 ` Wang Yugui
2021-10-13 10:20 ` Filipe Manana
@ 2021-10-15 10:40 ` David Sterba
2 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2021-10-15 10:40 UTC (permalink / raw)
To: Omar Sandoval; +Cc: linux-btrfs, kernel-team
On Tue, Oct 12, 2021 at 05:13:45PM -0700, Omar Sandoval wrote:
> From: Omar Sandoval <osandov@fb.com>
>
> Attempting to defragment a Btrfs file containing a transparent huge page
> immediately deadlocks with the following stack trace:
>
> #0 context_switch (kernel/sched/core.c:4940:2)
> #1 __schedule (kernel/sched/core.c:6287:8)
> #2 schedule (kernel/sched/core.c:6366:3)
> #3 io_schedule (kernel/sched/core.c:8389:2)
> #4 wait_on_page_bit_common (mm/filemap.c:1356:4)
> #5 __lock_page (mm/filemap.c:1648:2)
> #6 lock_page (./include/linux/pagemap.h:625:3)
> #7 pagecache_get_page (mm/filemap.c:1910:4)
> #8 find_or_create_page (./include/linux/pagemap.h:420:9)
> #9 defrag_prepare_one_page (fs/btrfs/ioctl.c:1068:9)
> #10 defrag_one_range (fs/btrfs/ioctl.c:1326:14)
> #11 defrag_one_cluster (fs/btrfs/ioctl.c:1421:9)
> #12 btrfs_defrag_file (fs/btrfs/ioctl.c:1523:9)
> #13 btrfs_ioctl_defrag (fs/btrfs/ioctl.c:3117:9)
> #14 btrfs_ioctl (fs/btrfs/ioctl.c:4872:10)
> #15 vfs_ioctl (fs/ioctl.c:51:10)
> #16 __do_sys_ioctl (fs/ioctl.c:874:11)
> #17 __se_sys_ioctl (fs/ioctl.c:860:1)
> #18 __x64_sys_ioctl (fs/ioctl.c:860:1)
> #19 do_syscall_x64 (arch/x86/entry/common.c:50:14)
> #20 do_syscall_64 (arch/x86/entry/common.c:80:7)
> #21 entry_SYSCALL_64+0x7c/0x15b (arch/x86/entry/entry_64.S:113)
>
> A huge page is represented by a compound page, which consists of a
> struct page for each PAGE_SIZE page within the huge page. The first
> struct page is the "head page", and the remaining are "tail pages".
>
> Defragmentation attempts to lock each page in the range. However,
> lock_page() on a tail page actually locks the corresponding head page.
> So, if defragmentation tries to lock more than one struct page in a
> compound page, it tries to lock the same head page twice and deadlocks
> with itself.
>
> There are a few options for dealing with this:
>
> 1. Return an error if defrag is called for a transparent huge page.
> 2. Silently skip over defragging transparent huge pages.
> 3. Implement real support for defragging transparent huge pages.
>
> This patch implements option 1. The error code (ETXTBUSY) is up for
> bikeshedding. Option 2 seems reasonable as well. Option 3, however,
> would require more effort. THP for filesystems is currently read-only,
> so I suspect that a lot of code is not ready for using these pages for
> I/O.
>
> This can be reproduced with:
>
> $ cat create_thp_file.c
> #include <fcntl.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <stdint.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
>
> static const char zeroes[1024 * 1024];
> static const size_t FILE_SIZE = 2 * 1024 * 1024;
>
> int main(int argc, char **argv)
> {
> if (argc != 2) {
> fprintf(stderr, "usage: %s PATH\n", argv[0]);
> return EXIT_FAILURE;
> }
> int fd = creat(argv[1], 0777);
> if (fd == -1) {
> perror("creat");
> return EXIT_FAILURE;
> }
> size_t written = 0;
> while (written < FILE_SIZE) {
> ssize_t ret = write(fd, zeroes,
> sizeof(zeroes) < FILE_SIZE - written ?
> sizeof(zeroes) : FILE_SIZE - written);
> if (ret < 0) {
> perror("write");
> return EXIT_FAILURE;
> }
> written += ret;
> }
> close(fd);
> fd = open(argv[1], O_RDONLY);
> if (fd == -1) {
> perror("open");
> return EXIT_FAILURE;
> }
>
> /*
> * Reserve some address space so that we can align the file mapping to
> * the huge page size.
> */
> void *placeholder_map = mmap(NULL, FILE_SIZE * 2, PROT_NONE,
> MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> if (placeholder_map == MAP_FAILED) {
> perror("mmap (placeholder)");
> return EXIT_FAILURE;
> }
>
> void *aligned_address =
> (void *)(((uintptr_t)placeholder_map + FILE_SIZE - 1) & ~(FILE_SIZE - 1));
>
> void *map = mmap(aligned_address, FILE_SIZE, PROT_READ | PROT_EXEC,
> MAP_SHARED | MAP_FIXED, fd, 0);
> if (map == MAP_FAILED) {
> perror("mmap");
> return EXIT_FAILURE;
> }
> if (madvise(map, FILE_SIZE, MADV_HUGEPAGE) < 0) {
> perror("madvise");
> return EXIT_FAILURE;
> }
>
> char *line = NULL;
> size_t line_capacity = 0;
> FILE *smaps_file = fopen("/proc/self/smaps", "r");
> if (!smaps_file) {
> perror("fopen");
> return EXIT_FAILURE;
> }
> for (;;) {
> for (size_t off = 0; off < FILE_SIZE; off += 4096)
> ((volatile char *)map)[off];
>
> ssize_t ret;
> bool this_mapping = false;
> while ((ret = getline(&line, &line_capacity, smaps_file)) > 0) {
> unsigned long start, end, huge;
> if (sscanf(line, "%lx-%lx", &start, &end) == 2) {
> this_mapping = (start <= (uintptr_t)map &&
> (uintptr_t)map < end);
> } else if (this_mapping &&
> sscanf(line, "FilePmdMapped: %ld", &huge) == 1 &&
> huge > 0) {
> return EXIT_SUCCESS;
> }
> }
>
> sleep(6);
> rewind(smaps_file);
> fflush(smaps_file);
> }
> }
> $ ./create_thp_file huge
> $ btrfs fi defrag -czstd ./huge
>
> Signed-off-by: Omar Sandoval <osandov@fb.com>
> ---
> fs/btrfs/ioctl.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 9eb0c1eb568e..47c1a72241da 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1069,7 +1069,10 @@ static struct page *defrag_prepare_one_page(struct btrfs_inode *inode,
> if (!page)
> return ERR_PTR(-ENOMEM);
>
> - ret = set_page_extent_mapped(page);
> + if (PageCompound(page))
> + ret = -ETXTBSY;
Returning here looks as an acceptable option, the error code is could be
misleading but I think we can reuse it. Defragmenting running binaries
also returns (or returned) ETXTBUSY so the meaning is "something's
happening with the pages, we can't defrag but it's temporary".
The defrag command skips errors by default so it's up to the user to
decide what to do, but it's not a hard error and failing defragmentation
is also not a hard error.
What I do not like is the bare call to PageCompound. At minimum it must
be documented because it's not obvious at all why it's here. I'd rather
see it wrapped in a helper (like set_page_extent_mapped_careful) with a
comment when to use it.
The defragmentation accesses the pages in a way that may expose the
compound pages and related problems but it may not be the only place
(now or in the future) so fixing any potential problems would be to use
the helper, not to add more PageCompound checks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-10-15 10:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 0:13 [RFC PATCH] btrfs: fix deadlock when defragging transparent huge pages Omar Sandoval
2021-10-13 3:41 ` Wang Yugui
2021-10-13 17:20 ` Omar Sandoval
2021-10-14 1:49 ` Wang Yugui
2021-10-13 10:20 ` Filipe Manana
2021-10-15 10:40 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.