| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Cc: | Andres Freund <andres(at)anarazel(dot)de> |
| Subject: | Remove obsolete locking comment from AddBufferToRing() |
| Date: | 2026-10-16 08:10:05 |
| Message-ID: | 17F7E4A7-AE49-4F2F-B4C7-57107CBB83F1@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
While working on another patch, I spotted this stale comment on AddBufferToRing():
```
/*
* AddBufferToRing -- add a buffer to the buffer ring
*
* Caller must hold the buffer header spinlock on the buffer. Since this
* is called with the spinlock held, it had better be quite cheap.
*/
static void
AddBufferToRing(BufferAccessStrategy strategy, BufferDesc *buf)
```
It says the caller must hold the buffer header spinlock, but this is no longer true. The only caller of AddBufferToRing() is StrategyGetBuffer(), which pins the buffer without acquiring the buffer header lock.
This seems to be an oversight in 5e899859287. The commit explicitly removed buffer header locking from StrategyGetBuffer() and updated its comment as:
```
- * To ensure that no one else can pin the buffer before we do, we must
- * return the buffer with the buffer header spinlock still held.
+ * It is the callers responsibility to ensure the buffer ownership can be
+ * tracked via TrackNewBufferPin().
```
In the attached patch, I just deleted the stale paragraph without adding anything.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Remove-obsolete-locking-comment-from-AddBufferToR.patch | application/octet-stream | 1.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | jian he | 2026-10-16 08:16:17 | Re: [PG19] Three bugs with a CHECK constraint that only the child enforces |
| Previous Message | David Rowley | 2026-10-16 08:08:39 | Re: Weird usages of sentinel_ok() |