On 2024/1/9 0:25, Alexander Duyck wrote: > On Mon, Jan 8, 2024 at 12:59 AM Yunsheng Lin <linyunsheng@xxxxxxxxxx> wrote: ... > >>> >>> 2. By starting at the end and working toward zero we can use built in >>> functionality of the CPU to only have to check and see if our result >>> would be signed rather than having to load two registers with the >>> values and then compare them which saves us a few cycles. In addition >>> it saves us from having to read both the size and the offset for every >>> page. >> >> I suppose the above is ok if we only use the page_frag_alloc*() API to >> allocate memory for skb->data, not for the frag in skb_shinfo(), as by >> starting at the end and working toward zero, it means we can not do skb >> coalescing. >> >> As page_frag_alloc*() is returning va now, I am assuming most of users >> is using the API for skb->data, I guess it is ok to drop this patch for >> now. >> >> If we allow page_frag_alloc*() to return struct page, we might need this >> patch to enable coalescing. > > I would argue this is not the interface for enabling coalescing. This > is one of the reasons why this is implemented the way it is. When you > are aligning fragments you aren't going to be able to coalesce the > frames anyway as the alignment would push the fragments apart. It seems the alignment requirement is the same for the same user of a page_frag instance, so the aligning does not seem to be a problem for coalescing? > . >
