Re: [PATCH 3/7] powerpc/eeh_cache: Add a way to dump the EEH address cache
Oliver writes:
> On Fri, Feb 8, 2019 at 8:47 PM Michael Ellerman wrote:
>> Oliver O'Halloran writes:
>> > diff --git a/arch/powerpc/kernel/eeh_cache.c
>> > b/arch/powerpc/kernel/eeh_cache.c
>> > index b2c320e0fcef..dba421a577e7 100644
>> > --- a/arch/powerpc/kernel/eeh_cache.c
>> > +++ b/arch/powerpc/kernel/eeh_cache.c
>> > @@ -298,9 +299,34 @@ void eeh_addr_cache_build(void)
>> > eeh_addr_cache_insert_dev(dev);
>> > eeh_sysfs_add_device(dev);
>> > }
>> > +}
>> >
>> > -#ifdef DEBUG
>> > - /* Verify tree built up above, echo back the list of addrs. */
>> > - eeh_addr_cache_print(_io_addr_cache_root);
>> > -#endif
>> > +static int eeh_addr_cache_show(struct seq_file *s, void *v)
>> > +{
>> > + struct rb_node *n = rb_first(_io_addr_cache_root.rb_root);
>> > + struct pci_io_addr_range *piar;
>> > + int cnt = 0;
>> > +
>> > + spin_lock(_io_addr_cache_root.piar_lock);
>> > + while (n) {
>> > + piar = rb_entry(n, struct pci_io_addr_range, rb_node);
>> > +
>> > + seq_printf(s, "%s addr range %3d [%pap-%pap]: %s\n",
>> > +(piar->flags & IORESOURCE_IO) ? "i/o" : "mem", cnt,
>> > +>addr_lo, >addr_hi,
>> > pci_name(piar->pcidev));
>> > +
>> > + n = rb_next(n);
>> > + cnt++;
>> > + }
>>
>> You can write that as a for loop can't you?
>>
>> struct rb_node *n;
>> int i = 0;
>>
>> for (n = rb_first(_io_addr_cache_root.rb_root); n; n =
>> rb_next(n), i++) {
>
> IIRC I did try that, but it's too long. 85 cols wide according to my editor.
Don't care.
Long lines aren't inherently evil, they have some downsides but so do
the other options. In a case like this 85 columns would be preferable to
splitting the line or writing it a while loop.
cheers
Re: [PATCH 3/7] powerpc/eeh_cache: Add a way to dump the EEH address cache
On Fri, Feb 8, 2019 at 8:47 PM Michael Ellerman wrote:
>
> Oliver O'Halloran writes:
> > diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> > index f6e65375a8de..d1f0bdf41fac 100644
> > --- a/arch/powerpc/kernel/eeh.c
> > +++ b/arch/powerpc/kernel/eeh.c
> > @@ -1810,7 +1810,7 @@ static int __init eeh_init_proc(void)
> > _enable_dbgfs_ops);
> > debugfs_create_u32("eeh_max_freezes", 0600,
> > powerpc_debugfs_root, _max_freezes);
> > -#endif
> > + eeh_cache_debugfs_init();
>
> Oops :)
Yeah :(
> > diff --git a/arch/powerpc/kernel/eeh_cache.c
> > b/arch/powerpc/kernel/eeh_cache.c
> > index b2c320e0fcef..dba421a577e7 100644
> > --- a/arch/powerpc/kernel/eeh_cache.c
> > +++ b/arch/powerpc/kernel/eeh_cache.c
> > @@ -298,9 +299,34 @@ void eeh_addr_cache_build(void)
> > eeh_addr_cache_insert_dev(dev);
> > eeh_sysfs_add_device(dev);
> > }
> > +}
> >
> > -#ifdef DEBUG
> > - /* Verify tree built up above, echo back the list of addrs. */
> > - eeh_addr_cache_print(_io_addr_cache_root);
> > -#endif
> > +static int eeh_addr_cache_show(struct seq_file *s, void *v)
> > +{
> > + struct rb_node *n = rb_first(_io_addr_cache_root.rb_root);
> > + struct pci_io_addr_range *piar;
> > + int cnt = 0;
> > +
> > + spin_lock(_io_addr_cache_root.piar_lock);
> > + while (n) {
> > + piar = rb_entry(n, struct pci_io_addr_range, rb_node);
> > +
> > + seq_printf(s, "%s addr range %3d [%pap-%pap]: %s\n",
> > +(piar->flags & IORESOURCE_IO) ? "i/o" : "mem", cnt,
> > +>addr_lo, >addr_hi,
> > pci_name(piar->pcidev));
> > +
> > + n = rb_next(n);
> > + cnt++;
> > + }
>
> You can write that as a for loop can't you?
>
> struct rb_node *n;
> int i = 0;
>
> for (n = rb_first(_io_addr_cache_root.rb_root); n; n =
> rb_next(n), i++) {
IIRC I did try that, but it's too long. 85 cols wide according to my editor.
> piar = rb_entry(n, struct pci_io_addr_range, rb_node);
>
> seq_printf(s, "%s addr range %3d [%pap-%pap]: %s\n",
>(piar->flags & IORESOURCE_IO) ? "i/o" : "mem", i,
>>addr_lo, >addr_hi,
> pci_name(piar->pcidev));
> }
>
> cheers
Re: [PATCH 3/7] powerpc/eeh_cache: Add a way to dump the EEH address cache
Oliver O'Halloran writes:
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index f6e65375a8de..d1f0bdf41fac 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -1810,7 +1810,7 @@ static int __init eeh_init_proc(void)
> _enable_dbgfs_ops);
> debugfs_create_u32("eeh_max_freezes", 0600,
> powerpc_debugfs_root, _max_freezes);
> -#endif
> + eeh_cache_debugfs_init();
Oops :)
> diff --git a/arch/powerpc/kernel/eeh_cache.c b/arch/powerpc/kernel/eeh_cache.c
> index b2c320e0fcef..dba421a577e7 100644
> --- a/arch/powerpc/kernel/eeh_cache.c
> +++ b/arch/powerpc/kernel/eeh_cache.c
> @@ -298,9 +299,34 @@ void eeh_addr_cache_build(void)
> eeh_addr_cache_insert_dev(dev);
> eeh_sysfs_add_device(dev);
> }
> +}
>
> -#ifdef DEBUG
> - /* Verify tree built up above, echo back the list of addrs. */
> - eeh_addr_cache_print(_io_addr_cache_root);
> -#endif
> +static int eeh_addr_cache_show(struct seq_file *s, void *v)
> +{
> + struct rb_node *n = rb_first(_io_addr_cache_root.rb_root);
> + struct pci_io_addr_range *piar;
> + int cnt = 0;
> +
> + spin_lock(_io_addr_cache_root.piar_lock);
> + while (n) {
> + piar = rb_entry(n, struct pci_io_addr_range, rb_node);
> +
> + seq_printf(s, "%s addr range %3d [%pap-%pap]: %s\n",
> +(piar->flags & IORESOURCE_IO) ? "i/o" : "mem", cnt,
> +>addr_lo, >addr_hi, pci_name(piar->pcidev));
> +
> + n = rb_next(n);
> + cnt++;
> + }
You can write that as a for loop can't you?
struct rb_node *n;
int i = 0;
for (n = rb_first(_io_addr_cache_root.rb_root); n; n = rb_next(n),
i++) {
piar = rb_entry(n, struct pci_io_addr_range, rb_node);
seq_printf(s, "%s addr range %3d [%pap-%pap]: %s\n",
(piar->flags & IORESOURCE_IO) ? "i/o" : "mem", i,
>addr_lo, >addr_hi, pci_name(piar->pcidev));
}
cheers
Re: [PATCH 3/7] powerpc/eeh_cache: Add a way to dump the EEH address cache
Hi Oliver,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.0-rc4 next-20190207]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Oliver-O-Halloran/powerpc-eeh-Use-debugfs_create_u32-for-eeh_max_freezes/20190208-145918
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=powerpc
Note: the
linux-review/Oliver-O-Halloran/powerpc-eeh-Use-debugfs_create_u32-for-eeh_max_freezes/20190208-145918
HEAD a8dcd44575537e3e67a44fe3139b273a64c0f620 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
>> arch/powerpc/kernel/eeh.c:1840: error: unterminated #ifdef
#ifdef CONFIG_DEBUG_FS
vim +1840 arch/powerpc/kernel/eeh.c
7f52a526f arch/powerpc/kernel/eeh.c Gavin Shan2014-04-24 1835
^1da177e4 arch/ppc64/kernel/eeh.c Linus Torvalds2005-04-16 1836 static
int __init eeh_init_proc(void)
^1da177e4 arch/ppc64/kernel/eeh.c Linus Torvalds2005-04-16 1837 {
7f52a526f arch/powerpc/kernel/eeh.c Gavin Shan2014-04-24 1838
if (machine_is(pseries) || machine_is(powernv)) {
3f3942aca arch/powerpc/kernel/eeh.c Christoph Hellwig 2018-05-15 1839
proc_create_single("powerpc/eeh", 0, NULL, proc_eeh_show);
7f52a526f arch/powerpc/kernel/eeh.c Gavin Shan2014-04-24 @1840 #ifdef
CONFIG_DEBUG_FS
:: The code at line 1840 was first introduced by commit
:: 7f52a526f64c69c913f0027fbf43821ff0b3a7d7 powerpc/eeh: Allow to disable
EEH
:: TO: Gavin Shan
:: CC: Benjamin Herrenschmidt
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
