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


Groups > linux.kernel > #1631218

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

From Christoph Hellwig <hch@lst.de>
Newsgroups linux.kernel
Subject Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions
Date 2017-04-26 09:50 +0200
Message-ID <tApS2-7Eb-19@gated-at.bofh.it> (permalink)
References <tAdnP-8d5-5@gated-at.bofh.it> <tAdnQ-8d5-41@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, Apr 25, 2017 at 12:20:48PM -0600, Logan Gunthorpe wrote:
> This patch introduces functions which kmap the pages inside an sgl.
> These functions replace a common pattern of kmap(sg_page(sg)) that is
> used in more than 50 places within the kernel.
> 
> The motivation for this work is to eventually safely support sgls that
> contain io memory. In order for that to work, any access to the contents
> of an iomem SGL will need to be done with iomemcpy or hit some warning.
> (The exact details of how this will work have yet to be worked out.)

I think we'll at least need a draft of those to make sense of these
patches.  Otherwise they just look very clumsy.

> + *   Use this function to map a page in the scatterlist at the specified
> + *   offset. sg->offset is already added for you. Note: the semantics of
> + *   this function are that it may fail. Thus, its output should be checked
> + *   with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
> + *   in the mapped page is returned.
> + *
> + *   Flags can be any of:
> + *	* SG_KMAP		- Use kmap to create the mapping
> + *	* SG_KMAP_ATOMIC	- Use kmap_atomic to map the page atommically.
> + *				  Thus, the rules of that function apply: the
> + *				  cpu may not sleep until it is unmaped.
> + *	* SG_MAP_MUST_NOT_FAIL	- Indicate that sg_map must not fail.
> + *				  If it does, it will issue a BUG_ON instead.
> + *				  This is intended for legacy code only, it
> + *				  is not to be used in new code.

I'm sorry but this API is just a trainwreck.  Right now we have the
nice little kmap_atomic API, which never fails and has a very nice
calling convention where we just pass back the return address, but does
not support sleeping inside the critical section.

And kmap, whіch may fail and requires the original page to be passed
back.  Anything that mixes these two concepts up is simply a non-starter.

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