public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: David Vrabel <david.vrabel@citrix•com>
To: Paul Durrant <Paul.Durrant@citrix•com>
Cc: Jonathan Davies <Jonathan.Davies@citrix•com>,
	Wei Liu <wei.liu2@citrix•com>,
	Ian Campbell <Ian.Campbell@citrix•com>,
	"netdev@vger•kernel.org" <netdev@vger•kernel.org>,
	"linux-kernel@vger•kernel.org" <linux-kernel@vger•kernel.org>,
	Stefano Stabellini <Stefano.Stabellini@citrix•com>,
	David Vrabel <david.vrabel@citrix•com>,
	Zoltan Kiss <zoltan.kiss@citrix•com>,
	"xen-devel@lists•xenproject.org" <xen-devel@lists•xenproject.org>,
	"boris.ostrovsky@oracle•com" <boris.ostrovsky@oracle•com>
Subject: Re: [PATCH] grant-table, xen-netback: Introduce helper functions for grant copy operations
Date: Thu, 3 Apr 2014 10:48:58 +0100	[thread overview]
Message-ID: <533D2E8A.90101@citrix.com> (raw)
In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD02BB106@AMSPEX01CL01.citrite.net>

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

  reply	other threads:[~2014-04-03  9:48 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-02 17:05 [PATCH] grant-table, xen-netback: Introduce helper functions for grant copy operations Zoltan Kiss
2014-04-03  8:12 ` Paul Durrant
2014-04-03  9:48   ` David Vrabel [this message]
2014-04-04 14:50     ` [Xen-devel] " Ian Campbell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=533D2E8A.90101@citrix.com \
    --to=david.vrabel@citrix$(echo .)com \
    --cc=Ian.Campbell@citrix$(echo .)com \
    --cc=Jonathan.Davies@citrix$(echo .)com \
    --cc=Paul.Durrant@citrix$(echo .)com \
    --cc=Stefano.Stabellini@citrix$(echo .)com \
    --cc=boris.ostrovsky@oracle$(echo .)com \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=wei.liu2@citrix$(echo .)com \
    --cc=xen-devel@lists$(echo .)xenproject.org \
    --cc=zoltan.kiss@citrix$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox