Page MenuHomeFreeBSD

pbuf_ctor(): Stop using LK_NOWAIT, use LK_NOWITNESS
ClosedPublic

Authored by rlibby on May 25 2024, 7:45 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Jun 12, 9:18 PM
Unknown Object (File)
Mon, Jun 3, 10:14 PM
Unknown Object (File)
May 27 2024, 6:57 PM
Unknown Object (File)
May 26 2024, 7:08 PM
Subscribers

Details

Summary

The LK_NOWAIT was added to suppress a witness warning, but LK_NOWITNESS
is more what we mean. This makes pbuf_ctor() more consistent with
buf_alloc(), although, unlike buf_alloc(), for pbuf there should not be
any danger of a wild locker relying on the type stability of the buf to
attempt a lock. That is, this is essentially cosmetic.

Relevant history:

Sponsored by: Dell EMC Isilon

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

I'm fine with this change as it semantically makes sense and mimics original behavior of pbuf_ctor()

...that and LK_NOWAIT, in theory, could return an unhandled error from pbuf_ctor()

This revision is now accepted and ready to land.May 25 2024, 7:28 PM

LK_NOWAIT is the common pattern for this situation. Besides buffers, another immediate example is the vm_map lock in vmspace_fork() (look for the new_map locking and comment).

In D45360#1034889, @kib wrote:

LK_NOWAIT is the common pattern for this situation. Besides buffers, another immediate example is the vm_map lock in vmspace_fork() (look for the new_map locking and comment).

Thanks for the pointer. Yeah, that case makes sense. We don't really have a more expressive API there like we do with lockmgr's flags. I wonder if we might want a way to say explicitly that this is a lock that no one else can know about, e.g. LK_INITIAL, that would avoid reporting a LOR when taken second, but also can KASSERT that the lock was not contended.

In D45360#1034889, @kib wrote:

LK_NOWAIT is the common pattern for this situation. Besides buffers, another immediate example is the vm_map lock in vmspace_fork() (look for the new_map locking and comment).

Thanks for the pointer. Yeah, that case makes sense. We don't really have a more expressive API there like we do with lockmgr's flags. I wonder if we might want a way to say explicitly that this is a lock that no one else can know about, e.g. LK_INITIAL, that would avoid reporting a LOR when taken second, but also can KASSERT that the lock was not contended.

This is an interesting proposal, I like it.