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

Groups > linux.kernel > #1291685 > unrolled thread

[PATCH 01/11] resource: Add System RAM resource type

Back to article view | Back to linux.kernel

Contents

  [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:40 +0100
    [PATCH 08/11] memremap: Change region_intersects() to use System RAM type Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:40 +0100
    [PATCH 06/11] kexec: Set IORESOURCE_SYSTEM_RAM to System RAM Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:40 +0100
    [PATCH 10/11] arm/samsung: Change s3c_pm_run_res() to use System RAM type Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:40 +0100
      Re: [PATCH 10/11] arm/samsung: Change s3c_pm_run_res() to use System  RAM type Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2015-12-15 01:30 +0100
    [PATCH 07/11] memory-hotplug: Set IORESOURCE_SYSTEM_RAM to System RAM Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:40 +0100
    [PATCH 04/11] arch: Set IORESOURCE_SYSTEM_RAM to System RAM Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:40 +0100
    [PATCH 09/11] resource: Change walk_system_ram to use System RAM type Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:40 +0100
    [PATCH 05/11] xen: Set IORESOURCE_SYSTEM_RAM to System RAM Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:50 +0100
    [PATCH 03/11] x86/e820: Set IORESOURCE_SYSTEM_RAM to System RAM Toshi Kani <toshi.kani@hpe.com> - 2015-12-15 00:50 +0100
    Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-16 13:30 +0100
      Re: [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-16 16:50 +0100
        Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-16 16:50 +0100
          Re: [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-16 17:40 +0100
            Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-16 18:50 +0100
              Re: [PATCH 01/11] resource: Add System RAM resource type Dan Williams <dan.j.williams@intel.com> - 2015-12-16 19:00 +0100
                Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-16 19:20 +0100
                  Re: [PATCH 01/11] resource: Add System RAM resource type Dan Williams <dan.j.williams@intel.com> - 2015-12-16 20:00 +0100
                    Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-16 20:20 +0100
                  Re: [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-16 23:00 +0100
                    Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-22 12:40 +0100
                      Re: [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-22 21:10 +0100
                        Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-23 15:30 +0100
                          Re: [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-24 03:30 +0100
                            Re: [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-24 18:10 +0100
                              Re: [PATCH 01/11] resource: Add System RAM resource type Borislav Petkov <bp@alien8.de> - 2015-12-24 21:00 +0100
                                Re: [PATCH 01/11] resource: Add System RAM resource type Toshi Kani <toshi.kani@hpe.com> - 2015-12-24 22:40 +0100

Page 2 of 2 — ← Prev page 1 [2]

#1296732

On Wed, Dec 16, 2015 at 02:52:38PM -0700, Toshi Kani wrote:
> This scheme may have a problem, though.  For instance, when someone writes
> a loadable module that searches for "foo", but the "foo" entry may be
> initialized in a distro kernel/driver that cannot be modified.  Since this
> search is only necessary to obtain a range initialized by other module,
> this scenario is likely to happen.  We no longer have ability to search for
> a new entry unless we modify the code that initializes the entry first.

Since when do we pay attention to out-of-tree modules which cannot be
changed?

Regardless, we don't necessarily need to change the callers - we could
add new ones of the form walk_iomem_resource_by_type() or whatever its
name is going to be which uses the ->type attribute of the resource and
phase out the old ones slowly. New code will call the better interfaces,
we should probably even add a checkpatch rule to check for that.

> Even if we avoid strcmp() with @name in the kernel, user applications will
> continue to use @name since that is the only type available in /proc/iomem.
>  For instance, kexec has its own search function with a string name.

See above.

> When a new commonly-used search name comes up, we can define it as a new
> extended I/O resource type similar to IORESOURCE_SYSTEM_RAM.  For the
> current remaining cases, i.e. crash, kexec, and einj, they have no impact
> to performance.  Leaving these special cases aside will keep the ability to
> search for any entry without changing the kernel, and save some memory
> space from adding the new 'type'.

Again, we can leave the old interfaces at peace but going forward, we
should make the searching for resources saner and stop using silly
strings.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
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]

#1297041

On Tue, 2015-12-22 at 12:34 +0100, Borislav Petkov wrote:
> On Wed, Dec 16, 2015 at 02:52:38PM -0700, Toshi Kani wrote:
> > This scheme may have a problem, though.  For instance, when someone 
> > writes a loadable module that searches for "foo", but the "foo" entry 
> > may be initialized in a distro kernel/driver that cannot be modified. 
> >  Since this search is only necessary to obtain a range initialized by 
> > other module, this scenario is likely to happen.  We no longer have 
> > ability to search for a new entry unless we modify the code that
> > initializes the entry first.
> 
> Since when do we pay attention to out-of-tree modules which cannot be
> changed?

The above example referred the case with distros, not with the upstream. 
 That is, one writes a new loadable module and makes it available in the
upstream.  Then s/he makes it work on a distro used by the customers, but
may or may not be able to change the distro kernel/drivers used by the
customers.

> Regardless, we don't necessarily need to change the callers - we could
> add new ones of the form walk_iomem_resource_by_type() or whatever its
> name is going to be which uses the ->type attribute of the resource and
> phase out the old ones slowly. New code will call the better interfaces,
> we should probably even add a checkpatch rule to check for that.

I agree that we can add new interfaces with the type check.  This 'type'
may need some clarification since it is an assigned type, which is
different from I/O resource type.  That is, "System RAM" is an I/O resource
type (i.e. IORESOURCE_SYSTEM_RAM), but "Crash kernel" is an assigned type
to a particular range of System RAM.  A range may be associated with
multiple names, so as multiple assigned types.  For lack of a better idea,
I may call it 'assign_type'.  I am open for a better name.

> > Even if we avoid strcmp() with @name in the kernel, user applications
> > will continue to use @name since that is the only type available in
> > /proc/iomem.  For instance, kexec has its own search function with a
> > string name.
> 
> See above.
> 
> > When a new commonly-used search name comes up, we can define it as a 
> > new extended I/O resource type similar to IORESOURCE_SYSTEM_RAM.  For 
> > the current remaining cases, i.e. crash, kexec, and einj, they have no
> > impact to performance.  Leaving these special cases aside will keep the
> > ability to search for any entry without changing the kernel, and save 
> > some memory space from adding the new 'type'.
> 
> Again, we can leave the old interfaces at peace but going forward, we
> should make the searching for resources saner and stop using silly
> strings.

OK, I will try to convert the existing callers with the new interfaces.

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]

#1297494

On Tue, Dec 22, 2015 at 01:04:32PM -0700, Toshi Kani wrote:
> The above example referred the case with distros, not with the upstream. 
>  That is, one writes a new loadable module and makes it available in the
> upstream.  Then s/he makes it work on a distro used by the customers, but
> may or may not be able to change the distro kernel/drivers used by the
> customers.

Huh?

Still sounds bogus to me. Distro kernels get stuff backported to them
all the time to accomodate new features, hw support.

Your new interfaces will be used only in new code so if distros want
it, they can either backport the new kernel interfaces or use an older
version with the strings.

> I agree that we can add new interfaces with the type check.  This 'type'
> may need some clarification since it is an assigned type, which is
> different from I/O resource type.  That is, "System RAM" is an I/O resource
> type (i.e. IORESOURCE_SYSTEM_RAM), but "Crash kernel" is an assigned type
> to a particular range of System RAM.  A range may be associated with
> multiple names, so as multiple assigned types.  For lack of a better idea,
> I may call it 'assign_type'.  I am open for a better name.

Or assigned_type or named_type or so...

I think we should avoid calling it "type" completely in order to avoid
confusion with the IORESOURCE_* types and call it "desc" or so to mean
description, sort, etc, because the name is also a description of the
resource to a certain degree...

> OK, I will try to convert the existing callers with the new interfaces.

Either that or add the new interfaces, use them in your use case, add
big fat comments explaining that people should use those from now on
when searching by name and add a check to checkpatch to catch future
mis-uses...

Thanks!

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
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]

#1297726

On Wed, 2015-12-23 at 15:23 +0100, Borislav Petkov wrote:
> On Tue, Dec 22, 2015 at 01:04:32PM -0700, Toshi Kani wrote:
 :
> > I agree that we can add new interfaces with the type check.  This
> > 'type'
> > may need some clarification since it is an assigned type, which is
> > different from I/O resource type.  That is, "System RAM" is an I/O 
> > resource type (i.e. IORESOURCE_SYSTEM_RAM), but "Crash kernel" is an 
> > assigned type to a particular range of System RAM.  A range may be 
> > associated with multiple names, so as multiple assigned types.  For 
> > lack of a better idea, I may call it 'assign_type'.  I am open for a
> > better name.
> 
> Or assigned_type or named_type or so...
> 
> I think we should avoid calling it "type" completely in order to avoid
> confusion with the IORESOURCE_* types and call it "desc" or so to mean
> description, sort, etc, because the name is also a description of the
> resource to a certain degree...

Agreed. I will use 'desc'.

> > OK, I will try to convert the existing callers with the new interfaces.
> 
> Either that or add the new interfaces, use them in your use case, add
> big fat comments explaining that people should use those from now on
> when searching by name and add a check to checkpatch to catch future
> mis-uses...

Sounds good.  I will look into it.

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]

#1298011

On Wed, 2015-12-23 at 19:23 -0700, Toshi Kani wrote:
> On Wed, 2015-12-23 at 15:23 +0100, Borislav Petkov wrote:
> > On Tue, Dec 22, 2015 at 01:04:32PM -0700, Toshi Kani wrote:
>  :
> > > I agree that we can add new interfaces with the type check.  This
> > > 'type'
> > > may need some clarification since it is an assigned type, which is
> > > different from I/O resource type.  That is, "System RAM" is an I/O 
> > > resource type (i.e. IORESOURCE_SYSTEM_RAM), but "Crash kernel" is an 
> > > assigned type to a particular range of System RAM.  A range may be 
> > > associated with multiple names, so as multiple assigned types.  For 
> > > lack of a better idea, I may call it 'assign_type'.  I am open for a
> > > better name.
> > 
> > Or assigned_type or named_type or so...
> > 
> > I think we should avoid calling it "type" completely in order to avoid
> > confusion with the IORESOURCE_* types and call it "desc" or so to mean
> > description, sort, etc, because the name is also a description of the
> > resource to a certain degree...
> 
> Agreed. I will use 'desc'.
> 
> > > OK, I will try to convert the existing callers with the new
> > > interfaces.
> > 
> > Either that or add the new interfaces, use them in your use case, add
> > big fat comments explaining that people should use those from now on
> > when searching by name and add a check to checkpatch to catch future
> > mis-uses...
> 
> Sounds good.  I will look into it.

As for checkpatch, I noticed that commit 9c0ece069b3 removed "feature
-removal.txt" file, and checkpatch removed this check in commit
78e3f1f01d2.  checkpatch does not have such check since then.  So, I am
inclined not to add this check back to checkpatch.

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]

#1298037

On Thu, Dec 24, 2015 at 10:08:57AM -0700, Toshi Kani wrote:
> As for checkpatch, I noticed that commit 9c0ece069b3 removed "feature
> -removal.txt" file, and checkpatch removed this check in commit
> 78e3f1f01d2.  checkpatch does not have such check since then.  So, I am
> inclined not to add this check back to checkpatch.

I didn't mean that.

Rather, something along the lines of, for example,
the DEFINE_PCI_DEVICE_TABLE matching but match those
resource matching functions using the strings, i.e.,
"(walk_iomem_res|find_next_iomem_res|region_intersects)" or so and
warn when new code uses them and that it should rather use the new
desc-matching variants.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
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]

#1298053

On Thu, 2015-12-24 at 20:58 +0100, Borislav Petkov wrote:
> On Thu, Dec 24, 2015 at 10:08:57AM -0700, Toshi Kani wrote:
> > As for checkpatch, I noticed that commit 9c0ece069b3 removed "feature
> > -removal.txt" file, and checkpatch removed this check in commit
> > 78e3f1f01d2.  checkpatch does not have such check since then.  So, I am
> > inclined not to add this check back to checkpatch.
> 
> I didn't mean that.
> 
> Rather, something along the lines of, for example,
> the DEFINE_PCI_DEVICE_TABLE matching but match those
> resource matching functions using the strings, i.e.,
> "(walk_iomem_res|find_next_iomem_res|region_intersects)" or so and
> warn when new code uses them and that it should rather use the new
> desc-matching variants.

OK, I will add a check to walk_iomem_res().  I will remove @name from
region_intersects(), and find_next_iomem_res() is an internal function.

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] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web