Message ID | 20200319131221.14044-4-david@redhat.com (mailing list archive) |
---|---|
State | Not Applicable |
Headers | show |
Series | [v3,1/8] drivers/base/memory: rename MMOP_ONLINE_KEEP to MMOP_ONLINE | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (8a445cbcb9f5090cb07ec6cbb89a8a1fc99a0ff7) |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 62 lines checked |
snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
> Let's use a simple array which we can reuse soon. While at it, move the > string->mmop conversion out of the device hotplug lock. > > Reviewed-by: Wei Yang <richard.weiyang@gmail.com> > Acked-by: Michal Hocko <mhocko@suse.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Baoquan He <bhe@redhat.com> > Cc: Wei Yang <richard.weiyang@gmail.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > drivers/base/memory.c | 38 +++++++++++++++++++++++--------------- > 1 file changed, 23 insertions(+), 15 deletions(-) > > diff --git a/drivers/base/memory.c b/drivers/base/memory.c > index e7e77cafef80..8a7f29c0bf97 100644 > --- a/drivers/base/memory.c > +++ b/drivers/base/memory.c > @@ -28,6 +28,24 @@ > > #define MEMORY_CLASS_NAME "memory" > > +static const char *const online_type_to_str[] = { > + [MMOP_OFFLINE] = "offline", > + [MMOP_ONLINE] = "online", > + [MMOP_ONLINE_KERNEL] = "online_kernel", > + [MMOP_ONLINE_MOVABLE] = "online_movable", > +}; > + > +static int memhp_online_type_from_str(const char *str) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) { > + if (sysfs_streq(str, online_type_to_str[i])) > + return i; > + } > + return -EINVAL; > +} > + > #define to_memory_block(dev) container_of(dev, struct memory_block, dev) > > static int sections_per_block; > @@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev) > static ssize_t state_store(struct device *dev, struct device_attribute *attr, > const char *buf, size_t count) > { > + const int online_type = memhp_online_type_from_str(buf); > struct memory_block *mem = to_memory_block(dev); > - int ret, online_type; > + int ret; > + > + if (online_type < 0) > + return -EINVAL; > > ret = lock_device_hotplug_sysfs(); > if (ret) > return ret; > > - if (sysfs_streq(buf, "online_kernel")) > - online_type = MMOP_ONLINE_KERNEL; > - else if (sysfs_streq(buf, "online_movable")) > - online_type = MMOP_ONLINE_MOVABLE; > - else if (sysfs_streq(buf, "online")) > - online_type = MMOP_ONLINE; > - else if (sysfs_streq(buf, "offline")) > - online_type = MMOP_OFFLINE; > - else { > - ret = -EINVAL; > - goto err; > - } > - > switch (online_type) { > case MMOP_ONLINE_KERNEL: > case MMOP_ONLINE_MOVABLE: > @@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr, > ret = -EINVAL; /* should never happen */ > } > > -err: > unlock_device_hotplug(); > > if (ret < 0) > -- Nice cleanup patch. Acked-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> > 2.24.1 > >
On 03/19/20 at 02:12pm, David Hildenbrand wrote: > Let's use a simple array which we can reuse soon. While at it, move the > string->mmop conversion out of the device hotplug lock. > > Reviewed-by: Wei Yang <richard.weiyang@gmail.com> > Acked-by: Michal Hocko <mhocko@suse.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Michal Hocko <mhocko@kernel.org> > Cc: Oscar Salvador <osalvador@suse.de> > Cc: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Baoquan He <bhe@redhat.com> > Cc: Wei Yang <richard.weiyang@gmail.com> > Signed-off-by: David Hildenbrand <david@redhat.com> > --- > drivers/base/memory.c | 38 +++++++++++++++++++++++--------------- > 1 file changed, 23 insertions(+), 15 deletions(-) > > diff --git a/drivers/base/memory.c b/drivers/base/memory.c > index e7e77cafef80..8a7f29c0bf97 100644 > --- a/drivers/base/memory.c > +++ b/drivers/base/memory.c > @@ -28,6 +28,24 @@ > > #define MEMORY_CLASS_NAME "memory" > > +static const char *const online_type_to_str[] = { > + [MMOP_OFFLINE] = "offline", > + [MMOP_ONLINE] = "online", > + [MMOP_ONLINE_KERNEL] = "online_kernel", > + [MMOP_ONLINE_MOVABLE] = "online_movable", > +}; > + > +static int memhp_online_type_from_str(const char *str) > +{ > + int i; I would change it as: for (int i = 0; i < ARRAY_SIZE(online_type_to_str); i++) { > + > + for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) { > + if (sysfs_streq(str, online_type_to_str[i])) > + return i; > + } > + return -EINVAL; > +} > + > #define to_memory_block(dev) container_of(dev, struct memory_block, dev) > > static int sections_per_block; > @@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev) > static ssize_t state_store(struct device *dev, struct device_attribute *attr, > const char *buf, size_t count) > { > + const int online_type = memhp_online_type_from_str(buf); > struct memory_block *mem = to_memory_block(dev); > - int ret, online_type; > + int ret; > + > + if (online_type < 0) > + return -EINVAL; > > ret = lock_device_hotplug_sysfs(); > if (ret) > return ret; > > - if (sysfs_streq(buf, "online_kernel")) > - online_type = MMOP_ONLINE_KERNEL; > - else if (sysfs_streq(buf, "online_movable")) > - online_type = MMOP_ONLINE_MOVABLE; > - else if (sysfs_streq(buf, "online")) > - online_type = MMOP_ONLINE; > - else if (sysfs_streq(buf, "offline")) > - online_type = MMOP_OFFLINE; > - else { > - ret = -EINVAL; > - goto err; > - } > - > switch (online_type) { > case MMOP_ONLINE_KERNEL: > case MMOP_ONLINE_MOVABLE: > @@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr, > ret = -EINVAL; /* should never happen */ > } > > -err: > unlock_device_hotplug(); > > if (ret < 0) > -- > 2.24.1 >
On 20.03.20 08:36, Baoquan He wrote: > On 03/19/20 at 02:12pm, David Hildenbrand wrote: >> Let's use a simple array which we can reuse soon. While at it, move the >> string->mmop conversion out of the device hotplug lock. >> >> Reviewed-by: Wei Yang <richard.weiyang@gmail.com> >> Acked-by: Michal Hocko <mhocko@suse.com> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> Cc: Andrew Morton <akpm@linux-foundation.org> >> Cc: Michal Hocko <mhocko@kernel.org> >> Cc: Oscar Salvador <osalvador@suse.de> >> Cc: "Rafael J. Wysocki" <rafael@kernel.org> >> Cc: Baoquan He <bhe@redhat.com> >> Cc: Wei Yang <richard.weiyang@gmail.com> >> Signed-off-by: David Hildenbrand <david@redhat.com> >> --- >> drivers/base/memory.c | 38 +++++++++++++++++++++++--------------- >> 1 file changed, 23 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/base/memory.c b/drivers/base/memory.c >> index e7e77cafef80..8a7f29c0bf97 100644 >> --- a/drivers/base/memory.c >> +++ b/drivers/base/memory.c >> @@ -28,6 +28,24 @@ >> >> #define MEMORY_CLASS_NAME "memory" >> >> +static const char *const online_type_to_str[] = { >> + [MMOP_OFFLINE] = "offline", >> + [MMOP_ONLINE] = "online", >> + [MMOP_ONLINE_KERNEL] = "online_kernel", >> + [MMOP_ONLINE_MOVABLE] = "online_movable", >> +}; >> + >> +static int memhp_online_type_from_str(const char *str) >> +{ >> + int i; > > I would change it as: > > for (int i = 0; i < ARRAY_SIZE(online_type_to_str); i++) { > That's not allowed by the C90 standard (and -std=gnu89). $ gcc main.c -std=gnu89 main.c: In function 'main': main.c:3:2: error: 'for' loop initial declarations are only allowed in C99 or C11 mode 3 | for (int i = 0; i < 8; i++) { | ^~~ One of the reasons why git grep "for (int " will result in very little hits (IOW, only 5 in driver code only).
On 03/20/20 at 10:50am, David Hildenbrand wrote: > On 20.03.20 08:36, Baoquan He wrote: > > On 03/19/20 at 02:12pm, David Hildenbrand wrote: > >> Let's use a simple array which we can reuse soon. While at it, move the > >> string->mmop conversion out of the device hotplug lock. > >> > >> Reviewed-by: Wei Yang <richard.weiyang@gmail.com> > >> Acked-by: Michal Hocko <mhocko@suse.com> > >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >> Cc: Andrew Morton <akpm@linux-foundation.org> > >> Cc: Michal Hocko <mhocko@kernel.org> > >> Cc: Oscar Salvador <osalvador@suse.de> > >> Cc: "Rafael J. Wysocki" <rafael@kernel.org> > >> Cc: Baoquan He <bhe@redhat.com> > >> Cc: Wei Yang <richard.weiyang@gmail.com> > >> Signed-off-by: David Hildenbrand <david@redhat.com> > >> --- > >> drivers/base/memory.c | 38 +++++++++++++++++++++++--------------- > >> 1 file changed, 23 insertions(+), 15 deletions(-) > >> > >> diff --git a/drivers/base/memory.c b/drivers/base/memory.c > >> index e7e77cafef80..8a7f29c0bf97 100644 > >> --- a/drivers/base/memory.c > >> +++ b/drivers/base/memory.c > >> @@ -28,6 +28,24 @@ > >> > >> #define MEMORY_CLASS_NAME "memory" > >> > >> +static const char *const online_type_to_str[] = { > >> + [MMOP_OFFLINE] = "offline", > >> + [MMOP_ONLINE] = "online", > >> + [MMOP_ONLINE_KERNEL] = "online_kernel", > >> + [MMOP_ONLINE_MOVABLE] = "online_movable", > >> +}; > >> + > >> +static int memhp_online_type_from_str(const char *str) > >> +{ > >> + int i; > > > > I would change it as: > > > > for (int i = 0; i < ARRAY_SIZE(online_type_to_str); i++) { > > > > That's not allowed by the C90 standard (and -std=gnu89). > > $ gcc main.c -std=gnu89 > main.c: In function 'main': > main.c:3:2: error: 'for' loop initial declarations are only allowed in > C99 or C11 mode > 3 | for (int i = 0; i < 8; i++) { > | ^~~ Good to know, thanks. > > One of the reasons why > git grep "for (int " > > will result in very little hits (IOW, only 5 in driver code only). > > -- > Thanks, > > David / dhildenb
diff --git a/drivers/base/memory.c b/drivers/base/memory.c index e7e77cafef80..8a7f29c0bf97 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -28,6 +28,24 @@ #define MEMORY_CLASS_NAME "memory" +static const char *const online_type_to_str[] = { + [MMOP_OFFLINE] = "offline", + [MMOP_ONLINE] = "online", + [MMOP_ONLINE_KERNEL] = "online_kernel", + [MMOP_ONLINE_MOVABLE] = "online_movable", +}; + +static int memhp_online_type_from_str(const char *str) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(online_type_to_str); i++) { + if (sysfs_streq(str, online_type_to_str[i])) + return i; + } + return -EINVAL; +} + #define to_memory_block(dev) container_of(dev, struct memory_block, dev) static int sections_per_block; @@ -236,26 +254,17 @@ static int memory_subsys_offline(struct device *dev) static ssize_t state_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { + const int online_type = memhp_online_type_from_str(buf); struct memory_block *mem = to_memory_block(dev); - int ret, online_type; + int ret; + + if (online_type < 0) + return -EINVAL; ret = lock_device_hotplug_sysfs(); if (ret) return ret; - if (sysfs_streq(buf, "online_kernel")) - online_type = MMOP_ONLINE_KERNEL; - else if (sysfs_streq(buf, "online_movable")) - online_type = MMOP_ONLINE_MOVABLE; - else if (sysfs_streq(buf, "online")) - online_type = MMOP_ONLINE; - else if (sysfs_streq(buf, "offline")) - online_type = MMOP_OFFLINE; - else { - ret = -EINVAL; - goto err; - } - switch (online_type) { case MMOP_ONLINE_KERNEL: case MMOP_ONLINE_MOVABLE: @@ -271,7 +280,6 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr, ret = -EINVAL; /* should never happen */ } -err: unlock_device_hotplug(); if (ret < 0)