Re: Re: [PATCH net-next v3 1/3] net: devmem: replace gen_pool with freelist

[Stanislav Fomichev <sdf.kernel@gmail.com>]

> gen_pool_free() -> gen_pool_free_owner() -> bitmap_clear_ll()
> 	BUG_ON(remain);
>
> Reaching this requires a separate page_pool refcount bug, since
> mp_dmabuf_devmem_release_page() rejects refcount != 1, so this is
> only a loss of a fail-stop diagnostic. Is dropping that check
> intentional to match io_uring zcrx, which uses the same unguarded
> LIFO?

Working as intended. A duplicate return already violates page_pool ownership.

> This is a pre-existing issue and not introduced here, since the
> return value of page_pool_set_dma_addr_netmem() is dropped.
>
> Would it be worth failing the bind here as well?

I plan to follow up with 2 more patches to cleanup freelist and will
fix it there (convert to the helpers). Let's consider this a pre-existing
issue that I'll fix separately?

> [Severity: Medium]
> Can this walk overrun owner->area.niovs[], binding->freelist[] and
> binding->tx_vec[]?
>
> Would adding the sum check or the post-loop equality check be
> worthwhile?

Same feedback as on patch 1: let's not do defensive programming. We
do have a contract where dmabuf->len == sum(sg->len) and it is
maintained for the in-tree dmabufs.