linux-kernel - Re: [Xen-devel] [PATCH v1 2/7] xen-blkback: use balloon pages for all mappings

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <516BB673.9060308@citrix.com>
Date:	Mon, 15 Apr 2013 10:12:35 +0200
From:	Roger Pau Monné <roger.pau@...rix.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"xen-devel@...ts.xen.org" <xen-devel@...ts.xen.org>
Subject: Re: [Xen-devel] [PATCH v1 2/7] xen-blkback: use balloon pages for
 all mappings

>> Perhaps you could update the comment from saying 'map this grant' (which
>> implies doing it NOW as opposed to have done it already), and say:
>>
>> /*
>> .. continue using the grant non-persistently. Note that
>> we mapped it in the earlier loop and the earlier if conditional
>> sets pending_handle(pending_req, i) = map[j].handle.
>> */
>>
>>
>>
>>>                       }
>>> -                     pending_handle(pending_req, i) =
>>> -                             persistent_gnts[i]->handle;
>>> -
>>> -                     if (ret)
>>> -                             continue;
>>> -             } else {
>>> -                     pending_handle(pending_req, i) = map[j++].handle;
>>> -                     bitmap_set(pending_req->unmap_seg, i, 1);
>>> -
>>> -                     if (ret)
>>> -                             continue;
>>> +                     persistent_gnt->gnt = map[j].ref;
>>> +                     persistent_gnt->handle = map[j].handle;
>>> +                     persistent_gnt->page = pages[i];
>>
>> Oh boy, that is a confusing. i and j. Keep loosing track which one is which.
>> It lookis right.
>>
>>> +                     if (add_persistent_gnt(&blkif->persistent_gnts,
>>> +                                            persistent_gnt)) {
>>> +                             kfree(persistent_gnt);
>>
>> I would also say 'persisten_gnt = NULL' for extra measure of safety

This loop was not completely right, I failed to set the unmap_seg bit if
we tried to map the grant persistently and failed, we would just jump to
next without doing:

bitmap_set(pending_req->unmap_seg, seg_idx, 1);
--
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/