From: "Glenn Streiff" <gstreiff@NetEffect.com>
To: "Roland Dreier" <rdreier@cisco.com>,
"Christoph Hellwig" <hch@infradead.org>
Cc: <linux-kernel@vger.kernel.org>, <general@lists.openfabrics.org>
Subject: RE: InfiniBand/RDMA merge plans for 2.6.25
Date: Thu, 24 Jan 2008 07:54:36 -0600 [thread overview]
Message-ID: <5E701717F2B2ED4EA60F87C8AA57B7CC0794FEB4@venom2> (raw)
In-Reply-To: <adave5kp1g0.fsf@cisco.com>
> From: Roland Dreier [mailto:rdreier@cisco.com]
> To: Christoph Hellwig; Glenn Streiff
>
> Rather than arguing over whether we have to have sparse clean code, I
> decided to annotate the code myself. Here's a patch that fixes most
> of the sparse warnings in the nes driver. There's still some stuff
> that actually looks buggy, like the way hte_index stuff is handled.
> You initialize hte_index_mask as:
>
> hte_index_mask = ((u32)1 << ((u32temp & 0x001f)+1))-1;
> nesadapter->hte_index_mask = hte_index_mask;
>
> but then compute hte_index stuff with:
>
> nesqp->hte_index = cpu_to_be32(
> crc32c(~0, (void *)&nes_quad,
> sizeof(nes_quad)) ^ 0xffffffff);
>
> and then do:
>
> nesqp->hte_index &= nesadapter->hte_index_mask;
>
> which seems odd to say the least (hte_index is big-endian,
> hte_index_mask is cpu-endian).
>
> And also, there's code with the loc_addr/rem_addr etc that seem very
> confused. For example
>
> cm_info->loc_addr = htonl(cm_info->loc_addr);
> cm_info->rem_addr = htonl(cm_info->rem_addr);
> cm_info->loc_port = htons(cm_info->loc_port);
> cm_info->rem_port = htons(cm_info->rem_port);
>
> which is obviously impossible to annotate correctly, and I couldn't
> keep track of the endianness stuff elsewhere.
Thanks for the additional review and patch. I take your point.
The part is little endian and the driver is functional for little and big endian
platforms. There may have been some expedience with the declarations there.
I think it can be improved. Let me take it up with the person who wrote that code.
Also, I want everyone to understand that my skill set is weighted more
towards build/install/config. And I guess I'll be patch wrangling as well.
So I'll rely on input from my developers for issues that drill down or
I'll have them post directly. I respect the work you guys do.
For now, let me get some qa cycles with your patch across x86_64 and power
(and probably a couple others).
Regards,
Glenn
>
> Anyway this is what I have in case the promised cleanups don't turn up
> in time...
>
> Signed-off-by: Roland Dreier <rolandd@cisco.com>
>
>
next prev parent reply other threads:[~2008-01-24 13:54 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-18 0:11 Roland Dreier
2008-01-18 17:10 ` [ofa-general] " Hal Rosenstock
2008-01-18 17:42 ` Sean Hefty
2008-01-18 18:25 ` Hal Rosenstock
2008-01-21 10:01 ` Christoph Hellwig
2008-01-22 21:56 ` Roland Dreier
2008-01-22 23:49 ` [ofa-general] " Glenn Streiff
2008-01-23 5:50 ` Christoph Hellwig
2008-01-23 21:23 ` Roland Dreier
2008-01-23 23:27 ` Roland Dreier
2008-01-24 13:54 ` Glenn Streiff [this message]
2008-01-24 7:13 ` [ofa-general] " Bart Van Assche
2008-01-24 16:09 ` Roland Dreier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5E701717F2B2ED4EA60F87C8AA57B7CC0794FEB4@venom2 \
--to=gstreiff@neteffect.com \
--cc=general@lists.openfabrics.org \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rdreier@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®