From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4F10D363096; Thu, 23 Jul 2026 17:21:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784827275; cv=none; b=Qua+Ka2KYyOEUEuaov5Tf9GV3wu57YfwyrnrOCerUBG7jhL82NCWo4sVZ5ATPhw+xqP2IvnSIt+apkY2aokEWu6PoWx/4UGh1Qs2Xwlm/Wc3Om/GgJGX5c0nbwliezXdhaOWFYvM/pZu36PlnhCB92KegqXzSTnatUWu2sqfad4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1784827275; c=relaxed/simple; bh=bOI1f352Lm6ZjvxBgCuM7sn4RX8BM4XwdBV1erDIacA=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=f4FtCiCmGajwzL0EYJWHka61GGy2A+01DPYe4Hku4Jy7ZDp6GgCkcL6Rt+PrKxe2ZBeknVQ2aXuFq3ATHdL3B91eYcn9R29ckw9JffUPVG16eublgdiwplhjxppea5mFCHaWrLB5re6lqq+xCCoVMHGvk3f7T6XclA+qJNKhz2s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA73D1F000E9; Thu, 23 Jul 2026 17:21:13 +0000 (UTC) Date: Thu, 23 Jul 2026 10:21:07 -0700 From: Jakub Kicinski To: Jakub Raczynski Cc: netdev@vger.kernel.org, andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com, pabeni@redhat.com, mcoquelin.stm32@gmail.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH net v3 0/2] net/stmmac: Secure against failures of DMA memory allocation Message-ID: <20260723102107.08464361@kernel.org> In-Reply-To: References: <20260715123602.51950-1-j.raczynski@samsung.com> <20260723072352.6db4df7f@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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.