Re: [Xen-devel] [PATCH] scripts: warn about invalid MAINTAINERS patterns
On Wed, 2017-11-01 at 16:05 -0400, Augie Fackler wrote: > > On Nov 1, 2017, at 13:11, Tom Saeger wrote: > > > > On Wed, Nov 01, 2017 at 09:50:05AM -0700, Joe Perches wrote: > > > (add mercurial-devel and xen-devel to cc's) > > > > > > On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote: > > > > Add "--pattern-checks" option to get_maintainer.pl to warn about invalid > > > > "F" and "X" patterns found in MAINTAINERS file(s). > > > > > > Hey again Tom. > > > > > > About mercurial/hg. > > > > > > While as far as I know there hasn't been a mercurial tree > > > for the linux kernel sources in many years, I believe the > > > mercurial command to list files should be different. > > > > > > > my %VCS_cmds_hg = ( > > > > @@ -167,6 +169,7 @@ my %VCS_cmds_hg = ( > > > > "subject_pattern" => "^HgSubject: (.*)", > > > > "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$", > > > > "file_exists_cmd" => "hg files \$file", > > > > +"list_files_cmd" => "hg files \$file", > > > > > > I think this should be > > > > > > "list_files_cmd" => "hg manifest -R \$file", > > > > Ok - I'll add to v2. > > Actually, I'd recommend `hg files` over `hg manifest` by a wide margin. why? hg files -R prefixes all the output hg manifest -R output is unprefixed ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/1] scripts: warn about invalid MAINTAINERS patterns
On Wed, 2017-11-01 at 13:13 -0500, Tom Saeger wrote:
> Add "--self-test" option to get_maintainer.pl to show potential
> issues in MAINTAINERS file(s) content.
This patch subject should be:
get_maintainer: Add --self-test for internal consistency tests
Andrew, can you please change the subject if/when you add it
to quilt?
> Pattern check warnings are shown for "F" and "X" patterns found in
> MAINTAINERS file(s) which do not match any files known by git.
>
> Signed-off-by: Tom Saeger
> Cc: Joe Perches
Otherwise,
Acked-by: Joe Perches
> ---
>
> v2:
>
> Incorporated suggestions from Joe Perches:
> - changed "--pattern-checks" to "--self-test" to allow for future work.
> - fixed vcs command "list_files_cmd" for mercurial.
> - "--self-test" option is all or nothing.
> - output to STDOUT
> - output format in emacs-style "filename:line: message"
> - changed self-test help to:
>
> --self-test => show potential issues with MAINTAINERS file content
>
> (Joe, I slightly reworded in hopes this rendition is clear and future proof).
>
> - Moved execution of $self_test to just after $help and $version.
> This prompted encapsulating main content code to read MAINTAINERS files into
> a function (read_all_maintainer_files) callable from $self_test. This
> has the side benefit of not having to special case for "$self_test" in other
> parts
> of main program flow.
This makes sense to me and is better program flow, thanks.
cheers, Joe
[v2 patch quoted below]
> scripts/get_maintainer.pl | 94
> ++-
> 1 file changed, 77 insertions(+), 17 deletions(-)
>
> diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
> index bc443201d3ef..c68a5d1ba709 100755
> --- a/scripts/get_maintainer.pl
> +++ b/scripts/get_maintainer.pl
> @@ -57,6 +57,7 @@ my $sections = 0;
> my $file_emails = 0;
> my $from_filename = 0;
> my $pattern_depth = 0;
> +my $self_test = 0;
> my $version = 0;
> my $help = 0;
> my $find_maintainer_files = 0;
> @@ -138,6 +139,7 @@ my %VCS_cmds_git = (
> "subject_pattern" => "^GitSubject: (.*)",
> "stat_pattern" => "^(\\d+)\\t(\\d+)\\t\$file\$",
> "file_exists_cmd" => "git ls-files \$file",
> +"list_files_cmd" => "git ls-files \$file",
> );
>
> my %VCS_cmds_hg = (
> @@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
> "subject_pattern" => "^HgSubject: (.*)",
> "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
> "file_exists_cmd" => "hg files \$file",
> +"list_files_cmd" => "hg manifest -R \$file",
> );
>
> my $conf = which_conf(".get_maintainer.conf");
> @@ -216,6 +219,14 @@ if (-f $ignore_file) {
> close($ignore);
> }
>
> +if ($#ARGV > 0) {
> +foreach (@ARGV) {
> +if ($_ eq "-self-test" || $_ eq "--self-test") {
> +die "$P: using --self-test does not allow any other option or
> argument\n";
> +}
> +}
> +}
> +
> if (!GetOptions(
> 'email!' => \$email,
> 'git!' => \$email_git,
> @@ -252,6 +263,7 @@ if (!GetOptions(
> 'fe|file-emails!' => \$file_emails,
> 'f|file' => \$from_filename,
> 'find-maintainer-files' => \$find_maintainer_files,
> + 'self-test' => \$self_test,
> 'v|version' => \$version,
> 'h|help|usage' => \$help,
> )) {
> @@ -268,6 +280,12 @@ if ($version != 0) {
> exit 0;
> }
>
> +if ($self_test) {
> +read_all_maintainer_files();
> +check_maintainers_patterns();
> +exit 0;
> +}
> +
> if (-t STDIN && !@ARGV) {
> # We're talking to a terminal, but have no command line arguments.
> die "$P: missing patchfile or -f file - use --help if necessary\n";
> @@ -311,12 +329,14 @@ if (!top_of_kernel_tree($lk_path)) {
> my @typevalue = ();
> my %keyword_hash;
> my @mfiles = ();
> +my @self_test_pattern_info = ();
>
> sub read_maintainer_file {
> my ($file) = @_;
>
> open (my $maint, '<', "$file")
> or die "$P: Can't open MAINTAINERS file '$file': $!\n";
> +my $i = 1;
> while (<$maint>) {
> my $line = $_;
>
> @@ -333,6 +353,9 @@ sub
Re: [Xen-devel] [PATCH] scripts: warn about invalid MAINTAINERS patterns
(add mercurial-devel and xen-devel to cc's)
On Tue, 2017-10-31 at 16:37 -0500, Tom Saeger wrote:
> Add "--pattern-checks" option to get_maintainer.pl to warn about invalid
> "F" and "X" patterns found in MAINTAINERS file(s).
Hey again Tom.
About mercurial/hg.
While as far as I know there hasn't been a mercurial tree
for the linux kernel sources in many years, I believe the
mercurial command to list files should be different.
> my %VCS_cmds_hg = (
> @@ -167,6 +169,7 @@ my %VCS_cmds_hg = (
> "subject_pattern" => "^HgSubject: (.*)",
> "stat_pattern" => "^(\\d+)\t(\\d+)\t\$file\$",
> "file_exists_cmd" => "hg files \$file",
> +"list_files_cmd" => "hg files \$file",
I think this should be
"list_files_cmd" => "hg manifest -R \$file",
It seems to work on a XEN test branch but does anyone
really care about hg support in get_maintainers?
btw: to the XEN maintainers
The XEN mercurial branch for MAINTAINERS has a few odd
entries and a few missing file patterns
I think the XEN MAINTAINERS file should be updated to:
---
diff -r c60f04b73240 MAINTAINERS
--- a/MAINTAINERS Mon Oct 16 15:24:44 2017 +0100
+++ b/MAINTAINERS Wed Nov 01 09:39:34 2017 -0700
@@ -246,7 +246,8 @@
KCONFIG
M: Doug Goldstein
S: Supported
-F: docs/misc/kconfig{,-language}.txt
+F: docs/misc/kconfig.txt
+F: docs/misc/kconfig-language.txt
F: xen/tools/kconfig/
KDD DEBUGGER
@@ -257,8 +258,8 @@
KEXEC
M: Andrew Cooper
S: Supported
-F: xen/common/{kexec,kimage}.c
-F: xen/include/{kexec,kimage}.h
+F: xen/common/kexec.[ch]
+F: xen/common/kimage.[ch]
F: xen/arch/x86/machine_kexec.c
F: xen/arch/x86/x86_64/kexec_reloc.S
---
After the patch above is applied, --self-test shows:
$ ~/linux/next/scripts/get_maintainer.pl --self-test
./MAINTAINERS:403: warning: no matches F: drivers/xen/usb*/
./MAINTAINERS:415: warning: no matches F:
xen/arch/x88/hvm/vm_event.c
./MAINTAINERS:429: warning: no matches F: extras/mini-os/tpm*
./MAINTAINERS:430: warning: no matches F:
extras/mini-os/include/tpm*
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] linux-next: manual merge of the xen-tip tree with the tip tree
On Thu, 2017-08-31 at 11:16 +0200, Ingo Molnar wrote:
> * Thomas Gleixner wrote:
> Low prio nitpicking, could we please write such table based initializers in a
> vertically organized, tabular fashion:
>
> > + { debug,xen_xendebug,
> > true },
> > + { int3, xen_xenint3,
> > true },
> > + { double_fault, xen_double_fault,
> > true },
> > +#ifdef CONFIG_X86_MCE
> > + { machine_check,xen_machine_check,
> > true },
> > +#endif
> > + { nmi, xen_nmi,
> > true },
> > + { overflow, xen_overflow,
> > false },
> > +#ifdef CONFIG_IA32_EMULATION
> > + { entry_INT80_compat, xen_entry_INT80_compat,
> > false },
> > +#endif
> > + { page_fault, xen_page_fault,
> > false },
> > + { divide_error, xen_divide_error,
> > false },
> > + { bounds, xen_bounds,
> > false },
> > + { invalid_op, xen_invalid_op,
> > false },
> > + { device_not_available, xen_device_not_available,
> > false },
> > + { coprocessor_segment_overrun, xen_coprocessor_segment_overrun,
> > false },
> > + { invalid_TSS, xen_invalid_TSS,
> > false },
> > + { segment_not_present, xen_segment_not_present,
> > false },
> > + { stack_segment,xen_stack_segment,
> > false },
> > + { general_protection, xen_general_protection,
> > false },
> > + { spurious_interrupt_bug, xen_spurious_interrupt_bug,
> > false },
> > + { coprocessor_error,xen_coprocessor_error,
> > false },
> > + { alignment_check, xen_alignment_check,
> > false },
> > + { simd_coprocessor_error, xen_simd_coprocessor_error,
> > false },
> > +#ifdef CONFIG_TRACING
> > + { trace_page_fault, xen_trace_page_fault,
> > false },
> > +#endif
> ,,
> ... as to me such a table is 100 times more readable - YMMV.
Yeah, kinda.
It's a lot of whitespace and eyeball left/right scanning.
And these tables require whitespace updating if a longer
name is ever used.
Given the near 1:1 mapping of to xen_
perhaps adding a macro would be nice.
#define xen_trap(trap, ist_ok) \
{ trap, xen_##trap, ist_ok }
{ debug,xen_xendebug, true },
{ int3, xen_xenint3,true },
#ifdef CONFIG_X86_MCE
xen_trap(machine_check, true),
#endif
xen_trap(double_fault, true),
xen_trap(nmi, true),
xen_trap(overflow, false),
...
ymmv.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration
On Tue, 2017-06-13 at 21:27 +0200, Juergen Gross wrote:
> On 13/06/17 18:31, Joe Perches wrote:
> > On Tue, 2017-06-13 at 18:13 +0200, Juergen Gross wrote:
> > > On 13/06/17 17:20, Ingo Molnar wrote:
> > > > * Juergen Gross wrote:
> > > >
> > > > > When running under Xen as dom0 /dev/mcelog is being registered by Xen
> > > > > instead of the normal mcelog driver. Avoid an error message being
> > > > > issued by the mcelog driver in this case. Instead issue an informative
> > > > > message that Xen has registered the device.
> >
> > []
> > > > > diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
> > > > > b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
> >
> > []
> > > > > @@ -388,9 +388,16 @@ static __init int dev_mcelog_init_device(void)
> > > > > /* register character device /dev/mcelog */
> > > > > err = misc_register(&mce_chrdev_device);
> > > > > if (err) {
> > > > > - pr_err("Unable to init device /dev/mcelog (rc: %d)\n",
> > > > > err);
> > > > > - return err;
> > > > > + if (err == -EBUSY)
> > > > > + /* Xen dom0 might have registered the device
> > > > > already. */
> > > > > + pr_info("Unable to init device /dev/mcelog,
> > > > > already registered");
> > > > > + else {
> > > > > + pr_err("Unable to init device /dev/mcelog (rc:
> > > > > %d)\n",
> > > > > +err);
> > > > > + return err;
> > > > > + }
> > > >
> > > > Please only use balanced curly braces in conditional statements.
> > >
> > > Okay, will change.
> >
> > Perhaps better is to reverse the test
> >
> > if (err != -EBUSY) {
> > pr_err("Unable to ", err);
> > return err;
> > }
> > pr_info("etc...");
>
> Okay.
>
> >
> > [ rest of code at this indentation ]
> >
> > but it looks like you added a logic defect too and
> > this code should be:
> >
> > if (err) {
> > if (err == -EBUSY)
> > pr_info(...)
> > else
> > pr_err(...)
> > return err;
> > }
> >
> > or less indented using
> >
> > err = misc_register(&mce_chrdev_device);
> > if (err == -EBUSY) {
> > pr_info(...);
> > return err;
> > } else if (err) {
> > pr_err(...);
> > return err;
> > }
>
> I didn't want to omit the call to mce_register_decode_chain() in the Xen
> case.
You should definitely mention that behavior change
in the changelog.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen/mce: don't issue error message for failed /dev/mcelog registration
On Tue, 2017-06-13 at 18:13 +0200, Juergen Gross wrote:
> On 13/06/17 17:20, Ingo Molnar wrote:
> > * Juergen Gross wrote:
> >
> > > When running under Xen as dom0 /dev/mcelog is being registered by Xen
> > > instead of the normal mcelog driver. Avoid an error message being
> > > issued by the mcelog driver in this case. Instead issue an informative
> > > message that Xen has registered the device.
[]
> > > diff --git a/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
> > > b/arch/x86/kernel/cpu/mcheck/dev-mcelog.c
[]
> > > @@ -388,9 +388,16 @@ static __init int dev_mcelog_init_device(void)
> > > /* register character device /dev/mcelog */
> > > err = misc_register(&mce_chrdev_device);
> > > if (err) {
> > > - pr_err("Unable to init device /dev/mcelog (rc: %d)\n", err);
> > > - return err;
> > > + if (err == -EBUSY)
> > > + /* Xen dom0 might have registered the device already. */
> > > + pr_info("Unable to init device /dev/mcelog, already
> > > registered");
> > > + else {
> > > + pr_err("Unable to init device /dev/mcelog (rc: %d)\n",
> > > +err);
> > > + return err;
> > > + }
> >
> > Please only use balanced curly braces in conditional statements.
>
> Okay, will change.
Perhaps better is to reverse the test
if (err != -EBUSY) {
pr_err("Unable to ", err);
return err;
}
pr_info("etc...");
[ rest of code at this indentation ]
but it looks like you added a logic defect too and
this code should be:
if (err) {
if (err == -EBUSY)
pr_info(...)
else
pr_err(...)
return err;
}
or less indented using
err = misc_register(&mce_chrdev_device);
if (err == -EBUSY) {
pr_info(...);
return err;
} else if (err) {
pr_err(...);
return err;
}
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn
On Thu, 2017-02-23 at 17:41 +, Emil Velikov wrote: > On 23 February 2017 at 17:18, Joe Perches wrote: > > On Thu, 2017-02-23 at 09:28 -0600, Rob Herring wrote: > > > On Fri, Feb 17, 2017 at 1:11 AM, Joe Perches wrote: > > > > There are ~4300 uses of pr_warn and ~250 uses of the older > > > > pr_warning in the kernel source tree. > > > > > > > > Make the use of pr_warn consistent across all kernel files. > > > > > > > > This excludes all files in tools/ as there is a separate > > > > define pr_warning for that directory tree and pr_warn is > > > > not used in tools/. > > > > > > > > Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing. > > > > [] > > > Where's the removal of pr_warning so we don't have more sneak in? > > > > After all of these actually get applied, > > and maybe a cycle or two later, one would > > get sent. > > > > By which point you'll get a few reincarnation of it. So you'll have to > do the same exercise again :-( Maybe to one or two files. Not a big deal. > I guess the question is - are you expecting to get the series merged > all together/via one tree ? No. The only person that could do that effectively is Linus. > If not, your plan is perfectly reasonable. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn
On Thu, 2017-02-23 at 09:28 -0600, Rob Herring wrote: > On Fri, Feb 17, 2017 at 1:11 AM, Joe Perches wrote: > > There are ~4300 uses of pr_warn and ~250 uses of the older > > pr_warning in the kernel source tree. > > > > Make the use of pr_warn consistent across all kernel files. > > > > This excludes all files in tools/ as there is a separate > > define pr_warning for that directory tree and pr_warn is > > not used in tools/. > > > > Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing. [] > Where's the removal of pr_warning so we don't have more sneak in? After all of these actually get applied, and maybe a cycle or two later, one would get sent. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH 09/35] x86: Convert remaining uses of pr_warning to pr_warn
To enable eventual removal of pr_warning
This makes pr_warn use consistent for arch/x86
Prior to this patch, there were 46 uses of pr_warning and
122 uses of pr_warn in arch/x86
Miscellanea:
o Coalesce a few formats and realign arguments
o Convert a couple of multiple line printks to single line
Signed-off-by: Joe Perches
---
arch/x86/kernel/amd_gart_64.c | 12 +++--
arch/x86/kernel/apic/apic.c| 46 --
arch/x86/kernel/apic/apic_noop.c | 2 +-
arch/x86/kernel/setup_percpu.c | 4 +--
arch/x86/kernel/tboot.c| 15 ++-
arch/x86/kernel/tsc_sync.c | 8 +++---
arch/x86/mm/kmmio.c| 8 +++---
arch/x86/mm/mmio-mod.c | 5 ++--
arch/x86/mm/numa.c | 12 -
arch/x86/mm/numa_emulation.c | 6 ++---
arch/x86/mm/testmmiotrace.c| 5 ++--
arch/x86/oprofile/op_x86_model.h | 6 ++---
arch/x86/platform/olpc/olpc-xo15-sci.c | 2 +-
arch/x86/platform/sfi/sfi.c| 3 +--
arch/x86/xen/debugfs.c | 2 +-
arch/x86/xen/setup.c | 2 +-
16 files changed, 63 insertions(+), 75 deletions(-)
diff --git a/arch/x86/kernel/amd_gart_64.c b/arch/x86/kernel/amd_gart_64.c
index 63ff468a7986..6bb37027cd70 100644
--- a/arch/x86/kernel/amd_gart_64.c
+++ b/arch/x86/kernel/amd_gart_64.c
@@ -535,10 +535,8 @@ static __init unsigned long check_iommu_size(unsigned long
aper, u64 aper_size)
iommu_size -= round_up(a, PMD_PAGE_SIZE) - a;
if (iommu_size < 64*1024*1024) {
- pr_warning(
- "PCI-DMA: Warning: Small IOMMU %luMB."
- " Consider increasing the AGP aperture in BIOS\n",
- iommu_size >> 20);
+ pr_warn("PCI-DMA: Warning: Small IOMMU %luMB. Consider
increasing the AGP aperture in BIOS\n",
+ iommu_size >> 20);
}
return iommu_size;
@@ -690,8 +688,7 @@ static __init int init_amd_gatt(struct agp_kern_info *info)
nommu:
/* Should not happen anymore */
- pr_warning("PCI-DMA: More than 4GB of RAM and no IOMMU\n"
- "falling back to iommu=soft.\n");
+ pr_warn("PCI-DMA: More than 4GB of RAM and no IOMMU - falling back to
iommu=soft\n");
return -1;
}
@@ -756,8 +753,7 @@ int __init gart_iommu_init(void)
!gart_iommu_aperture ||
(no_agp && init_amd_gatt(&info) < 0)) {
if (max_pfn > MAX_DMA32_PFN) {
- pr_warning("More than 4GB of memory but GART IOMMU not
available.\n");
- pr_warning("falling back to iommu=soft.\n");
+ pr_warn("More than 4GB of memory but GART IOMMU not
available - falling back to iommu=soft\n");
}
return 0;
}
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 4261b3282ad9..37e9129da8b3 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -685,8 +685,8 @@ calibrate_by_pmtimer(long deltapm, long *delta, long
*deltatsc)
res = (((u64)deltapm) * mult) >> 22;
do_div(res, 100);
- pr_warning("APIC calibration not consistent "
- "with PM-Timer: %ldms instead of 100ms\n",(long)res);
+ pr_warn("APIC calibration not consistent with PM-Timer: %ldms instead
of 100ms\n",
+ (long)res);
/* Correct the lapic counter value */
res = (((u64)(*delta)) * pm_100ms);
@@ -805,7 +805,7 @@ static int __init calibrate_APIC_clock(void)
*/
if (lapic_timer_frequency < (100 / HZ)) {
local_irq_enable();
- pr_warning("APIC frequency too slow, disabling apic timer\n");
+ pr_warn("APIC frequency too slow, disabling apic timer\n");
return -1;
}
@@ -848,8 +848,8 @@ static int __init calibrate_APIC_clock(void)
local_irq_enable();
if (levt->features & CLOCK_EVT_FEAT_DUMMY) {
- pr_warning("APIC timer disabled due to verification failure\n");
- return -1;
+ pr_warn("APIC timer disabled due to verification failure\n");
+ return -1;
}
return 0;
@@ -923,7 +923,7 @@ static void local_apic_timer_interrupt(void)
* spurious.
*/
if (!evt->event_handler) {
- pr_warning("Spurious LAPIC timer interrupt on cpu %d\n", cpu);
+ pr_warn("Spurious LAPIC timer interrupt on cpu %d\n", cpu);
/* Switch it off */
lapic_timer_shutdown(evt);
return;
@@ -1503,1
[Xen-devel] [PATCH 00/35] treewide trivial patches converting pr_warning to pr_warn
There are ~4300 uses of pr_warn and ~250 uses of the older pr_warning in the kernel source tree. Make the use of pr_warn consistent across all kernel files. This excludes all files in tools/ as there is a separate define pr_warning for that directory tree and pr_warn is not used in tools/. Done with 'sed s/\bpr_warning\b/pr_warn/' and some emacsing. Miscellanea: o Coalesce formats and realign arguments Some files not compiled - no cross-compilers Joe Perches (35): alpha: Convert remaining uses of pr_warning to pr_warn ARM: ep93xx: Convert remaining uses of pr_warning to pr_warn arm64: Convert remaining uses of pr_warning to pr_warn arch/blackfin: Convert remaining uses of pr_warning to pr_warn ia64: Convert remaining use of pr_warning to pr_warn powerpc: Convert remaining uses of pr_warning to pr_warn sh: Convert remaining uses of pr_warning to pr_warn sparc: Convert remaining use of pr_warning to pr_warn x86: Convert remaining uses of pr_warning to pr_warn drivers/acpi: Convert remaining uses of pr_warning to pr_warn block/drbd: Convert remaining uses of pr_warning to pr_warn gdrom: Convert remaining uses of pr_warning to pr_warn drivers/char: Convert remaining use of pr_warning to pr_warn clocksource: Convert remaining use of pr_warning to pr_warn drivers/crypto: Convert remaining uses of pr_warning to pr_warn fmc: Convert remaining use of pr_warning to pr_warn drivers/gpu: Convert remaining uses of pr_warning to pr_warn drivers/ide: Convert remaining uses of pr_warning to pr_warn drivers/input: Convert remaining uses of pr_warning to pr_warn drivers/isdn: Convert remaining uses of pr_warning to pr_warn drivers/macintosh: Convert remaining uses of pr_warning to pr_warn drivers/media: Convert remaining use of pr_warning to pr_warn drivers/mfd: Convert remaining uses of pr_warning to pr_warn drivers/mtd: Convert remaining uses of pr_warning to pr_warn drivers/of: Convert remaining uses of pr_warning to pr_warn drivers/oprofile: Convert remaining uses of pr_warning to pr_warn drivers/platform: Convert remaining uses of pr_warning to pr_warn drivers/rapidio: Convert remaining use of pr_warning to pr_warn drivers/scsi: Convert remaining use of pr_warning to pr_warn drivers/sh: Convert remaining use of pr_warning to pr_warn drivers/tty: Convert remaining uses of pr_warning to pr_warn drivers/video: Convert remaining uses of pr_warning to pr_warn kernel/trace: Convert remaining uses of pr_warning to pr_warn lib: Convert remaining uses of pr_warning to pr_warn sound/soc: Convert remaining uses of pr_warning to pr_warn arch/alpha/kernel/perf_event.c | 4 +- arch/arm/mach-ep93xx/core.c| 4 +- arch/arm64/include/asm/syscall.h | 8 ++-- arch/arm64/kernel/hw_breakpoint.c | 8 ++-- arch/arm64/kernel/smp.c| 4 +- arch/blackfin/kernel/nmi.c | 2 +- arch/blackfin/kernel/ptrace.c | 2 +- arch/blackfin/mach-bf533/boards/stamp.c| 2 +- arch/blackfin/mach-bf537/boards/cm_bf537e.c| 2 +- arch/blackfin/mach-bf537/boards/cm_bf537u.c| 2 +- arch/blackfin/mach-bf537/boards/stamp.c| 2 +- arch/blackfin/mach-bf537/boards/tcm_bf537.c| 2 +- arch/blackfin/mach-bf561/boards/cm_bf561.c | 2 +- arch/blackfin/mach-bf561/boards/ezkit.c| 2 +- arch/blackfin/mm/isram-driver.c| 4 +- arch/ia64/kernel/setup.c | 6 +-- arch/powerpc/kernel/pci-common.c | 4 +- arch/powerpc/mm/init_64.c | 5 +-- arch/powerpc/mm/mem.c | 3 +- arch/powerpc/platforms/512x/mpc512x_shared.c | 4 +- arch/powerpc/platforms/85xx/socrates_fpga_pic.c| 7 ++-- arch/powerpc/platforms/86xx/mpc86xx_hpcn.c | 2 +- arch/powerpc/platforms/pasemi/dma_lib.c| 4 +- arch/powerpc/platforms/powernv/opal.c | 8 ++-- arch/powerpc/platforms/powernv/pci-ioda.c | 10 ++--- arch/powerpc/platforms/ps3/device-init.c | 14 +++ arch/powerpc/platforms/ps3/mm.c| 4 +- arch/powerpc/platforms/ps3/os-area.c | 2 +- arch/powerpc/platforms/pseries/iommu.c | 8 ++-- arch/powerpc/platforms/pseries/setup.c | 4 +- arch/powerpc/sysdev/fsl_pci.c | 9 ++--- arch/powerpc/sysdev/mpic.c | 10 ++--- arch/powerpc/sysdev/xics/icp-native.c | 10 ++--- arch/powerpc/sysdev/xics/ics-opal.c| 4 +- arch/powerpc/sysdev/xics/ics-rtas.c| 4 +- arch/powerpc/sysdev/xics/xics-common.c | 8 ++-- arch/sh/boards/mach-sdk7786/nmi.c | 2 +- arch/sh/drivers/pci/fixups-sdk7786.c | 2 +- arch/sh/kernel/io
Re: [Xen-devel] [PATCH] xenbus: Neaten xenbus_va_dev_error
On Wed, 2017-02-08 at 12:14 -0500, Boris Ostrovsky wrote: > On 02/08/2017 11:05 AM, Joe Perches wrote: > > On Wed, 2017-02-08 at 10:33 -0500, Boris Ostrovsky wrote: > > > On 02/08/2017 06:33 AM, Joe Perches wrote: > > > > This function error patch can be simplified, so do so. > > > > > > > > Remove fail: label and somewhat obfuscating, used once "error_path" > > > > function. > > > > btw: I left it alone, but likely > > > > #define PRINTF_BUFFER_SIZE 4096 > > > > is probably excessive as the maximum printk > > buffer is 1024. > > > > The xenbus_write might be longer though so > > maybe it's OK to use 4096, but there is some > > inequivalence there. > > > > xenbus_write() handles writes up to 4K. However we are filling the > buffer with sprintf() which I assume is limited to 1K too vsnprintf is bounded only by addressable memory. ie: (void *)-1 > so it indeed > doesn't seem useful to have PRINTF_BUFFER_SIZE set to 4. > > -boris > ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xenbus: Neaten xenbus_va_dev_error
On Wed, 2017-02-08 at 10:33 -0500, Boris Ostrovsky wrote: > On 02/08/2017 06:33 AM, Joe Perches wrote: > > This function error patch can be simplified, so do so. > > > > Remove fail: label and somewhat obfuscating, used once "error_path" > > function. btw: I left it alone, but likely #define PRINTF_BUFFER_SIZE 4096 is probably excessive as the maximum printk buffer is 1024. The xenbus_write might be longer though so maybe it's OK to use 4096, but there is some inequivalence there. ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
[Xen-devel] [PATCH] xenbus: Neaten xenbus_va_dev_error
This function error patch can be simplified, so do so.
Remove fail: label and somewhat obfuscating, used once "error_path"
function.
Signed-off-by: Joe Perches
---
drivers/xen/xenbus/xenbus_client.c | 39 ++
1 file changed, 10 insertions(+), 29 deletions(-)
diff --git a/drivers/xen/xenbus/xenbus_client.c
b/drivers/xen/xenbus/xenbus_client.c
index 056da6ee1a35..915d77785193 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -259,53 +259,34 @@ int xenbus_frontend_closed(struct xenbus_device *dev)
}
EXPORT_SYMBOL_GPL(xenbus_frontend_closed);
-/**
- * Return the path to the error node for the given device, or NULL on failure.
- * If the value returned is non-NULL, then it is the caller's to kfree.
- */
-static char *error_path(struct xenbus_device *dev)
-{
- return kasprintf(GFP_KERNEL, "error/%s", dev->nodename);
-}
-
-
static void xenbus_va_dev_error(struct xenbus_device *dev, int err,
const char *fmt, va_list ap)
{
unsigned int len;
- char *printf_buffer = NULL;
- char *path_buffer = NULL;
+ char *printf_buffer;
+ char *path_buffer;
#define PRINTF_BUFFER_SIZE 4096
+
printf_buffer = kmalloc(PRINTF_BUFFER_SIZE, GFP_KERNEL);
- if (printf_buffer == NULL)
- goto fail;
+ if (!printf_buffer)
+ return;
len = sprintf(printf_buffer, "%i ", -err);
- vsnprintf(printf_buffer+len, PRINTF_BUFFER_SIZE-len, fmt, ap);
+ vsnprintf(printf_buffer + len, PRINTF_BUFFER_SIZE - len, fmt, ap);
dev_err(&dev->dev, "%s\n", printf_buffer);
- path_buffer = error_path(dev);
-
- if (path_buffer == NULL) {
+ path_buffer = kasprintf(GFP_KERNEL, "error/%s", dev->nodename);
+ if (!path_buffer ||
+ xenbus_write(XBT_NIL, path_buffer, "error", printf_buffer))
dev_err(&dev->dev, "failed to write error node for %s (%s)\n",
- dev->nodename, printf_buffer);
- goto fail;
- }
+ dev->nodename, printf_buffer);
- if (xenbus_write(XBT_NIL, path_buffer, "error", printf_buffer) != 0) {
- dev_err(&dev->dev, "failed to write error node for %s (%s)\n",
- dev->nodename, printf_buffer);
- goto fail;
- }
-
-fail:
kfree(printf_buffer);
kfree(path_buffer);
}
-
/**
* xenbus_dev_error
* @dev: xenbus device
--
2.10.0.rc2.1.g053435c
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 0/3] checkpatch: handling of memory barriers
On Mon, 2016-01-11 at 13:00 +0200, Michael S. Tsirkin wrote: > As part of memory barrier cleanup, this patchset > extends checkpatch to make it easier to stop > incorrect memory barrier usage. Thanks Michael. Acked-by: Joe Perches ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 3/3] checkpatch: add virt barriers
On Mon, 2016-01-11 at 09:13 +1100, Julian Calaby wrote:
> On Mon, Jan 11, 2016 at 6:31 AM, Michael S. Tsirkin wrote:
> > Add virt_ barriers to list of barriers to check for
> > presence of a comment.
[]
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> > @@ -5133,7 +5133,8 @@ sub process {
> > }x;
> > my $all_barriers = qr{
> > $barriers|
> > - smp_(?:$smp_barrier_stems)
> > + smp_(?:$smp_barrier_stems)|
> > + virt_(?:$smp_barrier_stems)
>
> Sorry I'm late to the party here, but would it make sense to write this as:
>
> (?:smp|virt)_(?:$smp_barrier_stems)
Yes. Perhaps the name might be better as barrier_stems.
Also, ideally this would be longest match first or use \b
after the matches so that $all_barriers could work
successfully without a following \s*\(
my $all_barriers = qr{
(?:smp|virt)_(?:barrier_stems)|
$barriers)
}x;
or maybe add separate $smp_barriers and $virt_barriers
it doesn't matter much in any case
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] checkpatch.pl: add missing memory barriers
On Sun, 2016-01-10 at 07:07 -0800, Joe Perches wrote:
> On Sun, 2016-01-10 at 13:56 +0200, Michael S. Tsirkin wrote:
> > SMP-only barriers were missing in checkpatch.pl
> >
> > Refactor code slightly to make adding more variants easier.
> []
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> If I use a variable called $smp_barriers, I'd expect
> it to actually be the smp_barriers, not to have to
> prefix it with smp_ before using it.
>
> my $smp_barriers = qr{
> smp_store_release|
> smp_load_acquire|
> smp_store_mb|
> smp_read_barrier_depends
That's missing (?:barriers) too.
btw: shouldn't this also have
smp_mb__(?:before|after)_atomic
?
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 2/3] checkpatch: check for __smp outside barrier.h
On Sun, 2016-01-10 at 13:57 +0200, Michael S. Tsirkin wrote:
> Introduction of __smp barriers cleans up a bunch of duplicate code, but
> it gives people an additional handle onto a "new" set of barriers - just
> because they're prefixed with __* unfortunately doesn't stop anyone from
> using it (as happened with other arch stuff before.)
>
> Add a checkpatch test so it will trigger a warning.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5141,6 +5141,16 @@ sub process {
> }
> }
>
> + my $underscore_smp_barriers = qr{__smp_($smp_barriers)}x;
another unnecessary capture group
> +
> + if ($realfile !~ m@^include/asm-generic/@ &&
> + $realfile !~ m@/barrier\.h$@ &&
> + $line =~ m/\b($underscore_smp_barriers)\s*\(/ &&
> + $line !~
> m/^.\s*\#\s*define\s+($underscore_smp_barriers)\s*\(/) {
> + WARN("MEMORY_BARRIER",
> + "__smp memory barriers shouldn't be used outside
> barrier.h and asm-generic\n" . $herecurr);
> + }
> +
> # check for waitqueue_active without a comment.
> if ($line =~ /\bwaitqueue_active\s*\(/) {
> if (!ctx_has_comment($first_line, $linenr)) {
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/3] checkpatch.pl: add missing memory barriers
On Sun, 2016-01-10 at 13:56 +0200, Michael S. Tsirkin wrote:
> SMP-only barriers were missing in checkpatch.pl
>
> Refactor code slightly to make adding more variants easier.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -5116,7 +5116,25 @@ sub process {
> }
> }
> # check for memory barriers without a comment.
> - if ($line =~
> /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/)
> {
> +
> + my $barriers = qr{
> + mb|
> + rmb|
> + wmb|
> + read_barrier_depends
> + }x;
> + my $smp_barriers = qr{
> + store_release|
> + load_acquire|
> + store_mb|
> + ($barriers)
> + }x;
If I use a variable called $smp_barriers, I'd expect
it to actually be the smp_barriers, not to have to
prefix it with smp_ before using it.
my $smp_barriers = qr{
smp_store_release|
smp_load_acquire|
smp_store_mb|
smp_read_barrier_depends
}x;
or
my $smp_barriers = qr{
smp_(?:store_release|load_acquire|store_mb|read_barrier_depends)
}x;
> + my $all_barriers = qr{
> + $barriers|
> + smp_($smp_barriers)
> + }x;
And this shouldn't have a capture group.
my $all_barriers = qr{
$barriers|
$smp_barriers
}x;
> +
> + if ($line =~ /\b($all_barriers)\s*\(/) {
This doesn't need the capture group either (?:all_barriers)
> if (!ctx_has_comment($first_line, $linenr))
> {
> WARN("MEMORY_BARRIER",
> "memory barrier without
> comment\n" . $herecurr);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] checkpatch.pl: add missing memory barriers
On Mon, 2016-01-04 at 22:45 +0200, Michael S. Tsirkin wrote:
> On Mon, Jan 04, 2016 at 08:07:40AM -0800, Joe Perches wrote:
> > On Mon, 2016-01-04 at 13:36 +0200, Michael S. Tsirkin wrote:
> > > SMP-only barriers were missing in checkpatch.pl
> > >
> > > Refactor code slightly to make adding more variants easier.
> > >
> > > Signed-off-by: Michael S. Tsirkin
> > > ---
> > > scripts/checkpatch.pl | 9 -
> > > 1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > > index 2b3c228..0245bbe 100755
> > > --- a/scripts/checkpatch.pl
> > > +++ b/scripts/checkpatch.pl
> > > @@ -5116,7 +5116,14 @@ sub process {
> > > }
> > > }
> > > # check for memory barriers without a comment.
> > > - if ($line =~
> > > /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/)
> > > {
> > > +
> > > + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends');
> > > + my @smp_barriers = ('smp_store_release', 'smp_load_acquire',
> > > 'smp_store_mb');
> > > +
> > > + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers);
> >
> > I think using map, which so far checkpatch doesn't use,
> > makes smp_barriers harder to understand and it'd be
> > better to enumerate them.
>
> Okay - I'll rewrite using foreach.
>
I think using the qr form (like 'our $Attribute' and
a bunch of others) is the general style that should
be used for checkpatch.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/3] checkpatch: add virt barriers
On Mon, 2016-01-04 at 23:07 +0200, Michael S. Tsirkin wrote: > On Mon, Jan 04, 2016 at 08:47:53AM -0800, Joe Perches wrote: > > On Mon, 2016-01-04 at 13:37 +0200, Michael S. Tsirkin wrote: > > > Add virt_ barriers to list of barriers to check for > > > presence of a comment. > > > > Are these virt_ barriers used anywhere? > > > > I see some virtio_ barrier like uses. > > They will be :) They are added and used by patchset > arch: barrier cleanup + barriers for virt > > See > http://article.gmane.org/gmane.linux.kernel.virtualization/26555 Ah, OK, thanks. Are the virtio_ barriers going away? If not, maybe those should be added too. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 3/3] checkpatch: add virt barriers
On Mon, 2016-01-04 at 13:37 +0200, Michael S. Tsirkin wrote: > Add virt_ barriers to list of barriers to check for > presence of a comment. Are these virt_ barriers used anywhere? I see some virtio_ barrier like uses. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] checkpatch.pl: add missing memory barriers
On Mon, 2016-01-04 at 16:11 +, Russell King - ARM Linux wrote:
> On Mon, Jan 04, 2016 at 08:07:40AM -0800, Joe Perches wrote:
> > On Mon, 2016-01-04 at 13:36 +0200, Michael S. Tsirkin wrote:
> > > + my $all_barriers = join('|', (@barriers, @smp_barriers));
> > > +
> > > + if ($line =~ /\b($all_barriers)\(/) {
> >
> > It would be better to use /\b$all_barriers\s*\(/
> > as there's no reason for the capture and there
> > could be a space between the function and the
> > open parenthesis.
>
> I think you mean
>
> /\b(?:$all_barriers)\s*\(/
>
> as 'all_barriers' will be:
>
> mb|wmb|rmb|smp_mb|smp_wmb|smp_rmb
>
> and putting that into your suggestion results in:
>
> /\bmb|wmb|rmb|smp_mb|smp_wmb|smp_rmb\s*\(/
>
> which is clearly wrong - the \b only applies to 'mb' and the \s*\( only
> applies to smp_rmb.
right, thanks.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/3] checkpatch.pl: add missing memory barriers
On Mon, 2016-01-04 at 13:36 +0200, Michael S. Tsirkin wrote:
> SMP-only barriers were missing in checkpatch.pl
>
> Refactor code slightly to make adding more variants easier.
>
> Signed-off-by: Michael S. Tsirkin
> ---
> scripts/checkpatch.pl | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 2b3c228..0245bbe 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -5116,7 +5116,14 @@ sub process {
> }
> }
> # check for memory barriers without a comment.
> - if ($line =~
> /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/)
> {
> +
> + my @barriers = ('mb', 'rmb', 'wmb', 'read_barrier_depends');
> + my @smp_barriers = ('smp_store_release', 'smp_load_acquire',
> 'smp_store_mb');
> +
> + @smp_barriers = (@smp_barriers, map {"smp_" . $_} @barriers);
I think using map, which so far checkpatch doesn't use,
makes smp_barriers harder to understand and it'd be
better to enumerate them.
> + my $all_barriers = join('|', (@barriers, @smp_barriers));
> +
> + if ($line =~ /\b($all_barriers)\(/) {
It would be better to use /\b$all_barriers\s*\(/
as there's no reason for the capture and there
could be a space between the function and the
open parenthesis.
> if (!ctx_has_comment($first_line, $linenr)) {
> WARN("MEMORY_BARRIER",
> "memory barrier without comment\n" .
> $herecurr);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 0/5] xen-netback: Fine-tuning for three function implementations
On Sat, 2016-01-02 at 18:50 +0100, SF Markus Elfring wrote:
> A few update suggestions were taken into account
> from static source code analysis.
While static analysis can be useful, I don't think these
specific conversions are generally useful.
Perhaps it would be more useful to convert the string
duplication or snprintf logic to kstrdup/kasprintf
This:
if (num_queues == 1) {
xspath = kzalloc(strlen(dev->otherend) + 1, GFP_KERNEL);
if (!xspath) {
xenbus_dev_fatal(dev, -ENOMEM,
"reading ring references");
return -ENOMEM;
}
strcpy(xspath, dev->otherend);
} else {
xspathsize = strlen(dev->otherend) + xenstore_path_ext_size;
xspath = kzalloc(xspathsize, GFP_KERNEL);
if (!xspath) {
xenbus_dev_fatal(dev, -ENOMEM,
"reading ring references");
return -ENOMEM;
}
snprintf(xspath, xspathsize, "%s/queue-%u", dev->otherend,
queue->id);
}
could be simplified to something like:
if (num_queues == 1)
xspath = kstrdup(dev->otherend, GFP_KERNEL);
else
xspath = kasprintf(GFP_KERNEL, "%s/queue-%u",
dev->otherend, queue->id);
if (!xspath)
etc...
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 8/8] treewide: Remove newlines inside DEFINE_PER_CPU() macros
On Mon, 2015-12-07 at 17:53 +0100, Michal Marek wrote: > On 2015-12-07 17:33, David Laight wrote: > > From: Michal Marek > > > Sent: 04 December 2015 15:26 > > > Otherwise make tags can't parse them: > > > > > > ctags: Warning: arch/ia64/kernel/smp.c:60: null expansion of name pattern > > > "\1" > > ... > > > > Seems to me you need to fix ctags. > > I'm sure the maintainers of ctags and etags would accept patches to > describe a custom context-free grammar via commandline options, but > until then, let's continue using the regular expressions in tags.sh and > remove newlines in macros that tags.sh is trying to expand. > Do you have a list of the most common macros? Perhaps it'd be good to add exceptions to checkpatch 80 column line rules for them. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] net/xen-netback: Don't mix hexa and decimal with 0x in the printf format
On Wed, 2015-06-17 at 01:29 +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 06/17/2015 01:09 AM, Joe Perches wrote:
>
> >>> Append 0x to all %x in order to avoid while reading when there is other
> >>> decimal value in the log.
>
> > []
>
> >>> @@ -874,7 +874,7 @@ static inline void xenvif_grant_handle_set(struct
> >>> xenvif_queue *queue,
> >>> if (unlikely(queue->grant_tx_handle[pending_idx] !=
> >>>NETBACK_INVALID_HANDLE)) {
> >>> netdev_err(queue->vif->dev,
> >>> -"Trying to overwrite active handle! pending_idx:
> >>> %x\n",
> >>> +"Trying to overwrite active handle! pending_idx:
> >>> 0x%x\n",
>
> >> Using "%#x" is shorter ind does the same.
>
> > That's true, but it's also far less common.
>
> Which is a pity... People just don't know the format specifiers well
> enough. :-(
>
> > $ git grep -E "%#[\*\d\.]*x" | wc -l
> > 1419
> > $ git grep "0x%" | wc -l
> > 29844
>
> Which means 29 KB could theoretically be saved on allyesconfig build. :-)
> (Actually less since the width specifiers will likely need to be fixed where
> present.)
And less than that because a lot of these are in
arch specific code.
0x%x is easier and simpler to visualize than %#x.
But you are welcome to try to make the kernel smaller.
One byte at a time.
There are ~14.5k uses of 0x%x in ~10.5k lines and
~2600 files that would be changed.
That's a lot of lines and a lot of patches.
$ git grep --name-only "0x%x" | xargs sed -i -e 's/0x%x/%#x/g'
$ git diff | wc
96250 415388 3949872
Only a 4M patch.
The pretty common (~5k) "0x%08x" would be "%#010x"
so that doesn't save any space.
but this one's a ~3.5M patch.
$ git grep --name-only -P "0x%\d+\w*x" | xargs perl -p -i -e
's/0x%0(\d+)(\w*)x/"\%#0" . eval($1 + 2) . "$2x"/eg'
$ git diff | wc
80857 344565 3306990
enjoy...
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v4 3/3] net/xen-netback: Don't mix hexa and decimal with 0x in the printf format
On Tue, 2015-06-16 at 23:07 +0300, Sergei Shtylyov wrote:
> On 06/16/2015 10:10 PM, Julien Grall wrote:
> > Append 0x to all %x in order to avoid while reading when there is other
> > decimal value in the log.
[]
> > @@ -874,7 +874,7 @@ static inline void xenvif_grant_handle_set(struct
> > xenvif_queue *queue,
> > if (unlikely(queue->grant_tx_handle[pending_idx] !=
> > NETBACK_INVALID_HANDLE)) {
> > netdev_err(queue->vif->dev,
> > - "Trying to overwrite active handle! pending_idx:
> > %x\n",
> > + "Trying to overwrite active handle! pending_idx:
> > 0x%x\n",
>
> Using "%#x" is shorter ind does the same.
That's true, but it's also far less common.
$ git grep -E "%#[\*\d\.]*x" | wc -l
1419
$ git grep "0x%" | wc -l
29844
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses
On Fri, 2015-06-05 at 13:34 +0100, Julien Grall wrote:
> On 04/06/15 17:25, Joe Perches wrote:
> > On Thu, 2015-06-04 at 13:52 +0100, Julien Grall wrote:
> >> On 04/06/15 13:46, David Vrabel wrote:
> >>> On 04/06/15 13:45, Julien Grall wrote:
> >>>> On 03/06/15 18:06, Joe Perches wrote:
> >>>>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
> >>>>>> rx->status is an int16_t, print it using %d rather than %u in order to
> >>>>>> have a meaningful value when the field is negative.
> >>>>> []
> >>>>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> >>>>> []
> >>>>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct
> >>>>>> netfront_queue *queue,
> >>>>>>if (unlikely(rx->status < 0 ||
> >>>>>> rx->offset + rx->status > PAGE_SIZE)) {
> >>>>>>if (net_ratelimit())
> >>>>>> - dev_warn(dev, "rx->offset: %x, size:
> >>>>>> %u\n",
> >>>>>> + dev_warn(dev, "rx->offset: %x, size:
> >>>>>> %d\n",
> >>>>>
> >>>>> If you're going to do this, perhaps it'd be sensible to
> >>>>> also change the %x to %#x or 0x%x so that people don't
> >>>>> mistake offset without an [a-f] for decimal.
> >>>>
> >>>> Good idea. I will resend a version of this series.
> >>>>
> >>>> David, can I keep you Reviewed-by for this change?#
> >>>
> >>> Can you make the offset %d instead?
> >
> > If you do, please change similar uses in
> > drivers/net/xen-netback/ in the same patch.
>
> The format is not really consistent accross the 2 drivers and even
> within the same driver (see pending_idx which is some times print with
> %x and %d).
>
> Anyway, ss it's a different drivers and maintainers I will prefer to
> send a separate patch for this.
It's all xen to me, but thanks.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses
On Thu, 2015-06-04 at 13:52 +0100, Julien Grall wrote:
> On 04/06/15 13:46, David Vrabel wrote:
> > On 04/06/15 13:45, Julien Grall wrote:
> >> On 03/06/15 18:06, Joe Perches wrote:
> >>> On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
> >>>> rx->status is an int16_t, print it using %d rather than %u in order to
> >>>> have a meaningful value when the field is negative.
> >>> []
> >>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> >>> []
> >>>> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct
> >>>> netfront_queue *queue,
> >>>> if (unlikely(rx->status < 0 ||
> >>>> rx->offset + rx->status > PAGE_SIZE)) {
> >>>> if (net_ratelimit())
> >>>> -dev_warn(dev, "rx->offset: %x, size:
> >>>> %u\n",
> >>>> +dev_warn(dev, "rx->offset: %x, size:
> >>>> %d\n",
> >>>
> >>> If you're going to do this, perhaps it'd be sensible to
> >>> also change the %x to %#x or 0x%x so that people don't
> >>> mistake offset without an [a-f] for decimal.
> >>
> >> Good idea. I will resend a version of this series.
> >>
> >> David, can I keep you Reviewed-by for this change?#
> >
> > Can you make the offset %d instead?
If you do, please change similar uses in
drivers/net/xen-netback/ in the same patch.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2 1/2] net/xen-netfront: Correct printf format in xennet_get_responses
On Wed, 2015-06-03 at 17:55 +0100, Julien Grall wrote:
> rx->status is an int16_t, print it using %d rather than %u in order to
> have a meaningful value when the field is negative.
[]
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
[]
> @@ -733,7 +733,7 @@ static int xennet_get_responses(struct netfront_queue
> *queue,
> if (unlikely(rx->status < 0 ||
>rx->offset + rx->status > PAGE_SIZE)) {
> if (net_ratelimit())
> - dev_warn(dev, "rx->offset: %x, size: %u\n",
> + dev_warn(dev, "rx->offset: %x, size: %d\n",
If you're going to do this, perhaps it'd be sensible to
also change the %x to %#x or 0x%x so that people don't
mistake offset without an [a-f] for decimal.
>rx->offset, rx->status);
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH] xen-blkback: define pr_fmt macro to avoid the duplication of DRV_PFX
On Tue, 2015-03-31 at 16:57 +0200, Roger Pau Monné wrote:
> El 31/03/15 a les 23.14, Tao Chen ha escrit:
> > Define pr_fmt macro with {xen-blkback: } prefix, then remove all use
> > of DRV_PFX in the pr and DPRINTK sentences. It will simplify the code.
[]
> > diff --git a/drivers/block/xen-blkback/xenbus.c
> > b/drivers/block/xen-blkback/xenbus.c
[]
> > @@ -14,6 +14,11 @@
> >
> > */
> >
> > +#define pr_fmt(fmt) "xen-blkback: " fmt
> > +#define DPRINTK(fmt, args...) \
> > + pr_debug("(%s:%d) " fmt ".\n", \
> > + __func__, __LINE__, ##args)
As dynamic debug can emit __func__ and __LINE__
I suggest converting DPRINTK to pr_debug.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen-scsiback: use DRV_PFX in the pr macros and DPRINTK
On Fri, 2015-03-06 at 17:33 +0800, Chentao (Boby) wrote: > #ifdef pr_fmt > #undef pr_fmt > #endif > #define pr_fmt(fmt) "xen-pvscsi: " fmt No, just use add #define pr_fmt(fmt) "xen-pvscsi: " fmt before the first #include. The #ifdef/#undef/#endif isn't necessary. > Then replace all DPRINTK with pr_debug. Yes on this one. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v2] xen-scsiback: use DRV_PFX in the pr macros and DPRINTK
On Wed, 2015-03-04 at 18:32 +, Tao Chen wrote:
> Defined the string of {xen-pvscsi: } as DRV_PFX, then use it in the pr
> sentences and DPRINTK.
> Also fixed up some comments just as eliminate redundant white spaces and
> format the code.
> These will make the code easier to read.
It'd probaby be better just to use pr_fmt
before any include and remove all the DRV_PRV uses
#define pr_fmt(fmt) "xen-pvscsi: " fmt
> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
[]
> @@ -69,8 +69,10 @@
> #include
> #include
>
> +#define DRV_PFX "xen-pvscsi: "
> #define DPRINTK(_f, _a...) \
> - pr_debug("(file=%s, line=%d) " _f, __FILE__ , __LINE__ , ## _a)
> + pr_debug(DRV_PFX "(file=%s, line=%d) " _f, \
> + __FILE__ , __LINE__ , ## _a)
I'd also remove DPRINTK and just use pr_debug directly
as dynamic_debug can emit file and line as desired.
> @@ -84,7 +86,7 @@ struct ids_tuple {
>
> struct v2p_entry {
> struct ids_tuple v; /* translate from */
> - struct scsiback_tpg *tpg; /* translate to */
> + struct scsiback_tpg *tpg; /* translate to */
superfluous change.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
[Xen-devel] [PATCH net-next 06/14] xen: Use eth__addr instead of memset
Use the built-in function instead of memset. Signed-off-by: Joe Perches --- drivers/net/xen-netback/interface.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c index f38227a..4ae98e2 100644 --- a/drivers/net/xen-netback/interface.c +++ b/drivers/net/xen-netback/interface.c @@ -438,7 +438,7 @@ struct xenvif *xenvif_alloc(struct device *parent, domid_t domid, * stolen by an Ethernet bridge for STP purposes. * (FE:FF:FF:FF:FF:FF) */ - memset(dev->dev_addr, 0xFF, ETH_ALEN); + eth_broadcast_addr(dev->dev_addr); dev->dev_addr[0] &= ~0x01; netif_carrier_off(dev); -- 2.1.2 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH net-next 00/14] Use eth__addr instead of memset
Joe Perches (14): etherdevice: Add eth__addr CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS code ethernet: Use eth__addr instead of memset net: usb: Use eth__addr instead of memset wireless: Use eth__addr instead of memset netconsole: Use eth__addr instead of memset xen: Use eth__addr instead of memset 8021q: Use eth__addr instead of memset appletalk: Use eth__addr instead of memset atm: Use eth__addr instead of memset bluetooth: Use eth__addr instead of memset ethernet: Use eth__addr instead of memset mac80211: Use eth__addr instead of memset wireless: Use eth__addr instead of memset l2tp: Use eth__addr instead of memset drivers/net/ethernet/amd/pcnet32.c | 2 +- .../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c| 2 +- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 6 ++-- drivers/net/ethernet/broadcom/bnx2x/bnx2x_sriov.c | 2 +- drivers/net/ethernet/cisco/enic/enic_main.c| 8 +++--- drivers/net/ethernet/emulex/benet/be_cmds.c| 2 +- drivers/net/ethernet/emulex/benet/be_main.c| 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 +-- drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 4 +-- drivers/net/ethernet/mellanox/mlx4/en_selftest.c | 2 +- drivers/net/ethernet/micrel/ksz884x.c | 2 +- drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 2 +- drivers/net/ethernet/qlogic/qlge/qlge_main.c | 2 +- drivers/net/ethernet/smsc/smsc911x.c | 2 +- drivers/net/ethernet/ti/netcp_core.c | 2 +- drivers/net/ethernet/toshiba/ps3_gelic_wireless.c | 4 +-- drivers/net/ethernet/xscale/ixp4xx_eth.c | 2 +- drivers/net/netconsole.c | 5 ++-- drivers/net/usb/catc.c | 4 +-- drivers/net/usb/cdc_mbim.c | 2 +- drivers/net/usb/lg-vl600.c | 2 +- drivers/net/usb/qmi_wwan.c | 2 +- drivers/net/wireless/airo.c| 4 +-- drivers/net/wireless/at76c50x-usb.c| 6 ++-- drivers/net/wireless/ath/ath10k/mac.c | 2 +- drivers/net/wireless/ath/ath5k/base.c | 2 +- drivers/net/wireless/ath/ath6kl/cfg80211.c | 2 +- drivers/net/wireless/ath/ath6kl/main.c | 2 +- drivers/net/wireless/ath/ath9k/htc_drv_main.c | 2 +- drivers/net/wireless/ath/ath9k/main.c | 4 +-- drivers/net/wireless/atmel.c | 16 +-- drivers/net/wireless/b43/main.c| 8 +++--- drivers/net/wireless/b43legacy/main.c | 8 +++--- drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c | 12 drivers/net/wireless/brcm80211/brcmfmac/flowring.c | 2 +- drivers/net/wireless/brcm80211/brcmfmac/p2p.c | 2 +- drivers/net/wireless/cw1200/sta.c | 2 +- drivers/net/wireless/cw1200/txrx.c | 2 +- drivers/net/wireless/hostap/hostap_80211_tx.c | 4 +-- drivers/net/wireless/hostap/hostap_ap.c| 8 +++--- drivers/net/wireless/hostap/hostap_info.c | 2 +- drivers/net/wireless/hostap/hostap_main.c | 4 +-- drivers/net/wireless/hostap/hostap_wlan.h | 33 +++--- drivers/net/wireless/ipw2x00/ipw2100.c | 8 +++--- drivers/net/wireless/ipw2x00/ipw2200.c | 6 ++-- drivers/net/wireless/iwlegacy/common.c | 2 +- drivers/net/wireless/iwlwifi/mvm/power.c | 3 +- drivers/net/wireless/libertas/main.c | 4 +-- drivers/net/wireless/libertas_tf/main.c| 4 +-- drivers/net/wireless/mac80211_hwsim.c | 6 ++-- drivers/net/wireless/mwifiex/cfg80211.c| 8 +++--- drivers/net/wireless/mwifiex/init.c| 4 +-- drivers/net/wireless/mwifiex/sta_event.c | 2 +- drivers/net/wireless/mwifiex/wmm.c | 2 +- drivers/net/wireless/mwl8k.c | 2 +- drivers/net/wireless/orinoco/wext.c| 2 +- drivers/net/wireless/p54/fwio.c| 2 +- drivers/net/wireless/p54/main.c| 8 +++--- drivers/net/wireless/ray_cs.c | 2 +- drivers/net/wireless/rndis_wlan.c | 28 +- drivers/net/wireless/rtlwifi/core.c| 6 ++-- drivers/net/wireless/ti/wl1251/main.c | 4 +-- drivers/net/wireless/ti/wlcore/cmd.c | 4 +-- drivers/net/xen-netback/interface.c| 2 +- include/linux/etherdevice.h| 12 +++- net/8021q/vlan_dev.c | 2 +- net/appletalk/aarp.c | 6 ++-- net/atm/lec.c | 4 +-- net/bluetooth/bnep/netdev.c| 2 +- net/ethernet/eth.c | 4 +-- net/l2tp/l2tp_eth.c
[Xen-devel] [PATCH] treewide: Convert clockevents_notify to use int cpu
As far as I can tell, there's no value indirecting
the cpu passed to this function via a void *.
Update all the callers and called functions from within
clockevents_notify.
Miscellanea:
Add pr_fmt and convert one printk(KERN_ERR to pr_err
Signed-off-by: Joe Perches
---
arch/arm/mach-omap2/cpuidle44xx.c | 7 +++
arch/arm/mach-tegra/cpuidle-tegra114.c | 4 ++--
arch/arm/mach-tegra/cpuidle-tegra20.c | 8
arch/arm/mach-tegra/cpuidle-tegra30.c | 8
arch/x86/kernel/process.c | 6 +++---
arch/x86/xen/suspend.c | 2 +-
drivers/acpi/acpi_pad.c| 9 +
drivers/acpi/processor_idle.c | 4 ++--
drivers/cpuidle/driver.c | 3 +--
drivers/idle/intel_idle.c | 7 +++
include/linux/clockchips.h | 6 +++---
kernel/sched/idle.c| 4 ++--
kernel/time/clockevents.c | 15 +++
kernel/time/hrtimer.c | 6 ++
kernel/time/tick-broadcast.c | 16
kernel/time/tick-common.c | 16
kernel/time/tick-internal.h| 18 +-
kernel/time/timekeeping.c | 4 ++--
18 files changed, 69 insertions(+), 74 deletions(-)
diff --git a/arch/arm/mach-omap2/cpuidle44xx.c
b/arch/arm/mach-omap2/cpuidle44xx.c
index 01e398a..5d50aa1 100644
--- a/arch/arm/mach-omap2/cpuidle44xx.c
+++ b/arch/arm/mach-omap2/cpuidle44xx.c
@@ -112,7 +112,7 @@ static int omap_enter_idle_coupled(struct cpuidle_device
*dev,
mpuss_can_lose_context = (cx->mpu_state == PWRDM_POWER_RET) &&
(cx->mpu_logic_state == PWRDM_POWER_OFF);
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &cpu_id);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, cpu_id);
/*
* Call idle CPU PM enter notifier chain so that
@@ -169,7 +169,7 @@ static int omap_enter_idle_coupled(struct cpuidle_device
*dev,
if (dev->cpu == 0 && mpuss_can_lose_context)
cpu_cluster_pm_exit();
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &cpu_id);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, cpu_id);
fail:
cpuidle_coupled_parallel_barrier(dev, &abort_barrier);
@@ -184,8 +184,7 @@ fail:
*/
static void omap_setup_broadcast_timer(void *arg)
{
- int cpu = smp_processor_id();
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, &cpu);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ON, smp_processor_id());
}
static struct cpuidle_driver omap4_idle_driver = {
diff --git a/arch/arm/mach-tegra/cpuidle-tegra114.c
b/arch/arm/mach-tegra/cpuidle-tegra114.c
index f2b586d..3b2fc3f 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra114.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra114.c
@@ -44,7 +44,7 @@ static int tegra114_idle_power_down(struct cpuidle_device
*dev,
tegra_set_cpu_in_lp2();
cpu_pm_enter();
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev->cpu);
call_firmware_op(prepare_idle);
@@ -52,7 +52,7 @@ static int tegra114_idle_power_down(struct cpuidle_device
*dev,
if (call_firmware_op(do_idle, 0) == -ENOSYS)
cpu_suspend(0, tegra30_sleep_cpu_secondary_finish);
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, dev->cpu);
cpu_pm_exit();
tegra_clear_cpu_in_lp2();
diff --git a/arch/arm/mach-tegra/cpuidle-tegra20.c
b/arch/arm/mach-tegra/cpuidle-tegra20.c
index 4f25a7c..ab30758 100644
--- a/arch/arm/mach-tegra/cpuidle-tegra20.c
+++ b/arch/arm/mach-tegra/cpuidle-tegra20.c
@@ -136,11 +136,11 @@ static bool tegra20_cpu_cluster_power_down(struct
cpuidle_device *dev,
if (tegra20_reset_cpu_1() || !tegra_cpu_rail_off_ready())
return false;
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev->cpu);
tegra_idle_lp2_last();
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, &dev->cpu);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_EXIT, dev->cpu);
if (cpu_online(1))
tegra20_wake_cpu1_from_reset();
@@ -153,13 +153,13 @@ static bool tegra20_idle_enter_lp2_cpu_1(struct
cpuidle_device *dev,
struct cpuidle_driver *drv,
int index)
{
- clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, &dev->cpu);
+ clockevents_notify(CLOCK_EVT_NOTIFY_BROADCAST_ENTER, dev->cpu);
cpu_suspend(0, tegra20_sleep_cpu_secondary_finish);
tegra20_cpu_clear_resettable();
-
