Re: [PATCH net v3 0/2] net/stmmac: Secure against failures of DMA memory allocation

[Jakub Kicinski]

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.