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

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

> You're warning here that you can't destroy the dmabuf binding but
> you're destroying it anyway. Something is off here. Do we want an
> early return or something else?

I'm assuming we had an early return because gen_pool_destroy would
BUG_ON on non-empty pool. Let me just reword this warning.

> The LLM thinks this NULL store in unnecassary. IDK if it will help
> anything in practice to remove it :-)

No preference, let me remove it.

> We don't need freelists on tx right? We should probably not allocate
> them then?

Yeah, good point, will make it rx only.

> Do we really need a secondary for_each_sgtable_dma_sg loop just to
> calculate the total_niovs? In what edge case is the total_niovs not
> just dmabuf_len / niov_len? We do a bunch of alignment checks to make
> sure it all works out to that no?

Agreed, that should work!

> binding->total_niovs and binding->area.num_niovs seem the same thing
> always. please get rid of one, probably binding->total_niovs.

Will do, thanks!

> I wonder if now that both zcrx and devmem use a freelist if the
> freelist should be part of the net_iov_area. The point of that field
> was to hold the common stuff actually, but I'm guessing there are
> micro-implementation differences that will make converging annoying.
> I'm fine either way. :shrug:

Let me write it down too look separately. Agreed that it'd be nice
to generalize.

> len is referenced once now; not worth a local var.

SG!

> if feels somewhat simple to exclude freelist from TX. Something like:
>
> if (direction == dma_to_device)
>   <store into tx_vec>
> else
>   <store in freelist>

Will do, yeah. Thank you for the review!