Re: [PATCH] PCI/AER: Print error message as per the TODO
On Mon, 2024-04-15 at 16:10 +, Abhinav Jain wrote:
> Add a pr_err() to print the add device error in find_device_iter()
[]
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
[]
> @@ -885,7 +885,8 @@ static int find_device_iter(struct pci_dev *dev, void
> *data)
> /* List this device */
> if (add_error_device(e_info, dev)) {
> /* We cannot handle more... Stop iteration */
> - /* TODO: Should print error message here? */
> + pr_err("find_device_iter: Cannot handle more devices.
> + Stopping iteration");
You are adding unnecessary whitespace after the period.
String concatenation keeps _all_ the whitespace.
The format is fine on a single line too.
Something like:
pr_notice("%s: Cannot handle more devices - iteration
stopped\n",
__func__);
Re: [PATCH v3 2/7] kexec_file: print out debugging message if required
On Thu, 2023-11-30 at 10:39 +0800, Baoquan He wrote:
> Replace pr_debug() with the newly added kexec_dprintk() in kexec_file
> loading related codes.
trivia:
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
[]
> @@ -551,9 +551,12 @@ int crash_prepare_elf64_headers(struct crash_mem *mem,
> int need_kernel_map,
> phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
> phdr->p_align = 0;
> ehdr->e_phnum++;
> - pr_debug("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx,
> paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
> - phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
> - ehdr->e_phnum, phdr->p_offset);
> +#ifdef CONFIG_KEXEC_FILE
> + kexec_dprintk("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx,
> paddr=0x%llx, "
> + "sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
> + phdr, phdr->p_vaddr, phdr->p_paddr,
> phdr->p_filesz,
> + ehdr->e_phnum, phdr->p_offset);
> +#endif
Perhaps run checkpatch and coalesce the format string.
Re: [PATCH v2 2/7] kexec_file: print out debugging message if required
On Fri, 2023-11-24 at 11:36 +0800, Baoquan He wrote:
> Replace pr_debug() with the newly added kexec_dprintk() in kexec_file
> loading related codes.
trivia for pr_debug -> kexec_dprintk conversions for
the entire patch set:
> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
[]
> @@ -551,9 +551,12 @@ int crash_prepare_elf64_headers(struct crash_mem *mem,
> int need_kernel_map,
> phdr->p_filesz = phdr->p_memsz = mend - mstart + 1;
> phdr->p_align = 0;
> ehdr->e_phnum++;
> - pr_debug("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx,
> paddr=0x%llx, sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
> +#ifdef CONFIG_KEXEC_FILE
> + kexec_dprintk("Crash PT_LOAD ELF header. phdr=%p vaddr=0x%llx,
> paddr=0x%llx, "
> + "sz=0x%llx e_phnum=%d p_offset=0x%llx\n",
> phdr, phdr->p_vaddr, phdr->p_paddr, phdr->p_filesz,
> ehdr->e_phnum, phdr->p_offset);
It's good form to rewrap continuation lines to the open parenthesis
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
[]
> @@ -389,11 +391,12 @@ SYSCALL_DEFINE5(kexec_file_load, int, kernel_fd, int,
> initrd_fd,
> if (ret)
> goto out;
>
> + kexec_dprintk("nr_segments = %lu\n", image->nr_segments);
> for (i = 0; i < image->nr_segments; i++) {
> struct kexec_segment *ksegment;
>
> ksegment = &image->segment[i];
> - pr_debug("Loading segment %d: buf=0x%p bufsz=0x%zx mem=0x%lx
> memsz=0x%zx\n",
> + kexec_dprintk("segment[%d]: buf=0x%p bufsz=0x%zx mem=0x%lx
> memsz=0x%zx\n",
>i, ksegment->buf, ksegment->bufsz, ksegment->mem,
>ksegment->memsz);
here too etc...
Re: [PATCH 1/7] kexec_file: add kexec_file flag to control debug printing
On Tue, 2023-11-14 at 23:32 +0800, Baoquan He wrote:
> When specifying 'kexec -c -d', kexec_load interface will print loading
> information, e.g the regions where kernel/initrd/purgatory/cmdline
> are put, the memmap passed to 2nd kernel taken as system RAM ranges,
> and printing all contents of struct kexec_segment, etc. These are
> very helpful for analyzing or positioning what's happening when
> kexec/kdump itself failed. The debugging printing for kexec_load
> interface is made in user space utility kexec-tools.
>
> Whereas, with kexec_file_load interface, 'kexec -s -d' print nothing.
> Because kexec_file code is mostly implemented in kernel space, and the
> debugging printing functionality is missed. It's not convenient when
> debugging kexec/kdump loading and jumping with kexec_file_load
> interface.
>
> Now add KEXEC_FILE_DEBUG to kexec_file flag to control the debugging
> message printing. And add global variable kexec_file_dbg_print and
> macro kexec_dprintk() to facilitate the printing.
>
> This is a preparation, later kexec_dprintk() will be used to replace the
> existing pr_debug(). Once 'kexec -s -d' is specified, it will print out
> kexec/kdump loading information. If '-d' is not specified, it regresses
> to pr_debug().
Not quite as pr_debug is completely eliminated with
zero object size when DEBUG is not #defined.
Now the object size will be larger and contain the
formats in .text.
[]
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
[]
> @@ -264,6 +264,18 @@ arch_kexec_apply_relocations(struct purgatory_info *pi,
> Elf_Shdr *section,
> return -ENOEXEC;
> }
> #endif
> +
> +extern bool kexec_file_dbg_print;
> +
> +#define kexec_dprintk(fmt, args...) \
> + do {\
> + if (kexec_file_dbg_print) \
> + printk(KERN_INFO fmt, ##args); \
> + else\
> + printk(KERN_DEBUG fmt, ##args); \
> + } while (0)
> +
> +
I don't know how many of these printks exist and if
overall object size matters but using
#define kexec_dprintkfmt, ...) \
printk("%s" fmt,\
kexec_file_dbg_print ? KERN_INFO : KERN_DEBUG, \
##__VA_ARGS__)
should reduce overall object size by eliminating the
mostly duplicated format in .text which differs only
by the KERN_
Re: [PATCH v2 09/10] powerpc: Use ppc_md_progress()
On Sat, 2023-02-18 at 10:15 +0100, Christophe Leroy wrote:
> Many places have:
>
> if (ppc.md_progress)
> ppc.md_progress();
>
> Use ppc_md_progress() instead.
>
> Note that checkpatch complains about using function names,
> but this is not a function taking format strings, so we
> leave the function names for now.
If you are changing almost all of these uses, why not
drop the unused 2nd argument 'hex' at the same time?
void ppc_printk_progress(char *s, unsigned short hex)
{
pr_info("%s\n", s);
}
> diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
[]
> @@ -127,8 +127,7 @@ __setup("l3cr=", ppc_setup_l3cr);
> static int __init ppc_init(void)
> {
> /* clear the progress line */
> - if (ppc_md.progress)
> - ppc_md.progress(" ", 0x);
> + ppc_md_progress(" ", 0x);
ppc_md_progress(" ");
Although this example seems especially useless.
> diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
[]
> @@ -347,7 +347,7 @@ void __init MMU_init_hw(void)
> if (!mmu_has_feature(MMU_FTR_HPTE_TABLE))
> return;
>
> - if ( ppc_md.progress ) ppc_md.progress("hash:enter", 0x105);
> + ppc_md_progress("hash:enter", 0x105);
ppc_md_progress("hash:enter");
[]
> diff --git a/arch/powerpc/platforms/52xx/efika.c
> b/arch/powerpc/platforms/52xx/efika.c
[]
> @@ -189,8 +189,7 @@ static void __init efika_setup_arch(void)
> mpc52xx_pm_init();
> #endif
>
> - if (ppc_md.progress)
> - ppc_md.progress("Linux/PPC " UTS_RELEASE " running on Efika
> ;-)\n", 0x0);
> + ppc_md_progress("Linux/PPC " UTS_RELEASE " running on Efika ;-)\n",
> 0x0);
And perhaps remove the extra newlines here and other places as
it doesn't seem useful to have unnecessary blank lines in the log.
Re: [PATCH AUTOSEL 4.9 1/3] powerpc/selftests: Use timersub() for gettimeofday()
On Fri, 2022-10-14 at 09:54 -0400, Sasha Levin wrote:
> From: ye xingchen
>
> [ Upstream commit c814bf958926ff45a9c1e899bd001006ab6cfbae ]
>
> Use timersub() function to simplify the code.
Why should a code simplification be backported?
>
> Reported-by: Zeal Robot
> Signed-off-by: ye xingchen
> Signed-off-by: Michael Ellerman
> Link: https://lore.kernel.org/r/20220816105106.82666-1-ye.xingc...@zte.com.cn
> Signed-off-by: Sasha Levin
> ---
> tools/testing/selftests/powerpc/benchmarks/gettimeofday.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/powerpc/benchmarks/gettimeofday.c
> b/tools/testing/selftests/powerpc/benchmarks/gettimeofday.c
> index 3af3c21e8036..7f4bb84f1c9c 100644
> --- a/tools/testing/selftests/powerpc/benchmarks/gettimeofday.c
> +++ b/tools/testing/selftests/powerpc/benchmarks/gettimeofday.c
> @@ -12,7 +12,7 @@ static int test_gettimeofday(void)
> {
> int i;
>
> - struct timeval tv_start, tv_end;
> + struct timeval tv_start, tv_end, tv_diff;
>
> gettimeofday(&tv_start, NULL);
>
> @@ -20,7 +20,9 @@ static int test_gettimeofday(void)
> gettimeofday(&tv_end, NULL);
> }
>
> - printf("time = %.6f\n", tv_end.tv_sec - tv_start.tv_sec +
> (tv_end.tv_usec - tv_start.tv_usec) * 1e-6);
> + timersub(&tv_start, &tv_end, &tv_diff);
> +
> + printf("time = %.6f\n", tv_diff.tv_sec + (tv_diff.tv_usec) * 1e-6);
>
> return 0;
> }
Re: [PATCH v1 3/5] treewide: use get_random_u32() when possible
On Wed, 2022-10-12 at 21:29 +, David Laight wrote: > From: Joe Perches > > Sent: 12 October 2022 20:17 > > > > On Wed, 2022-10-05 at 23:48 +0200, Jason A. Donenfeld wrote: > > > The prandom_u32() function has been a deprecated inline wrapper around > > > get_random_u32() for several releases now, and compiles down to the > > > exact same code. Replace the deprecated wrapper with a direct call to > > > the real function. > > [] > > > diff --git a/drivers/infiniband/hw/cxgb4/cm.c > > > b/drivers/infiniband/hw/cxgb4/cm.c > > [] > > > @@ -734,7 +734,7 @@ static int send_connect(struct c4iw_ep *ep) > > > &ep->com.remote_addr; > > > int ret; > > > enum chip_type adapter_type = ep->com.dev->rdev.lldi.adapter_type; > > > - u32 isn = (prandom_u32() & ~7UL) - 1; > > > + u32 isn = (get_random_u32() & ~7UL) - 1; > > > > trivia: > > > > There are somewhat odd size mismatches here. > > > > I had to think a tiny bit if random() returned a value from 0 to 7 > > and was promoted to a 64 bit value then truncated to 32 bit. > > > > Perhaps these would be clearer as ~7U and not ~7UL > > That makes no difference - the compiler will generate the same code. True, more or less. It's more a question for the reader. > The real question is WTF is the code doing? True. > The '& ~7u' clears the bottom 3 bits. > The '- 1' then sets the bottom 3 bits and decrements the > (random) high bits. Right. > So is the same as get_random_u32() | 7. True, it's effectively the same as the upper 29 bits are random anyway and the bottom 3 bits are always set. > But I bet the coder had something else in mind. Likely. And it was also likely copy/pasted a few times.
Re: [PATCH v1 3/5] treewide: use get_random_u32() when possible
On Wed, 2022-10-05 at 23:48 +0200, Jason A. Donenfeld wrote:
> The prandom_u32() function has been a deprecated inline wrapper around
> get_random_u32() for several releases now, and compiles down to the
> exact same code. Replace the deprecated wrapper with a direct call to
> the real function.
[]
> diff --git a/drivers/infiniband/hw/cxgb4/cm.c
> b/drivers/infiniband/hw/cxgb4/cm.c
[]
> @@ -734,7 +734,7 @@ static int send_connect(struct c4iw_ep *ep)
> &ep->com.remote_addr;
> int ret;
> enum chip_type adapter_type = ep->com.dev->rdev.lldi.adapter_type;
> - u32 isn = (prandom_u32() & ~7UL) - 1;
> + u32 isn = (get_random_u32() & ~7UL) - 1;
trivia:
There are somewhat odd size mismatches here.
I had to think a tiny bit if random() returned a value from 0 to 7
and was promoted to a 64 bit value then truncated to 32 bit.
Perhaps these would be clearer as ~7U and not ~7UL
> struct net_device *netdev;
> u64 params;
>
> @@ -2469,7 +2469,7 @@ static int accept_cr(struct c4iw_ep *ep, struct sk_buff
> *skb,
> }
>
> if (!is_t4(adapter_type)) {
> - u32 isn = (prandom_u32() & ~7UL) - 1;
> + u32 isn = (get_random_u32() & ~7UL) - 1;
etc...
drivers/infiniband/hw/cxgb4/cm.c: u32 isn = (prandom_u32() & ~7UL) - 1;
drivers/infiniband/hw/cxgb4/cm.c: u32 isn = (prandom_u32() &
~7UL) - 1;
drivers/net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c:rpl5->iss =
cpu_to_be32((prandom_u32() & ~7UL) - 1);
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c: u32 isn = (prandom_u32() &
~7UL) - 1;
drivers/scsi/cxgbi/cxgb4i/cxgb4i.c: u32 isn = (prandom_u32() &
~7UL) - 1;
drivers/target/iscsi/cxgbit/cxgbit_cm.c:rpl5->iss =
cpu_to_be32((prandom_u32() & ~7UL) - 1);
Re: [PATCH v1 3/3] checkpatch: warn on usage of VM_BUG_ON() and other BUG variants
On Thu, 2022-09-22 at 19:05 -0700, John Hubbard wrote:
> On 9/20/22 05:23, David Hildenbrand wrote:
> > checkpatch does not point out that VM_BUG_ON() and friends should be
> > avoided, however, Linus notes:
> >
> > VM_BUG_ON() has the exact same semantics as BUG_ON. It is literally
> > no different, the only difference is "we can make the code smaller
> > because these are less important". [1]
> >
> > So let's warn on VM_BUG_ON() and other BUG variants as well. While at it,
> > make it clearer that the kernel really shouldn't be crashed.
> >
> > As there are some subsystem BUG macros that actually don't end up crashing
> > the kernel -- for example, KVM_BUG_ON() -- exclude these manually.
> >
> > [1]
> > https://lore.kernel.org/r/CAHk-=wg40eazofo16eviaj7mfqdhz2gvebvfsmf6gyzsprj...@mail.gmail.com
[]
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > @@ -4695,12 +4695,12 @@ sub process {
> > }
> > }
> >
> > -# avoid BUG() or BUG_ON()
> > - if ($line =~ /\b(?:BUG|BUG_ON)\b/) {
> > +# do not use BUG() or variants
> > + if ($line =~
> > /\b(?!AA_|BUILD_|DCCP_|IDA_|KVM_|RWLOCK_|snd_|SPIN_)(?:[a-zA-Z_]*_)?BUG(?:_ON)?(?:_[A-Z_]+)?\s*\(/)
> > {
>
> Should this be a separate patch? Adding a bunch of exceptions to the BUG()
> rules is
> a separate and distinct thing from adding VM_BUG_ON() and other *BUG*()
> variants to
> the mix.
Not in my opinion.
> > my $msg_level = \&WARN;
> > $msg_level = \&CHK if ($file);
> > &{$msg_level}("AVOID_BUG",
> > - "Avoid crashing the kernel - try using
> > WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr);
> > + "Do not crash the kernel unless it is
> > unavoidable - use WARN_ON_ONCE & recovery code (if reasonable) rather than
> > BUG() or variants.\n" . $herecurr);
>
> Here's a requested tweak, to clean up the output and fix punctuation:
>
> "Avoid crashing the kernel--use WARN_ON_ONCE() plus recovery code (if
> feasible) instead of BUG() or variants.\n" . $herecurr);
Fixing punctuation here would be removing the trailing period as checkpatch
only has periods for multi-sentence output messages.
And I think that "Do not crash" is a stronger statement than "Avoid crashing"
so I prefer the original suggestion but it's not a big deal either way.
Re: [PATCH] tty/hvc_opal: simplify if-if to if-else
On Sun, 2022-04-24 at 17:25 +0800, Wan Jiabing wrote:
> Use if and else instead of if(A) and if (!A).
[]
> diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
[]
> @@ -344,14 +344,15 @@ void __init hvc_opal_init_early(void)
> opal = of_find_node_by_path("/ibm,opal/consoles");
> if (opal)
> pr_devel("hvc_opal: Found consoles in new location\n");
> - if (!opal) {
> + else {
> opal = of_find_node_by_path("/ibm,opal");
> if (opal)
> pr_devel("hvc_opal: "
>"Found consoles in old location\n");
> + else
> + return;
A few things:
o add {} braces to first block before else
o see about using pr_fmt to prefix the pr_ output
o reverse the test and unindent the pr_devel
if (!opal)
return;
pr_devel("...);
Maybe a few more just to quiet checkpatch noise. Something like:
---
drivers/tty/hvc/hvc_opal.c | 58 +-
1 file changed, 31 insertions(+), 27 deletions(-)
diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 84776bc641e6b..a42d5697ae198 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -5,6 +5,8 @@
* Copyright 2011 Benjamin Herrenschmidt , IBM Corp.
*/
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#undef DEBUG
#include
@@ -43,6 +45,7 @@ struct hvc_opal_priv {
hv_protocol_t proto; /* Raw data or HVSI packets */
struct hvsi_privhvsi; /* HVSI specific data */
};
+
static struct hvc_opal_priv *hvc_opal_privs[MAX_NR_HVC_CONSOLES];
/* For early boot console */
@@ -124,7 +127,7 @@ static int hvc_opal_hvsi_tiocmget(struct hvc_struct *hp)
}
static int hvc_opal_hvsi_tiocmset(struct hvc_struct *hp, unsigned int set,
- unsigned int clear)
+ unsigned int clear)
{
struct hvc_opal_priv *pv = hvc_opal_privs[hp->vtermno];
@@ -167,8 +170,7 @@ static int hvc_opal_probe(struct platform_device *dev)
proto = HV_PROTOCOL_HVSI;
ops = &hvc_opal_hvsi_ops;
} else {
- pr_err("hvc_opal: Unknown protocol for %pOF\n",
- dev->dev.of_node);
+ pr_err("Unknown protocol for %pOF\n", dev->dev.of_node);
return -ENXIO;
}
@@ -195,15 +197,16 @@ static int hvc_opal_probe(struct platform_device *dev)
termno, 0);
}
- /* Instanciate now to establish a mapping index==vtermno */
+ /* Instantiate now to establish a mapping index==vtermno */
hvc_instantiate(termno, termno, ops);
} else {
- pr_err("hvc_opal: Device %pOF has duplicate terminal number
#%d\n",
+ pr_err("Device %pOF has duplicate terminal number #%d\n",
dev->dev.of_node, termno);
return -ENXIO;
}
- pr_info("hvc%d: %s protocol on %pOF%s\n", termno,
+ pr_info("hvc%d: %s protocol on %pOF%s\n",
+ termno,
proto == HV_PROTOCOL_RAW ? "raw" : "hvsi",
dev->dev.of_node,
boot ? " (boot console)" : "");
@@ -211,13 +214,13 @@ static int hvc_opal_probe(struct platform_device *dev)
irq = irq_of_parse_and_map(dev->dev.of_node, 0);
if (!irq) {
pr_info("hvc%d: No interrupts property, using OPAL event\n",
- termno);
+ termno);
irq = opal_event_request(ilog2(OPAL_EVENT_CONSOLE_INPUT));
}
if (!irq) {
- pr_err("hvc_opal: Unable to map interrupt for device %pOF\n",
- dev->dev.of_node);
+ pr_err("Unable to map interrupt for device %pOF\n",
+ dev->dev.of_node);
return irq;
}
@@ -275,7 +278,7 @@ static void udbg_opal_putc(char c)
udbg_opal_putc('\r');
do {
- switch(hvc_opal_boot_priv.proto) {
+ switch (hvc_opal_boot_priv.proto) {
case HV_PROTOCOL_RAW:
count = opal_put_chars(termno, &c, 1);
break;
@@ -288,7 +291,7 @@ static void udbg_opal_putc(char c)
* when there aren't any interrupts.
*/
opal_flush_console(termno);
- } while(count == 0 || count == -EAGAIN);
+ } while (count == 0 || count == -EAGAIN);
}
static int udbg_opal_getc_poll(void)
@@ -297,7 +300,7 @@ static int udbg_opal_getc_poll(void)
int rc = 0;
char c;
- switch(hvc_opal_boot_priv.proto) {
+ switch (hvc_opal_boot_priv.proto) {
Re: [PATCH 16/22] dvb-usb: Replace comments with C99 initializers
On Sat, 2022-03-26 at 19:27 +0100, Mauro Carvalho Chehab wrote:
> Em Sat, 26 Mar 2022 19:24:54 +0100
> Mauro Carvalho Chehab escreveu:
>
> > Em Sat, 26 Mar 2022 17:59:03 +0100
> > Benjamin Stürz escreveu:
> >
> > > This replaces comments with C99's designated
> > > initializers because the kernel supports them now.
> > >
> > > Signed-off-by: Benjamin Stürz
> > > ---
> > > drivers/media/usb/dvb-usb/dibusb-mb.c | 62 +--
> > > drivers/media/usb/dvb-usb/dibusb-mc.c | 34 +++
> > > 2 files changed, 48 insertions(+), 48 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/dvb-usb/dibusb-mb.c
> > > b/drivers/media/usb/dvb-usb/dibusb-mb.c
> > > index e9dc27f73970..f188e07f518b 100644
> > > --- a/drivers/media/usb/dvb-usb/dibusb-mb.c
> > > +++ b/drivers/media/usb/dvb-usb/dibusb-mb.c
> > > @@ -122,40 +122,40 @@ static int dibusb_probe(struct usb_interface *intf,
> > >
> > > /* do not change the order of the ID table */
> > > static struct usb_device_id dibusb_dib3000mb_table [] = {
> > > -/* 00 */ { USB_DEVICE(USB_VID_WIDEVIEW,
> > > USB_PID_AVERMEDIA_DVBT_USB_COLD) },
> > > -/* 01 */ { USB_DEVICE(USB_VID_WIDEVIEW,
> > > USB_PID_AVERMEDIA_DVBT_USB_WARM) },
> > > -/* 02 */ { USB_DEVICE(USB_VID_COMPRO,
> > > USB_PID_COMPRO_DVBU2000_COLD) },
> > > -/* 03 */ { USB_DEVICE(USB_VID_COMPRO,
> > > USB_PID_COMPRO_DVBU2000_WARM) },
> > > -/* 04 */ { USB_DEVICE(USB_VID_COMPRO_UNK,
> > > USB_PID_COMPRO_DVBU2000_UNK_COLD) },
> > > -/* 05 */ { USB_DEVICE(USB_VID_DIBCOM,
> > > USB_PID_DIBCOM_MOD3000_COLD) },
> > > -/* 06 */ { USB_DEVICE(USB_VID_DIBCOM,
> > > USB_PID_DIBCOM_MOD3000_WARM) },
> > > -/* 07 */ { USB_DEVICE(USB_VID_EMPIA,
> > > USB_PID_KWORLD_VSTREAM_COLD) },
> > > -/* 08 */ { USB_DEVICE(USB_VID_EMPIA,
> > > USB_PID_KWORLD_VSTREAM_WARM) },
> > > -/* 09 */ { USB_DEVICE(USB_VID_GRANDTEC,
> > > USB_PID_GRANDTEC_DVBT_USB_COLD) },
> > > -/* 10 */ { USB_DEVICE(USB_VID_GRANDTEC,
> > > USB_PID_GRANDTEC_DVBT_USB_WARM) },
> > > -/* 11 */ { USB_DEVICE(USB_VID_GRANDTEC,
> > > USB_PID_DIBCOM_MOD3000_COLD) },
> > > -/* 12 */ { USB_DEVICE(USB_VID_GRANDTEC,
> > > USB_PID_DIBCOM_MOD3000_WARM) },
> > > -/* 13 */ { USB_DEVICE(USB_VID_HYPER_PALTEK,
> > > USB_PID_UNK_HYPER_PALTEK_COLD) },
> > > -/* 14 */ { USB_DEVICE(USB_VID_HYPER_PALTEK,
> > > USB_PID_UNK_HYPER_PALTEK_WARM) },
> > > -/* 15 */ { USB_DEVICE(USB_VID_VISIONPLUS,
> > > USB_PID_TWINHAN_VP7041_COLD) },
> > > -/* 16 */ { USB_DEVICE(USB_VID_VISIONPLUS,
> > > USB_PID_TWINHAN_VP7041_WARM) },
> > > -/* 17 */ { USB_DEVICE(USB_VID_TWINHAN,
> > > USB_PID_TWINHAN_VP7041_COLD) },
> > > -/* 18 */ { USB_DEVICE(USB_VID_TWINHAN,
> > > USB_PID_TWINHAN_VP7041_WARM) },
> > > -/* 19 */ { USB_DEVICE(USB_VID_ULTIMA_ELECTRONIC,
> > > USB_PID_ULTIMA_TVBOX_COLD) },
> > > -/* 20 */ { USB_DEVICE(USB_VID_ULTIMA_ELECTRONIC,
> > > USB_PID_ULTIMA_TVBOX_WARM) },
> > > -/* 21 */ { USB_DEVICE(USB_VID_ULTIMA_ELECTRONIC,
> > > USB_PID_ULTIMA_TVBOX_AN2235_COLD) },
> > > -/* 22 */ { USB_DEVICE(USB_VID_ULTIMA_ELECTRONIC,
> > > USB_PID_ULTIMA_TVBOX_AN2235_WARM) },
> > > -/* 23 */ { USB_DEVICE(USB_VID_ADSTECH,
> > > USB_PID_ADSTECH_USB2_COLD) },
> > > +[0] = { USB_DEVICE(USB_VID_WIDEVIEW,
> > > USB_PID_AVERMEDIA_DVBT_USB_COLD) },
> > > +[1] = { USB_DEVICE(USB_VID_WIDEVIEW,
> > > USB_PID_AVERMEDIA_DVBT_USB_WARM) },
> >
> > While here, please properly indent this table, and respect the 80-columns
> > limit,
> > e. g.:
> >
> > static struct usb_device_id dibusb_dib3000mb_table [] = {
> > [0] = { USB_DEVICE(USB_VID_WIDEVIEW
> >USB_PID_AVERMEDIA_DVBT_USB_COLD)
> > },
> > [1] = { USB_DEVICE(USB_VID_WIDEVIEW,
> > USB_PID_AVERMEDIA_DVBT_USB_WARM)
> > },
> > ...
>
> Err something went wrong with my space bar and I ended hitting send to
> soon... I meant:
>
> static struct usb_device_id dibusb_dib3000mb_table [] = {
> [0] = { USB_DEVICE(USB_VID_WIDEVIEW
> USB_PID_AVERMEDIA_DVBT_USB_COLD)
> },
> [1] = { USB_DEVICE(USB_VID_WIDEVIEW,
> USB_PID_AVERMEDIA_DVBT_USB_WARM)
> },
> ...
> };
maybe static const too
and
maybe
#define DIB_DEVICE(vid, pid)\
{ USB_DEVICE(USB_VID_ ## vid, USB_PID_ ## pid) }
so maybe
static const struct usb_device_id dibusb_dib3000mb_table[] = {
[0] = DIB_DEVICE(WIDEVIEW, AVERMEDIA_DVBT_USB_COLD),
[1] = DIB_DEVICE(WIDEVIEW, AVERMEDIA_DVBT_USB_WARM),
...
};
though I _really_ doubt the value of the specific indexing.
I think this isn't really worth changing at all.
Re: [PATCH 02/22] s3c: Replace comments with C99 initializers
On Sat, 2022-03-26 at 17:58 +0100, Benjamin Stürz wrote:
> This replaces comments with C99's designated
> initializers because the kernel supports them now.
[]
> diff --git a/arch/arm/mach-s3c/bast-irq.c b/arch/arm/mach-s3c/bast-irq.c
[]
> @@ -29,22 +29,22 @@
> * the irq is not implemented
> */
> static const unsigned char bast_pc104_irqmasks[] = {
> - 0, /* 0 */
> - 0, /* 1 */
> - 0, /* 2 */
> - 1, /* 3 */
> - 0, /* 4 */
> - 2, /* 5 */
> - 0, /* 6 */
> - 4, /* 7 */
> - 0, /* 8 */
> - 0, /* 9 */
> - 8, /* 10 */
> - 0, /* 11 */
> - 0, /* 12 */
> - 0, /* 13 */
> - 0, /* 14 */
> - 0, /* 15 */
> + [0] = 0,
> + [1] = 0,
> + [2] = 0,
> + [3] = 1,
> + [4] = 0,
> + [5] = 2,
> + [6] = 0,
> + [7] = 4,
> + [8] = 0,
> + [9] = 0,
> + [10] = 8,
> + [11] = 0,
> + [12] = 0,
> + [13] = 0,
> + [14] = 0,
> + [15] = 0,
> };
I don't find this better than the initial array.
>
> static const unsigned char bast_pc104_irqs[] = { 3, 5, 7, 10 };
For the same reason this array is just an array
without the specified indexing.
Re: [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body
On Mon, 2022-02-28 at 14:24 +0300, Dan Carpenter wrote: > a multi-line indent gets curly braces for readability even though > it's not required by C. And then both sides would get curly braces. That's more your personal preference than a coding style guideline.
Re: [PATCH net v3] net: Force inlining of checksum functions in net/checksum.h
On Thu, 2022-02-17 at 13:19 +0100, Christophe Leroy wrote:
> All functions defined as static inline in net/checksum.h are
> meant to be inlined for performance reason.
>
> But since commit ac7c3e4ff401 ("compiler: enable
> CONFIG_OPTIMIZE_INLINING forcibly") the compiler is allowed to
> uninline functions when it wants.
>
> Fair enough in the general case, but for tiny performance critical
> checksum helpers that's counter-productive.
Thanks. Trivial style notes:
> diff --git a/include/net/checksum.h b/include/net/checksum.h
[]
> @@ -22,7 +22,7 @@
> #include
>
> #ifndef _HAVE_ARCH_COPY_AND_CSUM_FROM_USER
> -static inline
> +static __always_inline
> __wsum csum_and_copy_from_user (const void __user *src, void *dst,
> int len)
> {
__wsum might be better placed on the previous line.
[]
> @@ -45,7 +45,7 @@ static __inline__ __wsum csum_and_copy_to_user
> #endif
>
> #ifndef _HAVE_ARCH_CSUM_AND_COPY
> -static inline __wsum
> +static __always_inline __wsum
> csum_partial_copy_nocheck(const void *src, void *dst, int len)
To be consistent with the location of the __wsum return value
when splitting the function definitions across multiple lines.
(like the below)
> @@ -88,42 +88,43 @@ static inline __wsum csum_shift(__wsum sum, int offset)
> return sum;
> }
>
> -static inline __wsum
> +static __always_inline __wsum
> csum_block_add(__wsum csum, __wsum csum2, int offset)
> {
> return csum_add(csum, csum_shift(csum2, offset));
> }
>
> -static inline __wsum
> +static __always_inline __wsum
> csum_block_add_ext(__wsum csum, __wsum csum2, int offset, int len)
> {
> return csum_block_add(csum, csum2, offset);
> }
>
> -static inline __wsum
> +static __always_inline __wsum
> csum_block_sub(__wsum csum, __wsum csum2, int offset)
> {
> return csum_block_add(csum, ~csum2, offset);
> }
>
> -static inline __wsum csum_unfold(__sum16 n)
> +static __always_inline __wsum csum_unfold(__sum16 n)
> {
> return (__force __wsum)n;
> }
>
[]
> -static inline __wsum csum_partial_ext(const void *buff, int len, __wsum sum)
> +static __always_inline
> +__wsum csum_partial_ext(const void *buff, int len, __wsum sum)
> {
> return csum_partial(buff, len, sum);
> }
And this __wsum could be moved too.
> @@ -150,15 +151,15 @@ void inet_proto_csum_replace16(__sum16 *sum, struct
> sk_buff *skb,
[]
> -static inline __wsum remcsum_adjust(void *ptr, __wsum csum,
> +static __always_inline __wsum remcsum_adjust(void *ptr, __wsum csum,
> int start, int offset)
> {
> __sum16 *psum = (__sum16 *)(ptr + offset);
And this one could be split like the above
static __always_inline __wsum
remcsum_adjust(void *ptr, __wsum csum, int start, int offset)
Re: [PATCH] scsi: ibmvfc: replace snprintf with sysfs_emit
On Wed, 2022-02-09 at 08:43 +0800, davidcomponent...@gmail.com wrote: > From: Yang Guang [] > diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c [] > @@ -3403,7 +3403,7 @@ static ssize_t ibmvfc_show_host_partition_name(struct > device *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.partition_name); Rewrap please return sysfs_emit(buf, "%s\n", vhost->login_buf->resp.partition_name); > } > > @@ -3413,7 +3413,7 @@ static ssize_t ibmvfc_show_host_device_name(struct > device *dev, > struct Scsi_Host *shost = class_to_shost(dev); > struct ibmvfc_host *vhost = shost_priv(shost); > > - return snprintf(buf, PAGE_SIZE, "%s\n", > + return sysfs_emit(buf, "%s\n", > vhost->login_buf->resp.device_name); etc...
Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
On Sun, 2021-11-28 at 09:43 -0800, Yury Norov wrote:
> On Sun, Nov 28, 2021 at 09:07:52AM -0800, Joe Perches wrote:
> > On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> > > Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> > > with one of new functions where appropriate. This allows num_*_cpus_*()
> > > to return earlier depending on the condition.
> > []
> > > diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
> > []
> > > @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> > >* if platform didn't set the present map already, do it now
> > >* boot cpu is set to present already by init/main.c
> > >*/
> > > - if (num_present_cpus() <= 1)
> > > + if (num_present_cpus_le(2))
> > > init_cpu_present(cpu_possible_mask);
> >
> > ? is this supposed to be 2 or 1
>
> X <= 1 is the equivalent of X < 2.
True. The call though is _le not _lt
Re: [PATCH 7/9] lib/cpumask: add num_{possible,present,active}_cpus_{eq,gt,le}
On Sat, 2021-11-27 at 19:57 -0800, Yury Norov wrote:
> Add num_{possible,present,active}_cpus_{eq,gt,le} and replace num_*_cpus()
> with one of new functions where appropriate. This allows num_*_cpus_*()
> to return earlier depending on the condition.
[]
> diff --git a/arch/arc/kernel/smp.c b/arch/arc/kernel/smp.c
[]
> @@ -103,7 +103,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>* if platform didn't set the present map already, do it now
>* boot cpu is set to present already by init/main.c
>*/
> - if (num_present_cpus() <= 1)
> + if (num_present_cpus_le(2))
> init_cpu_present(cpu_possible_mask);
? is this supposed to be 2 or 1
> diff --git a/drivers/cpufreq/pcc-cpufreq.c b/drivers/cpufreq/pcc-cpufreq.c
[]
> @@ -593,7 +593,7 @@ static int __init pcc_cpufreq_init(void)
> return ret;
> }
>
> - if (num_present_cpus() > 4) {
> + if (num_present_cpus_gt(4)) {
> pcc_cpufreq_driver.flags |= CPUFREQ_NO_AUTO_DYNAMIC_SWITCHING;
> pr_err("%s: Too many CPUs, dynamic performance scaling
> disabled\n",
> __func__);
It looks as if the present variants should be using the same values
so the _le test above with 1 changed to 2 looks odd.
Re: [PATCH] macintosh: no need to initilise statics to 0
On Tue, 2021-08-17 at 13:59 +0200, Christophe Leroy wrote: > > Le 17/08/2021 à 13:51, Jason Wang a écrit : > > Global static variables dont need to be initialised to 0. Because > > the compiler will initilise them. > > It is not the compiler, it is the Kernel. It is done here: > > https://elixir.bootlin.com/linux/v5.14-rc6/source/arch/powerpc/kernel/early_32.c I don't know why that's done generally. >From memory, it's also required by the c spec unless it's for a union where the first union member is smaller in size than other members.
Re: [PATCH 6/9] tty: hvc_console: Fix coding style issues of block comments
On Thu, 2021-05-20 at 10:21 +0200, Johan Hovold wrote:
> On Tue, May 18, 2021 at 12:01:22PM +0800, Xiaofei Tan wrote:
> > On 2021/5/17 22:15, Johan Hovold wrote:
> > > On Mon, May 17, 2021 at 02:37:10PM +0800, Xiaofei Tan wrote:
> > > > Fix coding style issues of block comments, reported by checkpatch.pl.
> > > > Besides, add a period at the end of the sentenses.
[]
> > > > diff --git a/drivers/tty/hvc/hvc_console.c
> > > > b/drivers/tty/hvc/hvc_console.c
[]
> > > > @@ -177,7 +177,8 @@ static void hvc_console_print(struct console *co,
> > > > const char *b,
> > > > r = cons_ops[index]->put_chars(vtermnos[index],
> > > > c, i);
> > > > if (r <= 0) {
> > > > /* throw away characters on error
> > > > -* but spin in case of -EAGAIN */
> > > > +* but spin in case of -EAGAIN.
> > > > +*/
> > >
> > > How is this an improvement? First, the multi-line comment style is
> > >
> > > /*
> > >* ...
> > >*/
> > >
> >
> > Yes, mostly we use this style. I can follow it if new version is needed.
>
> This is the preferred style outside of networking.
>
> > BTW, How about add the '/*' check into checkpatch.pl?
>
> Checkpatch already has too many checks IMO
I sometimes agree. What checkpatch messages do you think are excessive?
> and I'm a bit surprised that
> it doesn't check this already. Perhaps it's because you used the -f to
> run checkpatch on in-kernel code, which you should not.
Likely not. If it was run on a suggested patch, checkpatch doesn't emit
many messages on unmodified patch context lines. And it shouldn't.
> it's just that you
> introduce noise in the logs and do pointless changes of context which
> makes it harder to use tools like git blame and makes backporting harder
> for no good reason.
Pretty pointless metric IMO. Context changes in comments are mostly harmless.
IMO: backporting of these sorts non-bug fix changes is done _far_ too often.
Re: [PATCH] Raise the minimum GCC version to 5.2
On Sat, 2021-05-15 at 09:14 +0200, Pavel Machek wrote: > On Sun 2021-05-02 00:15:38, Masahiro Yamada wrote: > > The current minimum GCC version is 4.9 except ARCH=arm64 requiring > > GCC 5.1. > > Please don't. I'm still on 4.9 on machine I can't easily update, Why is that? Later compiler versions are available. http://cdn.kernel.org/pub/tools/crosstool/ Is there some other reason your machine can not have the compiler version updated?
Re: [PATCH v2 04/14] PCI/MSI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
On Sat, 2021-05-15 at 05:24 +, Krzysztof Wilczyński wrote: > The sysfs_emit() and sysfs_emit_at() functions were introduced to make > it less ambiguous which function is preferred when writing to the output > buffer in a device attribute's "show" callback [1]. > > Convert the PCI sysfs object "show" functions from sprintf(), snprintf() > and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the > latter is aware of the PAGE_SIZE buffer and correctly returns the number > of bytes written into the buffer. > > No functional change intended. [] > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c [] > @@ -465,8 +465,8 @@ static ssize_t msi_mode_show(struct device *dev, struct > device_attribute *attr, > > entry = irq_get_msi_desc(irq); > if (entry) > - return sprintf(buf, "%s\n", > - entry->msi_attrib.is_msix ? "msix" : "msi"); > + return sysfs_emit(buf, "%s\n", > + entry->msi_attrib.is_msix ? "msix" : "msi"); > > > return -ENODEV; > } trivia: reversing the test would be more common style if (!entry) return -ENODEV; return sysfs_emit(...); }
Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
On Sat, 2021-05-15 at 05:24 +, Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
>
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
[]
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
[]
> @@ -6439,7 +6439,7 @@ static ssize_t resource_alignment_show(struct bus_type
> *bus, char *buf)
>
>
> spin_lock(&resource_alignment_lock);
> if (resource_alignment_param)
> - count = scnprintf(buf, PAGE_SIZE, "%s",
> resource_alignment_param);
> + count = sysfs_emit(buf, "%s", resource_alignment_param);
> spin_unlock(&resource_alignment_lock);
Ideally, the additional newline check below this would use sysfs_emit_at
drivers/pci/pci.c- /*
drivers/pci/pci.c: * When set by the command line,
resource_alignment_param will not
drivers/pci/pci.c- * have a trailing line feed, which is ugly. So
conditionally add
drivers/pci/pci.c- * it here.
drivers/pci/pci.c- */
drivers/pci/pci.c- if (count >= 2 && buf[count - 2] != '\n' && count <
PAGE_SIZE - 1) {
drivers/pci/pci.c- buf[count - 1] = '\n';
drivers/pci/pci.c- buf[count++] = 0;
drivers/pci/pci.c- }
drivers/pci/pci.c-
drivers/pci/pci.c- return count;
Re: [PATCH] Raise the minimum GCC version to 5.2
On Mon, 2021-05-03 at 09:34 +0200, Alexander Dahl wrote: > Desktops and servers are all nice, however I just want to make you > aware, there are embedded users forced to stick to older cross > toolchains for different reasons as well, e.g. in industrial > environment. :-) In your embedded case, what kernel version do you use? For older toolchains, unless it's kernel version 5.13+, it wouldn't matter. And all the supported architectures have gcc 10.3 available at http://cdn.kernel.org/pub/tools/crosstool/
Re: [PATCH] Raise the minimum GCC version to 5.2
On Sun, 2021-05-02 at 15:32 -0500, Segher Boessenkool wrote: > On Sun, May 02, 2021 at 01:00:28PM -0700, Joe Perches wrote: [] > > Perhaps 8 might be best as that has a __diag warning control mechanism. > > I have no idea what you mean? ? read the last bit of compiler-gcc.h
Re: [PATCH] Raise the minimum GCC version to 5.2
On Sun, 2021-05-02 at 13:30 -0500, Segher Boessenkool wrote: > On Sat, May 01, 2021 at 07:41:53PM -0700, Joe Perches wrote: > > Why not raise the minimum gcc compiler version even higher? On Sun, 2021-05-02 at 13:37 -0500, Segher Boessenkool wrote: > Everyone should always use an as new release as practical [] > The latest GCC 5 release is only three and a half years old. You argue slightly against yourself here. Yes, it's mostly a question of practicality vs latest. clang requires a _very_ recent version. gcc _could_ require a later version. Perhaps 8 might be best as that has a __diag warning control mechanism. gcc 8.1 is now 3 years old today.
Re: [PATCH] Raise the minimum GCC version to 5.2
On Sat, 2021-05-01 at 17:52 +0200, Miguel Ojeda wrote: > On Sat, May 1, 2021 at 5:17 PM Masahiro Yamada wrote: > > > > More cleanups will be possible as follow-up patches, but this one must > > be agreed and applied to the mainline first. > > +1 This will allow me to remove the __has_attribute hack in > include/linux/compiler_attributes.h. Why not raise the minimum gcc compiler version even higher? https://gcc.gnu.org/releases.html
Re: [PATCH v15 09/10] arm64: Call kmalloc() to allocate DTB buffer
On Thu, 2021-01-28 at 00:52 -0300, Thiago Jung Bauermann wrote: > The problem is that this patch implements only part of the suggestion, > which isn't useful in itself. So the patch series should either drop > this patch or consolidate the FDT allocation between the arches. > > I just tested on powernv and pseries platforms and powerpc can use > vmalloc for the FDT buffer. Perhaps more sensible to use kvmalloc/kvfree.
Re: [PATCH] powerpc/xmon: Change printk() to pr_cont()
On Fri, 2020-12-04 at 21:56 +1100, Michael Ellerman wrote:
> Christophe Leroy writes:
> > Since some time now, printk() adds carriage return, leading to
> > unusable xmon output:
> >
> > [ 54.288722] sysrq: Entering xmon
> > [ 54.292209] Vector: 0 at [cace3d2c]
> > [ 54.292274] pc:
> > [ 54.292331] c0023650
>
> ...
>
> > diff --git a/arch/powerpc/xmon/nonstdio.c b/arch/powerpc/xmon/nonstdio.c
> > index 5c1a50912229..9b0d85bff021 100644
> > --- a/arch/powerpc/xmon/nonstdio.c
> > +++ b/arch/powerpc/xmon/nonstdio.c
> > @@ -178,7 +178,7 @@ void xmon_printf(const char *format, ...)
> >
> >
> > if (n && rc == 0) {
> > /* No udbg hooks, fallback to printk() - dangerous */
> > - printk("%s", xmon_outbuf);
> > + pr_cont("%s", xmon_outbuf);
> > }
>
> Ah OK, in the case where there's no udbg backend. We basically always
> have a udbg backend on 64-bit, via hvc console. Which explains why we
> haven't noticed it.
>
> Will pick up the patch.
>
> cheers
Perhaps all of these bare printks should be inspected for defects:
$ git grep -P -n '\bprintk\s*\(\s*(?!KERN_\w+)"[^\\n]*"' arch/powerpc
arch/powerpc/kernel/process.c:1475: printk("NIP: "REG" LR: "REG" CTR:
"REG"\n",
arch/powerpc/kernel/process.c:1479: printk("MSR: "REG" ", regs->msr);
arch/powerpc/kernel/process.c:1513: printk("NIP ["REG"] %pS\n",
regs->nip, (void *)regs->nip);
arch/powerpc/kernel/process.c:1514: printk("LR ["REG"] %pS\n",
regs->link, (void *)regs->link);
arch/powerpc/kernel/process.c:2157: printk("%s["REG"]
["REG"] %pS",
arch/powerpc/kernel/traps.c:621:printk("Caused by (from MCSR=%lx): ",
reason);
arch/powerpc/kernel/traps.c:726:printk("Caused by (from MCSR=%lx): ",
reason);
arch/powerpc/kernel/traps.c:766:printk("Caused by (from MCSR=%lx): ",
reason);
arch/powerpc/kernel/traps.c:791:printk("Caused by (from SRR1=%lx): ",
reason);
arch/powerpc/kernel/udbg.c:95: printk("%s", s);
arch/powerpc/math-emu/fabs.c:13:printk("%s: D %p, B %p: ", __func__,
frD, frB);
arch/powerpc/math-emu/fctiw.c:22: printk("%s: D %p, B %p: ", __func__,
frD, frB);
arch/powerpc/math-emu/fctiwz.c:29: printk("%s: D %p, B %p: ", __func__,
frD, frB);
arch/powerpc/math-emu/fmr.c:13: printk("%s: D %p, B %p: ", __func__, frD, frB);
arch/powerpc/math-emu/fnabs.c:13: printk("%s: D %p, B %p: ", __func__,
frD, frB);
arch/powerpc/math-emu/fneg.c:13:printk("%s: D %p, B %p: ", __func__,
frD, frB);
arch/powerpc/math-emu/lfd.c:15: printk("%s: D %p, ea %p: ", __func__, frD, ea);
arch/powerpc/math-emu/stfd.c:11:printk("%s: S %p, ea %p: ", __func__,
frS, ea);
arch/powerpc/mm/nohash/44x.c:192:
printk("%d ", i);
arch/powerpc/platforms/4xx/machine_check.c:19: printk("Data");
arch/powerpc/platforms/chrp/pci.c:256: printk(" at %llx",
(unsigned long long)r.start);
arch/powerpc/platforms/embedded6xx/ls_uart.c:47:
printk("%c", in_8(avr_addr + UART_RX));
arch/powerpc/platforms/powermac/pfunc_core.c:83:printk("%s", title);
arch/powerpc/platforms/powermac/pfunc_core.c:85:printk("%02x ",
*((u8 *)blob));
arch/powerpc/platforms/powernv/pci-ioda.c:81: printk("%spci %s: [PE# %.2x]
%pV",
arch/powerpc/sysdev/tsi108_pci.c:63:printk("PCI CFG write : ");
arch/powerpc/sysdev/tsi108_pci.c:64:printk("%d:0x%x:0x%x ", bus->number,
devfunc, offset);
arch/powerpc/sysdev/tsi108_pci.c:65:printk("%d ADDR=0x%08x ", len, (uint)
cfg_addr);
arch/powerpc/sysdev/tsi108_pci.c:164: printk("PCI CFG read : ");
arch/powerpc/sysdev/tsi108_pci.c:165: printk("%d:0x%x:0x%x ",
bus->number, devfn, offset);
arch/powerpc/sysdev/tsi108_pci.c:166: printk("%d ADDR=0x%08x ", len,
(uint) cfg_addr);
arch/powerpc/sysdev/tsi108_pci.c:315: printk("cfg_ctl=0x%08x ", temp);
arch/powerpc/xmon/nonstdio.c:181: printk("%s", xmon_outbuf);
Re: [PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Fri, 2020-10-23 at 08:08 +0200, Miguel Ojeda wrote:
> On Thu, Oct 22, 2020 at 4:36 AM Joe Perches wrote:
> >
> > Use a more generic form for __section that requires quotes to avoid
> > complications with clang and gcc differences.
>
> I performed visual inspection (one by one...) and the only thing I saw
> is that sometimes the `__attribute__` has a whitespace afterwards and
> sometimes it doesn't, same for the commas inside, e.g.:
>
> - __used __attribute__((section(".modinfo"), unused, aligned(1))) \
> + __used __section(".modinfo") __attribute__((unused, aligned(1))) \
>
> and:
>
> -__attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void *
> \
> +__section("__param") __attribute__ ((unused, aligned(sizeof(void * \
>
> I think the patch tries to follow the style of the replaced line, but
> for the commas in this last case it didn't. Anyway, it is not
> important.
Here the change follows the kernel style of space after comma.
> I can pick it up in my queue along with the __alias one and keep it
> for a few weeks in -next.
Thanks Miguel, but IMO it doesn't need time in next.
Applying it just before an rc1 minimizes conflicts.
Re: [PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Thu, 2020-10-22 at 13:42 -0700, Nick Desaulniers wrote: > .On Wed, Oct 21, 2020 at 7:36 PM Joe Perches wrote: > > Use a more generic form for __section that requires quotes to avoid > > complications with clang and gcc differences. [] > > a quick test of x86_64 and s390 would be good. x86_64 was compiled here. I believe the robot tested the others.
[PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
Use a more generic form for __section that requires quotes to avoid
complications with clang and gcc differences.
Remove the quote operator # from compiler_attributes.h __section macro.
Convert all unquoted __section(foo) uses to quoted __section("foo").
Also convert __attribute__((section("foo"))) uses to __section("foo")
even if the __attribute__ has multiple list entry forms.
Conversion done using a script:
Link:
https://lore.kernel.org/lkml/75393e5ddc272dc7403de74d645e6c6e0f4e70eb.ca...@perches.com/2-convert_section.pl
Signed-off-by: Joe Perches
---
This conversion was previously submitted to -next last month
https://lore.kernel.org/lkml/46f69161e60b802488ba8c8f3f8bbf922aa3b49b.ca...@perches.com/
Nick Desaulniers found a defect in the conversion of 2 boot files
for powerpc, but no other defect was found for any other arch.
The script was corrected to avoid converting these 2 files.
There is no difference between the script output when run on today's -next
and Linus' tree through commit f804b3159482, so this should be reasonable to
apply now.
arch/arc/include/asm/linkage.h| 8 +++
arch/arc/include/asm/mach_desc.h | 2 +-
arch/arc/plat-hsdk/platform.c | 2 +-
arch/arm/include/asm/cache.h | 2 +-
arch/arm/include/asm/cpuidle.h| 2 +-
arch/arm/include/asm/idmap.h | 2 +-
arch/arm/include/asm/mach/arch.h | 4 ++--
arch/arm/include/asm/setup.h | 2 +-
arch/arm/include/asm/smp.h| 2 +-
arch/arm/include/asm/tcm.h| 8 +++
arch/arm/kernel/cpuidle.c | 2 +-
arch/arm/kernel/devtree.c | 2 +-
arch/arm64/include/asm/cache.h| 2 +-
arch/arm64/kernel/efi.c | 2 +-
arch/arm64/kernel/smp_spin_table.c| 2 +-
arch/arm64/mm/mmu.c | 2 +-
arch/csky/include/asm/tcm.h | 8 +++
arch/ia64/include/asm/cache.h | 2 +-
arch/microblaze/kernel/setup.c| 2 +-
arch/mips/include/asm/cache.h | 2 +-
arch/mips/include/asm/machine.h | 2 +-
arch/mips/kernel/setup.c | 2 +-
arch/mips/mm/init.c | 2 +-
arch/parisc/include/asm/cache.h | 2 +-
arch/parisc/include/asm/ldcw.h| 2 +-
arch/parisc/kernel/ftrace.c | 2 +-
arch/parisc/mm/init.c | 6 ++---
arch/powerpc/include/asm/cache.h | 2 +-
arch/powerpc/include/asm/machdep.h| 2 +-
arch/powerpc/kernel/btext.c | 2 +-
arch/powerpc/kernel/prom_init.c | 2 +-
arch/powerpc/kvm/book3s_64_vio_hv.c | 2 +-
arch/riscv/include/asm/soc.h | 4 ++--
arch/riscv/kernel/cpu_ops.c | 4 ++--
arch/riscv/kernel/setup.c | 4 ++--
arch/s390/boot/startup.c | 2 +-
arch/s390/include/asm/cache.h | 2 +-
arch/s390/include/asm/sections.h | 4 ++--
arch/s390/mm/init.c | 2 +-
arch/sh/boards/of-generic.c | 2 +-
arch/sh/include/asm/cache.h | 2 +-
arch/sh/include/asm/machvec.h | 2 +-
arch/sh/include/asm/smp.h | 2 +-
arch/sparc/include/asm/cache.h| 2 +-
arch/sparc/kernel/btext.c | 2 +-
arch/um/include/shared/init.h | 22 -
arch/um/kernel/skas/clone.c | 2 +-
arch/um/kernel/um_arch.c | 2 +-
arch/x86/boot/compressed/pgtable_64.c | 2 +-
arch/x86/boot/tty.c | 8 +++
arch/x86/boot/video.h | 2 +-
arch/x86/include/asm/apic.h | 4 ++--
arch/x86/include/asm/cache.h | 2 +-
arch/x86/include/asm/intel-mid.h | 2 +-
arch/x86/include/asm/irqflags.h | 2 +-
arch/x86/include/asm/mem_encrypt.h| 2 +-
arch/x86/include/asm/setup.h | 2 +-
arch/x86/kernel/cpu/cpu.h | 2 +-
arch/x86/kernel/head64.c | 2 +-
arch/x86/mm/mem_encrypt.c | 6 ++---
arch/x86/mm/mem_encrypt_identity.c| 2 +-
arch/x86/platform/pvh/enlighten.c | 4 ++--
arch/x86/purgatory/purgatory.c| 4 ++--
arch/x86/um/stub_segv.c | 2 +-
arch/x86/xen/enlighten.c | 2 +-
arch/x86/xen/enlighten_pvh.c | 2 +-
arch/xtensa/kernel/setup.c| 2 +-
drivers/clk/clk.c | 2 +-
drivers/clocksource/timer-probe.c | 2 +-
drivers/irqchip/irqchip.c | 2 +-
drivers/of/of_reserved_mem.c | 2 +-
drivers/thermal/thermal_core.h| 2 +-
fs/xfs/xfs_message.h | 2 +-
include/asm-generic/bug.h | 6 ++---
include/asm-generic/error-injection.h | 2 +-
include/asm-generic/kprobes.h | 4 ++--
include/kunit/test.h | 2 +-
include/linux/acpi.h | 4 ++--
include/linux/cache.h | 2 +-
include/linux/compiler.h | 8 +++
include/linux/compiler_attributes.h | 2 +-
in
Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Tue, 2020-10-06 at 00:34 +, Joel Stanley wrote: > arch/powerpc/boot is the powerpc wrapper, and it's not built with the > same includes or flags as the rest of the kernel. It doesn't include > any of the headers in the top level include/ directory for hysterical > raisins. > > The straightforward fix would be to exclude this directory from your script. I agree and that's why I submitted another script that does just that. https://lore.kernel.org/lkml/75393e5ddc272dc7403de74d645e6c6e0f4e70eb.ca...@perches.com/
Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Mon, 2020-10-05 at 11:36 -0700, Nick Desaulniers wrote:
> I don't think there's anything wrong with manually including it and adding `-I
> ` (capital i) if needed.
All of this is secondary to the actual change to use
quoted __section("foo") rather than __section(foo)
I'd rather get that done first and then figure out if
additional changes could be done later.
Re: Where is the declaration of buffer used in kernel_param_ops .get functions?
On Sun, 2020-10-04 at 02:36 +0100, Matthew Wilcox wrote:
> On Sat, Oct 03, 2020 at 06:19:18PM -0700, Joe Perches wrote:
> > These patches came up because I was looking for
> > the location of the declaration of the buffer used
> > in kernel/params.c struct kernel_param_ops .get
> > functions.
> >
> > I didn't find it.
> >
> > I want to see if it's appropriate to convert the
> > sprintf family of functions used in these .get
> > functions to sysfs_emit.
> >
> > Patches submitted here:
> > https://lore.kernel.org/lkml/5d606519698ce4c8f1203a2b35797d8254c6050a.1600285923.git@perches.com/T/
> >
> > Anyone know if it's appropriate to change the
> > sprintf-like uses in these functions to sysfs_emit
> > and/or sysfs_emit_at?
>
> There's a lot of preprocessor magic to wade through.
>
> I'm pretty sure this comes through include/linux/moduleparam.h
> and kernel/module.c.
Dunno, looked there, still can't find it.
btw:
The __module_param_call macro looks very dodgy
as it uses both __used and __attribute__((unused))
and likely one of them should be removed (unused?)
It looks like the comes from varying definitions of
__attribute_used__ eventually converted to __used
for old gcc versions 2, 3, and 4.
1da177e4c3f4:include/linux/compiler-gcc2.h:#define __attribute_used__
__attribute__((__unused__))
1da177e4c3f4:include/linux/compiler-gcc3.h:# define __attribute_used__
__attribute__((__used__))
1da177e4c3f4:include/linux/compiler-gcc3.h:# define __attribute_used__
__attribute__((__unused__))
1da177e4c3f4:include/linux/compiler-gcc4.h:#define __attribute_used__
__attribute__((__used__))
Maybe:
---
include/linux/moduleparam.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h
index 47879fc7f75e..fc820b27fb00 100644
--- a/include/linux/moduleparam.h
+++ b/include/linux/moduleparam.h
@@ -288,10 +288,10 @@ struct kparam_array
/* Default value instead of permissions? */ \
static const char __param_str_##name[] = prefix #name; \
static struct kernel_param __moduleparam_const __param_##name \
- __used \
-__attribute__ ((unused,__section__ ("__param"),aligned(sizeof(void * \
- = { __param_str_##name, THIS_MODULE, ops, \
- VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } }
+ __used __section("__param") __aligned(sizeof(void *)) = { \
+ __param_str_##name, THIS_MODULE, ops, \
+ VERIFY_OCTAL_PERMISSIONS(perm), level, flags, { arg } \
+ }
/* Obsolete - use module_param_cb() */
#define module_param_call(name, _set, _get, arg, perm) \
Where is the declaration of buffer used in kernel_param_ops .get functions?
These patches came up because I was looking for the location of the declaration of the buffer used in kernel/params.c struct kernel_param_ops .get functions. I didn't find it. I want to see if it's appropriate to convert the sprintf family of functions used in these .get functions to sysfs_emit. Patches submitted here: https://lore.kernel.org/lkml/5d606519698ce4c8f1203a2b35797d8254c6050a.1600285923.git@perches.com/T/ Anyone know if it's appropriate to change the sprintf-like uses in these functions to sysfs_emit and/or sysfs_emit_at?
[PATCH 1/4] KVM: PPC: Book3S HV: Make struct kernel_param_ops definition const
This should be const, so make it so.
Signed-off-by: Joe Perches
---
arch/powerpc/kvm/book3s_hv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 4ba06a2a306c..2b215852cdc9 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -111,7 +111,7 @@ module_param(one_vm_per_core, bool, S_IRUGO | S_IWUSR);
MODULE_PARM_DESC(one_vm_per_core, "Only run vCPUs from the same VM on a core
(requires indep_threads_mode=N)");
#ifdef CONFIG_KVM_XICS
-static struct kernel_param_ops module_param_ops = {
+static const struct kernel_param_ops module_param_ops = {
.set = param_set_int,
.get = param_get_int,
};
--
2.26.0
[PATCH 0/4] treewide: Make definitions of struct kernel_param_ops const
Using const is good as it reduces data size. Joe Perches (4): KVM: PPC: Book3S HV: Make struct kernel_param_ops definition const kvm x86/mmu: Make struct kernel_param_ops definitions const rcu/tree: Make struct kernel_param_ops definitions const mm/zswap: Make struct kernel_param_ops definitions const arch/powerpc/kvm/book3s_hv.c | 2 +- arch/x86/kvm/mmu/mmu.c | 4 ++-- kernel/rcu/tree.c| 4 ++-- mm/zswap.c | 6 +++--- 4 files changed, 8 insertions(+), 8 deletions(-) -- 2.26.0
Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Thu, 2020-10-01 at 14:39 -0500, Segher Boessenkool wrch/ote: > Hi! > > On Thu, Oct 01, 2020 at 12:15:39PM +0200, Miguel Ojeda wrote: > > > So it looks like the best option is to exclude these > > > 2 files from conversion. > > > > Agreed. Nevertheless, is there any reason arch/powerpc/* should not be > > compiling cleanly with compiler.h? (CC'ing the rest of the PowerPC > > reviewers and ML). > > You need to #include compiler_types.h to get this #define? Actually no, you need to add #include to both files and then it builds properly. Ideally though nothing should include this file directly. > (The twice-defined thing is a warning, not an error. It should be fixed > of course, but it is less important; although it may be pointing to a > deeper problem.) > > > Segher
Re: [RFC PATCH next-20200930] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Thu, 2020-10-01 at 12:15 +0200, Miguel Ojeda wrote: > Hi Joe, Buenas Miguel. > On Thu, Oct 1, 2020 at 12:56 AM Joe Perches wrote: > > So I installed the powerpc cross compiler, and > > nope, that doesn't work, it makes a mess. > > Thanks a lot for reviving the script and sending the treewide cleanup! No charge... I think the end result is cleaner and more obvious. > > So it looks like the best option is to exclude these > > 2 files from conversion. > > Agreed. Nevertheless, is there any reason arch/powerpc/* should not be > compiling cleanly with compiler.h? (CC'ing the rest of the PowerPC > reviewers and ML). That's not a can of worms I care to open. Perhaps the powerpc folk can do some fishing.
Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to break;
On Thu, 2020-09-10 at 15:21 +0100, Robin Murphy wrote:
> On 2020-09-09 21:06, Joe Perches wrote:
> > fallthrough to a separate case/default label break; isn't very readable.
> >
> > Convert pseudo-keyword fallthrough; statements to a simple break; when
> > the next label is case or default and the only statement in the next
> > label block is break;
> >
> > Found using:
> >
> > $ grep-2.5.4 -rP --include=*.[ch] -n
> > "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
> >
> > Miscellanea:
> >
> > o Move or coalesce a couple label blocks above a default: block.
> >
> > Signed-off-by: Joe Perches
> > ---
> >
> > Compiled allyesconfig x86-64 only.
> > A few files for other arches were not compiled.
> >
>
> [...]
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index c192544e874b..743db1abec40 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -3777,7 +3777,7 @@ static int arm_smmu_device_hw_probe(struct
> > arm_smmu_device *smmu)
> > switch (FIELD_GET(IDR0_TTF, reg)) {
> > case IDR0_TTF_AARCH32_64:
> > smmu->ias = 40;
> > - fallthrough;
> > + break;
> > case IDR0_TTF_AARCH64:
> > break;
> > default:
>
> I have to say I don't really agree with the readability argument for
> this one - a fallthrough is semantically correct here, since the first
> case is a superset of the second. It just happens that anything we would
> do for the common subset is implicitly assumed (there are other
> potential cases we simply haven't added support for at the moment), thus
> the second case is currently empty.
> This change actively obfuscates that distinction.
Then perhaps comments should be added to usefully
describe the mechanisms.
case IDR0_TTF_AARCH32_64:
smmu->ias = 40;
fallthrough;/* and still do the 64 bit processing */
case IDR0_TTF_AARCH64:
/* Nothing specific yet */
break;
> Robin.
Re: [trivial PATCH] treewide: Convert switch/case fallthrough; to break;
On Wed, 2020-09-09 at 19:36 -0300, Jason Gunthorpe wrote:
> On Wed, Sep 09, 2020 at 01:06:39PM -0700, Joe Perches wrote:
> > fallthrough to a separate case/default label break; isn't very readable.
> >
> > Convert pseudo-keyword fallthrough; statements to a simple break; when
> > the next label is case or default and the only statement in the next
> > label block is break;
> >
> > Found using:
> >
> > $ grep-2.5.4 -rP --include=*.[ch] -n
> > "fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
> >
> > Miscellanea:
> >
> > o Move or coalesce a couple label blocks above a default: block.
> >
> > Signed-off-by: Joe Perches
> > ---
> >
> > Compiled allyesconfig x86-64 only.
> > A few files for other arches were not compiled.
>
> IB part looks OK, I prefer it like this
>
> You could do the same for continue as well, I saw a few of those..
I saw some continue uses as well but wasn't sure
and didn't look to see if the switch/case with
continue was in a for/while loop.
[trivial PATCH] treewide: Convert switch/case fallthrough; to break;
fallthrough to a separate case/default label break; isn't very readable.
Convert pseudo-keyword fallthrough; statements to a simple break; when
the next label is case or default and the only statement in the next
label block is break;
Found using:
$ grep-2.5.4 -rP --include=*.[ch] -n
"fallthrough;(\s*(case\s+\w+|default)\s*:\s*){1,7}break;" *
Miscellanea:
o Move or coalesce a couple label blocks above a default: block.
Signed-off-by: Joe Perches
---
Compiled allyesconfig x86-64 only.
A few files for other arches were not compiled.
arch/arm/mach-mmp/pm-pxa910.c | 2 +-
arch/arm64/kvm/handle_exit.c | 2 +-
arch/mips/kernel/cpu-probe.c | 2 +-
arch/mips/math-emu/cp1emu.c | 2 +-
arch/s390/pci/pci.c | 2 +-
crypto/tcrypt.c | 4 ++--
drivers/ata/sata_mv.c | 2 +-
drivers/atm/lanai.c | 2 +-
drivers/gpu/drm/i915/display/intel_sprite.c | 2 +-
drivers/gpu/drm/nouveau/nvkm/engine/disp/hdmi.c | 2 +-
drivers/hid/wacom_wac.c | 2 +-
drivers/i2c/busses/i2c-i801.c | 2 +-
drivers/infiniband/ulp/rtrs/rtrs-clt.c| 14 +++---
drivers/infiniband/ulp/rtrs/rtrs-srv.c| 6 +++---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/irqchip/irq-vic.c | 4 ++--
drivers/md/dm.c | 2 +-
drivers/media/dvb-frontends/drxd_hard.c | 2 +-
drivers/media/i2c/ov5640.c| 2 +-
drivers/media/i2c/ov6650.c| 5 ++---
drivers/media/i2c/smiapp/smiapp-core.c| 2 +-
drivers/media/i2c/tvp5150.c | 2 +-
drivers/media/pci/ddbridge/ddbridge-core.c| 2 +-
drivers/media/usb/cpia2/cpia2_core.c | 2 +-
drivers/mfd/iqs62x.c | 3 +--
drivers/mmc/host/atmel-mci.c | 2 +-
drivers/mtd/nand/raw/nandsim.c| 2 +-
drivers/net/ethernet/intel/e1000e/phy.c | 2 +-
drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_adminq.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 +-
drivers/net/ethernet/intel/iavf/iavf_txrx.c | 2 +-
drivers/net/ethernet/intel/igb/e1000_phy.c| 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_82599.c| 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +-
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c| 2 +-
drivers/net/ethernet/intel/ixgbevf/vf.c | 2 +-
drivers/net/ethernet/netronome/nfp/nfpcore/nfp6000_pcie.c | 2 +-
drivers/net/ethernet/qlogic/qed/qed_mcp.c | 2 +-
drivers/net/ethernet/sfc/falcon/farch.c | 2 +-
drivers/net/ethernet/sfc/farch.c | 2 +-
drivers/net/phy/adin.c| 3 +--
drivers/net/usb/pegasus.c | 4 ++--
drivers/net/usb/usbnet.c | 2 +-
drivers/net/wireless/ath/ath5k/eeprom.c | 2 +-
drivers/net/wireless/mediatek/mt7601u/dma.c | 8
drivers/nvme/host/core.c | 12 ++--
drivers/pcmcia/db1xxx_ss.c| 4 ++--
drivers/power/supply/abx500_chargalg.c| 2 +-
drivers/power/supply/charger-manager.c| 2 +-
drivers/rtc/rtc-pcf85063.c| 2 +-
drivers/s390/scsi/zfcp_fsf.c | 2 +-
drivers/scsi/aic7xxx/aic79xx_core.c | 4 ++--
drivers/scsi/aic94xx/aic94xx_tmf.c| 2 +-
drivers/scsi/lpfc/lpfc_sli.c | 2 +-
drivers/scsi/smartpqi/smartpqi_init.c | 2 +-
drivers/scsi/sr.c | 2 +-
drivers/tty/serial/sunsu.c| 2 +-
drivers/tty/serial/sunzilog.c | 2 +-
drivers/tty/vt/vt_ioctl.c | 2 +-
drivers/usb/dwc3/core.c | 2 +-
drivers/usb/gadget/legacy/inode.c | 2 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 4 ++--
drivers/usb/host/ohci-hcd.c | 2 +-
drivers/usb/isp1760/isp1760-hcd.c | 2 +-
drivers/usb/
[PATCH 17/29] fs_enet: Avoid comma separated statements
Use semicolons and braces.
Signed-off-by: Joe Perches
---
drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index bf846b42bc74..78e008b81374 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -562,10 +562,13 @@ fs_enet_start_xmit(struct sk_buff *skb, struct net_device
*dev)
BD_ENET_TX_TC);
CBDS_SC(bdp, BD_ENET_TX_READY);
- if ((CBDR_SC(bdp) & BD_ENET_TX_WRAP) == 0)
- bdp++, curidx++;
- else
- bdp = fep->tx_bd_base, curidx = 0;
+ if ((CBDR_SC(bdp) & BD_ENET_TX_WRAP) == 0) {
+ bdp++;
+ curidx++;
+ } else {
+ bdp = fep->tx_bd_base;
+ curidx = 0;
+ }
len = skb_frag_size(frag);
CBDW_BUFADDR(bdp, skb_frag_dma_map(fep->dev, frag, 0, len,
--
2.26.0
[PATCH 00/29] treewide: Convert comma separated statements
There are many comma separated statements in the kernel. See:https://lore.kernel.org/lkml/alpine.DEB.2.22.394.2008201856110.2524@hadrien/ Convert the comma separated statements that are in if/do/while blocks to use braces and semicolons. Many comma separated statements still exist but those are changes for another day. Joe Perches (29): coding-style.rst: Avoid comma statements alpha: Avoid comma separated statements ia64: Avoid comma separated statements sparc: Avoid comma separated statements ata: Avoid comma separated statements drbd: Avoid comma separated statements lp: Avoid comma separated statements dma-buf: Avoid comma separated statements drm/gma500: Avoid comma separated statements drm/i915: Avoid comma separated statements hwmon: (scmi-hwmon): Avoid comma separated statements Input: MT - Avoid comma separated statements bcache: Avoid comma separated statements media: Avoid comma separated statements mtd: Avoid comma separated statements 8390: Avoid comma separated statements fs_enet: Avoid comma separated statements wan: sbni: Avoid comma separated statements s390/tty3270: Avoid comma separated statements scai/arm: Avoid comma separated statements media: atomisp: Avoid comma separated statements video: fbdev: Avoid comma separated statements fuse: Avoid comma separated statements reiserfs: Avoid comma separated statements lib/zlib: Avoid comma separated statements lib: zstd: Avoid comma separated statements ipv6: fib6: Avoid comma separated statements sunrpc: Avoid comma separated statements tools: Avoid comma separated statements Documentation/process/coding-style.rst| 17 + arch/alpha/kernel/pci_iommu.c | 8 +- arch/alpha/oprofile/op_model_ev4.c| 22 +- arch/alpha/oprofile/op_model_ev5.c| 8 +- arch/ia64/kernel/smpboot.c| 7 +- arch/sparc/kernel/smp_64.c| 7 +- drivers/ata/pata_icside.c | 21 +- drivers/block/drbd/drbd_receiver.c| 6 +- drivers/char/lp.c | 6 +- drivers/dma-buf/st-dma-fence.c| 7 +- drivers/gpu/drm/gma500/mdfld_intel_display.c | 44 ++- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 8 +- drivers/gpu/drm/i915/gt/intel_gt_requests.c | 6 +- .../gpu/drm/i915/gt/selftest_workarounds.c| 6 +- drivers/gpu/drm/i915/intel_runtime_pm.c | 6 +- drivers/hwmon/scmi-hwmon.c| 6 +- drivers/input/input-mt.c | 11 +- drivers/md/bcache/bset.c | 12 +- drivers/md/bcache/sysfs.c | 6 +- drivers/media/i2c/msp3400-kthreads.c | 12 +- drivers/media/pci/bt8xx/bttv-cards.c | 6 +- drivers/media/pci/saa7134/saa7134-video.c | 7 +- drivers/mtd/devices/lart.c| 10 +- drivers/net/ethernet/8390/axnet_cs.c | 19 +- drivers/net/ethernet/8390/lib8390.c | 14 +- drivers/net/ethernet/8390/pcnet_cs.c | 6 +- .../ethernet/freescale/fs_enet/fs_enet-main.c | 11 +- drivers/net/wan/sbni.c| 101 +++--- drivers/s390/char/tty3270.c | 6 +- drivers/scsi/arm/cumana_2.c | 19 +- drivers/scsi/arm/eesox.c | 9 +- drivers/scsi/arm/powertec.c | 9 +- .../media/atomisp/pci/atomisp_subdev.c| 6 +- drivers/video/fbdev/tgafb.c | 12 +- fs/fuse/dir.c | 24 +- fs/reiserfs/fix_node.c| 36 ++- lib/zlib_deflate/deftree.c| 49 ++- lib/zstd/compress.c | 120 --- lib/zstd/fse_compress.c | 24 +- lib/zstd/huf_compress.c | 6 +- net/ipv6/ip6_fib.c| 12 +- net/sunrpc/sysctl.c | 6 +- tools/lib/subcmd/help.c | 10 +- tools/power/cpupower/utils/cpufreq-set.c | 14 +- tools/testing/selftests/vm/gup_benchmark.c| 18 +- tools/testing/selftests/vm/userfaultfd.c | 296 +++--- 46 files changed, 694 insertions(+), 382 deletions(-) -- 2.26.0
Re: [PATCH v2 11/25] powerpc/signal: Refactor bad frame logging
On Tue, 2020-08-18 at 17:19 +, Christophe Leroy wrote:
> The logging of bad frame appears half a dozen of times
> and is pretty similar.
[]
> diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
[]
> @@ -355,3 +355,14 @@ static unsigned long get_tm_stackpointer(struct
> task_struct *tsk)
> #endif
> return ret;
> }
> +
> +static const char fm32[] = KERN_INFO "%s[%d]: bad frame in %s: %p nip %08lx
> lr %08lx\n";
> +static const char fm64[] = KERN_INFO "%s[%d]: bad frame in %s: %p nip %016lx
> lr %016lx\n";
Why not remove this and use it in place with
%08lx/%016x used as %px with a case to (void *)?
> +void signal_fault(struct task_struct *tsk, struct pt_regs *regs,
> + const char *where, void __user *ptr)
> +{
> + if (show_unhandled_signals)
> + printk_ratelimited(regs->msr & MSR_64BIT ? fm64 : fm32,
> tsk->comm,
> +task_pid_nr(tsk), where, ptr, regs->nip,
> regs->link);
pr_info_ratelimited("%s[%d]: bad frame in %s: %p nip %016lx lr
%016lx\n",
tsk->comm, task_pid_nr(tsk), where, ptr,
(void *)regs->nip, (void *)regs->link);
Re: [PATCH 0/9] powerpc: delete duplicated words
On Sun, 2020-07-26 at 12:08 -0700, Randy Dunlap wrote:
> v0.1 of this script also found lots of repeated numbers and strings of
> special characters (ASCII art etc.), so now it ignores duplicated numbers
> or special characters -- since it is really looking for duplicate words.
>
> Anyway, I might as well attach it. It's no big deal.
> And if someone else wants to tackle using it, go for it.
This might be a reasonable thing to add to checkpatch.
And here's another possible similar perl word deduplicator attached:
Assuming you have git, this could be used like:
$ git ls-files -- | xargs perl deduplicate_words.pl
And it would overwrite all files with duplicated words.
No guarantees any changes it makes are right of course.
It still needs a human to verify any change.
For instance:
$ git ls-files kernel/trace/*.[ch] | xargs perl deduplicate_words.pl
$ git diff kernel/trace
kernel/trace/ftrace.c | 2 +-
kernel/trace/trace.c | 2 +-
kernel/trace/trace_dynevent.c | 2 +-
kernel/trace/trace_events_synth.c | 2 +-
kernel/trace/tracing_map.c| 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index a3093a84bae3..b7f085a4f71a 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -2405,7 +2405,7 @@ struct ftrace_ops direct_ops = {
*
* If the record has the FTRACE_FL_REGS set, that means that it
* wants to convert to a callback that saves all regs. If FTRACE_FL_REGS
- * is not not set, then it wants to convert to the normal callback.
+ * is not set, then it wants to convert to the normal callback.
*
* Returns the address of the trampoline to set to
*/
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 5aa5c01e2fed..4d3dcfb06d6d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -9253,7 +9253,7 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)
/*
* We need to stop all tracing on all CPUS to read the
-* the next buffer. This is a bit expensive, but is
+* next buffer. This is a bit expensive, but is
* not done often. We fill all what we can read,
* and then release the locks again.
*/
diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index 2c435fdef565..8c1e7e168505 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -402,7 +402,7 @@ void dynevent_arg_init(struct dynevent_arg *arg,
* whitespace, all followed by a separator, if applicable. After the
* first arg string is successfully appended to the command string,
* the optional @operator is appended, followed by the second arg and
- * and optional @separator. If no separator was specified when
+ * optional @separator. If no separator was specified when
* initializing the arg, a space will be appended.
*/
void dynevent_arg_pair_init(struct dynevent_arg_pair *arg_pair,
diff --git a/kernel/trace/trace_events_synth.c
b/kernel/trace/trace_events_synth.c
index e2a623f2136c..3801d3088744 100644
--- a/kernel/trace/trace_events_synth.c
+++ b/kernel/trace/trace_events_synth.c
@@ -1211,7 +1211,7 @@ __synth_event_trace_start(struct trace_event_file *file,
* ENABLED bit is set (which attaches the probe thus allowing
* this code to be called, etc). Because this is called
* directly by the user, we don't have that but we still need
-* to honor not logging when disabled. For the the iterated
+* to honor not logging when disabled. For the iterated
* trace case, we save the enabed state upon start and just
* ignore the following data calls.
*/
diff --git a/kernel/trace/tracing_map.c b/kernel/trace/tracing_map.c
index 74738c9856f1..4b50fc0cb12c 100644
--- a/kernel/trace/tracing_map.c
+++ b/kernel/trace/tracing_map.c
@@ -260,7 +260,7 @@ int tracing_map_add_var(struct tracing_map *map)
* to use cmp_fn.
*
* A key can be a subset of a compound key; for that purpose, the
- * offset param is used to describe where within the the compound key
+ * offset param is used to describe where within the compound key
* the key referenced by this key field resides.
*
* Return: The index identifying the field in the map and associated
deduplicate_words.pl
Description: Perl program
Re: [PATCH 0/9] powerpc: delete duplicated words
On 2020-07-26 12:08, Randy Dunlap wrote:
On 7/26/20 10:49 AM, Joe Perches wrote:
On Sun, 2020-07-26 at 10:23 -0700, Randy Dunlap wrote:
On 7/26/20 7:29 AM, Christophe Leroy wrote:
Randy Dunlap a écrit :
Drop duplicated words in arch/powerpc/ header files.
How did you detect them ? Do you have some script for tgat, or you
just read all comments ?
Yes, it's a script that finds lots of false positives, so I have to
check
each and every one of them for validity.
And it's a lot of work too. (thanks Randy)
It could be something like:
$ grep-2.5.4 -nrP --include=*.[ch] '\b([A-Z]?[a-z]{2,}\b)[ \t]*(?:\n[
\t]*\*[ \t]*|)\1\b' * | \
grep -vP '\b(?:struct|enum|union)\s+([A-Z]?[a-z]{2,})\s+\*?\s*\1\b'
| \
grep -vP '\blong\s+long\b' | \
grep -vP '\b([A-Z]?[a-z]{2,})(?:\t+| {2,})\1\b'
Hi Joe,
Hi Randy
(what is grep-2.5.4 ?)
It's the last version of grep that allowed spanning multiple lines.
That's to find the comment second lines that start with *
It looks like you tried a few iterations of this -- since it drops
things
like "long long". There are lots of data types that are repeated &
valid.
And many struct names, like "struct kref kref", "struct completion
completion",
and "struct mutex mutex". I handle (ignore) those manually
that's the first exclude pattern.
Re: [PATCH 0/9] powerpc: delete duplicated words
On Sun, 2020-07-26 at 10:23 -0700, Randy Dunlap wrote:
> On 7/26/20 7:29 AM, Christophe Leroy wrote:
> > Randy Dunlap a écrit :
> >
> > > Drop duplicated words in arch/powerpc/ header files.
> >
> > How did you detect them ? Do you have some script for tgat, or you just
> > read all comments ?
>
> Yes, it's a script that finds lots of false positives, so I have to check
> each and every one of them for validity.
And it's a lot of work too. (thanks Randy)
It could be something like:
$ grep-2.5.4 -nrP --include=*.[ch] '\b([A-Z]?[a-z]{2,}\b)[ \t]*(?:\n[ \t]*\*[
\t]*|)\1\b' * | \
grep -vP '\b(?:struct|enum|union)\s+([A-Z]?[a-z]{2,})\s+\*?\s*\1\b' | \
grep -vP '\blong\s+long\b' | \
grep -vP '\b([A-Z]?[a-z]{2,})(?:\t+| {2,})\1\b'
Re: [PATCH] pci: pcie: AER: Fix logging of Correctable errors
On Fri, 2020-06-19 at 13:17 -0400, Sinan Kaya wrote:
> On 6/18/2020 11:55 AM, Matt Jolly wrote:
>
> > + pci_warn(dev, " device [%04x:%04x] error
> > status/mask=%08x/%08x\n",
> > + dev->vendor, dev->device,
> > + info->status, info->mask);
> > + } else {
>
>
>
> > + pci_err(dev, " device [%04x:%04x] error
> > status/mask=%08x/%08x\n",
> > + dev->vendor, dev->device,
> > + info->status, info->mask);
>
> Function pointers for pci_warn vs. pci_err ?
Not really possible as both are function-like macros.
Re: [PATCH v4 0/3] mm, treewide: Rename kzfree() to kfree_sensitive()
On Thu, 2020-06-18 at 00:31 +0300, Denis Efremov wrote: > > On 6/16/20 9:53 PM, Joe Perches wrote: > > On Mon, 2020-06-15 at 21:57 -0400, Waiman Long wrote: > > > v4: > > > - Break out the memzero_explicit() change as suggested by Dan Carpenter > > > so that it can be backported to stable. > > > - Drop the "crypto: Remove unnecessary memzero_explicit()" patch for > > > now as there can be a bit more discussion on what is best. It will be > > > introduced as a separate patch later on after this one is merged. > > > > To this larger audience and last week without reply: > > https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.ca...@perches.com/ > > > > Are there _any_ fastpath uses of kfree or vfree? > > > > Many patches have been posted recently to fix mispairings > > of specific types of alloc and free functions. > > I've prepared a coccinelle script to highlight these mispairings in a function > a couple of days ago: https://lkml.org/lkml/2020/6/5/953 > I've listed all the fixes in the commit message. > > Not so many mispairings actually, and most of them are harmless like: > kmalloc(E) -> kvfree(E) > > However, coccinelle script can't detect cross-functions mispairings, i.e. > allocation in one function, free in another funtion. Hey Denis, thanks for those patches. If possible, it's probably better to not require these pairings and use a single standard kfree/free function. Given the existing ifs in kfree in slab/slob/slub, it seems likely that adding a few more wouldn't have much impact.
Re: [PATCH v4 0/3] mm, treewide: Rename kzfree() to kfree_sensitive()
On Mon, 2020-06-15 at 21:57 -0400, Waiman Long wrote:
> v4:
> - Break out the memzero_explicit() change as suggested by Dan Carpenter
> so that it can be backported to stable.
> - Drop the "crypto: Remove unnecessary memzero_explicit()" patch for
> now as there can be a bit more discussion on what is best. It will be
> introduced as a separate patch later on after this one is merged.
To this larger audience and last week without reply:
https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.ca...@perches.com/
Are there _any_ fastpath uses of kfree or vfree?
Many patches have been posted recently to fix mispairings
of specific types of alloc and free functions.
To eliminate these mispairings at a runtime cost of four
comparisons, should the kfree/vfree/kvfree/kfree_const
functions be consolidated into a single kfree?
Something like the below:
void kfree(const void *addr)
{
if (is_kernel_rodata((unsigned long)addr))
return;
if (is_vmalloc_addr(addr))
_vfree(addr);
else
_kfree(addr);
}
#define kvfree kfree
#define vfreekfree
#define kfree_const kfree
Re: [PATCH] pwm: Add missing "CONFIG_" prefix
On Thu, 2020-06-04 at 14:52 -0700, Kees Cook wrote:
> On Wed, Jun 03, 2020 at 04:04:31PM -0700, Joe Perches wrote:
> > On Wed, 2020-06-03 at 15:40 -0700, Kees Cook wrote:
> > > The IS_ENABLED() use was missing the CONFIG_ prefix which would have
> > > lead to skipping this code.
> > >
> > > Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
> > > Signed-off-by: Kees Cook
> > > ---
> > > drivers/pwm/core.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> > > index 9973c442b455..6b3cbc0490c6 100644
> > > --- a/drivers/pwm/core.c
> > > +++ b/drivers/pwm/core.c
> > > @@ -121,7 +121,7 @@ static int pwm_device_request(struct pwm_device *pwm,
> > > const char *label)
> > > pwm->chip->ops->get_state(pwm->chip, pwm, &pwm->state);
> > > trace_pwm_get(pwm, &pwm->state);
> > >
> > > - if (IS_ENABLED(PWM_DEBUG))
> > > + if (IS_ENABLED(CONFIG_PWM_DEBUG))
> > > pwm->last = pwm->state;
> > > }
> > >
> > > --
> > > 2.25.1
> > >
> >
> > more odd uses (mostly in comments)
> >
> > $ git grep -P -oh '\bIS_ENABLED\s*\(\s*\w+\s*\)'| \
> > sed -r 's/\s+//g'| \
> > grep -v '(CONFIG_' | \
> > sort | uniq -c | sort -rn
> > 7 IS_ENABLED(DEBUG)
> > 4 IS_ENABLED(DRM_I915_SELFTEST)
> > 4 IS_ENABLED(cfg)
> > 2 IS_ENABLED(opt_name)
> > 2 IS_ENABLED(DEBUG_PRINT_TRIE_GRAPHVIZ)
> > 2 IS_ENABLED(config)
> > 2 IS_ENABLED(cond)
> > 2 IS_ENABLED(__BIG_ENDIAN)
> > 1 IS_ENABLED(x)
> > 1 IS_ENABLED(STRICT_KERNEL_RWX)
> > 1 IS_ENABLED(PWM_DEBUG)
> > 1 IS_ENABLED(option)
> > 1 IS_ENABLED(ETHTOOL_NETLINK)
> > 1 IS_ENABLED(DEBUG_RANDOM_TRIE)
> > 1 IS_ENABLED(DEBUG_CHACHA20POLY1305_SLOW_CHUNK_TEST)
> >
> > STRICT_KERNEL_RWX is misused here in ppc
> >
> > ---
> >
> > Fix pr_warn without newline too.
> >
> > arch/powerpc/mm/book3s64/hash_utils.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/powerpc/mm/book3s64/hash_utils.c
> > b/arch/powerpc/mm/book3s64/hash_utils.c
> > index 51e3c15f7aff..dd60c5f2b991 100644
> > --- a/arch/powerpc/mm/book3s64/hash_utils.c
> > +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> > @@ -660,11 +660,10 @@ static void __init htab_init_page_sizes(void)
> > * Pick a size for the linear mapping. Currently, we only
> > * support 16M, 1M and 4K which is the default
> > */
> > - if (IS_ENABLED(STRICT_KERNEL_RWX) &&
> > + if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) &&
> > (unsigned long)_stext % 0x100) {
> > if (mmu_psize_defs[MMU_PAGE_16M].shift)
> > - pr_warn("Kernel not 16M aligned, "
> > - "disabling 16M linear map alignment");
> > + pr_warn("Kernel not 16M aligned, disabling 16M
> > linear map alignment\n");
> > aligned = false;
> > }
>
> Joe, I was going to send all of the fixes for these issues, but your
> patch doesn't have a SoB. Shall I add one for the above patch?
sure if you want, or submit it yourself.
My feeling about these types of changes is the maintainers
of the subsystems, in this case ppc, should manage this
themselves and shouldn't require anyone else to actually
bother to send real patches.
Re: [PATCH] pwm: Add missing "CONFIG_" prefix
On Wed, 2020-06-03 at 15:40 -0700, Kees Cook wrote:
> The IS_ENABLED() use was missing the CONFIG_ prefix which would have
> lead to skipping this code.
>
> Fixes: 3ad1f3a33286 ("pwm: Implement some checks for lowlevel drivers")
> Signed-off-by: Kees Cook
> ---
> drivers/pwm/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/core.c b/drivers/pwm/core.c
> index 9973c442b455..6b3cbc0490c6 100644
> --- a/drivers/pwm/core.c
> +++ b/drivers/pwm/core.c
> @@ -121,7 +121,7 @@ static int pwm_device_request(struct pwm_device *pwm,
> const char *label)
> pwm->chip->ops->get_state(pwm->chip, pwm, &pwm->state);
> trace_pwm_get(pwm, &pwm->state);
>
> - if (IS_ENABLED(PWM_DEBUG))
> + if (IS_ENABLED(CONFIG_PWM_DEBUG))
> pwm->last = pwm->state;
> }
>
> --
> 2.25.1
>
more odd uses (mostly in comments)
$ git grep -P -oh '\bIS_ENABLED\s*\(\s*\w+\s*\)'| \
sed -r 's/\s+//g'| \
grep -v '(CONFIG_' | \
sort | uniq -c | sort -rn
7 IS_ENABLED(DEBUG)
4 IS_ENABLED(DRM_I915_SELFTEST)
4 IS_ENABLED(cfg)
2 IS_ENABLED(opt_name)
2 IS_ENABLED(DEBUG_PRINT_TRIE_GRAPHVIZ)
2 IS_ENABLED(config)
2 IS_ENABLED(cond)
2 IS_ENABLED(__BIG_ENDIAN)
1 IS_ENABLED(x)
1 IS_ENABLED(STRICT_KERNEL_RWX)
1 IS_ENABLED(PWM_DEBUG)
1 IS_ENABLED(option)
1 IS_ENABLED(ETHTOOL_NETLINK)
1 IS_ENABLED(DEBUG_RANDOM_TRIE)
1 IS_ENABLED(DEBUG_CHACHA20POLY1305_SLOW_CHUNK_TEST)
STRICT_KERNEL_RWX is misused here in ppc
---
Fix pr_warn without newline too.
arch/powerpc/mm/book3s64/hash_utils.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c
b/arch/powerpc/mm/book3s64/hash_utils.c
index 51e3c15f7aff..dd60c5f2b991 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -660,11 +660,10 @@ static void __init htab_init_page_sizes(void)
* Pick a size for the linear mapping. Currently, we only
* support 16M, 1M and 4K which is the default
*/
- if (IS_ENABLED(STRICT_KERNEL_RWX) &&
+ if (IS_ENABLED(CONFIG_STRICT_KERNEL_RWX) &&
(unsigned long)_stext % 0x100) {
if (mmu_psize_defs[MMU_PAGE_16M].shift)
- pr_warn("Kernel not 16M aligned, "
- "disabling 16M linear map alignment");
+ pr_warn("Kernel not 16M aligned, disabling 16M
linear map alignment\n");
aligned = false;
}
Re: [PATCH v7 2/5] seq_buf: Export seq_buf_printf() to external modules
On Fri, 2020-05-08 at 17:30 +0530, Vaibhav Jain wrote:
> Hi Boris,
>
> Borislav Petkov writes:
>
> > On Fri, May 08, 2020 at 04:19:19PM +0530, Vaibhav Jain wrote:
> > > 'seq_buf' provides a very useful abstraction for writing to a string
> > > buffer without needing to worry about it over-flowing. However even
> > > though the API has been stable for couple of years now its stills not
> > > exported to external modules limiting its usage.
> > >
> > > Hence this patch proposes update to 'seq_buf.c' to mark
> > > seq_buf_printf() which is part of the seq_buf API to be exported to
> > > external GPL modules. This symbol will be used in later parts of this
> >
> > What is "external GPL modules"?
> I am referring to Kernel Loadable Modules with MODULE_LICENSE("GPL")
> here.
Any reason why these Kernel Loadable Modules with MODULE_LICENSE("GPL")
are not in the kernel tree?
Re: [PATCH v2 2/2] crypto: Remove unnecessary memzero_explicit()
On Tue, 2020-04-14 at 15:37 -0400, Waiman Long wrote: > OK, I can change it to clear the key length when the allocation failed > which isn't likely. Perhaps: kfree_sensitive(op->key); op->key = NULL; op->keylen = 0; but I don't know that it impacts any possible state.
Re: [PATCH 2/2] crypto: Remove unnecessary memzero_explicit()
On Mon, 2020-04-13 at 17:15 -0400, Waiman Long wrote:
> Since kfree_sensitive() will do an implicit memzero_explicit(), there
> is no need to call memzero_explicit() before it. Eliminate those
> memzero_explicit() and simplify the call sites.
2 bits of trivia:
> diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
> b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c
[]
> @@ -391,10 +388,7 @@ int sun8i_ce_aes_setkey(struct crypto_skcipher *tfm,
> const u8 *key,
> dev_dbg(ce->dev, "ERROR: Invalid keylen %u\n", keylen);
> return -EINVAL;
> }
> - if (op->key) {
> - memzero_explicit(op->key, op->keylen);
> - kfree(op->key);
> - }
> + kfree_sensitive(op->key);
> op->keylen = keylen;
> op->key = kmemdup(key, keylen, GFP_KERNEL | GFP_DMA);
> if (!op->key)
It might be a defect to set op->keylen before the kmemdup succeeds.
> @@ -416,10 +410,7 @@ int sun8i_ce_des3_setkey(struct crypto_skcipher *tfm,
> const u8 *key,
> if (err)
> return err;
>
> - if (op->key) {
> - memzero_explicit(op->key, op->keylen);
> - kfree(op->key);
> - }
> + free_sensitive(op->key, op->keylen);
Why not kfree_sensitive(op->key) ?
Re: [PATCH v4 08/25] ocxl: Emit a log message showing how much LPC memory was detected
On Wed, 2020-04-01 at 01:49 -0700, Dan Williams wrote:
> On Sun, Mar 29, 2020 at 10:23 PM Alastair D'Silva
> wrote:
> > This patch emits a message showing how much LPC memory & special purpose
> > memory was detected on an OCXL device.
[]
> > diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
[]
> > @@ -568,6 +568,10 @@ static int read_afu_lpc_memory_info(struct pci_dev
> > *dev,
> > afu->special_purpose_mem_size =
> > total_mem_size - lpc_mem_size;
> > }
> > +
> > + dev_info(&dev->dev, "Probed LPC memory of %#llx bytes and special
> > purpose memory of %#llx bytes\n",
> > +afu->lpc_mem_size, afu->special_purpose_mem_size);
>
> A patch for a single log message is too fine grained for my taste,
> let's squash this into another patch in the series.
Is the granularity of lpc_mem_size actually bytes?
Might this be better as KiB or something using functions
Maybe something like:
unsigned long si_val(unsigned long val)
{
static const char units[] = "BKMGTPE";
const char *unit = units;
while (!(val & 1023) && unit[1]) {
val >>= 10;
unit++;
}
return val;
}
char si_type(unsigned long val)
{
static const char units[] = "BKMGTPE";
const char *unit = units;
while (!(val & 1023) && unit[1]) {
val >>= 10;
unit++;
}
return *unit;
}
so this could be something like:
dev_info(&dev->dev, "Probed LPC memory of %#llu%c and special purpose
memory of %#llu%c\n",
si_val(afu->lpc_mem_size), si_type(afu->lpc_mem_size),
si_val(afu->special_purpose_mem_size),
si_type(afu->special_purpose_mem_size));
Re: [PATCH v12 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
(removed a bunch of cc's) On Fri, 2020-03-20 at 18:31 +0200, Andy Shevchenko wrote: > On Fri, Mar 20, 2020 at 07:42:03AM -0700, Joe Perches wrote: > > On Fri, 2020-03-20 at 14:42 +0200, Andy Shevchenko wrote: > > > On Fri, Mar 20, 2020 at 12:23:38PM +0100, Michal Suchánek wrote: > > > > On Fri, Mar 20, 2020 at 12:33:50PM +0200, Andy Shevchenko wrote: > > > > > On Fri, Mar 20, 2020 at 11:20:19AM +0100, Michal Suchanek wrote: > > > > > > While at it also simplify the existing perf patterns. > > > > > And still missed fixes from parse-maintainers.pl. > > > > > > > > Oh, that script UX is truly ingenious. > > > > > > You have at least two options, their combinations, etc: > > > - complain to the author :-) > > > - send a patch :-) > > > > Recently: > > > > https://lore.kernel.org/lkml/4d5291fa3fb4962b1fa55e8fd9ef421ef0c1b1e5.ca...@perches.com/ > > But why? > > Shouldn't we rather run MAINTAINERS clean up once and require people to use > parse-maintainers.pl for good? That can basically only be done by Linus just before he releases an RC1. I am for it. One day...
Re: [PATCH v12 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
On Fri, 2020-03-20 at 14:42 +0200, Andy Shevchenko wrote: > On Fri, Mar 20, 2020 at 12:23:38PM +0100, Michal Suchánek wrote: > > On Fri, Mar 20, 2020 at 12:33:50PM +0200, Andy Shevchenko wrote: > > > On Fri, Mar 20, 2020 at 11:20:19AM +0100, Michal Suchanek wrote: > > > > While at it also simplify the existing perf patterns. > > > And still missed fixes from parse-maintainers.pl. > > > > Oh, that script UX is truly ingenious. > > You have at least two options, their combinations, etc: > - complain to the author :-) > - send a patch :-) Recently: https://lore.kernel.org/lkml/4d5291fa3fb4962b1fa55e8fd9ef421ef0c1b1e5.ca...@perches.com/
Re: [PATCH v11 8/8] MAINTAINERS: perf: Add pattern that matches ppc perf to the perf entry.
On Thu, 2020-03-19 at 13:19 +0100, Michal Suchanek wrote: > Signed-off-by: Michal Suchanek > --- > v10: new patch > --- > MAINTAINERS | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index bc8dbe4fe4c9..329bf4a31412 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -13088,6 +13088,8 @@ F:arch/*/kernel/*/perf_event*.c > F: arch/*/kernel/*/*/perf_event*.c > F: arch/*/include/asm/perf_event.h > F: arch/*/kernel/perf_callchain.c > +F: arch/*/perf/* > +F: arch/*/perf/*/* While I understand the desire, I believe that repetitive listings like this don't really help much. Having a single entry of: F: arch/*/perf/ would serve the same purpose. Nominally, the difference between the 2 entries vs the 1 entry is this: F: arch/*/perf/* Only the specific files in any directory that matches this pattern but not any files in their subdirectories are maintained. F: arch/*/perf/*/* Only the files in any top level subdirectory of any directory that matches this pattern are maintained but not files in any directory of those subdirectories. F: arch/*/perf/ Any file or any file in any subdirectory of any directory that matches this pattern is maintained.
Re: [PATCH -next 016/491] KERNEL VIRTUAL MACHINE FOR POWERPC (KVM/powerpc): Use fallthrough;
On Thu, 2020-03-19 at 12:18 +1100, Paul Mackerras wrote: > On Tue, Mar 10, 2020 at 09:51:30PM -0700, Joe Perches wrote: > > Convert the various uses of fallthrough comments to fallthrough; > > > > Done via script > > Link: > > https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/ > > > > Signed-off-by: Joe Perches > > The subject line should look like "KVM: PPC: Use fallthrough". There's no way to generate a subject line like that via a script so far as I can tell. > Apart from that, > > Acked-by: Paul Mackerras > > How are these patches going upstream? Do you want me to take this via > my tree? If you want. Ideally, these changes would go in treewide via a script run by Linus at an -rc1, but if the change is OK with you, it'd be fine to have you apply it now. cheers, Joe
Re: [PATCH] treewide: Rename "unencrypted" to "decrypted"
On Tue, 2020-03-17 at 14:24 -0700, Dave Hansen wrote: > On 3/17/20 2:06 PM, Borislav Petkov wrote: > > On Tue, Mar 17, 2020 at 01:35:12PM -0700, Dave Hansen wrote: > > > On 3/17/20 4:18 AM, Borislav Petkov wrote: > > > > Back then when the whole SME machinery started getting mainlined, it > > > > was agreed that for simplicity, clarity and sanity's sake, the terms > > > > denoting encrypted and not-encrypted memory should be "encrypted" and > > > > "decrypted". And the majority of the code sticks to that convention > > > > except those two. So rename them. > > > Don't "unencrypted" and "decrypted" mean different things? > > > > > > Unencrypted to me means "encryption was never used for this data". > > > > > > Decrypted means "this was/is encrypted but here is a plaintext copy". > > Maybe but linguistical semantics is not the point here. > > > > The idea is to represent a "binary" concept of memory being encrypted > > or memory being not encrypted. And at the time we decided to use > > "encrypted" and "decrypted" for those two things. > > Yeah, agreed. We're basically trying to name "!encrypted". > > > Do you see the need to differentiate a third "state", so to speak, of > > memory which was never encrypted? > > No, there are just two states. I just think the "!encrypted" case > should not be called "decrypted". Nor do I, it's completely misleading.
[PATCH -next 000/491] treewide: use fallthrough;
There is a new fallthrough pseudo-keyword macro that can be used
to replace the various /* fallthrough */ style comments that are
used to indicate a case label code block is intended to fallthrough
to the next case label block.
See commit 294f69e662d1 ("compiler_attributes.h: Add 'fallthrough'
pseudo keyword for switch/case use")
These patches are intended to allow clang to detect missing
switch/case fallthrough uses.
Do a depth-first pass on the MAINTAINERS file and find the various
F: pattern files and convert the fallthrough comments to fallthrough;
for all files matched by all F: patterns in in each section.
Done via the perl script below and the previously posted
cvt_fallthrough.pl script.
Link:
https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/
These patches are based on next-20200310 and are available in
git://repo.or.cz/linux-2.6/trivial-mods.git in branch 20200310_fallthrough_2
$ cat commit_fallthrough.pl
#!/usr/bin/env perl
use sort 'stable';
#
# Reorder a sorted array so file entries are before directory entries
# depends on a trailing / for directories
# so:
# foo/
# foo/bar.c
# becomes
# foo/bar.c
# foo/
#
sub file_before_directory {
my ($array_ref) = (@_);
my $count = scalar(@$array_ref);
for (my $i = 1; $i < $count; $i++) {
if (substr(@$array_ref[$i - 1], -1) eq '/' &&
substr(@$array_ref[$i], 0, length(@$array_ref[$i - 1])) eq
@$array_ref[$i - 1]) {
my $string = @$array_ref[$i - 1];
@$array_ref[$i - 1] = @$array_ref[$i];
@$array_ref[$i] = $string;
}
}
}
sub uniq {
my (@parms) = @_;
my %saw;
@parms = grep(!$saw{$_}++, @parms);
return @parms;
}
# Get all the F: file patterns in MAINTAINERS that could be a .[ch] file
my $maintainer_patterns = `grep -P '^F:\\s+' MAINTAINERS`;
my @patterns = split('\n', $maintainer_patterns);
s/^F:\s*// for @patterns;
@patterns = grep(!/^(?:Documentation|tools|scripts)\//, @patterns);
@patterns = grep(!/\.(?:dtsi?|rst|config)$/, @patterns);
@patterns = sort @patterns;
@patterns = sort { $b =~ tr/\//\// cmp $a =~ tr/\//\// } @patterns;
file_before_directory(\@patterns);
my %sections_done;
foreach my $pattern (@patterns) {
# Find the files the pattern matches
my $pattern_files = `git ls-files -- $pattern`;
my @new_patterns = split('\n', $pattern_files);
$pattern_files = join(' ', @new_patterns);
next if ($pattern_files =~ /^\s*$/);
# Find the section the first file matches
my $pattern_file = @new_patterns[0];
my $section_output = `./scripts/get_maintainer.pl --nogit --nogit-fallback
--sections --pattern-depth=1 $pattern_file`;
my @section = split('\n', $section_output);
my $section_header = @section[0];
print("Section: <$section_header>\n");
# Skip the section if it's already done
print("Already done '$section_header'\n") if
($sections_done{$section_header});
next if ($sections_done{$section_header}++);
# Find all the .[ch] files in all F: lines in that section
my @new_section;
foreach my $line (@section) {
last if ($line =~ /^\s*$/);
push(@new_section, $line);
}
@section = grep(/^F:/, @new_section);
s/^F:\s*// for @section;
@section = grep(!/^(?:Documentation|tools|scripts)\//, @section);
@section = grep(!/\.(?:dtsi?|rst|config)$/, @section);
@section = sort @section;
@section = uniq(@section);
my $section_files = join(' ', @section);
print("section_files: <$section_files>\n");
next if ($section_files =~ /^\s*$/);
my $cvt_files = `git ls-files -- $section_files`;
my @files = split('\n', $cvt_files);
@files = grep(!/^(?:Documentation|tools|scripts)\//, @files);
@files = grep(!/\.(?:dtsi?|rst|config)$/, @files);
@files = grep(/\.[ch]$/, @files);
@files = sort @files;
@files = uniq(@files);
$cvt_files = join(' ', @files);
print("files: <$cvt_files>\n");
next if (scalar(@files) < 1);
# Convert fallthroughs for all [.ch] files in the section
print("doing cvt_fallthrough.pl -- $cvt_files\n");
`cvt_fallthrough.pl -- $cvt_files`;
# If nothing changed, nothing to commit
`git diff-index --quiet HEAD --`;
next if (!$?);
# Commit the changes
my $fh;
open($fh, "+>", "cvt_fallthrough.commit_msg") or die "$0: can't create
temporary file: $!\n";
print $fh <https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git@perches.com/
EOF
;
close $fh;
`git commit -s -a -F cvt_fallthrough.commit_msg`;
}
Joe Perches (491):
MELLANOX ETHERNET INNOVA DRIVERS: Use fallthrough;
MARVELL OCTEONTX2 RVU ADMIN FUNCTION DRI
[PATCH -next 017/491] CELL BROADBAND ENGINE ARCHITECTURE: Use fallthrough;
Convert the various uses of fallthrough comments to fallthrough; Done via script Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/ Signed-off-by: Joe Perches --- arch/powerpc/platforms/cell/spufs/switch.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/cell/spufs/switch.c b/arch/powerpc/platforms/cell/spufs/switch.c index 5c3f5d0..d56b4e3 100644 --- a/arch/powerpc/platforms/cell/spufs/switch.c +++ b/arch/powerpc/platforms/cell/spufs/switch.c @@ -177,7 +177,7 @@ static inline void save_mfc_cntl(struct spu_state *csa, struct spu *spu) POLL_WHILE_FALSE((in_be64(&priv2->mfc_control_RW) & MFC_CNTL_SUSPEND_DMA_STATUS_MASK) == MFC_CNTL_SUSPEND_COMPLETE); - /* fall through */ + fallthrough; case MFC_CNTL_SUSPEND_COMPLETE: if (csa) csa->priv2.mfc_control_RW = -- 2.24.0
[PATCH -next 016/491] KERNEL VIRTUAL MACHINE FOR POWERPC (KVM/powerpc): Use fallthrough;
Convert the various uses of fallthrough comments to fallthrough; Done via script Link: https://lore.kernel.org/lkml/b56602fcf79f849e733e7b521bb0e17895d390fa.1582230379.git.joe.com/ Signed-off-by: Joe Perches --- arch/powerpc/kvm/book3s_32_mmu.c | 2 +- arch/powerpc/kvm/book3s_64_mmu.c | 2 +- arch/powerpc/kvm/book3s_pr.c | 2 +- arch/powerpc/kvm/booke.c | 6 +++--- arch/powerpc/kvm/powerpc.c | 1 - 5 files changed, 6 insertions(+), 7 deletions(-) diff --git a/arch/powerpc/kvm/book3s_32_mmu.c b/arch/powerpc/kvm/book3s_32_mmu.c index f21e734..3fbd57 100644 --- a/arch/powerpc/kvm/book3s_32_mmu.c +++ b/arch/powerpc/kvm/book3s_32_mmu.c @@ -234,7 +234,7 @@ static int kvmppc_mmu_book3s_32_xlate_pte(struct kvm_vcpu *vcpu, gva_t eaddr, case 2: case 6: pte->may_write = true; - /* fall through */ + fallthrough; case 3: case 5: case 7: diff --git a/arch/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c index 5991332..26b8b2 100644 --- a/arch/powerpc/kvm/book3s_64_mmu.c +++ b/arch/powerpc/kvm/book3s_64_mmu.c @@ -311,7 +311,7 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr, case 2: case 6: gpte->may_write = true; - /* fall through */ + fallthrough; case 3: case 5: case 7: diff --git a/arch/powerpc/kvm/book3s_pr.c b/arch/powerpc/kvm/book3s_pr.c index 729a0f..7db3695 100644 --- a/arch/powerpc/kvm/book3s_pr.c +++ b/arch/powerpc/kvm/book3s_pr.c @@ -740,7 +740,7 @@ int kvmppc_handle_pagefault(struct kvm_run *run, struct kvm_vcpu *vcpu, (vcpu->arch.hflags & BOOK3S_HFLAG_SPLIT_HACK) && ((pte.raddr & SPLIT_HACK_MASK) == SPLIT_HACK_OFFS)) pte.raddr &= ~SPLIT_HACK_MASK; - /* fall through */ + fallthrough; case MSR_IR: vcpu->arch.mmu.esid_to_vsid(vcpu, eaddr >> SID_SHIFT, &vsid); diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index 7b27604..be47815 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -421,11 +421,11 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, case BOOKE_IRQPRIO_DATA_STORAGE: case BOOKE_IRQPRIO_ALIGNMENT: update_dear = true; - /* fall through */ + fallthrough; case BOOKE_IRQPRIO_INST_STORAGE: case BOOKE_IRQPRIO_PROGRAM: update_esr = true; - /* fall through */ + fallthrough; case BOOKE_IRQPRIO_ITLB_MISS: case BOOKE_IRQPRIO_SYSCALL: case BOOKE_IRQPRIO_FP_UNAVAIL: @@ -459,7 +459,7 @@ static int kvmppc_booke_irqprio_deliver(struct kvm_vcpu *vcpu, case BOOKE_IRQPRIO_DECREMENTER: case BOOKE_IRQPRIO_FIT: keep_irq = true; - /* fall through */ + fallthrough; case BOOKE_IRQPRIO_EXTERNAL: case BOOKE_IRQPRIO_DBELL: allowed = vcpu->arch.shared->msr & MSR_EE; diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 1af96fb5..c242a79 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -525,7 +525,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext) r = 1; break; case KVM_CAP_PPC_GUEST_DEBUG_SSTEP: - /* fall through */ case KVM_CAP_PPC_PAIRED_SINGLES: case KVM_CAP_PPC_OSI: case KVM_CAP_PPC_GET_PVINFO: -- 2.24.0
Re: [PATCH -next] powerpc/maple: fix comparing pointer to 0
On Tue, 2020-01-21 at 01:47 -0600, Segher Boessenkool wrote: > On Mon, Jan 20, 2020 at 05:52:15PM -0800, Joe Perches wrote: > > On Tue, 2020-01-21 at 09:31 +0800, Chen Zhou wrote: > > > Fixes coccicheck warning: > > > ./arch/powerpc/platforms/maple/setup.c:232:15-16: > > > WARNING comparing pointer to 0 > > > > Does anyone have or use these powerpc maple boards anymore? > > > > Maybe the whole codebase should just be deleted instead. > > This is used for *all* non-Apple 970 systems (not running virtualized), > not just actual Maple. OK, then likely this Kconfig description should be updated (and the http://www.970eval.com link is no longer about powerpc) $ cat arch/powerpc/platforms/maple/Kconfig # SPDX-License-Identifier: GPL-2.0 config PPC_MAPLE depends on PPC64 && PPC_BOOK3S && CPU_BIG_ENDIAN bool "Maple 970FX Evaluation Board" select FORCE_PCI select MPIC select U3_DART select MPIC_U3_HT_IRQS select GENERIC_TBSYNC select PPC_UDBG_16550 select PPC_970_NAP select PPC_NATIVE select PPC_RTAS select MMIO_NVRAM select ATA_NONSTANDARD if ATA help This option enables support for the Maple 970FX Evaluation Board. For more information, refer to <http://www.970eval.com>
Re: [PATCH -next] powerpc/maple: fix comparing pointer to 0
On Tue, 2020-01-21 at 09:31 +0800, Chen Zhou wrote:
> Fixes coccicheck warning:
> ./arch/powerpc/platforms/maple/setup.c:232:15-16:
> WARNING comparing pointer to 0
Does anyone have or use these powerpc maple boards anymore?
Maybe the whole codebase should just be deleted instead.
If not, setup.c has an unused DBG macro that could be removed too.
---
arch/powerpc/platforms/maple/setup.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/arch/powerpc/platforms/maple/setup.c
b/arch/powerpc/platforms/maple/setup.c
index 47f7310..d6a083c 100644
--- a/arch/powerpc/platforms/maple/setup.c
+++ b/arch/powerpc/platforms/maple/setup.c
@@ -57,12 +57,6 @@
#include "maple.h"
-#ifdef DEBUG
-#define DBG(fmt...) udbg_printf(fmt)
-#else
-#define DBG(fmt...)
-#endif
-
static unsigned long maple_find_nvram_base(void)
{
struct device_node *rtcs;
Re: [PATCH v2 00/33] Kill pr_warning in the whole linux code
(Adding Stephen Rothwell) On Fri, 2019-10-18 at 17:22 +0200, Christoph Hellwig wrote: > As I said before: please just send Linus a scripted conversion after > the next -rc1. There is no point in creating all this churn. I again ask for a scripted mechanism to be added to -next to allow these types of scriptable patches to get compilation coverage testing.
Re: [EXTERNAL][PATCH 1/5] PCI: Convert pci_resource_to_user to a weak function
On Sun, 2019-07-28 at 22:49 +, Paul Burton wrote:
> Hi Denis,
>
> On Sun, Jul 28, 2019 at 11:22:09PM +0300, Denis Efremov wrote:
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 9e700d9f9f28..1a19d0151b0a 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1870,25 +1870,13 @@ static inline const char *pci_name(const struct
> > pci_dev *pdev)
> > return dev_name(&pdev->dev);
> > }
> >
> > -
> > /*
> > * Some archs don't want to expose struct resource to userland as-is
> > * in sysfs and /proc
> > */
> > -#ifdef HAVE_ARCH_PCI_RESOURCE_TO_USER
> > -void pci_resource_to_user(const struct pci_dev *dev, int bar,
> > - const struct resource *rsrc,
> > - resource_size_t *start, resource_size_t *end);
> > -#else
> > -static inline void pci_resource_to_user(const struct pci_dev *dev, int bar,
> > - const struct resource *rsrc, resource_size_t *start,
> > - resource_size_t *end)
> > -{
> > - *start = rsrc->start;
> > - *end = rsrc->end;
> > -}
> > -#endif /* HAVE_ARCH_PCI_RESOURCE_TO_USER */
> > -
> > +void __weak pci_resource_to_user(const struct pci_dev *dev, int bar,
> > +const struct resource *rsrc,
> > +resource_size_t *start, resource_size_t *end);
> >
> > /*
> > * The world is not perfect and supplies us with broken PCI devices.
>
> This is wrong - using __weak on the declaration in a header will cause
> the weak attribute to be applied to all implementations too (presuming
> the C files containing the implementations include the header). You then
> get whichever impleentation the linker chooses, which isn't necessarily
> the one you wanted.
>
> checkpatch.pl should produce an error about this - see the
> WEAK_DECLARATION error introduced in commit 619a908aa334 ("checkpatch:
> add error on use of attribute((weak)) or __weak declarations").
Unfortunately, checkpatch is pretty stupid and only emits
this on single line declarations.
[PATCH 7/8] tty: hvcs: Fix odd use of strlcpy
Use the typical style of array, not the equivalent &array[0]. Signed-off-by: Joe Perches --- drivers/tty/hvc/hvcs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/tty/hvc/hvcs.c b/drivers/tty/hvc/hvcs.c index cb4db1b3ca3c..b6c1c1be06f9 100644 --- a/drivers/tty/hvc/hvcs.c +++ b/drivers/tty/hvc/hvcs.c @@ -871,8 +871,8 @@ static void hvcs_set_pi(struct hvcs_partner_info *pi, struct hvcs_struct *hvcsd) hvcsd->p_partition_ID = pi->partition_ID; /* copy the null-term char too */ - strlcpy(&hvcsd->p_location_code[0], - &pi->location_code[0], sizeof(hvcsd->p_location_code)); + strlcpy(hvcsd->p_location_code, pi->location_code, + sizeof(hvcsd->p_location_code)); } /* -- 2.15.0
[PATCH 0/8] treewide: correct misuses of strscpy/strlcpy
These are all likely copy/paste defects where the field size of the 'copied to' array is incorrect. Each patch in this series is independent. Joe Perches (8): Input: synaptics: Fix misuse of strlcpy leds: as3645a: Fix misuse of strlcpy media: m2m-deinterlace: Fix misuse of strscpy media: go7007: Fix misuse of strscpy net: ethernet: sun4i-emac: Fix misuse of strlcpy net: nixge: Fix misuse of strlcpy tty: hvcs: Fix odd use of strlcpy nfsd: Fix misuse of strlcpy drivers/input/mouse/synaptics.c | 2 +- drivers/leds/leds-as3645a.c | 2 +- drivers/media/platform/m2m-deinterlace.c| 2 +- drivers/media/usb/go7007/snd-go7007.c | 2 +- drivers/net/ethernet/allwinner/sun4i-emac.c | 4 ++-- drivers/net/ethernet/ni/nixge.c | 2 +- drivers/tty/hvc/hvcs.c | 4 ++-- fs/nfsd/nfs4idmap.c | 2 +- 8 files changed, 10 insertions(+), 10 deletions(-) -- 2.15.0
Re: [PATCH v2] powerpc/setup_64: fix -Wempty-body warnings
On Thu, 2019-06-27 at 15:52 -0400, Qian Cai wrote:
> On Wed, 2019-06-05 at 16:53 -0400, Qian Cai wrote:
> > At the beginning of setup_64.c, it has,
> >
> > #ifdef DEBUG
> > #define DBG(fmt...) udbg_printf(fmt)
> > #else
> > #define DBG(fmt...)
> > #endif
> >
> > where DBG() could be compiled away, and generate warnings,
> >
> > arch/powerpc/kernel/setup_64.c: In function 'initialize_cache_info':
> > arch/powerpc/kernel/setup_64.c:579:49: warning: suggest braces around
> > empty body in an 'if' statement [-Wempty-body]
> > DBG("Argh, can't find dcache properties !\n");
> > ^
> > arch/powerpc/kernel/setup_64.c:582:49: warning: suggest braces around
> > empty body in an 'if' statement [-Wempty-body]
> > DBG("Argh, can't find icache properties !\n");
[]
> > diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
[]
> > @@ -71,7 +71,7 @@
> > #ifdef DEBUG
> > #define DBG(fmt...) udbg_printf(fmt)
> > #else
> > -#define DBG(fmt...)
> > +#define DBG(fmt...) do { } while (0)
I suggest
#define DBG(fmt...) do { if (0) udbg_printf(fmt); } while (0)
so that format and argument are always verified by the compiler.
I would also prefer a more generic form using ##__VA_ARGS__
#ifdef DEBUG
#define DBG(fmt, ...) udbg_printf(fmt, ##__VA_ARGS__)
#else
#define DBG(fmt, ...) do { if (0) udbg_printf(fmt, ##__VA_ARGS__); } while (0)
#endif
or maybe use the no_printk macro like
#ifdef DEBUG
#define DBG(fmt, ...) udbg_printf(fmt, ##__VA_ARGS__)
#else
#define DBG(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
#endif
Re: [PATCH 7/7] powerpc/kprobes: Allow probing on any ftrace address
On Fri, 2019-06-21 at 23:50 +0900, Masami Hiramatsu wrote:
> On Tue, 18 Jun 2019 20:17:06 +0530
> "Naveen N. Rao" wrote:
trivia:
> > diff --git a/arch/powerpc/kernel/kprobes-ftrace.c
> > b/arch/powerpc/kernel/kprobes-ftrace.c
[]
> > @@ -57,6 +82,11 @@ NOKPROBE_SYMBOL(kprobe_ftrace_handler);
> >
> > int arch_prepare_kprobe_ftrace(struct kprobe *p)
> > {
> > + if ((unsigned long)p->addr & 0x03) {
> > + printk("Attempt to register kprobe at an unaligned address\n");
Please use the appropriate KERN_ or pr_
[PATCH] powerpc/powernv: Rename pe_level_printk to pe_printk and embed KERN_LEVEL in format
Remove the separate KERN_ from each pe_level_printk and
instead add the KERN_ to the format.
pfix in pe_level_printk could also be used uninitialized so
add a new else and set pfx to the hex value of pe->flags.
Rename pe_level_printk to pe_printk and update the pe_
macros.
Signed-off-by: Joe Perches
---
arch/powerpc/platforms/powernv/pci-ioda.c | 14 --
arch/powerpc/platforms/powernv/pci.h | 11 +--
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c
b/arch/powerpc/platforms/powernv/pci-ioda.c
index 10cc42b9e541..60fc36ae626a 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -50,15 +50,23 @@
static const char * const pnv_phb_names[] = { "IODA1", "IODA2", "NPU_NVLINK",
"NPU_OCAPI" };
-void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
- const char *fmt, ...)
+void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...)
{
struct va_format vaf;
va_list args;
char pfix[32];
+ char level[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = "\0";
va_start(args, fmt);
+ while (printk_get_level(fmt)) {
+ size_t size = printk_skip_level(fmt) - fmt;
+
+ memcpy(level, fmt, size);
+ level[size] = '\0';
+ fmt += size;
+ }
+
vaf.fmt = fmt;
vaf.va = &args;
@@ -74,6 +82,8 @@ void pe_level_printk(const struct pnv_ioda_pe *pe, const char
*level,
(pe->rid & 0xff00) >> 8,
PCI_SLOT(pe->rid), PCI_FUNC(pe->rid));
#endif /* CONFIG_PCI_IOV*/
+ else
+ sprintf(pfix, "(flags: 0x%lx)", pe->flags);
printk("%spci %s: [PE# %.2x] %pV",
level, pfix, pe->pe_number, &vaf);
diff --git a/arch/powerpc/platforms/powernv/pci.h
b/arch/powerpc/platforms/powernv/pci.h
index be26ab3d99e0..870b21f55b3f 100644
--- a/arch/powerpc/platforms/powernv/pci.h
+++ b/arch/powerpc/platforms/powernv/pci.h
@@ -205,15 +205,14 @@ extern unsigned long pnv_pci_ioda2_get_table_size(__u32
page_shift,
__u64 window_size, __u32 levels);
extern int pnv_eeh_post_init(void);
-__printf(3, 4)
-extern void pe_level_printk(const struct pnv_ioda_pe *pe, const char *level,
- const char *fmt, ...);
+__printf(2, 3)
+extern void pe_printk(const struct pnv_ioda_pe *pe, const char *fmt, ...);
#define pe_err(pe, fmt, ...) \
- pe_level_printk(pe, KERN_ERR, fmt, ##__VA_ARGS__)
+ pe_printk(pe, KERN_ERR fmt, ##__VA_ARGS__)
#define pe_warn(pe, fmt, ...) \
- pe_level_printk(pe, KERN_WARNING, fmt, ##__VA_ARGS__)
+ pe_printk(pe, KERN_WARNING fmt, ##__VA_ARGS__)
#define pe_info(pe, fmt, ...) \
- pe_level_printk(pe, KERN_INFO, fmt, ##__VA_ARGS__)
+ pe_printk(pe, KERN_INFO fmt, ##__VA_ARGS__)
/* Nvlink functions */
extern void pnv_npu_try_dma_set_bypass(struct pci_dev *gpdev, bool bypass);
Re: [PATCH v1 02/15] crypto: talitos - rename alternative AEAD algos.
On Tue, 2019-05-21 at 17:54 +, Christophe Leroy wrote:
> Hi Joe & Andy
[]
> diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
[]
> > @@ -2356,7 +2356,7 @@ static struct talitos_alg_template driver_algs[] = {
> > .base = {
> > .cra_name = "authenc(hmac(sha1),cbc(aes))",
> > .cra_driver_name = "authenc-hmac-sha1-"
> > - "cbc-aes-talitos",
> > + "cbc-aes-talitos-hsna",
>
> checkpatch reports the following warning on the above:
>
> WARNING: quoted string split across lines
> #27: FILE: drivers/crypto/talitos.c:2359:
> .cra_driver_name = "authenc-hmac-sha1-"
> +"cbc-aes-talitos-hsna",
>
>
> But when I fixes the patch as follows, I get another warning:
>
> @@ -2355,8 +2355,7 @@ static struct talitos_alg_template driver_algs[] = {
> .alg.aead = {
> .base = {
> .cra_name = "authenc(hmac(sha1),cbc(aes))",
> - .cra_driver_name = "authenc-hmac-sha1-"
> -"cbc-aes-talitos",
> + .cra_driver_name =
> "authenc-hmac-sha1-cbc-aes-talitos-hsna",
> .cra_blocksize = AES_BLOCK_SIZE,
> .cra_flags = CRYPTO_ALG_ASYNC,
> },
>
>
>
> WARNING: line over 80 characters
> #28: FILE: drivers/crypto/talitos.c:2358:
> + .cra_driver_name =
> "authenc-hmac-sha1-cbc-aes-talitos-hsna",
>
>
> So, how should this be fixed ?
For at least now, by ignoring this checkpatch message.
It's a brainless script. You are not brainless.
Re: [PATCH] powerpc/mm: mark more tlb functions as __always_inline
On Tue, 2019-05-21 at 16:27 +0900, Masahiro Yamada wrote: > On Tue, May 21, 2019 at 3:54 PM Christophe Leroy > > powerpc accepts lines up to 90 chars, see arch/powerpc/tools/checkpatch.pl > > Ugh, I did not know this. Horrible. > > The Linux coding style should be global in the kernel tree. > No subsystem should adopts its own coding style. I don't see a problem using 90 column lines by arch/ There are other subsystem specific variations like the net/ /* multiline comments without initial blank comment lines * look like this... */ If there were arch specific drivers with style variations in say drivers/net, then that might be more of an issue.
Re: [PATCH] powerpc/mm: mark more tlb functions as __always_inline
On Tue, 2019-05-21 at 08:53 +0200, Christophe Leroy wrote: > powerpc accepts lines up to 90 chars, see arch/powerpc/tools/checkpatch.pl arch/powerpc/tools/checkpatch.sh
Re: [PATCH] powerpc/powernv/ioda2: Add __printf format/argument verification
On Fri, 2019-05-03 at 16:59 +1000, Michael Ellerman wrote: > On Thu, 2017-03-30 at 10:19:25 UTC, Joe Perches wrote: > > Fix fallout too. > > > > Signed-off-by: Joe Perches > > Applied to powerpc next, thanks. > > https://git.kernel.org/powerpc/c/1e496391a8452101308a23b7395cdd49 2+ years later.
Re: [PATCH] MAINTAINERS: Update remaining @linux.vnet.ibm.com addresses
On Thu, 2019-04-11 at 06:12 -0700, Paul E. McKenney wrote: > If my email address were > to change again, I would instead go with the "(IBM)" approach and let > the git log and MAINTAINERS file keep the contact information. Not that > we get to update the git log, of course. ;-) Add entries to .mailmap works too.
Re: [PATCH] MAINTAINERS: Update remaining @linux.vnet.ibm.com addresses
On Thu, 2019-04-11 at 22:07 +1000, Michael Ellerman wrote:
> Joe Perches writes:
> > On Thu, 2019-04-11 at 06:27 +0200, Lukas Bulwahn wrote:
> > > Paul McKenney attempted to update all email addresses @linux.vnet.ibm.com
> > > to @linux.ibm.com in commit 1dfddcdb95c4
> > > ("MAINTAINERS: Update from @linux.vnet.ibm.com to @linux.ibm.com"), but
> > > some still remained.
> > >
> > > We update the remaining email addresses in MAINTAINERS, hopefully finally
> > > catching all cases for good.
> >
> > Perhaps update all the similar addresses in other files too
> >
> > $ git grep --name-only 'linux\.vnet\.ibm\.com' | wc -l
> > 315
>
> A good number of them are no longer valid. So I'm not sure it's worth
> updating them en masse to addresses that won't ever work.
>
> We have git now, we don't need email addresses in files, they're just
> prone to bitrot like this.
>
> Should we just change them all like so?
>
> -arch/powerpc/boot/dts/bamboo.dts: * Josh Boyer
> +arch/powerpc/boot/dts/bamboo.dts: * Josh Boyer (IBM)
>
> To indicate the author was at IBM when they wrote it?
If that's desired, perhaps:
$ git grep -P --name-only '?' | \
grep -vP '\.mailmap|MAINTAINERS' | \
xargs perl -p -i -e 's/?/(IBM)/g'
> Or should we try and update them with current addresses? Though then the
> authors might start getting mails they don't want.
That'd be my preference.
If authors get emails they don't want, then those contact
emails should be removed.
Re: [PATCH] MAINTAINERS: Update remaining @linux.vnet.ibm.com addresses
On Thu, 2019-04-11 at 06:27 +0200, Lukas Bulwahn wrote:
> Paul McKenney attempted to update all email addresses @linux.vnet.ibm.com
> to @linux.ibm.com in commit 1dfddcdb95c4
> ("MAINTAINERS: Update from @linux.vnet.ibm.com to @linux.ibm.com"), but
> some still remained.
>
> We update the remaining email addresses in MAINTAINERS, hopefully finally
> catching all cases for good.
Perhaps update all the similar addresses in other files too
$ git grep --name-only 'linux\.vnet\.ibm\.com' | wc -l
315
Re: Bad file pattern in MAINTAINERS section 'IBM Power Virtual Accelerator Switchboard'
On Mon, 2019-03-25 at 16:35 -0700, Joe Perches wrote: > A file pattern line in this section of the MAINTAINERS file in linux-next > does not have a match in the linux source files. > > This could occur because a matching filename was never added, was deleted > or renamed in some other commit. > > The commits that added and if found renamed or removed the file pattern > are shown below. > > Please fix this defect appropriately. > > 1: --- > > linux-next MAINTAINERS section: > > 7396IBM Power Virtual Accelerator Switchboard > 7397M: Sukadev Bhattiprolu btw: Your name needs an email address here too. This should be: M: Sukadev Bhattiprolu
Bad file pattern in MAINTAINERS section 'IBM Power Virtual Accelerator Switchboard'
A file pattern line in this section of the MAINTAINERS file in linux-next does not have a match in the linux source files. This could occur because a matching filename was never added, was deleted or renamed in some other commit. The commits that added and if found renamed or removed the file pattern are shown below. Please fix this defect appropriately. 1: --- linux-next MAINTAINERS section: 7396IBM Power Virtual Accelerator Switchboard 7397M: Sukadev Bhattiprolu 7398L: linuxppc-dev@lists.ozlabs.org 7399S: Supported 7400F: arch/powerpc/platforms/powernv/vas* 7401F: arch/powerpc/platforms/powernv/copy-paste.h 7402F: arch/powerpc/include/asm/vas.h --> 7403F: arch/powerpc/include/uapi/asm/vas.h 2: --- The most recent commit that added or modified file pattern 'arch/powerpc/include/uapi/asm/vas.h': commit 4dea2d1a927c61114a168d4509b56329ea6effb7 Author: Sukadev Bhattiprolu Date: Mon Aug 28 23:23:33 2017 -0700 powerpc/powernv/vas: Define vas_init() and vas_exit() Implement vas_init() and vas_exit() functions for a new VAS module. This VAS module is essentially a library for other device drivers and kernel users of the NX coprocessors like NX-842 and NX-GZIP. In the future this will be extended to add support for user space to access the NX coprocessors. VAS is currently only supported with 64K page size. Signed-off-by: Sukadev Bhattiprolu Signed-off-by: Michael Ellerman .../devicetree/bindings/powerpc/ibm,vas.txt| 22 +++ MAINTAINERS| 8 ++ arch/powerpc/platforms/powernv/Kconfig | 14 ++ arch/powerpc/platforms/powernv/Makefile| 1 + arch/powerpc/platforms/powernv/vas-window.c| 19 +++ arch/powerpc/platforms/powernv/vas.c | 151 + arch/powerpc/platforms/powernv/vas.h | 2 + 7 files changed, 217 insertions(+) 3: --- No commit with file pattern 'arch/powerpc/include/uapi/asm/vas.h' was found
Re: [PATCH v3 2/5] ocxl: Clean up printf formats
On Wed, 2019-03-20 at 16:34 +1100, Alastair D'Silva wrote: > From: Alastair D'Silva > > Use %# instead of using a literal '0x' I do not suggest this as reasonable. There are 10's of thousands of uses of 0x%x in the kernel and converting them to save a byte seems unnecessary. $ git grep -P '0x%[\*\d\.]*[xX]' | wc -l 26120 And the %#x style is by far the lesser used form $ git grep -P '%#[\*\d\.]*[xX]' | wc -l 2726 Also, the sized form of %#[size]x is frequently misused where the size does not account for the initial 0x output. > diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c [] > @@ -178,9 +178,9 @@ static int read_dvsec_vendor(struct pci_dev *dev) > pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_DLX_VERS, &dlx); > > dev_dbg(&dev->dev, "Vendor specific DVSEC:\n"); > - dev_dbg(&dev->dev, " CFG version = 0x%x\n", cfg); > - dev_dbg(&dev->dev, " TLX version = 0x%x\n", tlx); > - dev_dbg(&dev->dev, " DLX version = 0x%x\n", dlx); > + dev_dbg(&dev->dev, " CFG version = %#x\n", cfg); > + dev_dbg(&dev->dev, " TLX version = %#x\n", tlx); > + dev_dbg(&dev->dev, " DLX version = %#x\n", dlx); etc...
Re: [PATCH 2/5] ocxl: Clean up printf formats
On Wed, 2019-02-27 at 15:57 +1100, Alastair D'Silva wrote: > From: Alastair D'Silva > > Use %# instead of using a literal '0x' I think it's better not to change this unless the compilation unit already uses a mix of styles. Overall, the kernel uses "0x%" over "%#" by ~8:1 $ git grep -P '0x%\d*[hl]*x' | wc -l 27654 $ git grep -P '%#\d*[hl]*x' | wc -l 3454 > diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c [] > @@ -178,9 +178,9 @@ static int read_dvsec_vendor(struct pci_dev *dev) > pci_read_config_dword(dev, pos + OCXL_DVSEC_VENDOR_DLX_VERS, &dlx); > > dev_dbg(&dev->dev, "Vendor specific DVSEC:\n"); > - dev_dbg(&dev->dev, " CFG version = 0x%x\n", cfg); > - dev_dbg(&dev->dev, " TLX version = 0x%x\n", tlx); > - dev_dbg(&dev->dev, " DLX version = 0x%x\n", dlx); > + dev_dbg(&dev->dev, " CFG version = %#x\n", cfg); > + dev_dbg(&dev->dev, " TLX version = %#x\n", tlx); > + dev_dbg(&dev->dev, " DLX version = %#x\n", dlx); [...]
Re: [PATCH] arch/powerpc: Use dma_zalloc_coherent
On Sat, 2018-11-17 at 12:40 +0530, Souptick Joarder wrote:
> Hi Joe,
Hi back.
> On Fri, Nov 16, 2018 at 12:55 AM Joe Perches wrote:
> > On Thu, 2018-11-15 at 23:29 +0530, Sabyasachi Gupta wrote:
> > > On Mon, Nov 5, 2018 at 8:58 AM Sabyasachi Gupta
> > > wrote:
> > > > Replaced dma_alloc_coherent + memset with dma_zalloc_coherent
> > > >
> > > > Signed-off-by: Sabyasachi Gupta
> > >
> > > Any comment on this patch?
> >
> > It's obviously correct.
> >
> > You might realign the arguments on the next lines
> > to the open parenthesis.
> >
> > Perhaps there should be new function calls
> > added for symmetry to the other alloc functions
> > for multiplication overflow protection.
> >
> > Perhaps:
> >
> > void *dma_alloc_array_coherent()
> > void *dma_calloc_coherent()
> >
> > Something like
> > ---
> > include/linux/dma-mapping.h | 19 +++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> > index 15bd41447025..95bebf8883b1 100644
> > --- a/include/linux/dma-mapping.h
> > +++ b/include/linux/dma-mapping.h
> > @@ -565,6 +565,25 @@ static inline void *dma_alloc_coherent(struct device
> > *dev, size_t size,
> > (gfp & __GFP_NOWARN) ? DMA_ATTR_NO_WARN : 0);
> > }
> >
> > +static inline void *dma_alloc_array_coherent(struct device *dev,
> > +size_t n, size_t size,
> > +dma_addr_t *dma_handle, gfp_t
> > gfp)
> > +{
> > + size_t bytes;
> > +
> > + if (unlikely(check_mul_overflow(n, size, &bytes)))
> > + return NULL;
> > + return dma_alloc_coherent(dev, bytes, dma_handle, gfp);
> > +}
> > +
> > +static inline void *dma_calloc_coherent(struct device *dev,
> > + size_t n, size_t size,
> > + dma_addr_t *dma_handle, gfp_t gfp)
> > +{
> > + return dma_alloc_array_coherent(dev, n, size, dma_handle,
> > + gfp | __GFP_ZERO);
> > +}
> > +
>
> If I understood correctly, you are talking about adding these 2 new inline
> functions. We can do that.
>
> Can you please help to understand the consumers of these 2 new inline ?
Any call to dma_alloc_coherent with a multiply
_might_overflow the multiplication. The
check_mul_overflow simply avoids the overflow.
Re: [PATCH] arch/powerpc: Use dma_zalloc_coherent
On Thu, 2018-11-15 at 23:29 +0530, Sabyasachi Gupta wrote:
> On Mon, Nov 5, 2018 at 8:58 AM Sabyasachi Gupta
> wrote:
> > Replaced dma_alloc_coherent + memset with dma_zalloc_coherent
> >
> > Signed-off-by: Sabyasachi Gupta
>
> Any comment on this patch?
It's obviously correct.
You might realign the arguments on the next lines
to the open parenthesis.
Perhaps there should be new function calls
added for symmetry to the other alloc functions
for multiplication overflow protection.
Perhaps:
void *dma_alloc_array_coherent()
void *dma_calloc_coherent()
Something like
---
include/linux/dma-mapping.h | 19 +++
1 file changed, 19 insertions(+)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 15bd41447025..95bebf8883b1 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -565,6 +565,25 @@ static inline void *dma_alloc_coherent(struct device *dev,
size_t size,
(gfp & __GFP_NOWARN) ? DMA_ATTR_NO_WARN : 0);
}
+static inline void *dma_alloc_array_coherent(struct device *dev,
+size_t n, size_t size,
+dma_addr_t *dma_handle, gfp_t gfp)
+{
+ size_t bytes;
+
+ if (unlikely(check_mul_overflow(n, size, &bytes)))
+ return NULL;
+ return dma_alloc_coherent(dev, bytes, dma_handle, gfp);
+}
+
+static inline void *dma_calloc_coherent(struct device *dev,
+ size_t n, size_t size,
+ dma_addr_t *dma_handle, gfp_t gfp)
+{
+ return dma_alloc_array_coherent(dev, n, size, dma_handle,
+ gfp | __GFP_ZERO);
+}
+
static inline void dma_free_coherent(struct device *dev, size_t size,
void *cpu_addr, dma_addr_t dma_handle)
{
---
> > diff --git a/arch/powerpc/platforms/pasemi/dma_lib.c
> > b/arch/powerpc/platforms/pasemi/dma_lib.c
[]
> > @@ -255,15 +255,13 @@ int pasemi_dma_alloc_ring(struct pasemi_dmachan
> > *chan, int ring_size)
> >
> > chan->ring_size = ring_size;
> >
> > - chan->ring_virt = dma_alloc_coherent(&dma_pdev->dev,
> > + chan->ring_virt = dma_zalloc_coherent(&dma_pdev->dev,
> > ring_size * sizeof(u64),
> > &chan->ring_dma, GFP_KERNEL);
> > en
> > if (!chan->ring_virt)
> > return -ENOMEM;
> >
> > - memset(chan->ring_virt, 0, ring_size * sizeof(u64));
> > -
> > return 0;
> > }
> > EXPORT_SYMBOL(pasemi_dma_alloc_ring);
Re: [PATCH] mm: convert totalram_pages, totalhigh_pages and managed_pages to atomic.
On Mon, 2018-10-22 at 22:53 +0530, Arun KS wrote: > Remove managed_page_count_lock spinlock and instead use atomic > variables. Perhaps better to define and use macros for the accesses instead of specific uses of atomic_long_ Something like: #define totalram_pages()(unsigned long)atomic_long_read(&_totalram_pages) #define totalram_pages_inc()(unsigned long)atomic_long_inc(&_totalram_pages) #define totalram_pages_dec()(unsigned long)atomic_long_dec(&_totalram_pages)
Re: [PATCH v3 13/18] of: overlay: check prevents multiple fragments touching same property
On Sun, 2018-10-14 at 18:52 -0700, Frank Rowand wrote: > On 10/14/18 18:06, Joe Perches wrote: > > On Sun, 2018-10-14 at 17:24 -0700, frowand.l...@gmail.com wrote: > > > From: Frank Rowand > > > > > > Add test case of two fragments updating the same property. After > > > adding the test case, the system hangs at end of boot, after > > > after slub stack dumps from kfree() in crypto modprobe code. [] > > I think this is worse performance than before. > > > > This now walks all entries when before it would > > return -EINVAL directly when it found a match. > > Yes, it is worse performance, but that is OK. > > This is a check that is done when a devicetree overlay is applied. > If an error occurs then that means that the overlay was incorrectly > specified. The file drivers/of/unittest-data/overlay_bad_add_dup_prop.dts > in this patch provides an example of how a bad overlay can be created. > > Once an error was detected, the check could return immediately, or it > could continue to give a complete list of detected errors. I chose to > give the complete list of detected errors. Swell. Please describe that in the commit message.
Re: [PATCH v3 13/18] of: overlay: check prevents multiple fragments touching same property
On Sun, 2018-10-14 at 17:24 -0700, frowand.l...@gmail.com wrote:
> From: Frank Rowand
>
> Add test case of two fragments updating the same property. After
> adding the test case, the system hangs at end of boot, after
> after slub stack dumps from kfree() in crypto modprobe code.
[]
> -static int check_changeset_dup_add_node(struct overlay_changeset *ovcs)
> +static int changeset_dup_entry_check(struct overlay_changeset *ovcs)
> {
> - struct of_changeset_entry *ce_1, *ce_2;
> - char *fn_1, *fn_2;
> - int name_match;
> + struct of_changeset_entry *ce_1;
> + int dup_entry = 0;
>
> list_for_each_entry(ce_1, &ovcs->cset.entries, node) {
> -
> - if (ce_1->action == OF_RECONFIG_ATTACH_NODE ||
> - ce_1->action == OF_RECONFIG_DETACH_NODE) {
> -
> - ce_2 = ce_1;
> - list_for_each_entry_continue(ce_2, &ovcs->cset.entries,
> node) {
> - if (ce_2->action == OF_RECONFIG_ATTACH_NODE ||
> - ce_2->action == OF_RECONFIG_DETACH_NODE) {
> - /* inexpensive name compare */
> - if (!of_node_cmp(ce_1->np->full_name,
> - ce_2->np->full_name)) {
> - /* expensive full path name
> compare */
> - fn_1 = kasprintf(GFP_KERNEL,
> "%pOF", ce_1->np);
> - fn_2 = kasprintf(GFP_KERNEL,
> "%pOF", ce_2->np);
> - name_match = !strcmp(fn_1,
> fn_2);
> - kfree(fn_1);
> - kfree(fn_2);
> - if (name_match) {
> - pr_err("ERROR: multiple
> overlay fragments add and/or delete node %pOF\n",
> -ce_1->np);
> - return -EINVAL;
> - }
> - }
> - }
> - }
> - }
> + dup_entry |= find_dup_cset_node_entry(ovcs, ce_1);
> + dup_entry |= find_dup_cset_prop(ovcs, ce_1);
I think this is worse performance than before.
This now walks all entries when before it would
return -EINVAL directly when it found a match.
Re: [PATCH v2 12/18] of: overlay: check prevents multiple fragments add or delete same node
On Fri, 2018-10-12 at 21:53 -0700, frowand.l...@gmail.com wrote:
> From: Frank Rowand
>
> Multiple overlay fragments adding or deleting the same node is not
> supported. Replace code comment of such, with check to detect the
> attempt and fail the overlay apply.
>
> Devicetree unittest where multiple fragments added the same node was
> added in the previous patch in the series. After applying this patch
> the unittest messages will no longer include:
>
>Duplicate name in motor-1, renamed to "controller#1"
>OF: overlay: of_overlay_apply() err=0
>### dt-test ### of_overlay_fdt_apply() expected -22, ret=0,
> overlay_bad_add_dup_node
>### dt-test ### FAIL of_unittest_overlay_high_level():2419 Adding overlay
> 'overlay_bad_add_dup_node' failed
>
>...
>
>### dt-test ### end of unittest - 210 passed, 1 failed
>
> but will instead include:
>
>OF: overlay: ERROR: multiple overlay fragments add and/or delete node
> /testcase-data-2/substation@100/motor-1/controller
>
>...
>
>### dt-test ### end of unittest - 211 passed, 0 failed
[]
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
[]
> @@ -523,6 +515,54 @@ static int build_changeset_symbols_node(struct
> overlay_changeset *ovcs,
> }
>
> /**
> + * check_changeset_dup_add_node() - changeset validation: duplicate add node
> + * @ovcs:Overlay changeset
> + *
> + * Check changeset @ovcs->cset for multiple add node entries for the same
> + * node.
> + *
> + * Returns 0 on success, -ENOMEM if memory allocation failure, or -EINVAL if
> + * invalid overlay in @ovcs->fragments[].
> + */
> +static int check_changeset_dup_add_node(struct overlay_changeset *ovcs)
> +{
> + struct of_changeset_entry *ce_1, *ce_2;
> + char *fn_1, *fn_2;
> + int name_match;
> +
> + list_for_each_entry(ce_1, &ovcs->cset.entries, node) {
> +
> + if (ce_1->action == OF_RECONFIG_ATTACH_NODE ||
> + ce_1->action == OF_RECONFIG_DETACH_NODE) {
> +
> + ce_2 = ce_1;
> + list_for_each_entry_continue(ce_2, &ovcs->cset.entries,
> node) {
> + if (ce_2->action == OF_RECONFIG_ATTACH_NODE ||
> + ce_2->action == OF_RECONFIG_DETACH_NODE) {
> + /* inexpensive name compare */
> + if (!of_node_cmp(ce_1->np->full_name,
> + ce_2->np->full_name)) {
A bit of odd indentation here.
This line is normally aligned to the second ( on the line above.
> + /* expensive full path name
> compare */
> + fn_1 = kasprintf(GFP_KERNEL,
> "%pOF", ce_1->np);
> + fn_2 = kasprintf(GFP_KERNEL,
> "%pOF", ce_2->np);
> + name_match = !strcmp(fn_1,
> fn_2);
> + kfree(fn_1);
> + kfree(fn_2);
> + if (name_match) {
> + pr_err("ERROR: multiple
> overlay fragments add and/or delete node %pOF\n",
> +ce_1->np);
> + return -EINVAL;
> + }
> + }
> + }
> + }
> + }
> + }
> +
> + return 0;
> +}
Style trivia:
Using inverted tests and continue would reduce indentation.
list_for_each_entry(ce_1, &ovcs->cset.entries, node) {
if (ce_1->action != OF_RECONFIG_ATTACH_NODE &&
ce_1->action != OF_RECONFIG_DETACH_NODE)
continue;
ce_2 = ce_1;
list_for_each_entry_continue(ce_2, &ovcs->cset.entries, node) {
if (ce_2->action != OF_RECONFIG_ATTACH_NODE &&
ce_2->action != OF_RECONFIG_DETACH_NODE)
continue;
/* inexpensive name compare */
if (of_node_cmp(ce_1->np->full_name,
ce_2->np->full_name))
continue;
/* expensive full path name compare */
fn_1 = kasprintf(GFP_KERNEL, "%pOF", ce_1->np);
fn_2 = kasprintf(GFP_KERNEL, "%pOF", ce_2->np);
name_match = !strcmp(fn_1, fn_2);
kfree(fn_1);
kfree(fn_2);
if (name_match) {
pr_err("ERROR: multiple overlay fragments add
and/or delete node %pOF\n",
ce_1->np);
return -EINVAL;
Re: [PATCH 15/36] dt-bindings: arm: Convert Actions Semi bindings to jsonschema
On Sat, 2018-10-06 at 12:40 +0200, Andreas Färber wrote: > > +++ b/Documentation/devicetree/bindings/arm/actions.yaml [] > > + > > +title: Actions Semi platforms device tree bindings > > + > > +maintainers: > > + - Andreas Färber > > Mani is now officially reviewer and the closest I have to a > co-maintainer. I suggest we add him here in some form. I assume this is > independent of MAINTAINERS patterns though, or will get_maintainers.pl > parse this, too? It _could_, if using the get_maintainers --file-emails option. Ideally, it would be added to the MAINTAINERS somewhere.
Re: [PATCH] powerpc/xmon/ppc-opc: Use ARRAY_SIZE macro
On Tue, 2018-10-09 at 14:43 +1100, Michael Ellerman wrote:
> Joe Perches writes:
>
> > On Thu, 2018-10-04 at 19:10 +0200, Gustavo A. R. Silva wrote:
> > > Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
> > []
> > > diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
> > []
> > > @@ -966,8 +966,7 @@ const struct powerpc_operand powerpc_operands[] =
> > >{ 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
> > > };
> > >
> > > -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> > > -/ sizeof (powerpc_operands[0]));
> > > +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
> >
> > It seems this is unused and could be deleted.
>
> The code in this file is copied from binutils.
>
> We don't want to needlessly diverge it.
>
> I've said this before:
>
>
> https://lore.kernel.org/linuxppc-dev/874lfxjnzl@concordia.ellerman.id.au/
Don't expect people to remember this.
> Is there some way we can blacklist this file from checkpatch, Coccinelle
> etc?
Modify both to look for some specific tag
in a file and then update the scripts to
read the file when looking at patches too.
Otherwise, no.
Re: [PATCH] powerpc/xmon/ppc-opc: Use ARRAY_SIZE macro
On Thu, 2018-10-04 at 19:10 +0200, Gustavo A. R. Silva wrote:
> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
[]
> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
[]
> @@ -966,8 +966,7 @@ const struct powerpc_operand powerpc_operands[] =
>{ 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
> };
>
> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> -/ sizeof (powerpc_operands[0]));
> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
It seems this is unused and could be deleted.
> /* The functions used to insert and extract complicated operands. */
>
> @@ -6980,8 +6979,7 @@ const struct powerpc_opcode powerpc_opcodes[] = {
> {"fcfidu.", XRC(63,974,1), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT,
> FRB}},
> };
>
> -const int powerpc_num_opcodes =
> - sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
This is used once and should probably be replaced where
it is used with ARRAY_SIZE
> /* The VLE opcode table.
>
> @@ -7219,8 +7217,7 @@ const struct powerpc_opcode vle_opcodes[] = {
> {"se_bl",BD8(58,0,1),BD8_MASK, PPCVLE, 0, {B8}},
> };
>
> -const int vle_num_opcodes =
> - sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
> +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
Also apparently unused and could be deleted.
>
> /* The macro table. This is only used by the assembler. */
>
> @@ -7288,5 +7285,4 @@ const struct powerpc_macro powerpc_macros[] = {
> {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
> };
> ld
> -const int powerpc_num_macros =
> - sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
Also apparently unused and could be deleted.
Re: Checkpatch bad Warning (Re: [PATCH] powerpc/kgdb: add kgdb_arch_set/remove_breakpoint())
On Tue, 2018-09-18 at 09:33 +, Christophe Leroy wrote: > On the below patch, checkpatch reports > > WARNING: struct kgdb_arch should normally be const > #127: FILE: arch/powerpc/kernel/kgdb.c:480: > +struct kgdb_arch arch_kgdb_ops; > > But when I add 'const', I get compilation failure So don't add const. checkpatch is stupid. You are not. _Always_ take checkpatch bleats with very large grains of salt. Perhaps send a patch to remove kgbd_arch from scripts/const_structs.checkpatch as it seems not ever to be const.
Re: [PATCH 2/2] powerpc: Use ARRAY_SIZE to replace its implementation
On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
> Use ARRAY_SIZE instead of dividing sizeof array with sizeof an element.
> So just replace it.
Better to remove the extern and the const altogether here as well.
$ git grep -w powerpc_num_opcodes
arch/powerpc/xmon/ppc-dis.c: opcode_end = powerpc_opcodes +
powerpc_num_opcodes;
arch/powerpc/xmon/ppc-opc.c:const int powerpc_num_opcodes =
arch/powerpc/xmon/ppc.h:extern const int powerpc_num_opcodes;
And this one could be removed instead:
$ git grep -w vle_num_opcodes
arch/powerpc/xmon/ppc-opc.c:const int vle_num_opcodes =
arch/powerpc/xmon/ppc.h:extern const int vle_num_opcodes;
> Signed-off-by: zhong jiang
> ---
> arch/powerpc/xmon/ppc-opc.c | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/xmon/ppc-opc.c b/arch/powerpc/xmon/ppc-opc.c
> index ac2b55b..f3f57a1 100644
> --- a/arch/powerpc/xmon/ppc-opc.c
> +++ b/arch/powerpc/xmon/ppc-opc.c
> @@ -966,8 +966,7 @@
>{ 0xff, 11, NULL, NULL, PPC_OPERAND_SIGNOPT },
> };
>
> -const unsigned int num_powerpc_operands = (sizeof (powerpc_operands)
> -/ sizeof (powerpc_operands[0]));
> +const unsigned int num_powerpc_operands = ARRAY_SIZE(powerpc_operands);
>
> /* The functions used to insert and extract complicated operands. */
>
> @@ -6980,8 +6979,7 @@
> {"fcfidu.", XRC(63,974,1), XRA_MASK, POWER7|PPCA2, PPCVLE, {FRT,
> FRB}},
> };
>
> -const int powerpc_num_opcodes =
> - sizeof (powerpc_opcodes) / sizeof (powerpc_opcodes[0]);
> +const int powerpc_num_opcodes = ARRAY_SIZE(powerpc_opcodes);
>
> /* The VLE opcode table.
>
> @@ -7219,8 +7217,7 @@
> {"se_bl",BD8(58,0,1),BD8_MASK, PPCVLE, 0, {B8}},
> };
>
> -const int vle_num_opcodes =
> - sizeof (vle_opcodes) / sizeof (vle_opcodes[0]);
> +const int vle_num_opcodes = ARRAY_SIZE(vle_opcodes);
>
> /* The macro table. This is only used by the assembler. */
>
> @@ -7288,5 +7285,4 @@
> {"e_clrlslwi",4, PPCVLE, "e_rlwinm %0,%1,%3,(%2)-(%3),31-(%3)"},
> };
>
> -const int powerpc_num_macros =
> - sizeof (powerpc_macros) / sizeof (powerpc_macros[0]);
> +const int powerpc_num_macros = ARRAY_SIZE(powerpc_macros);
Re: [PATCH 1/2] ia64: Use ARRAY_SIZE to replace its implementation
On Tue, 2018-08-14 at 10:46 +0800, zhong jiang wrote:
> We prefer to ARRAY_SIZE rather than duplicating its implementation.
> So just replace it.
[]
> diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
[]
> @@ -4645,7 +4645,7 @@ static char *pfmfs_dname(struct dentry *dentry, char
> *buffer, int buflen)
> /* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY,
> pfarg_dbreg_t, NULL),
> /* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY,
> pfarg_dbreg_t, NULL)
> };
> -#define PFM_CMD_COUNT(sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
> +#define PFM_CMD_COUNTARRAY_SIZE(pfm_cmd_tab)
Better would be to remove the #define altogether and change
the one place where it's used to ARRAY_SIZE(...)
---
arch/ia64/kernel/perfmon.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index a9d4dc6c0427..08ece2c7b6e1 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -4645,7 +4645,6 @@ static pfm_cmd_desc_t pfm_cmd_tab[]={
/* 32 */PFM_CMD(pfm_write_ibrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY,
pfarg_dbreg_t, NULL),
/* 33 */PFM_CMD(pfm_write_dbrs, PFM_CMD_PCLRWS, PFM_CMD_ARG_MANY,
pfarg_dbreg_t, NULL)
};
-#define PFM_CMD_COUNT (sizeof(pfm_cmd_tab)/sizeof(pfm_cmd_desc_t))
static int
pfm_check_task_state(pfm_context_t *ctx, int cmd, unsigned long flags)
@@ -4770,7 +4769,7 @@ sys_perfmonctl (int fd, int cmd, void __user *arg, int
count)
*/
if (unlikely(pmu_conf == NULL)) return -ENOSYS;
- if (unlikely(cmd < 0 || cmd >= PFM_CMD_COUNT)) {
+ if (unlikely(cmd < 0 || cmd >= ARRAY_SIZE(pfm_cmd_tab)) {
DPRINT(("invalid cmd=%d\n", cmd));
return -EINVAL;
}
Re: [PATCH v4 5/6] powerpc: Add show_user_instructions()
On Thu, 2018-08-02 at 21:42 -0300, Murilo Opsfelder Araujo wrote:
> > > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
[]
> > > @@ -1299,6 +1299,46 @@ static void show_instructions(struct pt_regs *regs)
> > > pr_cont("\n");
> > > }
> > > +void show_user_instructions(struct pt_regs *regs)
> > > +{
> > > + int i;
> > > + const char *prefix = KERN_INFO "%s[%d]: code: ";
> > > + unsigned long pc = regs->nip - (instructions_to_print * 3 / 4 *
> > > + sizeof(int));
> > > +
> > > + printk(prefix, current->comm, current->pid);
> >
> > Why not use pr_info() and remove KERN_INFO from *prefix ?
>
> Because it doesn't compile:
>
> arch/powerpc/kernel/process.c:1317:10: error: expected ‘)’ before ‘prefix’
> pr_info(prefix, current->comm, current->pid);
> ^
> ./include/linux/printk.h:288:21: note: in definition of macro ‘pr_fmt’
>#define pr_fmt(fmt) fmt
> ^
What being suggested is using:
pr_info("%s[%d]: code: ", current->comm, current->pid);
Re: [PATCH v3 5/9] powerpc/traps: Print signal name for unhandled signals
On Wed, 2018-08-01 at 08:37 +0200, Christophe LEROY wrote:
> Le 31/07/2018 à 16:50, Murilo Opsfelder Araujo a écrit :
> > This adds a human-readable name in the unhandled signal message.
> > Before this patch, a page fault looked like:
> >pandafault[6303]: unhandled signal 11 at 17d0 nip 161c lr
> > 7fff93c55100 code 2 in pandafault[1000+1]
> > After this patch, a page fault looks like:
> >pandafault[6352]: segfault (11) at 13a2a09f8 nip 13a2a086c lr
> > 7fffb63e5100 code 2 in pandafault[13a2a+1]
]]
> > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
[]
> > @@ -96,6 +96,41 @@ EXPORT_SYMBOL(__debugger_fault_handler);
> > #define TM_DEBUG(x...) do { } while(0)
> > #endif
> >
> > +static const char *signames[SIGRTMIN + 1] = {
> > + "UNKNOWN",
> > + "SIGHUP", // 1
> > + "SIGINT", // 2
[]
> I don't think is is worth having that full table when we only use a few
> of them. (As discussed in v1 https://patchwork.ozlabs.org/patch/948802/)
>
> I would suggest to instead use a function like this:
>
> static const char *signame(int signr)
> {
> if (signr == SIGBUS)
> return "bus error";
> if (signr == SIGFPE)
> return "floating point exception";
> if (signr == SIGILL)
> return "illegal instruction";
> if (signr == SIGILL)
> return "segfault";
> if (signr == SIGTRAP)
> return "unhandled trap";
> return "unknown signal";
> }
trivia:
Unless the if tests are ordered most to least likely,
perhaps it would be better to use a switch/case and
let the compiler decide.
switch (signr) {
case SIGBUS:return "bus error";
case SIGFPE:return "floating point exception";
case SIGILL:return "illegal instruction";
case SIGSEGV: return "segfault";
case SIGTRAP: return "unhandled trap";
}
return "unknown signal";
}
Re: [PATCH v2 04/10] powerpc/traps: Use REG_FMT in show_signal_msg()
On Fri, 2018-07-27 at 18:40 +0200, LEROY Christophe wrote:
> Murilo Opsfelder Araujo a écrit :
>
> > Simplify the message format by using REG_FMT as the register format. This
> > avoids having two different formats and avoids checking for MSR_64BIT.
>
> Are you sure it is what we want ?
>
> Won't it change the behaviour for a 32 bits app running on a 64bits kernel ?
[]
> > diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
[]
> > @@ -311,17 +311,13 @@ static bool show_unhandled_signals_ratelimited(void)
> > static void show_signal_msg(int signr, struct pt_regs *regs, int code,
> > unsigned long addr)
> > {
> > - const char fmt32[] = KERN_INFO "%s[%d]: unhandled signal %d " \
> > - "at %08lx nip %08lx lr %08lx code %x\n";
> > - const char fmt64[] = KERN_INFO "%s[%d]: unhandled signal %d " \
> > - "at %016lx nip %016lx lr %016lx code %x\n";
> > -
> > if (!unhandled_signal(current, signr))
> > return;
> >
> > - printk(regs->msr & MSR_64BIT ? fmt64 : fmt32,
> > - current->comm, current->pid, signr,
> > - addr, regs->nip, regs->link, code);
> > + pr_info("%s[%d]: unhandled signal %d at "REG_FMT \
I think it better to use a space after the close "
and also the line continuation is unnecessary.
> > + " nip "REG_FMT" lr "REG_FMT" code %x\n",
And spaces before the open quotes too.
I'd also prefer the format on a single line:
pr_info("%s[%d]: unhandled signal %d at " REG_FMT " nip " REG_FMT " lr
" REG_FMT " code %x\n",
> > + current->comm, current->pid, signr, addr,
> > + regs->nip, regs->link, code);
Seeing as these are all unsigned long, a better way to do
this is to use %p and cast to pointer.
This might be better anyway as this output exposes pointer
addresses and instead would now use pointer hashed output.
pr_info("%s[%d]: unhandled signal %d at %p nip %p lr %p code %x\n",
current->comm, current->pid, signr,
(void *)addr, (void *)regs->nip, (void *)regs->link, code);
Use %px if you _really_ need to emit unhashed addresses.
see: Documentation/core-api/printk-formats.rst
