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

Groups > linux.kernel > #1631905

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

Show all headers | View raw

On Wed, Apr 26, 2017 at 12:11:33PM -0600, Logan Gunthorpe wrote:
> Ok, well for starters I think you are mistaken about kmap being able to
> fail. I'm having a hard time finding many users of that function that
> bother to check for an error when calling it.

A quick audit of the arch code shows you're right - kmap can't fail
anywhere anymore.

> 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.

> There are a fair number of cases in the kernel that do something like:
> 
> if (something)
>     x = kmap(page);
> else
>     x = kmap_atomic(page);
> ...
> if (something)
>     kunmap(page)
> else
>     kunmap_atomic(x)
> 
> Which just seems cumbersome to me.

Passing a different flag based on something isn't really much better.

> In any case, if you can accept an sg_kmap and sg_kmap_atomic api just
> say so and I'll make the change. But I'll still need a flags variable
> for SG_MAP_MUST_NOT_FAIL to support legacy cases that have no fail path
> and both of those functions will need to be pretty nearly replicas of
> each other.

Again, wrong way.  Suddenly making things fail for your special case
that normally don't fail is a receipe for bugs.

Back to linux.kernel | Previous | Next — Previous 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