Hi,
The SLIST_REMOVE within a SLIST_FOREACH loop without SAFE looks
scary. The old code removed the entries from the root link ony by
one. This can be done in a single step.
Replace some manual loops with SLIST macros and remove unnecessary
code.
I am running this with softraid crypto, could someone with raid 0
or 1 try it?
ok?
bluhm
Index: dev/softraid.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/softraid.c,v
retrieving revision 1.350
diff -u -p -r1.350 softraid.c
--- dev/softraid.c 14 Mar 2015 03:38:46 -0000 1.350
+++ dev/softraid.c 28 Mar 2015 10:30:55 -0000
@@ -263,8 +263,7 @@ sr_meta_attach(struct sr_discipline *sd,
goto bad;
/* Force chunks into correct order now that metadata is attached. */
- SLIST_FOREACH(ch_entry, cl, src_link)
- SLIST_REMOVE(cl, ch_entry, sr_chunk, src_link);
+ SLIST_INIT(cl);
for (i = 0; i < chunk_no; i++) {
ch_entry = sd->sd_vol.sv_chunks[i];
chunk2 = NULL;
@@ -1137,7 +1136,7 @@ sr_boot_assembly(struct sr_softc *sc)
struct sr_boot_volume_head bvh;
struct sr_boot_chunk_head bch, kdh;
struct sr_boot_volume *bv, *bv1, *bv2;
- struct sr_boot_chunk *bc, *bcnext, *bc1, *bc2;
+ struct sr_boot_chunk *bc, *bc1, *bc2;
struct sr_disk_head sdklist;
struct sr_disk *sdk;
struct disk *dk;
@@ -1201,10 +1200,9 @@ sr_boot_assembly(struct sr_softc *sc)
/*
* Create a list of volumes and associate chunks with each volume.
*/
- for (bc = SLIST_FIRST(&bch); bc != NULL; bc = bcnext) {
-
- bcnext = SLIST_NEXT(bc, sbc_link);
- SLIST_REMOVE(&bch, bc, sr_boot_chunk, sbc_link);
+ while (!SLIST_EMPTY(&bch)) {
+ bc = SLIST_FIRST(&bch);
+ SLIST_REMOVE_HEAD(&bch, sbc_link);
bc->sbc_chunk_id = bc->sbc_metadata->ssdi.ssd_chunk_id;
/* Handle key disks separately. */
@@ -1456,11 +1454,8 @@ sr_boot_assembly(struct sr_softc *sc)
/* done with metadata */
unwind:
/* Free boot volumes and associated chunks. */
- for (bv1 = SLIST_FIRST(&bvh); bv1 != NULL; bv1 = bv2) {
- bv2 = SLIST_NEXT(bv1, sbv_link);
- for (bc1 = SLIST_FIRST(&bv1->sbv_chunks); bc1 != NULL;
- bc1 = bc2) {
- bc2 = SLIST_NEXT(bc1, sbc_link);
+ SLIST_FOREACH_SAFE(bv1, &bvh, sbv_link, bv2) {
+ SLIST_FOREACH_SAFE(bc1, &bv1->sbv_chunks, sbc_link, bc2) {
if (bc1->sbc_metadata)
free(bc1->sbc_metadata, M_DEVBUF, 0);
free(bc1, M_DEVBUF, 0);
@@ -1468,15 +1463,13 @@ unwind:
free(bv1, M_DEVBUF, 0);
}
/* Free keydisks chunks. */
- for (bc1 = SLIST_FIRST(&kdh); bc1 != NULL; bc1 = bc2) {
- bc2 = SLIST_NEXT(bc1, sbc_link);
+ SLIST_FOREACH_SAFE(bc1, &kdh, sbc_link, bc2) {
if (bc1->sbc_metadata)
free(bc1->sbc_metadata, M_DEVBUF, 0);
free(bc1, M_DEVBUF, 0);
}
/* Free unallocated chunks. */
- for (bc1 = SLIST_FIRST(&bch); bc1 != NULL; bc1 = bc2) {
- bc2 = SLIST_NEXT(bc1, sbc_link);
+ SLIST_FOREACH_SAFE(bc1, &bch, sbc_link, bc2) {
if (bc1->sbc_metadata)
free(bc1->sbc_metadata, M_DEVBUF, 0);
free(bc1, M_DEVBUF, 0);
@@ -1662,10 +1655,7 @@ sr_meta_native_attach(struct sr_discipli
/* mixed metadata versions; mark bad disks offline */
if (old_meta) {
d = 0;
- for (ch_entry = SLIST_FIRST(cl); ch_entry != NULL;
- ch_entry = ch_next, d++) {
- ch_next = SLIST_NEXT(ch_entry, src_link);
-
+ SLIST_FOREACH_SAFE(ch_entry, cl, src_link, ch_next) {
/* XXX do we want to read this again? */
if (ch_entry->src_dev_mm == NODEV)
panic("src_dev_mm == NODEV");
@@ -1675,6 +1665,7 @@ sr_meta_native_attach(struct sr_discipli
if (md->ssd_ondisk != version)
sd->sd_vol.sv_chunks[d]->src_meta.scm_status =
BIOC_SDOFFLINE;
+ d++;
}
}
@@ -1751,8 +1742,6 @@ sr_hotplug_unregister(struct sr_discipli
SLIST_REMOVE(&sr_hotplug_callbacks, mhe,
sr_hotplug_list, shl_link);
free(mhe, M_DEVBUF, 0);
- if (SLIST_EMPTY(&sr_hotplug_callbacks))
- SLIST_INIT(&sr_hotplug_callbacks);
return;
}
}
@@ -3769,9 +3758,7 @@ sr_chunks_unwind(struct sr_softc *sc, st
if (!cl)
return;
- for (ch_entry = SLIST_FIRST(cl); ch_entry != NULL; ch_entry = ch_next) {
- ch_next = SLIST_NEXT(ch_entry, src_link);
-
+ SLIST_FOREACH_SAFE(ch_entry, cl, src_link, ch_next) {
DNPRINTF(SR_D_IOCTL, "%s: sr_chunks_unwind closing: %s\n",
DEVNAME(sc), ch_entry->src_devname);
if (ch_entry->src_vn) {
@@ -3796,7 +3783,6 @@ sr_discipline_free(struct sr_discipline
{
struct sr_softc *sc;
struct sr_discipline *sdtmp1, *sdtmp2;
- struct sr_meta_opt_head *som;
struct sr_meta_opt_item *omi, *omi_next;
if (!sd)
@@ -3816,9 +3802,7 @@ sr_discipline_free(struct sr_discipline
if (sd->sd_meta_foreign)
free(sd->sd_meta_foreign, M_DEVBUF, 0);
- som = &sd->sd_meta_opt;
- for (omi = SLIST_FIRST(som); omi != NULL; omi = omi_next) {
- omi_next = SLIST_NEXT(omi, omi_link);
+ SLIST_FOREACH_SAFE(omi, &sd->sd_meta_opt, omi_link, omi_next) {
if (omi->omi_som)
free(omi->omi_som, M_DEVBUF, 0);
free(omi, M_DEVBUF, 0);
Index: dev/softraid_crypto.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/dev/softraid_crypto.c,v
retrieving revision 1.117
diff -u -p -r1.117 softraid_crypto.c
--- dev/softraid_crypto.c 14 Mar 2015 03:38:46 -0000 1.117
+++ dev/softraid_crypto.c 28 Mar 2015 10:30:55 -0000
@@ -879,8 +879,7 @@ sr_crypto_read_key_disk(struct sr_discip
open = 0;
done:
- for (omi = SLIST_FIRST(&som); omi != NULL; omi = omi_next) {
- omi_next = SLIST_NEXT(omi, omi_link);
+ SLIST_FOREACH_SAFE(omi, &som, omi_link, omi_next) {
if (omi->omi_som)
free(omi->omi_som, M_DEVBUF, 0);
free(omi, M_DEVBUF, 0);
