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

Groups > linux.kernel > #1205445 > unrolled thread

Re: [PATCH v2 08/25] arch: introduce memremap()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 08/25] arch: introduce memremap() "Luis R. Rodriguez" <mcgrof@suse.com> - 2015-08-11 23:40 +0200
    Re: [PATCH v2 08/25] arch: introduce memremap() Toshi Kani <toshi.kani@hp.com> - 2015-08-12 00:50 +0200
      Re: [PATCH v2 08/25] arch: introduce memremap() "Luis R. Rodriguez" <mcgrof@suse.com> - 2015-08-12 01:00 +0200
        Re: [PATCH v2 08/25] arch: introduce memremap() Dan Williams <dan.j.williams@intel.com> - 2015-08-12 01:20 +0200

#1205445 — Re: [PATCH v2 08/25] arch: introduce memremap()

On Wed, Jul 29, 2015 at 06:00:04PM -0600, Toshi Kani wrote:
> On Wed, 2015-07-29 at 23:43 +0200, Luis R. Rodriguez wrote:
> > On Wed, Jul 29, 2015 at 03:00:38PM -0600, Toshi Kani wrote:
> > > On Wed, 2015-07-29 at 11:33 -0700, Dan Williams wrote:
> > > > On Wed, Jul 29, 2015 at 11:27 AM, Luis R. Rodriguez <mcgrof@suse.com> 
> > > > wrote:
> > > > > On Wed, Jul 29, 2015 at 08:50:04AM +0200, Christoph Hellwig wrote:
> > > > > > On Mon, Jul 27, 2015 at 04:26:03PM -0700, Dan Williams wrote:
> > > > > > > Oh, because all we have at this point is ioremap_cache() which
> > > > > > > silently falls back.  It's not until the introduction of
> > > > > > > arch_memremp() where we update the arch code to break that 
> > > > > > > behavior.
> > > > > > 
> > > > > > Ok, makes sense.  Might be worth to document in the changelog.
> > > > > > 
> > > > > > > That said, I think it may be beneficial to allow a fallback if 
> > > > > > > the user cares.  So maybe memremap() can call plain ioremap() if
> > > > > > > MEMREMAP_STRICT is not set and none of the other mapping types 
> > > > > > > are satisfied.
> > > > > > 
> > > > > > Is there a real use case for it?  Fallback APIs always seem 
> > > > > > confusing and it might make more sense to do this in the caller(s) 
> > > > > > that actually need it.
> > > > > 
> > > > > It seems semantics-wise we are trying to separate these two really, 
> > > > > so I agree with this. Having a fallback would onloy make things more
> > > > > complicated for any sanitizer / checker / etc, and I don't think the 
> > > > > practical gains of having a fallback outweight the gains of having a 
> > > > > clear semantic separation on intended memory type and interactions 
> > > > > with it.
> > > > > 
> > > > 
> > > > Yup, consider it dropped.  Drivers that want fallback behavior can do
> > > > it explicitly.
> > > 
> > > I agree in general.  However, for the drivers to be able to fall back
> > > properly, they will need to know the cache type they can fall back with. 
> > > 
> > 
> > That would depend on the purpose of the region and the driver developer 
> > should in theory know best. One issue with this of course is that, as 
> > we've discovered, the semantics of on the ioremap*() variant front 
> > regarding cache types is not clearly well defined, or at least it may be 
> > only well defined implicitly on x86 only, so the driver developer can only 
> > *hope* for the best across architectures. Our ambiguity on our semantics 
> > on ioremap*() variants therefore means driver developers can resonably be 
> > puzzled by what fallbacks to use. That also means architectures 
> > maintainers should not whip driver developers for any misuse. Such 
> > considerations are why although we're now revisiting semantics for 
> > ioremap*() variants I was in hopes we could be at least somewhat
> > pedantic about memremap() semantics.
> 
> I agree.  However, there are a few exceptions like /dev/mem, which can map a
> target range without knowledge.

Still, the expectation to require support for overlapping ioremap() calls would
seem to be more of an exception than the norm, so I'd argue that requiring
callers who know they do need overlapping support to be explicit about it may
help us simplify expecations on semantics in other areas of the kernel.

> > For instance since memremap() only has 2 types right now can a respective
> > fallback be documented as an alternative to help with this ? Or can we not
> > generalize this ?  One for MEMREMAP_WB and one for MEMREMAP_WT ?
> 
> Yes, if a target range can be only mapped by memremap().  However, there is
> no restriction that a range can be mapped with a single interface alone. 
>  For example, a range can be mapped with remap_pfn_range() to user space
> with any cache type.  So, in theory, memremap() can overlap with any other
> types.

Shouldn't that be an issue or area of concern ? It seems the flakiness on
ioremap() and its wide array flexibility would spill over the any semantics
which folks would be trying to set out with memremap(). That should not stop
the evolution of memremap() though, just pointing out that perhaps we should be
a bit more restrictive over how things can criss-cross and who areas can do it.

> > > ioremap() falls back to the cache type of an existing mapping to avoid
> > > aliasing.
> > 
> > Does that assume an existing ioremap*() call was used on a bigger range?
> > Do you know if that happens to only be the case for x86 (I'd think so)
> > or if its the same for other architectures ?
> 
> In the /dev/mem example, it is remap_pfn_range().  I think other archs have
> the same issue, but I do not know if they fall back in case of overlapping
> call. 

What should happen if remap_pfn_range() was used with pgprot_writecombine()
and later memremap() is used for instance with a smaller overlappin section,
or perhaps bigger?

  Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1205475

On Tue, 2015-08-11 at 23:30 +0200, Luis R. Rodriguez wrote:
> On Wed, Jul 29, 2015 at 06:00:04PM -0600, Toshi Kani wrote:
> > On Wed, 2015-07-29 at 23:43 +0200, Luis R. Rodriguez wrote:
> > > On Wed, Jul 29, 2015 at 03:00:38PM -0600, Toshi Kani wrote:
> > > > On Wed, 2015-07-29 at 11:33 -0700, Dan Williams wrote:
  :
> > > That would depend on the purpose of the region and the driver 
> > > developer should in theory know best. One issue with this of course is 
> > > that, as we've discovered, the semantics of on the ioremap*() variant 
> > > front regarding cache types is not clearly well defined, or at least 
> > > it may be only well defined implicitly on x86 only, so the driver 
> > > developer can only *hope* for the best across architectures. Our 
> > > ambiguity on our semantics on ioremap*() variants therefore means 
> > > driver developers can resonably be puzzled by what fallbacks to use. 
> > > That also means architectures maintainers should not whip driver 
> > > developers for any misuse. Such considerations are why although we're 
> > > now revisiting semantics for ioremap*() variants I was in hopes we 
> > > could be at least somewhat pedantic about memremap() semantics.
> > 
> > I agree.  However, there are a few exceptions like /dev/mem, which can 
> > map a target range without knowledge.
> 
> Still, the expectation to require support for overlapping ioremap() calls 
> would seem to be more of an exception than the norm, so I'd argue that 
> requiring callers who know they do need overlapping support to be explicit 
> about it may help us simplify expecations on semantics in other areas of 
> the kernel.

Again, I agree.  I am simply saying that the fallback in an overlapping case
may need to remain supported for such exceptional cases, possibly with a
separate interface.

> > > For instance since memremap() only has 2 types right now can a 
> > > respective fallback be documented as an alternative to help with this 
> > > ? Or can we not generalize this ?  One for MEMREMAP_WB and one for 
> > > MEMREMAP_WT ?
> > 
> > Yes, if a target range can be only mapped by memremap().  However, there 
> > is no restriction that a range can be mapped with a single interface 
> > alone.  For example, a range can be mapped with remap_pfn_range() to 
> > user space with any cache type.  So, in theory, memremap() can overlap 
> > with any other types.
> 
> Shouldn't that be an issue or area of concern ? It seems the flakiness on
> ioremap() and its wide array flexibility would spill over the any 
> semantics which folks would be trying to set out with memremap(). That 
> should not stop the evolution of memremap() though, just pointing out that 
> perhaps we should be a bit more restrictive over how things can criss
> -cross and who areas can do it.

reserve_pfn_range() allows the caller to specify if the fallback needs to be
enabled or disabled with 'strict_prot'.  track_pfn_remap() called from
remap_pfn_range() enables it, and track_pfn_copy() disables it.  I think we
can do similar for the memremap and ioremap families as well.  The fallback
could be set disabled in the regular interfaces, and enabled in some
interface as necessary.  This also allows gradual transition, ex. disable in
memremap while ioremap remains enabled for now. 

> > > > ioremap() falls back to the cache type of an existing mapping to 
> > > > avoid aliasing.
> > > 
> > > Does that assume an existing ioremap*() call was used on a bigger 
> > > range?  Do you know if that happens to only be the case for x86 (I'd 
> > > think so) or if its the same for other architectures ?
> > 
> > In the /dev/mem example, it is remap_pfn_range().  I think other archs 
> > have the same issue, but I do not know if they fall back in case of
> > overlapping call.
> 
> What should happen if remap_pfn_range() was used with 
> pgprot_writecombine() and later memremap() is used for instance with a 
> smaller overlappin section, or perhaps bigger?

With the fallback disabled, memremap() should fail in this case. 

Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205482

On Tue, Aug 11, 2015 at 3:40 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Tue, 2015-08-11 at 23:30 +0200, Luis R. Rodriguez wrote:
>> On Wed, Jul 29, 2015 at 06:00:04PM -0600, Toshi Kani wrote:
>> > On Wed, 2015-07-29 at 23:43 +0200, Luis R. Rodriguez wrote:
>> > > On Wed, Jul 29, 2015 at 03:00:38PM -0600, Toshi Kani wrote:
>> > > > On Wed, 2015-07-29 at 11:33 -0700, Dan Williams wrote:
>   :
>> > > That would depend on the purpose of the region and the driver
>> > > developer should in theory know best. One issue with this of course is
>> > > that, as we've discovered, the semantics of on the ioremap*() variant
>> > > front regarding cache types is not clearly well defined, or at least
>> > > it may be only well defined implicitly on x86 only, so the driver
>> > > developer can only *hope* for the best across architectures. Our
>> > > ambiguity on our semantics on ioremap*() variants therefore means
>> > > driver developers can resonably be puzzled by what fallbacks to use.
>> > > That also means architectures maintainers should not whip driver
>> > > developers for any misuse. Such considerations are why although we're
>> > > now revisiting semantics for ioremap*() variants I was in hopes we
>> > > could be at least somewhat pedantic about memremap() semantics.
>> >
>> > I agree.  However, there are a few exceptions like /dev/mem, which can
>> > map a target range without knowledge.
>>
>> Still, the expectation to require support for overlapping ioremap() calls
>> would seem to be more of an exception than the norm, so I'd argue that
>> requiring callers who know they do need overlapping support to be explicit
>> about it may help us simplify expecations on semantics in other areas of
>> the kernel.
>
> Again, I agree.  I am simply saying that the fallback in an overlapping case
> may need to remain supported for such exceptional cases, possibly with a
> separate interface.

Great.

>> > > For instance since memremap() only has 2 types right now can a
>> > > respective fallback be documented as an alternative to help with this
>> > > ? Or can we not generalize this ?  One for MEMREMAP_WB and one for
>> > > MEMREMAP_WT ?
>> >
>> > Yes, if a target range can be only mapped by memremap().  However, there
>> > is no restriction that a range can be mapped with a single interface
>> > alone.  For example, a range can be mapped with remap_pfn_range() to
>> > user space with any cache type.  So, in theory, memremap() can overlap
>> > with any other types.
>>
>> Shouldn't that be an issue or area of concern ? It seems the flakiness on
>> ioremap() and its wide array flexibility would spill over the any
>> semantics which folks would be trying to set out with memremap(). That
>> should not stop the evolution of memremap() though, just pointing out that
>> perhaps we should be a bit more restrictive over how things can criss
>> -cross and who areas can do it.
>
> reserve_pfn_range() allows the caller to specify if the fallback needs to be
> enabled or disabled with 'strict_prot'.  track_pfn_remap() called from
> remap_pfn_range() enables it, and track_pfn_copy() disables it.  I think we
> can do similar for the memremap and ioremap families as well.  The fallback
> could be set disabled in the regular interfaces, and enabled in some
> interface as necessary.  This also allows gradual transition, ex. disable in
> memremap while ioremap remains enabled for now.

Sounds sexy to me.

>> > > > ioremap() falls back to the cache type of an existing mapping to
>> > > > avoid aliasing.
>> > >
>> > > Does that assume an existing ioremap*() call was used on a bigger
>> > > range?  Do you know if that happens to only be the case for x86 (I'd
>> > > think so) or if its the same for other architectures ?
>> >
>> > In the /dev/mem example, it is remap_pfn_range().  I think other archs
>> > have the same issue, but I do not know if they fall back in case of
>> > overlapping call.
>>
>> What should happen if remap_pfn_range() was used with
>> pgprot_writecombine() and later memremap() is used for instance with a
>> smaller overlappin section, or perhaps bigger?
>
> With the fallback disabled, memremap() should fail in this case.

Excellent.

 Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1205496

On Tue, Aug 11, 2015 at 3:52 PM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Tue, Aug 11, 2015 at 3:40 PM, Toshi Kani <toshi.kani@hp.com> wrote:
>> On Tue, 2015-08-11 at 23:30 +0200, Luis R. Rodriguez wrote:
>>> On Wed, Jul 29, 2015 at 06:00:04PM -0600, Toshi Kani wrote:
>>> > On Wed, 2015-07-29 at 23:43 +0200, Luis R. Rodriguez wrote:
>>> > > On Wed, Jul 29, 2015 at 03:00:38PM -0600, Toshi Kani wrote:
>>> > > > On Wed, 2015-07-29 at 11:33 -0700, Dan Williams wrote:
>>   :
>>> > > That would depend on the purpose of the region and the driver
>>> > > developer should in theory know best. One issue with this of course is
>>> > > that, as we've discovered, the semantics of on the ioremap*() variant
>>> > > front regarding cache types is not clearly well defined, or at least
>>> > > it may be only well defined implicitly on x86 only, so the driver
>>> > > developer can only *hope* for the best across architectures. Our
>>> > > ambiguity on our semantics on ioremap*() variants therefore means
>>> > > driver developers can resonably be puzzled by what fallbacks to use.
>>> > > That also means architectures maintainers should not whip driver
>>> > > developers for any misuse. Such considerations are why although we're
>>> > > now revisiting semantics for ioremap*() variants I was in hopes we
>>> > > could be at least somewhat pedantic about memremap() semantics.
>>> >
>>> > I agree.  However, there are a few exceptions like /dev/mem, which can
>>> > map a target range without knowledge.
>>>
>>> Still, the expectation to require support for overlapping ioremap() calls
>>> would seem to be more of an exception than the norm, so I'd argue that
>>> requiring callers who know they do need overlapping support to be explicit
>>> about it may help us simplify expecations on semantics in other areas of
>>> the kernel.
>>
>> Again, I agree.  I am simply saying that the fallback in an overlapping case
>> may need to remain supported for such exceptional cases, possibly with a
>> separate interface.
>
> Great.
>
>>> > > For instance since memremap() only has 2 types right now can a
>>> > > respective fallback be documented as an alternative to help with this
>>> > > ? Or can we not generalize this ?  One for MEMREMAP_WB and one for
>>> > > MEMREMAP_WT ?
>>> >
>>> > Yes, if a target range can be only mapped by memremap().  However, there
>>> > is no restriction that a range can be mapped with a single interface
>>> > alone.  For example, a range can be mapped with remap_pfn_range() to
>>> > user space with any cache type.  So, in theory, memremap() can overlap
>>> > with any other types.
>>>
>>> Shouldn't that be an issue or area of concern ? It seems the flakiness on
>>> ioremap() and its wide array flexibility would spill over the any
>>> semantics which folks would be trying to set out with memremap(). That
>>> should not stop the evolution of memremap() though, just pointing out that
>>> perhaps we should be a bit more restrictive over how things can criss
>>> -cross and who areas can do it.
>>
>> reserve_pfn_range() allows the caller to specify if the fallback needs to be
>> enabled or disabled with 'strict_prot'.  track_pfn_remap() called from
>> remap_pfn_range() enables it, and track_pfn_copy() disables it.  I think we
>> can do similar for the memremap and ioremap families as well.  The fallback
>> could be set disabled in the regular interfaces, and enabled in some
>> interface as necessary.  This also allows gradual transition, ex. disable in
>> memremap while ioremap remains enabled for now.
>
> Sounds sexy to me.

Cool, sounds like something we can tackle in 4.4 along with the
ioremap_cache removal cleanups.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web