Roger Pau Monne
2013-Jun-21 10:56 UTC
[PATCH 3/4] xen-blkback: check the number of iovecs before allocating a bios
With the introduction of indirect segments we can receive requests with a number of segments bigger than the maximum number of allowed iovecs in a bios, so make sure that blkback doesn't try to allocate a bios with more iovecs than BIO_MAX_PAGES Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- drivers/block/xen-blkback/blkback.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index d622d86..876116b 100644 --- a/drivers/block/xen-blkback/blkback.c +++ b/drivers/block/xen-blkback/blkback.c @@ -1236,7 +1236,8 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif, seg[i].nsec << 9, seg[i].offset) == 0)) { - bio = bio_alloc(GFP_KERNEL, nseg-i); + int nr_iovecs = (nseg-i) > BIO_MAX_PAGES ? BIO_MAX_PAGES : (nseg-i); + bio = bio_alloc(GFP_KERNEL, nr_iovecs); if (unlikely(bio == NULL)) goto fail_put_bio; -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Jan Beulich
2013-Jun-21 11:46 UTC
Re: [PATCH 3/4] xen-blkback: check the number of iovecs before allocating a bios
>>> On 21.06.13 at 12:56, Roger Pau Monne <roger.pau@citrix.com> wrote: > @@ -1236,7 +1236,8 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif, > seg[i].nsec << 9, > seg[i].offset) == 0)) { > > - bio = bio_alloc(GFP_KERNEL, nseg-i); > + int nr_iovecs = (nseg-i) > BIO_MAX_PAGES ? BIO_MAX_PAGES : (nseg-i);I''m sure this results in a compiler warning (declaration after statement). And it surely would read much better if you used some form of min() - the shorter line would at once allow you to properly blank separate operands from operators. Jan> + bio = bio_alloc(GFP_KERNEL, nr_iovecs); > if (unlikely(bio == NULL)) > goto fail_put_bio; >
Roger Pau Monné
2013-Jun-21 12:02 UTC
Re: [PATCH 3/4] xen-blkback: check the number of iovecs before allocating a bios
On 21/06/13 13:46, Jan Beulich wrote:>>>> On 21.06.13 at 12:56, Roger Pau Monne <roger.pau@citrix.com> wrote: >> @@ -1236,7 +1236,8 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif, >> seg[i].nsec << 9, >> seg[i].offset) == 0)) { >> >> - bio = bio_alloc(GFP_KERNEL, nseg-i); >> + int nr_iovecs = (nseg-i) > BIO_MAX_PAGES ? BIO_MAX_PAGES : (nseg-i); > > I''m sure this results in a compiler warning (declaration after > statement).No, because it''s the first statement inside the while loop.> And it surely would read much better if you used some form of > min() - the shorter line would at once allow you to properly > blank separate operands from operators.Sure, I will switch it to min, thanks for the comments.> > Jan > >> + bio = bio_alloc(GFP_KERNEL, nr_iovecs); >> if (unlikely(bio == NULL)) >> goto fail_put_bio; >> > >
Roger Pau Monne
2013-Jun-22 07:59 UTC
[PATCH v2 3/4] xen-blkback: check the number of iovecs before allocating a bios
With the introduction of indirect segments we can receive requests with a number of segments bigger than the maximum number of allowed iovecs in a bios, so make sure that blkback doesn't try to allocate a bios with more iovecs than BIO_MAX_PAGES Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- drivers/block/xen-blkback/blkback.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c index d622d86..b3897f5d 100644 --- a/drivers/block/xen-blkback/blkback.c +++ b/drivers/block/xen-blkback/blkback.c @@ -1236,7 +1236,8 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif, seg[i].nsec << 9, seg[i].offset) == 0)) { - bio = bio_alloc(GFP_KERNEL, nseg-i); + int nr_iovecs = min_t(int, (nseg-i), BIO_MAX_PAGES); + bio = bio_alloc(GFP_KERNEL, nr_iovecs); if (unlikely(bio == NULL)) goto fail_put_bio; -- 1.7.7.5 (Apple Git-26) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Konrad Rzeszutek Wilk
2013-Jun-24 13:28 UTC
Re: [PATCH v2 3/4] xen-blkback: check the number of iovecs before allocating a bios
On Sat, Jun 22, 2013 at 09:59:17AM +0200, Roger Pau Monne wrote:> With the introduction of indirect segments we can receive requests > with a number of segments bigger than the maximum number of allowed > iovecs in a bios, so make sure that blkback doesn''t try to allocate a > bios with more iovecs than BIO_MAX_PAGESShouldn''t we just gate the feature-indirect-descriptor value to take this into account? What happens if the nseg is > BIO_MAX_PAGES? Do we "lose" the request for the remaining segments?> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > --- > drivers/block/xen-blkback/blkback.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c > index d622d86..b3897f5d 100644 > --- a/drivers/block/xen-blkback/blkback.c > +++ b/drivers/block/xen-blkback/blkback.c > @@ -1236,7 +1236,8 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif, > seg[i].nsec << 9, > seg[i].offset) == 0)) { > > - bio = bio_alloc(GFP_KERNEL, nseg-i); > + int nr_iovecs = min_t(int, (nseg-i), BIO_MAX_PAGES); > + bio = bio_alloc(GFP_KERNEL, nr_iovecs); > if (unlikely(bio == NULL)) > goto fail_put_bio; > > -- > 1.7.7.5 (Apple Git-26) >
Roger Pau Monné
2013-Jun-25 08:03 UTC
Re: [PATCH v2 3/4] xen-blkback: check the number of iovecs before allocating a bios
On 24/06/13 15:28, Konrad Rzeszutek Wilk wrote:> On Sat, Jun 22, 2013 at 09:59:17AM +0200, Roger Pau Monne wrote: >> With the introduction of indirect segments we can receive requests >> with a number of segments bigger than the maximum number of allowed >> iovecs in a bios, so make sure that blkback doesn''t try to allocate a >> bios with more iovecs than BIO_MAX_PAGES > > Shouldn''t we just gate the feature-indirect-descriptor value to > take this into account? > > What happens if the nseg is > BIO_MAX_PAGES? Do we "lose" the request > for the remaining segments?No, we just allocate several bios in order to send the request to the underlying device.