linux-kernel - Re: [PATCH v4 2/2] io_uring: Add support for napi_busy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <628f7ac96c15b0a3d5f2ddcdf4df79a03950b17e.camel@trillion01.com>
Date:   Tue, 01 Mar 2022 15:14:58 -0500
From:   Olivier Langlois <olivier@...llion01.com>
To:     Hao Xu <haoxu@...ux.alibaba.com>, Jens Axboe <axboe@...nel.dk>,
        Pavel Begunkov <asml.silence@...il.com>
Cc:     io-uring <io-uring@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 2/2] io_uring: Add support for napi_busy_poll

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.