This series fixing two issues related to fails of
__alloc_dma_rx_desc_resources(). Original issue from 1st patch is related to
page_pool that has happened in testing env, while second was requested by
Sashiko to have similar change for DMA allocation. 
To have complete fix for all failures of __alloc_dma_rx_desc_resources(),
merge two fixes into series.

---
Changes in v3:
- Added null assignment to page pointers (suggested by sashiko)
- Convert IS_ERR_OR_NULL check to just NULL check

Changes in v2:
- Added reviewed by Maxime to first patch
- Dropped null check as page_pool_destroy() does provide that
- Modified comment to reflect that

Link to v2:
https://lore.kernel.org/all/20260707174115.1264466-1-j.raczynski@samsung.com/

Link to v1:
https://lore.kernel.org/netdev/20260630100953.747868-1-j.raczynski@samsung.com/

Jakub Raczynski (2):
  net/stmmac: Set Rx queue page_pool to NULL when freeing DMA resources
  net/stmmac: Prevent dma queue NULL free on allocation failure

 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 32 ++++++++++++-------
 1 file changed, 21 insertions(+), 11 deletions(-)

-- 
2.34.1

On Wed, 15 Jul 2026 14:36:00 +0200 Jakub Raczynski wrote:
> This series fixing two issues related to fails of
> __alloc_dma_rx_desc_resources(). Original issue from 1st patch is related to
> page_pool that has happened in testing env, while second was requested by
> Sashiko to have similar change for DMA allocation. 
> To have complete fix for all failures of __alloc_dma_rx_desc_resources(),
> merge two fixes into series.

Clashiko is not impressed by the second patch.
Is it possible to avoid calling the functions in semi-consistent state?
-- 
pw-bot: cr

On Thu, Jul 23, 2026 at 07:23:52AM -0700, Jakub Kicinski wrote:
> On Wed, 15 Jul 2026 14:36:00 +0200 Jakub Raczynski wrote:
> > This series fixing two issues related to fails of
> > __alloc_dma_rx_desc_resources(). Original issue from 1st patch is related to
> > page_pool that has happened in testing env, while second was requested by
> > Sashiko to have similar change for DMA allocation. 
> > To have complete fix for all failures of __alloc_dma_rx_desc_resources(),
> > merge two fixes into series.
> 
> Clashiko is not impressed by the second patch.
> Is it possible to avoid calling the functions in semi-consistent state?

Again clash against AI lost, damn you AI. Although I cannot say its wrong.
My bad I didn't really respond to it sooner, especially 13 character Fixes tag,
wonder how that slipped past internal review...

Now being serious, regarding calling in semi-consistent, it is matter of
symmetry between open/close or alloc/dealloc paths.
Since __alloc_dma_{tx/rx}_desc_resources does full initialization,
__free_dma_{tx/rx}_desc_resources should be able to handle whole cycle.
So if __alloc_ failed in the middle, __free_ should handle that state,
whatever it might be.

One thing I will say that AI review is not even about patches themselves,
but about
"If the intent is to make __free_dma_rx_desc_resources() safe to
run twice on the same queue, [...]",
which is the point, although original patch was generated by
real issue that occured. Other issues it reports are valid but did not
trigger.

So AI is right that everything should be handled in one patchset when
this is touched, but funnily it didn't report it previous review.
Will send another version that will fix all these issues/complains
at some point.

BR
Jakub Raczynski

On Thu, 23 Jul 2026 17:24:58 +0200 Jakub Raczynski wrote:
> On Thu, Jul 23, 2026 at 07:23:52AM -0700, Jakub Kicinski wrote:
> > On Wed, 15 Jul 2026 14:36:00 +0200 Jakub Raczynski wrote:  
> > > This series fixing two issues related to fails of
> > > __alloc_dma_rx_desc_resources(). Original issue from 1st patch is related to
> > > page_pool that has happened in testing env, while second was requested by
> > > Sashiko to have similar change for DMA allocation. 
> > > To have complete fix for all failures of __alloc_dma_rx_desc_resources(),
> > > merge two fixes into series.  
> > 
> > Clashiko is not impressed by the second patch.
> > Is it possible to avoid calling the functions in semi-consistent state?  
> 
> Again clash against AI lost, damn you AI. Although I cannot say its wrong.
> My bad I didn't really respond to it sooner, especially 13 character Fixes tag,
> wonder how that slipped past internal review...
> 
> Now being serious, regarding calling in semi-consistent, it is matter of
> symmetry between open/close or alloc/dealloc paths.
> Since __alloc_dma_{tx/rx}_desc_resources does full initialization,
> __free_dma_{tx/rx}_desc_resources should be able to handle whole cycle.
> So if __alloc_ failed in the middle, __free_ should handle that state,
> whatever it might be.
> 
> One thing I will say that AI review is not even about patches themselves,
> but about
> "If the intent is to make __free_dma_rx_desc_resources() safe to
> run twice on the same queue, [...]",
> which is the point, although original patch was generated by
> real issue that occured. Other issues it reports are valid but did not
> trigger.
> 
> So AI is right that everything should be handled in one patchset when
> this is touched, but funnily it didn't report it previous review.
> Will send another version that will fix all these issues/complains
> at some point.

As you fix these issues it'd be great to step back and figure out what
model we want to follow. Personally I find the "idempotent cleanup"
to be inferior, it's better to know what state we're in. Failing that
a single indicator of state being initialized is usually fine. Having
field-by-field safeties is a recipe for 1000 fixes. IOW stmmac is
terribly architected, so we should figure out the end goal first, 
and target that, instead of addressing issues one by one.