Re: [Qemu-devel] [Qemu-block] [PATCH] block: Swap request limit definitions

[Kevin Wolf]

Am 15.02.2017 um 17:48 hat Max Reitz geschrieben:
> On 15.02.2017 17:44, Kevin Wolf wrote:
> > Am 15.02.2017 um 14:42 hat Max Reitz geschrieben:
> >> On 14.02.2017 10:52, Alberto Garcia wrote:
> >>> On Mon 13 Feb 2017 06:13:38 PM CET, Max Reitz wrote:
> >>>
> >>>>>> -#define BDRV_REQUEST_MAX_SECTORS MIN(SIZE_MAX >> BDRV_SECTOR_BITS, \
> >>>>>> -                                     INT_MAX >> BDRV_SECTOR_BITS)
> >>>>>> -#define BDRV_REQUEST_MAX_BYTES (BDRV_REQUEST_MAX_SECTORS << 
> >>>>>> BDRV_SECTOR_BITS)
> >>>>>> +#define BDRV_REQUEST_MAX_BYTES      MIN(SIZE_MAX, INT_MAX)
> >>>>>> +#define BDRV_REQUEST_MAX_SECTORS    (BDRV_REQUEST_MAX_BYTES >> 
> >>>>>> BDRV_SECTOR_BITS)
> >>>>>
> >>>>> I'm just pointing it out because I don't know if this can cause
> >>>>> problems, but this patch would make BDRV_REQUEST_MAX_BYTES not a
> >>>>> multiple of the sector size (INT_MAX is actually a prime number).
> >>>>
> >>>> Very good point. I don't think this could be an issue, though. For one
> >>>> thing, the use of BDRV_REQUEST_MAX_BYTES is very limited.
> >>>
> >>> Ok, but then I wonder what's the benefit of increasing
> >>> BDRV_REQUEST_MAX_BYTES.
> >>
> >> The benefit is that the definition looks cleaner.
> > 
> > Whatever way we want to write it, I think MAX_BYTES = MAX_SECTORS * 512
> > should be a given. Everything else is bound to confuse people and
> > introduce bugs sooner or later.
> 
> Probably only sooner and not later, considering we are switching to byte
> granularity overall anyway. And if something confuses people, I'd argue
> it's the fact that we still have sector granularity all over the place
> and not that your requests can be a bit bigger if you submit them in
> bytes than if you submit them in sectors.
> 
> Anyway, if MAX_BYTES should be a multiple of the sector size, then I
> can't think of a much better way to write this than what we currently
> have and this patch is unneeded.

Maybe we can just get rid of BDRV_REQUEST_MAX_SECTORS? Or do we need to
do a few more conversion before that?

Kevin

pgp0AZG7RCfzW.pgp
Description: PGP signature