Re: Remove obsolete locking comment from AddBufferToRing()

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Pierre Forstmann <pierre(dot)forstmann(at)gmail(dot)com>
Subject: Re: Remove obsolete locking comment from AddBufferToRing()
Date: 2026-11-05 15:10:43
Message-ID: 0b63ea5a-0742-4b34-8e72-21e7da277d6a@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 16/10/2026 11:10, Chao Li wrote:
> 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.

Committed, thanks

- Heikki

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2026-11-05 15:24:00 Re: file_fdw: Fix ANALYZE does not enforce REJECT_LIMIT with on_error 'ignore'
Previous Message Tomas Vondra 2026-11-05 14:57:50 Re: Fix BRIN numeric_minmax_multi_ops out of range value range merging