All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bernard Pidoux <pidoux@ccr.jussieu.fr>
To: David Miller <davem@davemloft.net>
Cc: ralf@linux-mips.org, linux-kernel@vger.kernel.org,
	linux-hams@vger.kernel.org,
	Linux Netdev List <netdev@vger.kernel.org>
Subject: Re: [PATCH] soft lockup rose_node_list_lock
Date: Mon, 21 Apr 2008 22:27:27 +0200	[thread overview]
Message-ID: <480CF8AF.9040501@ccr.jussieu.fr> (raw)
In-Reply-To: <20080420.155924.86075645.davem@davemloft.net>

Hi David,

I also spent a lot of time to understand how rose behaved and I agree 
that it is difficult to decifer a code especially dealing
with socket programming and when it was written by someone else.
But as a radioamateur, Linux is a hobby for me and I like to learn.

Actually, rose_get_neigh() is called when two different events are 
occuring :

- first, it is called by rose_connect() in order to find if an adjacent 
node is ready to route to a specific ROSE address.
- second, rose_route_frame() calls rose_get_neigh() every time an 
incoming frame must be routed to an appropriate AX25 connection.

By the way, rose_get_neigh() function is not optimized for it does not 
check if an adjacent node is already connected before a new connect is 
requested.
For this purpose I have derived a new function, I named 
rose_get_route(), that is called by rose_route_frame() to find a route 
via an adjacent node.
This function has been tested for months now and it works fine.
It adds the automatic frames routing that rose needed desperately.
I will send next a patch with this new rose_get_route().

Bernard Pidoux

p.s. my email client is set for MIME attachements, but it seems corrupted.
    I will fix that. Sorry for the unvoluntary increase of workload it 
gave you.


David Miller a écrit :
> From: Bernard Pidoux <pidoux@ccr.jussieu.fr>
> Date: Sun, 20 Apr 2008 19:09:23 +0200
>
>   
>> Since rose_route_frame() does not use rose_node_list we can safely
>> remove rose_node_list_lock spin lock here and let it be free for
>> rose_get_neigh().
>>
>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>>     
>
> Indeed, I went over this code several times and I can't
> see any reason for rose_route_frame() to take the node
> list lock.
>
> Patch applied, thanks Bernard.  But one thing...
>
>   
>> diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
>> index fb9359f..5053a53 100644
>> --- a/net/rose/rose_route.c
>> +++ b/net/rose/rose_route.c
>> @@ -857,7 +857,6 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
>>          src_addr  = (rose_address *)(skb->data + 9);
>>          dest_addr = (rose_address *)(skb->data + 4);
>>
>> -       spin_lock_bh(&rose_node_list_lock);
>>          spin_lock_bh(&rose_neigh_list_lock);
>>          spin_lock_bh(&rose_route_list_lock);
>>
>>     
>
> Could you please fix your email client so it doesn't corrupt
> patches like this?  I've had to apply all of your patches by
> hand because the tabs have been converted into spaces.  Use
> MIME attachments if you have to.
>
> Thanks again.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-hams" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Bernard Pidoux <pidoux@ccr.jussieu.fr>
To: David Miller <davem@davemloft.net>
Cc: ralf@linux-mips.org, linux-kernel@vger.kernel.org,
	linux-hams@vger.kernel.org,
	Linux Netdev List <netdev@vger.kernel.org>
Subject: Re: [PATCH] soft lockup rose_node_list_lock
Date: Mon, 21 Apr 2008 22:27:27 +0200	[thread overview]
Message-ID: <480CF8AF.9040501@ccr.jussieu.fr> (raw)
In-Reply-To: <20080420.155924.86075645.davem@davemloft.net>

Hi David,

I also spent a lot of time to understand how rose behaved and I agree 
that it is difficult to decifer a code especially dealing
with socket programming and when it was written by someone else.
But as a radioamateur, Linux is a hobby for me and I like to learn.

Actually, rose_get_neigh() is called when two different events are 
occuring :

- first, it is called by rose_connect() in order to find if an adjacent 
node is ready to route to a specific ROSE address.
- second, rose_route_frame() calls rose_get_neigh() every time an 
incoming frame must be routed to an appropriate AX25 connection.

By the way, rose_get_neigh() function is not optimized for it does not 
check if an adjacent node is already connected before a new connect is 
requested.
For this purpose I have derived a new function, I named 
rose_get_route(), that is called by rose_route_frame() to find a route 
via an adjacent node.
This function has been tested for months now and it works fine.
It adds the automatic frames routing that rose needed desperately.
I will send next a patch with this new rose_get_route().

Bernard Pidoux

p.s. my email client is set for MIME attachements, but it seems corrupted.
    I will fix that. Sorry for the unvoluntary increase of workload it 
gave you.


David Miller a écrit :
> From: Bernard Pidoux <pidoux@ccr.jussieu.fr>
> Date: Sun, 20 Apr 2008 19:09:23 +0200
>
>   
>> Since rose_route_frame() does not use rose_node_list we can safely
>> remove rose_node_list_lock spin lock here and let it be free for
>> rose_get_neigh().
>>
>> Signed-off-by: Bernard Pidoux <f6bvp@amsat.org>
>>     
>
> Indeed, I went over this code several times and I can't
> see any reason for rose_route_frame() to take the node
> list lock.
>
> Patch applied, thanks Bernard.  But one thing...
>
>   
>> diff --git a/net/rose/rose_route.c b/net/rose/rose_route.c
>> index fb9359f..5053a53 100644
>> --- a/net/rose/rose_route.c
>> +++ b/net/rose/rose_route.c
>> @@ -857,7 +857,6 @@ int rose_route_frame(struct sk_buff *skb, ax25_cb *ax25)
>>          src_addr  = (rose_address *)(skb->data + 9);
>>          dest_addr = (rose_address *)(skb->data + 4);
>>
>> -       spin_lock_bh(&rose_node_list_lock);
>>          spin_lock_bh(&rose_neigh_list_lock);
>>          spin_lock_bh(&rose_route_list_lock);
>>
>>     
>
> Could you please fix your email client so it doesn't corrupt
> patches like this?  I've had to apply all of your patches by
> hand because the tabs have been converted into spaces.  Use
> MIME attachments if you have to.
>
> Thanks again.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hams" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>   


  reply	other threads:[~2008-04-21 20:27 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-19 21:12 [PATCH] rose_node_list_lock was not released before returning to user space Bernard Pidoux
2008-04-20  1:38 ` David Miller
2008-04-20  1:40 ` David Miller
2008-04-20 17:09   ` [PATCH] soft lockup rose_node_list_lock Bernard Pidoux
2008-04-20 22:59     ` David Miller
2008-04-21 20:27       ` Bernard Pidoux [this message]
2008-04-21 20:27         ` Bernard Pidoux
2008-04-20 18:43 ` FPAC and Fedora8 Neville Cross
2008-04-21 10:57   ` Bernard Pidoux
2008-04-22  3:28     ` Neville Cross
2008-04-22  5:58       ` Bernard Pidoux

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=480CF8AF.9040501@ccr.jussieu.fr \
    --to=pidoux@ccr.jussieu.fr \
    --cc=davem@davemloft.net \
    --cc=linux-hams@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=ralf@linux-mips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.