lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240805114024.00007877@linux.intel.com>
Date: Mon, 5 Aug 2024 11:40:24 +0200
From: Mariusz Tkaczyk <mariusz.tkaczyk@...ux.intel.com>
To: Yu Kuai <yukuai1@...weicloud.com>
Cc: song@...nel.org, linux-raid@...r.kernel.org,
 linux-kernel@...r.kernel.org, yukuai3@...wei.com, yi.zhang@...wei.com,
 yangerkun@...wei.com
Subject: Re: [PATCH -next] md: wake up mdmon after setting badblocks

On Thu,  1 Aug 2024 20:51:48 +0800
Yu Kuai <yukuai1@...weicloud.com> wrote:

> From: Yu Kuai <yukuai3@...wei.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@...wei.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ