On 24/01/2022 10:46, Tony Lu wrote: > On Fri, Jan 21, 2022 at 12:06:56PM +0100, Karsten Graul wrote: >> On 21/01/2022 04:24, Tony Lu wrote: >>> On Thu, Jan 20, 2022 at 03:50:26PM +0100, Karsten Graul wrote: >>>> On 20/01/2022 15:09, Tony Lu wrote: >> I am still not fully convinced of this change. It does not harm and the overhead of >> a vmalloc() is acceptable because a link group is not created so often. But since >> kvzmalloc() will first try to use normal kmalloc() and if that fails switch to the >> (more expensive) vmalloc() this will not _save_ any contiguous memory. >> And for the subsequent required allocations of at least one RMB we need another 16KB. > I agree with you. kvzmalloc doesn't save contiguous memory for the most > time, only when high order contiguous memory is used out, or malloc link > group when another link group just freed its buffer. This race window is > too small to reach it in real world. Okay I see we are in sync with that, and we should drop your kvzalloc() patch. It generates noise and doesn't solve a real problem. I appreciate your work on this topic, but when I see the numbers then the whole lgr WITH all links inside it would occupy less than one 4K page of memory (~3808 bytes). The vast majority of memory in this struct is needed by the struct smc_rtoken rtokens[255][3]; /* 3472 12240 */ array. This is where continuous space could be saved, but that needs some effort to provide an equivalent fast way to store and lookup the RMBs. Moving out the links from the lgr will not help here. A link group holds up to 255 connections, so even with your 10000 connection test we need no more than 40 instances of lgr...I am not sure if it is worth the time that you need to spend for this particular change (lgr). The other topics you listed also sound interesting!
