* Question about raid1_make_request
@ 2016-11-28 23:41 Robert LeBlanc
2016-11-28 23:53 ` Doug Dumitru
0 siblings, 1 reply; 3+ messages in thread
From: Robert LeBlanc @ 2016-11-28 23:41 UTC (permalink / raw)
To: linux-raid
I'm looking through the code and am wondering why this is at the start
of raid1_make_request. It seems that it is only needed for WRITE and
would unnecessarily delay READ requests waiting for the superblock.
Would it make more sense to put this at the start of the WRITE branch?
What am I missing?
1057 /*
1058 * Register the new request and wait if the
reconstruction
1059 * thread has put up a bar for new requests.
1060 * Continue immediately if no resync is active currently.
1061 */
1062
1063 md_write_start(mddev, bio); /* wait on superblock update
early */
1064
1065 if (bio_data_dir(bio) == WRITE &&
1066 ((bio_end_sector(bio) > mddev->suspend_lo &&
1067 bio->bi_iter.bi_sector < mddev->suspend_hi) ||
1068 (mddev_is_clustered(mddev) &&
1069 md_cluster_ops->area_resyncing(mddev, WRITE,
1070 bio->bi_iter.bi_sector,
bio_end_sector(bio))))) {
1071 /* As the suspend_* range is controlled by
1072 * userspace, we want an interruptible
1073 * wait.
1074 */
1075 DEFINE_WAIT(w);
1076 for (;;) {
1077 flush_signals(current);
1078 prepare_to_wait(&conf->wait_barrier,
1079 &w, TASK_INTERRUPTIBLE);
1080 if (bio_end_sector(bio) <=
mddev->suspend_lo ||
1081 bio->bi_iter.bi_sector >=
mddev->suspend_hi ||
1082 (mddev_is_clustered(mddev) &&
1083
!md_cluster_ops->area_resyncing(mddev, WRITE,
1084 bio->bi_iter.bi_sector,
bio_end_sector(bio))))
1085 break;
1086 schedule();
1087 }
1088 finish_wait(&conf->wait_barrier, &w);
1089 }
1090
1091 start_next_window = wait_barrier(conf, bio);
1092
1093 bitmap = mddev->bitmap;
1094
Thanks
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Question about raid1_make_request
2016-11-28 23:41 Question about raid1_make_request Robert LeBlanc
@ 2016-11-28 23:53 ` Doug Dumitru
2016-11-29 0:04 ` Robert LeBlanc
0 siblings, 1 reply; 3+ messages in thread
From: Doug Dumitru @ 2016-11-28 23:53 UTC (permalink / raw)
To: Robert LeBlanc; +Cc: linux-raid
Not the best code organization, but no real harm done. Look at the
front of md_write_start:
void md_write_start(struct mddev *mddev, struct bio *bi)
{
int did_change = 0;
if (bio_data_dir(bi) != WRITE)
return;
So writes fall thru really quickly.
Doug
On Mon, Nov 28, 2016 at 3:41 PM, Robert LeBlanc <robert@leblancnet.us> wrote:
> I'm looking through the code and am wondering why this is at the start
> of raid1_make_request. It seems that it is only needed for WRITE and
> would unnecessarily delay READ requests waiting for the superblock.
> Would it make more sense to put this at the start of the WRITE branch?
> What am I missing?
>
> 1057 /*
> 1058 * Register the new request and wait if the
> reconstruction
> 1059 * thread has put up a bar for new requests.
> 1060 * Continue immediately if no resync is active currently.
> 1061 */
> 1062
> 1063 md_write_start(mddev, bio); /* wait on superblock update
> early */
> 1064
> 1065 if (bio_data_dir(bio) == WRITE &&
> 1066 ((bio_end_sector(bio) > mddev->suspend_lo &&
> 1067 bio->bi_iter.bi_sector < mddev->suspend_hi) ||
> 1068 (mddev_is_clustered(mddev) &&
> 1069 md_cluster_ops->area_resyncing(mddev, WRITE,
> 1070 bio->bi_iter.bi_sector,
> bio_end_sector(bio))))) {
> 1071 /* As the suspend_* range is controlled by
> 1072 * userspace, we want an interruptible
> 1073 * wait.
> 1074 */
> 1075 DEFINE_WAIT(w);
> 1076 for (;;) {
> 1077 flush_signals(current);
> 1078 prepare_to_wait(&conf->wait_barrier,
> 1079 &w, TASK_INTERRUPTIBLE);
> 1080 if (bio_end_sector(bio) <=
> mddev->suspend_lo ||
> 1081 bio->bi_iter.bi_sector >=
> mddev->suspend_hi ||
> 1082 (mddev_is_clustered(mddev) &&
> 1083
> !md_cluster_ops->area_resyncing(mddev, WRITE,
> 1084 bio->bi_iter.bi_sector,
> bio_end_sector(bio))))
> 1085 break;
> 1086 schedule();
> 1087 }
> 1088 finish_wait(&conf->wait_barrier, &w);
> 1089 }
> 1090
> 1091 start_next_window = wait_barrier(conf, bio);
> 1092
> 1093 bitmap = mddev->bitmap;
> 1094
>
> Thanks
>
> ----------------
> Robert LeBlanc
> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Doug Dumitru
EasyCo LLC
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Question about raid1_make_request
2016-11-28 23:53 ` Doug Dumitru
@ 2016-11-29 0:04 ` Robert LeBlanc
0 siblings, 0 replies; 3+ messages in thread
From: Robert LeBlanc @ 2016-11-29 0:04 UTC (permalink / raw)
To: doug; +Cc: linux-raid
I think I looked at the comments to that function and then didn't read
the code too closely. A few extra tests and jumps, but in the grand
scheme of things when dealing with disks, not noticeable like I
thought before. Thanks for pointing it out.
----------------
Robert LeBlanc
PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
On Mon, Nov 28, 2016 at 4:53 PM, Doug Dumitru <doug@easyco.com> wrote:
> Not the best code organization, but no real harm done. Look at the
> front of md_write_start:
>
> void md_write_start(struct mddev *mddev, struct bio *bi)
> {
> int did_change = 0;
> if (bio_data_dir(bi) != WRITE)
> return;
>
> So writes fall thru really quickly.
>
> Doug
>
>
> On Mon, Nov 28, 2016 at 3:41 PM, Robert LeBlanc <robert@leblancnet.us> wrote:
>> I'm looking through the code and am wondering why this is at the start
>> of raid1_make_request. It seems that it is only needed for WRITE and
>> would unnecessarily delay READ requests waiting for the superblock.
>> Would it make more sense to put this at the start of the WRITE branch?
>> What am I missing?
>>
>> 1057 /*
>> 1058 * Register the new request and wait if the
>> reconstruction
>> 1059 * thread has put up a bar for new requests.
>> 1060 * Continue immediately if no resync is active currently.
>> 1061 */
>> 1062
>> 1063 md_write_start(mddev, bio); /* wait on superblock update
>> early */
>> 1064
>> 1065 if (bio_data_dir(bio) == WRITE &&
>> 1066 ((bio_end_sector(bio) > mddev->suspend_lo &&
>> 1067 bio->bi_iter.bi_sector < mddev->suspend_hi) ||
>> 1068 (mddev_is_clustered(mddev) &&
>> 1069 md_cluster_ops->area_resyncing(mddev, WRITE,
>> 1070 bio->bi_iter.bi_sector,
>> bio_end_sector(bio))))) {
>> 1071 /* As the suspend_* range is controlled by
>> 1072 * userspace, we want an interruptible
>> 1073 * wait.
>> 1074 */
>> 1075 DEFINE_WAIT(w);
>> 1076 for (;;) {
>> 1077 flush_signals(current);
>> 1078 prepare_to_wait(&conf->wait_barrier,
>> 1079 &w, TASK_INTERRUPTIBLE);
>> 1080 if (bio_end_sector(bio) <=
>> mddev->suspend_lo ||
>> 1081 bio->bi_iter.bi_sector >=
>> mddev->suspend_hi ||
>> 1082 (mddev_is_clustered(mddev) &&
>> 1083
>> !md_cluster_ops->area_resyncing(mddev, WRITE,
>> 1084 bio->bi_iter.bi_sector,
>> bio_end_sector(bio))))
>> 1085 break;
>> 1086 schedule();
>> 1087 }
>> 1088 finish_wait(&conf->wait_barrier, &w);
>> 1089 }
>> 1090
>> 1091 start_next_window = wait_barrier(conf, bio);
>> 1092
>> 1093 bitmap = mddev->bitmap;
>> 1094
>>
>> Thanks
>>
>> ----------------
>> Robert LeBlanc
>> PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-raid" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Doug Dumitru
> EasyCo LLC
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-29 0:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-28 23:41 Question about raid1_make_request Robert LeBlanc
2016-11-28 23:53 ` Doug Dumitru
2016-11-29 0:04 ` Robert LeBlanc
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.