[net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[kbuild test robot]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head:   1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2
commit: 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 [98/98] myri10ge: Add vlan rx for better GRO perf.


sparse warnings:

+ drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
+ drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
+ drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
+ drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
+ drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:16: sparse: restricted __be16 degrades to integer
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1888:16: sparse: incorrect type in argument 1 (different base types)
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1888:16:    expected unsigned int [unsigned] val
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1888:16:    got restricted __be32 [usertype] <noident>
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:2334:24: sparse: incorrect type in assignment (different address spaces)
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:2334:24:    expected unsigned char [usertype] *itable
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:2334:24:    got unsigned char [noderef] [usertype] <asn:2>*
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:2336:48: sparse: incorrect type in argument 2 (different address spaces)
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:2336:48:    expected void volatile [noderef] <asn:2>*addr
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:2336:48:    got unsigned char [usertype] *
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:2760:60: sparse: dubious: x & !y
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3839:13: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3839:13: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3839:13: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3839:13: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3839:13: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3839:13: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3841:26: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3841:26: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3841:26: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3841:26: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3841:26: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:3841:26: sparse: cast to restricted __be32
drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1434:35: sparse: context imbalance in 'myri10ge_intr' - different lock contexts for basic block

vim +1286 drivers/net/ethernet/myricom/myri10ge/myri10ge.c

1b4c44e6 Andrew Gallatin 2012-11-30  1270   * the tag ourselves to be able to achieve GRO performance that
1b4c44e6 Andrew Gallatin 2012-11-30  1271   * is comparable to LRO.
1b4c44e6 Andrew Gallatin 2012-11-30  1272   */
1b4c44e6 Andrew Gallatin 2012-11-30  1273  
1b4c44e6 Andrew Gallatin 2012-11-30  1274  static inline void
1b4c44e6 Andrew Gallatin 2012-11-30  1275  myri10ge_vlan_rx(struct net_device *dev, void *addr, struct sk_buff *skb)
1b4c44e6 Andrew Gallatin 2012-11-30  1276  {
1b4c44e6 Andrew Gallatin 2012-11-30  1277  	u8 *va;
1b4c44e6 Andrew Gallatin 2012-11-30  1278  	struct vlan_ethhdr *veh;
1b4c44e6 Andrew Gallatin 2012-11-30  1279  	struct skb_frag_struct *frag;
1b4c44e6 Andrew Gallatin 2012-11-30  1280  	__wsum vsum;
1b4c44e6 Andrew Gallatin 2012-11-30  1281  
1b4c44e6 Andrew Gallatin 2012-11-30  1282  	va = addr;
1b4c44e6 Andrew Gallatin 2012-11-30  1283  	va += MXGEFW_PAD;
1b4c44e6 Andrew Gallatin 2012-11-30  1284  	veh = (struct vlan_ethhdr *)va;
1b4c44e6 Andrew Gallatin 2012-11-30  1285  	if ((dev->features & NETIF_F_HW_VLAN_RX) == NETIF_F_HW_VLAN_RX &&
1b4c44e6 Andrew Gallatin 2012-11-30 @1286  	    veh->h_vlan_proto == ntohs(ETH_P_8021Q)) {
1b4c44e6 Andrew Gallatin 2012-11-30  1287  		/* fixup csum if needed */
1b4c44e6 Andrew Gallatin 2012-11-30  1288  		if (skb->ip_summed == CHECKSUM_COMPLETE) {
1b4c44e6 Andrew Gallatin 2012-11-30  1289  			vsum = csum_partial(va + ETH_HLEN, VLAN_HLEN, 0);
1b4c44e6 Andrew Gallatin 2012-11-30  1290  			skb->csum = csum_sub(skb->csum, vsum);
1b4c44e6 Andrew Gallatin 2012-11-30  1291  		}
1b4c44e6 Andrew Gallatin 2012-11-30  1292  		/* pop tag */
1b4c44e6 Andrew Gallatin 2012-11-30  1293  		__vlan_hwaccel_put_tag(skb, ntohs(veh->h_vlan_TCI));
1b4c44e6 Andrew Gallatin 2012-11-30  1294  		memmove(va + VLAN_HLEN, va, 2 * ETH_ALEN);

---
0-DAY kernel build testing backend         Open Source Technology Center
Fengguang Wu, Yuanhan Liu                              Intel Corporation

Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[Andrew Gallatin]

On 11/30/12 16:02, kbuild test robot wrote:
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> head:   1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2
> commit: 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 [98/98] myri10ge: Add vlan rx for better GRO perf.
> 
> 
> sparse warnings:
> 
> + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:16: sparse: restricted __be16 degrades to integer


OK, maybe a dumb question again, but how do I get sparse to produce
the 'cast to restricted' warnings?  I ran sparse before submission,
but it only showed the pre-existing, non "cast to restricted"
warnings, so I did not know I was introducing a new warning.
Do I need to use a different architecture? (I was using x86_64).

Also, the line it is warning about is this:

> 1b4c44e6 Andrew Gallatin 2012-11-30 @1286  	    veh->h_vlan_proto == ntohs(ETH_P_8021Q)) {


Which seems to be nearly identical to the usage in
if_vlan.h:__vlan_get_tag, which I was treating as canonical..
So I'm a bit confused as to how to fix it.


Thanks,

Drew

Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[Stephen Hemminger]

On Fri, 30 Nov 2012 16:51:41 -0500
Andrew Gallatin <gallatin@myri.com> wrote:

> On 11/30/12 16:02, kbuild test robot wrote:
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> > head:   1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2
> > commit: 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 [98/98] myri10ge: Add vlan rx for better GRO perf.
> > 
> > 
> > sparse warnings:
> > 
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:16: sparse: restricted __be16 degrades to integer
> 
> 
> OK, maybe a dumb question again, but how do I get sparse to produce
> the 'cast to restricted' warnings?  I ran sparse before submission,
> but it only showed the pre-existing, non "cast to restricted"
> warnings, so I did not know I was introducing a new warning.
> Do I need to use a different architecture? (I was using x86_64).

See Documentation/sparse.txt
  
  The optional make variable CF can be used to pass arguments to sparse.  The
  build system passes -Wbitwise to sparse automatically.  To perform endianness
  checks, you may define __CHECK_ENDIAN__:

        make C=2 CF="-D__CHECK_ENDIAN__"

  These checks are disabled by default as they generate a host of warnings.

Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[Fengguang Wu]

On Fri, Nov 30, 2012 at 04:51:41PM -0500, Andrew Gallatin wrote:
> On 11/30/12 16:02, kbuild test robot wrote:
> > tree:   git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
> > head:   1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2
> > commit: 1b4c44e6369dbbafd113f1e00b406f1eda5ab5b2 [98/98] myri10ge: Add vlan rx for better GRO perf.
> > 
> > 
> > sparse warnings:
> > 
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16
> > + drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:16: sparse: restricted __be16 degrades to integer
> 
> 
> OK, maybe a dumb question again, but how do I get sparse to produce
> the 'cast to restricted' warnings?
[snip]
> Also, the line it is warning about is this:
> 
> > 1b4c44e6 Andrew Gallatin 2012-11-30 @1286  	    veh->h_vlan_proto == ntohs(ETH_P_8021Q)) {
> 
> 
> Which seems to be nearly identical to the usage in
> if_vlan.h:__vlan_get_tag, which I was treating as canonical..
> So I'm a bit confused as to how to fix it.
 
Andrew, here is the explanations from Christopher Li:

On Thu, Nov 29, 2012 at 09:58:58AM -0800, Christopher Li wrote:
> On Wed, Nov 28, 2012 at 2:42 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Wed, 28 Nov 2012 17:23:47 +0800
> >>
> >> + fs/hfsplus/xattr.c:363:23: sparse: cast to restricted __be32
> 
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  361   hfs_bnode_read(fd.bnode, &record_type,
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  362                   fd.entryoffset, sizeof(record_type));
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27 @363   record_type = be32_to_cpu(record_type);
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  364   if (record_type == HFSPLUS_ATTR_INLINE_DATA) {
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  365           record_length = hfs_bnode_read_u16(fd.bnode,
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  366                           fd.entryoffset +
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  367                           offsetof(struct hfsplus_attr_inline_data,
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  368                           length));
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  369           if (record_length > HFSPLUS_MAX_INLINE_DATA_SIZE) {
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  370                   printk(KERN_ERR "hfs: invalid xattr record size\n");
> >> 54d776ef Vyacheslav Dubeyko 2012-11-27  371                   res = -EIO;
> >>
> >
> > I don't know what that warning means :(
> 
> Who does any way :-).
> 
> >
> > Chris, can you shed some light here?
> 
> I think it is likely cause by record_type get value assigned.
> What you want here is have one variable for record_type store in back
> end endian.
> Then have a different variable to store the record_type in CPU endian.
> It is bad idea to store both endian in the same variable. That is what sparse is
> complaining right now.
> 
> The detail cause of the complain is that, record_type has type __be32 __u32.
> After be32_to_cpu() it return __u32 type.
> When you assign __u32 type to a __be32_u32, sparse find out it has
> type mismatch,
> so it will do implicitly up cast. Think about if you assign char
> variable to int, the compiler
> will need to insert a cast to do the sign extension. That case is
> causing the error message
> because __be32 can't be cased.
> 
> Any way, it seems sparse is doing what it suppose to do here. The suggested
> way to fix the warning is give different variable for back end and
> CPU. That should get rid
> of the warning.
> 
> Chris

Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[Andrew Gallatin]

Thanks guys.

In this case, it found a real typo (use of ntohs() rather than htons()).
If it would not have been for this tool making me stare at it, I never
would have seen this.

I need audit the rest of the warnings in the driver..

Sorry for the noise & thanks for this service!

Drew

Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[Andrew Gallatin]

I think I have a handle on most of the pre-existing warnings.  The
device responds to BAR reads/writes with big endian data, while
I think everything expects little endian.  These will be easy
to fix. The warning I don't see a fix for is this:

drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1434:35: warning:
context imbalance in 'myri10ge_intr' - different lock contexts for basic
block

Which is apparently triggered by using __netif_tx_trylock().
Is there something I'm missing, or does sparse just not like
__netif_tx_trylock() because it divides spinlock acquisition and
release into 2 different functions?

Thanks,

Drew

Re: [net-next:master 98/98] drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1286:34: sparse: cast to restricted __be16

[Christopher Li]

On Mon, Dec 3, 2012 at 11:21 AM, Andrew Gallatin <gallatin@myri.com> wrote:
> drivers/net/ethernet/myricom/myri10ge/myri10ge.c:1434:35: warning:
> context imbalance in 'myri10ge_intr' - different lock contexts for basic
> block
>
> Which is apparently triggered by using __netif_tx_trylock().
> Is there something I'm missing, or does sparse just not like
> __netif_tx_trylock() because it divides spinlock acquisition and
> release into 2 different functions?
>

Right. Sparse currently does not have cross function analyse.
It will complain if the lock is not balanced with lock split into
different functions.

Chris