2009-05-12 00:13:19

by Steve Wise

[permalink] [raw]
Subject: Re: [PATCH 2.6.30] xprtrdma: The frmr iova_start values are truncated by the nfs rdma client.

Trond Myklebust wrote:
> On Mon, 2009-05-11 at 17:25 -0500, Steve Wise wrote:
>
>> Hey Trond,
>>
>> Will this bug fix make 2.6.30?
>>
>> Thanks,
>>
>> Steve.
>>
>
> Not in the form it is in now. As I've said earlier, I'm not happy about
> the sunrpc layer having to circumvent ordinary type checking on
> non-sunrpc structures.
>
> Cheers
> Trond
How is it circumventing? It's currently incorrectly casting a pointer
into a u64. That seems just broken to me. Also, its really the sunrpc
rdma transport layer. It deals specifically with rdma. It _should_
know about rdma interfaces and types.

But I'll whip up get/put type accessor methods for this field if that's
what you require. In the meantime, customers will just crash I guess.


Steve.





2009-05-12 00:24:28

by Tom Talpey

[permalink] [raw]
Subject: Re: [PATCH 2.6.30] xprtrdma: The frmr iova_start values are truncated by the nfs rdma client.

At 08:13 PM 5/11/2009, Steve Wise wrote:
>Trond Myklebust wrote:
>> On Mon, 2009-05-11 at 17:25 -0500, Steve Wise wrote:
>>
>>> Hey Trond,
>>>
>>> Will this bug fix make 2.6.30?
>>>
>>> Thanks,
>>>
>>> Steve.
>>>
>>
>> Not in the form it is in now. As I've said earlier, I'm not happy about
>> the sunrpc layer having to circumvent ordinary type checking on
>> non-sunrpc structures.
>>
>> Cheers
>> Trond
>How is it circumventing? It's currently incorrectly casting a pointer
>into a u64. That seems just broken to me.

The cast is definitely broken, and since your patch removes it I agree
with the change, for the record. Especially since with the change, the
code doesn't work on 32-bit CPU / 64-bit IOMMU platforms.

> Also, its really the sunrpc
>rdma transport layer. It deals specifically with rdma. It _should_
>know about rdma interfaces and types.

IOW, this is an issue for the OFA API to address? I agree with Trond that
there are some hardware-specific warts in there. It might be a good idea
for us to go through both the client and server code overall - you up for
doing that? I'm in, if so.

Tom.


>
>But I'll whip up get/put type accessor methods for this field if that's
>what you require. In the meantime, customers will just crash I guess.
>
>
>Steve.
>
>
>
>


2009-05-12 00:44:30

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH 2.6.30] xprtrdma: The frmr iova_start values are truncated by the nfs rdma client.

On Mon, 2009-05-11 at 19:13 -0500, Steve Wise wrote:
> Trond Myklebust wrote:
> > On Mon, 2009-05-11 at 17:25 -0500, Steve Wise wrote:
> >
> >> Hey Trond,
> >>
> >> Will this bug fix make 2.6.30?
> >>
> >> Thanks,
> >>
> >> Steve.
> >>
> >
> > Not in the form it is in now. As I've said earlier, I'm not happy about
> > the sunrpc layer having to circumvent ordinary type checking on
> > non-sunrpc structures.
> >
> > Cheers
> > Trond
> How is it circumventing? It's currently incorrectly casting a pointer
> into a u64. That seems just broken to me. Also, its really the sunrpc
> rdma transport layer. It deals specifically with rdma. It _should_
> know about rdma interfaces and types.

The fact is that I'm simply not interested enough in rdma to tolerate
hacks. If it isn't done cleanly, in a manner that I can maintain, then
the whole transport layer comes out...

Trond