On Thu, Jan 25, 2024 at 8:07 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
>
> On Thu, Jan 25, 2024 at 10:33:17AM +0000, Eric Dumazet wrote:
> > +++ b/net/ipv4/tcp.c
> > @@ -1786,7 +1786,17 @@ static skb_frag_t *skb_advance_to_frag(struct sk_buff *skb, u32 offset_skb,
> >
> > static bool can_map_frag(const skb_frag_t *frag)
> > {
> > - return skb_frag_size(frag) == PAGE_SIZE && !skb_frag_off(frag);
> > + struct page *page;
> > +
> > + if (skb_frag_size(frag) != PAGE_SIZE || skb_frag_off(frag))
> > + return false;
> > +
> > + page = skb_frag_page(frag);
> > +
> > + if (PageCompound(page) || page->mapping)
> > + return false;
>
> I'm not entirely sure why you're testing PageCompound here. If a driver
> allocates a compound page, we'd still want to be able to insert it,
> right?
>
Resend b/c I forgot I was in HTML mode email, oops.
Is there a common use case for a NIC driver to be doing this? I was
under the impression NIC drivers would get pages individually since it
would be harder to find physically contiguous groupings in general.
Anyways, a possible reason to not allow compound pages - if we have
some large memory range and different receiving users are interested
in different parts of the range in userspace - since the whole range
is pinned till everyone is done with it, it can lead to worse cases of
memory lingering when some user only wanted 10 bytes out of some N *
PAGE_SIZE range for a multi-minute long period.
-Arjun
> I have a feeling that we want to fix this in the VM layer. There are
> some weird places calling vm_insert_page() and we should probably make
> them all fail.
>
> Something like this, perhaps?
>
> diff --git a/mm/memory.c b/mm/memory.c
> index 1a60faad2e49..ae0abab56d38 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1871,6 +1871,10 @@ static int insert_page_into_pte_locked(struct vm_area_struct *vma, pte_t *pte,
>
> if (!pte_none(ptep_get(pte)))
> return -EBUSY;
> + if (folio->mapping &&
> + ((addr - vma->vm_start) / PAGE_SIZE + vma->vm_pgoff) !=
> + (folio->index + folio_page_idx(folio, page)))
> + return -EINVAL;
> /* Ok, finally just insert the thing.. */
> folio_get(folio);
> inc_mm_counter(vma->vm_mm, mm_counter_file(folio));
