Re: [PATCH] rtl8188eu: rtw_xmit: Replace rcu_dereference() with rcu_access_pointer()
The rcu_dereference() calls are no longer there.
They were removed.
Sorry,
Andreea
2014-09-08 23:48 GMT+03:00 Greg KH :
> On Thu, Sep 04, 2014 at 08:15:48PM +0300, Andreea-Cristina Bernat wrote:
>> The "br_port" local variables obtained through the rcu_dereference() calls
>> are
>> not dereferenced in the rest of their function.
>> Therefore, it is recommended to use rcu_access_pointer() instead of
>> rcu_dereference().
>> This patch makes the replacements.
>>
>> The first step to detect this was made with the following Coccinelle semantic
>> patch:
>> @@
>> identifier p;
>> @@
>>
>> * p = rcu_dereference(...)
>> ... when any
>> when != p
>> (
>> * if( (<+...p...+>) ) { ... }
>> |
>> * while( (<+...p...+>) ) { ... }
>> )
>> ... when != p
>>
>> After the analysis of the output, the change was made manually.
>>
>> Signed-off-by: Andreea-Cristina Bernat
>> ---
>> drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
>> b/drivers/staging/rtl8188eu/core/rtw_xmit.c
>> index 1413ec8..93228dc 100644
>> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
>> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
>> @@ -1684,7 +1684,7 @@ static int rtw_br_client_tx(struct adapter *padapter,
>> struct sk_buff **pskb)
>> void *br_port = NULL;
>>
>> rcu_read_lock();
>> - br_port = rcu_dereference(padapter->pnetdev->rx_handler_data);
>> + br_port = rcu_access_pointer(padapter->pnetdev->rx_handler_data);
>> rcu_read_unlock();
>> spin_lock_bh(>br_ext_lock);
>> if (!(skb->data[0] & 1) && br_port &&
>> @@ -1868,7 +1868,7 @@ s32 rtw_xmit(struct adapter *padapter, struct sk_buff
>> **ppkt)
>> }
>>
>> rcu_read_lock();
>> - br_port = rcu_dereference(padapter->pnetdev->rx_handler_data);
>> + br_port = rcu_access_pointer(padapter->pnetdev->rx_handler_data);
>> rcu_read_unlock();
>>
>> if (br_port && check_fwstate(pmlmepriv,
>> WIFI_STATION_STATE|WIFI_ADHOC_STATE)) {
>
> This patch doesn't apply at all to my staging-next branch of the
> staging.git tree on git.kernel.org :(
>
--
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] rtl8188eu: rtw_xmit: Replace rcu_dereference() with rcu_access_pointer()
The rcu_dereference() calls are no longer there.
They were removed.
Sorry,
Andreea
2014-09-08 23:48 GMT+03:00 Greg KH gre...@linuxfoundation.org:
On Thu, Sep 04, 2014 at 08:15:48PM +0300, Andreea-Cristina Bernat wrote:
The br_port local variables obtained through the rcu_dereference() calls
are
not dereferenced in the rest of their function.
Therefore, it is recommended to use rcu_access_pointer() instead of
rcu_dereference().
This patch makes the replacements.
The first step to detect this was made with the following Coccinelle semantic
patch:
@@
identifier p;
@@
* p = rcu_dereference(...)
... when any
when != p
(
* if( (+...p...+) ) { ... }
|
* while( (+...p...+) ) { ... }
)
... when != p
After the analysis of the output, the change was made manually.
Signed-off-by: Andreea-Cristina Bernat bernat@gmail.com
---
drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 1413ec8..93228dc 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -1684,7 +1684,7 @@ static int rtw_br_client_tx(struct adapter *padapter,
struct sk_buff **pskb)
void *br_port = NULL;
rcu_read_lock();
- br_port = rcu_dereference(padapter-pnetdev-rx_handler_data);
+ br_port = rcu_access_pointer(padapter-pnetdev-rx_handler_data);
rcu_read_unlock();
spin_lock_bh(padapter-br_ext_lock);
if (!(skb-data[0] 1) br_port
@@ -1868,7 +1868,7 @@ s32 rtw_xmit(struct adapter *padapter, struct sk_buff
**ppkt)
}
rcu_read_lock();
- br_port = rcu_dereference(padapter-pnetdev-rx_handler_data);
+ br_port = rcu_access_pointer(padapter-pnetdev-rx_handler_data);
rcu_read_unlock();
if (br_port check_fwstate(pmlmepriv,
WIFI_STATION_STATE|WIFI_ADHOC_STATE)) {
This patch doesn't apply at all to my staging-next branch of the
staging.git tree on git.kernel.org :(
--
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] rtl8188eu: Replace rcu_dereference() with rcu_access_pointer()
2014-09-05 0:38 GMT+03:00 Greg KH : > On Thu, Sep 04, 2014 at 11:58:36PM +0300, Andreea Bernat wrote: >> Hello, >> >> I cloned this: >> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git >> >> and after moved to staging-next branch, but in both cases, in >> those files I don't find any use of rcu_dereference() call (the call >> which I am looking for to modify). > > Then there's nothing left to be done here, right? :) Yes :-) Thank you, Andreea > > thanks, > > greg k-h -- 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] rtl8188eu: Replace rcu_dereference() with rcu_access_pointer()
2014-09-05 0:38 GMT+03:00 Greg KH gre...@linuxfoundation.org: On Thu, Sep 04, 2014 at 11:58:36PM +0300, Andreea Bernat wrote: Hello, I cloned this: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git and after moved to staging-next branch, but in both cases, in those files I don't find any use of rcu_dereference() call (the call which I am looking for to modify). Then there's nothing left to be done here, right? :) Yes :-) Thank you, Andreea thanks, greg k-h -- 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] rtl8188eu: Replace rcu_dereference() with rcu_access_pointer()
Hello,
I cloned this:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
and after moved to staging-next branch, but in both cases, in
those files I don't find any use of rcu_dereference() call (the call
which I am looking for to modify).
Thank you,
Andreea
2014-08-30 23:42 GMT+03:00 Greg KH :
> On Sun, Aug 17, 2014 at 02:43:37PM +0300, Andreea-Cristina Bernat wrote:
>> The "rcu_dereference()" call is used directly in a condition.
>> Since its return value is never dereferenced it is recommended to use
>> "rcu_access_pointer()" instead of "rcu_dereference()".
>> Therefore, this patch makes the replacement.
>>
>> The following Coccinelle semantic patch was used:
>> @@
>> @@
>>
>> (
>> if(
>> (<+...
>> - rcu_dereference
>> + rcu_access_pointer
>> (...)
>> ...+>)) {...}
>> |
>> while(
>> (<+...
>> - rcu_dereference
>> + rcu_access_pointer
>> (...)
>> ...+>)) {...}
>> )
>>
>> Signed-off-by: Andreea-Cristina Bernat
>> ---
>> drivers/staging/rtl8188eu/core/rtw_mlme.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> This patch fails to apply to my tree, can you refresh it against the
> staging-next branch of the staging.git tree on kernel.org and resend?
>
> thanks,
>
> greg k-h
--
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] rtl8188eu: Replace rcu_dereference() with rcu_access_pointer()
Hello,
I cloned this:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
and after moved to staging-next branch, but in both cases, in
those files I don't find any use of rcu_dereference() call (the call
which I am looking for to modify).
Thank you,
Andreea
2014-08-30 23:42 GMT+03:00 Greg KH gre...@linuxfoundation.org:
On Sun, Aug 17, 2014 at 02:43:37PM +0300, Andreea-Cristina Bernat wrote:
The rcu_dereference() call is used directly in a condition.
Since its return value is never dereferenced it is recommended to use
rcu_access_pointer() instead of rcu_dereference().
Therefore, this patch makes the replacement.
The following Coccinelle semantic patch was used:
@@
@@
(
if(
(+...
- rcu_dereference
+ rcu_access_pointer
(...)
...+)) {...}
|
while(
(+...
- rcu_dereference
+ rcu_access_pointer
(...)
...+)) {...}
)
Signed-off-by: Andreea-Cristina Bernat bernat@gmail.com
---
drivers/staging/rtl8188eu/core/rtw_mlme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
This patch fails to apply to my tree, can you refresh it against the
staging-next branch of the staging.git tree on kernel.org and resend?
thanks,
greg k-h
--
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 RFC] rcu: assoc_array: Add critical section to avoid a bug
On Fri, Aug 29, 2014 at 11:49:04PM +0100, David Howells wrote: > Andreea Bernat wrote: > > > Looks good to me. > > Can I put that down as a Reviewed-by? Yes, it is OK. Regards, Andreea > > Thanks, > David -- 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 RFC] rcu: assoc_array: Add critical section to avoid a bug
On Thu, Aug 28, 2014 at 12:32:31PM +0100, David Howells wrote: > Andreea-Cristina Bernat wrote: > > > * The function "assoc_array_gc()" could be preempted between the call to > > "assoc_array_apply_edit()" call and the assignment > > "edit->array->nr_leaves_on_tree = nr_leaves_on_tree;", so the grace > > period could complete. > > The bug is real, but this patch isn't the right solution. > > The edit script should be considered inaccessible to a function the moment it > calls assoc_array_apply_edit() or assoc_array_cancel_edit(). I think this is > a better way. > > David > --- > assoc_array.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > commit 10c26fcc224c0515e15272515e7b9006cb08adc8 > Author: David Howells > Date: Wed Aug 27 18:39:44 2014 +0100 > > KEYS: Fix use-after-free in assoc_array_gc() > > An edit script should be considered inaccessible by a function once it has > called assoc_array_apply_edit() or assoc_array_cancel_edit(). > > However, assoc_array_gc() is accessing the edit script just after the > gc_complete: label. > > Reported-by: Andreea-Cristina Bernat > Signed-off-by: David Howells > > diff --git a/lib/assoc_array.c b/lib/assoc_array.c > index c0b1007011e1..ae146f0734eb 100644 > --- a/lib/assoc_array.c > +++ b/lib/assoc_array.c > @@ -1735,7 +1735,7 @@ ascend_old_tree: > gc_complete: > edit->set[0].to = new_root; > assoc_array_apply_edit(edit); > - edit->array->nr_leaves_on_tree = nr_leaves_on_tree; > + array->nr_leaves_on_tree = nr_leaves_on_tree; > return 0; Looks good to me. Regards, Andreea > > enomem: -- 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 RFC] rcu: assoc_array: Add critical section to avoid a bug
On Thu, Aug 28, 2014 at 12:32:31PM +0100, David Howells wrote: Andreea-Cristina Bernat bernat@gmail.com wrote: * The function assoc_array_gc() could be preempted between the call to assoc_array_apply_edit() call and the assignment edit-array-nr_leaves_on_tree = nr_leaves_on_tree;, so the grace period could complete. The bug is real, but this patch isn't the right solution. The edit script should be considered inaccessible to a function the moment it calls assoc_array_apply_edit() or assoc_array_cancel_edit(). I think this is a better way. David --- assoc_array.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) commit 10c26fcc224c0515e15272515e7b9006cb08adc8 Author: David Howells dhowe...@redhat.com Date: Wed Aug 27 18:39:44 2014 +0100 KEYS: Fix use-after-free in assoc_array_gc() An edit script should be considered inaccessible by a function once it has called assoc_array_apply_edit() or assoc_array_cancel_edit(). However, assoc_array_gc() is accessing the edit script just after the gc_complete: label. Reported-by: Andreea-Cristina Bernat bernat@gmail.com Signed-off-by: David Howells dhowe...@redhat.com diff --git a/lib/assoc_array.c b/lib/assoc_array.c index c0b1007011e1..ae146f0734eb 100644 --- a/lib/assoc_array.c +++ b/lib/assoc_array.c @@ -1735,7 +1735,7 @@ ascend_old_tree: gc_complete: edit-set[0].to = new_root; assoc_array_apply_edit(edit); - edit-array-nr_leaves_on_tree = nr_leaves_on_tree; + array-nr_leaves_on_tree = nr_leaves_on_tree; return 0; Looks good to me. Regards, Andreea enomem: -- 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 RFC] rcu: assoc_array: Add critical section to avoid a bug
On Fri, Aug 29, 2014 at 11:49:04PM +0100, David Howells wrote: Andreea Bernat bernat@gmail.com wrote: Looks good to me. Can I put that down as a Reviewed-by? Yes, it is OK. Regards, Andreea Thanks, David -- 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] carl9170: Replace rcu_dereference() with rcu_access_pointer()
On Wed, Aug 20, 2014 at 01:20:02PM -0700, Christian Lamparter wrote:
> On Wednesday, August 20, 2014 08:32:11 PM Andreea Bernat wrote:
> > On Mon, Aug 18, 2014 at 09:29:36PM +0200, Christian Lamparter wrote:
> > > On Sunday, August 17, 2014 01:48:07 PM Andreea-Cristina Bernat wrote:
> > > > The rcu_dereference() call is used directly in a condition.
> > > > Since its return value is never dereferenced it is recommended to use
> > > > "rcu_access_pointer()" instead of "rcu_dereference()".
> > > > Therefore, this patch makes the replacement.
> > > > [...]
> > > > Signed-off-by: Andreea-Cristina Bernat
> > > > ---
> > > > drivers/net/wireless/ath/carl9170/main.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/wireless/ath/carl9170/main.c
> > > > b/drivers/net/wireless/ath/carl9170/main.c
> > > > index f8ded84..12018ff 100644
> > > > --- a/drivers/net/wireless/ath/carl9170/main.c
> > > > +++ b/drivers/net/wireless/ath/carl9170/main.c
> > > > @@ -1431,7 +1431,7 @@ static int carl9170_op_ampdu_action(struct
> > > > ieee80211_hw *hw,
> > > > return -EOPNOTSUPP;
> > > >
> > > > rcu_read_lock();
> > > > - if (rcu_dereference(sta_info->agg[tid])) {
> > > > + if (rcu_access_pointer(sta_info->agg[tid])) {
> > > > rcu_read_unlock();
> > > > return -EBUSY;
> > > > }
> > >
> > > There's more. The check does not do a whole lot. I think *it* [the check]
> > > and the
> > > rcu_read_[un]lock [and the return -EBUSY] can be removed completely from
> > > the
> > > IEEE80211_AMPDU_TX_START code-path in carl9170_op_ampdu_action.
> > >
> > > It would be awesome, if you could you make a patch which removes this
> > > unneeded cosmic-ray-protection check :-) .
> >
> > Could you tell me why you think that those lines have to be removed?
> The carl9170_op_ampdu_action callback is used exclusively by the mac80211
> framework to notify the driver about setup and tear down of TX and RX
> aggregation sessions. Hence, mac80211 takes great care of performing
> sanity checks and properly serializing calls to the driver's ampdu_action
> callback.
>
> Specifically mac80211 already prevents the START of an TX aggregation session,
> if the aggregation session is already active [0]. Therefore the driver doesn't
> need to perform a similar check as well. This is why:
> - the expression (rcu_dereference(sta_info->agg[tid])) never evaluates to
> true
> -> the -EBUSY exit path is "dead code"
>
> And without the rcu_dereference(...) the rcu_read protection is not needed
> either. So it can be removed for this case as well.
>
> > I would like to fully understand this before I remove them.
> Let me know if the explanation above answers sufficient :).
> If not, I need some *pointers* to what needs further
> explanation.
Your explanation is clear. I will send a version two of the patch with
those lines removed.
Thank you,
Andreea
>
> Regards
> Christian
>
> [0] <http://lxr.free-electrons.com/source/net/mac80211/agg-tx.c#L583>
>
--
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] carl9170: Replace rcu_dereference() with rcu_access_pointer()
On Wed, Aug 20, 2014 at 01:20:02PM -0700, Christian Lamparter wrote:
On Wednesday, August 20, 2014 08:32:11 PM Andreea Bernat wrote:
On Mon, Aug 18, 2014 at 09:29:36PM +0200, Christian Lamparter wrote:
On Sunday, August 17, 2014 01:48:07 PM Andreea-Cristina Bernat wrote:
The rcu_dereference() call is used directly in a condition.
Since its return value is never dereferenced it is recommended to use
rcu_access_pointer() instead of rcu_dereference().
Therefore, this patch makes the replacement.
[...]
Signed-off-by: Andreea-Cristina Bernat bernat@gmail.com
---
drivers/net/wireless/ath/carl9170/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/carl9170/main.c
b/drivers/net/wireless/ath/carl9170/main.c
index f8ded84..12018ff 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1431,7 +1431,7 @@ static int carl9170_op_ampdu_action(struct
ieee80211_hw *hw,
return -EOPNOTSUPP;
rcu_read_lock();
- if (rcu_dereference(sta_info-agg[tid])) {
+ if (rcu_access_pointer(sta_info-agg[tid])) {
rcu_read_unlock();
return -EBUSY;
}
There's more. The check does not do a whole lot. I think *it* [the check]
and the
rcu_read_[un]lock [and the return -EBUSY] can be removed completely from
the
IEEE80211_AMPDU_TX_START code-path in carl9170_op_ampdu_action.
It would be awesome, if you could you make a patch which removes this
unneeded cosmic-ray-protection check :-) .
Could you tell me why you think that those lines have to be removed?
The carl9170_op_ampdu_action callback is used exclusively by the mac80211
framework to notify the driver about setup and tear down of TX and RX
aggregation sessions. Hence, mac80211 takes great care of performing
sanity checks and properly serializing calls to the driver's ampdu_action
callback.
Specifically mac80211 already prevents the START of an TX aggregation session,
if the aggregation session is already active [0]. Therefore the driver doesn't
need to perform a similar check as well. This is why:
- the expression (rcu_dereference(sta_info-agg[tid])) never evaluates to
true
- the -EBUSY exit path is dead code
And without the rcu_dereference(...) the rcu_read protection is not needed
either. So it can be removed for this case as well.
I would like to fully understand this before I remove them.
Let me know if the explanation above answers sufficient :).
If not, I need some *pointers* to what needs further
explanation.
Your explanation is clear. I will send a version two of the patch with
those lines removed.
Thank you,
Andreea
Regards
Christian
[0] http://lxr.free-electrons.com/source/net/mac80211/agg-tx.c#L583
--
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] carl9170: Replace rcu_dereference() with rcu_access_pointer()
On Mon, Aug 18, 2014 at 09:29:36PM +0200, Christian Lamparter wrote:
> On Sunday, August 17, 2014 01:48:07 PM Andreea-Cristina Bernat wrote:
> > The rcu_dereference() call is used directly in a condition.
> > Since its return value is never dereferenced it is recommended to use
> > "rcu_access_pointer()" instead of "rcu_dereference()".
> > Therefore, this patch makes the replacement.
> > [...]
> > Signed-off-by: Andreea-Cristina Bernat
> > ---
> > drivers/net/wireless/ath/carl9170/main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/carl9170/main.c
> > b/drivers/net/wireless/ath/carl9170/main.c
> > index f8ded84..12018ff 100644
> > --- a/drivers/net/wireless/ath/carl9170/main.c
> > +++ b/drivers/net/wireless/ath/carl9170/main.c
> > @@ -1431,7 +1431,7 @@ static int carl9170_op_ampdu_action(struct
> > ieee80211_hw *hw,
> > return -EOPNOTSUPP;
> >
> > rcu_read_lock();
> > - if (rcu_dereference(sta_info->agg[tid])) {
> > + if (rcu_access_pointer(sta_info->agg[tid])) {
> > rcu_read_unlock();
> > return -EBUSY;
> > }
>
> There's more. The check does not do a whole lot. I think *it* [the check] and
> the
> rcu_read_[un]lock [and the return -EBUSY] can be removed completely from the
> IEEE80211_AMPDU_TX_START code-path in carl9170_op_ampdu_action.
>
> It would be awesome, if you could you make a patch which removes this
> unneeded cosmic-ray-protection check :-) .
Could you tell me why you think that those lines have to be removed?
I would like to fully understand this before I remove them.
Thank you,
Andreea
>
> Thanks
> Christian
--
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] carl9170: Replace rcu_dereference() with rcu_access_pointer()
On Mon, Aug 18, 2014 at 09:29:36PM +0200, Christian Lamparter wrote:
On Sunday, August 17, 2014 01:48:07 PM Andreea-Cristina Bernat wrote:
The rcu_dereference() call is used directly in a condition.
Since its return value is never dereferenced it is recommended to use
rcu_access_pointer() instead of rcu_dereference().
Therefore, this patch makes the replacement.
[...]
Signed-off-by: Andreea-Cristina Bernat bernat@gmail.com
---
drivers/net/wireless/ath/carl9170/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/carl9170/main.c
b/drivers/net/wireless/ath/carl9170/main.c
index f8ded84..12018ff 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1431,7 +1431,7 @@ static int carl9170_op_ampdu_action(struct
ieee80211_hw *hw,
return -EOPNOTSUPP;
rcu_read_lock();
- if (rcu_dereference(sta_info-agg[tid])) {
+ if (rcu_access_pointer(sta_info-agg[tid])) {
rcu_read_unlock();
return -EBUSY;
}
There's more. The check does not do a whole lot. I think *it* [the check] and
the
rcu_read_[un]lock [and the return -EBUSY] can be removed completely from the
IEEE80211_AMPDU_TX_START code-path in carl9170_op_ampdu_action.
It would be awesome, if you could you make a patch which removes this
unneeded cosmic-ray-protection check :-) .
Could you tell me why you think that those lines have to be removed?
I would like to fully understand this before I remove them.
Thank you,
Andreea
Thanks
Christian
--
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] rcu: Replace rcu_dereference() with rcu_access_pointer()
Sorry, I will send a version two of the patches with the subject line changed. Thank you, Andreea 2014-08-17 4:50 GMT+03:00 David Miller : > > Please do not use the same exact Subject line for all of these patches. > > Otherwise nobody reading the shortlog can tell what might be different > from one patch to another. > > Instead, use a subsystem prefix for the place where you're making > your change. For example, for the two networking patches you could > replace "rcu: " with "bonding: " and "cnic: ". > > 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] rcu: Replace rcu_dereference() with rcu_access_pointer()
Sorry, I will send a version two of the patches with the subject line changed. Thank you, Andreea 2014-08-17 4:50 GMT+03:00 David Miller da...@davemloft.net: Please do not use the same exact Subject line for all of these patches. Otherwise nobody reading the shortlog can tell what might be different from one patch to another. Instead, use a subsystem prefix for the place where you're making your change. For example, for the two networking patches you could replace rcu: with bonding: and cnic: . 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/
