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);
> }
