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

[Omar Elghoul <oelghoul@linux.ibm.com>]

On 9/23/26 6:20 PM, Matthew Rosato wrote:
> 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@linux.ibm.com>
> 
> [...]
> 
>> +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.

Sure, how's this for a new commit message? Hopefully this is clearer and
doesn't read as upside down. I also added a blurb about firmware
implicitly disabling FMB when we disable the device.

"Don't free the FMB buffer when disabling measurement in
zpci_fmb_disable_device(). Instead, make the buffer persistent for the
lifetime of the device and reuse it across enable/disable cycles. Defer
freeing the buffer until teardown in zpci_release_device().

To support the persistent buffers, add the fmb_enabled field to struct
zpci_dev to decouple whether FMB is enabled from whether the buffer has
been allocated. Audit the only consumer of zdev->fmb as a liveness check
and update it to reflect this change.

Introduce the function zpci_fmb_reenable_device() to ensure that the FMB
is enabled. If it was already enabled, disable it, zero the counters,
and re-enable it. This allows the function to be used in both first-time
enabling and re-enabling measurement. Call it in zpci_reenable_device()
to preserve the FMB enablement if it had been implicitly disabled by
firmware in zpci_disable_device()."