Re: [RFC PATCH 28/30] Improved symbolic error names
On Tue, 2022-08-30 at 14:49 -0700, Suren Baghdasaryan wrote:
> From: Kent Overstreet
>
> This patch adds per-error-site error codes, with error strings that
> include their file and line number.
>
> To use, change code that returns an error, e.g.
> return -ENOMEM;
> to
> return -ERR(ENOMEM);
>
> Then, errname() will return a string that includes the file and line
> number of the ERR() call, for example
> printk("Got error %s!\n", errname(err));
> will result in
> Got error ENOMEM at foo.c:1234
Why? Something wrong with just using %pe ?
printk("Got error %pe at %s:%d!\n", ERR_PTR(err), __FILE__, __LINE__);
Likely __FILE__ and __LINE__ aren't particularly useful.
And using ERR would add rather a lot of bloat as each codetag_error_code
struct would be unique.
+#define ERR(_err) \
+({ \
+ static struct codetag_error_code\
+ __used \
+ __section("error_code_tags")\
+ __aligned(8) e = { \
+ .str= #_err " at " __FILE__ ":" __stringify(__LINE__),\
+ .err= _err, \
+ }; \
+ \
+ e.err; \
+})
Re: [PATCH] xen-blkfront: Use the bitmap API when applicable
On Sat, 2021-12-04 at 07:57 +0100, Christophe JAILLET wrote:
> So, maybe adding an "official" 'bitmap_size()' (which is already
> existing and duplicated in a few places) would ease things.
>
> It would replace the 'nr_minors = BITS_TO_LONGS(end) * BITS_PER_LONG;'
> and hide the implementation details of the bitmap API.
>
> Something like:
> static __always_inline size_t bitmap_size(unsigned long nr_bits)
> {
> return BITS_TO_LONGS(nr_bits) * sizeof(long);
> }
Or maybe a bitmap_realloc
Re: [PATCH] xen-blkfront: Use the bitmap API when applicable
On Fri, 2021-12-03 at 16:54 +0100, Christophe JAILLET wrote:
> Le 03/12/2021 à 04:03, Joe Perches a écrit :
> > On Thu, 2021-12-02 at 20:07 +0100, Christophe JAILLET wrote:
> > > Le 02/12/2021 à 19:16, Joe Perches a écrit :
> > > > On Thu, 2021-12-02 at 19:12 +0100, Christophe JAILLET wrote:
> > > > > Le 02/12/2021 à 07:12, Juergen Gross a écrit :
> > > > > > On 01.12.21 22:10, Christophe JAILLET wrote:
> > > > > > > Use 'bitmap_zalloc()' to simplify code, improve the semantic and
> > > > > > > avoid
> > > > > > > some open-coded arithmetic in allocator arguments.
> > > > > > >
> > > > > > > Also change the corresponding 'kfree()' into 'bitmap_free()' to
> > > > > > > keep
> > > > > > > consistency.
> > > > > > >
> > > > > > > Use 'bitmap_copy()' to avoid an explicit 'memcpy()'
> > > > []
> > > > > > > diff --git a/drivers/block/xen-blkfront.c
> > > > > > > b/drivers/block/xen-blkfront.c
> > > > []
> > > > > > > @@ -442,16 +442,14 @@ static int xlbd_reserve_minors(unsigned int
> > > > > > > minor, unsigned int nr)
> > > > > > > if (end > nr_minors) {
> > > > > > > unsigned long *bitmap, *old;
> > > > > > > - bitmap = kcalloc(BITS_TO_LONGS(end), sizeof(*bitmap),
> > > > > > > - GFP_KERNEL);
> > > > > > > + bitmap = bitmap_zalloc(end, GFP_KERNEL);
> > > > > > > if (bitmap == NULL)
> > > > > > > return -ENOMEM;
> > > > > > > spin_lock(&minor_lock);
> > > > > > > if (end > nr_minors) {
> > > > > > > old = minors;
> > > > > > > - memcpy(bitmap, minors,
> > > > > > > - BITS_TO_LONGS(nr_minors) * sizeof(*bitmap));
> > > > > > > + bitmap_copy(bitmap, minors, nr_minors);
> > > > > > > minors = bitmap;
> > > > > > > nr_minors = BITS_TO_LONGS(end) * BITS_PER_LONG;
> > > >
> > > > nr_minors = end;
> > > > ?
> > > >
> > >
> > > No,
> > > My understanding of the code is that if we lack space (end > nr_minors),
> > > we need to allocate more. In such a case, we want to keep track of what
> > > we have allocated, not what we needed.
> > > The "padding" bits in the "long align" allocation, can be used later.
> >
> > >
> > > first call
> > > --
> > > end = 65
> > > nr_minors = 63
> > >
> > > --> we need some space
> > > --> we allocate 2 longs = 128 bits
> > > --> we now use 65 bits of these 128 bits
> >
> > or 96, 32 or 64 bit longs remember.
>
> 32 and 64 for sure, but I was not aware of 96. On which arch?
For more clarity that should have been a period instead of comma after 96.
> > The initial allocation is now bitmap_zalloc which
> > specifies only bits and the nr_minors is then in
> > BITS_TO_LONGS(bits) * BITS_PER_LONG
> >
> > Perhaps that assumes too much about the internal
> > implementation of bitmap_alloc
>
> I get your point now, and I agree with you.
>
> Maybe something as what is done in mc-entity.c?
> Explicitly require more bits (which will be allocated anyway), instead
> of taking advantage (read "hoping") that it will be done.
Sure, that's sensible.
> Could be:
>
> @@ -440,26 +440,25 @@ static int xlbd_reserve_minors(unsigned int minor,
> unsigned int nr)
> int rc;
>
> if (end > nr_minors) {
> unsigned long *bitmap, *old;
>
> - bitmap = kcalloc(BITS_TO_LONGS(end), sizeof(*bitmap),
> - GFP_KERNEL);
> + end = ALIGN(end, BITS_PER_LONG);
Though it may be more sensible to use some other alignment
like round_up and not use BITS_PER_LONG at all as the
number of these may not be dependent on 32/64 bit arches
at all.
Maybe something like:
#define GROW_MINORS 64
end = round_up(nr_minors, GROW_MINORS);
etc...
Re: [PATCH] xen-blkfront: Use the bitmap API when applicable
On Thu, 2021-12-02 at 20:07 +0100, Christophe JAILLET wrote:
> Le 02/12/2021 à 19:16, Joe Perches a écrit :
> > On Thu, 2021-12-02 at 19:12 +0100, Christophe JAILLET wrote:
> > > Le 02/12/2021 à 07:12, Juergen Gross a écrit :
> > > > On 01.12.21 22:10, Christophe JAILLET wrote:
> > > > > Use 'bitmap_zalloc()' to simplify code, improve the semantic and avoid
> > > > > some open-coded arithmetic in allocator arguments.
> > > > >
> > > > > Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
> > > > > consistency.
> > > > >
> > > > > Use 'bitmap_copy()' to avoid an explicit 'memcpy()'
> > []
> > > > > diff --git a/drivers/block/xen-blkfront.c
> > > > > b/drivers/block/xen-blkfront.c
> > []
> > > > > @@ -442,16 +442,14 @@ static int xlbd_reserve_minors(unsigned int
> > > > > minor, unsigned int nr)
> > > > > if (end > nr_minors) {
> > > > > unsigned long *bitmap, *old;
> > > > > - bitmap = kcalloc(BITS_TO_LONGS(end), sizeof(*bitmap),
> > > > > - GFP_KERNEL);
> > > > > + bitmap = bitmap_zalloc(end, GFP_KERNEL);
> > > > > if (bitmap == NULL)
> > > > > return -ENOMEM;
> > > > > spin_lock(&minor_lock);
> > > > > if (end > nr_minors) {
> > > > > old = minors;
> > > > > - memcpy(bitmap, minors,
> > > > > - BITS_TO_LONGS(nr_minors) * sizeof(*bitmap));
> > > > > + bitmap_copy(bitmap, minors, nr_minors);
> > > > > minors = bitmap;
> > > > > nr_minors = BITS_TO_LONGS(end) * BITS_PER_LONG;
> >
> > nr_minors = end;
> > ?
> >
>
> No,
> My understanding of the code is that if we lack space (end > nr_minors),
> we need to allocate more. In such a case, we want to keep track of what
> we have allocated, not what we needed.
> The "padding" bits in the "long align" allocation, can be used later.
>
> first call
> --
> end = 65
> nr_minors = 63
>
> --> we need some space
> --> we allocate 2 longs = 128 bits
> --> we now use 65 bits of these 128 bits
or 96, 32 or 64 bit longs remember.
>
> new call
>
> end = 68
> nr_minors = 128 (from previous call)
The initial allocation is now bitmap_zalloc which
specifies only bits and the nr_minors is then in
BITS_TO_LONGS(bits) * BITS_PER_LONG
Perhaps that assumes too much about the internal
implementation of bitmap_alloc
Re: [PATCH] xen-blkfront: Use the bitmap API when applicable
On Thu, 2021-12-02 at 19:12 +0100, Christophe JAILLET wrote:
> Le 02/12/2021 à 07:12, Juergen Gross a écrit :
> > On 01.12.21 22:10, Christophe JAILLET wrote:
> > > Use 'bitmap_zalloc()' to simplify code, improve the semantic and avoid
> > > some open-coded arithmetic in allocator arguments.
> > >
> > > Also change the corresponding 'kfree()' into 'bitmap_free()' to keep
> > > consistency.
> > >
> > > Use 'bitmap_copy()' to avoid an explicit 'memcpy()'
[]
> > > diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
[]
> > > @@ -442,16 +442,14 @@ static int xlbd_reserve_minors(unsigned int
> > > minor, unsigned int nr)
> > > if (end > nr_minors) {
> > > unsigned long *bitmap, *old;
> > > - bitmap = kcalloc(BITS_TO_LONGS(end), sizeof(*bitmap),
> > > - GFP_KERNEL);
> > > + bitmap = bitmap_zalloc(end, GFP_KERNEL);
> > > if (bitmap == NULL)
> > > return -ENOMEM;
> > > spin_lock(&minor_lock);
> > > if (end > nr_minors) {
> > > old = minors;
> > > - memcpy(bitmap, minors,
> > > - BITS_TO_LONGS(nr_minors) * sizeof(*bitmap));
> > > + bitmap_copy(bitmap, minors, nr_minors);
> > > minors = bitmap;
> > > nr_minors = BITS_TO_LONGS(end) * BITS_PER_LONG;
nr_minors = end;
?
Re: [PATCH V7 01/18] perf/core: Use static_call to optimize perf_guest_info_callbacks
On Fri, 2021-07-02 at 17:38 +0100, Mark Rutland wrote:
> On Fri, Jul 02, 2021 at 09:00:22AM -0700, Joe Perches wrote:
> > On Fri, 2021-07-02 at 13:22 +0200, Peter Zijlstra wrote:
> > > On Tue, Jun 22, 2021 at 05:42:49PM +0800, Zhu Lingshan wrote:
[]
> > > > + if (perf_guest_cbs && perf_guest_cbs->handle_intel_pt_intr)
> > > > + static_call_update(x86_guest_handle_intel_pt_intr,
> > > > +
> > > > perf_guest_cbs->handle_intel_pt_intr);
> > > > +}
> > >
> > > Coding style wants { } on that last if().
> >
> > That's just your personal preference.
> >
> > The coding-style document doesn't require that.
> >
> > It just says single statement. It's not the number of
> > vertical lines or characters required for the statement.
> >
> > --
> >
> > Do not unnecessarily use braces where a single statement will do.
> >
> > .. code-block:: c
> >
> > if (condition)
> > action();
> >
> > and
> >
> > .. code-block:: none
> >
> > if (condition)
> > do_this();
> > else
> > do_that();
> >
> > This does not apply if only one branch of a conditional statement is a
> > single
> > statement; in the latter case use braces in both branches:
>
> Immediately after this, we say:
>
> > Also, use braces when a loop contains more than a single simple statement:
> >
> > .. code-block:: c
> >
> > while (condition) {
> > if (test)
> > do_something();
> > }
> >
>
> ... and while that says "a loop", the principle is obviously supposed to
> apply to conditionals too; structurally they're no different. We should
> just fix the documentation to say "a loop or conditional", or something
> to that effect.
Maybe.
I think there are _way_ too many existing obvious uses where the
statement that follows a conditional is multi-line.
if (foo)
printk(fmt,
args...);
where the braces wouldn't add anything other than more vertical space.
I don't much care one way or another other than Peter's somewhat ambiguous
use of the phrase "coding style".
checkpatch doesn't emit a message either way.
-
$ cat t_multiline.c
// SPDX-License-Identifier: GPL-2.0-only
void foo(void)
{
if (foo) {
pr_info(fmt,
args);
}
if (foo)
pr_info(fmt,
args);
if (foo)
pr_info(fmt, args);
}
$ ./scripts/checkpatch.pl -f --strict t_multiline.c
total: 0 errors, 0 warnings, 0 checks, 16 lines checked
t_multiline.c has no obvious style problems and is ready for submission.
-
cheers, Joe
Re: [PATCH V7 01/18] perf/core: Use static_call to optimize perf_guest_info_callbacks
On Fri, 2021-07-02 at 18:19 +0200, Peter Zijlstra wrote:
> On Fri, Jul 02, 2021 at 09:00:22AM -0700, Joe Perches wrote:
> > On Fri, 2021-07-02 at 13:22 +0200, Peter Zijlstra wrote:
> > > On Tue, Jun 22, 2021 at 05:42:49PM +0800, Zhu Lingshan wrote:
> > > > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> > []
> > > > + if (perf_guest_cbs && perf_guest_cbs->handle_intel_pt_intr)
> > > > + static_call_update(x86_guest_handle_intel_pt_intr,
> > > > +
> > > > perf_guest_cbs->handle_intel_pt_intr);
> > > > +}
> > >
> > > Coding style wants { } on that last if().
> >
> > That's just your personal preference.
>
> As a maintainer, those carry weight, also that's tip rules:
>
> https://lore.kernel.org/lkml/20181107171149.165693...@linutronix.de/
Right, definitely so.
But merely referencing 'coding style' is ambiguous at best.
btw:
ASCII commonly refers to '{' and '}', the curly brackets, to be braces
and '[' and ']', the square brackets, to be brackets.
It might be clearer to use that terminology.
belts and braces, etc...
cheers, Joe
+Bracket rules
+^
+
+Brackets should be omitted only if the statement which follows 'if', 'for',
+'while' etc. is truly a single line::
+
+ if (foo)
+ do_something();
+
+The following is not considered to be a single line statement even
+though C does not require brackets::
+
+ for (i = 0; i < end; i++)
+ if (foo[i])
+ do_something(foo[i]);
+
+Adding brackets around the outer loop enhances the reading flow::
+
+ for (i = 0; i < end; i++) {
+ if (foo[i])
+ do_something(foo[i]);
+ }
Re: [PATCH V7 01/18] perf/core: Use static_call to optimize perf_guest_info_callbacks
On Fri, 2021-07-02 at 13:22 +0200, Peter Zijlstra wrote:
> On Tue, Jun 22, 2021 at 05:42:49PM +0800, Zhu Lingshan wrote:
> > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
[]
> > @@ -90,6 +90,27 @@ DEFINE_STATIC_CALL_NULL(x86_pmu_pebs_aliases,
> > *x86_pmu.pebs_aliases);
> > */
> > DEFINE_STATIC_CALL_RET0(x86_pmu_guest_get_msrs, *x86_pmu.guest_get_msrs);
> >
> >
> > +DEFINE_STATIC_CALL_RET0(x86_guest_state, *(perf_guest_cbs->state));
> > +DEFINE_STATIC_CALL_RET0(x86_guest_get_ip, *(perf_guest_cbs->get_ip));
> > +DEFINE_STATIC_CALL_RET0(x86_guest_handle_intel_pt_intr,
> > *(perf_guest_cbs->handle_intel_pt_intr));
> > +
> > +void arch_perf_update_guest_cbs(void)
> > +{
> > + static_call_update(x86_guest_state, (void *)&__static_call_return0);
> > + static_call_update(x86_guest_get_ip, (void *)&__static_call_return0);
> > + static_call_update(x86_guest_handle_intel_pt_intr, (void
> > *)&__static_call_return0);
> > +
> > + if (perf_guest_cbs && perf_guest_cbs->state)
> > + static_call_update(x86_guest_state, perf_guest_cbs->state);
> > +
> > + if (perf_guest_cbs && perf_guest_cbs->get_ip)
> > + static_call_update(x86_guest_get_ip, perf_guest_cbs->get_ip);
> > +
> > + if (perf_guest_cbs && perf_guest_cbs->handle_intel_pt_intr)
> > + static_call_update(x86_guest_handle_intel_pt_intr,
> > + perf_guest_cbs->handle_intel_pt_intr);
> > +}
>
> Coding style wants { } on that last if().
That's just your personal preference.
The coding-style document doesn't require that.
It just says single statement. It's not the number of
vertical lines or characters required for the statement.
--
Do not unnecessarily use braces where a single statement will do.
.. code-block:: c
if (condition)
action();
and
.. code-block:: none
if (condition)
do_this();
else
do_that();
This does not apply if only one branch of a conditional statement is a single
statement; in the latter case use braces in both branches:
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Tue, 2020-11-24 at 11:58 +1100, Finn Thain wrote: > it's not for me to prove that such patches don't affect code > generation. That's for the patch author and (unfortunately) for reviewers. Ideally, that proof would be provided by the compilation system itself and not patch authors nor reviewers nor maintainers. Unfortunately gcc does not guarantee repeatability or deterministic output. To my knowledge, neither does clang.
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Mon, 2020-11-23 at 07:58 -0800, James Bottomley wrote: > We're also complaining about the inability to recruit maintainers: > > https://www.theregister.com/2020/06/30/hard_to_find_linux_maintainers_says_torvalds/ > > And burn out: > > http://antirez.com/news/129 https://www.wired.com/story/open-source-coders-few-tired/ > What I'm actually trying to articulate is a way of measuring value of > the patch vs cost ... it has nothing really to do with who foots the > actual bill. It's unclear how to measure value in consistency. But one way that costs can be reduced is by automation and _not_ involving maintainers when the patch itself is provably correct. > One thesis I'm actually starting to formulate is that this continual > devaluing of maintainers is why we have so much difficulty keeping and > recruiting them. The linux kernel has something like 1500 different maintainers listed in the MAINTAINERS file. That's not a trivial number. $ git grep '^M:' MAINTAINERS | sort | uniq -c | wc -l 1543 $ git grep '^M:' MAINTAINERS| cut -f1 -d'<' | sort | uniq -c | wc -l 1446 I think the question you are asking is about trust and how it effects development. And back to that wired story, the actual number of what you might be considering to be maintainers is likely less than 10% of the listed numbers above.
Re: [RFC] MAINTAINERS tag for cleanup robot
On Mon, 2020-11-23 at 09:33 +1100, Finn Thain wrote:
> On Sun, 22 Nov 2020, Joe Perches wrote:
> > But provably correct conversions IMO _should_ be done and IMO churn
> > considerations should generally have less importance.
[]
> Moreover, the patch review workload for skilled humans is being generated
> by the automation, which is completely backwards: the machine is supposed
> to be helping.
Which is why the provably correct matters.
coccinelle transforms can be, but are not necessarily, provably correct.
The _show transforms done via the sysfs_emit_dev.cocci script are correct
as in commit aa838896d87a ("drivers core: Use sysfs_emit and sysfs_emit_at
for show(device *...) functions")
Worthwhile? A different question, but I think yes as it reduces the
overall question space of the existing other sprintf overrun possibilities.
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Sun, 2020-11-22 at 11:12 -0800, James Bottomley wrote: > On Sun, 2020-11-22 at 10:25 -0800, Joe Perches wrote: > > On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote: > > > Please tell me our reward for all this effort isn't a single > > > missing error print. > > > > There were quite literally dozens of logical defects found > > by the fallthrough additions. Very few were logging only. > > So can you give us the best examples (or indeed all of them if someone > is keeping score)? hopefully this isn't a US election situation ... Gustavo? Are you running for congress now? https://lwn.net/Articles/794944/
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Sun, 2020-11-22 at 10:21 -0800, James Bottomley wrote: > Please tell me > our reward for all this effort isn't a single missing error print. There were quite literally dozens of logical defects found by the fallthrough additions. Very few were logging only.
Re: [RFC] MAINTAINERS tag for cleanup robot
On Sun, 2020-11-22 at 08:49 -0800, James Bottomley wrote: > We can enforce sysfs_emit going forwards > using tools like checkpatch It's not really possible for checkpatch to find or warn about sysfs uses of sprintf. checkpatch is really just a trivial line-by-line parser and it has no concept of code intent. It just can't warn on every use of the sprintf family. There are just too many perfectly valid uses. > but there's no benefit and a lot of harm to > be done by trying to churn the entire tree Single uses of sprintf for sysfs is not really any problem. But likely there are still several possible overrun sprintf/snprintf paths in sysfs. Some of them are very obscure and unlikely to be found by a robot as the logic for sysfs buf uses can be fairly twisty. But provably correct conversions IMO _should_ be done and IMO churn considerations should generally have less importance.
Re: [RFC] MAINTAINERS tag for cleanup robot
On Sun, 2020-11-22 at 08:33 -0800, Tom Rix wrote: > On 11/21/20 9:10 AM, Joe Perches wrote: > > On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote: > > > A difficult part of automating commits is composing the subsystem > > > preamble in the commit log. For the ongoing effort of a fixer producing > > > one or two fixes a release the use of 'treewide:' does not seem > > > appropriate. > > > > > > It would be better if the normal prefix was used. Unfortunately normal is > > > not consistent across the tree. > > > > > > So I am looking for comments for adding a new tag to the MAINTAINERS file > > > > > > D: Commit subsystem prefix > > > > > > ex/ for FPGA DFL DRIVERS > > > > > > D: fpga: dfl: > > I'm all for it. Good luck with the effort. It's not completely trivial. > > > > From a decade ago: > > > > https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/ > > > > (and that thread started with extra semicolon patches too) > > Reading the history, how about this. > > get_maintainer.pl outputs a single prefix, if multiple files have the > same prefix it works, if they don't its an error. > > Another script 'commit_one_file.sh' does the call to get_mainainter.pl > to get the prefix and be called by run-clang-tools.py to get the fixer > specific message. It's not whether the script used is get_maintainer or any other script, the question is really if the MAINTAINERS file is the appropriate place to store per-subsystem patch specific prefixes. It is. Then the question should be how are the forms described and what is the inheritance priority. My preference would be to have a default of inherit the parent base and add basename(subsystem dirname). Commit history seems to have standardized on using colons as the separator between the commit prefix and the subject. A good mechanism to explore how various subsystems have uses prefixes in the past might be something like: $ git log --no-merges --pretty='%s' - | \ perl -n -e 'print substr($_, 0, rindex($_, ":") + 1) . "\n";' | \ sort | uniq -c | sort -rn Using 1 for commit_count and drivers/scsi for subsystem_path, the top 40 entries are below: About 1% don't have a colon, and there is no real consistency even within individual drivers below scsi. For instance, qla2xxx: 1 814 scsi: qla2xxx: 2 691 scsi: lpfc: 3 389 scsi: hisi_sas: 4 354 scsi: ufs: 5 339 scsi: 6 291 qla2xxx: 7 256 scsi: megaraid_sas: 8 249 scsi: mpt3sas: 9 200 hpsa: 10 190 scsi: aacraid: 11 174 lpfc: 12 153 scsi: qedf: 13 144 scsi: smartpqi: 14 139 scsi: cxlflash: 15 122 scsi: core: 16 110 [SCSI] qla2xxx: 17 108 ncr5380: 18 98 scsi: hpsa: 19 97 20 89 treewide: 21 88 mpt3sas: 22 86 scsi: libfc: 23 85 scsi: qedi: 24 84 scsi: be2iscsi: 25 81 [SCSI] qla4xxx: 26 81 hisi_sas: 27 81 block: 28 75 megaraid_sas: 29 71 scsi: sd: 30 69 [SCSI] hpsa: 31 68 cxlflash: 32 65 scsi: libsas: 33 65 scsi: fnic: 34 61 scsi: scsi_debug: 35 60 scsi: arcmsr: 36 57 be2iscsi: 37 53 atp870u: 38 51 scsi: bfa: 39 50 scsi: storvsc: 40 48 sd:
Re: [RFC] MAINTAINERS tag for cleanup robot
On Sat, 2020-11-21 at 09:18 -0800, James Bottomley wrote: > On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote: > > A difficult part of automating commits is composing the subsystem > > preamble in the commit log. For the ongoing effort of a fixer > > producing one or two fixes a release the use of 'treewide:' does > > not seem appropriate. > > > > It would be better if the normal prefix was used. Unfortunately > > normal is not consistent across the tree. > > > > D: Commit subsystem prefix > > > > ex/ for FPGA DFL DRIVERS > > > > D: fpga: dfl: > > I've got to bet this is going to cause more issues than it solves. > SCSI uses scsi: : for drivers but not every driver has a > MAINTAINERS entry. We use either scsi: or scsi: core: for mid layer > things, but we're not consistent. Block uses blk-: for all > of it's stuff but almost no s have a MAINTAINERS entry. So > the next thing you're going to cause is an explosion of suggested > MAINTAINERs entries. As well as some changes require simultaneous changes across multiple subsystems. > Has anyone actually complained about treewide:? It depends on what you mean by treewide: If a treewide: patch is applied by some "higher level" maintainer, then generally, no. If the treewide patch is also cc'd to many individual maintainers, then yes, many many times. Mostly because patches cause what is in their view churn or that changes are not specific to their subsystem grounds. The treewide patch is sometimes dropped, sometimes broken up and generally not completely applied. What would be useful in many cases like this is for a pre and post application of the treewide patch to be compiled and the object code verified for lack of any logic change. Unfortunately, gcc does not guarantee deterministic compilation so this isn't feasible with at least gcc. Does clang guarantee this? I'm not sure it's possible: https://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html
Re: [RFC] MAINTAINERS tag for cleanup robot
On Sat, 2020-11-21 at 08:50 -0800, t...@redhat.com wrote:
> A difficult part of automating commits is composing the subsystem
> preamble in the commit log. For the ongoing effort of a fixer producing
> one or two fixes a release the use of 'treewide:' does not seem appropriate.
>
> It would be better if the normal prefix was used. Unfortunately normal is
> not consistent across the tree.
>
> So I am looking for comments for adding a new tag to the MAINTAINERS file
>
> D: Commit subsystem prefix
>
> ex/ for FPGA DFL DRIVERS
>
> D: fpga: dfl:
I'm all for it. Good luck with the effort. It's not completely trivial.
>From a decade ago:
https://lore.kernel.org/lkml/1289919077.28741.50.camel@Joe-Laptop/
(and that thread started with extra semicolon patches too)
> Continuing with cleaning up clang's -Wextra-semi-stmt
> diff --git a/Makefile b/Makefile
[]
> @@ -1567,20 +1567,21 @@ help:
> echo ''
> @echo 'Static analysers:'
> @echo ' checkstack - Generate a list of stack hogs'
> @echo ' versioncheck- Sanity check on version.h usage'
> @echo ' includecheck- Check for duplicate included header files'
> @echo ' export_report - List the usages of all exported symbols'
> @echo ' headerdep - Detect inclusion cycles in headers'
> @echo ' coccicheck - Check with Coccinelle'
> @echo ' clang-analyzer - Check with clang static analyzer'
> @echo ' clang-tidy - Check with clang-tidy'
> + @echo ' clang-tidy-fix - Check and fix with clang-tidy'
A pity the ordering of the code below isn't the same as the above.
> -PHONY += clang-tidy clang-analyzer
> +PHONY += clang-tidy-fix clang-tidy clang-analyzer
[]
> -clang-tidy clang-analyzer: $(extmod-prefix)compile_commands.json
> +clang-tidy-fix clang-tidy clang-analyzer:
> $(extmod-prefix)compile_commands.json
> $(call cmd,clang_tools)
> else
> -clang-tidy clang-analyzer:
> +clang-tidy-fix clang-tidy clang-analyzer:
[]
> diff --git a/scripts/clang-tools/run-clang-tools.py
> b/scripts/clang-tools/run-clang-tools.py
[]
> @@ -22,43 +22,57 @@ def parse_arguments():
[]
> parser.add_argument("type",
> -choices=["clang-tidy", "clang-analyzer"],
> +choices=["clang-tidy-fix", "clang-tidy",
> "clang-analyzer"],
etc...
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Fri, 2020-11-20 at 12:21 -0600, Gustavo A. R. Silva wrote: > Hi all, > > This series aims to fix almost all remaining fall-through warnings in > order to enable -Wimplicit-fallthrough for Clang. > > In preparation to enable -Wimplicit-fallthrough for Clang, explicitly > add multiple break/goto/return/fallthrough statements instead of just > letting the code fall through to the next case. > > Notice that in order to enable -Wimplicit-fallthrough for Clang, this > change[1] is meant to be reverted at some point. So, this patch helps > to move in that direction. This was a bit hard to parse for a second or three. Thanks Gustavo. How was this change done?
Re: [RFC] treewide: cleanup unreachable breaks
On Mon, 2020-10-19 at 12:42 -0700, Nick Desaulniers wrote: > On Sat, Oct 17, 2020 at 10:43 PM Greg KH wrote: > > On Sat, Oct 17, 2020 at 09:09:28AM -0700, t...@redhat.com wrote: > > > From: Tom Rix > > > > > > This is a upcoming change to clean up a new warning treewide. > > > I am wondering if the change could be one mega patch (see below) or > > > normal patch per file about 100 patches or somewhere half way by > > > collecting > > > early acks. > > > > Please break it up into one-patch-per-subsystem, like normal, and get it > > merged that way. > > > > Sending us a patch, without even a diffstat to review, isn't going to > > get you very far... > > Tom, > If you're able to automate this cleanup, I suggest checking in a > script that can be run on a directory. Then for each subsystem you > can say in your commit "I ran scripts/fix_whatever.py on this subdir." > Then others can help you drive the tree wide cleanup. Then we can > enable -Wunreachable-code-break either by default, or W=2 right now > might be a good idea. > > Ah, George (gbiv@, cc'ed), did an analysis recently of > `-Wunreachable-code-loop-increment`, `-Wunreachable-code-break`, and > `-Wunreachable-code-return` for Android userspace. From the review: > ``` > Spoilers: of these, it seems useful to turn on > -Wunreachable-code-loop-increment and -Wunreachable-code-return by > default for Android > ... > While these conventions about always having break arguably became > obsolete when we enabled -Wfallthrough, my sample turned up zero > potential bugs caught by this warning, and we'd need to put a lot of > effort into getting a clean tree. So this warning doesn't seem to be > worth it. > ``` > Looks like there's an order of magnitude of `-Wunreachable-code-break` > than the other two. > > We probably should add all 3 to W=2 builds (wrapped in cc-option). > I've filed https://github.com/ClangBuiltLinux/linux/issues/1180 to > follow up on. I suggest using W=1 as people that are doing cleanups generally use that and not W=123 or any other style. Every other use of W= is still quite noisy and these code warnings are relatively trivially to fix up.
Re: [Ocfs2-devel] [RFC] treewide: cleanup unreachable breaks
On Sun, 2020-10-18 at 19:59 +0100, Matthew Wilcox wrote: > On Sat, Oct 17, 2020 at 09:09:28AM -0700, t...@redhat.com wrote: > > clang has a number of useful, new warnings see > > https://urldefense.com/v3/__https://clang.llvm.org/docs/DiagnosticsReference.html__;!!GqivPVa7Brio!Krxz78O3RKcB9JBMVo_F98FupVhj_jxX60ddN6tKGEbv_cnooXc1nnBmchm-e_O9ieGnyQ$ > > > > Please get your IT department to remove that stupidity. If you can't, > please send email from a non-Red Hat email address. I didn't get it this way, neither did lore. It's on your end. https://lore.kernel.org/lkml/20201017160928.12698-1-t...@redhat.com/ > I don't understand why this is a useful warning to fix. Precision in coding style intent and code minimization would be the biggest factors IMO. > What actual problem is caused by the code below? Obviously none.
Re: [Cocci] [RFC] treewide: cleanup unreachable breaks
On Sat, 2020-10-17 at 20:21 +0200, Julia Lawall wrote: > On Sat, 17 Oct 2020, Joe Perches wrote: > > On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote: > > > From: Tom Rix > > > > > > This is a upcoming change to clean up a new warning treewide. > > > I am wondering if the change could be one mega patch (see below) or > > > normal patch per file about 100 patches or somewhere half way by > > > collecting > > > early acks. > > > > > > clang has a number of useful, new warnings see > > > https://clang.llvm.org/docs/DiagnosticsReference.html > > > > > > This change cleans up -Wunreachable-code-break > > > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break > > > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64. > > > > Early acks/individual patches by subsystem would be good. > > Better still would be an automated cocci script. > > Coccinelle is not especially good at this, because it is based on control > flow, and a return or goto diverts the control flow away from the break. > A hack to solve the problem is to put an if around the return or goto, but > that gives the break a meaningless file name and line number. I collected > the following list, but it only has 439 results, so fewer than clang. But > maybe there are some files that are not considered by clang in the x86 > allyesconfig configuration. > > Probably checkpatch is the best solution here, since it is not > configuration sensitive and doesn't care about control flow. Likely the clang compiler is the best option here. It might be useful to add -Wunreachable-code-break to W=1 or just always enable it if it isn't already enabled. diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn index 95e4cdb94fe9..3819787579d5 100644 --- a/scripts/Makefile.extrawarn +++ b/scripts/Makefile.extrawarn @@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable) KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable) KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned) KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation) +KBUILD_CFLAGS += $(call cc-option, -Wunreachable-code-break) # The following turn off the warnings enabled by -Wextra KBUILD_CFLAGS += -Wno-missing-field-initializers KBUILD_CFLAGS += -Wno-sign-compare (and thank you Tom for pushing this forward) checkpatch can't find instances like: case FOO: if (foo) return 1; else return 2; break; As it doesn't track flow and relies on the number of tabs to be the same for any goto/return and break; checkpatch will warn on: case FOO: ... goto bar; break;
Re: [RFC] treewide: cleanup unreachable breaks
On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote: > From: Tom Rix > > This is a upcoming change to clean up a new warning treewide. > I am wondering if the change could be one mega patch (see below) or > normal patch per file about 100 patches or somewhere half way by collecting > early acks. > > clang has a number of useful, new warnings see > https://clang.llvm.org/docs/DiagnosticsReference.html > > This change cleans up -Wunreachable-code-break > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64. Early acks/individual patches by subsystem would be good. Better still would be an automated cocci script. The existing checkpatch test for UNNECESSARY_BREAK has a few too many false positives. >From a script run on next on July 28th: arch/arm/mach-s3c24xx/mach-rx1950.c:266: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/arm/nwfpe/fpa11_cprt.c:38: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/arm/nwfpe/fpa11_cprt.c:41: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/include/asm/mach-au1x00/au1000.h:684: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/include/asm/mach-au1x00/au1000.h:687: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/include/asm/mach-au1x00/au1000.h:690: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/include/asm/mach-au1x00/au1000.h:693: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/include/asm/mach-au1x00/au1000.h:697: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/include/asm/mach-au1x00/au1000.h:700: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:276: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:279: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:282: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:287: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/loongson2ef/common/cs5536/cs5536_isa.c:290: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/rb532/setup.c:76: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/mips/rb532/setup.c:79: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/powerpc/include/asm/kvm_book3s_64.h:231: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/powerpc/include/asm/kvm_book3s_64.h:234: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/powerpc/include/asm/kvm_book3s_64.h:237: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/powerpc/include/asm/kvm_book3s_64.h:240: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/powerpc/net/bpf_jit_comp.c:455: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/powerpc/platforms/cell/spufs/switch.c:2047: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/powerpc/platforms/cell/spufs/switch.c:2077: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/sh/boards/mach-landisk/gio.c:111: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/x86/kernel/cpu/mce/core.c:1734: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/x86/kernel/cpu/mce/core.c:1738: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return arch/x86/kernel/cpu/microcode/amd.c:218: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/acpi/utils.c:107: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/acpi/utils.c:132: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/acpi/utils.c:147: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/acpi/utils.c:158: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/ata/libata-scsi.c:3973: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/base/power/main.c:366: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/block/xen-blkback/blkback.c:1272: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/char/ipmi/ipmi_devintf.c:493: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/char/lp.c:625: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/char/mwave/mwavedd.c:406: WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return drivers/cpufreq/
Re: [PATCH 1/2] xen-pciback: Use dev_printk() when possible
On Wed, 2020-05-27 at 15:34 -0700, Boris Ostrovsky wrote:
> On 5/27/20 1:43 PM, Bjorn Helgaas wrote:
> > @@ -155,8 +157,8 @@ int xen_pcibk_config_read(struct pci_dev *dev, int
> > offset, int size,
> > u32 value = 0, tmp_val;
> >
> > if (unlikely(verbose_request))
> > - printk(KERN_DEBUG DRV_NAME ": %s: read %d bytes at 0x%x\n",
> > - pci_name(dev), size, offset);
> > + dev_printk(KERN_DEBUG, &dev->dev, "read %d bytes at 0x%x\n",
> > + size, offset);
>
> Maybe then dev_dbg() ?
It likely would be better to remove verbose_request altogether
and just use dynamic debugging and dev_dbg for all the output.
$ git grep -w -A3 verbose_request
drivers/pci/xen-pcifront.c:static int verbose_request;
drivers/pci/xen-pcifront.c:module_param(verbose_request, int, 0644);
drivers/pci/xen-pcifront.c-
drivers/pci/xen-pcifront.c-static int errno_to_pcibios_err(int errno)
drivers/pci/xen-pcifront.c-{
--
drivers/pci/xen-pcifront.c: if (verbose_request)
drivers/pci/xen-pcifront.c- dev_info(&pdev->xdev->dev,
drivers/pci/xen-pcifront.c- "read dev=%04x:%02x:%02x.%d -
offset %x size %d\n",
drivers/pci/xen-pcifront.c- pci_domain_nr(bus),
bus->number, PCI_SLOT(devfn),
--
drivers/pci/xen-pcifront.c: if (verbose_request)
drivers/pci/xen-pcifront.c- dev_info(&pdev->xdev->dev,
"read got back value %x\n",
drivers/pci/xen-pcifront.c- op.value);
drivers/pci/xen-pcifront.c-
--
drivers/pci/xen-pcifront.c: if (verbose_request)
drivers/pci/xen-pcifront.c- dev_info(&pdev->xdev->dev,
drivers/pci/xen-pcifront.c- "write dev=%04x:%02x:%02x.%d -
"
drivers/pci/xen-pcifront.c- "offset %x size %d val %x\n",
--
drivers/xen/xen-pciback/conf_space.c: if (unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space.c- printk(KERN_DEBUG DRV_NAME ":
%s: read %d bytes at 0x%x\n",
drivers/xen/xen-pciback/conf_space.c- pci_name(dev), size,
offset);
drivers/xen/xen-pciback/conf_space.c-
--
drivers/xen/xen-pciback/conf_space.c: if (unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space.c- printk(KERN_DEBUG DRV_NAME ":
%s: read %d bytes at 0x%x = %x\n",
drivers/xen/xen-pciback/conf_space.c- pci_name(dev), size,
offset, value);
drivers/xen/xen-pciback/conf_space.c-
--
drivers/xen/xen-pciback/conf_space.c: if (unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space.c- printk(KERN_DEBUG
drivers/xen/xen-pciback/conf_space.c- DRV_NAME ": %s: write
request %d bytes at 0x%x = %x\n",
drivers/xen/xen-pciback/conf_space.c- pci_name(dev), size,
offset, value);
--
drivers/xen/xen-pciback/conf_space_header.c:if
(unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space_header.c-
printk(KERN_DEBUG DRV_NAME ": %s: enable\n",
drivers/xen/xen-pciback/conf_space_header.c-
pci_name(dev));
drivers/xen/xen-pciback/conf_space_header.c-err =
pci_enable_device(dev);
--
drivers/xen/xen-pciback/conf_space_header.c:if
(unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space_header.c-
printk(KERN_DEBUG DRV_NAME ": %s: disable\n",
drivers/xen/xen-pciback/conf_space_header.c-
pci_name(dev));
drivers/xen/xen-pciback/conf_space_header.c-pci_disable_device(dev);
--
drivers/xen/xen-pciback/conf_space_header.c:if
(unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space_header.c-
printk(KERN_DEBUG DRV_NAME ": %s: set bus master\n",
drivers/xen/xen-pciback/conf_space_header.c-
pci_name(dev));
drivers/xen/xen-pciback/conf_space_header.c-pci_set_master(dev);
--
drivers/xen/xen-pciback/conf_space_header.c:if
(unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space_header.c-
printk(KERN_DEBUG DRV_NAME ": %s: clear bus master\n",
drivers/xen/xen-pciback/conf_space_header.c-
pci_name(dev));
drivers/xen/xen-pciback/conf_space_header.c-pci_clear_master(dev);
--
drivers/xen/xen-pciback/conf_space_header.c:if
(unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space_header.c-
printk(KERN_DEBUG
drivers/xen/xen-pciback/conf_space_header.c- DRV_NAME
": %s: enable memory-write-invalidate\n",
drivers/xen/xen-pciback/conf_space_header.c-
pci_name(dev));
--
drivers/xen/xen-pciback/conf_space_header.c:if
(unlikely(verbose_request))
drivers/xen/xen-pciback/conf_space_header.c-
printk(KERN_DEBUG
drivers/xen/xen-pciback/conf_space_header.c- DRV_NAME
": %s: disable memory-write-invalidate\n",
drivers
[Xen-devel] [PATCH -next 020/491] XEN HYPERVISOR INTERFACE: 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 --- drivers/block/xen-blkfront.c | 5 ++--- drivers/net/xen-netfront.c | 2 +- drivers/pci/xen-pcifront.c | 2 +- drivers/scsi/xen-scsifront.c | 2 +- drivers/xen/pvcalls-front.c| 2 +- drivers/xen/xen-acpi-memhotplug.c | 2 +- drivers/xen/xen-pciback/xenbus.c | 2 +- drivers/xen/xen-scsiback.c | 2 +- drivers/xen/xenbus/xenbus_probe_frontend.c | 6 ++ 9 files changed, 11 insertions(+), 14 deletions(-) diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c index 9df516..fb07ee1 100644 --- a/drivers/block/xen-blkfront.c +++ b/drivers/block/xen-blkfront.c @@ -1402,7 +1402,6 @@ static enum blk_req_status blkif_rsp_to_req_status(int rsp) case BLKIF_RSP_EOPNOTSUPP: return REQ_EOPNOTSUPP; case BLKIF_RSP_ERROR: - /* Fallthrough. */ default: return REQ_ERROR; } @@ -1642,7 +1641,7 @@ static irqreturn_t blkif_interrupt(int irq, void *dev_id) info->feature_flush = 0; xlvbd_flush(info); } - /* fall through */ + fallthrough; case BLKIF_OP_READ: case BLKIF_OP_WRITE: if (unlikely(bret->status != BLKIF_RSP_OKAY)) @@ -2480,7 +2479,7 @@ static void blkback_changed(struct xenbus_device *dev, case XenbusStateClosed: if (dev->state == XenbusStateClosed) break; - /* fall through */ + fallthrough; case XenbusStateClosing: if (info) blkfront_closing(info); diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c index 482c6c..2001606 100644 --- a/drivers/net/xen-netfront.c +++ b/drivers/net/xen-netfront.c @@ -2038,7 +2038,7 @@ static void netback_changed(struct xenbus_device *dev, case XenbusStateClosed: if (dev->state == XenbusStateClosed) break; - /* Fall through - Missed the backend's CLOSING state. */ + fallthrough;/* Missed the backend's CLOSING state */ case XenbusStateClosing: xenbus_frontend_closed(dev); break; diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index d1b16c..093ab8 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -1103,7 +1103,7 @@ static void __ref pcifront_backend_changed(struct xenbus_device *xdev, case XenbusStateClosed: if (xdev->state == XenbusStateClosed) break; - /* fall through - Missed the backend's CLOSING state. */ + fallthrough;/* Missed the backend's CLOSING state */ case XenbusStateClosing: dev_warn(&xdev->dev, "backend going away!\n"); pcifront_try_disconnect(pdev); diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c index f0068e..259fc248 100644 --- a/drivers/scsi/xen-scsifront.c +++ b/drivers/scsi/xen-scsifront.c @@ -,7 +,7 @@ static void scsifront_backend_changed(struct xenbus_device *dev, case XenbusStateClosed: if (dev->state == XenbusStateClosed) break; - /* fall through - Missed the backend's Closing state */ + fallthrough;/* Missed the backend's Closing state */ case XenbusStateClosing: scsifront_disconnect(info); break; diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c index 57592a6..0fccf0 100644 --- a/drivers/xen/pvcalls-front.c +++ b/drivers/xen/pvcalls-front.c @@ -1260,7 +1260,7 @@ static void pvcalls_front_changed(struct xenbus_device *dev, if (dev->state == XenbusStateClosed) break; /* Missed the backend's CLOSING state */ - /* fall through */ + fallthrough; case XenbusStateClosing: xenbus_frontend_closed(dev); break; diff --git a/drivers/xen/xen-acpi-memhotplug.c b/drivers/xen/xen-acpi-memhotplug.c index 745721..f914b72 100644 --- a/drivers/xen/xen-acpi-memhotplug.c +++ b/drivers/xen/xen-acpi-memhotplug.c @@ -229,7 +229,7 @@ static void acpi_memory_device_notify(acpi_handle handle, u32 event, void *data) case ACPI_NOTIFY_BUS_CHECK: ACPI_DEBUG_PRINT((ACPI_DB_INFO, "\nReceived BUS CHECK notification for device\n")); - /*
Re: [Xen-devel] [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable
On Wed, 2019-07-17 at 08:49 +0200, Juergen Gross wrote:
> On 17.07.19 08:46, Joe Perches wrote:
> > On Tue, 2019-07-16 at 12:26 +0800, Zhenzhong Duan wrote:
> > > .. as "nopv" support needs it to be changeable at boot up stage.
> > >
> > > Checkpatch reports warning, so move variable declarations from
> > > hypervisor.c to hypervisor.h
> > []
> > > diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> > []
> > > @@ -259,7 +259,7 @@ static __init void xen_hvm_guest_late_init(void)
> > > #endif
> > > }
> > >
> > > -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> > > +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
> >
> > static?
>
> It is being referenced from arch/x86/kernel/cpu/hypervisor.c
But wasn't it also removed from the list of externs?
Rereading the .h file, no it wasn't. I missed that.
Perhaps the extern list could be reordered to move this
x86_hyper_xen_hvm to be next to x86_hyper_type.
I also suggest that "extern bool nopv" might be a bit
non-specific and could use a longer identifier.
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v8 4/5] x86/paravirt: Remove const mark from x86_hyper_xen_hvm variable
On Tue, 2019-07-16 at 12:26 +0800, Zhenzhong Duan wrote:
> .. as "nopv" support needs it to be changeable at boot up stage.
>
> Checkpatch reports warning, so move variable declarations from
> hypervisor.c to hypervisor.h
[]
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
[]
> @@ -259,7 +259,7 @@ static __init void xen_hvm_guest_late_init(void)
> #endif
> }
>
> -const __initconst struct hypervisor_x86 x86_hyper_xen_hvm = {
> +struct hypervisor_x86 x86_hyper_xen_hvm __initdata = {
static?
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [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) ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] block drivers/block: Use octal not symbolic permissions
On Thu, 2018-05-24 at 06:47 -0600, Jens Axboe wrote: > On 5/23/18 4:35 PM, Joe Perches wrote: > > On Wed, 2018-05-23 at 15:27 -0600, Jens Axboe wrote: > > > On 5/23/18 2:05 PM, Joe Perches wrote: > > > > Convert the S_ symbolic permissions to their octal equivalents as > > > > using octal and not symbolic permissions is preferred by many as more > > > > readable. > > > > > > > > see: https://lkml.org/lkml/2016/8/2/1945 > > > > > > > > Done with automated conversion via: > > > > $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace > > > > > > > > > > > > Miscellanea: > > > > > > > > o Wrapped modified multi-line calls to a single line where appropriate > > > > o Realign modified multi-line calls to open parenthesis > > > > > > Honestly, I see this as pretty needless churn. > > > > btw: > > > > There is currently a mixture of symbolic and octal > > permissions uses in block and drivers/block > > > > ie: 94 octal and 146 symbolic uses. > > > > If this is applied, all would become octal. > > That does help justify the change. My main worry here is creating > unnecessary conflicts, which is always annoying. But it's even more > annoying when the change creating the conflict isn't really that > important at all. Case in point, the patch doesn't apply to the > for-4.18/block branch that it should go into... Done against most recent -next as it's basically impossible to do anything against multiple private trees. Also, the script that generated the patch is in the changelog so it's simple to rerun. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] block drivers/block: Use octal not symbolic permissions
On Wed, 2018-05-23 at 15:27 -0600, Jens Axboe wrote: > On 5/23/18 2:05 PM, Joe Perches wrote: > > Convert the S_ symbolic permissions to their octal equivalents as > > using octal and not symbolic permissions is preferred by many as more > > readable. > > > > see: https://lkml.org/lkml/2016/8/2/1945 > > > > Done with automated conversion via: > > $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace > > > > Miscellanea: > > > > o Wrapped modified multi-line calls to a single line where appropriate > > o Realign modified multi-line calls to open parenthesis > > Honestly, I see this as pretty needless churn. btw: There is currently a mixture of symbolic and octal permissions uses in block and drivers/block ie: 94 octal and 146 symbolic uses. If this is applied, all would become octal. ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] block drivers/block: Use octal not symbolic permissions
On Wed, 2018-05-23 at 15:27 -0600, Jens Axboe wrote: > On 5/23/18 2:05 PM, Joe Perches wrote: > > Convert the S_ symbolic permissions to their octal equivalents as > > using octal and not symbolic permissions is preferred by many as more > > readable. > > > > see: https://lkml.org/lkml/2016/8/2/1945 > > > > Done with automated conversion via: > > $ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace > > > > Miscellanea: > > > > o Wrapped modified multi-line calls to a single line where appropriate > > o Realign modified multi-line calls to open parenthesis > > Honestly, I see this as pretty needless churn. It's just for consistency and the ability to highlight somewhat unusual permissions uses like just 0400. Apply it at your leisure or ignore it. btw: Joshua Morris' email address is bouncing. Maybe it should be removed from MAINTAINERS. --- MAINTAINERS | 1 - 1 file changed, 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 9051a9ca24a2..0d546a10d0b9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5598,7 +5598,6 @@ F:drivers/base/firmware_loader/ F: include/linux/firmware.h FLASH ADAPTER DRIVER (IBM Flash Adapter 900GB Full Height PCI Flash Card) -M: Joshua Morris M: Philip Kelleher S: Maintained F: drivers/block/rsxx/ ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] block drivers/block: Use octal not symbolic permissions
Convert the S_ symbolic permissions to their octal equivalents as
using octal and not symbolic permissions is preferred by many as more
readable.
see: https://lkml.org/lkml/2016/8/2/1945
Done with automated conversion via:
$ ./scripts/checkpatch.pl -f --types=SYMBOLIC_PERMS --fix-inplace
Miscellanea:
o Wrapped modified multi-line calls to a single line where appropriate
o Realign modified multi-line calls to open parenthesis
Signed-off-by: Joe Perches
---
block/blk-integrity.c | 12 +++
block/blk-mq-sysfs.c| 6 ++--
block/blk-sysfs.c | 68 ++---
block/cfq-iosched.c | 2 +-
block/deadline-iosched.c| 3 +-
block/genhd.c | 37 ++--
block/mq-deadline.c | 3 +-
block/partition-generic.c | 22 ++--
drivers/block/DAC960.c | 3 +-
drivers/block/aoe/aoeblk.c | 10 +++---
drivers/block/brd.c | 6 ++--
drivers/block/drbd/drbd_debugfs.c | 20 +--
drivers/block/drbd/drbd_main.c | 4 +--
drivers/block/floppy.c | 2 +-
drivers/block/loop.c| 6 ++--
drivers/block/mtip32xx/mtip32xx.c | 11 +++---
drivers/block/nbd.c | 2 +-
drivers/block/null_blk.c| 30
drivers/block/pktcdvd.c | 4 +--
drivers/block/rbd.c | 44
drivers/block/rsxx/core.c | 6 ++--
drivers/block/virtio_blk.c | 6 ++--
drivers/block/xen-blkback/blkback.c | 2 +-
drivers/block/xen-blkback/xenbus.c | 4 +--
drivers/block/xen-blkfront.c| 7 ++--
25 files changed, 156 insertions(+), 164 deletions(-)
diff --git a/block/blk-integrity.c b/block/blk-integrity.c
index feb30570eaf5..6121611e1316 100644
--- a/block/blk-integrity.c
+++ b/block/blk-integrity.c
@@ -333,34 +333,34 @@ static ssize_t integrity_device_show(struct blk_integrity
*bi, char *page)
}
static struct integrity_sysfs_entry integrity_format_entry = {
- .attr = { .name = "format", .mode = S_IRUGO },
+ .attr = { .name = "format", .mode = 0444 },
.show = integrity_format_show,
};
static struct integrity_sysfs_entry integrity_tag_size_entry = {
- .attr = { .name = "tag_size", .mode = S_IRUGO },
+ .attr = { .name = "tag_size", .mode = 0444 },
.show = integrity_tag_size_show,
};
static struct integrity_sysfs_entry integrity_interval_entry = {
- .attr = { .name = "protection_interval_bytes", .mode = S_IRUGO },
+ .attr = { .name = "protection_interval_bytes", .mode = 0444 },
.show = integrity_interval_show,
};
static struct integrity_sysfs_entry integrity_verify_entry = {
- .attr = { .name = "read_verify", .mode = S_IRUGO | S_IWUSR },
+ .attr = { .name = "read_verify", .mode = 0644 },
.show = integrity_verify_show,
.store = integrity_verify_store,
};
static struct integrity_sysfs_entry integrity_generate_entry = {
- .attr = { .name = "write_generate", .mode = S_IRUGO | S_IWUSR },
+ .attr = { .name = "write_generate", .mode = 0644 },
.show = integrity_generate_show,
.store = integrity_generate_store,
};
static struct integrity_sysfs_entry integrity_device_entry = {
- .attr = { .name = "device_is_integrity_capable", .mode = S_IRUGO },
+ .attr = { .name = "device_is_integrity_capable", .mode = 0444 },
.show = integrity_device_show,
};
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index a54b4b070f1c..aafb44224c89 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -166,15 +166,15 @@ static struct attribute *default_ctx_attrs[] = {
};
static struct blk_mq_hw_ctx_sysfs_entry blk_mq_hw_sysfs_nr_tags = {
- .attr = {.name = "nr_tags", .mode = S_IRUGO },
+ .attr = {.name = "nr_tags", .mode = 0444 },
.show = blk_mq_hw_sysfs_nr_tags_show,
};
static struct blk_mq_hw_ctx_sysfs_entry blk_mq_hw_sysfs_nr_reserved_tags = {
- .attr = {.name = "nr_reserved_tags", .mode = S_IRUGO },
+ .attr = {.name = "nr_reserved_tags", .mode = 0444 },
.show = blk_mq_hw_sysfs_nr_reserved_tags_show,
};
static struct blk_mq_hw_ctx_sysfs_entry blk_mq_hw_sysfs_cpus = {
- .attr = {.name = "cpu_list", .mode = S_IRUGO },
+ .attr = {.name = "cpu_list", .mode = 0444 },
.show = blk_mq_hw_sysfs_cpus_show,
};
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index cae525b7aae6..31347e31daa3 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -502,187 +502,187 @@ static ssize_t queue_dax_show(struct request_queue *q,
char *page)
}
static struct queue_sysfs_entry queue_requests_entry = {
- .attr = {.n
[Xen-devel] [PATCH 4/4] drivers/net: Use octal not symbolic permissions
Prefer the direct use of octal for permissions.
Done with checkpatch -f --types=SYMBOLIC_PERMS --fix-inplace
and some typing.
Miscellanea:
o Whitespace neatening around these conversions.
Signed-off-by: Joe Perches
---
drivers/net/bonding/bond_procfs.c | 2 +-
drivers/net/bonding/bond_sysfs.c | 73 +-
drivers/net/bonding/bond_sysfs_slave.c | 4 +-
drivers/net/caif/caif_serial.c | 32 +++
drivers/net/caif/caif_spi.c| 16
drivers/net/caif/caif_virtio.c | 16
drivers/net/can/at91_can.c | 3 +-
drivers/net/can/cc770/cc770.c | 4 +-
drivers/net/can/cc770/cc770_isa.c | 16
drivers/net/can/grcan.c| 4 +-
drivers/net/can/janz-ican3.c | 6 +--
drivers/net/can/sja1000/sja1000_isa.c | 14 +++
drivers/net/can/softing/softing_main.c | 4 +-
drivers/net/can/spi/mcp251x.c | 2 +-
drivers/net/can/usb/esd_usb2.c | 6 +--
drivers/net/can/vcan.c | 2 +-
drivers/net/hamradio/bpqether.c| 3 +-
drivers/net/hamradio/yam.c | 2 +-
drivers/net/hyperv/netvsc_drv.c| 4 +-
drivers/net/ieee802154/at86rf230.c | 2 +-
drivers/net/phy/spi_ks8995.c | 2 +-
drivers/net/ppp/ppp_generic.c | 2 +-
drivers/net/ppp/pppoe.c| 2 +-
drivers/net/usb/cdc_ncm.c | 12 +++---
drivers/net/usb/hso.c | 8 ++--
drivers/net/xen-netback/xenbus.c | 4 +-
drivers/net/xen-netfront.c | 6 +--
27 files changed, 124 insertions(+), 127 deletions(-)
diff --git a/drivers/net/bonding/bond_procfs.c
b/drivers/net/bonding/bond_procfs.c
index f7799321dffb..01059f1a7bca 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -287,7 +287,7 @@ void bond_create_proc_entry(struct bonding *bond)
if (bn->proc_dir) {
bond->proc_entry = proc_create_data(bond_dev->name,
- S_IRUGO, bn->proc_dir,
+ 0444, bn->proc_dir,
&bond_info_fops, bond);
if (bond->proc_entry == NULL)
netdev_warn(bond_dev, "Cannot create /proc/net/%s/%s\n",
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 040b493f60ae..6096440e96ea 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -147,7 +147,7 @@ static ssize_t bonding_store_bonds(struct class *cls,
static const struct class_attribute class_attr_bonding_masters = {
.attr = {
.name = "bonding_masters",
- .mode = S_IWUSR | S_IRUGO,
+ .mode = 0644,
},
.show = bonding_show_bonds,
.store = bonding_store_bonds,
@@ -202,7 +202,7 @@ static ssize_t bonding_show_slaves(struct device *d,
return res;
}
-static DEVICE_ATTR(slaves, S_IRUGO | S_IWUSR, bonding_show_slaves,
+static DEVICE_ATTR(slaves, 0644, bonding_show_slaves,
bonding_sysfs_store_option);
/* Show the bonding mode. */
@@ -216,8 +216,7 @@ static ssize_t bonding_show_mode(struct device *d,
return sprintf(buf, "%s %d\n", val->string, BOND_MODE(bond));
}
-static DEVICE_ATTR(mode, S_IRUGO | S_IWUSR,
- bonding_show_mode, bonding_sysfs_store_option);
+static DEVICE_ATTR(mode, 0644, bonding_show_mode, bonding_sysfs_store_option);
/* Show the bonding transmit hash method. */
static ssize_t bonding_show_xmit_hash(struct device *d,
@@ -231,7 +230,7 @@ static ssize_t bonding_show_xmit_hash(struct device *d,
return sprintf(buf, "%s %d\n", val->string, bond->params.xmit_policy);
}
-static DEVICE_ATTR(xmit_hash_policy, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(xmit_hash_policy, 0644,
bonding_show_xmit_hash, bonding_sysfs_store_option);
/* Show arp_validate. */
@@ -247,7 +246,7 @@ static ssize_t bonding_show_arp_validate(struct device *d,
return sprintf(buf, "%s %d\n", val->string, bond->params.arp_validate);
}
-static DEVICE_ATTR(arp_validate, S_IRUGO | S_IWUSR, bonding_show_arp_validate,
+static DEVICE_ATTR(arp_validate, 0644, bonding_show_arp_validate,
bonding_sysfs_store_option);
/* Show arp_all_targets. */
@@ -263,7 +262,7 @@ static ssize_t bonding_show_arp_all_targets(struct device
*d,
return sprintf(buf, "%s %d\n",
val->string, bond->params.arp_all_targets);
}
-static DEVICE_ATTR(arp_all_targets, S_IRUGO | S_IWUSR,
+static DEVICE_ATTR(arp_all_targets, 0644,
bonding_show_arp_all_targets, bonding_sysfs_store_option);
/* Show fail_over_mac. */
@@ -279,7 +278,7 @@ static ssize_t bonding_show_fail_over
[Xen-devel] [PATCH 0/4] net: drivers/net: Use octal permissions
Using octal and not symbolic permissions is preferred by many as more readable. https://lkml.org/lkml/2016/8/2/1945 Rather than getting these piecemeal, just do them all. Done with checkpatch and some typing. Joe Perches (4): ethernet: Use octal not symbolic permissions wireless: Use octal not symbolic permissions net: Use octal not symbolic permissions drivers/net: Use octal not symbolic permissions drivers/net/bonding/bond_procfs.c | 2 +- drivers/net/bonding/bond_sysfs.c | 73 +++--- drivers/net/bonding/bond_sysfs_slave.c | 4 +- drivers/net/caif/caif_serial.c | 32 +++--- drivers/net/caif/caif_spi.c| 16 +-- drivers/net/caif/caif_virtio.c | 16 +-- drivers/net/can/at91_can.c | 3 +- drivers/net/can/cc770/cc770.c | 4 +- drivers/net/can/cc770/cc770_isa.c | 16 +-- drivers/net/can/grcan.c| 4 +- drivers/net/can/janz-ican3.c | 6 +- drivers/net/can/sja1000/sja1000_isa.c | 14 +-- drivers/net/can/softing/softing_main.c | 4 +- drivers/net/can/spi/mcp251x.c | 2 +- drivers/net/can/usb/esd_usb2.c | 6 +- drivers/net/can/vcan.c | 2 +- drivers/net/ethernet/8390/apne.c | 2 +- drivers/net/ethernet/8390/lib8390.c| 2 +- drivers/net/ethernet/8390/ne.c | 2 +- drivers/net/ethernet/8390/ne2k-pci.c | 2 +- drivers/net/ethernet/8390/smc-ultra.c | 2 +- drivers/net/ethernet/8390/stnic.c | 2 +- drivers/net/ethernet/8390/wd.c | 2 +- drivers/net/ethernet/altera/altera_tse_main.c | 6 +- drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 10 +- drivers/net/ethernet/amd/xgbe/xgbe-main.c | 2 +- drivers/net/ethernet/broadcom/bnx2.c | 2 +- drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c | 12 +-- drivers/net/ethernet/broadcom/sb1250-mac.c | 10 +- drivers/net/ethernet/broadcom/tg3.c| 6 +- drivers/net/ethernet/brocade/bna/bnad.c| 2 +- drivers/net/ethernet/brocade/bna/bnad_debugfs.c| 10 +- drivers/net/ethernet/cavium/thunder/nicvf_main.c | 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c| 6 +- drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 112 ++--- .../net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c| 10 +- drivers/net/ethernet/ec_bhf.c | 2 +- drivers/net/ethernet/emulex/benet/be_main.c| 6 +- drivers/net/ethernet/ibm/ehea/ehea_main.c | 7 +- drivers/net/ethernet/ibm/ibmveth.c | 2 +- drivers/net/ethernet/intel/igb/igb_hwmon.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 2 +- drivers/net/ethernet/marvell/mvneta.c | 8 +- drivers/net/ethernet/marvell/skge.c| 2 +- drivers/net/ethernet/marvell/sky2.c| 2 +- drivers/net/ethernet/mellanox/mlx4/main.c | 16 +-- drivers/net/ethernet/mellanox/mlxsw/core_hwmon.c | 10 +- drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 32 +++--- .../net/ethernet/netronome/nfp/nfp_net_debugfs.c | 6 +- .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 14 +-- drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c | 30 +++--- drivers/net/ethernet/qualcomm/qca_debug.c | 2 +- drivers/net/ethernet/samsung/sxgbe/sxgbe_main.c| 4 +- drivers/net/ethernet/sfc/mcdi_mon.c| 2 +- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 26 ++--- drivers/net/ethernet/sun/niu.c | 10 +- drivers/net/hamradio/bpqether.c| 3 +- drivers/net/hamradio/yam.c | 2 +- drivers/net/hyperv/netvsc_drv.c| 4 +- drivers/net/ieee802154/at86rf230.c | 2 +- drivers/net/phy/spi_ks8995.c | 2 +- drivers/net/ppp/ppp_generic.c | 2 +- drivers/net/ppp/pppoe.c| 2 +- drivers/net/usb/cdc_ncm.c | 12 +-- drivers/net/usb/hso.c | 8 +- drivers/net/wireless/ath/ath5k/base.c | 6 +- drivers/net/wireless/ath/ath5k/debug.c | 37 ++- drivers/net/wireless/ath/ath5k/sysfs.c | 8 +- drivers/net/wireless/ath/ath6kl/debug.c| 43 drivers/net/wireless/ath/ath9k/common-debug.c | 9 +- drivers/net/wireless/ath/ath9k/common-spectral.c | 10 +- drivers/net/wireless/ath/ath9k/debug.c | 40 drivers/net/wireless/ath/ath9k/debug_sta.c | 6 +- drivers/net/wireless/ath/ath9k/dfs_debug.c | 4
Re: [Xen-devel] [PATCH v2] x86-64/Xen: eliminate W+X mappings
On Mon, 2017-12-18 at 13:28 +0100, Ingo Molnar wrote:
> * Jan Beulich wrote:
>
> > A few thousand such pages are usually left around due to the re-use of
> > L1 tables having been provided by the hypervisor (Dom0) or tool stack
> > (DomU). Set NX in the direct map variant, which needs to be done in L2
> > due to the dual use of the re-used L1s.
> >
> > For x86_configure_nx() to actually do what it is supposed to do, call
> > get_cpu_cap() first. This was broken by commit 4763ed4d45 ("x86, mm:
> > Clean up and simplify NX enablement") when switching away from the
> > direct EFER read.
[]
> > --- 4.15-rc4/arch/x86/xen/mmu_pv.c
> > +++ 4.15-rc4-x86_64-Xen-avoid-W+X/arch/x86/xen/mmu_pv.c
> > @@ -1902,6 +1902,18 @@ void __init xen_setup_kernel_pagetable(p
> > /* Graft it onto L4[511][510] */
> > copy_page(level2_kernel_pgt, l2);
> >
> > + /*
> > +* Zap execute permission from the ident map. Due to the sharing of
> > +* L1 entries we need to do this in the L2.
> > +*/
> > + if (__supported_pte_mask & _PAGE_NX)
> > + for (i = 0; i < PTRS_PER_PMD; ++i) {
> > + if (pmd_none(level2_ident_pgt[i]))
> > + continue;
> > + level2_ident_pgt[i] = pmd_set_flags(level2_ident_pgt[i],
> > + _PAGE_NX);
> > + }
> > +
>
> This chunk has two stylistic problems:
>
> - Curly braces need to be added
> - Line broken in an ugly fashion: just make it long and ignore the
> checkpatch col80 warning
>
> looks good otherwise.
stylistic trivia:
Instead of repeating level2_ident_pgt[i] multiple times,
it might be nicer to use temporaries and not use i at all.
Something like:
if (__supported_pte_mask & _PAGE_NX) {
pmd_t *pmd = level2_ident_pgt;
pmd_t *end = pmd + PTRS_PER_PMD;
for (; pmd < end; pmd++) {
if (!pmd_none(pmd))
*pmd = pmd_set_flags(pmd, _PAGE_NX);
}
}
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH] xen-netback: Fix logging message with spurious period after newline
Using a period after a newline causes bad output.
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 d6dff347f896..78ebe494fef0 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -186,7 +186,7 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct
net_device *dev)
/* Obtain the queue to be used to transmit this packet */
index = skb_get_queue_mapping(skb);
if (index >= num_queues) {
- pr_warn_ratelimited("Invalid queue %hu for packet on interface
%s\n.",
+ pr_warn_ratelimited("Invalid queue %hu for packet on interface
%s\n",
index, vif->dev->name);
index %= num_queues;
}
--
2.15.0
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
