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: <81a915d3-cf5f-a884-4649-704a5cf26835@linux.alibaba.com>
Date:   Wed, 2 Mar 2022 14:38:09 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH v4 2/2] io_uring: Add support for napi_busy_poll
Content-Language: en-US
From:   Hao Xu <haoxu@linux.alibaba.com>
To:     Olivier Langlois <olivier@trillion01.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>
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>
 <4af380e8-796b-2dd6-4ebc-e40e7fa51ce1@linux.alibaba.com>
In-Reply-To: <4af380e8-796b-2dd6-4ebc-e40e7fa51ce1@linux.alibaba.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


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