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.
