Lustre RAID 5/6 patches
cover Lustre RAID 5/6 patches
The Lustre Group at Sun (formerly Cluster File Systems, Inc.) has produced a number of Linux MD RAID 5/6 improvements, mainly aimed at improving performance. They were originally written for RHEL 4 then ported to RHEL 5. I am now porting them to the upstream kernel and submitting them for comment and eventual inclusion.
This is the first of 8. More to follow over the coming weeks.
RFC PATCH 1/1 Track raid5/6 statistics.
This patch tracks various statistics related to the performance of a RAID 5 or 6 array. These have been useful to us in the past to help solve performance issues. They are reported via /proc/mdstat.
I realize that the format of the statistics may not be the best, and there may be a better location than /proc/mdstat, so I welcome suggestions on where to put them.
I will add documentation once we've decided on the format and location (or if nobody objects to the current format and location.)
drivers/md/raid5.c +72 -11
@@ -136,7 +136,7 @@ static inline int raid6_next_disk(int direturn (disk < raid_disks) ? disk :0 ; } -static void return_io (struct bio *return_bi) +static void return_io (struct bio *return_bi,raid5_conf_t *conf) {struct bio *bi = return_bi;while (bi) {
@@ -145,6 +145,7 @@ static void return_io(struct bio *return bi->bi_next =NULL ; bi->bi_size =0 ;bio_endio (bi,0 ); +atomic_dec (&conf->in_reqs_in_queue); bi = return_bi; } }
@@ -167,10 +168,12 @@ static void __release_stripe(raid5_conf_if (test_bit (STRIPE_DELAYED , &sh->state)) {list_add_tail (&sh->lru, &conf->delayed_list);blk_plug_device (conf->mddev->queue); +atomic_inc (&conf->delayed); }else if (test_bit (STRIPE_BIT_DELAY , &sh->state) && sh->bm_seq - conf->seq_write >0 ) {list_add_tail (&sh->lru, &conf->bitmap_list);blk_plug_device (conf->mddev->queue); +atomic_inc (&conf->bit_delayed); }else {clear_bit (STRIPE_BIT_DELAY , &sh->state);list_add_tail (&sh->lru, &conf->handle_list);
@@ -347,6 +350,7 @@ static struct stripe_head *get_active_stif (noblock && sh ==NULL )break ;if (!sh) { +atomic_inc (&conf->out_of_stripes); conf->inactive_blocked =1 ; wait_event_lock_irq(conf->wait_for_stripe, !list_empty(&conf->inactive_list) &&
@@ -369,6 +373,10 @@ static struct stripe_head *get_active_st !test_bit(STRIPE_EXPANDING, &sh->state))BUG ();list_del_init (&sh->lru); +if (test_bit (STRIPE_DELAYED , &sh->state)) +atomic_dec (&conf->delayed); +if (test_bit (STRIPE_BIT_DELAY , &sh->state)) +atomic_dec (&conf->bit_delayed); } } }while (sh ==NULL );
@@ -406,10 +414,13 @@ static void ops_run_io(struct stripe_hea bi = &sh->dev[i].req; bi->bi_rw = rw; -if (rw ==WRITE ) +if (rw ==WRITE ) { +atomic_inc (&conf->writes_out); bi->bi_end_io = raid5_end_write_request; -else + }else { +atomic_inc (&conf->reads_out); bi->bi_end_io = raid5_end_read_request; + }rcu_read_lock (); rdev =rcu_dereference (conf->disks[i].rdev);
@@ -444,6 +455,7 @@ static void ops_run_io(struct stripe_hea test_bit(R5_ReWrite, &sh->dev[i].flags))atomic_add (STRIPE_SECTORS , &rdev->corrected_errors); +atomic_inc (&conf->out_reqs_in_queue);generic_make_request (bi); }else {if (rw ==WRITE )
@@ -547,7 +559,7 @@ static void ops_complete_biofill(void *sspin_unlock_irq (&conf->device_lock);clear_bit (STRIPE_BIOFILL_RUN , &sh->state); -return_io (return_bi); +return_io (return_bi, conf);set_bit (STRIPE_HANDLE , &sh->state);release_stripe (sh);
@@ -1074,6 +1086,8 @@ static void raid5_end_read_request(strucmdk_rdev_t *rdev; +atomic_dec (&conf->out_reqs_in_queue); +for (i=0 ; i<disks; i++)if (bi == &sh->dev[i].req)break ;
@@ -1153,6 +1167,8 @@ static void raid5_end_write_request(struint disks = sh->disks, i;int uptodate =test_bit (BIO_UPTODATE , &bi->bi_flags); +atomic_dec (&conf->out_reqs_in_queue); +for (i=0 ; i<disks; i++)if (bi == &sh->dev[i].req)break ;
@@ -2131,6 +2147,7 @@ static void handle_stripe_dirtying5(raidset_bit (R5_LOCKED , &dev->flags);set_bit (R5_Wantread, &dev->flags); s->locked++; +atomic_inc (&conf->reads_for_rmw); }else {set_bit (STRIPE_DELAYED , &sh->state);set_bit (STRIPE_HANDLE , &sh->state);
@@ -2154,6 +2171,7 @@ static void handle_stripe_dirtying5(raidset_bit (R5_LOCKED , &dev->flags);set_bit (R5_Wantread, &dev->flags); s->locked++; +atomic_inc (&conf->reads_for_rcw); }else {set_bit (STRIPE_DELAYED , &sh->state);set_bit (STRIPE_HANDLE , &sh->state);
@@ -2219,6 +2237,7 @@ static void handle_stripe_dirtying6(raidset_bit (R5_LOCKED , &dev->flags);set_bit (R5_Wantread, &dev->flags); s->locked++; +atomic_inc (&conf->reads_for_rcw); }else { pr_debug("Request delayed stripe %llu " "block %d for Reconstruct\n",
@@ -2556,6 +2575,8 @@ static bool handle_stripe5(struct stripeclear_bit (STRIPE_HANDLE , &sh->state);clear_bit (STRIPE_DELAYED , &sh->state); +atomic_inc (&conf->handle_called); + s.syncing =test_bit (STRIPE_SYNCING , &sh->state); s.expanding =test_bit (STRIPE_EXPAND_SOURCE , &sh->state); s.expanded =test_bit (STRIPE_EXPAND_READY , &sh->state);
@@ -2789,7 +2810,7 @@ static bool handle_stripe5(struct stripeops_run_io (sh, &s); -return_io (return_bi); +return_io (return_bi, conf);return blocked_rdev ==NULL ; }
@@ -2816,6 +2837,8 @@ static bool handle_stripe6(struct stripeclear_bit (STRIPE_HANDLE , &sh->state);clear_bit (STRIPE_DELAYED , &sh->state); +atomic_inc (&conf->handle_called); + s.syncing =test_bit (STRIPE_SYNCING , &sh->state); s.expanding =test_bit (STRIPE_EXPAND_SOURCE , &sh->state); s.expanded =test_bit (STRIPE_EXPAND_READY , &sh->state);
@@ -3011,7 +3034,7 @@ static bool handle_stripe6(struct stripeops_run_io (sh, &s); -return_io (return_bi); +return_io (return_bi, conf);return blocked_rdev ==NULL ; }
@@ -3039,6 +3062,7 @@ static void raid5_activate_delayed(raid5if (!test_and_set_bit (STRIPE_PREREAD_ACTIVE , &sh->state))atomic_inc (&conf->preread_active_stripes);list_add_tail (&sh->lru, &conf->hold_list); +atomic_dec (&conf->delayed); } }else blk_plug_device (conf->mddev->queue);
@@ -3217,6 +3241,7 @@ static void raid5_align_endio(struct bio raid_bi->bi_next =NULL ;rdev_dec_pending (rdev, conf->mddev); +atomic_dec (&conf->out_reqs_in_queue);if (!error && uptodate) {bio_endio (raid_bi,0 );
@@ -3265,6 +3290,7 @@ static int chunk_aligned_read(struct reqpr_debug ("chunk_aligned_read : non aligned\n" );return 0 ; } +atomic_inc (&conf->aligned_reads);/* * use bio_clone to make a copy of the bio */
@@ -3287,11 +3313,13 @@ static int chunk_aligned_read(struct req &pd_idx, conf); +atomic_dec (&conf->in_reqs_in_queue);rcu_read_lock (); rdev =rcu_dereference (conf->disks[dd_idx].rdev);if (rdev &&test_bit (In_sync, &rdev->flags)) {atomic_inc (&rdev->nr_pending);rcu_read_unlock (); +atomic_inc (&conf->reads_out); raid_bio->bi_next = (void *)rdev; align_bi->bi_bdev = rdev->bdev; align_bi->bi_flags &= ~(1 <<BIO_SEG_VALID );
@@ -3311,6 +3339,7 @@ static int chunk_aligned_read(struct reqatomic_inc (&conf->active_aligned_reads);spin_unlock_irq (&conf->device_lock); +atomic_inc (&conf->out_reqs_in_queue);generic_make_request (align_bi);return 1 ; }else {
@@ -3384,6 +3413,8 @@ static int make_request(struct request_qconst int rw =bio_data_dir (bi);int cpu, remaining; +atomic_inc (&conf->in_reqs_in_queue); +if (unlikely (bio_barrier (bi))) {bio_endio (bi, -EOPNOTSUPP );return 0 ;
@@ -3397,6 +3428,11 @@ static int make_request(struct request_q bio_sectors(bi));part_stat_unlock (); +if (rw ==WRITE ) +atomic_inc (&conf->writes_in); +else +atomic_inc (&conf->reads_in); +if (rw ==READ && mddev->reshape_position == MaxSector &&chunk_aligned_read (q,bi))
@@ -3508,6 +3544,7 @@ static int make_request(struct request_qif ( rw ==WRITE )md_write_end (mddev); +atomic_dec (&conf->in_reqs_in_queue);bio_endio (bi,0 ); }
@@ -3862,6 +3899,7 @@ static void raid5d(mddev_t *mddev)if (!ok)break ; handled++; +atomic_inc (&conf->handled_in_raid5d); } sh =__get_priority_stripe (conf);
@@ -3871,6 +3909,7 @@ static void raid5d(mddev_t *mddev)spin_unlock_irq (&conf->device_lock); handled++; +atomic_inc (&conf->handled_in_raid5d);handle_stripe (sh, conf->spare_page);release_stripe (sh);
@@ -4330,15 +4369,37 @@ static void status(struct seq_file *seq,raid5_conf_t *conf = (raid5_conf_t *) mddev->private;int i; -seq_printf (seq," level %d, %dk chunk, algorithm %d" , mddev->level, mddev->chunk_size >>10 , mddev->layout); -seq_printf (seq," [%d/%d] [" , conf->raid_disks, conf->raid_disks - mddev->degraded); +seq_printf (seq," level %d, %dk chunk, algorithm %d" , mddev->level, + mddev->chunk_size >>10 , mddev->layout); +seq_printf (seq," [%d/%d] [" , conf->raid_disks, + conf->raid_disks - mddev->degraded);for (i =0 ; i < conf->raid_disks; i++) -seq_printf (seq,"%s" , +seq_printf (seq,"%s" , conf->disks[i].rdev &&test_bit (In_sync, &conf->disks[i].rdev->flags) ?"U" :"_" ); -seq_printf (seq,"]" ); +seq_printf (seq,"]\n" ); +seq_printf (seq,"\tin: %u reads, %u writes; out: %u reads, %u writes\n" , +atomic_read (&conf->reads_in), +atomic_read (&conf->writes_in), +atomic_read (&conf->reads_out), +atomic_read (&conf->writes_out)); +seq_printf (seq,"\t%u in raid5d, %u out of stripes, %u handle called\n" , +atomic_read (&conf->handled_in_raid5d), +atomic_read (&conf->out_of_stripes), +atomic_read (&conf->handle_called)); +seq_printf (seq,"\treads: %u for rmw, %u for rcw, %u aligned,\n" , +atomic_read (&conf->reads_for_rmw), +atomic_read (&conf->reads_for_rcw), +atomic_read (&conf->aligned_reads)); +seq_printf (seq,"\t%u delayed, %u bit delayed, %u active, " , +atomic_read (&conf->delayed), +atomic_read (&conf->bit_delayed), +atomic_read (&conf->active_stripes)); +seq_printf (seq,"queues: %u in, %u out\n" , +atomic_read (&conf->in_reqs_in_queue), +atomic_read (&conf->out_reqs_in_queue));#ifdef DEBUG -seq_printf (seq,"\n" ); +seq_printf (seq,"\n" );printall (seq, conf);#endif }
include/linux/raid/raid5.h +20 -0
@@ -385,6 +385,26 @@ struct raid5_private_data {int pool_size;/* number of disks in stripeheads in pool */ spinlock_t device_lock;struct disk_info *disks; + +/* +* Stats +*/ +atomic_t reads_in; +atomic_t writes_in; +atomic_t reads_out; +atomic_t writes_out; +atomic_t handled_in_raid5d; +atomic_t out_of_stripes; +atomic_t reads_for_rmw; +atomic_t reads_for_rcw; +atomic_t aligned_reads; +atomic_t writes_zcopy; +atomic_t writes_copied; +atomic_t handle_called; +atomic_t delayed; +atomic_t bit_delayed; +atomic_t in_reqs_in_queue; +atomic_t out_reqs_in_queue; };typedef struct raid5_private_data raid5_conf_t ;
Re: [RFC patch 1/1] Track raid5/6 statistics.
Quoted 11 lines · On Wed, Nov 26, 2008 at 12:04 PM, <[email protected]> wrote:
This patch tracks various statistics related to the performance of a RAID 5 or 6 array. These have been useful to us in the past to help solve performance issues. They are reported via
/proc/mdstat.I realize that the format of the statistics may not be the best, and there may be a better location than
/proc/mdstat, so I welcome suggestions on where to put them.I will add documentation once we've decided on the format and location (or if nobody objects to the current format and location.)
Hi Jody,
I have done some work in this area too and have a lingering desire to hook MD up to the tracing infrastructure du jour. Have you considered using tracepoints [1]? This would save the overhead of atomic operations when we're not profiling.
Regards,
Dan
Re: [RFC patch 1/1] Track raid5/6 statistics.
Hi Dan,
Quoted 1 line · Dan Williams
I have done some work in this area too
Got any preliminary patches you can share?
Quoted 4 lines
and have a lingering desire to hook MD up to the tracing infrastructure du jour. Have you considered using tracepoints [1]? This would save the overhead of atomic operations when we're not profiling.
Thanks for the suggestion, I'll look into that.
Cheers,
Jody
Re: [RFC patch 1/1] Track raid5/6 statistics.
Quoted 8 lines · Jody McIntyre
Hi Dan,
Quoted 1 line · Dan Williams
I have done some work in this area too
Got any preliminary patches you can share?
A patch for streaming write performance. The get_priority_stripe [1] patch was the end result of a discussion and initial investigation of raid5 write performance [2] started by Raz Ben-Jehuda(caro). It helps, but is not a panacea...
--
Dan
[1]: http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=8b3e6cdc53b7f29f7026955d6cb6902a49322a15
[2]: http://marc.info/?l=linux-raid&m=117697457216290&w=2
Re: [RFC patch 1/1] Track raid5/6 statistics.
In reply to [email protected]Quoted 3 lines · On Wed, Nov 26, 2008 at 02:04:53PM -0500, [email protected] wrote:
I realize that the format of the statistics may not be the best, and there may be a better location than
/proc/mdstat, so I welcome suggestions on where to put them.
Are these values useful for mere mortals trying to create a RAID array best suited for a specific workload, or are these only useful for people hacking on the RAID code? If the former, then I'd suggest to use sysfs (something like /sys/block/<dev>/md/statistics, similar to /sys/class/net/<dev>/statistics). If the latter, then debugfs would be better.
Gabor
Signature · 4 lines
---------------------------------------------------------
MTA SZTAKI Computer and Automation Research Institute
Hungarian Academy of Sciences
---------------------------------------------------------Re: [RFC patch 1/1] Track raid5/6 statistics.
Hi Gabor,
Quoted 3 lines · Gabor Gombas
Are these values useful for mere mortals trying to create a RAID array best suited for a specific workload, or are these only useful for people hacking on the RAID code?
I believe the average administrator could understand what most (if not all) of the statistics mean, but doing something about it might require a developer or at least someone with a bit of RAID experience. In our use, we get customers to report the values to us then make tuning suggestions (more likely) or provide a patch.
Quoted 4 lines
If the former, then I'd suggest to use sysfs (something like /sys/block/<dev>/md/statistics, similar to /sys/class/net/<dev>/statistics). If the latter, then debugfs would be better.
I was thinking about your first suggestion but note that there is also /proc/diskstats as well as /sys/block/<dev>/stat. The advantage of having everything in one file is that you only have to open one file when gathering statistics on many arrays, and /proc/mdstat already exists as a precedent. I could add individual array statistics files as well if useful.
I'm also going to investigate tracepoints as suggested by Dan, so all this is subject to change :)
Cheers,
Jody
Quoted 8 lines
Gabor
Signature · 4 lines
--------------------------------------------------------- MTA SZTAKI Computer and Automation Research Institute Hungarian Academy of Sciences ---------------------------------------------------------
Re: [RFC patch 1/1] Track raid5/6 statistics.
Quoted 6 lines · Jody McIntyre
I was thinking about your first suggestion but note that there is also
/proc/diskstatsas well as /sys/block/<dev>/stat. The advantage of having everything in one file is that you only have to open one file when gathering statistics on many arrays, and/proc/mdstatalready exists as a precedent. I could add individual array statistics files as well if useful.
/proc contains a lot of legacy junk but nowadays the trend is that you should not add new files under /proc that are not process-related. Changing /proc/mdstat is IMHO out of question since it is part of the user visible ABI and breaking that is a big no-no. So if you want all info in a single file that pretty much leaves only debugfs.
Gabor
Signature · 4 lines
---------------------------------------------------------
MTA SZTAKI Computer and Automation Research Institute
Hungarian Academy of Sciences
---------------------------------------------------------Re: [RFC patch 1/1] Track raid5/6 statistics.
Quoted 2 lines · Gabor Gombas
/proc contains a lot of legacy junk but nowadays the trend is that you should not add new files under /proc that are not process-related.
Agreed. I'm not proposing that at all.
Quoted 3 lines
Changing
/proc/mdstatis IMHO out of question since it is part of the user visible ABI and breaking that is a big no-no. So if you want all info in a single file that pretty much leaves only debugfs.
AFAICT it was last changed on 2005-09-09 (appearing in 2.6.14). This suggests we can change it given a sufficiently good reason.
debugfs isn't a good idea since ordinary systems won't (or shouldn't) have it mounted. In case I wasn't clear: some of these statistics _are_ useful to administrators with sufficient RAID knowledge. For example, if "out of stripes" is large and you have lots of memory, increasing stripe_cache_size will likely be useful.
Cheers,
Jody
Quoted 7 lines
Gabor
Signature · 4 lines
--------------------------------------------------------- MTA SZTAKI Computer and Automation Research Institute Hungarian Academy of Sciences ---------------------------------------------------------
Re: [RFC patch 1/1] Track raid5/6 statistics.
Hello Jody (& All) ,
Quoted 14 lines · Jody McIntyre
Quoted 2 lines · Gabor Gombas
/proc contains a lot of legacy junk but nowadays the trend is that you should not add new files under /proc that are not process-related.
Agreed. I'm not proposing that at all.
Quoted 3 lines
Changing
/proc/mdstatis IMHO out of question since it is part of the user visible ABI and breaking that is a big no-no. So if you want all info in a single file that pretty much leaves only debugfs.AFAICT it was last changed on 2005-09-09 (appearing in 2.6.14). This suggests we can change it given a sufficiently good reason.
Quoted 8 lines
debugfs isn't a good idea since ordinary systems won't (or shouldn't) have it mounted. In case I wasn't clear: some of these statistics _are_ useful to administrators with sufficient RAID knowledge. For example, if "out of stripes" is large and you have lots of memory, increasing
stripe_cache_sizewill likely be useful.Cheers,
Jody
Seems as if changing /proc/mdstat might be problematic for all the
present tools that monitor status , of an array & the disks , using it .
Tho somewhere I remember seeing "howto extemd a /proc/..." saying
something about always "add to the end of a line any new data" & "never insert -before- present lines in those files" .
That said , /sys might actually be a convenient structure to place a
single file for these statistics . Try asking the /sys maintainer .
But , I'd like to further suggest that the creation criteria for this
file be a boot: time option Or of course the ubiquitous module can be installed .
Thus the file is not created with every boot unless grub/lilo configs
are modified , but just when the 'System Manager' needs to take a look .
Just $.02 , Twyl , JimL
Signature · 5 lines
+------------------------------------------------------------------+ | James W. Laferriere | System Techniques | Give me VMS | | Network&System Engineer | 2133 McCullam Ave | Give me Linux | | [email protected] | Fairbanks, AK. 99701 | only on AXP | +------------------------------------------------------------------+
Re: [RFC patch 1/1] Track raid5/6 statistics.
In reply to Jody McIntyreQuoted 10 lines · Jody McIntyre
Quoted 3 lines · Gabor Gombas
/proc contains a lot of legacy junk but nowadays the trend is that you should not add new files under /proc that are not process-related.
Agreed. I'm not proposing that at all.
Quoted 8 lines
Quoted 4 lines
Changing
/proc/mdstatis IMHO out of question since it is part of the user visible ABI and breaking that is a big no-no. So if you want all info in a single file that pretty much leaves only debugfs.AFAICT it was last changed on 2005-09-09 (appearing in 2.6.14). This suggests we can change it given a sufficiently good reason.
I have to think that providing information useful to a subset of users, while possibly breaking existing tools, is going to be a hard sell and probably indefensible, since your new stuff could as well go in a new file in /sys or even debugfs. I would rather not see you mix policy with the technical features of these patches. I'd rather not see you do anything which makes my fragile perl scripts break, either. ;-)
I think this is a step in the right direction, tuning an array is always time consuming, and I suspect that many of us settle for "better" rather than "optimal" just for that reason.
Signature · 3 lines
Bill Davidsen <[email protected]> "Woe unto the statesman who makes war without a reason that will still be valid when the war is over..." Otto von Bismark