| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 19 Mar 2013 14:55:56 +0000
From: "Jan Beulich" <JBeulich@...e.com>
To: "Roger Pau Monne" <roger.pau@...rix.com>,
"Konrad Rzeszutek Wilk" <konrad.wilk@...cle.com>
Cc: <xen-devel@...ts.xen.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [Xen-devel] [PATCH 1/5] xen-blkback: don't store
dev_bus_addr
>>> On 19.03.13 at 15:32, Konrad Rzeszutek Wilk <konrad.wilk@...cle.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.pau@...rix.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.pau@...rix.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> Cc: xen-devel@...ts.xen.org
> [v1: s/buf/offset/]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@...cle.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-devel@...ts.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists