[PATCH v2 0/2] net/xen: Clean up

[PATCH v2 0/2] net/xen: Clean up

[Julien Grall]

Hi,

Those 2 patches was originally part of the Xen 64KB series [1]. They are
already acked/reviewed and  can go in Linux via the net tree now
without waiting the rest of the series.

Sincerely yours,

Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: netdev@vger.kernel.org

[1] http://lkml.org/lkml/2015/5/14/533

Julien Grall (2):
  net/xen-netfront: Correct printf format in xennet_get_responses
  net/xen-netback: Remove unused code in xenvif_rx_action

 drivers/net/xen-netback/netback.c | 5 -----
 drivers/net/xen-netfront.c        | 2 +-
 2 files changed, 1 insertion(+), 6 deletions(-)

-- 
2.1.4

[PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[Julien Grall]

rx->status is an int16_t, print it using %d rather than %u in order to
have a meaningful value when the field is negative.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: netdev@vger.kernel.org

---
    Changes in v2:
        - Add David's Reviewed-by
---
 drivers/net/xen-netfront.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index e031c94..c9768f8 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
 		if (unlikely(rx->status < 0 ||
 			     rx->offset + rx->status > PAGE_SIZE)) {
 			if (net_ratelimit())
-				dev_warn(dev, "rx->offset: %x, size: %u\n",
+				dev_warn(dev, "rx->offset: %x, size: %d\n",
 					 rx->offset, rx->status);
 			xennet_move_rx_slot(queue, skb, ref);
 			err = -EINVAL;
-- 
2.1.4

Re: [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[Joe Perches]

On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
> rx->status is an int16_t, print it using %d rather than %u in order to
> have a meaningful value when the field is negative.
[]
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
[]
> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
>  		if (unlikely(rx->status < 0 ||
>  			     rx->offset + rx->status > PAGE_SIZE)) {
>  			if (net_ratelimit())
> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
> +				dev_warn(dev, "rx->offset: %x, size: %d\n",

If you're going to do this, perhaps it'd be sensible to
also change the %x to %#x or 0x%x so that people don't
mistake offset without an [a-f] for decimal.

>  					 rx->offset, rx->status);

Re: [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[Julien Grall]

On 03/06/15 18:06, Joe Perches wrote:
> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
>> rx->status is an int16_t, print it using %d rather than %u in order to
>> have a meaningful value when the field is negative.
> []
>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> []
>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
>>  		if (unlikely(rx->status < 0 ||
>>  			     rx->offset + rx->status > PAGE_SIZE)) {
>>  			if (net_ratelimit())
>> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
>> +				dev_warn(dev, "rx->offset: %x, size: %d\n",
> 
> If you're going to do this, perhaps it'd be sensible to
> also change the %x to %#x or 0x%x so that people don't
> mistake offset without an [a-f] for decimal.

Good idea. I will resend a version of this series.

David, can I keep you Reviewed-by for this change?

-- 
Julien Grall

Re: [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[David Vrabel]

On 04/06/15 13:45, Julien Grall wrote:
> On 03/06/15 18:06, Joe Perches wrote:
>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
>>> rx->status is an int16_t, print it using %d rather than %u in order to
>>> have a meaningful value when the field is negative.
>> []
>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> []
>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
>>>  		if (unlikely(rx->status < 0 ||
>>>  			     rx->offset + rx->status > PAGE_SIZE)) {
>>>  			if (net_ratelimit())
>>> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
>>> +				dev_warn(dev, "rx->offset: %x, size: %d\n",
>>
>> If you're going to do this, perhaps it'd be sensible to
>> also change the %x to %#x or 0x%x so that people don't
>> mistake offset without an [a-f] for decimal.
> 
> Good idea. I will resend a version of this series.
> 
> David, can I keep you Reviewed-by for this change?#

Can you make the offset %d instead?

You can keep the reviewed-by if you do this.

David

Re: [Xen-devel] [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[Julien Grall]

On 04/06/15 13:46, David Vrabel wrote:
> On 04/06/15 13:45, Julien Grall wrote:
>> On 03/06/15 18:06, Joe Perches wrote:
>>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
>>>> rx->status is an int16_t, print it using %d rather than %u in order to
>>>> have a meaningful value when the field is negative.
>>> []
>>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>>> []
>>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
>>>>  		if (unlikely(rx->status < 0 ||
>>>>  			     rx->offset + rx->status > PAGE_SIZE)) {
>>>>  			if (net_ratelimit())
>>>> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
>>>> +				dev_warn(dev, "rx->offset: %x, size: %d\n",
>>>
>>> If you're going to do this, perhaps it'd be sensible to
>>> also change the %x to %#x or 0x%x so that people don't
>>> mistake offset without an [a-f] for decimal.
>>
>> Good idea. I will resend a version of this series.
>>
>> David, can I keep you Reviewed-by for this change?#
> 
> Can you make the offset %d instead?

Sure.

> 
> You can keep the reviewed-by if you do this.

Thank you.



-- 
Julien Grall

Re: [Xen-devel] [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[Joe Perches]

On Thu, 2015-06-04 at 13:52 +0100, Julien Grall wrote:
> On 04/06/15 13:46, David Vrabel wrote:
> > On 04/06/15 13:45, Julien Grall wrote:
> >> On 03/06/15 18:06, Joe Perches wrote:
> >>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
> >>>> rx->status is an int16_t, print it using %d rather than %u in order to
> >>>> have a meaningful value when the field is negative.
> >>> []
> >>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> >>> []
> >>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
> >>>>  		if (unlikely(rx->status < 0 ||
> >>>>  			     rx->offset + rx->status > PAGE_SIZE)) {
> >>>>  			if (net_ratelimit())
> >>>> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
> >>>> +				dev_warn(dev, "rx->offset: %x, size: %d\n",
> >>>
> >>> If you're going to do this, perhaps it'd be sensible to
> >>> also change the %x to %#x or 0x%x so that people don't
> >>> mistake offset without an [a-f] for decimal.
> >>
> >> Good idea. I will resend a version of this series.
> >>
> >> David, can I keep you Reviewed-by for this change?#
> > 
> > Can you make the offset %d instead?

If you do, please change similar uses in
drivers/net/xen-netback/ in the same patch.

Re: [Xen-devel] [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[Julien Grall]

On 04/06/15 17:25, Joe Perches wrote:
> On Thu, 2015-06-04 at 13:52 +0100, Julien Grall wrote:
>> On 04/06/15 13:46, David Vrabel wrote:
>>> On 04/06/15 13:45, Julien Grall wrote:
>>>> On 03/06/15 18:06, Joe Perches wrote:
>>>>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
>>>>>> rx->status is an int16_t, print it using %d rather than %u in order to
>>>>>> have a meaningful value when the field is negative.
>>>>> []
>>>>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>>>>> []
>>>>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
>>>>>>  		if (unlikely(rx->status < 0 ||
>>>>>>  			     rx->offset + rx->status > PAGE_SIZE)) {
>>>>>>  			if (net_ratelimit())
>>>>>> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
>>>>>> +				dev_warn(dev, "rx->offset: %x, size: %d\n",
>>>>>
>>>>> If you're going to do this, perhaps it'd be sensible to
>>>>> also change the %x to %#x or 0x%x so that people don't
>>>>> mistake offset without an [a-f] for decimal.
>>>>
>>>> Good idea. I will resend a version of this series.
>>>>
>>>> David, can I keep you Reviewed-by for this change?#
>>>
>>> Can you make the offset %d instead?
> 
> If you do, please change similar uses in
> drivers/net/xen-netback/ in the same patch.

The format is not really consistent accross the 2 drivers and even
within the same driver (see pending_idx which is some times print with
%x and %d).

Anyway, ss it's a different drivers and maintainers I will prefer to
send a separate patch for this.

Regards,

-- 
Julien Grall

Re: [Xen-devel] [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[Joe Perches]

On Fri, 2015-06-05 at 13:34 +0100, Julien Grall wrote:
> On 04/06/15 17:25, Joe Perches wrote:
> > On Thu, 2015-06-04 at 13:52 +0100, Julien Grall wrote:
> >> On 04/06/15 13:46, David Vrabel wrote:
> >>> On 04/06/15 13:45, Julien Grall wrote:
> >>>> On 03/06/15 18:06, Joe Perches wrote:
> >>>>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
> >>>>>> rx->status is an int16_t, print it using %d rather than %u in order to
> >>>>>> have a meaningful value when the field is negative.
> >>>>> []
> >>>>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> >>>>> []
> >>>>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
> >>>>>>  		if (unlikely(rx->status < 0 ||
> >>>>>>  			     rx->offset + rx->status > PAGE_SIZE)) {
> >>>>>>  			if (net_ratelimit())
> >>>>>> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
> >>>>>> +				dev_warn(dev, "rx->offset: %x, size: %d\n",
> >>>>>
> >>>>> If you're going to do this, perhaps it'd be sensible to
> >>>>> also change the %x to %#x or 0x%x so that people don't
> >>>>> mistake offset without an [a-f] for decimal.
> >>>>
> >>>> Good idea. I will resend a version of this series.
> >>>>
> >>>> David, can I keep you Reviewed-by for this change?#
> >>>
> >>> Can you make the offset %d instead?
> > 
> > If you do, please change similar uses in
> > drivers/net/xen-netback/ in the same patch.
> 
> The format is not really consistent accross the 2 drivers and even
> within the same driver (see pending_idx which is some times print with
> %x and %d).
> 
> Anyway, ss it's a different drivers and maintainers I will prefer to
> send a separate patch for this.

It's all xen to me, but thanks.

Re: [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses

[David Miller]

From: Julien Grall <julien.grall@citrix.com>
Date: Thu, 4 Jun 2015 13:45:00 +0100

> On 03/06/15 18:06, Joe Perches wrote:
>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
>>> rx->status is an int16_t, print it using %d rather than %u in order to
>>> have a meaningful value when the field is negative.
>> []
>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>> []
>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue *queue,
>>>  		if (unlikely(rx->status < 0 ||
>>>  			     rx->offset + rx->status > PAGE_SIZE)) {
>>>  			if (net_ratelimit())
>>> -				dev_warn(dev, "rx->offset: %x, size: %u\n",
>>> +				dev_warn(dev, "rx->offset: %x, size: %d\n",
>> 
>> If you're going to do this, perhaps it'd be sensible to
>> also change the %x to %#x or 0x%x so that people don't
>> mistake offset without an [a-f] for decimal.
> 
> Good idea. I will resend a version of this series.
> 
> David, can I keep you Reviewed-by for this change?

Sure.

[PATCH v2 2/2] net/xen-netback: Remove unused code in xenvif_rx_action

[Julien Grall]

The variables old_req_cons and ring_slots_used are assigned but never
used since commit 1650d5455bd2dc6b5ee134bd6fc1a3236c266b5b "xen-netback:
always fully coalesce guest Rx packets".

Signed-off-by: Julien Grall <julien.grall@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: netdev@vger.kernel.org

---
    Changes in v2:
        - Add Wei's Acked-by
---
 drivers/net/xen-netback/netback.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 0d25943..ba3ae30 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -515,14 +515,9 @@ static void xenvif_rx_action(struct xenvif_queue *queue)
 
 	while (xenvif_rx_ring_slots_available(queue, XEN_NETBK_RX_SLOTS_MAX)
 	       && (skb = xenvif_rx_dequeue(queue)) != NULL) {
-		RING_IDX old_req_cons;
-		RING_IDX ring_slots_used;
-
 		queue->last_rx_time = jiffies;
 
-		old_req_cons = queue->rx.req_cons;
 		XENVIF_RX_CB(skb)->meta_slots_used = xenvif_gop_skb(skb, &npo, queue);
-		ring_slots_used = queue->rx.req_cons - old_req_cons;
 
 		__skb_queue_tail(&rxq, skb);
 	}
-- 
2.1.4