Let the passed in array be const (and thus placed in rodata) instead of
a mutable array of const pointers.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
---
fs/sysfs/file.c | 4 ++--
include/linux/sysfs.h | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 0a7252aecfa5..bb71db63c99c 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -334,7 +334,7 @@ int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr,
}
EXPORT_SYMBOL_GPL(sysfs_create_file_ns);
-int sysfs_create_files(struct kobject *kobj, const struct attribute **ptr)
+int sysfs_create_files(struct kobject *kobj, const struct attribute * const *ptr)
{
int err = 0;
int i;
@@ -493,7 +493,7 @@ bool sysfs_remove_file_self(struct kobject *kobj, const struct attribute *attr)
return ret;
}
-void sysfs_remove_files(struct kobject *kobj, const struct attribute **ptr)
+void sysfs_remove_files(struct kobject *kobj, const struct attribute * const *ptr)
{
int i;
for (i = 0; ptr[i]; i++)
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 987cefa337de..786816cf4aa5 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -234,7 +234,7 @@ int __must_check sysfs_create_file_ns(struct kobject *kobj,
const struct attribute *attr,
const void *ns);
int __must_check sysfs_create_files(struct kobject *kobj,
- const struct attribute **attr);
+ const struct attribute * const *attr);
int __must_check sysfs_chmod_file(struct kobject *kobj,
const struct attribute *attr, umode_t mode);
struct kernfs_node *sysfs_break_active_protection(struct kobject *kobj,
@@ -243,7 +243,7 @@ void sysfs_unbreak_active_protection(struct kernfs_node *kn);
void sysfs_remove_file_ns(struct kobject *kobj, const struct attribute *attr,
const void *ns);
bool sysfs_remove_file_self(struct kobject *kobj, const struct attribute *attr);
-void sysfs_remove_files(struct kobject *kobj, const struct attribute **attr);
+void sysfs_remove_files(struct kobject *kobj, const struct attribute * const *attr);
int __must_check sysfs_create_bin_file(struct kobject *kobj,
const struct bin_attribute *attr);
@@ -342,7 +342,7 @@ static inline int sysfs_create_file_ns(struct kobject *kobj,
}
static inline int sysfs_create_files(struct kobject *kobj,
- const struct attribute **attr)
+ const struct attribute * const *attr)
{
return 0;
}
@@ -377,7 +377,7 @@ static inline bool sysfs_remove_file_self(struct kobject *kobj,
}
static inline void sysfs_remove_files(struct kobject *kobj,
- const struct attribute **attr)
+ const struct attribute * const *attr)
{
}
--
2.11.0
They don't need to be modified.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Signed-off-by: Jani Nikula <[email protected]>
---
drivers/gpu/drm/i915/i915_sysfs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index e5e6f6bb2b05..b9aa8515254e 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -483,7 +483,7 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr
return snprintf(buf, PAGE_SIZE, "%d\n", val);
}
-static const struct attribute *gen6_attrs[] = {
+static const struct attribute * const gen6_attrs[] = {
&dev_attr_gt_act_freq_mhz.attr,
&dev_attr_gt_cur_freq_mhz.attr,
&dev_attr_gt_boost_freq_mhz.attr,
@@ -495,7 +495,7 @@ static const struct attribute *gen6_attrs[] = {
NULL,
};
-static const struct attribute *vlv_attrs[] = {
+static const struct attribute * const vlv_attrs[] = {
&dev_attr_gt_act_freq_mhz.attr,
&dev_attr_gt_cur_freq_mhz.attr,
&dev_attr_gt_boost_freq_mhz.attr,
--
2.11.0
On Thu, Oct 4, 2018 at 4:38 PM Jani Nikula <[email protected]> wrote:
>
> Let the passed in array be const (and thus placed in rodata) instead of
> a mutable array of const pointers.
I'm not sure if the changes guarantee what you want. If I'm not
mistaken, they just mean that the function itself cannot modify either
the pointer passed to it, or the contents of the array pointed to by
that pointer. They don't imply the location of the array itself,
though.
As for the changes:
Reviewed-by: Rafael J. Wysocki <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Signed-off-by: Jani Nikula <[email protected]>
> ---
> fs/sysfs/file.c | 4 ++--
> include/linux/sysfs.h | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index 0a7252aecfa5..bb71db63c99c 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -334,7 +334,7 @@ int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr,
> }
> EXPORT_SYMBOL_GPL(sysfs_create_file_ns);
>
> -int sysfs_create_files(struct kobject *kobj, const struct attribute **ptr)
> +int sysfs_create_files(struct kobject *kobj, const struct attribute * const *ptr)
> {
> int err = 0;
> int i;
> @@ -493,7 +493,7 @@ bool sysfs_remove_file_self(struct kobject *kobj, const struct attribute *attr)
> return ret;
> }
>
> -void sysfs_remove_files(struct kobject *kobj, const struct attribute **ptr)
> +void sysfs_remove_files(struct kobject *kobj, const struct attribute * const *ptr)
> {
> int i;
> for (i = 0; ptr[i]; i++)
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 987cefa337de..786816cf4aa5 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -234,7 +234,7 @@ int __must_check sysfs_create_file_ns(struct kobject *kobj,
> const struct attribute *attr,
> const void *ns);
> int __must_check sysfs_create_files(struct kobject *kobj,
> - const struct attribute **attr);
> + const struct attribute * const *attr);
> int __must_check sysfs_chmod_file(struct kobject *kobj,
> const struct attribute *attr, umode_t mode);
> struct kernfs_node *sysfs_break_active_protection(struct kobject *kobj,
> @@ -243,7 +243,7 @@ void sysfs_unbreak_active_protection(struct kernfs_node *kn);
> void sysfs_remove_file_ns(struct kobject *kobj, const struct attribute *attr,
> const void *ns);
> bool sysfs_remove_file_self(struct kobject *kobj, const struct attribute *attr);
> -void sysfs_remove_files(struct kobject *kobj, const struct attribute **attr);
> +void sysfs_remove_files(struct kobject *kobj, const struct attribute * const *attr);
>
> int __must_check sysfs_create_bin_file(struct kobject *kobj,
> const struct bin_attribute *attr);
> @@ -342,7 +342,7 @@ static inline int sysfs_create_file_ns(struct kobject *kobj,
> }
>
> static inline int sysfs_create_files(struct kobject *kobj,
> - const struct attribute **attr)
> + const struct attribute * const *attr)
> {
> return 0;
> }
> @@ -377,7 +377,7 @@ static inline bool sysfs_remove_file_self(struct kobject *kobj,
> }
>
> static inline void sysfs_remove_files(struct kobject *kobj,
> - const struct attribute **attr)
> + const struct attribute * const *attr)
> {
> }
>
> --
> 2.11.0
>
On Thu, Oct 4, 2018 at 4:38 PM Jani Nikula <[email protected]> wrote:
>
> They don't need to be modified.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Signed-off-by: Jani Nikula <[email protected]>
Reviewed-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/gpu/drm/i915/i915_sysfs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
> index e5e6f6bb2b05..b9aa8515254e 100644
> --- a/drivers/gpu/drm/i915/i915_sysfs.c
> +++ b/drivers/gpu/drm/i915/i915_sysfs.c
> @@ -483,7 +483,7 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr
> return snprintf(buf, PAGE_SIZE, "%d\n", val);
> }
>
> -static const struct attribute *gen6_attrs[] = {
> +static const struct attribute * const gen6_attrs[] = {
> &dev_attr_gt_act_freq_mhz.attr,
> &dev_attr_gt_cur_freq_mhz.attr,
> &dev_attr_gt_boost_freq_mhz.attr,
> @@ -495,7 +495,7 @@ static const struct attribute *gen6_attrs[] = {
> NULL,
> };
>
> -static const struct attribute *vlv_attrs[] = {
> +static const struct attribute * const vlv_attrs[] = {
> &dev_attr_gt_act_freq_mhz.attr,
> &dev_attr_gt_cur_freq_mhz.attr,
> &dev_attr_gt_boost_freq_mhz.attr,
> --
> 2.11.0
>
On Fri, 05 Oct 2018, "Rafael J. Wysocki" <[email protected]> wrote:
> On Thu, Oct 4, 2018 at 4:38 PM Jani Nikula <[email protected]> wrote:
>>
>> Let the passed in array be const (and thus placed in rodata) instead of
>> a mutable array of const pointers.
>
> I'm not sure if the changes guarantee what you want. If I'm not
> mistaken, they just mean that the function itself cannot modify either
> the pointer passed to it, or the contents of the array pointed to by
> that pointer. They don't imply the location of the array itself,
> though.
I mean, this change allows the caller to add the appropriate const
qualifiers to the array definition, allowing the placement in
rodata. Can't do that withouth the extra const in the function.
> As for the changes:
>
> Reviewed-by: Rafael J. Wysocki <[email protected]>
Thanks for the review.
BR,
Jani.
>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>> Signed-off-by: Jani Nikula <[email protected]>
>> ---
>> fs/sysfs/file.c | 4 ++--
>> include/linux/sysfs.h | 8 ++++----
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
>> index 0a7252aecfa5..bb71db63c99c 100644
>> --- a/fs/sysfs/file.c
>> +++ b/fs/sysfs/file.c
>> @@ -334,7 +334,7 @@ int sysfs_create_file_ns(struct kobject *kobj, const struct attribute *attr,
>> }
>> EXPORT_SYMBOL_GPL(sysfs_create_file_ns);
>>
>> -int sysfs_create_files(struct kobject *kobj, const struct attribute **ptr)
>> +int sysfs_create_files(struct kobject *kobj, const struct attribute * const *ptr)
>> {
>> int err = 0;
>> int i;
>> @@ -493,7 +493,7 @@ bool sysfs_remove_file_self(struct kobject *kobj, const struct attribute *attr)
>> return ret;
>> }
>>
>> -void sysfs_remove_files(struct kobject *kobj, const struct attribute **ptr)
>> +void sysfs_remove_files(struct kobject *kobj, const struct attribute * const *ptr)
>> {
>> int i;
>> for (i = 0; ptr[i]; i++)
>> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
>> index 987cefa337de..786816cf4aa5 100644
>> --- a/include/linux/sysfs.h
>> +++ b/include/linux/sysfs.h
>> @@ -234,7 +234,7 @@ int __must_check sysfs_create_file_ns(struct kobject *kobj,
>> const struct attribute *attr,
>> const void *ns);
>> int __must_check sysfs_create_files(struct kobject *kobj,
>> - const struct attribute **attr);
>> + const struct attribute * const *attr);
>> int __must_check sysfs_chmod_file(struct kobject *kobj,
>> const struct attribute *attr, umode_t mode);
>> struct kernfs_node *sysfs_break_active_protection(struct kobject *kobj,
>> @@ -243,7 +243,7 @@ void sysfs_unbreak_active_protection(struct kernfs_node *kn);
>> void sysfs_remove_file_ns(struct kobject *kobj, const struct attribute *attr,
>> const void *ns);
>> bool sysfs_remove_file_self(struct kobject *kobj, const struct attribute *attr);
>> -void sysfs_remove_files(struct kobject *kobj, const struct attribute **attr);
>> +void sysfs_remove_files(struct kobject *kobj, const struct attribute * const *attr);
>>
>> int __must_check sysfs_create_bin_file(struct kobject *kobj,
>> const struct bin_attribute *attr);
>> @@ -342,7 +342,7 @@ static inline int sysfs_create_file_ns(struct kobject *kobj,
>> }
>>
>> static inline int sysfs_create_files(struct kobject *kobj,
>> - const struct attribute **attr)
>> + const struct attribute * const *attr)
>> {
>> return 0;
>> }
>> @@ -377,7 +377,7 @@ static inline bool sysfs_remove_file_self(struct kobject *kobj,
>> }
>>
>> static inline void sysfs_remove_files(struct kobject *kobj,
>> - const struct attribute **attr)
>> + const struct attribute * const *attr)
>> {
>> }
>>
>> --
>> 2.11.0
>>
--
Jani Nikula, Intel Open Source Graphics Center
On Fri, Oct 5, 2018 at 9:36 AM Jani Nikula <[email protected]> wrote:
>
> On Fri, 05 Oct 2018, "Rafael J. Wysocki" <[email protected]> wrote:
> > On Thu, Oct 4, 2018 at 4:38 PM Jani Nikula <[email protected]> wrote:
> >>
> >> Let the passed in array be const (and thus placed in rodata) instead of
> >> a mutable array of const pointers.
> >
> > I'm not sure if the changes guarantee what you want. If I'm not
> > mistaken, they just mean that the function itself cannot modify either
> > the pointer passed to it, or the contents of the array pointed to by
> > that pointer. They don't imply the location of the array itself,
> > though.
>
> I mean, this change allows the caller to add the appropriate const
> qualifiers to the array definition, allowing the placement in
> rodata. Can't do that withouth the extra const in the function.
I figured that out, but the changelog is a bit unclear. If you said
"Allow ..." instead of "Let ...", it would be somewhat clearer IMO.
:-)
On Thu, Oct 04, 2018 at 05:37:49PM +0300, Jani Nikula wrote:
> Let the passed in array be const (and thus placed in rodata) instead of
> a mutable array of const pointers.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Signed-off-by: Jani Nikula <[email protected]>
> ---
> fs/sysfs/file.c | 4 ++--
> include/linux/sysfs.h | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Greg Kroah-Hartman <[email protected]>
On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
> On Thu, Oct 04, 2018 at 05:37:49PM +0300, Jani Nikula wrote:
>> Let the passed in array be const (and thus placed in rodata) instead of
>> a mutable array of const pointers.
>>
>> Cc: Greg Kroah-Hartman <[email protected]>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>> Signed-off-by: Jani Nikula <[email protected]>
>> ---
>> fs/sysfs/file.c | 4 ++--
>> include/linux/sysfs.h | 8 ++++----
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> Reviewed-by: Greg Kroah-Hartman <[email protected]>
Thanks; who should pick them up?
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Tue, Oct 16, 2018 at 02:38:53PM +0300, Jani Nikula wrote:
> On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
> > On Thu, Oct 04, 2018 at 05:37:49PM +0300, Jani Nikula wrote:
> >> Let the passed in array be const (and thus placed in rodata) instead of
> >> a mutable array of const pointers.
> >>
> >> Cc: Greg Kroah-Hartman <[email protected]>
> >> Cc: "Rafael J. Wysocki" <[email protected]>
> >> Signed-off-by: Jani Nikula <[email protected]>
> >> ---
> >> fs/sysfs/file.c | 4 ++--
> >> include/linux/sysfs.h | 8 ++++----
> >> 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > Reviewed-by: Greg Kroah-Hartman <[email protected]>
>
> Thanks; who should pick them up?
I could pick the first one up, but as you need this for a drm patch,
both should probably go through that tree, right?
thanks,
greg k-h
On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
> On Tue, Oct 16, 2018 at 02:38:53PM +0300, Jani Nikula wrote:
>> On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
>> > On Thu, Oct 04, 2018 at 05:37:49PM +0300, Jani Nikula wrote:
>> >> Let the passed in array be const (and thus placed in rodata) instead of
>> >> a mutable array of const pointers.
>> >>
>> >> Cc: Greg Kroah-Hartman <[email protected]>
>> >> Cc: "Rafael J. Wysocki" <[email protected]>
>> >> Signed-off-by: Jani Nikula <[email protected]>
>> >> ---
>> >> fs/sysfs/file.c | 4 ++--
>> >> include/linux/sysfs.h | 8 ++++----
>> >> 2 files changed, 6 insertions(+), 6 deletions(-)
>> >
>> > Reviewed-by: Greg Kroah-Hartman <[email protected]>
>>
>> Thanks; who should pick them up?
>
> I could pick the first one up, but as you need this for a drm patch,
> both should probably go through that tree, right?
I can pick them both if it doesn't bother you that the sysfs parts will
be merged in the merge window for v4.21 i.e. the change would be in drm
trees for an entire release cycle. We've been done for v4.20 merge
window for a while now.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Tue, Oct 16, 2018 at 03:10:24PM +0300, Jani Nikula wrote:
> On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
> > On Tue, Oct 16, 2018 at 02:38:53PM +0300, Jani Nikula wrote:
> >> On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
> >> > On Thu, Oct 04, 2018 at 05:37:49PM +0300, Jani Nikula wrote:
> >> >> Let the passed in array be const (and thus placed in rodata) instead of
> >> >> a mutable array of const pointers.
> >> >>
> >> >> Cc: Greg Kroah-Hartman <[email protected]>
> >> >> Cc: "Rafael J. Wysocki" <[email protected]>
> >> >> Signed-off-by: Jani Nikula <[email protected]>
> >> >> ---
> >> >> fs/sysfs/file.c | 4 ++--
> >> >> include/linux/sysfs.h | 8 ++++----
> >> >> 2 files changed, 6 insertions(+), 6 deletions(-)
> >> >
> >> > Reviewed-by: Greg Kroah-Hartman <[email protected]>
> >>
> >> Thanks; who should pick them up?
> >
> > I could pick the first one up, but as you need this for a drm patch,
> > both should probably go through that tree, right?
>
> I can pick them both if it doesn't bother you that the sysfs parts will
> be merged in the merge window for v4.21 i.e. the change would be in drm
> trees for an entire release cycle. We've been done for v4.20 merge
> window for a while now.
No objection from me.
greg k-h
On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
> On Tue, Oct 16, 2018 at 03:10:24PM +0300, Jani Nikula wrote:
>> On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
>> > On Tue, Oct 16, 2018 at 02:38:53PM +0300, Jani Nikula wrote:
>> >> On Tue, 16 Oct 2018, Greg Kroah-Hartman <[email protected]> wrote:
>> >> > On Thu, Oct 04, 2018 at 05:37:49PM +0300, Jani Nikula wrote:
>> >> >> Let the passed in array be const (and thus placed in rodata) instead of
>> >> >> a mutable array of const pointers.
>> >> >>
>> >> >> Cc: Greg Kroah-Hartman <[email protected]>
>> >> >> Cc: "Rafael J. Wysocki" <[email protected]>
>> >> >> Signed-off-by: Jani Nikula <[email protected]>
>> >> >> ---
>> >> >> fs/sysfs/file.c | 4 ++--
>> >> >> include/linux/sysfs.h | 8 ++++----
>> >> >> 2 files changed, 6 insertions(+), 6 deletions(-)
>> >> >
>> >> > Reviewed-by: Greg Kroah-Hartman <[email protected]>
>> >>
>> >> Thanks; who should pick them up?
>> >
>> > I could pick the first one up, but as you need this for a drm patch,
>> > both should probably go through that tree, right?
>>
>> I can pick them both if it doesn't bother you that the sysfs parts will
>> be merged in the merge window for v4.21 i.e. the change would be in drm
>> trees for an entire release cycle. We've been done for v4.20 merge
>> window for a while now.
>
> No objection from me.
Argh, I dropped the ball a bit. May I take that as an ack to merge via
drm-intel? I try to be careful not to assume acks and reviews.
Thanks,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center