On Wed, Sep 09, 2026 at 05:56:15PM +0200, Dariusz Swiderski wrote:
> A softraid mirror rebuild on a volume with 4096-byte sectors fails at
> approximately 12% with:
>
> softraid0: could not create read io
>
> sr_rebuild() maintains lba and sz variables in 512-byte DEV_BSIZE units but
> places these values directly into READ(16) and WRITE(16) CDBs.
> sr_validate_io() subsequently interprets the CDB values as native-sector
> units and multiplies them by ssd_secsize / DEV_BSIZE.
>
> With 4096-byte sectors, the effective offset is therefore multiplied by
> eight. Validation reaches the end of the volume after roughly 1/8 (12.5%) of
> the rebuild and rejects the read request.
> The CDB LBA and transfer length should be divided by:
>
> ssd_secsize / DEV_BSIZE
>
> before being encoded. The issue affects both RAID1 and RAID1C because they
> share the generic sr_rebuild() implementation.
> A patched 7.9-current GENERIC.MP got past the 12% mark without issues.
>
This looks sensible to me. I just wonder if it whould not be better to
track lba and sz in ssd_secsize / DEV_BSIZE units?
> diff --git a/sys/dev/softraid.c b/sys/dev/softraid.c
> --- a/sys/dev/softraid.c
> +++ b/sys/dev/softraid.c
> @@ -4677,13 +4677,16 @@ sr_rebuild(struct sr_discipline *sd)
> {
> struct sr_softc *sc = sd->sd_sc;
> u_int64_t sz, whole_blk, partial_blk, blk, restart;
> + u_int32_t blks_per_sec;
> daddr_t lba;
> struct sr_workunit *wu_r, *wu_w;
> struct scsi_xfer xs_r, xs_w;
> struct scsi_rw_16 *cr, *cw;
> int c, s, slept, percent = 0, old_percent = -1;
> u_int8_t *buf;
>
> + blks_per_sec = sd->sd_meta->ssdi.ssd_secsize / DEV_BSIZE;
> +
> whole_blk = sd->sd_meta->ssdi.ssd_size / SR_REBUILD_IO_SIZE;
> partial_blk = sd->sd_meta->ssdi.ssd_size % SR_REBUILD_IO_SIZE;
>
> @@ -4736,15 +4739,19 @@ sr_rebuild(struct sr_discipline *sd)
> xs_r.cmdlen = sizeof(*cr);
> cr = (struct scsi_rw_16 *)&xs_r.cmd;
> cr->opcode = READ_16;
> - _lto4b(sz, cr->length);
> - _lto8b(lba, cr->addr);
> + /*
> + * lba and sz are in DEV_BSIZE blocks, while SCSI CDB
> addresses
> + * and lengths are in logical sectors.
> + */
> + _lto4b(sz / blks_per_sec, cr->length);
> + _lto8b(lba / blks_per_sec, cr->addr);
> wu_r->swu_state = SR_WU_CONSTRUCT;
> wu_r->swu_flags |= SR_WUF_REBUILD;
> wu_r->swu_xs = &xs_r;
> if (sd->sd_scsi_rw(wu_r)) {
> printf("%s: could not create read io\n",
> DEVNAME(sc));
> - goto fail;
> + goto fail_wu;
> }
>
> /* setup write io */
> @@ -4756,15 +4763,15 @@ sr_rebuild(struct sr_discipline *sd)
> xs_w.cmdlen = sizeof(*cw);
> cw = (struct scsi_rw_16 *)&xs_w.cmd;
> cw->opcode = WRITE_16;
> - _lto4b(sz, cw->length);
> - _lto8b(lba, cw->addr);
> + _lto4b(sz / blks_per_sec, cw->length);
> + _lto8b(lba / blks_per_sec, cw->addr);
> wu_w->swu_state = SR_WU_CONSTRUCT;
> wu_w->swu_flags |= SR_WUF_REBUILD | SR_WUF_WAKEUP;
> wu_w->swu_xs = &xs_w;
> if (sd->sd_scsi_rw(wu_w)) {
> printf("%s: could not create write io\n",
> DEVNAME(sc));
> - goto fail;
> + goto fail_wu;
> }
>
> /*
> @@ -4826,6 +4833,10 @@ abort:
> if (sr_meta_save(sd, SR_META_DIRTY))
> printf("%s: could not save metadata to %s\n",
> DEVNAME(sc), sd->sd_meta->ssd_devname);
> + goto fail;
> +fail_wu:
> + sr_scsi_wu_put(sd, wu_r);
> + sr_scsi_wu_put(sd, wu_w);
> fail:
> dma_free(buf, SR_REBUILD_IO_SIZE << DEV_BSHIFT);
> }
>
--
:wq Claudio
