On Mon, 2014-03-31 at 16:08 +0100, Zoltan Kiss wrote:
>
> check_frags:
> - for (i = start; i < nr_frags; i++) {
> + for (i = 0; i < nr_frags; i++, gop_map++) {
> int j, newerr;
>
> pending_idx = frag_get_pending_idx(&shinfo->frags[i]);
> - tx_info = &vif->pending_tx_info[pending_idx];
>
> /* Check error status: if okay then remember grant handle. */
> - newerr = (++gop_map)->status;
> + newerr = (gop_map)->status;
You've reworked the handling of gop_map and when and where it is
incremented, which might be a legit cleanup but does it relate to the
bulk of this change somehow that I'm missing?
> [...]
> __skb_put(skb, data_len);
> + vif->tx_copy_ops[*copy_ops].source.u.ref = txreq.gref;
> + vif->tx_copy_ops[*copy_ops].source.domid = vif->domid;
> + vif->tx_copy_ops[*copy_ops].source.offset = txreq.offset;
> +
> + vif->tx_copy_ops[*copy_ops].dest.u.gmfn =
> + virt_to_mfn(skb->data);
> + vif->tx_copy_ops[*copy_ops].dest.domid = DOMID_SELF;
> + vif->tx_copy_ops[*copy_ops].dest.offset =
> + offset_in_page(skb->data);
> +
> + vif->tx_copy_ops[*copy_ops].len = data_len;
> + vif->tx_copy_ops[*copy_ops].flags = GNTCOPY_source_gref;
We have gnttab_set_map_op. Should we have gnttap_set_copy_op too?
> - BUG_ON(ret);
> + else {
> + gnttab_batch_copy(vif->tx_copy_ops, nr_cops);
> + if (nr_mops != 0) {
if (nr_mops) would do.
> + ret = gnttab_map_refs(vif->tx_map_ops,
So we use gnttab_batch_copy and gnttab_map_refs.
Shouldn't we either use gnttab_batch_copy and gnttab_batch_map or
gnttab_copy gnttab_map_refs. (where gnttab_copy might be a bare
GNTTABOP_copy or might be a helper wrapper).
The point of the batch interface is to handle page unsharing etc, but
doing it only for copies seems like a waste one way or another.
#include <paul's-comments>
Ian.
--
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/
