Re: [PATCH v6 3/7] configfs-tsm: Introduce a shared ABI for attestation reports

[Dan Williams <dan.j.williams@intel.com>]

Alexey Kardashevskiy wrote:
[..]
> >>> +			       size_t count, enum tsm_data_select select)
> >>> +{
> >>> +	struct tsm_report_state *state = to_state(report);
> >>> +	const struct tsm_ops *ops;
> >>> +	ssize_t rc;
> >>> +
> >>> +	/* try to read from the existing report if present and valid... */
> >>> +	rc = read_cached_report(report, buf, count, select);
> >>> +	if (rc >= 0 || rc != -EWOULDBLOCK)
> >>> +		return rc;
> >>> +
> >>> +	/* slow path, report may need to be regenerated... */
> >>> +	guard(rwsem_write)(&tsm_rwsem);
> >>> +	ops = provider.ops;
> >>> +	if (!report->desc.inblob_len)
> >>> +		return -EINVAL;
> >>> +
> >>> +	/* did another thread already generate this report? */
> >>> +	if (report->outblob &&
> >>> +	    state->read_generation == state->write_generation)
> >>> +		goto out;
> >>> +	kvfree(report->outblob);
> >>> +	kvfree(report->auxblob);
> >>> +	report->outblob = NULL;
> >>> +	report->auxblob = NULL;
> >>> +	rc = ops->report_new(report, provider.data);
> >>
> >>
> >> Drop @ops and use "provider.ops" here?
> > 
> > Shrug, ok.
> 
> I asked as at first I thought it is stored right after rwsem_write in 
> @ops for a reason but then it was not even checked for NULL so it could 
> be initialized where declared.

I went back to declaring an @ops variable because this sequence needs to
validate that the ops are non-NULL under the lock, and typing
provider.ops several times is a bit grating.

        /* slow path, report may need to be regenerated... */
        guard(rwsem_write)(&tsm_rwsem);
        ops = provider.ops;
        if (!ops)
                return -ENOTTY;