> On 10 Sep 2026, at 13:29, Dariusz Swiderski <[email protected]> wrote:
>
>
>
>> On 10 Sep 2026, at 13:12, Claudio Jeker <[email protected]> wrote:
>>
>> 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?
>
> I’ve tried to do a minimalistic change, but let me rewrite it :)
>
>>
>>> 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
>
>
Attached a rewrite. I have not tested it yet, as I’m lacking free 4k drives atm.
I will be able to test it over the weekend.
greets
Dariusz Swiderski
--
A mouse is a device to select which xterm to type into
diff --git a/sys/dev/softraid.c b/sys/dev/softraid.c
--- a/sys/dev/softraid.c
+++ b/sys/dev/softraid.c
@@ -4678,12 +4678,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, secs_per_io;
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;
+ secs_per_io = SR_REBUILD_IO_SIZE / blks_per_sec;
whole_blk = sd->sd_meta->ssdi.ssd_size / SR_REBUILD_IO_SIZE;
partial_blk = sd->sd_meta->ssdi.ssd_size % SR_REBUILD_IO_SIZE;
@@ -4712,12 +4716,12 @@ sr_rebuild(struct sr_discipline *sd)
/* currently this is 64k therefore we can use dma_alloc */
buf = dma_alloc(SR_REBUILD_IO_SIZE << DEV_BSHIFT, PR_WAITOK);
for (blk = restart; blk <= whole_blk; blk++) {
- lba = blk * SR_REBUILD_IO_SIZE;
- sz = SR_REBUILD_IO_SIZE;
+ lba = blk * secs_per_io;
+ sz = secs_per_io;
if (blk == whole_blk) {
if (partial_blk == 0)
break;
- sz = partial_blk;
+ sz = partial_blk / blks_per_sec;
}
/* get some wu */
@@ -4731,7 +4735,7 @@ sr_rebuild(struct sr_discipline *sd)
bzero(&xs_r, sizeof xs_r);
xs_r.error = XS_NOERROR;
xs_r.flags = SCSI_DATA_IN;
- xs_r.datalen = sz << DEV_BSHIFT;
+ xs_r.datalen = sz * sd->sd_meta->ssdi.ssd_secsize;
xs_r.data = buf;
xs_r.cmdlen = sizeof(*cr);
cr = (struct scsi_rw_16 *)&xs_r.cmd;
@@ -4751,7 +4755,7 @@ sr_rebuild(struct sr_discipline *sd)
bzero(&xs_w, sizeof xs_w);
xs_w.error = XS_NOERROR;
xs_w.flags = SCSI_DATA_OUT;
- xs_w.datalen = sz << DEV_BSHIFT;
+ xs_w.datalen = sz * sd->sd_meta->ssdi.ssd_secsize;
xs_w.data = buf;
xs_w.cmdlen = sizeof(*cw);
cw = (struct scsi_rw_16 *)&xs_w.cmd;
@@ -4796,7 +4800,7 @@ sr_rebuild(struct sr_discipline *sd)
sr_scsi_wu_put(sd, wu_r);
sr_scsi_wu_put(sd, wu_w);
- sd->sd_meta->ssd_rebuild = lba;
+ sd->sd_meta->ssd_rebuild = lba * blks_per_sec;
/* XXX - this should be based on size, not percentage. */
/* save metadata every percent */
