Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <628f7ac96c15b0a3d5f2ddcdf4df79a03950b17e.camel@trillion01.com>
Subject: Re: [PATCH v4 2/2] io_uring: Add support for napi_busy_poll
From:   Olivier Langlois <olivier@trillion01.com>
To:     Hao Xu <haoxu@linux.alibaba.com>, Jens Axboe <axboe@kernel.dk>,
        Pavel Begunkov <asml.silence@gmail.com>
Cc:     io-uring <io-uring@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>
Date:   Tue, 01 Mar 2022 15:14:58 -0500
In-Reply-To: <4f01857ca757ab4f0995420e6b1a6e3668a40da5.camel@trillion01.com>
References: <cover.1646142288.git.olivier@trillion01.com>
         <aa38a667ef28cce54c08212fdfa1e2b3747ad3ec.1646142288.git.olivier@trillion01.com>
         <29bad95d-06f8-ea7c-29fe-81e52823c90a@linux.alibaba.com>
         <4f01857ca757ab4f0995420e6b1a6e3668a40da5.camel@trillion01.com>
Organization: Trillion01 Inc
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.42.4 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 2022-03-01 at 15:06 -0500, Olivier Langlois wrote:
> On Wed, 2022-03-02 at 02:31 +0800, Hao Xu wrote:
> > 
> > 
> > How about:
> > 
> > if (list is singular) {
> > 
> > ???? do something;
> > 
> > ???? return;
> > 
> > }
> > 
> > while (!io_busy_loop_end() && io_napi_busy_loop())
> > 
> > ???? ;
> > 
> 
> is there a concern with the current code?
> What would be the benefit of your suggestion over current code?
> 
> To me, it seems that if io_blocking_napi_busy_loop() is called, a
> reasonable expectation would be that some busy looping is done or
> else
> you could return the function without doing anything which would,
> IMHO,
> be misleading.
> 
> By definition, napi_busy_loop() is not blocking and if you desire the
> device to be in busy poll mode, you need to do it once in a while or
> else, after a certain time, the device will return back to its
> interrupt mode.
> 
> IOW, io_blocking_napi_busy_loop() follows the same logic used by
> napi_busy_loop() that does not call loop_end() before having perform
> 1
> loop iteration.
> 
> > Btw, start_time seems not used in singular branch.
> 
> I know. This is why it is conditionally initialized.
> 
> Greetings,
> 
Another argument for not touching the code the way that it is:
io_napi_busy_loop() has another function on top of iterating the
napi_list and calling napi_busy_loop() for each of them.

The function also check the list entries validity and frees them when
they time out. Not calling io_napi_busy_loop() you would bypass this
check and that could result in timed out entries to never be disposed.