[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <533D2E8A.90101@citrix.com>
Date: Thu, 3 Apr 2014 10:48:58 +0100
From: David Vrabel <david.vrabel@...rix.com>
To: Paul Durrant <Paul.Durrant@...rix.com>
CC: Zoltan Kiss <zoltan.kiss@...rix.com>,
Ian Campbell <Ian.Campbell@...rix.com>,
Wei Liu <wei.liu2@...rix.com>,
"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
"konrad.wilk@...cle.com" <konrad.wilk@...cle.com>,
"boris.ostrovsky@...cle.com" <boris.ostrovsky@...cle.com>,
David Vrabel <david.vrabel@...rix.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Stefano Stabellini <Stefano.Stabellini@...rix.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Jonathan Davies <Jonathan.Davies@...rix.com>
Subject: Re: [Xen-devel] [PATCH] grant-table, xen-netback: Introduce helper
functions for grant copy operations
On 03/04/14 09:12, Paul Durrant wrote:
> Zoltan Kiss wrote:
>>
>> Create helper functions for grant copy operations and use them in netback.
>>
[...]
>> --- a/drivers/net/xen-netback/netback.c
>> +++ b/drivers/net/xen-netback/netback.c
>> @@ -275,23 +275,29 @@ static void xenvif_gop_frag_copy(struct xenvif *vif,
>> struct sk_buff *skb,
>> bytes = MAX_BUFFER_OFFSET - npo->copy_off;
>>
>> copy_gop = npo->copy + npo->copy_prod++;
>> - copy_gop->flags = GNTCOPY_dest_gref;
>> - copy_gop->len = bytes;
>>
>> if (foreign_vif) {
>> - copy_gop->source.domid = foreign_vif->domid;
>> - copy_gop->source.u.ref = foreign_gref;
>> - copy_gop->flags |= GNTCOPY_source_gref;
>> + gnttab_set_copy_op_ref_to_ref(copy_gop,
>> + foreign_gref,
>> + npo->copy_gref,
>> + foreign_vif->domid,
>> + offset,
>> + vif->domid,
>> + npo->copy_off,
>> + bytes,
>> + GNTCOPY_dest_gref |
>> + GNTCOPY_source_gref);
>
> Can't you lose the flags here (and in the other variants)? Since
> they are implied by the variant of the function they could just be hardcoded
> there, couldn't they?
Even with that change these are still functions with 7 or 8 parameters
with no obvious order. That's just awful. The open-coded struct
initialization is better.
David
--
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