* superfluous md_wakeup_thread()
@ 2015-01-08 22:53 Jes Sorensen
2015-01-29 3:28 ` NeilBrown
0 siblings, 1 reply; 3+ messages in thread
From: Jes Sorensen @ 2015-01-08 22:53 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
Neil,
I was looking over some md patches, and in
commit 67f455486d2ea20b2d94d6adf5b9b783d079e321
Author: NeilBrown <neilb@suse.de>
Date: Wed May 28 13:39:22 2014 +1000
md/raid56: Don't perform reads to support writes until stripe is ready.
You add the following:
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ad1b9be..c1e8607 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -292,9 +292,12 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
BUG_ON(atomic_read(&conf->active_stripes)==0);
if (test_bit(STRIPE_HANDLE, &sh->state)) {
if (test_bit(STRIPE_DELAYED, &sh->state) &&
- !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
+ !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
list_add_tail(&sh->lru, &conf->delayed_list);
- else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
+ if (atomic_read(&conf->preread_active_stripes)
+ < IO_THRESHOLD)
+ md_wakeup_thread(conf->mddev->thread);
+ } else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
sh->bm_seq - conf->seq_write > 0)
list_add_tail(&sh->lru, &conf->bitmap_list);
else {
However the additional md_wakeup_thread() seems unecessary as the
resulting code now reads (pasted from current upstream):
static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
struct list_head *temp_inactive_list)
{
BUG_ON(!list_empty(&sh->lru));
BUG_ON(atomic_read(&conf->active_stripes)==0);
if (test_bit(STRIPE_HANDLE, &sh->state)) {
if (test_bit(STRIPE_DELAYED, &sh->state) &&
!test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
list_add_tail(&sh->lru, &conf->delayed_list);
if (atomic_read(&conf->preread_active_stripes)
< IO_THRESHOLD)
md_wakeup_thread(conf->mddev->thread);
} else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
sh->bm_seq - conf->seq_write > 0)
list_add_tail(&sh->lru, &conf->bitmap_list);
else {
clear_bit(STRIPE_DELAYED, &sh->state);
clear_bit(STRIPE_BIT_DELAY, &sh->state);
if (conf->worker_cnt_per_group == 0) {
list_add_tail(&sh->lru, &conf->handle_list);
} else {
raid5_wakeup_stripe_thread(sh);
return;
}
}
md_wakeup_thread(conf->mddev->thread);
Is there a reason to wake the thread twice?
Cheers,
Jes
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: superfluous md_wakeup_thread()
2015-01-08 22:53 superfluous md_wakeup_thread() Jes Sorensen
@ 2015-01-29 3:28 ` NeilBrown
2015-01-29 15:07 ` Jes Sorensen
0 siblings, 1 reply; 3+ messages in thread
From: NeilBrown @ 2015-01-29 3:28 UTC (permalink / raw)
To: Jes Sorensen; +Cc: linux-raid
[-- Attachment #1: Type: text/plain, Size: 3456 bytes --]
On Thu, 08 Jan 2015 17:53:15 -0500 Jes Sorensen <Jes.Sorensen@redhat.com>
wrote:
> Neil,
>
> I was looking over some md patches, and in
> commit 67f455486d2ea20b2d94d6adf5b9b783d079e321
> Author: NeilBrown <neilb@suse.de>
> Date: Wed May 28 13:39:22 2014 +1000
>
> md/raid56: Don't perform reads to support writes until stripe is ready.
>
> You add the following:
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ad1b9be..c1e8607 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -292,9 +292,12 @@ static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
> BUG_ON(atomic_read(&conf->active_stripes)==0);
> if (test_bit(STRIPE_HANDLE, &sh->state)) {
> if (test_bit(STRIPE_DELAYED, &sh->state) &&
> - !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state))
> + !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
> list_add_tail(&sh->lru, &conf->delayed_list);
> - else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
> + if (atomic_read(&conf->preread_active_stripes)
> + < IO_THRESHOLD)
> + md_wakeup_thread(conf->mddev->thread);
> + } else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
> sh->bm_seq - conf->seq_write > 0)
> list_add_tail(&sh->lru, &conf->bitmap_list);
> else {
>
> However the additional md_wakeup_thread() seems unecessary as the
> resulting code now reads (pasted from current upstream):
>
> static void do_release_stripe(struct r5conf *conf, struct stripe_head *sh,
> struct list_head *temp_inactive_list)
> {
> BUG_ON(!list_empty(&sh->lru));
> BUG_ON(atomic_read(&conf->active_stripes)==0);
> if (test_bit(STRIPE_HANDLE, &sh->state)) {
> if (test_bit(STRIPE_DELAYED, &sh->state) &&
> !test_bit(STRIPE_PREREAD_ACTIVE, &sh->state)) {
> list_add_tail(&sh->lru, &conf->delayed_list);
> if (atomic_read(&conf->preread_active_stripes)
> < IO_THRESHOLD)
> md_wakeup_thread(conf->mddev->thread);
> } else if (test_bit(STRIPE_BIT_DELAY, &sh->state) &&
> sh->bm_seq - conf->seq_write > 0)
> list_add_tail(&sh->lru, &conf->bitmap_list);
> else {
> clear_bit(STRIPE_DELAYED, &sh->state);
> clear_bit(STRIPE_BIT_DELAY, &sh->state);
> if (conf->worker_cnt_per_group == 0) {
> list_add_tail(&sh->lru, &conf->handle_list);
> } else {
> raid5_wakeup_stripe_thread(sh);
> return;
> }
> }
> md_wakeup_thread(conf->mddev->thread);
>
> Is there a reason to wake the thread twice?
Nope. No reason at all. Clearly I need to find a way to get people to
review my patches *before* I commit them....
Maybe I should try posting them to the list more :-)
Would you like to send a patch to revert that pointless change?
Thanks,
NeilBrown
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: superfluous md_wakeup_thread()
2015-01-29 3:28 ` NeilBrown
@ 2015-01-29 15:07 ` Jes Sorensen
0 siblings, 0 replies; 3+ messages in thread
From: Jes Sorensen @ 2015-01-29 15:07 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-raid
NeilBrown <neilb@suse.de> writes:
>> Is there a reason to wake the thread twice?
>
> Nope. No reason at all. Clearly I need to find a way to get people to
> review my patches *before* I commit them....
> Maybe I should try posting them to the list more :-)
>
> Would you like to send a patch to revert that pointless change?
>
> Thanks,
> NeilBrown
Ah phew, so I didn't lose my last marbles ... I read over it a couple of
times and I was sure you had a reason, but I just couldn't figure out
why :)
I'll send you a patch!
Cheers,
Jes
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-01-29 15:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-08 22:53 superfluous md_wakeup_thread() Jes Sorensen
2015-01-29 3:28 ` NeilBrown
2015-01-29 15:07 ` Jes Sorensen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).