* ieee80211_i.h: shouldn't struct ps_data.tim be aligned unsigned long?
@ 2013-12-18 8:16 Joe Perches
2013-12-18 9:19 ` Johannes Berg
0 siblings, 1 reply; 5+ messages in thread
From: Joe Perches @ 2013-12-18 8:16 UTC (permalink / raw)
To: Jiri Benc, Johannes Berg; +Cc: linux-wireless, netdev
about
net/mac80211/tx.c: have_bits = !bitmap_empty((unsigned long*)ps->tim,
and
net/mac80211/ieee80211_i.h
d012a6051 (Marco Porsch 2012-10-10 12:39:50 -0700 244) struct ps_data {
f0706e828 (Jiri Benc 2007-05-05 11:45:53 -0700 245) /* yes, this looks ugly, but guarantees that we can later use
f0706e828 (Jiri Benc 2007-05-05 11:45:53 -0700 246) * bitmap_empty :)
004c872e7 (Johannes Berg 2008-02-20 11:21:35 +0100 247) * NB: don't touch this bitmap, use sta_info_{set,clear}_tim_bit */
f0706e828 (Jiri Benc 2007-05-05 11:45:53 -0700 248) u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)];
d012a6051 (Marco Porsch 2012-10-10 12:39:50 -0700 249) struct sk_buff_head bc_buf;
056cdd599 (Johannes Berg 2008-03-26 23:21:47 +0100 250) atomic_t num_sta_ps; /* number of stations in PS mode */
5dfdaf58d (Johannes Berg 2007-12-19 02:03:33 +0100 251) int dtim_count;
512119b36 (Christian Lamparter 2011-01-31 20:48:44 +0200 252) bool dtim_bc_mc;
f0706e828 (Jiri Benc 2007-05-05 11:45:53 -0700 253) };
Perhaps the
u8 tim[sizeof...]
member should be marked
__aligned(sizeof(unsigned long))
so that the cast to ulong when bitmap_empty
is actually forced to be ulong aligned?
Perhaps the "guarantee" isn't valid without it.
---
net/mac80211/ieee80211_i.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ed5bf8b..e5a9c51 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -245,7 +245,8 @@ struct ps_data {
/* yes, this looks ugly, but guarantees that we can later use
* bitmap_empty :)
* NB: don't touch this bitmap, use sta_info_{set,clear}_tim_bit */
- u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)];
+ u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)]
+ __aligned(sizeof(unsigned long));
struct sk_buff_head bc_buf;
atomic_t num_sta_ps; /* number of stations in PS mode */
int dtim_count;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: ieee80211_i.h: shouldn't struct ps_data.tim be aligned unsigned long?
2013-12-18 8:16 ieee80211_i.h: shouldn't struct ps_data.tim be aligned unsigned long? Joe Perches
@ 2013-12-18 9:19 ` Johannes Berg
2013-12-18 9:23 ` Joe Perches
2013-12-19 0:52 ` [PATCH] ieee80211_i.h: Align struct ps_data.tim to unsigned long Joe Perches
0 siblings, 2 replies; 5+ messages in thread
From: Johannes Berg @ 2013-12-18 9:19 UTC (permalink / raw)
To: Joe Perches; +Cc: Jiri Benc, linux-wireless, netdev
On Wed, 2013-12-18 at 00:16 -0800, Joe Perches wrote:
> Perhaps the
> u8 tim[sizeof...]
> member should be marked
> __aligned(sizeof(unsigned long))
> so that the cast to ulong when bitmap_empty
> is actually forced to be ulong aligned?
yeah, I suppose that's true.
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index ed5bf8b..e5a9c51 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -245,7 +245,8 @@ struct ps_data {
> /* yes, this looks ugly, but guarantees that we can later use
> * bitmap_empty :)
> * NB: don't touch this bitmap, use sta_info_{set,clear}_tim_bit */
> - u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)];
> + u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)]
> + __aligned(sizeof(unsigned long));
Care to send a proper patch? Otherwise I'll just re-do it and pick it up
that way ...
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: ieee80211_i.h: shouldn't struct ps_data.tim be aligned unsigned long?
2013-12-18 9:19 ` Johannes Berg
@ 2013-12-18 9:23 ` Joe Perches
2013-12-19 0:52 ` [PATCH] ieee80211_i.h: Align struct ps_data.tim to unsigned long Joe Perches
1 sibling, 0 replies; 5+ messages in thread
From: Joe Perches @ 2013-12-18 9:23 UTC (permalink / raw)
To: Johannes Berg; +Cc: Jiri Benc, linux-wireless, netdev
On Wed, 2013-12-18 at 10:19 +0100, Johannes Berg wrote:
> On Wed, 2013-12-18 at 00:16 -0800, Joe Perches wrote:
>
> > Perhaps the
> > u8 tim[sizeof...]
> > member should be marked
> > __aligned(sizeof(unsigned long))
> > so that the cast to ulong when bitmap_empty
> > is actually forced to be ulong aligned?
>
> yeah, I suppose that's true.
>
> > diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> > index ed5bf8b..e5a9c51 100644
> > --- a/net/mac80211/ieee80211_i.h
> > +++ b/net/mac80211/ieee80211_i.h
> > @@ -245,7 +245,8 @@ struct ps_data {
> > /* yes, this looks ugly, but guarantees that we can later use
> > * bitmap_empty :)
> > * NB: don't touch this bitmap, use sta_info_{set,clear}_tim_bit */
> > - u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)];
> > + u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)]
> > + __aligned(sizeof(unsigned long));
>
> Care to send a proper patch? Otherwise I'll just re-do it and pick it up
> that way ...
Hi Johannes.
You can redo it easier than applying a patch
from me.
I just noticed it when the reformatting
patch went by.
No need to give a credit or anything.
It's just a nit.
cheers, Joe
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] ieee80211_i.h: Align struct ps_data.tim to unsigned long
2013-12-18 9:19 ` Johannes Berg
2013-12-18 9:23 ` Joe Perches
@ 2013-12-19 0:52 ` Joe Perches
2013-12-19 8:19 ` Johannes Berg
1 sibling, 1 reply; 5+ messages in thread
From: Joe Perches @ 2013-12-19 0:52 UTC (permalink / raw)
To: Johannes Berg; +Cc: Jiri Benc, linux-wireless, netdev
Its address is used as an unsigned long *, so make sure
that the tim u8 array is properly aligned.
Signed-off-by: Joe Perches <joe@perches.com
---
> Care to send a proper patch? Otherwise I'll just re-do it and pick it up
> that way ...
I had a passing thought about this when out today.
It should really be set to __aligned(__alignof(unsigned long))
so here's a proper patch.
net/mac80211/ieee80211_i.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ed5bf8b..9a61eef 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -245,7 +245,8 @@ struct ps_data {
/* yes, this looks ugly, but guarantees that we can later use
* bitmap_empty :)
* NB: don't touch this bitmap, use sta_info_{set,clear}_tim_bit */
- u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)];
+ u8 tim[sizeof(unsigned long) * BITS_TO_LONGS(IEEE80211_MAX_AID + 1)]
+ __aligned(__alignof__(unsigned long));
struct sk_buff_head bc_buf;
atomic_t num_sta_ps; /* number of stations in PS mode */
int dtim_count;
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ieee80211_i.h: Align struct ps_data.tim to unsigned long
2013-12-19 0:52 ` [PATCH] ieee80211_i.h: Align struct ps_data.tim to unsigned long Joe Perches
@ 2013-12-19 8:19 ` Johannes Berg
0 siblings, 0 replies; 5+ messages in thread
From: Johannes Berg @ 2013-12-19 8:19 UTC (permalink / raw)
To: Joe Perches; +Cc: Jiri Benc, linux-wireless, netdev
On Wed, 2013-12-18 at 16:52 -0800, Joe Perches wrote:
> Its address is used as an unsigned long *, so make sure
> that the tim u8 array is properly aligned.
>
> Signed-off-by: Joe Perches <joe@perches.com
> ---
> > Care to send a proper patch? Otherwise I'll just re-do it and pick it up
> > that way ...
>
> I had a passing thought about this when out today.
> It should really be set to __aligned(__alignof(unsigned long))
> so here's a proper patch.
Hmm, yeah, I guess that's true, though sizeof() is probably >= on
anything where Linux runs (and otherwise arrays get problematic anyway?)
Anyway, I've dropped my patch in favour of yours.
johannes
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-12-19 8:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-18 8:16 ieee80211_i.h: shouldn't struct ps_data.tim be aligned unsigned long? Joe Perches
2013-12-18 9:19 ` Johannes Berg
2013-12-18 9:23 ` Joe Perches
2013-12-19 0:52 ` [PATCH] ieee80211_i.h: Align struct ps_data.tim to unsigned long Joe Perches
2013-12-19 8:19 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).