* [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation @ 2023-03-24 3:16 xiaoshoukui 2023-03-27 23:05 ` David Sterba 0 siblings, 1 reply; 7+ messages in thread From: xiaoshoukui @ 2023-03-24 3:16 UTC (permalink / raw) To: clm, josef, dsterba; +Cc: linux-btrfs, linux-kernel, xiaoshoukui, xiaoshoukui with fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD enter btrfs_ioctl_add_dev function , exclusive_operation will be classified as in paused balance operation. After return from btrfs_ioctl_add_dev, exclusive_operation will be restore to BTRFS_EXCLOP_BALANCE_PAUSED which is not its original state. Signed-off-by: xiaoshoukui <xiaoshoukui@ruijie.com.cn> --- fs/btrfs/ioctl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index a0ef1a1784c7..aab5fdb9445c 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2629,7 +2629,7 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg) } if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_DEV_ADD)) { - if (!btrfs_exclop_start_try_lock(fs_info, BTRFS_EXCLOP_DEV_ADD)) + if (fs_info->exclusive_operation != BTRFS_EXCLOP_BALANCE_PAUSED) return BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS; /* @@ -2637,8 +2637,9 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg) * change the exclusive op type and remember we should bring * back the paused balance */ + spin_lock(&fs_info->super_lock); fs_info->exclusive_operation = BTRFS_EXCLOP_DEV_ADD; - btrfs_exclop_start_unlock(fs_info); + spin_unlock(&fs_info->super_lock); restore_op = true; } -- 2.20.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation 2023-03-24 3:16 [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation xiaoshoukui @ 2023-03-27 23:05 ` David Sterba 0 siblings, 0 replies; 7+ messages in thread From: David Sterba @ 2023-03-27 23:05 UTC (permalink / raw) To: xiaoshoukui; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui On Thu, Mar 23, 2023 at 11:16:11PM -0400, xiaoshoukui wrote: > with fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD enter > btrfs_ioctl_add_dev function , exclusive_operation will be classified > as in paused balance operation. After return from btrfs_ioctl_add_dev, > exclusive_operation will be restore to BTRFS_EXCLOP_BALANCE_PAUSED which > is not its original state. Sorry, I don't understand what you mean. The paused balance and 'device add' are supposed to be compatible exclusive operations (see commit a174c0a2e857 ("btrfs: allow device add if balance is paused")). Have you found some bug with the above or is there other combination of the exclusive operations that should not work? The changes to the state values are the same, besides the wrong locking. > Signed-off-by: xiaoshoukui <xiaoshoukui@ruijie.com.cn> > --- > fs/btrfs/ioctl.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index a0ef1a1784c7..aab5fdb9445c 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -2629,7 +2629,7 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg) > } > > if (!btrfs_exclop_start(fs_info, BTRFS_EXCLOP_DEV_ADD)) { > - if (!btrfs_exclop_start_try_lock(fs_info, BTRFS_EXCLOP_DEV_ADD)) > + if (fs_info->exclusive_operation != BTRFS_EXCLOP_BALANCE_PAUSED) This is removing the atomicity of the check so it's racy and could forcibly overwrite the exclusive operation to BTRFS_EXCLOP_DEV_ADD without the protecting the whole critical section. > return BTRFS_ERROR_DEV_EXCL_RUN_IN_PROGRESS; > > /* > @@ -2637,8 +2637,9 @@ static long btrfs_ioctl_add_dev(struct btrfs_fs_info *fs_info, void __user *arg) > * change the exclusive op type and remember we should bring > * back the paused balance > */ > + spin_lock(&fs_info->super_lock); What if there's another exclusive operation started before this lock is taken? > fs_info->exclusive_operation = BTRFS_EXCLOP_DEV_ADD; > - btrfs_exclop_start_unlock(fs_info); > + spin_unlock(&fs_info->super_lock); > restore_op = true; > } ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20230327230553.GJ10580 () twin ! jikos ! cz>]
* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation [not found] <20230327230553.GJ10580 () twin ! jikos ! cz> @ 2023-03-28 9:43 ` xiaoshoukui 2023-03-31 17:45 ` David Sterba 0 siblings, 1 reply; 7+ messages in thread From: xiaoshoukui @ 2023-03-28 9:43 UTC (permalink / raw) To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui > Have you found some bug with the above or is there other combination of > the exclusive operations that should not work? The changes to the state > values are the same, besides the wrong locking. Yes, there is a racy bewteen btrfs_exclop_balance and btrfs_exclop_finish in btrfs_ioctl_add_dev, when cocurrently adding multiple devices to the same mnt point. That will cause the assertion in btrfs_exclop_balance to fail. > void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, > enum btrfs_exclusive_operation op) > { > switch (op) { > case BTRFS_EXCLOP_BALANCE_PAUSED: > spin_lock(&fs_info->super_lock); > ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || > fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD); when btrfs_exclop_finish function was executed before the ASSERT, the fs_info->exclusive_operation will change to BTRFS_EXCLOP_NONE. So this assert will failed. We can easily reproduce it with the following code. #include <stdio.h> #include <stdlib.h> #include <fcntl.h> #include <unistd.h> #include <sys/ioctl.h> #include <linux/btrfs.h> #include <pthread.h> #include <errno.h> #include <dirent.h> #include <string.h> static int fd; static int i; static int controller = 0; static int random_delay1 = 0; static int random_delay2 = 0; //const static int delta = 10000; static pthread_t thread_id[10]; static char *path = "/dev/vda"; static char *btrfsmntpt = "/mnt/my_btrfs"; void prepare_random() { random_delay1 = rand() % 80; random_delay2 = rand() % 20; } void *thr1(void *arg) { int ret; while(!controller); //usleep(delta); usleep(random_delay1); ret = ioctl(fd, BTRFS_IOC_ADD_DEV, 0); if (ret != 0) { printf("Failed to add device %s, errno %d\n", path, errno); return NULL; } printf("Device %s added successfully\n", path); return NULL; } void *thr2(void *arg) { int ret; while(!controller); usleep(random_delay2); ret = ioctl(fd, BTRFS_IOC_ADD_DEV, 0); if (ret != 0) { printf("Failed to add device %s, errno %d\n", path, errno); return NULL; } printf("Device %s added successfully\n", path); return NULL; } int main(int argc, char **argv) { DIR * dir; pthread_t th1, th2; srand(time(NULL)); while(1) { controller = 0; //prepare random delay prepare_random(); dir = opendir(btrfsmntpt); if (!dir) { printf("Failed to open btrfs mount point %s, errno %d\n", btrfsmntpt, errno); return 1; } fd = dirfd(dir); if (fd < 0) { printf("Failed to get directory stream file descriptor, errno %d\n", errno); return 1; } pthread_create(&th2, NULL, thr2, NULL); pthread_create(&th1, NULL, thr1, NULL); // pthread_create(&th2, NULL, closing_thread, &test); controller = 1; // start pthread_join(th1, NULL); pthread_join(th2, NULL); closedir(dir); close(fd); } return 0; } The procedure to reproduce the problem: 1、mount a device to /mnt/my_btrfs; 2、excute the above compiled repro; The log of my test: root@syzkaller:/home/xsk# mount btrfs.img /mnt/my_btrfs/ root@syzkaller:/home/xsk# mount /dev/sda on / type ext4 (rw,relatime) .... /home/xsk/btrfs.img on /mnt/my_btrfs type btrfs (rw,relatime,discard=async,space_cache,subvolid=5,subvol=/) root@syzkaller:/home/xsk# ./repro Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add de[ 69.838916][ T8108] assertion failed: fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD, in fs/btrfs/ioctl.c:456 vice /dev/vd[a , e69.840759][ T8108] invalid opcode: 0000 [#1] PREEMPT SMP KASAN 9rn[o 1 46 .841461][ T8108] CPU: 0 PID: 8108 Comm: repro Not tainted 6.2.0 #5 [ 69.842245][ T8108] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 [ 69.843292][ T8108] RIP: 0010:btrfs_assertfail+0x2c/0x2e [ 69.843899][ T8108] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 20 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6 [ 69.845934][ T8108] RSP: 0018:ffffc90005f1fe28 EFLAGS: 00010282 [ 69.846588][ T8108] RAX: 0000000000000097 RBX: 0000000000000000 RCX: 0000000000000000 [ 69.847426][ T8108] RDX: ffff88801e8b2440 RSI: ffffffff81637e7c RDI: fffff52000be3fb7 [ 69.848267][ T8108] RBP: ffffffff8a533d80 R08: 0000000000000097 R09: 0000000000000000 [ 69.849103][ T8108] R10: 0000000080000001 R11: 0000000000000001 R12: ffffffff8a533d40 [ 69.849945][ T8108] R13: 00000000000001c8 R14: 000000005000940a R15: 0000000000000000 [ 69.850792][ T8108] FS: 00007ff3e2004640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000 [ 69.851747][ T8108] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 69.852457][ T8108] CR2: 0000000000000000 CR3: 00000000182c8000 CR4: 0000000000750ef0 [ 69.853311][ T8108] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 69.854167][ T8108] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 69.854935][ T8108] PKRU: 55555554 [ 69.855263][ T8108] Call Trace: [ 69.855575][ T8108] <TASK> [ 69.855851][ T8108] btrfs_exclop_balance+0x13c/0x310 [ 69.856337][ T8108] ? memdup_user+0xab/0xc0 [ 69.856749][ T8108] ? PTR_ERR+0x17/0x20 [ 69.857138][ T8108] btrfs_ioctl_add_dev+0x2ee/0x320 [ 69.857716][ T8108] btrfs_ioctl+0x9d5/0x10d0 [ 69.858222][ T8108] ? btrfs_ioctl_encoded_write+0xb80/0xb80 [ 69.858801][ T8108] __x64_sys_ioctl+0x197/0x210 [ 69.859253][ T8108] do_syscall_64+0x3c/0xb0 [ 69.859668][ T8108] entry_SYSCALL_64_after_hwframe+0x63/0xcd [ 69.860217][ T8108] RIP: 0033:0x4546af [ 69.860582][ T8108] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00 [ 69.862533][ T8108] RSP: 002b:00007ff3e2004150 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 69.863299][ T8108] RAX: ffffffffffffffda RBX: 00007ff3e2004640 RCX: 00000000004546af [ 69.864025][ T8108] RDX: 0000000000000000 RSI: 000000005000940a RDI: 0000000000000003 [ 69.864739][ T8108] RBP: 00007ff3e20041d0 R08: 0000000000000000 R09: 00007ffd746cf81f [ 69.865528][ T8108] R10: 0000000000000000 R11: 0000000000000246 R12: 00007ff3e2004640 [ 69.866375][ T8108] R13: 0000000000000009 R14: 000000000041b320 R15: 00007ff3e1804000 [ 69.867115][ T8108] </TASK> [ 69.867411][ T8108] Modules linked in: [ 69.867888][ T8108] ---[ end trace 0000000000000000 ]--- [ 69.868392][ T8108] RIP: 0010:btrfs_assertfail+0x2c/0x2e [ 69.868901][ T8108] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 20 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6 [ 69.870857][ T8108] RSP: 0018:ffffc90005f1fe28 EFLAGS: 00010282 [ 69.871422][ T8108] RAX: 0000000000000097 RBX: 0000000000000000 RCX: 0000000000000000 [ 69.872148][ T8108] RDX: ffff88801e8b2440 RSI: ffffffff81637e7c RDI: fffff52000be3fb7 [ 69.872866][ T8108] RBP: ffffffff8a533d80 R08: 0000000000000097 R09: 0000000000000000 [ 69.873681][ T8108] R10: 0000000080000001 R11: 0000000000000001 R12: ffffffff8a533d40 [ 69.874530][ T8108] R13: 00000000000001c8 R14: 000000005000940a R15: 0000000000000000 [ 69.875270][ T8108] FS: 00007ff3e2004640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000 [ 69.876084][ T8108] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 69.876681][ T8108] CR2: 0000000000000000 CR3: 00000000182c8000 CR4: 0000000000750ef0 [ 69.877464][ T8108] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 69.878317][ T8108] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 69.879062][ T8108] PKRU: 55555554 [ 69.879400][ T8108] Kernel panic - not syncing: Fatal exception [ 69.880096][ T8108] Kernel Offset: disabled [ 69.880563][ T8108] Rebooting in 86400 seconds.. Please review whether we should patch the assert to add BTRFS_EXCLOP_NONE condtion. I'll post a patch if needed. thx. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation 2023-03-28 9:43 ` xiaoshoukui @ 2023-03-31 17:45 ` David Sterba 0 siblings, 0 replies; 7+ messages in thread From: David Sterba @ 2023-03-31 17:45 UTC (permalink / raw) To: xiaoshoukui Cc: dsterba, clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui On Tue, Mar 28, 2023 at 05:43:35AM -0400, xiaoshoukui wrote: > > Have you found some bug with the above or is there other combination of > > the exclusive operations that should not work? The changes to the state > > values are the same, besides the wrong locking. > > Yes, there is a racy bewteen btrfs_exclop_balance and btrfs_exclop_finish > in btrfs_ioctl_add_dev, when cocurrently adding multiple devices to the > same mnt point. That will cause the assertion in btrfs_exclop_balance to fail. > > > void btrfs_exclop_balance(struct btrfs_fs_info *fs_info, > > enum btrfs_exclusive_operation op) > > { > > switch (op) { > > case BTRFS_EXCLOP_BALANCE_PAUSED: > > spin_lock(&fs_info->super_lock); > > ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || > > fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD); > > when btrfs_exclop_finish function was executed before the ASSERT, the > fs_info->exclusive_operation will change to BTRFS_EXCLOP_NONE. So this > assert will failed. > > Please review whether we should patch the assert to add BTRFS_EXCLOP_NONE condtion. > I'll post a patch if needed. thx. Yeah I think the assertion should also check for NONE status. The paused balance makes the state tracking harder but in user-started (manual or scripted) commands it's typically not racing. btrfs_exclop_start_try_lock does not allow to do the change from none -> op mandating an explicit btrfs_exclop_start first but the assertions do not care about that. ^ permalink raw reply [flat|nested] 7+ messages in thread
[parent not found: <20230331174533.GZ10580 () twin ! jikos ! cz>]
* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation [not found] <20230331174533.GZ10580 () twin ! jikos ! cz> @ 2023-04-03 9:37 ` xiaoshoukui 2023-04-04 19:10 ` David Sterba 0 siblings, 1 reply; 7+ messages in thread From: xiaoshoukui @ 2023-04-03 9:37 UTC (permalink / raw) To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui > Yeah I think the assertion should also check for NONE status. The paused > balance makes the state tracking harder but in user-started (manual or > scripted) commands it's typically not racing. An assertion failure means that the code may not have taken careful consideration. After I patched the BTRFS_EXCLOP_NONE to the assertion, regression tests shows that another scenario I missed. With started state == BTRFS_EXCLOP_BALANCE_PAUSED, cocurrently adding multiple devices to the same mount point and btrfs_exclop_balance executed finish before the latter thread execute assertion in btrfs_exclop_balance, exclusive_operation will changed to BTRFS_EXCLOP_BALANCE_PAUSED state. I also added btrfs_info before ASSERT to help troubleshooting: > btrfs_info(fs_info, "fs_info exclusive_operation: %d", > fs_info->exclusive_operation); > ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || > fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD || > fs_info->exclusive_operation == BTRFS_EXCLOP_NONE); Regression test log as follow: The enum value of 1 correspond to BTRFS_EXCLOP_BALANCE_PAUSED: root@syzkaller:/home/xsk# ./repro Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 Failed to add de[ 416.611428][ T7970] BTRFS info (device loop0): fs_info exclusive_operation: 0 vice /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 [ 416.613973][ T7971] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, errno 14 [ 416.615456][ T7972] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, errno 14 [ 416.617528][ T7973] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add de[ 416.618359][ T7974] BTRFS info (device loop0): fs_info exclusive_operation: 3 vice /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 [ 416.622589][ T7975] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, e[ 416.624034][ T7976] BTRFS info (device loop0): fs_info exclusive_operation: 3 rrno 14 Failed to add device /dev/vda, errno 14 [ 416.626420][ T7977] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, errno 14 [ 416.627643][ T7978] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, errno 14 [ 416.629006][ T7979] BTRFS info (device loop0): fs_info exclusive_operation: 3 [ 416.630298][ T7980] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, errno 14 Failed to add device /dev/vda, errno 14 [ 416.632787][ T7981] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, errno 14 [ 416.634282][ T7982] BTRFS info (device loop0): fs_info exclusive_operation: 3 Failed to add device /dev/vda, errno 14 [ 416.636202][ T7983] BTRFS info (device loop0): fs_info exclusive_operation: 3 [ 416.637012][ T7984] BTRFS info (device loop0): fs_info exclusive_operation: 1 Failed to add de[ 416.637759][ T7984] assertion failed: fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD || fs_info->exclusive_operation == BTRFS_EXCLOP_NONE, in fs/btrfs/ioctl.c:458 vice /dev/vda, [e 416.639845][ T7984] invalid opcode: 0000 [#1] PREEMPT SMP KASAN rrno [1 4 416.640485][ T7984] CPU: 0 PID: 7984 Comm: repro Not tainted 6.2.0 #7 [ 416.641172][ T7984] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 [ 416.642090][ T7984] RIP: 0010:btrfs_assertfail+0x2c/0x2e [ 416.642629][ T7984] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 c0 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6 [ 416.644423][ T7984] RSP: 0018:ffffc90003ea7e28 EFLAGS: 00010282 [ 416.645018][ T7984] RAX: 00000000000000cc RBX: 0000000000000000 RCX: 0000000000000000 [ 416.645763][ T7984] RDX: ffff88801d030000 RSI: ffffffff81637e7c RDI: fffff520007d4fb7 [ 416.646554][ T7984] RBP: ffffffff8a533de0 R08: 00000000000000cc R09: 0000000000000000 [ 416.647299][ T7984] R10: 0000000000000001 R11: 0000000000000001 R12: ffffffff8a533da0 [ 416.648041][ T7984] R13: 00000000000001ca R14: 000000005000940a R15: 0000000000000000 [ 416.648785][ T7984] FS: 00007fa2985d4640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000 [ 416.649616][ T7984] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 416.650238][ T7984] CR2: 0000000000000000 CR3: 0000000018e5e000 CR4: 0000000000750ef0 [ 416.650980][ T7984] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 416.651725][ T7984] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 416.652502][ T7984] PKRU: 55555554 [ 416.652888][ T7984] Call Trace: [ 416.653241][ T7984] <TASK> [ 416.653527][ T7984] btrfs_exclop_balance+0x240/0x410 [ 416.654036][ T7984] ? memdup_user+0xab/0xc0 [ 416.654465][ T7984] ? PTR_ERR+0x17/0x20 [ 416.654874][ T7984] btrfs_ioctl_add_dev+0x2ee/0x320 [ 416.655380][ T7984] btrfs_ioctl+0x9d5/0x10d0 [ 416.655822][ T7984] ? btrfs_ioctl_encoded_write+0xb80/0xb80 [ 416.656400][ T7984] __x64_sys_ioctl+0x197/0x210 [ 416.656874][ T7984] do_syscall_64+0x3c/0xb0 [ 416.657346][ T7984] entry_SYSCALL_64_after_hwframe+0x63/0xcd [ 416.657922][ T7984] RIP: 0033:0x4546af [ 416.658304][ T7984] Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <41> 89 c0 3d 00 f0 ff ff 77 1f 48 8b 44 24 18 64 48 2b 04 25 28 00 [ 416.660170][ T7984] RSP: 002b:00007fa2985d4150 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 [ 416.660972][ T7984] RAX: ffffffffffffffda RBX: 00007fa2985d4640 RCX: 00000000004546af [ 416.661714][ T7984] RDX: 0000000000000000 RSI: 000000005000940a RDI: 0000000000000003 [ 416.662449][ T7984] RBP: 00007fa2985d41d0 R08: 0000000000000000 R09: 00007ffee37a4c4f [ 416.663195][ T7984] R10: 0000000000000000 R11: 0000000000000246 R12: 00007fa2985d4640 [ 416.663951][ T7984] R13: 0000000000000009 R14: 000000000041b320 R15: 00007fa297dd4000 [ 416.664703][ T7984] </TASK> [ 416.665040][ T7984] Modules linked in: [ 416.665590][ T7984] ---[ end trace 0000000000000000 ]--- [ 416.666176][ T7984] RIP: 0010:btrfs_assertfail+0x2c/0x2e [ 416.666774][ T7984] Code: 1f 00 41 55 41 89 d5 41 54 49 89 f4 55 48 89 fd e8 b9 84 e8 f7 44 89 e9 4c 89 e2 48 89 ee 48 c7 c7 c0 af 54 8a e8 cb 49 f3 ff <0f> 0b 66 0f 1f 00 55 48 89 fd e8 95 84 e8 f7 48 89 ef 5d 48 c7 c6 [ 416.668775][ T7984] RSP: 0018:ffffc90003ea7e28 EFLAGS: 00010282 [ 416.669425][ T7984] RAX: 00000000000000cc RBX: 0000000000000000 RCX: 0000000000000000 [ 416.670235][ T7984] RDX: ffff88801d030000 RSI: ffffffff81637e7c RDI: fffff520007d4fb7 [ 416.671050][ T7984] RBP: ffffffff8a533de0 R08: 00000000000000cc R09: 0000000000000000 [ 416.671867][ T7984] R10: 0000000000000001 R11: 0000000000000001 R12: ffffffff8a533da0 [ 416.672685][ T7984] R13: 00000000000001ca R14: 000000005000940a R15: 0000000000000000 [ 416.673501][ T7984] FS: 00007fa2985d4640(0000) GS:ffff88802cc00000(0000) knlGS:0000000000000000 [ 416.674425][ T7984] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 416.675114][ T7984] CR2: 0000000000000000 CR3: 0000000018e5e000 CR4: 0000000000750ef0 [ 416.675933][ T7984] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 416.676760][ T7984] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 416.677544][ T7984] PKRU: 55555554 [ 416.677885][ T7984] Kernel panic - not syncing: Fatal exception [ 416.678592][ T7984] Kernel Offset: disabled [ 416.679008][ T7984] Rebooting in 86400 seconds.. I think the assertion should also check for BTRFS_EXCLOP_BALANCE_PAUSED status. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation 2023-04-03 9:37 ` xiaoshoukui @ 2023-04-04 19:10 ` David Sterba 2023-04-06 6:58 ` xiaoshoukui 0 siblings, 1 reply; 7+ messages in thread From: David Sterba @ 2023-04-04 19:10 UTC (permalink / raw) To: xiaoshoukui; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui On Mon, Apr 03, 2023 at 05:37:57AM -0400, xiaoshoukui wrote: > > Yeah I think the assertion should also check for NONE status. The paused > > balance makes the state tracking harder but in user-started (manual or > > scripted) commands it's typically not racing. > > An assertion failure means that the code may not have taken careful consideration. > After I patched the BTRFS_EXCLOP_NONE to the assertion, regression tests shows that > another scenario I missed. > > With started state == BTRFS_EXCLOP_BALANCE_PAUSED, cocurrently adding multiple devices > to the same mount point and btrfs_exclop_balance executed finish before the latter > thread execute assertion in btrfs_exclop_balance, exclusive_operation will changed to > BTRFS_EXCLOP_BALANCE_PAUSED state. > > I also added btrfs_info before ASSERT to help troubleshooting: > > btrfs_info(fs_info, "fs_info exclusive_operation: %d", > > fs_info->exclusive_operation); > > ASSERT(fs_info->exclusive_operation == BTRFS_EXCLOP_BALANCE || > > fs_info->exclusive_operation == BTRFS_EXCLOP_DEV_ADD || > > fs_info->exclusive_operation == BTRFS_EXCLOP_NONE); > I think the assertion should also check for BTRFS_EXCLOP_BALANCE_PAUSED status. Agreed and this looks like the complete set of the compatible operations. I hope this does not enable some combination that should not be allowed but the enabling side does the try lock and allows only the paused + dev_add combination. Please send a fix with the analysis you dit and add the relevant parts of stack traces. The reproducer would be good to have in fstests, for the changelog please describe the conditions that could trigger the assertion, the reproducer itself is too long. Thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation 2023-04-04 19:10 ` David Sterba @ 2023-04-06 6:58 ` xiaoshoukui 0 siblings, 0 replies; 7+ messages in thread From: xiaoshoukui @ 2023-04-06 6:58 UTC (permalink / raw) To: dsterba; +Cc: clm, josef, dsterba, linux-btrfs, linux-kernel, xiaoshoukui > Please send a fix with the analysis you dit and add the relevant parts > of stack traces. The reproducer would be good to have in fstests, for > the changelog please describe the conditions that could trigger the > assertion, the reproducer itself is too long. Thanks. Already posted the fix patch in a new thread. Thanks for your advice on reproducer program. It will take some time to refactor it to fstests testcase. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-04-06 6:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-24 3:16 [PATCH] btrfs: ioctl: fix inaccurate determination of exclusive_operation xiaoshoukui
2023-03-27 23:05 ` David Sterba
[not found] <20230327230553.GJ10580 () twin ! jikos ! cz>
2023-03-28 9:43 ` xiaoshoukui
2023-03-31 17:45 ` David Sterba
[not found] <20230331174533.GZ10580 () twin ! jikos ! cz>
2023-04-03 9:37 ` xiaoshoukui
2023-04-04 19:10 ` David Sterba
2023-04-06 6:58 ` xiaoshoukui
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox