Page MenuHomeFreeBSD

Add function to OSD to get values without taking the lock.
AcceptedPublic

Authored by stevek on Apr 4 2024, 12:18 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, May 20, 4:14 PM
Unknown Object (File)
Sat, Apr 27, 1:03 PM
Unknown Object (File)
Sat, Apr 27, 1:03 PM
Unknown Object (File)
Sat, Apr 27, 11:54 AM
Unknown Object (File)
Fri, Apr 26, 8:35 PM
Unknown Object (File)
Fri, Apr 26, 4:45 AM
Unknown Object (File)
Apr 20 2024, 7:45 PM
Unknown Object (File)
Apr 7 2024, 9:02 PM
Subscribers

Details

Reviewers
markj
Summary

There are some cases of OSD use where the value is only initialized once
at a point where successive access of the value can be done so safely
without the need to take the lock.

Obtained from: Juniper Networks, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 57717
Build 54605: arc lint + arc unit

Event Timeline

Some uses of OSD may do so in a way where the value is only set once at a point where successive get of the value can be done so safely without the need to take the lock.

I think this sentence is not quite right. :)

sys/kern/kern_osd.c
281

osd_get_unlocked() would be more consistent with this kind of pattern elsewhere in the tree.

291

Changed from _nolock to _unlocked in naming and updated commit log message.

sys/kern/kern_osd.c
291

Assuming you just didn't see it, I think this suggestion should still be applied. Or is there some reason not to?

sys/kern/kern_osd.c
291

I 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?

sys/kern/kern_osd.c
291

It'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.

Use atomic to read OSD slot data

This revision is now accepted and ready to land.Fri, May 17, 2:27 PM