Hello,
I think I've found a bug on software RAID1 implementation of handling
write errors. IMHO code should check if every write to every chunk
succeed. If not, then there is an error which it needs to handle.
Proposed patch handles such error by off-lining the problematic drive.
The patch compiles on current, but so far it's not tested at all -- I
send it to get some advice from developers if this is the way to go or
not and if so, then how exactly proceed with patch send etc.
Thanks,
Karel
Index: sys/dev/softraid_raid1.c
===================================================================
RCS file: /cvs/src/sys/dev/softraid_raid1.c,v
retrieving revision 1.60
diff -u -r1.60 softraid_raid1.c
--- sys/dev/softraid_raid1.c 27 Jan 2015 10:12:45 -0000 1.60
+++ sys/dev/softraid_raid1.c 10 Jul 2015 17:56:36 -0000
@@ -415,14 +415,28 @@
{
struct sr_discipline *sd = wu->swu_dis;
struct scsi_xfer *xs = wu->swu_xs;
+ int chunk_no;
+ int succeeding_ios;
+ struct sr_ccb *ccb_error;
- /* If at least one I/O succeeded, we are okay. */
- if (wu->swu_ios_succeeded > 0) {
+ chunk_no = sd->sd_meta->ssdi.ssd_chunk_no;
+ succeeding_ios = wu->swu_ios_succeeded;
+
+ /*
+ * In case of reads, we're OK when exactly one I/O succeeds,
+ * in case of writes we need to have all write I/Os succeeding
+ * since otherwise we may end up with reading from the failed
+ * write in the future.
+ */
+ if ((xs->flags & SCSI_DATA_IN) && succeeding_ios == 1) {
xs->error = XS_NOERROR;
return SR_WU_OK;
}
-
- /* If all I/O failed, retry reads and give up on writes. */
+ if ((xs->flags & SCSI_DATA_OUT) && succeeding_ios == chunk_no) {
+ xs->error = XS_NOERROR;
+ return SR_WU_OK;
+ }
+ /* Something failed here so retry reads and give up on writes. */
if (xs->flags & SCSI_DATA_IN) {
printf("%s: retrying read on block %lld\n",
sd->sd_meta->ssd_devname, (long long)wu->swu_blk_start);
@@ -436,6 +450,13 @@
} else {
printf("%s: permanently failing write on block %lld\n",
sd->sd_meta->ssd_devname, (long long)wu->swu_blk_start);
+ TAILQ_FOREACH(ccb_error, &wu->swu_ccb, ccb_link) {
+ if (ccb_error->ccb_state == SR_CCB_FAILED) {
+ printf("%s: chunk: %d going off-line
due to write error.\n",
+ sd->sd_meta->ssd_devname,
ccb_error->ccb_target);
+ sr_raid1_set_chunk_state(sd,
ccb_error->ccb_target, BIOC_SDOFFLINE);
+ }
+ }
}
wu->swu_state = SR_WU_FAILED;
