* [PATCH V2 0/2] md: Change active_io to percpu @ 2023-01-31 5:17 Xiao Ni 2023-01-31 5:17 ` [PATCH 1/2] md: Factor out is_md_suspended helper Xiao Ni 2023-01-31 5:17 ` [PATCH V2 2/2] md: Change active_io to percpu Xiao Ni 0 siblings, 2 replies; 7+ messages in thread From: Xiao Ni @ 2023-01-31 5:17 UTC (permalink / raw) To: song; +Cc: linux-raid, ming.lei, ncroxon, heinzm Hi all This is a optimization for active_io. Now it's atomic type and added/decreased when io comes, but it's only needed to be checked when suspending raid. So we can change it to percpu type. The first patch factors out a helper function which is used in the second patch. The patches have been tested by regression test under tests directory. I did a performance test too. In my environment there is no big change. Xiao Ni (2): Factor out is_md_suspended helper md: Change active_io to percpu drivers/md/md.c | 50 ++++++++++++++++++++++++++++++------------------- drivers/md/md.h | 2 +- 2 files changed, 32 insertions(+), 20 deletions(-) -- 2.32.0 (Apple Git-132) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] md: Factor out is_md_suspended helper 2023-01-31 5:17 [PATCH V2 0/2] md: Change active_io to percpu Xiao Ni @ 2023-01-31 5:17 ` Xiao Ni 2023-01-31 5:17 ` [PATCH V2 2/2] md: Change active_io to percpu Xiao Ni 1 sibling, 0 replies; 7+ messages in thread From: Xiao Ni @ 2023-01-31 5:17 UTC (permalink / raw) To: song; +Cc: linux-raid, ming.lei, ncroxon, heinzm This helper function will be used in next patch. It's easy for understanding. Signed-off-by: Xiao Ni <xni@redhat.com> --- drivers/md/md.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index 775f1dde190a..d3627aad981a 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -380,6 +380,13 @@ EXPORT_SYMBOL_GPL(md_new_event); static LIST_HEAD(all_mddevs); static DEFINE_SPINLOCK(all_mddevs_lock); +static bool is_md_suspended(struct mddev *mddev) +{ + if (mddev->suspended) + return true; + else + return false; +} /* Rather than calling directly into the personality make_request function, * IO requests come here first so that we can check if the device is * being suspended pending a reconfiguration. @@ -389,7 +396,7 @@ static DEFINE_SPINLOCK(all_mddevs_lock); */ static bool is_suspended(struct mddev *mddev, struct bio *bio) { - if (mddev->suspended) + if (is_md_suspended(mddev)) return true; if (bio_data_dir(bio) != WRITE) return false; @@ -434,7 +441,7 @@ void md_handle_request(struct mddev *mddev, struct bio *bio) goto check_suspended; } - if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended) + if (atomic_dec_and_test(&mddev->active_io) && is_md_suspended(mddev)) wake_up(&mddev->sb_wait); } EXPORT_SYMBOL(md_handle_request); @@ -6217,7 +6224,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes); static void mddev_detach(struct mddev *mddev) { md_bitmap_wait_behind_writes(mddev); - if (mddev->pers && mddev->pers->quiesce && !mddev->suspended) { + if (mddev->pers && mddev->pers->quiesce && !is_md_suspended(mddev)) { mddev->pers->quiesce(mddev, 1); mddev->pers->quiesce(mddev, 0); } @@ -8529,7 +8536,7 @@ bool md_write_start(struct mddev *mddev, struct bio *bi) return true; wait_event(mddev->sb_wait, !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) || - mddev->suspended); + is_md_suspended(mddev)); if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)) { percpu_ref_put(&mddev->writes_pending); return false; @@ -9257,7 +9264,7 @@ void md_check_recovery(struct mddev *mddev) wake_up(&mddev->sb_wait); } - if (mddev->suspended) + if (is_md_suspended(mddev)) return; if (mddev->bitmap) -- 2.32.0 (Apple Git-132) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH V2 2/2] md: Change active_io to percpu 2023-01-31 5:17 [PATCH V2 0/2] md: Change active_io to percpu Xiao Ni 2023-01-31 5:17 ` [PATCH 1/2] md: Factor out is_md_suspended helper Xiao Ni @ 2023-01-31 5:17 ` Xiao Ni 2023-02-01 7:16 ` Song Liu 1 sibling, 1 reply; 7+ messages in thread From: Xiao Ni @ 2023-01-31 5:17 UTC (permalink / raw) To: song; +Cc: linux-raid, ming.lei, ncroxon, heinzm Now the type of active_io is atomic. It's used to count how many ios are in the submitting process and it's added and decreased very time. But it only needs to check if it's zero when suspending the raid. So we can switch atomic to percpu to improve the performance. After switching active_io to percpu type, we use the state of active_io to judge if the raid device is suspended. And we don't need to wake up ->sb_wait in md_handle_request anymore. It's done in the callback function which is registered when initing active_io. The argument mddev->suspended is only used to count how many users are trying to set raid to suspend state. Signed-off-by: Xiao Ni <xni@redhat.com> --- v2: remove all rcu api in md_handle_request drivers/md/md.c | 43 ++++++++++++++++++++++++------------------- drivers/md/md.h | 2 +- 2 files changed, 25 insertions(+), 20 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index d3627aad981a..0eb31bef1f01 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -382,10 +382,7 @@ static DEFINE_SPINLOCK(all_mddevs_lock); static bool is_md_suspended(struct mddev *mddev) { - if (mddev->suspended) - return true; - else - return false; + return percpu_ref_is_dying(&mddev->active_io); } /* Rather than calling directly into the personality make_request function, * IO requests come here first so that we can check if the device is @@ -412,12 +409,10 @@ static bool is_suspended(struct mddev *mddev, struct bio *bio) void md_handle_request(struct mddev *mddev, struct bio *bio) { check_suspended: - rcu_read_lock(); if (is_suspended(mddev, bio)) { DEFINE_WAIT(__wait); /* Bail out if REQ_NOWAIT is set for the bio */ if (bio->bi_opf & REQ_NOWAIT) { - rcu_read_unlock(); bio_wouldblock_error(bio); return; } @@ -426,23 +421,19 @@ void md_handle_request(struct mddev *mddev, struct bio *bio) TASK_UNINTERRUPTIBLE); if (!is_suspended(mddev, bio)) break; - rcu_read_unlock(); schedule(); - rcu_read_lock(); } finish_wait(&mddev->sb_wait, &__wait); } - atomic_inc(&mddev->active_io); - rcu_read_unlock(); + if (!percpu_ref_tryget_live(&mddev->active_io)) + goto check_suspended; if (!mddev->pers->make_request(mddev, bio)) { - atomic_dec(&mddev->active_io); - wake_up(&mddev->sb_wait); + percpu_ref_put(&mddev->active_io); goto check_suspended; } - if (atomic_dec_and_test(&mddev->active_io) && is_md_suspended(mddev)) - wake_up(&mddev->sb_wait); + percpu_ref_put(&mddev->active_io); } EXPORT_SYMBOL(md_handle_request); @@ -488,11 +479,10 @@ void mddev_suspend(struct mddev *mddev) lockdep_assert_held(&mddev->reconfig_mutex); if (mddev->suspended++) return; - synchronize_rcu(); wake_up(&mddev->sb_wait); set_bit(MD_ALLOW_SB_UPDATE, &mddev->flags); - smp_mb__after_atomic(); - wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0); + percpu_ref_kill(&mddev->active_io); + wait_event(mddev->sb_wait, percpu_ref_is_zero(&mddev->active_io)); mddev->pers->quiesce(mddev, 1); clear_bit_unlock(MD_ALLOW_SB_UPDATE, &mddev->flags); wait_event(mddev->sb_wait, !test_bit(MD_UPDATING_SB, &mddev->flags)); @@ -510,6 +500,7 @@ void mddev_resume(struct mddev *mddev) lockdep_assert_held(&mddev->reconfig_mutex); if (--mddev->suspended) return; + percpu_ref_resurrect(&mddev->active_io); wake_up(&mddev->sb_wait); mddev->pers->quiesce(mddev, 0); @@ -688,7 +679,6 @@ void mddev_init(struct mddev *mddev) timer_setup(&mddev->safemode_timer, md_safemode_timeout, 0); atomic_set(&mddev->active, 1); atomic_set(&mddev->openers, 0); - atomic_set(&mddev->active_io, 0); spin_lock_init(&mddev->lock); atomic_set(&mddev->flush_pending, 0); init_waitqueue_head(&mddev->sb_wait); @@ -5765,6 +5755,12 @@ static void md_safemode_timeout(struct timer_list *t) } static int start_dirty_degraded; +static void active_io_release(struct percpu_ref *ref) +{ + struct mddev *mddev = container_of(ref, struct mddev, active_io); + + wake_up(&mddev->sb_wait); +} int md_run(struct mddev *mddev) { @@ -5845,10 +5841,15 @@ int md_run(struct mddev *mddev) nowait = nowait && bdev_nowait(rdev->bdev); } + err = percpu_ref_init(&mddev->active_io, active_io_release, + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); + if (err) + return err; + if (!bioset_initialized(&mddev->bio_set)) { err = bioset_init(&mddev->bio_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); if (err) - return err; + goto exit_active_io; } if (!bioset_initialized(&mddev->sync_set)) { err = bioset_init(&mddev->sync_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); @@ -6036,6 +6037,8 @@ int md_run(struct mddev *mddev) bioset_exit(&mddev->sync_set); exit_bio_set: bioset_exit(&mddev->bio_set); +exit_active_io: + percpu_ref_exit(&mddev->active_io); return err; } EXPORT_SYMBOL_GPL(md_run); @@ -6260,6 +6263,7 @@ void md_stop(struct mddev *mddev) */ __md_stop_writes(mddev); __md_stop(mddev); + percpu_ref_exit(&mddev->active_io); bioset_exit(&mddev->bio_set); bioset_exit(&mddev->sync_set); } @@ -7833,6 +7837,7 @@ static void md_free_disk(struct gendisk *disk) struct mddev *mddev = disk->private_data; percpu_ref_exit(&mddev->writes_pending); + percpu_ref_exit(&mddev->active_io); bioset_exit(&mddev->bio_set); bioset_exit(&mddev->sync_set); diff --git a/drivers/md/md.h b/drivers/md/md.h index 554a9026669a..6335cb86e52e 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h @@ -315,7 +315,7 @@ struct mddev { unsigned long sb_flags; int suspended; - atomic_t active_io; + struct percpu_ref active_io; int ro; int sysfs_active; /* set when sysfs deletes * are happening, so run/ -- 2.32.0 (Apple Git-132) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH V2 2/2] md: Change active_io to percpu 2023-01-31 5:17 ` [PATCH V2 2/2] md: Change active_io to percpu Xiao Ni @ 2023-02-01 7:16 ` Song Liu 0 siblings, 0 replies; 7+ messages in thread From: Song Liu @ 2023-02-01 7:16 UTC (permalink / raw) To: Xiao Ni; +Cc: linux-raid, ming.lei, ncroxon, heinzm On Mon, Jan 30, 2023 at 9:17 PM Xiao Ni <xni@redhat.com> wrote: > > Now the type of active_io is atomic. It's used to count how many ios are > in the submitting process and it's added and decreased very time. But it > only needs to check if it's zero when suspending the raid. So we can > switch atomic to percpu to improve the performance. > > After switching active_io to percpu type, we use the state of active_io > to judge if the raid device is suspended. And we don't need to wake up > ->sb_wait in md_handle_request anymore. It's done in the callback function > which is registered when initing active_io. The argument mddev->suspended > is only used to count how many users are trying to set raid to suspend > state. > > Signed-off-by: Xiao Ni <xni@redhat.com> Applied to md-next. Thanks, Song > --- > v2: remove all rcu api in md_handle_request > drivers/md/md.c | 43 ++++++++++++++++++++++++------------------- > drivers/md/md.h | 2 +- > 2 files changed, 25 insertions(+), 20 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index d3627aad981a..0eb31bef1f01 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -382,10 +382,7 @@ static DEFINE_SPINLOCK(all_mddevs_lock); > > static bool is_md_suspended(struct mddev *mddev) > { > - if (mddev->suspended) > - return true; > - else > - return false; > + return percpu_ref_is_dying(&mddev->active_io); > } > /* Rather than calling directly into the personality make_request function, > * IO requests come here first so that we can check if the device is > @@ -412,12 +409,10 @@ static bool is_suspended(struct mddev *mddev, struct bio *bio) > void md_handle_request(struct mddev *mddev, struct bio *bio) > { > check_suspended: > - rcu_read_lock(); > if (is_suspended(mddev, bio)) { > DEFINE_WAIT(__wait); > /* Bail out if REQ_NOWAIT is set for the bio */ > if (bio->bi_opf & REQ_NOWAIT) { > - rcu_read_unlock(); > bio_wouldblock_error(bio); > return; > } > @@ -426,23 +421,19 @@ void md_handle_request(struct mddev *mddev, struct bio *bio) > TASK_UNINTERRUPTIBLE); > if (!is_suspended(mddev, bio)) > break; > - rcu_read_unlock(); > schedule(); > - rcu_read_lock(); > } > finish_wait(&mddev->sb_wait, &__wait); > } > - atomic_inc(&mddev->active_io); > - rcu_read_unlock(); > + if (!percpu_ref_tryget_live(&mddev->active_io)) > + goto check_suspended; > > if (!mddev->pers->make_request(mddev, bio)) { > - atomic_dec(&mddev->active_io); > - wake_up(&mddev->sb_wait); > + percpu_ref_put(&mddev->active_io); > goto check_suspended; > } > > - if (atomic_dec_and_test(&mddev->active_io) && is_md_suspended(mddev)) > - wake_up(&mddev->sb_wait); > + percpu_ref_put(&mddev->active_io); > } > EXPORT_SYMBOL(md_handle_request); > > @@ -488,11 +479,10 @@ void mddev_suspend(struct mddev *mddev) > lockdep_assert_held(&mddev->reconfig_mutex); > if (mddev->suspended++) > return; > - synchronize_rcu(); > wake_up(&mddev->sb_wait); > set_bit(MD_ALLOW_SB_UPDATE, &mddev->flags); > - smp_mb__after_atomic(); > - wait_event(mddev->sb_wait, atomic_read(&mddev->active_io) == 0); > + percpu_ref_kill(&mddev->active_io); > + wait_event(mddev->sb_wait, percpu_ref_is_zero(&mddev->active_io)); > mddev->pers->quiesce(mddev, 1); > clear_bit_unlock(MD_ALLOW_SB_UPDATE, &mddev->flags); > wait_event(mddev->sb_wait, !test_bit(MD_UPDATING_SB, &mddev->flags)); > @@ -510,6 +500,7 @@ void mddev_resume(struct mddev *mddev) > lockdep_assert_held(&mddev->reconfig_mutex); > if (--mddev->suspended) > return; > + percpu_ref_resurrect(&mddev->active_io); > wake_up(&mddev->sb_wait); > mddev->pers->quiesce(mddev, 0); > > @@ -688,7 +679,6 @@ void mddev_init(struct mddev *mddev) > timer_setup(&mddev->safemode_timer, md_safemode_timeout, 0); > atomic_set(&mddev->active, 1); > atomic_set(&mddev->openers, 0); > - atomic_set(&mddev->active_io, 0); > spin_lock_init(&mddev->lock); > atomic_set(&mddev->flush_pending, 0); > init_waitqueue_head(&mddev->sb_wait); > @@ -5765,6 +5755,12 @@ static void md_safemode_timeout(struct timer_list *t) > } > > static int start_dirty_degraded; > +static void active_io_release(struct percpu_ref *ref) > +{ > + struct mddev *mddev = container_of(ref, struct mddev, active_io); > + > + wake_up(&mddev->sb_wait); > +} > > int md_run(struct mddev *mddev) > { > @@ -5845,10 +5841,15 @@ int md_run(struct mddev *mddev) > nowait = nowait && bdev_nowait(rdev->bdev); > } > > + err = percpu_ref_init(&mddev->active_io, active_io_release, > + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL); > + if (err) > + return err; > + > if (!bioset_initialized(&mddev->bio_set)) { > err = bioset_init(&mddev->bio_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); > if (err) > - return err; > + goto exit_active_io; > } > if (!bioset_initialized(&mddev->sync_set)) { > err = bioset_init(&mddev->sync_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); > @@ -6036,6 +6037,8 @@ int md_run(struct mddev *mddev) > bioset_exit(&mddev->sync_set); > exit_bio_set: > bioset_exit(&mddev->bio_set); > +exit_active_io: > + percpu_ref_exit(&mddev->active_io); > return err; > } > EXPORT_SYMBOL_GPL(md_run); > @@ -6260,6 +6263,7 @@ void md_stop(struct mddev *mddev) > */ > __md_stop_writes(mddev); > __md_stop(mddev); > + percpu_ref_exit(&mddev->active_io); > bioset_exit(&mddev->bio_set); > bioset_exit(&mddev->sync_set); > } > @@ -7833,6 +7837,7 @@ static void md_free_disk(struct gendisk *disk) > struct mddev *mddev = disk->private_data; > > percpu_ref_exit(&mddev->writes_pending); > + percpu_ref_exit(&mddev->active_io); > bioset_exit(&mddev->bio_set); > bioset_exit(&mddev->sync_set); > > diff --git a/drivers/md/md.h b/drivers/md/md.h > index 554a9026669a..6335cb86e52e 100644 > --- a/drivers/md/md.h > +++ b/drivers/md/md.h > @@ -315,7 +315,7 @@ struct mddev { > unsigned long sb_flags; > > int suspended; > - atomic_t active_io; > + struct percpu_ref active_io; > int ro; > int sysfs_active; /* set when sysfs deletes > * are happening, so run/ > -- > 2.32.0 (Apple Git-132) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 0/2] md: Change active_io to percpu @ 2023-01-21 1:39 Xiao Ni 2023-01-21 1:39 ` [PATCH 1/2] md: Factor out is_md_suspended helper Xiao Ni 0 siblings, 1 reply; 7+ messages in thread From: Xiao Ni @ 2023-01-21 1:39 UTC (permalink / raw) To: song; +Cc: linux-raid, ming.lei, ncroxon, heinzm Hi all This is a optimization for active_io. Now it's atomic type and added/decreased when io comes, but it's only needed to be checked when suspending raid. So we can change it to percpu type. The first patch factors out a helper function which is used in the second patch. The patches have been tested by regression test under tests directory. I did a performance test too. In my environment there is no big change. Xiao Ni (2): Factor out is_md_suspended helper Change active_io to percpu drivers/md/md.c | 46 ++++++++++++++++++++++++++++++---------------- drivers/md/md.h | 2 +- 2 files changed, 31 insertions(+), 17 deletions(-) -- 2.32.0 (Apple Git-132) ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] md: Factor out is_md_suspended helper 2023-01-21 1:39 [PATCH 0/2] " Xiao Ni @ 2023-01-21 1:39 ` Xiao Ni 2023-01-23 13:18 ` Paul Menzel 0 siblings, 1 reply; 7+ messages in thread From: Xiao Ni @ 2023-01-21 1:39 UTC (permalink / raw) To: song; +Cc: linux-raid, ming.lei, ncroxon, heinzm This helper function will be used in next patch. It's easy for understanding. Signed-off-by: Xiao Ni <xni@redhat.com> --- drivers/md/md.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index 775f1dde190a..d3627aad981a 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -380,6 +380,13 @@ EXPORT_SYMBOL_GPL(md_new_event); static LIST_HEAD(all_mddevs); static DEFINE_SPINLOCK(all_mddevs_lock); +static bool is_md_suspended(struct mddev *mddev) +{ + if (mddev->suspended) + return true; + else + return false; +} /* Rather than calling directly into the personality make_request function, * IO requests come here first so that we can check if the device is * being suspended pending a reconfiguration. @@ -389,7 +396,7 @@ static DEFINE_SPINLOCK(all_mddevs_lock); */ static bool is_suspended(struct mddev *mddev, struct bio *bio) { - if (mddev->suspended) + if (is_md_suspended(mddev)) return true; if (bio_data_dir(bio) != WRITE) return false; @@ -434,7 +441,7 @@ void md_handle_request(struct mddev *mddev, struct bio *bio) goto check_suspended; } - if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended) + if (atomic_dec_and_test(&mddev->active_io) && is_md_suspended(mddev)) wake_up(&mddev->sb_wait); } EXPORT_SYMBOL(md_handle_request); @@ -6217,7 +6224,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes); static void mddev_detach(struct mddev *mddev) { md_bitmap_wait_behind_writes(mddev); - if (mddev->pers && mddev->pers->quiesce && !mddev->suspended) { + if (mddev->pers && mddev->pers->quiesce && !is_md_suspended(mddev)) { mddev->pers->quiesce(mddev, 1); mddev->pers->quiesce(mddev, 0); } @@ -8529,7 +8536,7 @@ bool md_write_start(struct mddev *mddev, struct bio *bi) return true; wait_event(mddev->sb_wait, !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) || - mddev->suspended); + is_md_suspended(mddev)); if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)) { percpu_ref_put(&mddev->writes_pending); return false; @@ -9257,7 +9264,7 @@ void md_check_recovery(struct mddev *mddev) wake_up(&mddev->sb_wait); } - if (mddev->suspended) + if (is_md_suspended(mddev)) return; if (mddev->bitmap) -- 2.32.0 (Apple Git-132) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] md: Factor out is_md_suspended helper 2023-01-21 1:39 ` [PATCH 1/2] md: Factor out is_md_suspended helper Xiao Ni @ 2023-01-23 13:18 ` Paul Menzel 2023-01-23 23:48 ` Xiao Ni 0 siblings, 1 reply; 7+ messages in thread From: Paul Menzel @ 2023-01-23 13:18 UTC (permalink / raw) To: Xiao Ni; +Cc: song, linux-raid, ming.lei, ncroxon, heinzm Dear Xiao, Thank you for the patch. Am 21.01.23 um 02:39 schrieb Xiao Ni: > This helper function will be used in next patch. It's easy for > understanding. > > Signed-off-by: Xiao Ni <xni@redhat.com> > --- > drivers/md/md.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > > diff --git a/drivers/md/md.c b/drivers/md/md.c > index 775f1dde190a..d3627aad981a 100644 > --- a/drivers/md/md.c > +++ b/drivers/md/md.c > @@ -380,6 +380,13 @@ EXPORT_SYMBOL_GPL(md_new_event); > static LIST_HEAD(all_mddevs); > static DEFINE_SPINLOCK(all_mddevs_lock); > > +static bool is_md_suspended(struct mddev *mddev) > +{ > + if (mddev->suspended) > + return true; > + else > + return false; suspended seems to be an integer, so this could be written as: return !!mddev->suspended; or return (mddev->suspended) ? true : false; Kind regards, Paul > +} > /* Rather than calling directly into the personality make_request function, > * IO requests come here first so that we can check if the device is > * being suspended pending a reconfiguration. > @@ -389,7 +396,7 @@ static DEFINE_SPINLOCK(all_mddevs_lock); > */ > static bool is_suspended(struct mddev *mddev, struct bio *bio) > { > - if (mddev->suspended) > + if (is_md_suspended(mddev)) > return true; > if (bio_data_dir(bio) != WRITE) > return false; > @@ -434,7 +441,7 @@ void md_handle_request(struct mddev *mddev, struct bio *bio) > goto check_suspended; > } > > - if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended) > + if (atomic_dec_and_test(&mddev->active_io) && is_md_suspended(mddev)) > wake_up(&mddev->sb_wait); > } > EXPORT_SYMBOL(md_handle_request); > @@ -6217,7 +6224,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes); > static void mddev_detach(struct mddev *mddev) > { > md_bitmap_wait_behind_writes(mddev); > - if (mddev->pers && mddev->pers->quiesce && !mddev->suspended) { > + if (mddev->pers && mddev->pers->quiesce && !is_md_suspended(mddev)) { > mddev->pers->quiesce(mddev, 1); > mddev->pers->quiesce(mddev, 0); > } > @@ -8529,7 +8536,7 @@ bool md_write_start(struct mddev *mddev, struct bio *bi) > return true; > wait_event(mddev->sb_wait, > !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) || > - mddev->suspended); > + is_md_suspended(mddev)); > if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)) { > percpu_ref_put(&mddev->writes_pending); > return false; > @@ -9257,7 +9264,7 @@ void md_check_recovery(struct mddev *mddev) > wake_up(&mddev->sb_wait); > } > > - if (mddev->suspended) > + if (is_md_suspended(mddev)) > return; > > if (mddev->bitmap) ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] md: Factor out is_md_suspended helper 2023-01-23 13:18 ` Paul Menzel @ 2023-01-23 23:48 ` Xiao Ni 0 siblings, 0 replies; 7+ messages in thread From: Xiao Ni @ 2023-01-23 23:48 UTC (permalink / raw) To: Paul Menzel; +Cc: song, linux-raid, ming.lei, ncroxon, heinzm On Mon, Jan 23, 2023 at 9:18 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > Dear Xiao, > > > Thank you for the patch. > > Am 21.01.23 um 02:39 schrieb Xiao Ni: > > This helper function will be used in next patch. It's easy for > > understanding. > > > > Signed-off-by: Xiao Ni <xni@redhat.com> > > --- > > drivers/md/md.c | 17 ++++++++++++----- > > 1 file changed, 12 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/md/md.c b/drivers/md/md.c > > index 775f1dde190a..d3627aad981a 100644 > > --- a/drivers/md/md.c > > +++ b/drivers/md/md.c > > @@ -380,6 +380,13 @@ EXPORT_SYMBOL_GPL(md_new_event); > > static LIST_HEAD(all_mddevs); > > static DEFINE_SPINLOCK(all_mddevs_lock); > > > > +static bool is_md_suspended(struct mddev *mddev) > > +{ > > + if (mddev->suspended) > > + return true; > > + else > > + return false; > > suspended seems to be an integer, so this could be written as: > > return !!mddev->suspended; > > or > > return (mddev->suspended) ? true : false; > > > Kind regards, > > Paul Hi Paul Thanks for this. Regards Xiao > > > > +} > > /* Rather than calling directly into the personality make_request function, > > * IO requests come here first so that we can check if the device is > > * being suspended pending a reconfiguration. > > @@ -389,7 +396,7 @@ static DEFINE_SPINLOCK(all_mddevs_lock); > > */ > > static bool is_suspended(struct mddev *mddev, struct bio *bio) > > { > > - if (mddev->suspended) > > + if (is_md_suspended(mddev)) > > return true; > > if (bio_data_dir(bio) != WRITE) > > return false; > > @@ -434,7 +441,7 @@ void md_handle_request(struct mddev *mddev, struct bio *bio) > > goto check_suspended; > > } > > > > - if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended) > > + if (atomic_dec_and_test(&mddev->active_io) && is_md_suspended(mddev)) > > wake_up(&mddev->sb_wait); > > } > > EXPORT_SYMBOL(md_handle_request); > > @@ -6217,7 +6224,7 @@ EXPORT_SYMBOL_GPL(md_stop_writes); > > static void mddev_detach(struct mddev *mddev) > > { > > md_bitmap_wait_behind_writes(mddev); > > - if (mddev->pers && mddev->pers->quiesce && !mddev->suspended) { > > + if (mddev->pers && mddev->pers->quiesce && !is_md_suspended(mddev)) { > > mddev->pers->quiesce(mddev, 1); > > mddev->pers->quiesce(mddev, 0); > > } > > @@ -8529,7 +8536,7 @@ bool md_write_start(struct mddev *mddev, struct bio *bi) > > return true; > > wait_event(mddev->sb_wait, > > !test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags) || > > - mddev->suspended); > > + is_md_suspended(mddev)); > > if (test_bit(MD_SB_CHANGE_PENDING, &mddev->sb_flags)) { > > percpu_ref_put(&mddev->writes_pending); > > return false; > > @@ -9257,7 +9264,7 @@ void md_check_recovery(struct mddev *mddev) > > wake_up(&mddev->sb_wait); > > } > > > > - if (mddev->suspended) > > + if (is_md_suspended(mddev)) > > return; > > > > if (mddev->bitmap) > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-01 7:16 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-31 5:17 [PATCH V2 0/2] md: Change active_io to percpu Xiao Ni 2023-01-31 5:17 ` [PATCH 1/2] md: Factor out is_md_suspended helper Xiao Ni 2023-01-31 5:17 ` [PATCH V2 2/2] md: Change active_io to percpu Xiao Ni 2023-02-01 7:16 ` Song Liu -- strict thread matches above, loose matches on Subject: below -- 2023-01-21 1:39 [PATCH 0/2] " Xiao Ni 2023-01-21 1:39 ` [PATCH 1/2] md: Factor out is_md_suspended helper Xiao Ni 2023-01-23 13:18 ` Paul Menzel 2023-01-23 23:48 ` Xiao Ni
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.