Re: null_handle_cmd() doesn't initialize data when reading

[Damien Le Moal <Damien.LeMoal@wdc.com>]

On 2020/05/11 21:58, Alexander Potapenko wrote:
> On Sun, May 10, 2020 at 6:20 PM Bart Van Assche <bvanassche@acm.org> wrote:
>>
>> On 2020-05-10 03:03, Alexander Potapenko wrote:
>>> Thanks for the explanation!
>>> The code has changed recently, and my patch does not apply anymore,
>>> yet the problem still persists.
>>> I ended up just calling null_handle_rq() at the end of
>>> null_process_cmd(), but we probably need a cleaner fix.
>>
>> Does this (totally untested) patch help? copy_to_nullb() guarantees that
>> it will write some data to the pages that it allocates but does not
>> guarantee yet that all data of the pages it allocates is initialized.
> 
> No, this does not help. Apparently null_insert_page() is never called
> in this scenario.
> If I modify __page_cache_alloc() to allocate zero-initialized pages,
> the reports go away.
> This means there's no other uninitialized buffer that's copied to the
> page cache, the nullb driver just forgets to write anything to the
> page cache.

Can you describe again the problem you are seeing please ? I can't find the
first email of this thread and forgot what the problem is.

> 
>> diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
>> index 8efd8778e209..06f5761fccb6 100644
>> --- a/drivers/block/null_blk_main.c
>> +++ b/drivers/block/null_blk_main.c
>> @@ -848,7 +848,7 @@ static struct nullb_page *null_insert_page(struct
>> nullb *nullb,
>>
>>         spin_unlock_irq(&nullb->lock);
>>
>> -       t_page = null_alloc_page(GFP_NOIO);
>> +       t_page = null_alloc_page(GFP_NOIO | __GFP_ZERO);
>>         if (!t_page)
>>                 goto out_lock;
>>
> 
> 


-- 
Damien Le Moal
Western Digital Research