Messages in this thread | | | Date | Thu, 23 Jun 2022 22:48:36 +0800 | Subject | Re: [RFC PATCH V4 1/1] swiotlb: Split up single swiotlb lock | From | Tianyu Lan <> |
| |
On 6/22/2022 6:54 PM, Christoph Hellwig wrote: > Thanks, > > this looks pretty good to me. A few comments below: >
Thanks for your review.
> On Fri, Jun 17, 2022 at 10:47:41AM -0400, Tianyu Lan wrote: >> +/** >> + * struct io_tlb_area - IO TLB memory area descriptor >> + * >> + * This is a single area with a single lock. >> + * >> + * @used: The number of used IO TLB block. >> + * @index: The slot index to start searching in this area for next round. >> + * @lock: The lock to protect the above data structures in the map and >> + * unmap calls. >> + */ >> +struct io_tlb_area { >> + unsigned long used; >> + unsigned int index; >> + spinlock_t lock; >> +}; > > This can go into swiotlb.c.
struct io_tlb_area is used in the struct io_tlb_mem.
> >> +void __init swiotlb_adjust_nareas(unsigned int nareas); > > And this should be marked static. > >> +#define DEFAULT_NUM_AREAS 1 > > I'd drop this define, the magic 1 and a > 1 comparism seems to > convey how it is used much better as the checks aren't about default > or not, but about larger than one. > > I also think that we want some good way to size the default, e.g. > by number of CPUs or memory size.
swiotlb_adjust_nareas() is exposed to platforms to set area number. When swiotlb_init() is called, smp_init() isn't called at that point and so standard API of checking cpu number (e.g, num_online_cpus()) doesn't work. Platforms may have other ways to get cpu number(e.g x86 may ACPI MADT table entries to get cpu nubmer) and set area number. I will post following patch to set cpu number via swiotlb_adjust_nareas(),
> >> +void __init swiotlb_adjust_nareas(unsigned int nareas) >> +{ >> + if (!is_power_of_2(nareas)) { >> + pr_err("swiotlb: Invalid areas parameter %d.\n", nareas); >> + return; >> + } >> + >> + default_nareas = nareas; >> + >> + pr_info("area num %d.\n", nareas); >> + /* Round up number of slabs to the next power of 2. >> + * The last area is going be smaller than the rest if >> + * default_nslabs is not power of two. >> + */ > > Please follow the normal kernel comment style with a /* on its own line. >
OK. Will update.
| |