WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: [PATCH 2/3] xen/blkback: Fix the inhibition to map p

On Tue, Oct 11, 2011 at 03:33:11PM +0800, Li Dongyang wrote:
> On Tue, Oct 11, 2011 at 3:25 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>>> On 10.10.11 at 17:28, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 
> >>>> wrote:
> >> The 'operation' parameters are the ones provided to the bio layer while
> >> the req->operation are the ones passed in between the backend and
> >> frontend. We used the wrong 'operation' value to squash the
> >> call to map pages when processing the discard operation resulting
> >> in mapping the pages unnecessarily.
> >>
> >> CC: Li Dongyang <lidongyang@xxxxxxxxxx>
> >> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> >> ---
> >>  drivers/block/xen-blkback/blkback.c |    2 +-
> >>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/block/xen-blkback/blkback.c
> >> b/drivers/block/xen-blkback/blkback.c
> >> index 184b133..3da9a40 100644
> >> --- a/drivers/block/xen-blkback/blkback.c
> >> +++ b/drivers/block/xen-blkback/blkback.c
> >> @@ -707,7 +707,7 @@ static int dispatch_rw_block_io(struct xen_blkif 
> >> *blkif,
> >>        * the hypercall to unmap the grants - that is all done in
> >>        * xen_blkbk_unmap.
> >>        */
> >> -     if (operation != BLKIF_OP_DISCARD &&
> >> +     if (operation != REQ_DISCARD &&
> >
> > Why is that check necessary in the first place? xen_blkbk_map() doesn't
> > do any harm when req->nr_segments is zero (as could also be the case
> > on WRITE_FLUSH ones).
> >
> Ah, you are right, we could remove this check then, Thanks

Except that req->nr_segments for blkif__request_discard is actually
the reserved field.

See:

struct blkif_request {
    uint8_t        operation;    /* BLKIF_OP_???                         */
    uint8_t        nr_segments;  /* number of segments                   */
    blkif_vdev_t   handle;       /* only for read/write requests         */
.. snip..

and:
struct blkif_request_discard {
    uint8_t        operation;    /* BLKIF_OP_DISCARD                     */
                                 /* ignored if 'discard-secure=0'        */
#define BLKIF_OP_DISCARD_FLAG_SECURE (1<<0)
    uint8_t        flag;         /* BLKIF_OP_DISCARD_FLAG_SECURE or 0    */
    blkif_vdev_t   handle;       /* same as for read/write requests      */

which will throw off the logic for nr_segments all wrong since for some
discard operations it would read the nr_segments as 1.

So we do need some logic in there to work with this.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>