On 10/9/24 1:05 PM, Pavel Begunkov wrote:
> On 10/9/24 19:02, Jens Axboe wrote:
>> On 10/7/24 4:15 PM, David Wei wrote:
> ...
>>> +struct io_zcrx_area {
>>> + struct net_iov_area nia;
>>> + struct io_zcrx_ifq *ifq;
>>> +
>>> + u16 area_id;
>>> + struct page **pages;
>>> +
>>> + /* freelist */
>>> + spinlock_t freelist_lock ____cacheline_aligned_in_smp;
>>> + u32 free_count;
>>> + u32 *freelist;
>>> +};
>>
>> I'm wondering if this really needs an aligned lock? Since it's only a
>> single structure, probably not a big deal. But unless there's evidence
>> to the contrary, might not be a bad idea to just kill that.
>
> napi and IORING_OP_RECV_ZC can run on different CPUs, I wouldn't
> want the fields before the lock being contended by the lock
> because of cache line sharing, would especially hurt until it's
> warmed up well. Not really profiled, but not like we need to
> care about space here.
Right, as mentioned it's just a single struct, so doesn't matter that
much. I guess my testing all ran with same cpu for napi + rx, so would
not have seen it regardless. We can keep it as-is.
--
Jens Axboe
