>>> On 19.03.13 at 15:32, Konrad Rzeszutek Wilk <konrad.w...@oracle.com> wrote: > On Mon, Mar 18, 2013 at 05:49:32PM +0100, Roger Pau Monne wrote: >> dev_bus_addr returned in the grant ref map operation is the mfn of the >> passed page, there's no need to store it in the persistent grant >> entry, since we can always get it provided that we have the page. >> >> This reduces the memory overhead of persistent grants in blkback. > > I took this patch, but I redid it a bit: > > commit 1d4cb410befdb8b373c6fad604b39e0200e0bee0 > Author: Roger Pau Monne <roger....@citrix.com> > Date: Mon Mar 18 17:49:32 2013 +0100 > > xen-blkback: don't store dev_bus_addr > > dev_bus_addr returned in the grant ref map operation is the mfn of the > passed page, there's no need to store it in the persistent grant > entry, since we can always get it provided that we have the page. > > This reduces the memory overhead of persistent grants in blkback. > > While at it, rename the 'seg[i].buf' to be 'seg[i].offset' as > it makes much more sense - as we use that value in bio_add_page > which as the fourth argument expects the offset. > > We hadn't used the physical address as part of this at all. > > Signed-off-by: Roger Pau Monné <roger....@citrix.com> > Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> > Cc: xen-de...@lists.xen.org > [v1: s/buf/offset/] > Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> > > diff --git a/drivers/block/xen-blkback/blkback.c > b/drivers/block/xen-blkback/blkback.c > index 2cf8381..061c202 100644 > --- a/drivers/block/xen-blkback/blkback.c > +++ b/drivers/block/xen-blkback/blkback.c > @@ -442,7 +442,7 @@ int xen_blkif_schedule(void *arg) > } > > struct seg_buf { > - unsigned long buf; > + unsigned long offset;
If you touch this anyway, why don't you reduce the type to "unsigned int", halving the overall structure size? Even more, the field seems pointless to me altogether, since ... > unsigned int nsec; > }; > /* > @@ -621,30 +621,21 @@ static int xen_blkbk_map(struct blkif_request *req, > * If this is a new persistent grant > * save the handler > */ > - persistent_gnts[i]->handle = map[j].handle; > - persistent_gnts[i]->dev_bus_addr = > - map[j++].dev_bus_addr; > + persistent_gnts[i]->handle = map[j++].handle; > } > pending_handle(pending_req, i) = > persistent_gnts[i]->handle; > > if (ret) > continue; > - > - seg[i].buf = persistent_gnts[i]->dev_bus_addr | > - (req->u.rw.seg[i].first_sect << 9); > } else { > - pending_handle(pending_req, i) = map[j].handle; > + pending_handle(pending_req, i) = map[j++].handle; > bitmap_set(pending_req->unmap_seg, i, 1); > > - if (ret) { > - j++; > + if (ret) > continue; > - } > - > - seg[i].buf = map[j++].dev_bus_addr | > - (req->u.rw.seg[i].first_sect << 9); > } > + seg[i].offset = (req->u.rw.seg[i].first_sect << 9); ... this uses "i" as index on both sides, so ... > } > return ret; > } > @@ -971,7 +962,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif, > (bio_add_page(bio, > pages[i], > seg[i].nsec << 9, > - seg[i].buf & ~PAGE_MASK) == 0)) { > + seg[i].offset & ~PAGE_MASK) == 0)) { ... this one could as well use the original field. And the masking with ~PAGE_MASK is not pointless in any case. Jan > > bio = bio_alloc(GFP_KERNEL, nseg-i); > if (unlikely(bio == NULL)) > diff --git a/drivers/block/xen-blkback/common.h > b/drivers/block/xen-blkback/common.h > index da78346..60103e2 100644 > --- a/drivers/block/xen-blkback/common.h > +++ b/drivers/block/xen-blkback/common.h > @@ -187,7 +187,6 @@ struct persistent_gnt { > struct page *page; > grant_ref_t gnt; > grant_handle_t handle; > - uint64_t dev_bus_addr; > struct rb_node node; > }; > > > _______________________________________________ > Xen-devel mailing list > xen-de...@lists.xen.org > http://lists.xen.org/xen-devel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/