netdev.vger.kernel.org archive mirror
 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

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

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <20080420.155924.86075645.davem@davemloft.net>]

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).