On 12/1/22 07:29, Bart Van Assche wrote: > On 11/23/22 17:40, Damien Le Moal wrote: >> On 11/24/22 05:57, Bart Van Assche wrote: >>> +static unsigned int g_max_segment_size = 1UL << 31; >> >> 1UL is unsigned long be this var is unsigned int. Why not simply use >> UINT_MAX here ? You prefer the 2GB value ? If yes, then may be at least >> change that to "1U << 31", no ? >> >> [ ... ] >>> @@ -2106,6 +2119,7 @@ static int null_add_dev(struct nullb_device *dev) >>> dev->max_sectors = min_t(unsigned int, dev->max_sectors, >>> BLK_DEF_MAX_SECTORS); >>> blk_queue_max_hw_sectors(nullb->q, dev->max_sectors); >>> + blk_queue_max_segment_size(nullb->q, dev->max_segment_size); >> >> Should we keep the ability to use the kernel default value as the default >> here ? >> E.g. >> >> if (dev->max_segment_size) >> blk_queue_max_segment_size(nullb->q, >> dev->max_segment_size); >> >> If yes, then g_max_segment_size initial value should be 0, meaning "kernel >> default". > > Hi Damien, > > How about changing the default value for g_max_segment_size from > 1UL << 31 into BLK_MAX_SEGMENT_SIZE? That will simplify the code and > also prevents that this patch changes the behavior of the null_blk > driver if g_max_segment_size is not modified by the user. Sounds good to me. > > Thanks, > > Bart. > -- Damien Le Moal Western Digital Research
