| Message ID | 1422888514-6495-1-git-send-email-pl@kamp.de |
|---|---|
| State | New |
| Headers | show |
Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben: > do not trim requests if the driver does not supply a limit > through BlockLimits. For write zeroes we still keep a limit > for the unsupported path to avoid allocating a big bounce buffer. > > Suggested-by: Kevin Wolf <kwolf@redhat.com> > Suggested-by: Denis V. Lunev <den@openvz.org> > Signed-off-by: Peter Lieven <pl@kamp.de> Thanks, applied to the block branch (and removed 'block/raw-posix: set max_write_zeroes to INT_MAX for regular files' from the queue). Kevin
On 02/02/15 19:13, Kevin Wolf wrote: > Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben: >> do not trim requests if the driver does not supply a limit >> through BlockLimits. For write zeroes we still keep a limit >> for the unsupported path to avoid allocating a big bounce buffer. >> >> Suggested-by: Kevin Wolf <kwolf@redhat.com> >> Suggested-by: Denis V. Lunev <den@openvz.org> >> Signed-off-by: Peter Lieven <pl@kamp.de> > Thanks, applied to the block branch (and removed 'block/raw-posix: set > max_write_zeroes to INT_MAX for regular files' from the queue). > > Kevin double checked the code. There are 2 things to patch for discard, write_zeroes is OK for me. Sorry, for not paying attention for discard branch :( Den
Am 02.02.2015 um 17:25 hat Denis V. Lunev geschrieben: > On 02/02/15 19:13, Kevin Wolf wrote: > >Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben: > >>do not trim requests if the driver does not supply a limit > >>through BlockLimits. For write zeroes we still keep a limit > >>for the unsupported path to avoid allocating a big bounce buffer. > >> > >>Suggested-by: Kevin Wolf <kwolf@redhat.com> > >>Suggested-by: Denis V. Lunev <den@openvz.org> > >>Signed-off-by: Peter Lieven <pl@kamp.de> > >Thanks, applied to the block branch (and removed 'block/raw-posix: set > >max_write_zeroes to INT_MAX for regular files' from the queue). > > > >Kevin > double checked the code. > > There are 2 things to patch for discard, write_zeroes is OK for me. > Sorry, for not paying attention for discard branch :( Good catch, thanks! But shouldn't we use the actual limits instead of INT_MAX, i.e. SIZE_MAX for gluster and UINT32_MAX for nbd? Kevin
On 02/02/15 19:45, Kevin Wolf wrote: > Am 02.02.2015 um 17:25 hat Denis V. Lunev geschrieben: >> On 02/02/15 19:13, Kevin Wolf wrote: >>> Am 02.02.2015 um 15:48 hat Peter Lieven geschrieben: >>>> do not trim requests if the driver does not supply a limit >>>> through BlockLimits. For write zeroes we still keep a limit >>>> for the unsupported path to avoid allocating a big bounce buffer. >>>> >>>> Suggested-by: Kevin Wolf <kwolf@redhat.com> >>>> Suggested-by: Denis V. Lunev <den@openvz.org> >>>> Signed-off-by: Peter Lieven <pl@kamp.de> >>> Thanks, applied to the block branch (and removed 'block/raw-posix: set >>> max_write_zeroes to INT_MAX for regular files' from the queue). >>> >>> Kevin >> double checked the code. >> >> There are 2 things to patch for discard, write_zeroes is OK for me. >> Sorry, for not paying attention for discard branch :( > Good catch, thanks! > > But shouldn't we use the actual limits instead of INT_MAX, i.e. SIZE_MAX > for gluster and UINT32_MAX for nbd? > > Kevin You are absolutely correct for NBD case but I do not get the point about SIZE_MAX for gluster. There is no such definition in their git at git://github.com/gluster/glusterfs nor in public API headers in Ubuntu :( Regards, Den
On 02/02/2015 18:00, Denis V. Lunev wrote: >> > You are absolutely correct for NBD case but I do not get the > point about SIZE_MAX for gluster. There is no such definition > in their git at git://github.com/gluster/glusterfs nor in > public API headers in Ubuntu :( SIZE_MAX is defined in stdint.h (provided by the C library). Paolo
On 02/02/15 20:34, Paolo Bonzini wrote: > > > On 02/02/2015 18:00, Denis V. Lunev wrote: >>> >> You are absolutely correct for NBD case but I do not get the >> point about SIZE_MAX for gluster. There is no such definition >> in their git at git://github.com/gluster/glusterfs nor in >> public API headers in Ubuntu :( > > SIZE_MAX is defined in stdint.h (provided by the C library). > > Paolo > In this case the code should avoid overflow on 64bit arch. OK, re-spinning.
diff --git a/block.c b/block.c index d45e4dd..ee7ff2c 100644 --- a/block.c +++ b/block.c @@ -3192,10 +3192,7 @@ int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs, BDRV_REQ_COPY_ON_READ); } -/* if no limit is specified in the BlockLimits use a default - * of 32768 512-byte sectors (16 MiB) per request. - */ -#define MAX_WRITE_ZEROES_DEFAULT 32768 +#define MAX_WRITE_ZEROES_BOUNCE_BUFFER 32768 static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, int64_t sector_num, int nb_sectors, BdrvRequestFlags flags) @@ -3206,7 +3203,7 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, int ret = 0; int max_write_zeroes = bs->bl.max_write_zeroes ? - bs->bl.max_write_zeroes : MAX_WRITE_ZEROES_DEFAULT; + bs->bl.max_write_zeroes : INT_MAX; while (nb_sectors > 0 && !ret) { int num = nb_sectors; @@ -3242,7 +3239,7 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs, if (ret == -ENOTSUP) { /* Fall back to bounce buffer if write zeroes is unsupported */ int max_xfer_len = MIN_NON_ZERO(bs->bl.max_transfer_length, - MAX_WRITE_ZEROES_DEFAULT); + MAX_WRITE_ZEROES_BOUNCE_BUFFER); num = MIN(num, max_xfer_len); iov.iov_len = num * BDRV_SECTOR_SIZE; if (iov.iov_base == NULL) { @@ -5097,11 +5094,6 @@ static void coroutine_fn bdrv_discard_co_entry(void *opaque) rwco->ret = bdrv_co_discard(rwco->bs, rwco->sector_num, rwco->nb_sectors); } -/* if no limit is specified in the BlockLimits use a default - * of 32768 512-byte sectors (16 MiB) per request. - */ -#define MAX_DISCARD_DEFAULT 32768 - int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, int nb_sectors) { @@ -5126,7 +5118,7 @@ int coroutine_fn bdrv_co_discard(BlockDriverState *bs, int64_t sector_num, return 0; } - max_discard = bs->bl.max_discard ? bs->bl.max_discard : MAX_DISCARD_DEFAULT; + max_discard = bs->bl.max_discard ? bs->bl.max_discard : INT_MAX; while (nb_sectors > 0) { int ret; int num = nb_sectors;
do not trim requests if the driver does not supply a limit through BlockLimits. For write zeroes we still keep a limit for the unsupported path to avoid allocating a big bounce buffer. Suggested-by: Kevin Wolf <kwolf@redhat.com> Suggested-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Peter Lieven <pl@kamp.de> --- block.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)