diff mbox

[dpdk-dev,1/3] kni: minor opto

Message ID 1433359137-12720-1-git-send-email-rolette@infiniteio.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Jay Rolette June 3, 2015, 7:18 p.m. UTC 
  Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from the list as you iterate over it

Signed-off-by: Jay Rolette <rolette@infiniteio.com>
---
 lib/librte_eal/linuxapp/kni/kni_misc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Bruce Richardson June 4, 2015, 1:39 p.m. UTC | #1 
On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from the list as you iterate over it
> 
> Signed-off-by: Jay Rolette <rolette@infiniteio.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Bruce Richardson June 4, 2015, 1:40 p.m. UTC | #2 
On Thu, Jun 04, 2015 at 02:39:17PM +0100, Bruce Richardson wrote:
> On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> > Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from the list as you iterate over it
> > 
> > Signed-off-by: Jay Rolette <rolette@infiniteio.com>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
Forgot to mention though, that the commit title needs to be a little more
descriptive.
Thomas Monjalon June 4, 2015, 3:02 p.m. UTC | #3 
2015-06-04 14:40, Bruce Richardson:
> On Thu, Jun 04, 2015 at 02:39:17PM +0100, Bruce Richardson wrote:
> > On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> > > Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from the list as you iterate over it
> > > 
> > > Signed-off-by: Jay Rolette <rolette@infiniteio.com>
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > 
> Forgot to mention though, that the commit title needs to be a little more
> descriptive.

So you should not ack this version ;)
Bruce Richardson June 4, 2015, 3:05 p.m. UTC | #4 
On Thu, Jun 04, 2015 at 05:02:06PM +0200, Thomas Monjalon wrote:
> 2015-06-04 14:40, Bruce Richardson:
> > On Thu, Jun 04, 2015 at 02:39:17PM +0100, Bruce Richardson wrote:
> > > On Wed, Jun 03, 2015 at 02:18:55PM -0500, Jay Rolette wrote:
> > > > Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from the list as you iterate over it
> > > > 
> > > > Signed-off-by: Jay Rolette <rolette@infiniteio.com>
> > > 
> > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > > 
> > Forgot to mention though, that the commit title needs to be a little more
> > descriptive.
> 
> So you should not ack this version ;)
> 
Code and text description looked fine, so I thought an ack otherwise appropriate.
However, I will refrain from doing so in future :-)

/Bruce
Zhang, Helin June 15, 2015, 2:07 a.m. UTC | #5 
Would it be better to modify the similar thing in kni_ioctl_create()?

- Helin

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette
> Sent: Thursday, June 4, 2015 3:19 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/3] kni: minor opto
> 
> Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from
> the list as you iterate over it
> 
> Signed-off-by: Jay Rolette <rolette@infiniteio.com>
> ---
>  lib/librte_eal/linuxapp/kni/kni_misc.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
> b/lib/librte_eal/linuxapp/kni/kni_misc.c
> index 1935d32..312f196 100644
> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> @@ -213,13 +213,12 @@ static int
>  kni_thread_single(void *unused)
>  {
>  	int j;
> -	struct kni_dev *dev, *n;
> +	struct kni_dev *dev;
> 
>  	while (!kthread_should_stop()) {
>  		down_read(&kni_list_lock);
>  		for (j = 0; j < KNI_RX_LOOP_NUM; j++) {
> -			list_for_each_entry_safe(dev, n,
> -					&kni_list_head, list) {
> +			list_for_each_entry(dev, &kni_list_head, list) {
>  #ifdef RTE_KNI_VHOST
>  				kni_chk_vhost_rx(dev);
>  #else
> --
> 2.3.2 (Apple Git-55)
Jay Rolette June 15, 2015, 12:42 p.m. UTC | #6 
On Sun, Jun 14, 2015 at 9:07 PM, Zhang, Helin <helin.zhang@intel.com> wrote:

> Would it be better to modify the similar thing in kni_ioctl_create()?
>

That one doesn't need to use the "safe" version of list_for_each_entry()
either, but it isn't in the packet processing path so the minor performance
improvement doesn't really matter.


>
> - Helin
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jay Rolette
> > Sent: Thursday, June 4, 2015 3:19 AM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/3] kni: minor opto
> >
> > Don't need the 'safe' version of list_for_each_entry() if you aren't
> deleting from
> > the list as you iterate over it
> >
> > Signed-off-by: Jay Rolette <rolette@infiniteio.com>
> > ---
> >  lib/librte_eal/linuxapp/kni/kni_misc.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c
> > b/lib/librte_eal/linuxapp/kni/kni_misc.c
> > index 1935d32..312f196 100644
> > --- a/lib/librte_eal/linuxapp/kni/kni_misc.c
> > +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
> > @@ -213,13 +213,12 @@ static int
> >  kni_thread_single(void *unused)
> >  {
> >       int j;
> > -     struct kni_dev *dev, *n;
> > +     struct kni_dev *dev;
> >
> >       while (!kthread_should_stop()) {
> >               down_read(&kni_list_lock);
> >               for (j = 0; j < KNI_RX_LOOP_NUM; j++) {
> > -                     list_for_each_entry_safe(dev, n,
> > -                                     &kni_list_head, list) {
> > +                     list_for_each_entry(dev, &kni_list_head, list) {
> >  #ifdef RTE_KNI_VHOST
> >                               kni_chk_vhost_rx(dev);
> >  #else
> > --
> > 2.3.2 (Apple Git-55)
>
>
Zhang, Helin June 16, 2015, 1:15 a.m. UTC | #7 
Hi Jay

From: Jay Rolette [mailto:rolette@infiniteio.com]

Sent: Monday, June 15, 2015 8:43 PM
To: Zhang, Helin
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/3] kni: minor opto

On Sun, Jun 14, 2015 at 9:07 PM, Zhang, Helin <helin.zhang@intel.com<mailto:helin.zhang@intel.com>> wrote:
Would it be better to modify the similar thing in kni_ioctl_create()?

That one doesn't need to use the "safe" version of list_for_each_entry() either, but it isn't in the packet processing path so the minor performance improvement doesn't really matter.
Yes, your patches are OK for me. I have acked it.


-          Helin


- Helin

> -----Original Message-----

> From: dev [mailto:dev-bounces@dpdk.org<mailto:dev-bounces@dpdk.org>] On Behalf Of Jay Rolette

> Sent: Thursday, June 4, 2015 3:19 AM

> To: dev@dpdk.org<mailto:dev@dpdk.org>

> Subject: [dpdk-dev] [PATCH 1/3] kni: minor opto

>

> Don't need the 'safe' version of list_for_each_entry() if you aren't deleting from

> the list as you iterate over it

>

> Signed-off-by: Jay Rolette <rolette@infiniteio.com<mailto:rolette@infiniteio.com>>

> ---

>  lib/librte_eal/linuxapp/kni/kni_misc.c | 5 ++---

>  1 file changed, 2 insertions(+), 3 deletions(-)

>

> diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c

> b/lib/librte_eal/linuxapp/kni/kni_misc.c

> index 1935d32..312f196 100644

> --- a/lib/librte_eal/linuxapp/kni/kni_misc.c

> +++ b/lib/librte_eal/linuxapp/kni/kni_misc.c

> @@ -213,13 +213,12 @@ static int

>  kni_thread_single(void *unused)

>  {

>       int j;

> -     struct kni_dev *dev, *n;

> +     struct kni_dev *dev;

>

>       while (!kthread_should_stop()) {

>               down_read(&kni_list_lock);

>               for (j = 0; j < KNI_RX_LOOP_NUM; j++) {

> -                     list_for_each_entry_safe(dev, n,

> -                                     &kni_list_head, list) {

> +                     list_for_each_entry(dev, &kni_list_head, list) {

>  #ifdef RTE_KNI_VHOST

>                               kni_chk_vhost_rx(dev);

>  #else

> --

> 2.3.2 (Apple Git-55)
Thomas Monjalon June 16, 2015, 3:20 p.m. UTC | #8 
2015-06-16 01:15, Zhang, Helin:
> Yes, your patches are OK for me. I have acked it.

Series applied, thanks
diff mbox

Patch

diff --git a/lib/librte_eal/linuxapp/kni/kni_misc.c b/lib/librte_eal/linuxapp/kni/kni_misc.c
index 1935d32..312f196 100644
--- a/lib/librte_eal/linuxapp/kni/kni_misc.c
+++ b/lib/librte_eal/linuxapp/kni/kni_misc.c
@@ -213,13 +213,12 @@  static int
 kni_thread_single(void *unused)
 {
 	int j;
-	struct kni_dev *dev, *n;
+	struct kni_dev *dev;
 
 	while (!kthread_should_stop()) {
 		down_read(&kni_list_lock);
 		for (j = 0; j < KNI_RX_LOOP_NUM; j++) {
-			list_for_each_entry_safe(dev, n,
-					&kni_list_head, list) {
+			list_for_each_entry(dev, &kni_list_head, list) {
 #ifdef RTE_KNI_VHOST
 				kni_chk_vhost_rx(dev);
 #else