2015-06-03 16:56:05

by Julien Grall

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

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 <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: David Vrabel <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: [email protected]

[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


2015-06-03 16:56:19

by Julien Grall

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

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 <[email protected]>
Reviewed-by: David Vrabel <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: [email protected]

---
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

2015-06-03 16:56:46

by Julien Grall

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

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 <[email protected]>
Acked-by: Wei Liu <[email protected]>
Cc: Ian Campbell <[email protected]>
Cc: [email protected]

---
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

2015-06-03 17:18:26

by Joe Perches

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

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);

2015-06-04 12:45:56

by Julien Grall

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

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

2015-06-04 12:46:48

by David Vrabel

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

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

2015-06-04 12:52:57

by Julien Grall

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

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

2015-06-04 16:25:41

by Joe Perches

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

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.

2015-06-04 18:43:10

by David Miller

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

From: Julien Grall <[email protected]>
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.

2015-06-05 12:35:09

by Julien Grall

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

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

2015-06-05 15:49:07

by Joe Perches

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

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.