| Message ID | 20180709072341.37649-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon 2018-07-09 10:23:41, Andy Shevchenko wrote: > Reuse DEFINE_SHOW_ATTRIBUTE() macro instead of open coding file > operations followed by custom ->open() callbacks per each attribute. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Is it supposed to fix the "grep /sys" crash? > --- > arch/arm/mach-omap2/pm-debug.c | 37 ++++++---------------------------- > 1 file changed, 6 insertions(+), 31 deletions(-) > > diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c > index acb698d5780f..5a8839203958 100644 > --- a/arch/arm/mach-omap2/pm-debug.c > +++ b/arch/arm/mach-omap2/pm-debug.c > @@ -47,11 +47,6 @@ static int pm_dbg_init_done; > > static int pm_dbg_init(void); > > -enum { > - DEBUG_FILE_COUNTERS = 0, > - DEBUG_FILE_TIMERS, > -}; > - > static const char pwrdm_state_names[][PWRDM_MAX_PWRSTS] = { > "OFF", > "RET", > @@ -141,39 +136,21 @@ static int pwrdm_dbg_show_timer(struct powerdomain *pwrdm, void *user) > return 0; > } > > -static int pm_dbg_show_counters(struct seq_file *s, void *unused) > +static int pm_dbg_counters_show(struct seq_file *s, void *unused) > { > pwrdm_for_each(pwrdm_dbg_show_counter, s); > clkdm_for_each(clkdm_dbg_show_counter, s); > > return 0; > } > +DEFINE_SHOW_ATTRIBUTE(pm_dbg_counters); > > -static int pm_dbg_show_timers(struct seq_file *s, void *unused) > +static int pm_dbg_timers_show(struct seq_file *s, void *unused) > { > pwrdm_for_each(pwrdm_dbg_show_timer, s); > return 0; > } > - > -static int pm_dbg_open(struct inode *inode, struct file *file) > -{ > - switch ((int)inode->i_private) { > - case DEBUG_FILE_COUNTERS: > - return single_open(file, pm_dbg_show_counters, > - &inode->i_private); > - case DEBUG_FILE_TIMERS: > - default: > - return single_open(file, pm_dbg_show_timers, > - &inode->i_private); > - } > -} > - > -static const struct file_operations debug_fops = { > - .open = pm_dbg_open, > - .read = seq_read, > - .llseek = seq_lseek, > - .release = single_release, > -}; > +DEFINE_SHOW_ATTRIBUTE(pm_dbg_timers); > > static int pwrdm_suspend_get(void *data, u64 *val) > { > @@ -259,10 +236,8 @@ static int __init pm_dbg_init(void) > if (!d) > return -EINVAL; > > - (void) debugfs_create_file("count", S_IRUGO, > - d, (void *)DEBUG_FILE_COUNTERS, &debug_fops); > - (void) debugfs_create_file("time", S_IRUGO, > - d, (void *)DEBUG_FILE_TIMERS, &debug_fops); > + (void) debugfs_create_file("count", 0444, d, NULL, &pm_dbg_counters_fops); > + (void) debugfs_create_file("time", 0444, d, NULL, &pm_dbg_timers_fops); > > pwrdm_for_each(pwrdms_setup, (void *)d); >
On Mon, Jul 9, 2018 at 10:30 AM, Pavel Machek <pavel@ucw.cz> wrote: > On Mon 2018-07-09 10:23:41, Andy Shevchenko wrote: >> Reuse DEFINE_SHOW_ATTRIBUTE() macro instead of open coding file >> operations followed by custom ->open() callbacks per each attribute. >> >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Is it supposed to fix the "grep /sys" crash? Nope. Just a clean up. You in Cc list since you are doing some investigations in this area. > > >> --- >> arch/arm/mach-omap2/pm-debug.c | 37 ++++++---------------------------- >> 1 file changed, 6 insertions(+), 31 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c >> index acb698d5780f..5a8839203958 100644 >> --- a/arch/arm/mach-omap2/pm-debug.c >> +++ b/arch/arm/mach-omap2/pm-debug.c >> @@ -47,11 +47,6 @@ static int pm_dbg_init_done; >> >> static int pm_dbg_init(void); >> >> -enum { >> - DEBUG_FILE_COUNTERS = 0, >> - DEBUG_FILE_TIMERS, >> -}; >> - >> static const char pwrdm_state_names[][PWRDM_MAX_PWRSTS] = { >> "OFF", >> "RET", >> @@ -141,39 +136,21 @@ static int pwrdm_dbg_show_timer(struct powerdomain *pwrdm, void *user) >> return 0; >> } >> >> -static int pm_dbg_show_counters(struct seq_file *s, void *unused) >> +static int pm_dbg_counters_show(struct seq_file *s, void *unused) >> { >> pwrdm_for_each(pwrdm_dbg_show_counter, s); >> clkdm_for_each(clkdm_dbg_show_counter, s); >> >> return 0; >> } >> +DEFINE_SHOW_ATTRIBUTE(pm_dbg_counters); >> >> -static int pm_dbg_show_timers(struct seq_file *s, void *unused) >> +static int pm_dbg_timers_show(struct seq_file *s, void *unused) >> { >> pwrdm_for_each(pwrdm_dbg_show_timer, s); >> return 0; >> } >> - >> -static int pm_dbg_open(struct inode *inode, struct file *file) >> -{ >> - switch ((int)inode->i_private) { >> - case DEBUG_FILE_COUNTERS: >> - return single_open(file, pm_dbg_show_counters, >> - &inode->i_private); >> - case DEBUG_FILE_TIMERS: >> - default: >> - return single_open(file, pm_dbg_show_timers, >> - &inode->i_private); >> - } >> -} >> - >> -static const struct file_operations debug_fops = { >> - .open = pm_dbg_open, >> - .read = seq_read, >> - .llseek = seq_lseek, >> - .release = single_release, >> -}; >> +DEFINE_SHOW_ATTRIBUTE(pm_dbg_timers); >> >> static int pwrdm_suspend_get(void *data, u64 *val) >> { >> @@ -259,10 +236,8 @@ static int __init pm_dbg_init(void) >> if (!d) >> return -EINVAL; >> >> - (void) debugfs_create_file("count", S_IRUGO, >> - d, (void *)DEBUG_FILE_COUNTERS, &debug_fops); >> - (void) debugfs_create_file("time", S_IRUGO, >> - d, (void *)DEBUG_FILE_TIMERS, &debug_fops); >> + (void) debugfs_create_file("count", 0444, d, NULL, &pm_dbg_counters_fops); >> + (void) debugfs_create_file("time", 0444, d, NULL, &pm_dbg_timers_fops); >> >> pwrdm_for_each(pwrdms_setup, (void *)d); >> > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Mon 2018-07-09 10:35:23, Andy Shevchenko wrote: > On Mon, Jul 9, 2018 at 10:30 AM, Pavel Machek <pavel@ucw.cz> wrote: > > On Mon 2018-07-09 10:23:41, Andy Shevchenko wrote: > >> Reuse DEFINE_SHOW_ATTRIBUTE() macro instead of open coding file > >> operations followed by custom ->open() callbacks per each attribute. > >> > >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > Is it supposed to fix the "grep /sys" crash? > > Nope. > Just a clean up. > You in Cc list since you are doing some investigations in this area. Aha. Thanks for the Cc. I test-booted kernel with patch applied, and it still crashes same way upon grep, so I guess that's Tested-by... :-). Pavel
On Mon, 2018-07-09 at 10:45 +0200, Pavel Machek wrote: > On Mon 2018-07-09 10:35:23, Andy Shevchenko wrote: > > On Mon, Jul 9, 2018 at 10:30 AM, Pavel Machek <pavel@ucw.cz> wrote: > > > On Mon 2018-07-09 10:23:41, Andy Shevchenko wrote: > > > > Reuse DEFINE_SHOW_ATTRIBUTE() macro instead of open coding file > > > > operations followed by custom ->open() callbacks per each > > > > attribute. > > > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.co > > > > m> > > > > > > Is it supposed to fix the "grep /sys" crash? > > > > Nope. > > Just a clean up. > > You in Cc list since you are doing some investigations in this area. > > Aha. Thanks for the Cc. > > I test-booted kernel with patch applied, and it still crashes same way > upon grep, so I guess that's Tested-by... :-). Yep, thanks! Btw, looking into problem you are experiencing I would bet that if crashes in clkdm_dbg_show_counter(), b/c grepping against "time" was successful. Other than that I can't imaging what is happening there. You may easily add debug prints there and see what exactly makes a crash.
* Andy Shevchenko <andriy.shevchenko@linux.intel.com> [180709 00:26]: > Reuse DEFINE_SHOW_ATTRIBUTE() macro instead of open coding file > operations followed by custom ->open() callbacks per each attribute. Thanks applying into omap-for-v4.19/soc. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/pm-debug.c b/arch/arm/mach-omap2/pm-debug.c index acb698d5780f..5a8839203958 100644 --- a/arch/arm/mach-omap2/pm-debug.c +++ b/arch/arm/mach-omap2/pm-debug.c @@ -47,11 +47,6 @@ static int pm_dbg_init_done; static int pm_dbg_init(void); -enum { - DEBUG_FILE_COUNTERS = 0, - DEBUG_FILE_TIMERS, -}; - static const char pwrdm_state_names[][PWRDM_MAX_PWRSTS] = { "OFF", "RET", @@ -141,39 +136,21 @@ static int pwrdm_dbg_show_timer(struct powerdomain *pwrdm, void *user) return 0; } -static int pm_dbg_show_counters(struct seq_file *s, void *unused) +static int pm_dbg_counters_show(struct seq_file *s, void *unused) { pwrdm_for_each(pwrdm_dbg_show_counter, s); clkdm_for_each(clkdm_dbg_show_counter, s); return 0; } +DEFINE_SHOW_ATTRIBUTE(pm_dbg_counters); -static int pm_dbg_show_timers(struct seq_file *s, void *unused) +static int pm_dbg_timers_show(struct seq_file *s, void *unused) { pwrdm_for_each(pwrdm_dbg_show_timer, s); return 0; } - -static int pm_dbg_open(struct inode *inode, struct file *file) -{ - switch ((int)inode->i_private) { - case DEBUG_FILE_COUNTERS: - return single_open(file, pm_dbg_show_counters, - &inode->i_private); - case DEBUG_FILE_TIMERS: - default: - return single_open(file, pm_dbg_show_timers, - &inode->i_private); - } -} - -static const struct file_operations debug_fops = { - .open = pm_dbg_open, - .read = seq_read, - .llseek = seq_lseek, - .release = single_release, -}; +DEFINE_SHOW_ATTRIBUTE(pm_dbg_timers); static int pwrdm_suspend_get(void *data, u64 *val) { @@ -259,10 +236,8 @@ static int __init pm_dbg_init(void) if (!d) return -EINVAL; - (void) debugfs_create_file("count", S_IRUGO, - d, (void *)DEBUG_FILE_COUNTERS, &debug_fops); - (void) debugfs_create_file("time", S_IRUGO, - d, (void *)DEBUG_FILE_TIMERS, &debug_fops); + (void) debugfs_create_file("count", 0444, d, NULL, &pm_dbg_counters_fops); + (void) debugfs_create_file("time", 0444, d, NULL, &pm_dbg_timers_fops); pwrdm_for_each(pwrdms_setup, (void *)d);
Reuse DEFINE_SHOW_ATTRIBUTE() macro instead of open coding file operations followed by custom ->open() callbacks per each attribute. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- arch/arm/mach-omap2/pm-debug.c | 37 ++++++---------------------------- 1 file changed, 6 insertions(+), 31 deletions(-)