Changeset View
Changeset View
Standalone View
Standalone View
sys/kern/kern_osd.c
| Show First 20 Lines • Show All 272 Lines • ▼ Show 20 Lines | |||||||||
| osd_free_reserved(void **rsv) | osd_free_reserved(void **rsv) | ||||||||
| { | { | ||||||||
| OSD_DEBUG("Discarding reserved slot array."); | OSD_DEBUG("Discarding reserved slot array."); | ||||||||
| free(rsv, M_OSD); | free(rsv, M_OSD); | ||||||||
| } | } | ||||||||
| void * | void * | ||||||||
| osd_get(u_int type, struct osd *osd, u_int slot) | osd_get_unlocked(u_int type, struct osd *osd, u_int slot) | ||||||||
markj: `osd_get_unlocked()` would be more consistent with this kind of pattern elsewhere in the tree. | |||||||||
| { | { | ||||||||
| struct rm_priotracker tracker; | |||||||||
| void *value; | void *value; | ||||||||
| KASSERT(type >= OSD_FIRST && type <= OSD_LAST, ("Invalid type.")); | |||||||||
| KASSERT(slot > 0, ("Invalid slot.")); | |||||||||
| rm_rlock(&osdm[type].osd_object_lock, &tracker); | |||||||||
| KASSERT(osdm[type].osd_destructors[slot - 1] != NULL, ("Unused slot.")); | KASSERT(osdm[type].osd_destructors[slot - 1] != NULL, ("Unused slot.")); | ||||||||
| if (slot > osd->osd_nslots) { | if (slot > osd->osd_nslots) { | ||||||||
| value = NULL; | value = NULL; | ||||||||
| OSD_DEBUG("Slot doesn't exist (type=%u, slot=%u).", type, slot); | OSD_DEBUG("Slot doesn't exist (type=%u, slot=%u).", type, slot); | ||||||||
| } else { | } else { | ||||||||
| value = osd->osd_slots[slot - 1]; | value = atomic_load_ptr(&osd->osd_slots[slot - 1]); | ||||||||
Not Done Inline Actions
markj: | |||||||||
Not Done Inline ActionsAssuming you just didn't see it, I think this suggestion should still be applied. Or is there some reason not to? markj: Assuming you just didn't see it, I think this suggestion should still be applied. Or is there… | |||||||||
Done Inline ActionsI probably would not be necessary in the cases that I have seen where unlocked access makes sense. However, I suppose one could do this for some of the potential cases, but wouldn't atomic_store_ptr be necessary also in the osd_set_reserved() function if doing so? stevek: I probably would not be necessary in the cases that I have seen where unlocked access makes… | |||||||||
Not Done Inline ActionsIt's not strictly necessary. Adding the atomic qualifier here 1) helpers readers understand that this isn't a normal, synchronized memory access, 2) provides more information to concurrency sanitizers to help avoid reporting false positives. I think it would make sense to modify osd_set_reserved() too. markj: It's not strictly necessary. Adding the atomic qualifier here 1) helpers readers understand… | |||||||||
| OSD_DEBUG("Returning slot value (type=%u, slot=%u, value=%p).", | OSD_DEBUG("Returning slot value (type=%u, slot=%u, value=%p).", | ||||||||
| type, slot, value); | type, slot, value); | ||||||||
| } | } | ||||||||
| return (value); | |||||||||
| } | |||||||||
| void * | |||||||||
| osd_get(u_int type, struct osd *osd, u_int slot) | |||||||||
| { | |||||||||
| struct rm_priotracker tracker; | |||||||||
| void *value; | |||||||||
| KASSERT(type >= OSD_FIRST && type <= OSD_LAST, ("Invalid type.")); | |||||||||
| KASSERT(slot > 0, ("Invalid slot.")); | |||||||||
| rm_rlock(&osdm[type].osd_object_lock, &tracker); | |||||||||
| value = osd_get_unlocked(type, osd, slot); | |||||||||
| rm_runlock(&osdm[type].osd_object_lock, &tracker); | rm_runlock(&osdm[type].osd_object_lock, &tracker); | ||||||||
| return (value); | return (value); | ||||||||
| } | } | ||||||||
| void | void | ||||||||
| osd_del(u_int type, struct osd *osd, u_int slot) | osd_del(u_int type, struct osd *osd, u_int slot) | ||||||||
| { | { | ||||||||
| struct rm_priotracker tracker; | struct rm_priotracker tracker; | ||||||||
| ▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines | |||||||||
osd_get_unlocked() would be more consistent with this kind of pattern elsewhere in the tree.