* [PATCH -next] md: wake up mdmon after setting badblocks @ 2024-08-01 12:51 Yu Kuai 2024-08-05 9:40 ` Mariusz Tkaczyk 0 siblings, 1 reply; 3+ messages in thread From: Yu Kuai @ 2024-08-01 12:51 UTC (permalink / raw) To: song; +Cc: linux-raid, linux-kernel, yukuai3, yukuai1, yi.zhang, yangerkun From: Yu Kuai <yukuai3@huawei.com> For external super_block, mdmon rely on "mddev->sysfs_state" to update super_block. However, rdev_set_badblocks() will set sb_flags without waking up mdmon, which might cauing IO hang due to suepr_block can't be updated. This problem is found by code review. Signed-off-by: Yu Kuai <yukuai3@huawei.com> --- drivers/md/md.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index 23cc77d51676..06d6ee8cd543 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -9831,10 +9831,12 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t s, int sectors, /* Make sure they get written out promptly */ if (test_bit(ExternalBbl, &rdev->flags)) sysfs_notify_dirent_safe(rdev->sysfs_unack_badblocks); - sysfs_notify_dirent_safe(rdev->sysfs_state); set_mask_bits(&mddev->sb_flags, 0, BIT(MD_SB_CHANGE_CLEAN) | BIT(MD_SB_CHANGE_PENDING)); md_wakeup_thread(rdev->mddev->thread); + + sysfs_notify_dirent_safe(rdev->sysfs_state); + sysfs_notify_dirent_safe(mddev->sysfs_state); return 1; } else return 0; -- 2.39.2 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH -next] md: wake up mdmon after setting badblocks 2024-08-01 12:51 [PATCH -next] md: wake up mdmon after setting badblocks Yu Kuai @ 2024-08-05 9:40 ` Mariusz Tkaczyk 2024-08-05 11:08 ` Yu Kuai 0 siblings, 1 reply; 3+ messages in thread From: Mariusz Tkaczyk @ 2024-08-05 9:40 UTC (permalink / raw) To: Yu Kuai; +Cc: song, linux-raid, linux-kernel, yukuai3, yi.zhang, yangerkun On Thu, 1 Aug 2024 20:51:48 +0800 Yu Kuai <yukuai1@huaweicloud.com> wrote: > From: Yu Kuai <yukuai3@huawei.com> > > For external super_block, mdmon rely on "mddev->sysfs_state" to update > super_block. However, rdev_set_badblocks() will set sb_flags without > waking up mdmon, which might cauing IO hang due to suepr_block can't be > updated. > > This problem is found by code review. > > Signed-off-by: Yu Kuai <yukuai3@huawei.com> > --- > drivers/md/md.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 23cc77d51676..06d6ee8cd543 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -9831,10 +9831,12 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t > s, int sectors, /* Make sure they get written out promptly */ > if (test_bit(ExternalBbl, &rdev->flags)) > sysfs_notify_dirent_safe(rdev->sysfs_unack_badblocks); > - sysfs_notify_dirent_safe(rdev->sysfs_state); > set_mask_bits(&mddev->sb_flags, 0, > BIT(MD_SB_CHANGE_CLEAN) | > BIT(MD_SB_CHANGE_PENDING)); md_wakeup_thread(rdev->mddev->thread); > + > + sysfs_notify_dirent_safe(rdev->sysfs_state); > + sysfs_notify_dirent_safe(mddev->sysfs_state); > return 1; > } else > return 0; Hey Kuai, Later, I realized that mdmon is attached to various fds: Here a mdmon code: add_fd(&rfds, &maxfd, a->info.state_fd); #mddev->sysfs_state add_fd(&rfds, &maxfd, a->action_fd); #mddev->sysfs_action add_fd(&rfds, &maxfd, a->sync_completed_fd); #mddev->resync_position for (mdi = a->info.devs ; mdi ; mdi = mdi->next) { #for each rdev add_fd(&rfds, &maxfd, mdi->state_fd); #rdev->sysfs_state add_fd(&rfds, &maxfd, mdi->bb_fd); #rdev->sysfs_badblocks add_fd(&rfds, &maxfd, mdi->ubb_fd); #rdev->sysfs_unack_badblocks } Notification in rdev->sysfs_state is fine. The problem was somewhere else (mdmon blocked on "rdev remove"). And It will be fixed by moving rdev remove to managemon because it is blocking action now. https://github.com/md-raid-utilities/mdadm/pull/66 I think it is fine to move this down after set_mask_bits, but we don't have to repeat notification to mddev->state. Thanks, Mariusz ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH -next] md: wake up mdmon after setting badblocks 2024-08-05 9:40 ` Mariusz Tkaczyk @ 2024-08-05 11:08 ` Yu Kuai 0 siblings, 0 replies; 3+ messages in thread From: Yu Kuai @ 2024-08-05 11:08 UTC (permalink / raw) To: Mariusz Tkaczyk, Yu Kuai Cc: song, linux-raid, linux-kernel, yi.zhang, yangerkun, yukuai (C) Hi, 在 2024/08/05 17:40, Mariusz Tkaczyk 写道: > On Thu, 1 Aug 2024 20:51:48 +0800 > Yu Kuai <yukuai1@huaweicloud.com> wrote: > >> From: Yu Kuai <yukuai3@huawei.com> >> >> For external super_block, mdmon rely on "mddev->sysfs_state" to update >> super_block. However, rdev_set_badblocks() will set sb_flags without >> waking up mdmon, which might cauing IO hang due to suepr_block can't be >> updated. >> >> This problem is found by code review. >> >> Signed-off-by: Yu Kuai <yukuai3@huawei.com> >> --- >> drivers/md/md.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/md/md.c b/drivers/md/md.c >> index 23cc77d51676..06d6ee8cd543 100644 >> --- a/drivers/md/md.c >> +++ b/drivers/md/md.c >> @@ -9831,10 +9831,12 @@ int rdev_set_badblocks(struct md_rdev *rdev, sector_t >> s, int sectors, /* Make sure they get written out promptly */ >> if (test_bit(ExternalBbl, &rdev->flags)) >> sysfs_notify_dirent_safe(rdev->sysfs_unack_badblocks); >> - sysfs_notify_dirent_safe(rdev->sysfs_state); >> set_mask_bits(&mddev->sb_flags, 0, >> BIT(MD_SB_CHANGE_CLEAN) | >> BIT(MD_SB_CHANGE_PENDING)); md_wakeup_thread(rdev->mddev->thread); >> + >> + sysfs_notify_dirent_safe(rdev->sysfs_state); >> + sysfs_notify_dirent_safe(mddev->sysfs_state); >> return 1; >> } else >> return 0; > > > Hey Kuai, > Later, I realized that mdmon is attached to various fds: > > Here a mdmon code: > > add_fd(&rfds, &maxfd, a->info.state_fd); #mddev->sysfs_state > add_fd(&rfds, &maxfd, a->action_fd); #mddev->sysfs_action > add_fd(&rfds, &maxfd, a->sync_completed_fd); #mddev->resync_position > > > for (mdi = a->info.devs ; mdi ; mdi = mdi->next) { #for each rdev > add_fd(&rfds, &maxfd, mdi->state_fd); #rdev->sysfs_state > add_fd(&rfds, &maxfd, mdi->bb_fd); #rdev->sysfs_badblocks > add_fd(&rfds, &maxfd, mdi->ubb_fd); > #rdev->sysfs_unack_badblocks } Ok, so I was wrong that mdmon rely on "mddev->sysfs_state". > > Notification in rdev->sysfs_state is fine. The problem was somewhere else > (mdmon blocked on "rdev remove"). And It will be fixed by moving rdev remove to > managemon because it is blocking action now. > > https://github.com/md-raid-utilities/mdadm/pull/66 > > I think it is fine to move this down after set_mask_bits, but we don't > have to repeat notification to mddev->state. Yes, I'll send a v2 patch to move down "rdev->sysfs_state". Thanks, Kuai > > Thanks, > Mariusz > > . > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-05 11:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-08-01 12:51 [PATCH -next] md: wake up mdmon after setting badblocks Yu Kuai 2024-08-05 9:40 ` Mariusz Tkaczyk 2024-08-05 11:08 ` Yu Kuai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox