Re: [dm-devel] [PATCH 8/9] scsi: sd_zbc: Cleanup sd_zbc_alloc_report_buffer()

[Bart Van Assche <bvanassche@acm.org>]

On 2019-11-08 18:54, Damien Le Moal wrote:
> On 2019/11/09 4:06, Bart Van Assche wrote:
>> On 11/7/19 5:57 PM, Damien Le Moal wrote:
>>> -	buf = vzalloc(bufsize);
>>> -	if (buf)
>>> -		*buflen = bufsize;
>>> +	while (bufsize >= SECTOR_SIZE) {
>>> +		buf = vzalloc(bufsize);
>>> +		if (buf) {
>>> +			*buflen = bufsize;
>>> +			return buf;
>>> +		}
>>> +		bufsize >>= 1;
>>> +	}
>>
>> Hi Damien,
>>
>> Has it been considered to pass the __GFP_NORETRY flag to this vzalloc() 
>> call?
> 
> Do you mean using
> 
> __vmalloc(bufsize,
> 	  GFP_KERNEL | __GFP_ZERO | __GFP_NORETRY, PAGE_KERNEL);
> 
> instead of vzalloc() ? (since we cannot pass GFP flags to vzalloc()...)
> 
> Note that this is called with GFP_NOIO set for the caller context in the
> case of revalidate zones, and default to GFP_KERNEL for
> blkdev_report_zones() unless the caller also tweaks the context memalloc
> flags.

Hi Damien,

Yes, that's what I meant. The following comment from mm/util.c explains
why __GFP_RETRY should be used if it is OK for an allocation to fail:

/*
 * We want to attempt a large physically contiguous block first because
 * it is less likely to fragment multiple larger blocks and therefore
 * contribute to a long term fragmentation less than vmalloc fallback.
 * However make sure that larger requests are not too disruptive - no
 * OOM killer and no allocation failure warnings as we have a fallback.
 */

Thanks,

Bart.