pbuf_ctor(): Stop using LK_NOWAIT, use LK_NOWITNESS
ClosedPublic
Actions

Authored by rlibby on May 25 2024, 7:45 AM.

Details

Reviewers

kib
rew

Commits

rG9c975a0d90f1: pbuf_ctor(): Stop using LK_NOWAIT, use LK_NOWITNESS

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:

531f8cfea06b Use dedicated lock name for pbufs
5875b94c7493 buf_alloc(): lock the buffer with LK_NOWAIT
c9e023541aef pbuf_ctor(): lock the buffer with LK_NOWAIT
1fb00c8f1060 buf_alloc(): Stop using LK_NOWAIT, use LK_NOWITNESS

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 57911
Build 54799: arc lint + arc unit

Event Timeline

rlibby created this revision.May 25 2024, 7:45 AM

Herald added a subscriber: imp. · View Herald TranscriptMay 25 2024, 7:45 AM

rlibby requested review of this revision.May 25 2024, 7:45 AM

Harbormaster completed remote builds in B57911: Diff 139076.May 25 2024, 7:45 AM

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.

Closed by commit rG9c975a0d90f1: pbuf_ctor(): Stop using LK_NOWAIT, use LK_NOWITNESS (authored by rlibby). · Explain WhyMay 26 2024, 5:37 PM

This revision was automatically updated to reflect the committed changes.

rlibby added a commit: rG9c975a0d90f1: pbuf_ctor(): Stop using LK_NOWAIT, use LK_NOWITNESS.

In D45360#1034898, @rlibby wrote:

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.

Revision Contents
Changeset List

Path

Size

sys/

vm/

vm_pager.c

2 lines

Diff 139076

View Options

pbuf_ctor(): Stop using LK_NOWAIT, use LK_NOWITNESSClosedPublicActions