[PATCH] drm: Add the mutex protection in drm_do_vm_fault.
There are no mutex protection for the dev->map_hash while calling
the drm_ht_find_item in the function drm_do_vm_fault. So try to
mutex firstly and then find the list for using to avoid this race
condition.
Signed-off-by: Chen Jun
---
drivers/gpu/drm/drm_vm.c | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index b5c5af7..1d95221 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -107,8 +107,11 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
if (!dev->agp || !dev->agp->cant_use_aperture)
goto vm_fault_error;
- if (drm_ht_find_item(>map_hash, vma->vm_pgoff, ))
+ mutex_lock(>struct_mutex);
+ if (drm_ht_find_item(>map_hash, vma->vm_pgoff, )) {
+ mutex_unlock(>struct_mutex);
goto vm_fault_error;
+ }
r_list = drm_hash_entry(hash, struct drm_map_list, hash);
map = r_list->map;
@@ -140,8 +143,10 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
break;
}
- if (>head == >agp->memory)
+ if (>head == >agp->memory) {
+ mutex_unlock(>struct_mutex);
goto vm_fault_error;
+ }
/*
* Get the page, inc the use count, and return it
@@ -151,6 +156,7 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
get_page(page);
vmf->page = page;
+ mutex_unlock(>struct_mutex);
DRM_DEBUG
("baddr = 0x%llx page = 0x%p, offset = 0x%llx, count=%d\n",
(unsigned long long)baddr,
@@ -159,6 +165,7 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
page_count(page));
return 0;
}
+ mutex_unlock(>struct_mutex);
vm_fault_error:
return VM_FAULT_SIGBUS; /* Disallow mremap */
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drm: Add the mutex protection in drm_do_vm_fault.
There are no mutex protection for the dev-map_hash while calling
the drm_ht_find_item in the function drm_do_vm_fault. So try to
mutex firstly and then find the list for using to avoid this race
condition.
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
drivers/gpu/drm/drm_vm.c | 11 +--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c
index b5c5af7..1d95221 100644
--- a/drivers/gpu/drm/drm_vm.c
+++ b/drivers/gpu/drm/drm_vm.c
@@ -107,8 +107,11 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
if (!dev-agp || !dev-agp-cant_use_aperture)
goto vm_fault_error;
- if (drm_ht_find_item(dev-map_hash, vma-vm_pgoff, hash))
+ mutex_lock(dev-struct_mutex);
+ if (drm_ht_find_item(dev-map_hash, vma-vm_pgoff, hash)) {
+ mutex_unlock(dev-struct_mutex);
goto vm_fault_error;
+ }
r_list = drm_hash_entry(hash, struct drm_map_list, hash);
map = r_list-map;
@@ -140,8 +143,10 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
break;
}
- if (agpmem-head == dev-agp-memory)
+ if (agpmem-head == dev-agp-memory) {
+ mutex_unlock(dev-struct_mutex);
goto vm_fault_error;
+ }
/*
* Get the page, inc the use count, and return it
@@ -151,6 +156,7 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
get_page(page);
vmf-page = page;
+ mutex_unlock(dev-struct_mutex);
DRM_DEBUG
(baddr = 0x%llx page = 0x%p, offset = 0x%llx, count=%d\n,
(unsigned long long)baddr,
@@ -159,6 +165,7 @@ static int drm_do_vm_fault(struct vm_area_struct *vma,
struct vm_fault *vmf)
page_count(page));
return 0;
}
+ mutex_unlock(dev-struct_mutex);
vm_fault_error:
return VM_FAULT_SIGBUS; /* Disallow mremap */
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Inet-hashtable: Change the range of sk->hash lock to avoid the race condition.
On Thu, 2013-09-12 at 22:40 -0700, Eric Dumazet wrote:
> On Fri, 2013-09-13 at 05:47 -0400, Jun Chen wrote:
> > On Thu, 2013-09-12 at 05:00 -0700, Eric Dumazet wrote:
> > > On Thu, 2013-09-12 at 12:32 -0400, Jun Chen wrote:
> > > > When try to add node to list in __inet_hash_nolisten function, first
> > > > get the
> > > > list and then to lock for using, but in extremeness case, others can
> > > > del this
> > > > node before locking it, then the node should be null.So this patch try
> > > > to lock
> > > > firstly and then get the list for using to avoid this race condition.
> > >
> > > I suspect another bug. This should not happen.
> > >
> > > Care to describe the problem you got ?
> > >
> > > Thanks
> > >
> > >
> >
> > Ok, I just got this call stack and no more info, pls help to look it.
> > thanks!
> >
> > <1>[ 88.548263] BUG: unable to handle kernel NULL pointer dereference at
> > 0004
> > <1>[ 88.548490] IP: [] __inet_hash_nolisten+0xc1/0x140
> > <4>[ 88.548617] *pde =
> > <4>[ 88.549927] EIP is at __inet_hash_nolisten+0xc1/0x140
> > <4>[ 88.550008] EAX: EBX: e08c ECX: edf846e0 EDX: e08c0020
> > <4>[ 88.550055] ESI: c20213c0 EDI: edc12dc0 EBP: ce4bfdfc ESP: ce4bfde8
> > <4>[ 88.550137] DS: 007b ES: 007b FS: 00d8 GS: 003b SS: 0068
> > <4>[ 88.550184] CR0: 80050033 CR2: 0004 CR3: 2b4ff000 CR4: 001007d0
> > <4>[ 88.550266] DR0: DR1: DR2: DR3:
> > <4>[ 88.550346] DR6: 0ff0 DR7: 0400
> > <0>[ 88.550392] Process WebViewCoreThre (pid: 2137, ti=ce4be000
> > task=eb193c80 task.ti=ce4be000)
> > <0>[ 88.551746] Call Trace:
> > <4>[ 88.551797] [] __inet_hash_connect+0x295/0x2d0
> > <4>[ 88.551883] [] inet_hash_connect+0x40/0x50
> > <4>[ 88.551932] [] ? inet_unhash+0x90/0x90
> > <4>[ 88.551981] [] ? __inet_lookup_listener+0x1b0/0x1b0
> > <4>[ 88.552067] [] tcp_v4_connect+0x247/0x4a0
> > <4>[ 88.552117] [] ? lock_sock_nested+0x3e/0x50
> > <4>[ 88.552205] [] inet_stream_connect+0xe2/0x290
> > <4>[ 88.552254] [] ? _copy_from_user+0x35/0x50
> > <4>[ 88.552342] [] sys_connect+0xb2/0xd0
> > <4>[ 88.552393] [] ? alloc_file+0x20/0xa0
> > <4>[ 88.552441] [] ? tcp_setsockopt+0x50/0x60
> > <4>[ 88.552525] [] ? fget_light+0x44/0xe0
> > <4>[ 88.552574] [] ? sock_common_setsockopt+0x27/0x40
> > <4>[ 88.552659] [] ? _copy_from_user+0x35/0x50
> > <4>[ 88.552708] [] sys_socketcall+0xab/0x2b0
> > <4>[ 88.552790] [] ? trace_hardirqs_on_thunk+0xc/0x10
> > <4>[ 88.552840] [] syscall_call+0x7/0xb
> > <4>[ 88.552923] [] ? mutex_trylock+0x30/0x140
> >
>
> This makes no sense to me. This could be a random memory corruption.
>
> Do you have disassembly of __inet_hash_nolisten ?
>
>
I had disassembled the __inet_hash_nolisten+0xc1,
the corruption is located on the:
__inet_hash_nolisten -->
__sk_nulls_add_node_rcu(sk, list); -->
__sk_nulls_add_node_rcu -->
static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
struct hlist_nulls_head *h)
{
...
if (!is_a_nulls(first))
first->pprev = >next; (this line trigger corruption)
...
}
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Inet-hashtable: Change the range of sk-hash lock to avoid the race condition.
On Thu, 2013-09-12 at 22:40 -0700, Eric Dumazet wrote:
On Fri, 2013-09-13 at 05:47 -0400, Jun Chen wrote:
On Thu, 2013-09-12 at 05:00 -0700, Eric Dumazet wrote:
On Thu, 2013-09-12 at 12:32 -0400, Jun Chen wrote:
When try to add node to list in __inet_hash_nolisten function, first
get the
list and then to lock for using, but in extremeness case, others can
del this
node before locking it, then the node should be null.So this patch try
to lock
firstly and then get the list for using to avoid this race condition.
I suspect another bug. This should not happen.
Care to describe the problem you got ?
Thanks
Ok, I just got this call stack and no more info, pls help to look it.
thanks!
1[ 88.548263] BUG: unable to handle kernel NULL pointer dereference at
0004
1[ 88.548490] IP: [] __inet_hash_nolisten+0xc1/0x140
4[ 88.548617] *pde =
4[ 88.549927] EIP is at __inet_hash_nolisten+0xc1/0x140
4[ 88.550008] EAX: EBX: e08c ECX: edf846e0 EDX: e08c0020
4[ 88.550055] ESI: c20213c0 EDI: edc12dc0 EBP: ce4bfdfc ESP: ce4bfde8
4[ 88.550137] DS: 007b ES: 007b FS: 00d8 GS: 003b SS: 0068
4[ 88.550184] CR0: 80050033 CR2: 0004 CR3: 2b4ff000 CR4: 001007d0
4[ 88.550266] DR0: DR1: DR2: DR3:
4[ 88.550346] DR6: 0ff0 DR7: 0400
0[ 88.550392] Process WebViewCoreThre (pid: 2137, ti=ce4be000
task=eb193c80 task.ti=ce4be000)
0[ 88.551746] Call Trace:
4[ 88.551797] [] __inet_hash_connect+0x295/0x2d0
4[ 88.551883] [] inet_hash_connect+0x40/0x50
4[ 88.551932] [] ? inet_unhash+0x90/0x90
4[ 88.551981] [] ? __inet_lookup_listener+0x1b0/0x1b0
4[ 88.552067] [] tcp_v4_connect+0x247/0x4a0
4[ 88.552117] [] ? lock_sock_nested+0x3e/0x50
4[ 88.552205] [] inet_stream_connect+0xe2/0x290
4[ 88.552254] [] ? _copy_from_user+0x35/0x50
4[ 88.552342] [] sys_connect+0xb2/0xd0
4[ 88.552393] [] ? alloc_file+0x20/0xa0
4[ 88.552441] [] ? tcp_setsockopt+0x50/0x60
4[ 88.552525] [] ? fget_light+0x44/0xe0
4[ 88.552574] [] ? sock_common_setsockopt+0x27/0x40
4[ 88.552659] [] ? _copy_from_user+0x35/0x50
4[ 88.552708] [] sys_socketcall+0xab/0x2b0
4[ 88.552790] [] ? trace_hardirqs_on_thunk+0xc/0x10
4[ 88.552840] [] syscall_call+0x7/0xb
4[ 88.552923] [] ? mutex_trylock+0x30/0x140
This makes no sense to me. This could be a random memory corruption.
Do you have disassembly of __inet_hash_nolisten ?
I had disassembled the __inet_hash_nolisten+0xc1,
the corruption is located on the:
__inet_hash_nolisten --
__sk_nulls_add_node_rcu(sk, list); --
__sk_nulls_add_node_rcu --
static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
struct hlist_nulls_head *h)
{
...
if (!is_a_nulls(first))
first-pprev = n-next; (this line trigger corruption)
...
}
Thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Inet-hashtable: Change the range of sk->hash lock to avoid the race condition.
On Thu, 2013-09-12 at 05:00 -0700, Eric Dumazet wrote: > On Thu, 2013-09-12 at 12:32 -0400, Jun Chen wrote: > > When try to add node to list in __inet_hash_nolisten function, first get the > > list and then to lock for using, but in extremeness case, others can del > > this > > node before locking it, then the node should be null.So this patch try to > > lock > > firstly and then get the list for using to avoid this race condition. > > I suspect another bug. This should not happen. > > Care to describe the problem you got ? > > Thanks > > Ok, I just got this call stack and no more info, pls help to look it. thanks! <1>[ 88.548263] BUG: unable to handle kernel NULL pointer dereference at 0004 <1>[ 88.548490] IP: [] __inet_hash_nolisten+0xc1/0x140 <4>[ 88.548617] *pde = <4>[ 88.549927] EIP is at __inet_hash_nolisten+0xc1/0x140 <4>[ 88.550008] EAX: EBX: e08c ECX: edf846e0 EDX: e08c0020 <4>[ 88.550055] ESI: c20213c0 EDI: edc12dc0 EBP: ce4bfdfc ESP: ce4bfde8 <4>[ 88.550137] DS: 007b ES: 007b FS: 00d8 GS: 003b SS: 0068 <4>[ 88.550184] CR0: 80050033 CR2: 0004 CR3: 2b4ff000 CR4: 001007d0 <4>[ 88.550266] DR0: DR1: DR2: DR3: <4>[ 88.550346] DR6: 0ff0 DR7: 0400 <0>[ 88.550392] Process WebViewCoreThre (pid: 2137, ti=ce4be000 task=eb193c80 task.ti=ce4be000) <0>[ 88.551746] Call Trace: <4>[ 88.551797] [] __inet_hash_connect+0x295/0x2d0 <4>[ 88.551883] [] inet_hash_connect+0x40/0x50 <4>[ 88.551932] [] ? inet_unhash+0x90/0x90 <4>[ 88.551981] [] ? __inet_lookup_listener+0x1b0/0x1b0 <4>[ 88.552067] [] tcp_v4_connect+0x247/0x4a0 <4>[ 88.552117] [] ? lock_sock_nested+0x3e/0x50 <4>[ 88.552205] [] inet_stream_connect+0xe2/0x290 <4>[ 88.552254] [] ? _copy_from_user+0x35/0x50 <4>[ 88.552342] [] sys_connect+0xb2/0xd0 <4>[ 88.552393] [] ? alloc_file+0x20/0xa0 <4>[ 88.552441] [] ? tcp_setsockopt+0x50/0x60 <4>[ 88.552525] [] ? fget_light+0x44/0xe0 <4>[ 88.552574] [] ? sock_common_setsockopt+0x27/0x40 <4>[ 88.552659] [] ? _copy_from_user+0x35/0x50 <4>[ 88.552708] [] sys_socketcall+0xab/0x2b0 <4>[ 88.552790] [] ? trace_hardirqs_on_thunk+0xc/0x10 <4>[ 88.552840] [] syscall_call+0x7/0xb <4>[ 88.552923] [] ? mutex_trylock+0x30/0x140 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Inet-hashtable: Change the range of sk->hash lock to avoid the race condition.
When try to add node to list in __inet_hash_nolisten function, first get the
list and then to lock for using, but in extremeness case, others can del this
node before locking it, then the node should be null.So this patch try to lock
firstly and then get the list for using to avoid this race condition.
Signed-off-by: Chen Jun
---
net/ipv4/inet_hashtables.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 7bd8983..76e846e 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -407,12 +407,15 @@ int __inet_hash_nolisten(struct sock *sk, struct
inet_timewait_sock *tw)
WARN_ON(!sk_unhashed(sk));
sk->sk_hash = inet_sk_ehashfn(sk);
- head = inet_ehash_bucket(hashinfo, sk->sk_hash);
- list = >chain;
lock = inet_ehash_lockp(hashinfo, sk->sk_hash);
spin_lock(lock);
- __sk_nulls_add_node_rcu(sk, list);
+ if (sk_hashed(sk)) {
+ head = inet_ehash_bucket(hashinfo, sk->sk_hash);
+ list = >chain;
+ __sk_nulls_add_node_rcu(sk, list);
+ }
+
if (tw) {
WARN_ON(sk->sk_hash != tw->tw_hash);
twrefcnt = inet_twsk_unhash(tw);
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Inet-hashtable: Change the range of sk-hash lock to avoid the race condition.
When try to add node to list in __inet_hash_nolisten function, first get the
list and then to lock for using, but in extremeness case, others can del this
node before locking it, then the node should be null.So this patch try to lock
firstly and then get the list for using to avoid this race condition.
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
net/ipv4/inet_hashtables.c |9 ++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 7bd8983..76e846e 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -407,12 +407,15 @@ int __inet_hash_nolisten(struct sock *sk, struct
inet_timewait_sock *tw)
WARN_ON(!sk_unhashed(sk));
sk-sk_hash = inet_sk_ehashfn(sk);
- head = inet_ehash_bucket(hashinfo, sk-sk_hash);
- list = head-chain;
lock = inet_ehash_lockp(hashinfo, sk-sk_hash);
spin_lock(lock);
- __sk_nulls_add_node_rcu(sk, list);
+ if (sk_hashed(sk)) {
+ head = inet_ehash_bucket(hashinfo, sk-sk_hash);
+ list = head-chain;
+ __sk_nulls_add_node_rcu(sk, list);
+ }
+
if (tw) {
WARN_ON(sk-sk_hash != tw-tw_hash);
twrefcnt = inet_twsk_unhash(tw);
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Inet-hashtable: Change the range of sk-hash lock to avoid the race condition.
On Thu, 2013-09-12 at 05:00 -0700, Eric Dumazet wrote: On Thu, 2013-09-12 at 12:32 -0400, Jun Chen wrote: When try to add node to list in __inet_hash_nolisten function, first get the list and then to lock for using, but in extremeness case, others can del this node before locking it, then the node should be null.So this patch try to lock firstly and then get the list for using to avoid this race condition. I suspect another bug. This should not happen. Care to describe the problem you got ? Thanks Ok, I just got this call stack and no more info, pls help to look it. thanks! 1[ 88.548263] BUG: unable to handle kernel NULL pointer dereference at 0004 1[ 88.548490] IP: [] __inet_hash_nolisten+0xc1/0x140 4[ 88.548617] *pde = 4[ 88.549927] EIP is at __inet_hash_nolisten+0xc1/0x140 4[ 88.550008] EAX: EBX: e08c ECX: edf846e0 EDX: e08c0020 4[ 88.550055] ESI: c20213c0 EDI: edc12dc0 EBP: ce4bfdfc ESP: ce4bfde8 4[ 88.550137] DS: 007b ES: 007b FS: 00d8 GS: 003b SS: 0068 4[ 88.550184] CR0: 80050033 CR2: 0004 CR3: 2b4ff000 CR4: 001007d0 4[ 88.550266] DR0: DR1: DR2: DR3: 4[ 88.550346] DR6: 0ff0 DR7: 0400 0[ 88.550392] Process WebViewCoreThre (pid: 2137, ti=ce4be000 task=eb193c80 task.ti=ce4be000) 0[ 88.551746] Call Trace: 4[ 88.551797] [] __inet_hash_connect+0x295/0x2d0 4[ 88.551883] [] inet_hash_connect+0x40/0x50 4[ 88.551932] [] ? inet_unhash+0x90/0x90 4[ 88.551981] [] ? __inet_lookup_listener+0x1b0/0x1b0 4[ 88.552067] [] tcp_v4_connect+0x247/0x4a0 4[ 88.552117] [] ? lock_sock_nested+0x3e/0x50 4[ 88.552205] [] inet_stream_connect+0xe2/0x290 4[ 88.552254] [] ? _copy_from_user+0x35/0x50 4[ 88.552342] [] sys_connect+0xb2/0xd0 4[ 88.552393] [] ? alloc_file+0x20/0xa0 4[ 88.552441] [] ? tcp_setsockopt+0x50/0x60 4[ 88.552525] [] ? fget_light+0x44/0xe0 4[ 88.552574] [] ? sock_common_setsockopt+0x27/0x40 4[ 88.552659] [] ? _copy_from_user+0x35/0x50 4[ 88.552708] [] sys_socketcall+0xab/0x2b0 4[ 88.552790] [] ? trace_hardirqs_on_thunk+0xc/0x10 4[ 88.552840] [] syscall_call+0x7/0xb 4[ 88.552923] [] ? mutex_trylock+0x30/0x140 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Modify the condition for the first skb to collapse
On Mon, 2013-06-17 at 06:21 -0700, Eric Dumazet wrote:
> On Mon, 2013-06-17 at 14:52 -0400, Jun Chen wrote:
> > On Mon, 2013-06-17 at 03:29 -0700, Eric Dumazet wrote:
> > > On Mon, 2013-06-17 at 13:29 -0400, Jun Chen wrote:
> > > > >
> > > > hi,
> > > > When the condition of tcp_win_from_space(skb->truesize) > skb->len is
> > > > true but the before(start, TCP_SKB_CB(skb)->seq) is also true, the final
> > > > condition will be true. The follow line:
> > > > int offset = start - TCP_SKB_CB(skb)->seq;
> > > > BUG_ON(offset < 0);
> > > > this BUG_ON will be triggered.
> > > >
> > >
> > > Really this should never happen, we must track what's happening here.
> > It's very very rare, but the logic of codes have such a little hole.
> > >
> > > Are you using a pristine kernel, without any patches ?
> > The based kernel version is 3.4.
> > >
> > > Are you able to reproduce this bug in a short amount of time ?
> > I can't reproduce it in short time, this log had just been found once
> > for long long time tests on many devices .
> > >
> > > What kind of driver is in use ? (your stack trace was truncated)
> >
> > I attach the whole stack traces for you.
> >
> Any other suspect messages before this, a memory allocation error for
> example ?
>
> I believe we have a bug in tcp_collapse() if one alloc_skb() returns
> NULL while we were in the middle of collapsing a big GRO packet.
>
> gro_skb needed 3 skb to be rebuilt, and only two skbs could be allocated
>
> skb1: seq=X end_seq=X+4000
> skb2: seq=X+4000 end_seq=X+8000
>
> grp_skb: seq=X end_seq=X+16000
>
> Next time we call tcp_collapse(), we might split again the GRO packet
> and get following incorrect queue :
>
> skb1: seq=X end_seq=X+4000
> skb2: seq=X+4000 end_seq=X+8000
> skb3: seq=X end_seq=X+4000
> skb4: seq=X+4000 end_seq=X+8000
> skb5: seq=X+8000 end_seq=X+12000
> skb6: seq=X+12000 end_seq=X+16000
>
>
> I would use the following patch instead, to narrow the problem
>
> If we really find in the ofo queue a skb with a lower seq than the
> previous one, we should complain instead of lowering @start, since
> this is going to crash later.
>
> receive_queue / ofo_queue should contain monotonically increasing
> skb->seq.
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 46271cdc..5507a09 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4513,8 +4513,10 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
> start = TCP_SKB_CB(skb)->seq;
> end = TCP_SKB_CB(skb)->end_seq;
> } else {
> - if (before(TCP_SKB_CB(skb)->seq, start))
> - start = TCP_SKB_CB(skb)->seq;
> + if (before(TCP_SKB_CB(skb)->seq, start)) {
> + pr_err_once("tcp_collapse_ofo_queue() : seq
> %08x before start %08X\n",
> + TCP_SKB_CB(skb)->seq, start);
> + }
> if (after(TCP_SKB_CB(skb)->end_seq, end))
> end = TCP_SKB_CB(skb)->end_seq;
> }
>
>
There are many warning for tcp_recvmsg before this crash. I can't find
other memory warning in the logs, but I'm not sure whether there are
memory issues because of the length limitation of saved logs. I think
this logs will give you more information.
<4>[ 7736.343742] [ cut here ]
<4>[ 7736.343759] WARNING:
at /data/buildbot/workdir/jb/kernel/net/ipv4/tcp.c:1496 tcp_recvmsg
+0x3bf/0x910()
<4>[ 7736.343775] recvmsg bug: copied AB57C870 seq AB57CD95 rcvnxt
AB57F19F fl 0
<4>[ 7736.343845] Call Trace:
<4>[ 7736.343865] [] warn_slowpath_common+0x72/0xa0
<4>[ 7736.343888] [] ? tcp_recvmsg+0x3bf/0x910
<4>[ 7736.343902] [] ? tcp_recvmsg+0x3bf/0x910
<4>[ 7736.343922] [] warn_slowpath_fmt+0x33/0x40
<4>[ 7736.343944] [] tcp_recvmsg+0x3bf/0x910
<4>[ 7736.343968] [] inet_recvmsg+0x85/0xa0
<4>[ 7736.343992] [] sock_aio_read+0x140/0x160
<4>[ 7736.344016] [] ? set_next_entity+0xc1/0xf0
<4>[ 7736.344039] [] do_sync_read+0xb7/0xf0
<4>[ 7736.344064] [] ? rw_verify_area+0x6c/0x120
<4>[ 7736.344077] [] ? sys_epoll_wait+0x68/0x360
<4>[ 7736.344098] [] vfs_read+0x149/0x160
<4>[ 7736.344120] [] ? fget_light+0x58/0xd0
<4>[ 7736.344142] [] sys_read+0x3d/0x70
<4>[ 7736.344164] [] syscall_call+0x7/0xb
<4>[ 7736.344187] [] ? perf_cpu_notify+0x45/0x89
<4>[ 7736.344205] ---[ end trace b3c5b245ce7ff5b5 ]---
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Modify the condition for the first skb to collapse
On Mon, 2013-06-17 at 03:29 -0700, Eric Dumazet wrote: > On Mon, 2013-06-17 at 13:29 -0400, Jun Chen wrote: > > > > > hi, > > When the condition of tcp_win_from_space(skb->truesize) > skb->len is > > true but the before(start, TCP_SKB_CB(skb)->seq) is also true, the final > > condition will be true. The follow line: > > int offset = start - TCP_SKB_CB(skb)->seq; > > BUG_ON(offset < 0); > > this BUG_ON will be triggered. > > > > Really this should never happen, we must track what's happening here. It's very very rare, but the logic of codes have such a little hole. > > Are you using a pristine kernel, without any patches ? The based kernel version is 3.4. > > Are you able to reproduce this bug in a short amount of time ? I can't reproduce it in short time, this log had just been found once for long long time tests on many devices . > > What kind of driver is in use ? (your stack trace was truncated) I attach the whole stack traces for you. <0>[ 7736.348788] Call Trace: <4>[ 7736.348861] [] tcp_prune_queue+0x120/0x2f0 <4>[ 7736.348984] [] tcp_data_queue+0x777/0xf00 <4>[ 7736.349055] [] ? ipt_do_table+0x1f8/0x480 <4>[ 7736.349126] [] ? ipt_do_table+0x1f8/0x480 <4>[ 7736.349196] [] tcp_rcv_established+0x114/0x680 <4>[ 7736.349269] [] tcp_v4_do_rcv+0x164/0x350 <4>[ 7736.349396] [] ? nf_nat_fn+0xb1/0x1d0 <4>[ 7736.349470] [] tcp_v4_rcv+0x6f1/0x7a0 <4>[ 7736.349599] [] ? nf_hook_slow+0x10d/0x150 <4>[ 7736.349673] [] ip_local_deliver_finish+0x8b/0x200 <4>[ 7736.349796] [] ip_local_deliver+0x8f/0xa0 <4>[ 7736.349867] [] ? ip_rcv_finish+0x300/0x300 <4>[ 7736.349937] [] ip_rcv_finish+0xdf/0x300 <4>[ 7736.350062] [] ip_rcv+0x258/0x330 <4>[ 7736.350132] [] ? inet_del_protocol+0x30/0x30 <4>[ 7736.350258] [] __netif_receive_skb+0x325/0x410 <4>[ 7736.350331] [] process_backlog+0x96/0x150 <4>[ 7736.350455] [] net_rx_action+0x115/0x210 <4>[ 7736.350525] [] ? tcp_out_of_resources+0xb0/0xb0 <4>[ 7736.350652] [] __do_softirq+0x9b/0x220 <4>[ 7736.350723] [] ? local_bh_enable_ip+0xd0/0xd0 > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Modify the condition for the first skb to collapse
On Mon, 2013-06-17 at 01:15 -0700, Eric Dumazet wrote:
> On Mon, 2013-06-17 at 10:18 -0400, Jun Chen wrote:
> > When search the first skb to collapse,the condition of overlap to the next
> > one have been
> > reached,but the start is less than TCP_SKB_CB(skb)->seq at this time, then
> > followed process
> > will trigger the BUG_ON of the offset(start - TCP_SKB_CB(skb)->seq).
> > So this patch add one check (! before(start,TCP_SKB_CB(skb)->seq)) to avoid
> > this ipanic.
> >
> > Signed-off-by: Chen Jun
> > ---
> > net/ipv4/tcp_input.c |3 ++-
> > 1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 9c62257..4c745c5 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -4465,7 +4465,8 @@ restart:
> > * overlaps to the next one.
> > */
> > if (!tcp_hdr(skb)->syn && !tcp_hdr(skb)->fin &&
> > - (tcp_win_from_space(skb->truesize) > skb->len ||
> > + ((tcp_win_from_space(skb->truesize) > skb->len &&
> > + !before(start, TCP_SKB_CB(skb)->seq)) ||
> > before(TCP_SKB_CB(skb)->seq, start))) {
> > end_of_skbs = false;
> > break;
>
> Hmm... I must say I do not understand this patch.
>
> If we find a skb with before(TCP_SKB_CB(skb)->seq, start), then the
> final condition will be true.
>
> Let's rewrite your code to equivalent one :
>
> if (!tcp_hdr(skb)->syn && !tcp_hdr(skb)->fin &&
> (before(TCP_SKB_CB(skb)->seq, start) ||
> tcp_win_from_space(skb->truesize) > skb->len)) {
>
> So it seems your patch would not solve the problem for all
> possible skbs (aka not bloated) ?
>
> Please tell us how you trigger this bug, and send the stack trace.
>
> Thanks
>
>
hi,
When the condition of tcp_win_from_space(skb->truesize) > skb->len is
true but the before(start, TCP_SKB_CB(skb)->seq) is also true, the final
condition will be true. The follow line:
int offset = start - TCP_SKB_CB(skb)->seq;
BUG_ON(offset < 0);
this BUG_ON will be triggered.
Follow line is my error logs:
<2>[ 7736.344508] kernel BUG
at /data/buildbot/workdir/jb/kernel/net/ipv4/tcp_input.c:4845!
<4>[ 7736.344578] invalid opcode: [#1] PREEMPT SMP
<4>[ 7736.344883] Modules linked in: atomisp lm3559 ov9724 imx1x5
bcm4335(O) cfg80211 bcm_bt_lpm videobuf_vmalloc videobuf_core matrix(C)
<4>[ 7736.345681]
<4>[ 7736.345748] Pid: 5189, comm: TimedEventQueue Tainted: GWC
O 3.4.43-186445-g3ada675 #1 Intel Corporation Merrifield/SALT BAY
<4>[ 7736.346059] EIP: 0060:[] EFLAGS: 00010297 CPU: 1
<4>[ 7736.346183] EIP is at tcp_collapse+0x3bd/0x3d0
<4>[ 7736.346250] EAX: ab57d2bb EBX: df428c00 ECX: c97dcd00 EDX:
10c0
<4>[ 7736.346372] ESI: df4289c0 EDI: fadb EBP: edca1d88 ESP:
edca1d60
<4>[ 7736.346441] DS: 007b ES: 007b FS: 00d8 GS: 003b SS: 0068
<4>[ 7736.346560] CR0: 8005003b CR2: 41d310bc CR3: 2d30 CR4:
001007d0
<4>[ 7736.346629] DR0: DR1: DR2: DR3:
<4>[ 7736.346749] DR6: 0ff0 DR7: 0400
<0>[ 7736.346816] Process TimedEventQueue (pid: 5189, ti=edca
task=dc30b660 task.ti=c9a6e000)
<0>[ 7736.346936] Stack:
<4>[ 7736.347002] fadb c97dcd5c 0001 c97dcd00
0e32 c97dcd00
<4>[ 7736.347615] c97dcd00 df428180 edca1db0 c18addd0 ab57c870
ab57f19f c97dcd00
<4>[ 7736.348175] c97dd198 80c0 c97dcd00 df428180 edca1df0 c18aea27
c18dc8f8
<0>[ 7736.348788] Call Trace:
<4>[ 7736.348861] [] tcp_prune_queue+0x120/0x2f0
<4>[ 7736.348984] [] tcp_data_queue+0x777/0xf00
<4>[ 7736.349055] [] ? ipt_do_table+0x1f8/0x480
<4>[ 7736.349126] [] ? ipt_do_table+0x1f8/0x480
<4>[ 7736.349196] [] tcp_rcv_established+0x114/0x680
<4>[ 7736.349269] [] tcp_v4_do_rcv+0x164/0x350
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] tcp: Modify the condition for the first skb to collapse
When search the first skb to collapse,the condition of overlap to the next one
have been
reached,but the start is less than TCP_SKB_CB(skb)->seq at this time, then
followed process
will trigger the BUG_ON of the offset(start - TCP_SKB_CB(skb)->seq).
So this patch add one check (! before(start,TCP_SKB_CB(skb)->seq)) to avoid
this ipanic.
Signed-off-by: Chen Jun
---
net/ipv4/tcp_input.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9c62257..4c745c5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4465,7 +4465,8 @@ restart:
* overlaps to the next one.
*/
if (!tcp_hdr(skb)->syn && !tcp_hdr(skb)->fin &&
- (tcp_win_from_space(skb->truesize) > skb->len ||
+ ((tcp_win_from_space(skb->truesize) > skb->len &&
+ !before(start, TCP_SKB_CB(skb)->seq)) ||
before(TCP_SKB_CB(skb)->seq, start))) {
end_of_skbs = false;
break;
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] tcp: Modify the condition for the first skb to collapse
When search the first skb to collapse,the condition of overlap to the next one
have been
reached,but the start is less than TCP_SKB_CB(skb)-seq at this time, then
followed process
will trigger the BUG_ON of the offset(start - TCP_SKB_CB(skb)-seq).
So this patch add one check (! before(start,TCP_SKB_CB(skb)-seq)) to avoid
this ipanic.
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
net/ipv4/tcp_input.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9c62257..4c745c5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4465,7 +4465,8 @@ restart:
* overlaps to the next one.
*/
if (!tcp_hdr(skb)-syn !tcp_hdr(skb)-fin
- (tcp_win_from_space(skb-truesize) skb-len ||
+ ((tcp_win_from_space(skb-truesize) skb-len
+ !before(start, TCP_SKB_CB(skb)-seq)) ||
before(TCP_SKB_CB(skb)-seq, start))) {
end_of_skbs = false;
break;
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Modify the condition for the first skb to collapse
On Mon, 2013-06-17 at 01:15 -0700, Eric Dumazet wrote:
On Mon, 2013-06-17 at 10:18 -0400, Jun Chen wrote:
When search the first skb to collapse,the condition of overlap to the next
one have been
reached,but the start is less than TCP_SKB_CB(skb)-seq at this time, then
followed process
will trigger the BUG_ON of the offset(start - TCP_SKB_CB(skb)-seq).
So this patch add one check (! before(start,TCP_SKB_CB(skb)-seq)) to avoid
this ipanic.
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
net/ipv4/tcp_input.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 9c62257..4c745c5 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4465,7 +4465,8 @@ restart:
* overlaps to the next one.
*/
if (!tcp_hdr(skb)-syn !tcp_hdr(skb)-fin
- (tcp_win_from_space(skb-truesize) skb-len ||
+ ((tcp_win_from_space(skb-truesize) skb-len
+ !before(start, TCP_SKB_CB(skb)-seq)) ||
before(TCP_SKB_CB(skb)-seq, start))) {
end_of_skbs = false;
break;
Hmm... I must say I do not understand this patch.
If we find a skb with before(TCP_SKB_CB(skb)-seq, start), then the
final condition will be true.
Let's rewrite your code to equivalent one :
if (!tcp_hdr(skb)-syn !tcp_hdr(skb)-fin
(before(TCP_SKB_CB(skb)-seq, start) ||
tcp_win_from_space(skb-truesize) skb-len)) {
So it seems your patch would not solve the problem for all
possible skbs (aka not bloated) ?
Please tell us how you trigger this bug, and send the stack trace.
Thanks
hi,
When the condition of tcp_win_from_space(skb-truesize) skb-len is
true but the before(start, TCP_SKB_CB(skb)-seq) is also true, the final
condition will be true. The follow line:
int offset = start - TCP_SKB_CB(skb)-seq;
BUG_ON(offset 0);
this BUG_ON will be triggered.
Follow line is my error logs:
2[ 7736.344508] kernel BUG
at /data/buildbot/workdir/jb/kernel/net/ipv4/tcp_input.c:4845!
4[ 7736.344578] invalid opcode: [#1] PREEMPT SMP
4[ 7736.344883] Modules linked in: atomisp lm3559 ov9724 imx1x5
bcm4335(O) cfg80211 bcm_bt_lpm videobuf_vmalloc videobuf_core matrix(C)
4[ 7736.345681]
4[ 7736.345748] Pid: 5189, comm: TimedEventQueue Tainted: GWC
O 3.4.43-186445-g3ada675 #1 Intel Corporation Merrifield/SALT BAY
4[ 7736.346059] EIP: 0060:[c18ad61d] EFLAGS: 00010297 CPU: 1
4[ 7736.346183] EIP is at tcp_collapse+0x3bd/0x3d0
4[ 7736.346250] EAX: ab57d2bb EBX: df428c00 ECX: c97dcd00 EDX:
10c0
4[ 7736.346372] ESI: df4289c0 EDI: fadb EBP: edca1d88 ESP:
edca1d60
4[ 7736.346441] DS: 007b ES: 007b FS: 00d8 GS: 003b SS: 0068
4[ 7736.346560] CR0: 8005003b CR2: 41d310bc CR3: 2d30 CR4:
001007d0
4[ 7736.346629] DR0: DR1: DR2: DR3:
4[ 7736.346749] DR6: 0ff0 DR7: 0400
0[ 7736.346816] Process TimedEventQueue (pid: 5189, ti=edca
task=dc30b660 task.ti=c9a6e000)
0[ 7736.346936] Stack:
4[ 7736.347002] fadb c97dcd5c 0001 c97dcd00
0e32 c97dcd00
4[ 7736.347615] c97dcd00 df428180 edca1db0 c18addd0 ab57c870
ab57f19f c97dcd00
4[ 7736.348175] c97dd198 80c0 c97dcd00 df428180 edca1df0 c18aea27
c18dc8f8
0[ 7736.348788] Call Trace:
4[ 7736.348861] [c18addd0] tcp_prune_queue+0x120/0x2f0
4[ 7736.348984] [c18aea27] tcp_data_queue+0x777/0xf00
4[ 7736.349055] [c18dc8f8] ? ipt_do_table+0x1f8/0x480
4[ 7736.349126] [c18dc8f8] ? ipt_do_table+0x1f8/0x480
4[ 7736.349196] [c18b2e84] tcp_rcv_established+0x114/0x680
4[ 7736.349269] [c18bb034] tcp_v4_do_rcv+0x164/0x350
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Modify the condition for the first skb to collapse
On Mon, 2013-06-17 at 03:29 -0700, Eric Dumazet wrote: On Mon, 2013-06-17 at 13:29 -0400, Jun Chen wrote: hi, When the condition of tcp_win_from_space(skb-truesize) skb-len is true but the before(start, TCP_SKB_CB(skb)-seq) is also true, the final condition will be true. The follow line: int offset = start - TCP_SKB_CB(skb)-seq; BUG_ON(offset 0); this BUG_ON will be triggered. Really this should never happen, we must track what's happening here. It's very very rare, but the logic of codes have such a little hole. Are you using a pristine kernel, without any patches ? The based kernel version is 3.4. Are you able to reproduce this bug in a short amount of time ? I can't reproduce it in short time, this log had just been found once for long long time tests on many devices . What kind of driver is in use ? (your stack trace was truncated) I attach the whole stack traces for you. 0[ 7736.348788] Call Trace: 4[ 7736.348861] [c18addd0] tcp_prune_queue+0x120/0x2f0 4[ 7736.348984] [c18aea27] tcp_data_queue+0x777/0xf00 4[ 7736.349055] [c18dc8f8] ? ipt_do_table+0x1f8/0x480 4[ 7736.349126] [c18dc8f8] ? ipt_do_table+0x1f8/0x480 4[ 7736.349196] [c18b2e84] tcp_rcv_established+0x114/0x680 4[ 7736.349269] [c18bb034] tcp_v4_do_rcv+0x164/0x350 4[ 7736.349396] [c18de301] ? nf_nat_fn+0xb1/0x1d0 4[ 7736.349470] [c18bc0c1] tcp_v4_rcv+0x6f1/0x7a0 4[ 7736.349599] [c1881dad] ? nf_hook_slow+0x10d/0x150 4[ 7736.349673] [c189b30b] ip_local_deliver_finish+0x8b/0x200 4[ 7736.349796] [c189b60f] ip_local_deliver+0x8f/0xa0 4[ 7736.349867] [c189b280] ? ip_rcv_finish+0x300/0x300 4[ 7736.349937] [c189b05f] ip_rcv_finish+0xdf/0x300 4[ 7736.350062] [c189b878] ip_rcv+0x258/0x330 4[ 7736.350132] [c189af80] ? inet_del_protocol+0x30/0x30 4[ 7736.350258] [c1864175] __netif_receive_skb+0x325/0x410 4[ 7736.350331] [c1864956] process_backlog+0x96/0x150 4[ 7736.350455] [c1864ba5] net_rx_action+0x115/0x210 4[ 7736.350525] [c18b7680] ? tcp_out_of_resources+0xb0/0xb0 4[ 7736.350652] [c123dc0b] __do_softirq+0x9b/0x220 4[ 7736.350723] [c123db70] ? local_bh_enable_ip+0xd0/0xd0 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] tcp: Modify the condition for the first skb to collapse
On Mon, 2013-06-17 at 06:21 -0700, Eric Dumazet wrote:
On Mon, 2013-06-17 at 14:52 -0400, Jun Chen wrote:
On Mon, 2013-06-17 at 03:29 -0700, Eric Dumazet wrote:
On Mon, 2013-06-17 at 13:29 -0400, Jun Chen wrote:
hi,
When the condition of tcp_win_from_space(skb-truesize) skb-len is
true but the before(start, TCP_SKB_CB(skb)-seq) is also true, the final
condition will be true. The follow line:
int offset = start - TCP_SKB_CB(skb)-seq;
BUG_ON(offset 0);
this BUG_ON will be triggered.
Really this should never happen, we must track what's happening here.
It's very very rare, but the logic of codes have such a little hole.
Are you using a pristine kernel, without any patches ?
The based kernel version is 3.4.
Are you able to reproduce this bug in a short amount of time ?
I can't reproduce it in short time, this log had just been found once
for long long time tests on many devices .
What kind of driver is in use ? (your stack trace was truncated)
I attach the whole stack traces for you.
Any other suspect messages before this, a memory allocation error for
example ?
I believe we have a bug in tcp_collapse() if one alloc_skb() returns
NULL while we were in the middle of collapsing a big GRO packet.
gro_skb needed 3 skb to be rebuilt, and only two skbs could be allocated
skb1: seq=X end_seq=X+4000
skb2: seq=X+4000 end_seq=X+8000
missing
grp_skb: seq=X end_seq=X+16000
Next time we call tcp_collapse(), we might split again the GRO packet
and get following incorrect queue :
skb1: seq=X end_seq=X+4000
skb2: seq=X+4000 end_seq=X+8000
skb3: seq=X end_seq=X+4000
skb4: seq=X+4000 end_seq=X+8000
skb5: seq=X+8000 end_seq=X+12000
skb6: seq=X+12000 end_seq=X+16000
I would use the following patch instead, to narrow the problem
If we really find in the ofo queue a skb with a lower seq than the
previous one, we should complain instead of lowering @start, since
this is going to crash later.
receive_queue / ofo_queue should contain monotonically increasing
skb-seq.
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 46271cdc..5507a09 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4513,8 +4513,10 @@ static void tcp_collapse_ofo_queue(struct sock *sk)
start = TCP_SKB_CB(skb)-seq;
end = TCP_SKB_CB(skb)-end_seq;
} else {
- if (before(TCP_SKB_CB(skb)-seq, start))
- start = TCP_SKB_CB(skb)-seq;
+ if (before(TCP_SKB_CB(skb)-seq, start)) {
+ pr_err_once(tcp_collapse_ofo_queue() : seq
%08x before start %08X\n,
+ TCP_SKB_CB(skb)-seq, start);
+ }
if (after(TCP_SKB_CB(skb)-end_seq, end))
end = TCP_SKB_CB(skb)-end_seq;
}
There are many warning for tcp_recvmsg before this crash. I can't find
other memory warning in the logs, but I'm not sure whether there are
memory issues because of the length limitation of saved logs. I think
this logs will give you more information.
4[ 7736.343742] [ cut here ]
4[ 7736.343759] WARNING:
at /data/buildbot/workdir/jb/kernel/net/ipv4/tcp.c:1496 tcp_recvmsg
+0x3bf/0x910()
4[ 7736.343775] recvmsg bug: copied AB57C870 seq AB57CD95 rcvnxt
AB57F19F fl 0
4[ 7736.343845] Call Trace:
4[ 7736.343865] [c1237032] warn_slowpath_common+0x72/0xa0
4[ 7736.343888] [c18a955f] ? tcp_recvmsg+0x3bf/0x910
4[ 7736.343902] [c18a955f] ? tcp_recvmsg+0x3bf/0x910
4[ 7736.343922] [c1237103] warn_slowpath_fmt+0x33/0x40
4[ 7736.343944] [c18a955f] tcp_recvmsg+0x3bf/0x910
4[ 7736.343968] [c18c9bb5] inet_recvmsg+0x85/0xa0
4[ 7736.343992] [c1852030] sock_aio_read+0x140/0x160
4[ 7736.344016] [c126b221] ? set_next_entity+0xc1/0xf0
4[ 7736.344039] [c130d627] do_sync_read+0xb7/0xf0
4[ 7736.344064] [c130dc6c] ? rw_verify_area+0x6c/0x120
4[ 7736.344077] [c1349aa8] ? sys_epoll_wait+0x68/0x360
4[ 7736.344098] [c130e1e9] vfs_read+0x149/0x160
4[ 7736.344120] [c130f518] ? fget_light+0x58/0xd0
4[ 7736.344142] [c130e23d] sys_read+0x3d/0x70
4[ 7736.344164] [c198c361] syscall_call+0x7/0xb
4[ 7736.344187] [c198] ? perf_cpu_notify+0x45/0x89
4[ 7736.344205] ---[ end trace b3c5b245ce7ff5b5 ]---
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] perf: Add the rcu_read_lock to protect the list_for_each_entry_rcu.
The list_for_each_entry_rcu should be guarded by rcu_read_lock().This patch add
rcu_read_lock to protect the list_for_each_entry_rcu.
Signed-off-by: Chen Jun
---
kernel/events/core.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 301079d..e2f2fa5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2442,6 +2442,7 @@ static void perf_adjust_freq_unthr_context(struct
perf_event_context *ctx,
raw_spin_lock(>lock);
perf_pmu_disable(ctx->pmu);
+ rcu_read_lock();
list_for_each_entry_rcu(event, >event_list, event_entry) {
if (event->state != PERF_EVENT_STATE_ACTIVE)
@@ -2483,6 +2484,7 @@ static void perf_adjust_freq_unthr_context(struct
perf_event_context *ctx,
event->pmu->start(event, delta > 0 ? PERF_EF_RELOAD : 0);
}
+ rcu_read_unlock();
perf_pmu_enable(ctx->pmu);
raw_spin_unlock(>lock);
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] perf: Add the rcu_read_lock to protect the list_for_each_entry_rcu.
The list_for_each_entry_rcu should be guarded by rcu_read_lock().This patch add
rcu_read_lock to protect the list_for_each_entry_rcu.
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
kernel/events/core.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 301079d..e2f2fa5 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2442,6 +2442,7 @@ static void perf_adjust_freq_unthr_context(struct
perf_event_context *ctx,
raw_spin_lock(ctx-lock);
perf_pmu_disable(ctx-pmu);
+ rcu_read_lock();
list_for_each_entry_rcu(event, ctx-event_list, event_entry) {
if (event-state != PERF_EVENT_STATE_ACTIVE)
@@ -2483,6 +2484,7 @@ static void perf_adjust_freq_unthr_context(struct
perf_event_context *ctx,
event-pmu-start(event, delta 0 ? PERF_EF_RELOAD : 0);
}
+ rcu_read_unlock();
perf_pmu_enable(ctx-pmu);
raw_spin_unlock(ctx-lock);
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Add the flag indicate to registe new device as children of master or not.
On Fri, 2012-12-21 at 19:06 +, Grant Likely wrote:
> On Fri, 21 Dec 2012 12:39:52 -0500, Jun Chen wrote:
> > On Wed, 2012-12-19 at 16:21 +, Grant Likely wrote:
> > > On Wed, 19 Dec 2012 09:04:16 +, Mark Brown
> > > wrote:
> > > > On Wed, Dec 19, 2012 at 04:44:03AM -0500, Jun Chen wrote:
> > > >
> > > > > This spi_alloc_device function will be called in the spi_new_device
> > > > > function to alloc new device as the master. But other way, it is
> > > > > called
> > > > > by the of_register_spi_devices function to register new device as the
> > > > > children of the master. I will update changlog to add it.
> > > >
> > > > But why is this a bad thing? You've said what's happening but not why
> > > > it's a problem.
> > >
> > > spi_devices should always be children of the spi_master. If that is not
> > > the case then it is a bug to be fixed.
> > >
> >
> > When many boards initializing, boards will call function
> > spi_register_board_info to create bi->board_info,Then spi driver probe
> > to call spi_register_master to register the driver and in the function
> > spi_match_master_to_boardinfo To create new spi device, and this cases
> > the spi_devices are not children of the spi_master.
> > Many drivers do these steps. If all spi_devices must be children of the
> > spi_master, Do spi core have plan to delete this way?
> > Or spi core can hold this way for many drivers.
>
> Let me make sure I understand what you're saying...
>
> Right now, every spi_device object is registered as a child of an
> spi_master object.
>
> With this proposed patch, spi_devices registered via spi_register_board_info
> will be siblings of the spi_master instead of children.
>
> Do I understand correctly so far?
>
Yes,
> The problem is that I don't understand why this change is necessary.
> spi_devices should always be children of an spi_master, not siblings.
> What is the problem you're trying to solve with this change?
>
When spi drivers try to use the core function(spi_register_master),it
will trigger error,because they use the function
spi_match_master_to_boardinfo to create new spi device as the children
of the master.
In the old version of spi core, the new devices are registered as
siblings of the spi_master. My spi driver based on the old version runs
normal.
But after applying for this patch:
{
spi: Fix device unregistration when unregistering the bus master
Device are added as children of the bus master's parent device, but
spi_unregister_master() looks for devices to unregister in the bus
master's children. This results in the child devices not being
unregistered.
Fix this by registering devices as direct children of the bus
master.
- spi->dev.parent = dev;
+ spi->dev.parent = >dev;
}
Then my driver will be crash.
Maybe I have mistake on this issue, thank for your more explanation and
detail replay.
> g.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Add the flag indicate to registe new device as children of master or not.
On Fri, 2012-12-21 at 19:06 +, Grant Likely wrote:
On Fri, 21 Dec 2012 12:39:52 -0500, Jun Chen jun.d.c...@intel.com wrote:
On Wed, 2012-12-19 at 16:21 +, Grant Likely wrote:
On Wed, 19 Dec 2012 09:04:16 +, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
On Wed, Dec 19, 2012 at 04:44:03AM -0500, Jun Chen wrote:
This spi_alloc_device function will be called in the spi_new_device
function to alloc new device as the master. But other way, it is
called
by the of_register_spi_devices function to register new device as the
children of the master. I will update changlog to add it.
But why is this a bad thing? You've said what's happening but not why
it's a problem.
spi_devices should always be children of the spi_master. If that is not
the case then it is a bug to be fixed.
When many boards initializing, boards will call function
spi_register_board_info to create bi-board_info,Then spi driver probe
to call spi_register_master to register the driver and in the function
spi_match_master_to_boardinfo To create new spi device, and this cases
the spi_devices are not children of the spi_master.
Many drivers do these steps. If all spi_devices must be children of the
spi_master, Do spi core have plan to delete this way?
Or spi core can hold this way for many drivers.
Let me make sure I understand what you're saying...
Right now, every spi_device object is registered as a child of an
spi_master object.
With this proposed patch, spi_devices registered via spi_register_board_info
will be siblings of the spi_master instead of children.
Do I understand correctly so far?
Yes,
The problem is that I don't understand why this change is necessary.
spi_devices should always be children of an spi_master, not siblings.
What is the problem you're trying to solve with this change?
When spi drivers try to use the core function(spi_register_master),it
will trigger error,because they use the function
spi_match_master_to_boardinfo to create new spi device as the children
of the master.
In the old version of spi core, the new devices are registered as
siblings of the spi_master. My spi driver based on the old version runs
normal.
But after applying for this patch:
{
spi: Fix device unregistration when unregistering the bus master
Device are added as children of the bus master's parent device, but
spi_unregister_master() looks for devices to unregister in the bus
master's children. This results in the child devices not being
unregistered.
Fix this by registering devices as direct children of the bus
master.
- spi-dev.parent = dev;
+ spi-dev.parent = master-dev;
}
Then my driver will be crash.
Maybe I have mistake on this issue, thank for your more explanation and
detail replay.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Add the flag indicate to registe new device as children of master or not.
On Wed, 2012-12-19 at 16:21 +, Grant Likely wrote: > On Wed, 19 Dec 2012 09:04:16 +, Mark Brown > wrote: > > On Wed, Dec 19, 2012 at 04:44:03AM -0500, Jun Chen wrote: > > > > > This spi_alloc_device function will be called in the spi_new_device > > > function to alloc new device as the master. But other way, it is called > > > by the of_register_spi_devices function to register new device as the > > > children of the master. I will update changlog to add it. > > > > But why is this a bad thing? You've said what's happening but not why > > it's a problem. > > spi_devices should always be children of the spi_master. If that is not > the case then it is a bug to be fixed. > When many boards initializing, boards will call function spi_register_board_info to create bi->board_info,Then spi driver probe to call spi_register_master to register the driver and in the function spi_match_master_to_boardinfo To create new spi device, and this cases the spi_devices are not children of the spi_master. Many drivers do these steps. If all spi_devices must be children of the spi_master, Do spi core have plan to delete this way? Or spi core can hold this way for many drivers. Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Add the flag indicate to registe new device as children of master or not.
On Wed, 2012-12-19 at 16:21 +, Grant Likely wrote: On Wed, 19 Dec 2012 09:04:16 +, Mark Brown broo...@opensource.wolfsonmicro.com wrote: On Wed, Dec 19, 2012 at 04:44:03AM -0500, Jun Chen wrote: This spi_alloc_device function will be called in the spi_new_device function to alloc new device as the master. But other way, it is called by the of_register_spi_devices function to register new device as the children of the master. I will update changlog to add it. But why is this a bad thing? You've said what's happening but not why it's a problem. spi_devices should always be children of the spi_master. If that is not the case then it is a bug to be fixed. When many boards initializing, boards will call function spi_register_board_info to create bi-board_info,Then spi driver probe to call spi_register_master to register the driver and in the function spi_match_master_to_boardinfo To create new spi device, and this cases the spi_devices are not children of the spi_master. Many drivers do these steps. If all spi_devices must be children of the spi_master, Do spi core have plan to delete this way? Or spi core can hold this way for many drivers. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Add the flag indicate to registe new device as children of master or not.
On Tue, 2012-12-18 at 15:26 +, Mark Brown wrote:
> On Tue, Dec 18, 2012 at 11:29:34AM -0500, Jun Chen wrote:
>
> > * @master: Controller to which device is connected
> > + * device_was_children_of_master is flag which the device is registed
> > + * as the children of the bus
>
> This isn't a kerneldoc style comment (it needs the @XXX: format). The
> name is also extremely long, can't we think of something more concise?
>
Thank for your suggestion, I will correct this comment and use concise
flag.
> > - spi->dev.parent = >dev;
> > + if (device_was_children_of_master == true)
> > + spi->dev.parent = >dev;
> > + else
> > + spi->dev.parent = dev;
>
> Can you provide an example of where this is useful? Your changelog
> didn't make it clear and the code doesn't make it obvious either.
This spi_alloc_device function will be called in the spi_new_device
function to alloc new device as the master. But other way, it is called
by the of_register_spi_devices function to register new device as the
children of the master. I will update changlog to add it.
@@ -434,7 +440,7 @@ struct spi_device *spi_new_device(struct spi_master
*master,
* suggests syslogged diagnostics are best here (ugh).
*/
- proxy = spi_alloc_device(master);
+ proxy = spi_alloc_device(master, false);
if (!proxy)
return NULL;
@@ -827,7 +833,7 @@ static void of_register_spi_devices(struct
spi_master *master)
for_each_available_child_of_node(master->dev.of_node, nc) {
/* Alloc an spi_device */
- spi = spi_alloc_device(master);
+ spi = spi_alloc_device(master, true);
If I have mistake, pls correct me, thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: Add the flag indicate to registe new device as children of master or not.
Because there are two aim when allocating the new device, one is for children
of master,
other is for master. So this patch add one flag to indicate different purpose.
Signed-off-by: Bi Chao
Signed-off-by: Chen Jun
---
drivers/spi/spi.c | 16 +++-
include/linux/spi/spi.h |3 ++-
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 718cc1f..06f69ce 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -300,6 +300,8 @@ static DEFINE_MUTEX(board_lock);
/**
* spi_alloc_device - Allocate a new SPI device
* @master: Controller to which device is connected
+ * device_was_children_of_master is flag which the device is registed
+ * as the children of the bus
* Context: can sleep
*
* Allows a driver to allocate and initialize a spi_device without
@@ -314,7 +316,8 @@ static DEFINE_MUTEX(board_lock);
*
* Returns a pointer to the new device, or NULL.
*/
-struct spi_device *spi_alloc_device(struct spi_master *master)
+struct spi_device *spi_alloc_device(struct spi_master *master,
+ bool device_was_children_of_master)
{
struct spi_device *spi;
struct device *dev = master->dev.parent;
@@ -330,7 +333,10 @@ struct spi_device *spi_alloc_device(struct spi_master
*master)
}
spi->master = master;
- spi->dev.parent = >dev;
+ if (device_was_children_of_master == true)
+ spi->dev.parent = >dev;
+ else
+ spi->dev.parent = dev;
spi->dev.bus = _bus_type;
spi->dev.release = spidev_release;
device_initialize(>dev);
@@ -434,7 +440,7 @@ struct spi_device *spi_new_device(struct spi_master *master,
* suggests syslogged diagnostics are best here (ugh).
*/
- proxy = spi_alloc_device(master);
+ proxy = spi_alloc_device(master, false);
if (!proxy)
return NULL;
@@ -827,7 +833,7 @@ static void of_register_spi_devices(struct spi_master
*master)
for_each_available_child_of_node(master->dev.of_node, nc) {
/* Alloc an spi_device */
- spi = spi_alloc_device(master);
+ spi = spi_alloc_device(master, true);
if (!spi) {
dev_err(>dev, "spi_device alloc error for %s\n",
nc->full_name);
@@ -939,7 +945,7 @@ static acpi_status acpi_spi_add_device(acpi_handle handle,
u32 level,
if (acpi_bus_get_status(adev) || !adev->status.present)
return AE_OK;
- spi = spi_alloc_device(master);
+ spi = spi_alloc_device(master, false);
if (!spi) {
dev_err(>dev, "failed to allocate SPI device for %s\n",
dev_name(>dev));
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index fa702ae..43d2f8e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -838,7 +838,8 @@ spi_register_board_info(struct spi_board_info const *info,
unsigned n)
* be defined using the board info.
*/
extern struct spi_device *
-spi_alloc_device(struct spi_master *master);
+spi_alloc_device(struct spi_master *master,
+ bool device_was_children_of_master);
extern int
spi_add_device(struct spi_device *spi);
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] spi: Add the flag indicate to registe new device as children of master or not.
Because there are two aim when allocating the new device, one is for children
of master,
other is for master. So this patch add one flag to indicate different purpose.
Signed-off-by: Bi Chao chao...@intel.com
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
drivers/spi/spi.c | 16 +++-
include/linux/spi/spi.h |3 ++-
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 718cc1f..06f69ce 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -300,6 +300,8 @@ static DEFINE_MUTEX(board_lock);
/**
* spi_alloc_device - Allocate a new SPI device
* @master: Controller to which device is connected
+ * device_was_children_of_master is flag which the device is registed
+ * as the children of the bus
* Context: can sleep
*
* Allows a driver to allocate and initialize a spi_device without
@@ -314,7 +316,8 @@ static DEFINE_MUTEX(board_lock);
*
* Returns a pointer to the new device, or NULL.
*/
-struct spi_device *spi_alloc_device(struct spi_master *master)
+struct spi_device *spi_alloc_device(struct spi_master *master,
+ bool device_was_children_of_master)
{
struct spi_device *spi;
struct device *dev = master-dev.parent;
@@ -330,7 +333,10 @@ struct spi_device *spi_alloc_device(struct spi_master
*master)
}
spi-master = master;
- spi-dev.parent = master-dev;
+ if (device_was_children_of_master == true)
+ spi-dev.parent = master-dev;
+ else
+ spi-dev.parent = dev;
spi-dev.bus = spi_bus_type;
spi-dev.release = spidev_release;
device_initialize(spi-dev);
@@ -434,7 +440,7 @@ struct spi_device *spi_new_device(struct spi_master *master,
* suggests syslogged diagnostics are best here (ugh).
*/
- proxy = spi_alloc_device(master);
+ proxy = spi_alloc_device(master, false);
if (!proxy)
return NULL;
@@ -827,7 +833,7 @@ static void of_register_spi_devices(struct spi_master
*master)
for_each_available_child_of_node(master-dev.of_node, nc) {
/* Alloc an spi_device */
- spi = spi_alloc_device(master);
+ spi = spi_alloc_device(master, true);
if (!spi) {
dev_err(master-dev, spi_device alloc error for %s\n,
nc-full_name);
@@ -939,7 +945,7 @@ static acpi_status acpi_spi_add_device(acpi_handle handle,
u32 level,
if (acpi_bus_get_status(adev) || !adev-status.present)
return AE_OK;
- spi = spi_alloc_device(master);
+ spi = spi_alloc_device(master, false);
if (!spi) {
dev_err(master-dev, failed to allocate SPI device for %s\n,
dev_name(adev-dev));
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index fa702ae..43d2f8e 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -838,7 +838,8 @@ spi_register_board_info(struct spi_board_info const *info,
unsigned n)
* be defined using the board info.
*/
extern struct spi_device *
-spi_alloc_device(struct spi_master *master);
+spi_alloc_device(struct spi_master *master,
+ bool device_was_children_of_master);
extern int
spi_add_device(struct spi_device *spi);
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] spi: Add the flag indicate to registe new device as children of master or not.
On Tue, 2012-12-18 at 15:26 +, Mark Brown wrote:
On Tue, Dec 18, 2012 at 11:29:34AM -0500, Jun Chen wrote:
* @master: Controller to which device is connected
+ * device_was_children_of_master is flag which the device is registed
+ * as the children of the bus
This isn't a kerneldoc style comment (it needs the @XXX: format). The
name is also extremely long, can't we think of something more concise?
Thank for your suggestion, I will correct this comment and use concise
flag.
- spi-dev.parent = master-dev;
+ if (device_was_children_of_master == true)
+ spi-dev.parent = master-dev;
+ else
+ spi-dev.parent = dev;
Can you provide an example of where this is useful? Your changelog
didn't make it clear and the code doesn't make it obvious either.
This spi_alloc_device function will be called in the spi_new_device
function to alloc new device as the master. But other way, it is called
by the of_register_spi_devices function to register new device as the
children of the master. I will update changlog to add it.
@@ -434,7 +440,7 @@ struct spi_device *spi_new_device(struct spi_master
*master,
* suggests syslogged diagnostics are best here (ugh).
*/
- proxy = spi_alloc_device(master);
+ proxy = spi_alloc_device(master, false);
if (!proxy)
return NULL;
@@ -827,7 +833,7 @@ static void of_register_spi_devices(struct
spi_master *master)
for_each_available_child_of_node(master-dev.of_node, nc) {
/* Alloc an spi_device */
- spi = spi_alloc_device(master);
+ spi = spi_alloc_device(master, true);
If I have mistake, pls correct me, thanks!
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: ifx6x60: Add modem power off function in the platform reboot process
On Thu, 2012-11-22 at 11:08 +, Alan Cox wrote: > On Thu, 22 Nov 2012 06:19:23 -0500 > Jun Chen wrote: > > > > > This patch add modem power off function in the reboot process > > according registering reboot callback to the reboot_notifier_list. > > Also realizing the spi shutdown function. > > Need to unregister the reboot notifier in the ifx_spi_exit path. If the > driver module is unloaded you leave the callback present and the kernel > will call into freed memory. > > Alan Yes,thanks! I will update this patch again. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: Add modem power off function in the platform reboot process
This patch add modem power off function in the reboot process according
registering
reboot callback to the reboot_notifier_list. Also realizing the spi shutdown
function.
Signed-off-by: Bi Chao
Signed-off-by: Chen Jun
---
drivers/tty/serial/ifx6x60.c | 56 +++--
1 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..f3c54e9 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -60,6 +60,7 @@
#include
#include
#include
+#include
#include "ifx6x60.h"
@@ -72,8 +73,14 @@
#define IFX_SPI_HEADER_0 (-1)
#define IFX_SPI_HEADER_F (-2)
+#define PO_POST_DELAY 200
+#define IFX_MDM_RST_PMU4
+
/* forward reference */
static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+ unsigned long event, void *data);
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev);
/* local variables */
static int spi_bpw = 16; /* 8, 16 or 32 bit word length */
@@ -81,6 +88,29 @@ static struct tty_driver *tty_drv;
static struct ifx_spi_device *saved_ifx_dev;
static struct lock_class_key ifx_spi_key;
+static struct notifier_block ifx_modem_reboot_notifier_block = {
+ .notifier_call = ifx_modem_reboot_callback,
+};
+
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev)
+{
+ gpio_set_value(IFX_MDM_RST_PMU, 1);
+ msleep(PO_POST_DELAY);
+
+ return 0;
+}
+
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+unsigned long event, void *data)
+{
+ if (saved_ifx_dev)
+ ifx_modem_power_off(saved_ifx_dev);
+ else
+ pr_warn("no ifx modem active;\n");
+
+ return NOTIFY_OK;
+}
+
/* GPIO/GPE settings */
/**
@@ -1219,6 +1249,9 @@ static int ifx_spi_spi_remove(struct spi_device *spi)
static void ifx_spi_spi_shutdown(struct spi_device *spi)
{
+ struct ifx_spi_device *ifx_dev = spi_get_drvdata(spi);
+
+ ifx_modem_power_off(ifx_dev);
}
/*
@@ -1354,7 +1387,9 @@ static void __exit ifx_spi_exit(void)
{
/* unregister */
tty_unregister_driver(tty_drv);
+ put_tty_driver(tty_drv);
spi_unregister_driver((void *)_spi_driver);
+ unregister_reboot_notifier(_modem_reboot_notifier_block);
}
/**
@@ -1389,16 +1424,31 @@ static int __init ifx_spi_init(void)
if (result) {
pr_err("%s: tty_register_driver failed(%d)",
DRVNAME, result);
- put_tty_driver(tty_drv);
- return result;
+ goto err_free_tty;
}
result = spi_register_driver((void *)_spi_driver);
if (result) {
pr_err("%s: spi_register_driver failed(%d)",
DRVNAME, result);
- tty_unregister_driver(tty_drv);
+ goto err_unreg_tty;
+ }
+
+ result = register_reboot_notifier(_modem_reboot_notifier_block);
+ if (result) {
+ pr_err("%s: register ifx modem reboot notifier failed(%d)",
+ DRVNAME, result);
+ goto err_unreg_spi;
}
+
+ return 0;
+err_unreg_spi:
+ spi_unregister_driver((void *)_spi_driver);
+err_unreg_tty:
+ tty_unregister_driver(tty_drv);
+err_free_tty:
+ put_tty_driver(tty_drv);
+
return result;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: Add modem power off function in the platform reboot process
This patch add modem power off function in the reboot process according
registering
reboot callback to the reboot_notifier_list. Also realizing the spi shutdown
function.
Signed-off-by: Bi Chao
Signed-off-by: Chen Jun
---
drivers/tty/serial/ifx6x60.c | 39 +++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..a91f4cb 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -60,6 +60,7 @@
#include
#include
#include
+#include
#include "ifx6x60.h"
@@ -72,8 +73,14 @@
#define IFX_SPI_HEADER_0 (-1)
#define IFX_SPI_HEADER_F (-2)
+#define PO_POST_DELAY 200
+#define IFX_MDM_RST_PMU4
+
/* forward reference */
static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+ unsigned long event, void *data);
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev);
/* local variables */
static int spi_bpw = 16; /* 8, 16 or 32 bit word length */
@@ -81,6 +88,29 @@ static struct tty_driver *tty_drv;
static struct ifx_spi_device *saved_ifx_dev;
static struct lock_class_key ifx_spi_key;
+static struct notifier_block ifx_modem_reboot_notifier_block = {
+ .notifier_call = ifx_modem_reboot_callback,
+};
+
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev)
+{
+ gpio_set_value(IFX_MDM_RST_PMU, 1);
+ msleep(PO_POST_DELAY);
+
+ return 0;
+}
+
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+unsigned long event, void *data)
+{
+ if (saved_ifx_dev)
+ ifx_modem_power_off(saved_ifx_dev);
+ else
+ pr_warn("no ifx modem active \n");
+
+ return NOTIFY_OK;
+}
+
/* GPIO/GPE settings */
/**
@@ -1219,6 +1249,9 @@ static int ifx_spi_spi_remove(struct spi_device *spi)
static void ifx_spi_spi_shutdown(struct spi_device *spi)
{
+ struct ifx_spi_device *ifx_dev = spi_get_drvdata(spi);
+
+ ifx_modem_power_off(ifx_dev);
}
/*
@@ -1368,6 +1401,7 @@ static void __exit ifx_spi_exit(void)
static int __init ifx_spi_init(void)
{
int result;
+ int ret;
tty_drv = alloc_tty_driver(1);
if (!tty_drv) {
@@ -1399,6 +1433,11 @@ static int __init ifx_spi_init(void)
DRVNAME, result);
tty_unregister_driver(tty_drv);
}
+
+ ret = register_reboot_notifier(_modem_reboot_notifier_block);
+ if (ret)
+ pr_err("cannot register reboot notifier (err=%d)\n", ret);
+
return result;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: Add modem power off function in the platform reboot process
This patch add modem power off function in the reboot process according
registering
reboot callback to the reboot_notifier_list. Also realizing the spi shutdown
function.
Signed-off-by: Bi Chao chao...@intel.com
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
drivers/tty/serial/ifx6x60.c | 39 +++
1 files changed, 39 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..a91f4cb 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -60,6 +60,7 @@
#include linux/pm_runtime.h
#include linux/spi/ifx_modem.h
#include linux/delay.h
+#include linux/reboot.h
#include ifx6x60.h
@@ -72,8 +73,14 @@
#define IFX_SPI_HEADER_0 (-1)
#define IFX_SPI_HEADER_F (-2)
+#define PO_POST_DELAY 200
+#define IFX_MDM_RST_PMU4
+
/* forward reference */
static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+ unsigned long event, void *data);
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev);
/* local variables */
static int spi_bpw = 16; /* 8, 16 or 32 bit word length */
@@ -81,6 +88,29 @@ static struct tty_driver *tty_drv;
static struct ifx_spi_device *saved_ifx_dev;
static struct lock_class_key ifx_spi_key;
+static struct notifier_block ifx_modem_reboot_notifier_block = {
+ .notifier_call = ifx_modem_reboot_callback,
+};
+
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev)
+{
+ gpio_set_value(IFX_MDM_RST_PMU, 1);
+ msleep(PO_POST_DELAY);
+
+ return 0;
+}
+
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+unsigned long event, void *data)
+{
+ if (saved_ifx_dev)
+ ifx_modem_power_off(saved_ifx_dev);
+ else
+ pr_warn(no ifx modem active \n);
+
+ return NOTIFY_OK;
+}
+
/* GPIO/GPE settings */
/**
@@ -1219,6 +1249,9 @@ static int ifx_spi_spi_remove(struct spi_device *spi)
static void ifx_spi_spi_shutdown(struct spi_device *spi)
{
+ struct ifx_spi_device *ifx_dev = spi_get_drvdata(spi);
+
+ ifx_modem_power_off(ifx_dev);
}
/*
@@ -1368,6 +1401,7 @@ static void __exit ifx_spi_exit(void)
static int __init ifx_spi_init(void)
{
int result;
+ int ret;
tty_drv = alloc_tty_driver(1);
if (!tty_drv) {
@@ -1399,6 +1433,11 @@ static int __init ifx_spi_init(void)
DRVNAME, result);
tty_unregister_driver(tty_drv);
}
+
+ ret = register_reboot_notifier(ifx_modem_reboot_notifier_block);
+ if (ret)
+ pr_err(cannot register reboot notifier (err=%d)\n, ret);
+
return result;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: Add modem power off function in the platform reboot process
This patch add modem power off function in the reboot process according
registering
reboot callback to the reboot_notifier_list. Also realizing the spi shutdown
function.
Signed-off-by: Bi Chao chao...@intel.com
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
drivers/tty/serial/ifx6x60.c | 56 +++--
1 files changed, 53 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..f3c54e9 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -60,6 +60,7 @@
#include linux/pm_runtime.h
#include linux/spi/ifx_modem.h
#include linux/delay.h
+#include linux/reboot.h
#include ifx6x60.h
@@ -72,8 +73,14 @@
#define IFX_SPI_HEADER_0 (-1)
#define IFX_SPI_HEADER_F (-2)
+#define PO_POST_DELAY 200
+#define IFX_MDM_RST_PMU4
+
/* forward reference */
static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+ unsigned long event, void *data);
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev);
/* local variables */
static int spi_bpw = 16; /* 8, 16 or 32 bit word length */
@@ -81,6 +88,29 @@ static struct tty_driver *tty_drv;
static struct ifx_spi_device *saved_ifx_dev;
static struct lock_class_key ifx_spi_key;
+static struct notifier_block ifx_modem_reboot_notifier_block = {
+ .notifier_call = ifx_modem_reboot_callback,
+};
+
+static int ifx_modem_power_off(struct ifx_spi_device *ifx_dev)
+{
+ gpio_set_value(IFX_MDM_RST_PMU, 1);
+ msleep(PO_POST_DELAY);
+
+ return 0;
+}
+
+static int ifx_modem_reboot_callback(struct notifier_block *nfb,
+unsigned long event, void *data)
+{
+ if (saved_ifx_dev)
+ ifx_modem_power_off(saved_ifx_dev);
+ else
+ pr_warn(no ifx modem active;\n);
+
+ return NOTIFY_OK;
+}
+
/* GPIO/GPE settings */
/**
@@ -1219,6 +1249,9 @@ static int ifx_spi_spi_remove(struct spi_device *spi)
static void ifx_spi_spi_shutdown(struct spi_device *spi)
{
+ struct ifx_spi_device *ifx_dev = spi_get_drvdata(spi);
+
+ ifx_modem_power_off(ifx_dev);
}
/*
@@ -1354,7 +1387,9 @@ static void __exit ifx_spi_exit(void)
{
/* unregister */
tty_unregister_driver(tty_drv);
+ put_tty_driver(tty_drv);
spi_unregister_driver((void *)ifx_spi_driver);
+ unregister_reboot_notifier(ifx_modem_reboot_notifier_block);
}
/**
@@ -1389,16 +1424,31 @@ static int __init ifx_spi_init(void)
if (result) {
pr_err(%s: tty_register_driver failed(%d),
DRVNAME, result);
- put_tty_driver(tty_drv);
- return result;
+ goto err_free_tty;
}
result = spi_register_driver((void *)ifx_spi_driver);
if (result) {
pr_err(%s: spi_register_driver failed(%d),
DRVNAME, result);
- tty_unregister_driver(tty_drv);
+ goto err_unreg_tty;
+ }
+
+ result = register_reboot_notifier(ifx_modem_reboot_notifier_block);
+ if (result) {
+ pr_err(%s: register ifx modem reboot notifier failed(%d),
+ DRVNAME, result);
+ goto err_unreg_spi;
}
+
+ return 0;
+err_unreg_spi:
+ spi_unregister_driver((void *)ifx_spi_driver);
+err_unreg_tty:
+ tty_unregister_driver(tty_drv);
+err_free_tty:
+ put_tty_driver(tty_drv);
+
return result;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] serial: ifx6x60: Add modem power off function in the platform reboot process
On Thu, 2012-11-22 at 11:08 +, Alan Cox wrote: On Thu, 22 Nov 2012 06:19:23 -0500 Jun Chen jun.d.c...@intel.com wrote: This patch add modem power off function in the reboot process according registering reboot callback to the reboot_notifier_list. Also realizing the spi shutdown function. Need to unregister the reboot notifier in the ifx_spi_exit path. If the driver module is unloaded you leave the callback present and the kernel will call into freed memory. Alan Yes,thanks! I will update this patch again. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: ifx_spi_write don't need to do mrdy_assert when fifo is not empty
This patch check whether the fifo lenth is empty before writing new data to
fifo.If condition
is true,ifx_spi_write need to trigger one mrdy_assert. If condition is
false,the mrdy_assert
will be trigger by the next ifx_spi_io.
Cc: Bi Chao
Signed-off-by: Chen Jun
---
drivers/tty/serial/ifx6x60.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..aa01989 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -469,9 +469,17 @@ static int ifx_spi_write(struct tty_struct *tty, const
unsigned char *buf,
{
struct ifx_spi_device *ifx_dev = tty->driver_data;
unsigned char *tmp_buf = (unsigned char *)buf;
- int tx_count = kfifo_in_locked(_dev->tx_fifo, tmp_buf, count,
- _dev->fifo_lock);
- mrdy_assert(ifx_dev);
+ int tx_count;
+ unsigned long flags;
+ bool is_fifo_empty;
+
+ spin_lock_irqsave(_dev->fifo_lock, flags);
+ is_fifo_empty = kfifo_is_empty(_dev->tx_fifo);
+ tx_count = kfifo_in(_dev->tx_fifo, tmp_buf, count);
+ spin_unlock_irqrestore(_dev->fifo_lock, flags);
+ if (is_fifo_empty)
+ mrdy_assert(ifx_dev);
+
return tx_count;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: ifx_spi_write don't need to do mrdy_assert when fifo is not empty
This patch check whether the fifo lenth is empty before writing new data to
fifo.If condition
is true,ifx_spi_write need to trigger one mrdy_assert. If condition is
false,the mrdy_assert
will be trigger by the next ifx_spi_io.
Cc: Bi Chao chao...@intel.com
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
drivers/tty/serial/ifx6x60.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..aa01989 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -469,9 +469,17 @@ static int ifx_spi_write(struct tty_struct *tty, const
unsigned char *buf,
{
struct ifx_spi_device *ifx_dev = tty-driver_data;
unsigned char *tmp_buf = (unsigned char *)buf;
- int tx_count = kfifo_in_locked(ifx_dev-tx_fifo, tmp_buf, count,
- ifx_dev-fifo_lock);
- mrdy_assert(ifx_dev);
+ int tx_count;
+ unsigned long flags;
+ bool is_fifo_empty;
+
+ spin_lock_irqsave(ifx_dev-fifo_lock, flags);
+ is_fifo_empty = kfifo_is_empty(ifx_dev-tx_fifo);
+ tx_count = kfifo_in(ifx_dev-tx_fifo, tmp_buf, count);
+ spin_unlock_irqrestore(ifx_dev-fifo_lock, flags);
+ if (is_fifo_empty)
+ mrdy_assert(ifx_dev);
+
return tx_count;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: Add module parameters to manage the modem status through sysfs
The Medfield Platform implements a recovery procedure consisting in an
escalation
from simple and light recovery procedures to stronger ones with increased
visibility
and impact to end-user.After platform find some problem from Modem,such as no
response,
platform will try do modem warm reset.If several tries failed, platform will
try to
do modem cold boot procedure.For Modem Cold Boot, AP is responsible to generate
blob (binary object containing PIN code and other necessary information).
Blob is stored in AP volatile memory. AP decides to read PIN code from cache
instead of
prompting end-user, and sends it to modem as if end-user had entered it.
This patch add module parameters to manage the modem status through sysfs.
Reset_modem can be read and write by user space.When read the reset_modem,user
space will
get the mdm_reset_state which used to avoid to run twice at once.When set the
reset_modem to
IFX_COLD_RESET_REQ,modem will do cold reset, other val value will trigger modem
reset.
Hangup_reasons used to give one interface to user space to know and clear the
modem reset reasons.
Now there are four reasons:SPI timeout, modem initiative reset,modem
coredump,spi tranfer error.
Cc: Bi Chao
Cc: Liu chuansheng
Acked-by: Alan Cox
Signed-off-by: Chen Jun
---
drivers/tty/serial/Kconfig |2 +-
drivers/tty/serial/ifx6x60.c | 193 ++
drivers/tty/serial/ifx6x60.h |8 ++
3 files changed, 202 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 2a53be5..640b36a 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1323,7 +1323,7 @@ config SERIAL_ALTERA_UART_CONSOLE
config SERIAL_IFX6X60
tristate "SPI protocol driver for Infineon 6x60 modem (EXPERIMENTAL)"
- depends on GPIOLIB && SPI
+ depends on GPIOLIB && SPI && X86_INTEL_MID && INTEL_SCU_IPC
help
Support for the IFX6x60 modem devices on Intel MID platforms.
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..c17efc6 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -60,6 +60,7 @@
#include
#include
#include
+#include
#include "ifx6x60.h"
@@ -72,6 +73,27 @@
#define IFX_SPI_HEADER_0 (-1)
#define IFX_SPI_HEADER_F (-2)
+
+/* Delays for powering up/resetting the modem, ms */
+#define PO_INTERLINE_DELAY 1
+#define PO_POST_DELAY 200
+
+#define IFX_COLD_RESET_REQ 1
+
+#define IFX_MDM_PWR_ON 3
+#define IFX_MDM_RST_PMU4
+
+/* For modem cold boot */
+#define V1P35CNT_W 0x0E0 /* PMIC register used to power off */
+/* the modem */
+#define V1P35_OFF 4
+#define V1P35_ON 6
+#define COLD_BOOT_DELAY_OFF_MIN2 /* 20 ms (use of usleep_range)
*/
+#define COLD_BOOT_DELAY_OFF_MAX25000 /* 25 ms (use of usleep_range)
*/
+#define COLD_BOOT_DELAY_ON_MIN 1 /* 10 ms (use of usleep_range) */
+#define COLD_BOOT_DELAY_ON_MAX 15000 /* 15 ms (use of usleep_range) */
+
+
/* forward reference */
static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
@@ -81,6 +103,35 @@ static struct tty_driver *tty_drv;
static struct ifx_spi_device *saved_ifx_dev;
static struct lock_class_key ifx_spi_key;
+
+/**
+ * do_modem_power - activity required to bring up modem
+ *
+ * Toggle gpios required to bring up modem power and start modem.
+ */
+static void do_modem_power(void)
+{
+ gpio_set_value(IFX_MDM_PWR_ON, 1);
+ mdelay(PO_INTERLINE_DELAY);
+ gpio_set_value(IFX_MDM_PWR_ON, 0);
+ msleep(PO_POST_DELAY);
+}
+
+/**
+ * do_modem_reset - activity required to reset modem
+ *
+ * Toggle gpios required to reset modem.
+ */
+static void do_modem_reset(void)
+{
+ gpio_set_value(IFX_MDM_RST_PMU, 0);
+ mdelay(PO_INTERLINE_DELAY);
+ gpio_set_value(IFX_MDM_RST_PMU, 1);
+ msleep(PO_POST_DELAY);
+}
+
+
+
/* GPIO/GPE settings */
/**
@@ -229,6 +280,7 @@ static void ifx_spi_timeout(unsigned long arg)
struct ifx_spi_device *ifx_dev = (struct ifx_spi_device *)arg;
dev_warn(_dev->spi_dev->dev, "*** SPI Timeout ***");
+ ifx_dev->hangup_reasons |= HU_TIMEOUT;
ifx_spi_ttyhangup(ifx_dev);
mrdy_set_low(ifx_dev);
clear_bit(IFX_SPI_STATE_TIMER_PENDING, _dev->flags);
@@ -881,6 +933,7 @@ static irqreturn_t ifx_spi_reset_interrupt(int irq, void
*dev)
/* exited reset */
clear_bit(MR_INPROGRESS, _dev->mdm_reset_state);
if (solreset) {
+ clear_bit(MR_START, _dev->mdm_reset_state);
set_bit(MR_COMPLETE, _dev->mdm_reset_state);
wake_up(_dev->mdm_reset_wait);
}
@@ -1405,6 +1458,146 @@ static int __init ifx_spi_init(void)
module_init(ifx_spi_init);
module_exit(ifx_spi_exit);
+/*
+ * Module parameters to manage the
[PATCH] serial: ifx6x60: ifx_spi_write don't need to do mrdy_assert when fifo is not empty.
This patch check whether the fifo lenth is empty before writing new data to
fifo.If condition
is true,ifx_spi_write need to trigger one mrdy_assert. If condition is
false,the mrdy_assert
will be trigger by the next ifx_spi_io.
Cc: Bi Chao
Signed-off-by: Chen Jun
---
drivers/tty/serial/ifx6x60.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..aa01989 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -469,9 +469,17 @@ static int ifx_spi_write(struct tty_struct *tty, const
unsigned char *buf,
{
struct ifx_spi_device *ifx_dev = tty->driver_data;
unsigned char *tmp_buf = (unsigned char *)buf;
- int tx_count = kfifo_in_locked(_dev->tx_fifo, tmp_buf, count,
- _dev->fifo_lock);
- mrdy_assert(ifx_dev);
+ int tx_count;
+ unsigned long flags;
+ bool is_fifo_empty;
+
+ spin_lock_irqsave(_dev->fifo_lock, flags);
+ is_fifo_empty = kfifo_is_empty(_dev->tx_fifo);
+ tx_count = kfifo_in(_dev->tx_fifo, tmp_buf, count);
+ spin_unlock_irqrestore(_dev->fifo_lock, flags);
+ if (is_fifo_empty)
+ mrdy_assert(ifx_dev);
+
return tx_count;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: ifx_spi_write don't need to do mrdy_assert when fifo is not empty.
This patch check whether the fifo lenth is empty before writing new data to
fifo.If condition
is true,ifx_spi_write need to trigger one mrdy_assert. If condition is
false,the mrdy_assert
will be trigger by the next ifx_spi_io.
Cc: Bi Chao chao...@intel.com
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
drivers/tty/serial/ifx6x60.c | 14 +++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..aa01989 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -469,9 +469,17 @@ static int ifx_spi_write(struct tty_struct *tty, const
unsigned char *buf,
{
struct ifx_spi_device *ifx_dev = tty-driver_data;
unsigned char *tmp_buf = (unsigned char *)buf;
- int tx_count = kfifo_in_locked(ifx_dev-tx_fifo, tmp_buf, count,
- ifx_dev-fifo_lock);
- mrdy_assert(ifx_dev);
+ int tx_count;
+ unsigned long flags;
+ bool is_fifo_empty;
+
+ spin_lock_irqsave(ifx_dev-fifo_lock, flags);
+ is_fifo_empty = kfifo_is_empty(ifx_dev-tx_fifo);
+ tx_count = kfifo_in(ifx_dev-tx_fifo, tmp_buf, count);
+ spin_unlock_irqrestore(ifx_dev-fifo_lock, flags);
+ if (is_fifo_empty)
+ mrdy_assert(ifx_dev);
+
return tx_count;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] serial: ifx6x60: Add module parameters to manage the modem status through sysfs
The Medfield Platform implements a recovery procedure consisting in an
escalation
from simple and light recovery procedures to stronger ones with increased
visibility
and impact to end-user.After platform find some problem from Modem,such as no
response,
platform will try do modem warm reset.If several tries failed, platform will
try to
do modem cold boot procedure.For Modem Cold Boot, AP is responsible to generate
blob (binary object containing PIN code and other necessary information).
Blob is stored in AP volatile memory. AP decides to read PIN code from cache
instead of
prompting end-user, and sends it to modem as if end-user had entered it.
This patch add module parameters to manage the modem status through sysfs.
Reset_modem can be read and write by user space.When read the reset_modem,user
space will
get the mdm_reset_state which used to avoid to run twice at once.When set the
reset_modem to
IFX_COLD_RESET_REQ,modem will do cold reset, other val value will trigger modem
reset.
Hangup_reasons used to give one interface to user space to know and clear the
modem reset reasons.
Now there are four reasons:SPI timeout, modem initiative reset,modem
coredump,spi tranfer error.
Cc: Bi Chao chao...@intel.com
Cc: Liu chuansheng chuansheng@intel.com
Acked-by: Alan Cox a...@linux.intel.com
Signed-off-by: Chen Jun jun.d.c...@intel.com
---
drivers/tty/serial/Kconfig |2 +-
drivers/tty/serial/ifx6x60.c | 193 ++
drivers/tty/serial/ifx6x60.h |8 ++
3 files changed, 202 insertions(+), 1 deletions(-)
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 2a53be5..640b36a 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1323,7 +1323,7 @@ config SERIAL_ALTERA_UART_CONSOLE
config SERIAL_IFX6X60
tristate SPI protocol driver for Infineon 6x60 modem (EXPERIMENTAL)
- depends on GPIOLIB SPI
+ depends on GPIOLIB SPI X86_INTEL_MID INTEL_SCU_IPC
help
Support for the IFX6x60 modem devices on Intel MID platforms.
diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 5b9bc19..c17efc6 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -60,6 +60,7 @@
#include linux/pm_runtime.h
#include linux/spi/ifx_modem.h
#include linux/delay.h
+#include asm/intel_scu_ipc.h
#include ifx6x60.h
@@ -72,6 +73,27 @@
#define IFX_SPI_HEADER_0 (-1)
#define IFX_SPI_HEADER_F (-2)
+
+/* Delays for powering up/resetting the modem, ms */
+#define PO_INTERLINE_DELAY 1
+#define PO_POST_DELAY 200
+
+#define IFX_COLD_RESET_REQ 1
+
+#define IFX_MDM_PWR_ON 3
+#define IFX_MDM_RST_PMU4
+
+/* For modem cold boot */
+#define V1P35CNT_W 0x0E0 /* PMIC register used to power off */
+/* the modem */
+#define V1P35_OFF 4
+#define V1P35_ON 6
+#define COLD_BOOT_DELAY_OFF_MIN2 /* 20 ms (use of usleep_range)
*/
+#define COLD_BOOT_DELAY_OFF_MAX25000 /* 25 ms (use of usleep_range)
*/
+#define COLD_BOOT_DELAY_ON_MIN 1 /* 10 ms (use of usleep_range) */
+#define COLD_BOOT_DELAY_ON_MAX 15000 /* 15 ms (use of usleep_range) */
+
+
/* forward reference */
static void ifx_spi_handle_srdy(struct ifx_spi_device *ifx_dev);
@@ -81,6 +103,35 @@ static struct tty_driver *tty_drv;
static struct ifx_spi_device *saved_ifx_dev;
static struct lock_class_key ifx_spi_key;
+
+/**
+ * do_modem_power - activity required to bring up modem
+ *
+ * Toggle gpios required to bring up modem power and start modem.
+ */
+static void do_modem_power(void)
+{
+ gpio_set_value(IFX_MDM_PWR_ON, 1);
+ mdelay(PO_INTERLINE_DELAY);
+ gpio_set_value(IFX_MDM_PWR_ON, 0);
+ msleep(PO_POST_DELAY);
+}
+
+/**
+ * do_modem_reset - activity required to reset modem
+ *
+ * Toggle gpios required to reset modem.
+ */
+static void do_modem_reset(void)
+{
+ gpio_set_value(IFX_MDM_RST_PMU, 0);
+ mdelay(PO_INTERLINE_DELAY);
+ gpio_set_value(IFX_MDM_RST_PMU, 1);
+ msleep(PO_POST_DELAY);
+}
+
+
+
/* GPIO/GPE settings */
/**
@@ -229,6 +280,7 @@ static void ifx_spi_timeout(unsigned long arg)
struct ifx_spi_device *ifx_dev = (struct ifx_spi_device *)arg;
dev_warn(ifx_dev-spi_dev-dev, *** SPI Timeout ***);
+ ifx_dev-hangup_reasons |= HU_TIMEOUT;
ifx_spi_ttyhangup(ifx_dev);
mrdy_set_low(ifx_dev);
clear_bit(IFX_SPI_STATE_TIMER_PENDING, ifx_dev-flags);
@@ -881,6 +933,7 @@ static irqreturn_t ifx_spi_reset_interrupt(int irq, void
*dev)
/* exited reset */
clear_bit(MR_INPROGRESS, ifx_dev-mdm_reset_state);
if (solreset) {
+ clear_bit(MR_START, ifx_dev-mdm_reset_state);
set_bit(MR_COMPLETE, ifx_dev-mdm_reset_state);
wake_up(ifx_dev-mdm_reset_wait);
}
@@ -1405,6
