Re: [PATCH] block: save user max_sectors limit

[Keith Busch <kbusch@kernel.org>]

On Fri, Dec 23, 2022 at 07:00:09AM +0100, Christoph Hellwig wrote:
> On Thu, Dec 22, 2022 at 09:52:04AM -0800, Keith Busch wrote:
> > +		max_sectors = min_t(unsigned int, max_sectors, BLK_DEF_MAX_SECTORS);
> 
> And please make BLK_DEF_MAX_SECTORS an unsigned constant as it should be
> and drop the min_t here.  Instead of working around type mismatches
> just avoid them from the beginning..

Let me just make sure I understand: you don't want BLK_DEF_MAX_SECTORS
defined in an enum, and instead want it a 'const unsigned int'? Or
#define?
 
> > +	} else if (max_sectors_kb > max_hw_sectors_kb ||
> > +		   max_sectors_kb < page_kb)  {
> >  		return -EINVAL;
> 
> And this check should probably move above the old one to keep the
> sanity checks first.
> 
> > +	} else {
> > +		q->limits.max_user_sectors = max_sectors_kb << 1;
> > +	}
> 
> i.e.
> 
> 	if (max_sectors_kb > max_hw_sectors_kb || max_sectors_kb < page_kb)
> 		return -EINVAL;
> 
> 	q->limits.max_user_sectors = max_sectors_kb << 1;
> 
> 	/* reset to default when the user clears max_sectors_kb: */
> 	if (max_sectors_kb == 0) {

We'll never reach here since 0 is < page_kb. But I think I can clean
this up a little bit for the next version.

> 		max_sectors_kb =
> 			min(max_hw_sectors_kb, BLK_DEF_MAX_SECTORS >> 1);
> 	}