Re: [PATCH v7 2/4] s390/pci: Reuse FMB buffer and preserve state in device re-enablement

[Matthew Rosato]

On 9/22/26 3:51 PM, Omar Elghoul wrote:
> Introduce the function zpci_fmb_reenable_device() that checks the state
> of function measurement and ensures it is enabled. Reset the counters to
> zero, disable, and re-enable the FMB if it was already enabled. Call
> this function from zpci_reenable_device().
> 
> Don't free the FMB buffer during disabling and reuse it when re-enabling
> measurement. Instead, free the buffer upon device teardown, allowing the
> same buffer to be reused in the enable path and add the bit fmb_enabled
> to struct zpci_dev. Audit the only consumer of zdev->fmb and update it
> to reflect the change in semantics.
> 
> Signed-off-by: Omar Elghoul <oelghoul@xxxxxxxxxxxxx>

[...]

> +int zpci_fmb_reenable_device(struct zpci_dev *zdev)
> +{
> +	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
> +	struct zpci_fib fib = {0};
> +	u8 cc, status;
> +
> +	lockdep_assert_held(&zdev->fmb_lock);
> +
> +	if (!zdev->fmb_enabled)
> +		return zpci_fmb_enable_device(zdev);

[...]

> +	if (zdev->fmb_enabled)
> +		zpci_fmb_reenable_device(zdev);
>  
Besides Gerd's comments, I was looking at this patch in isolation and
this combination made me wonder why you were adding what appears to be
dead code (of course, patch 4 adds another caller that doesn't check
zdev->fmb_enabled before calling)

Maybe you could add a bit to the commit message besides 'Call
> this function from zpci_reenable_device().' to indicate that this
function is also being setup for future re-use where we might be going
disabled->enabled rather than enabled->disabled->enabled.