Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1632179

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

From Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
Date 2017-04-27 17:30 +0200
Message-ID <tATwJ-2t3-13@gated-at.bofh.it> (permalink)
References <tAdnP-8d5-5@gated-at.bofh.it> <tAdnQ-8d5-41@gated-at.bofh.it> <tApS2-7Eb-19@gated-at.bofh.it> <tABJw-7at-5@gated-at.bofh.it> <tALzb-5ht-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Thu, Apr 27, 2017 at 08:53:38AM +0200, Christoph Hellwig wrote:

> > The main difficulty we
> > have now is that neither of those functions are expected to fail and we
> > need them to be able to in cases where the page doesn't map to system
> > RAM. This patch series is trying to address it for users of scatterlist.
> > I'm certainly open to other suggestions.
> 
> I think you'll need to follow the existing kmap semantics and never
> fail the iomem version either.  Otherwise you'll have a special case
> that's almost never used that has a different error path.

How about first switching as many call sites as possible to use
sg_copy_X_buffer instead of kmap?

A random audit of Logan's series suggests this is actually a fairly
common thing.

eg drivers/mmc/host/sdhci.c is only doing this:

                                        buffer = sdhci_kmap_atomic(sg, &flags);
                                        memcpy(buffer, align, size);
                                        sdhci_kunmap_atomic(buffer, &flags);

drivers/scsi/mvsas/mv_sas.c is this:

+			to = sg_map(sg_resp, 0, SG_KMAP_ATOMIC);
+			memcpy(to,
+			       slot->response + sizeof(struct mvs_err_info),
+			       sg_dma_len(sg_resp));
+			sg_unmap(sg_resp, to, 0, SG_KMAP_ATOMIC);

etc.

Lots of other places seem similar, if not sometimes a little bit more
convoluted..

Switching all the trivial cases to use copy might bring more clarity
as to what is actually required for the remaining few users? If there
are only a few then it may no longer matter if the API is not idyllic.

Jason

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Logan Gunthorpe <logang@deltatee.com> - 2017-04-25 20:30 +0200
  Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Christoph Hellwig <hch@lst.de> - 2017-04-26 09:50 +0200
    Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Logan Gunthorpe <logang@deltatee.com> - 2017-04-26 22:30 +0200
      Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Christoph Hellwig <hch@lst.de> - 2017-04-27 09:00 +0200
        Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-04-27 17:30 +0200
          Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Logan Gunthorpe <logang@deltatee.com> - 2017-04-27 18:00 +0200
        Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Logan Gunthorpe <logang@deltatee.com> - 2017-04-27 17:50 +0200
    Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Logan Gunthorpe <logang@deltatee.com> - 2017-04-27 22:20 +0200
  Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions Logan Gunthorpe <logang@deltatee.com> - 2017-04-27 01:40 +0200

csiph-web