Linux NFS development
 help / color / mirror / Atom feed
From: Tom Tucker <tom@opengridcomputing.com>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-nfs <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH] svcrdma: Fix compile warning on 32b platforms
Date: Tue, 27 May 2008 09:08:38 -0500	[thread overview]
Message-ID: <1211897318.4114.53.camel@trinity.ogc.int> (raw)
In-Reply-To: <20080525190112.GC8094@fieldses.org>


On Sun, 2008-05-25 at 15:01 -0400, J. Bruce Fields wrote:
> On Fri, May 23, 2008 at 10:39:53AM -0500, Tom Tucker wrote:
> > The ib_sge is used to store both dma_addr_t and virtual addresses.
> > This causes a warning on 32b platforms when casting ptrs to the
> > u64 addr field of the ib_sge. There is work underway to reduce the
> > memory footprint of the WR context in 2.6.27 the time frame that will
> > remove the overloading of the ib_sge.addr field.
> 
> Naively, this and "svcrdma: Refactor RDMA_WRITE dma mapping logic" still
> look like a step in the wrong direction; the old code seemed to have a
> much more clear separation between kernel addresses (void *'s) and
> on-the-wire addresses (stored in u64's).  Stuff like:
> 
> > +					  (void*)(unsigned long)
> > +					  sge[i].addr, sge[i].length,
> 
> makes me nervous.
> 

The svc_rdma_op_ctxt is being used for both the context of the NFS_ RPC
request (that can map up to 2M) and the RDMA WR (which is bounded by the
SGE limit of the device). 

What changed is when the dma_map is performed. Originally it was
performed when the NFS_ request was mapped to a local SGL. The problem
with this is that the temporary context used to keep this mapping is
freed (and unmapped) before the WR completes and therefore the memory
could be unmapped too early. Since the unmap is a no-op on x86 -- this
never caused a problem, however, for IA64 and others -- it is a problem.

What needs to be done is to replace the svc_rdma_op_ctxt with a suitable
temporary mapping structure with the appropriate type. I will have this
patch soon. 

I knew it was broken and was concerned that it would be found in the
field, but since we don't hear the screams - it sounds like we should
just ditch these interim dma changes and then we won't have code
upstream that has this interim typing issue.

If someone does find this problem, I can point them at my tree to give
it whirl.

Sound ok?

Tom
> --b.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


      parent reply	other threads:[~2008-05-27 14:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-23 15:39 [PATCH] svcrdma: Fix compile warning on 32b platforms Tom Tucker
     [not found] ` <1211557193.4114.21.camel-SMNkleLxa3ZimH42XvhXlA@public.gmane.org>
2008-05-25 19:01   ` J. Bruce Fields
2008-05-25 20:21     ` J. Bruce Fields
     [not found]       ` <C45F878E.19918%tom@opengridcomputing.com>
2008-05-28 19:07         ` J. Bruce Fields
2008-05-28 19:49           ` Tom Tucker
2008-05-27 14:08     ` Tom Tucker [this message]

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=1211897318.4114.53.camel@trinity.ogc.int \
    --to=tom@opengridcomputing.com \
    --cc=akpm@linux-foundation.org \
    --cc=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    /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