Re: [PATCH v4 2/2] io_uring: Add support for napi_busy_poll

[Hao Xu]

On 3/2/22 14:27, Hao Xu wrote:
> On 3/2/22 04:06, Olivier Langlois wrote:
> > On Wed, 2022-03-02 at 02:31 +0800, Hao Xu wrote:
> > > > +       ne = kmalloc(sizeof(*ne), GFP_NOWAIT);
> > > > +       if (!ne)
> > > > +               goto out;
> > > IMHO, we need to handle -ENOMEM here, I cut off the error handling
> > > when
> > >
> > > I did the quick coding. Sorry for misleading.
> > If you are correct, I would be shocked about this.
> >
> > I did return in my 'Linux Device Drivers' book and nowhere it is
> > mentionned that the kmalloc() can return something else than a pointer
> >
> > No mention at all about the return value
> >
> > in man page:
> > https://www.kernel.org/doc/htmldocs/kernel-api/API-kmalloc.html
> > API doc:
> >
> > https://www.kernel.org/doc/html/latest/core-api/mm-api.html?highlight=kmalloc#c.kmalloc 
> >
> >
> > header file:
> > https://elixir.bootlin.com/linux/latest/source/include/linux/slab.h#L522
> >
> > I did browse into the kmalloc code. There is a lot of paths to cover
> > but from preliminary reading, it pretty much seems that kmalloc only
> > returns a valid pointer or NULL...
> >
> > /**
> >   * kmem_cache_alloc - Allocate an object
> >   * @cachep: The cache to allocate from.
> >   * @flags: See kmalloc().
> >   *
> >   * Allocate an object from this cache.  The flags are only relevant
> >   * if the cache has no available objects.
> >   *
> >   * Return: pointer to the new object or %NULL in case of error
> >   */
> >     /**
> >   * __do_kmalloc - allocate memory
> >   * @size: how many bytes of memory are required.
> >   * @flags: the type of memory to allocate (see kmalloc).
> >   * @caller: function caller for debug tracking of the caller
> >   *
> >   * Return: pointer to the allocated memory or %NULL in case of error
> >   */
> >
> > I'll need someone else to confirm about possible kmalloc() return
> > values with perhaps an example
> >
> > I am a bit skeptic that something special needs to be done here...
> >
> > Or perhaps you are suggesting that io_add_napi() returns an error code
> > when allocation fails.
> This is what I mean.
> > as done here:
> > https://elixir.bootlin.com/linux/latest/source/arch/alpha/kernel/core_marvel.c#L867 
> >
> >
> > If that is what you suggest, what would this info do for the caller?
> >
> > IMHO, it wouldn't help in any way...
>
> Hmm, I'm not sure, you're probably right based on that ENOMEM here 
> shouldn't
>
> fail the arm poll, but we wanna do it, we can do something like what 
> we do for
                            ^---but if we wanna do it