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
>
>
>
next prev parent 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.