Re: [PATCH V4 05/18] iommu/ioasid: Redefine IOASID set and allocation APIs

Hi Jason,

On Tue, 4 May 2021 15:00:50 -0300, Jason Gunthorpe [off-list ref] wrote:

On Tue, May 04, 2021 at 08:41:48AM -0700, Jacob Pan wrote:

quoted

quoted

quoted

(also looking at ioasid.c, why do we need such a thin and odd
wrapper around xarray?)
    

I'll leave it to Jean and Jacob.  

quoted

Could you elaborate?  

I mean stuff like this:

int ioasid_set_data(ioasid_t ioasid, void *data)
{
        struct ioasid_data *ioasid_data;
        int ret = 0;

        spin_lock(&ioasid_allocator_lock);
        ioasid_data = xa_load(&active_allocator->xa, ioasid);
        if (ioasid_data)
                rcu_assign_pointer(ioasid_data->private, data);
        else
                ret = -ENOENT;
        spin_unlock(&ioasid_allocator_lock);

        /*
         * Wait for readers to stop accessing the old private data, so the
         * caller can free it.
         */
        if (!ret)
                synchronize_rcu();

        return ret;
}
EXPORT_SYMBOL_GPL(ioasid_set_data);

It is a weird way to use xarray to have a structure which
itself is just a wrapper around another RCU protected structure.

Make the caller supply the ioasid_data memory, embedded in its own
element, get rid of the void * and rely on XA_ZERO_ENTRY to hold
allocated but not active entries.

Let me try to paraphrase to make sure I understand. Currently
struct ioasid_data is private to the iasid core, its memory is allocated by
the ioasid core.

You are suggesting the following:
1. make struct ioasid_data public
2. caller allocates memory for ioasid_data, initialize it then pass it to
ioasid_alloc to store in the xarray
3. caller will be responsible for setting private data inside ioasid_data
and do call_rcu after update if needed.

Correct?

Make the synchronize_rcu() the caller responsiblity, and callers
should really be able to use call_rcu()

Jason

Thanks,

Jacob