Re: [PATCH v5 3/3] PCI/AER: Document that aer_recover_queue() takes ownership of aer_regs

[Kuppuswamy Sathyanarayanan]

Hi,

On 9/28/2026 10:40 AM, Priyank Rathod wrote:
> ghes_handle_aer() allocates the AER register snapshot that it passes to
> aer_recover_queue() from ghes_estatus_pool.  aer_recover_queue() returns
> void, so the caller cannot tell whether the record was queued, and the
> AER code owns the buffer from then on and must free it on every path.
> 
> None of this is documented at the definition of this exported function.
> With GHES enabled, a new caller that passed a buffer from any other
> allocator would hit the BUG() in gen_pool_free_owner() when the AER code
> returns the buffer to ghes_estatus_pool, and a caller that freed the
> buffer itself would cause a double free.
> 
> Add a kernel-doc comment that describes the parameters and states that
> aer_recover_queue() takes ownership of @aer_regs, which must have been
> allocated from ghes_estatus_pool.
> 
> No functional change.
> 

Thanks, this matches what I had in mind. 

Reviewed-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>

Bjorn, I see you already applied the series to pci/aer. Feel free to
pick up the tag if it is still convenient.

> Suggested-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/4513e7d4-4e2f-42d8-8f0c-2f0e03815dee@xxxxxxxxxxxxxxx
> Signed-off-by: Priyank Rathod <rathodpriyank@xxxxxxxxxx>
> ---
>  drivers/pci/pcie/aer.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index a6600801af6e..a58244e00bc4 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1404,6 +1404,24 @@ static void aer_recover_work_func(struct work_struct *work)
>  static DEFINE_SPINLOCK(aer_recover_ring_lock);
>  static DECLARE_WORK(aer_recover_work, aer_recover_work_func);
>  
> +/**
> + * aer_recover_queue - queue an AER error record reported by firmware
> + * @domain: PCI domain (segment) of the device that reported the error
> + * @bus: bus number of the device that reported the error
> + * @devfn: encoded device and function number, as returned by PCI_DEVFN()
> + * @severity: AER_CORRECTABLE, AER_NONFATAL or AER_FATAL
> + * @aer_regs: snapshot of the device's AER Capability registers
> + *
> + * Queue an error record received from firmware through APEI GHES.  The
> + * record is processed later from a workqueue, which logs the error and,
> + * for uncorrectable errors, attempts recovery of the device.
> + *
> + * Takes ownership of @aer_regs, which must have been allocated from
> + * ghes_estatus_pool with a size of sizeof(struct aer_capability_regs).
> + * The buffer is freed with ghes_estatus_pool_region_free() by the work
> + * item that processes the record, or immediately if the queue is full.
> + * The caller must not access or free @aer_regs after this call.
> + */
>  void aer_recover_queue(int domain, unsigned int bus, unsigned int devfn,
>  		       int severity, struct aer_capability_regs *aer_regs)
>  {
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer