Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

[Julia Lawall]

On Sat, 27 Oct 2018, Joe Perches wrote:

> On Fri, 2018-10-26 at 22:54 +0200, Julia Lawall wrote:
> > [Adding Joe Perches]
> >
> > On Fri, 26 Oct 2018, Sasha Levin wrote:
> >
> > > On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote:
> > > > This change was suggested by checkpath.pl. Use unsigned int with 
> > > > bitfield
> > > > allocate only one bit to the boolean variable.
> > > >
> > > > CHECK: Avoid using bool structure members because of possible alignment
> > > > issues
> > > >
> > > > Signed-off-by: Shayenne da Luz Moura 
> > > > ---
> > > > drivers/staging/vboxvideo/vbox_drv.h| 14 +++---
> > > > drivers/staging/vboxvideo/vboxvideo_guest.h |  2 +-
> > > > 2 files changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> > > > b/drivers/staging/vboxvideo/vbox_drv.h
> > > > index 594f84272957..7d3e329a6b1c 100644
> > > > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > > > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > > > @@ -81,7 +81,7 @@ struct vbox_private {
> > > > u8 __iomem *vbva_buffers;
> > > > struct gen_pool *guest_pool;
> > > > struct vbva_buf_ctx *vbva_info;
> > > > -   bool any_pitch;
> > > > +   unsigned int any_pitch:1;
> > > > u32 num_crtcs;
> > > > /** Amount of available VRAM, including space used for buffers. 
> > > > */
> > > > u32 full_vram_size;
> > >
> > > Using bitfields for booleans in these cases is less efficient than just
> > > using "regular" booleans for two reasons:
> > >
> > > 1. It will use the same amount of space. Due to alignment requirements,
> > > the compiler can't squeeze in anything into the 7 bits that are now
> > > "free". Each member, unless it's another bitfield, must start at a whole
> > > byte.

Since this is between a pointer and a u32, won't the compiler put a lot
more padding, in both cases?

> > >
> > > 2. This is actually less efficient (slower) for the compiler to work
> > > with. The smallest granularity we have to access memory is 1 byte; we
> > > can't set individual bits directly in memory. For the original code, the
> > > assembly for 'vbox_private.any_pitch = true' would look something like
> > > this:
> > >
> > >   movl   $0x1,-0x10(%rsp)
> > >
> > > As you can see, the compiler can directly write into the variable.
> > > However, when we switch to using bitfields, the compiler must preserve
> > > the original value of the other 7 bits, so it must first read them from
> > > memory, manipulate the value and write it back. The assembly would
> > > look something like this:
> > >
> > >   movzbl -0x10(%rsp),%eax
> > >   or $0x1,%eax
> > >   mov%al,-0x10(%rsp)
> > >
> > > Which is less efficient than what was previously happening.
> >
> > Maybe checkpatch could be more precise about what kind of bools should be
> > changed?
>
> Probably so, what verbiage would you suggest?

I don't know what are the conditions.  Sasha?

julia

> Also, any conversion from bool to int would
> have to take care than any assigment uses !!
> where appropriate.
>
>
>

Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

[Julia Lawall]

On Sat, 27 Oct 2018, Joe Perches wrote:

> On Fri, 2018-10-26 at 22:54 +0200, Julia Lawall wrote:
> > [Adding Joe Perches]
> >
> > On Fri, 26 Oct 2018, Sasha Levin wrote:
> >
> > > On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote:
> > > > This change was suggested by checkpath.pl. Use unsigned int with 
> > > > bitfield
> > > > allocate only one bit to the boolean variable.
> > > >
> > > > CHECK: Avoid using bool structure members because of possible alignment
> > > > issues
> > > >
> > > > Signed-off-by: Shayenne da Luz Moura 
> > > > ---
> > > > drivers/staging/vboxvideo/vbox_drv.h| 14 +++---
> > > > drivers/staging/vboxvideo/vboxvideo_guest.h |  2 +-
> > > > 2 files changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> > > > b/drivers/staging/vboxvideo/vbox_drv.h
> > > > index 594f84272957..7d3e329a6b1c 100644
> > > > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > > > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > > > @@ -81,7 +81,7 @@ struct vbox_private {
> > > > u8 __iomem *vbva_buffers;
> > > > struct gen_pool *guest_pool;
> > > > struct vbva_buf_ctx *vbva_info;
> > > > -   bool any_pitch;
> > > > +   unsigned int any_pitch:1;
> > > > u32 num_crtcs;
> > > > /** Amount of available VRAM, including space used for buffers. 
> > > > */
> > > > u32 full_vram_size;
> > >
> > > Using bitfields for booleans in these cases is less efficient than just
> > > using "regular" booleans for two reasons:
> > >
> > > 1. It will use the same amount of space. Due to alignment requirements,
> > > the compiler can't squeeze in anything into the 7 bits that are now
> > > "free". Each member, unless it's another bitfield, must start at a whole
> > > byte.

Since this is between a pointer and a u32, won't the compiler put a lot
more padding, in both cases?

> > >
> > > 2. This is actually less efficient (slower) for the compiler to work
> > > with. The smallest granularity we have to access memory is 1 byte; we
> > > can't set individual bits directly in memory. For the original code, the
> > > assembly for 'vbox_private.any_pitch = true' would look something like
> > > this:
> > >
> > >   movl   $0x1,-0x10(%rsp)
> > >
> > > As you can see, the compiler can directly write into the variable.
> > > However, when we switch to using bitfields, the compiler must preserve
> > > the original value of the other 7 bits, so it must first read them from
> > > memory, manipulate the value and write it back. The assembly would
> > > look something like this:
> > >
> > >   movzbl -0x10(%rsp),%eax
> > >   or $0x1,%eax
> > >   mov%al,-0x10(%rsp)
> > >
> > > Which is less efficient than what was previously happening.
> >
> > Maybe checkpatch could be more precise about what kind of bools should be
> > changed?
>
> Probably so, what verbiage would you suggest?

I don't know what are the conditions.  Sasha?

julia

> Also, any conversion from bool to int would
> have to take care than any assigment uses !!
> where appropriate.
>
>
>

linux-next: Signed-off-by missing for commit in the c6x tree

[Stephen Rothwell]

Hi Mark,

Commit

  fe381767b94f ("c6x: switch to NO_BOOTMEM")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell


pgplP20KbtClc.pgp
Description: OpenPGP digital signature

linux-next: Signed-off-by missing for commit in the c6x tree

[Stephen Rothwell]

Hi Mark,

Commit

  fe381767b94f ("c6x: switch to NO_BOOTMEM")

is missing a Signed-off-by from its committer.

-- 
Cheers,
Stephen Rothwell


pgplP20KbtClc.pgp
Description: OpenPGP digital signature

Re: [RFC] rcu: doc: update example about stale data

[Joel Fernandes]

On Sat, Oct 27, 2018 at 7:16 PM, Joel Fernandes (Google)
 wrote:
> The RCU example for 'rejecting stale data' on system-call auditting
> stops iterating through the rules if a deleted one is found. It makes
> more sense to continue looking at other rules once a deleted one is
> rejected. Although the original example is fine, this makes it more
> meaningful.

Sorry, I messed up the patch title, it is supposed to be 'doc: rcu:
...'. I can resend it if you want.

thanks,

- Joel

Re: [RFC] rcu: doc: update example about stale data

[Joel Fernandes]

On Sat, Oct 27, 2018 at 7:16 PM, Joel Fernandes (Google)
 wrote:
> The RCU example for 'rejecting stale data' on system-call auditting
> stops iterating through the rules if a deleted one is found. It makes
> more sense to continue looking at other rules once a deleted one is
> rejected. Although the original example is fine, this makes it more
> meaningful.

Sorry, I messed up the patch title, it is supposed to be 'doc: rcu:
...'. I can resend it if you want.

thanks,

- Joel

perf synthesized mmap timeouts

[David Miller]

If I understand the commit message for:

commit 8cc42de736b617827a4e7664fb8d7a325bc125bc
Author: Kan Liang 
Date:   Thu Jan 18 13:26:32 2018 -0800

perf top: Check the latency of perf_top__mmap_read()

properly, the problem is that a malicious or out of control
app can be doing endless mmaps causing perf to loop forever
processing the /proc/$PID/maps file.

But that is not what this commit is handling at all.

It is instead applying a large hammer which quits if it is taking a
long time to process the maps, not if the process's mmap list is
growing endlessly while we process it.

This triggers any time I run perf top on a fully loaded system making
perf less useful than it should be.

And it triggers simply because the perf synthesize threads have to
share the cpu with the workload already running.

So it takes more than half a second to process emacs's 527 maps when
the number of running processes is ~NCPUS?  Big deal.  We should let
it finish

The tradeoff choosen here is really bad.

Guess what happens if you don't have maps for a given process?

What happens is that for every single sample we get within that range,
we get a completely unique histogram entry.

This means potentially millions and millions of histogram entries
where there should only be a few hundred.

This makes the histogram rbtree huge, and slow to process.

So not only is top unable to provide correct histogram output, it is
also running sluggishly.

A way to mitigate the actual problem would be to snapshot the maps
file into a large buffer, if possible.  We can get the full contents
faster than the process in question can make more maps.  At most we
will do one additional read at the end if they were able to sneak in
one new mmap during the initial read.

No timeout necessary.  We have the complete maps file, our processing
time is therefore bounded.

Thanks.

perf synthesized mmap timeouts

[David Miller]

If I understand the commit message for:

commit 8cc42de736b617827a4e7664fb8d7a325bc125bc
Author: Kan Liang 
Date:   Thu Jan 18 13:26:32 2018 -0800

perf top: Check the latency of perf_top__mmap_read()

properly, the problem is that a malicious or out of control
app can be doing endless mmaps causing perf to loop forever
processing the /proc/$PID/maps file.

But that is not what this commit is handling at all.

It is instead applying a large hammer which quits if it is taking a
long time to process the maps, not if the process's mmap list is
growing endlessly while we process it.

This triggers any time I run perf top on a fully loaded system making
perf less useful than it should be.

And it triggers simply because the perf synthesize threads have to
share the cpu with the workload already running.

So it takes more than half a second to process emacs's 527 maps when
the number of running processes is ~NCPUS?  Big deal.  We should let
it finish

The tradeoff choosen here is really bad.

Guess what happens if you don't have maps for a given process?

What happens is that for every single sample we get within that range,
we get a completely unique histogram entry.

This means potentially millions and millions of histogram entries
where there should only be a few hundred.

This makes the histogram rbtree huge, and slow to process.

So not only is top unable to provide correct histogram output, it is
also running sluggishly.

A way to mitigate the actual problem would be to snapshot the maps
file into a large buffer, if possible.  We can get the full contents
faster than the process in question can make more maps.  At most we
will do one additional read at the end if they were able to sneak in
one new mmap during the initial read.

No timeout necessary.  We have the complete maps file, our processing
time is therefore bounded.

Thanks.

[PATCH] staging: olpc_dcon: olpc_dcon_xo_1.c: Switch to the gpio descriptor interface

[Nishad Kamdar]

Use the gpiod interface instead of the deprecated old non-descriptor
interface in olpc_dcon_xo_1.c.

Signed-off-by: Nishad Kamdar 
---
 drivers/staging/olpc_dcon/olpc_dcon.h  | 12 
 drivers/staging/olpc_dcon/olpc_dcon_xo_1.c | 74 ++
 2 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.h 
b/drivers/staging/olpc_dcon/olpc_dcon.h
index c987aaf894e7..6590d251eb6e 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.h
+++ b/drivers/staging/olpc_dcon/olpc_dcon.h
@@ -57,6 +57,18 @@
 /* Interrupt */
 #define DCON_IRQ6
 
+struct dcon_gpio {
+   struct gpio_desc **ptr;
+   const char *name;
+   unsigned long flags;
+};
+
+struct gpio_desc *dcon_stat0;
+struct gpio_desc *dcon_stat1;
+struct gpio_desc *dcon_irq;
+struct gpio_desc *dcon_load;
+struct gpio_desc *dcon_blank;
+
 struct dcon_priv {
struct i2c_client *client;
struct fb_info *fbinfo;
diff --git a/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c 
b/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
index ff145d493e1b..83607efcb21f 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
@@ -11,7 +11,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -20,26 +20,29 @@
 static int dcon_init_xo_1(struct dcon_priv *dcon)
 {
unsigned char lob;
-
-   if (gpio_request(OLPC_GPIO_DCON_STAT0, "OLPC-DCON")) {
-   pr_err("failed to request STAT0 GPIO\n");
-   return -EIO;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_STAT1, "OLPC-DCON")) {
-   pr_err("failed to request STAT1 GPIO\n");
-   goto err_gp_stat1;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_IRQ, "OLPC-DCON")) {
-   pr_err("failed to request IRQ GPIO\n");
-   goto err_gp_irq;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_LOAD, "OLPC-DCON")) {
-   pr_err("failed to request LOAD GPIO\n");
-   goto err_gp_load;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_BLANK, "OLPC-DCON")) {
-   pr_err("failed to request BLANK GPIO\n");
-   goto err_gp_blank;
+   int ret, i;
+   struct dcon_gpio *pin;
+   unsigned long flags = GPIOD_ASIS;
+
+   struct dcon_gpio gpios[] = {
+   { .ptr = _stat0, .name = "dcon_stat0", .flags = flags },
+   { .ptr = _stat1, .name = "dcon_stat1", .flags = flags },
+   { .ptr = _irq, .name = "dcon_irq", .flags = flags },
+   { .ptr = _load, .name = "dcon_load", .flags = flags },
+   { .ptr = _blank, .name = "dcon_blank", .flags = flags },
+   };
+
+   for (i = 0; i < ARRAY_SIZE(gpios); i++) {
+   pin = [i];
+   *pin->ptr = devm_gpiod_get(>bl_dev->dev, pin->name,
+  pin->flags);
+   if (IS_ERR(*pin->ptr)) {
+   ret = PTR_ERR(*pin->ptr);
+   dev_err(>bl_dev->dev,
+   "failed to request %s GPIO: %d\n",
+   pin->name, ret);
+   return ret;
+   }
}
 
/* Turn off the event enable for GPIO7 just to be safe */
@@ -61,12 +64,11 @@ static int dcon_init_xo_1(struct dcon_priv *dcon)
dcon->pending_src = dcon->curr_src;
 
/* Set the directions for the GPIO pins */
-   gpio_direction_input(OLPC_GPIO_DCON_STAT0);
-   gpio_direction_input(OLPC_GPIO_DCON_STAT1);
-   gpio_direction_input(OLPC_GPIO_DCON_IRQ);
-   gpio_direction_input(OLPC_GPIO_DCON_BLANK);
-   gpio_direction_output(OLPC_GPIO_DCON_LOAD,
- dcon->curr_src == DCON_SOURCE_CPU);
+   gpiod_direction_input(dcon_stat0);
+   gpiod_direction_input(dcon_stat1);
+   gpiod_direction_input(dcon_irq);
+   gpiod_direction_input(dcon_blank);
+   gpiod_direction_output(dcon_load, dcon->curr_src == DCON_SOURCE_CPU);
 
/* Set up the interrupt mappings */
 
@@ -125,18 +127,6 @@ static int dcon_init_xo_1(struct dcon_priv *dcon)
cs5535_gpio_set(OLPC_GPIO_DCON_BLANK, GPIO_EVENTS_ENABLE);
 
return 0;
-
-err_req_irq:
-   gpio_free(OLPC_GPIO_DCON_BLANK);
-err_gp_blank:
-   gpio_free(OLPC_GPIO_DCON_LOAD);
-err_gp_load:
-   gpio_free(OLPC_GPIO_DCON_IRQ);
-err_gp_irq:
-   gpio_free(OLPC_GPIO_DCON_STAT1);
-err_gp_stat1:
-   gpio_free(OLPC_GPIO_DCON_STAT0);
-   return -EIO;
 }
 
 static void dcon_wiggle_xo_1(void)
@@ -180,13 +170,13 @@ static void dcon_wiggle_xo_1(void)
 
 static void dcon_set_dconload_1(int val)
 {
-   gpio_set_value(OLPC_GPIO_DCON_LOAD, val);
+   gpiod_set_value(dcon_load, val);
 }
 
 static int dcon_read_status_xo_1(u8 *status)
 {
-   *status = gpio_get_value(OLPC_GPIO_DCON_STAT0);
-   *status |= gpio_get_value(OLPC_GPIO_DCON_STAT1) << 1;
+   *status = 

[PATCH] staging: olpc_dcon: olpc_dcon_xo_1.c: Switch to the gpio descriptor interface

[Nishad Kamdar]

Use the gpiod interface instead of the deprecated old non-descriptor
interface in olpc_dcon_xo_1.c.

Signed-off-by: Nishad Kamdar 
---
 drivers/staging/olpc_dcon/olpc_dcon.h  | 12 
 drivers/staging/olpc_dcon/olpc_dcon_xo_1.c | 74 ++
 2 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/olpc_dcon/olpc_dcon.h 
b/drivers/staging/olpc_dcon/olpc_dcon.h
index c987aaf894e7..6590d251eb6e 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon.h
+++ b/drivers/staging/olpc_dcon/olpc_dcon.h
@@ -57,6 +57,18 @@
 /* Interrupt */
 #define DCON_IRQ6
 
+struct dcon_gpio {
+   struct gpio_desc **ptr;
+   const char *name;
+   unsigned long flags;
+};
+
+struct gpio_desc *dcon_stat0;
+struct gpio_desc *dcon_stat1;
+struct gpio_desc *dcon_irq;
+struct gpio_desc *dcon_load;
+struct gpio_desc *dcon_blank;
+
 struct dcon_priv {
struct i2c_client *client;
struct fb_info *fbinfo;
diff --git a/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c 
b/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
index ff145d493e1b..83607efcb21f 100644
--- a/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
+++ b/drivers/staging/olpc_dcon/olpc_dcon_xo_1.c
@@ -11,7 +11,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include 
-#include 
+#include 
 #include 
 #include 
 
@@ -20,26 +20,29 @@
 static int dcon_init_xo_1(struct dcon_priv *dcon)
 {
unsigned char lob;
-
-   if (gpio_request(OLPC_GPIO_DCON_STAT0, "OLPC-DCON")) {
-   pr_err("failed to request STAT0 GPIO\n");
-   return -EIO;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_STAT1, "OLPC-DCON")) {
-   pr_err("failed to request STAT1 GPIO\n");
-   goto err_gp_stat1;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_IRQ, "OLPC-DCON")) {
-   pr_err("failed to request IRQ GPIO\n");
-   goto err_gp_irq;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_LOAD, "OLPC-DCON")) {
-   pr_err("failed to request LOAD GPIO\n");
-   goto err_gp_load;
-   }
-   if (gpio_request(OLPC_GPIO_DCON_BLANK, "OLPC-DCON")) {
-   pr_err("failed to request BLANK GPIO\n");
-   goto err_gp_blank;
+   int ret, i;
+   struct dcon_gpio *pin;
+   unsigned long flags = GPIOD_ASIS;
+
+   struct dcon_gpio gpios[] = {
+   { .ptr = _stat0, .name = "dcon_stat0", .flags = flags },
+   { .ptr = _stat1, .name = "dcon_stat1", .flags = flags },
+   { .ptr = _irq, .name = "dcon_irq", .flags = flags },
+   { .ptr = _load, .name = "dcon_load", .flags = flags },
+   { .ptr = _blank, .name = "dcon_blank", .flags = flags },
+   };
+
+   for (i = 0; i < ARRAY_SIZE(gpios); i++) {
+   pin = [i];
+   *pin->ptr = devm_gpiod_get(>bl_dev->dev, pin->name,
+  pin->flags);
+   if (IS_ERR(*pin->ptr)) {
+   ret = PTR_ERR(*pin->ptr);
+   dev_err(>bl_dev->dev,
+   "failed to request %s GPIO: %d\n",
+   pin->name, ret);
+   return ret;
+   }
}
 
/* Turn off the event enable for GPIO7 just to be safe */
@@ -61,12 +64,11 @@ static int dcon_init_xo_1(struct dcon_priv *dcon)
dcon->pending_src = dcon->curr_src;
 
/* Set the directions for the GPIO pins */
-   gpio_direction_input(OLPC_GPIO_DCON_STAT0);
-   gpio_direction_input(OLPC_GPIO_DCON_STAT1);
-   gpio_direction_input(OLPC_GPIO_DCON_IRQ);
-   gpio_direction_input(OLPC_GPIO_DCON_BLANK);
-   gpio_direction_output(OLPC_GPIO_DCON_LOAD,
- dcon->curr_src == DCON_SOURCE_CPU);
+   gpiod_direction_input(dcon_stat0);
+   gpiod_direction_input(dcon_stat1);
+   gpiod_direction_input(dcon_irq);
+   gpiod_direction_input(dcon_blank);
+   gpiod_direction_output(dcon_load, dcon->curr_src == DCON_SOURCE_CPU);
 
/* Set up the interrupt mappings */
 
@@ -125,18 +127,6 @@ static int dcon_init_xo_1(struct dcon_priv *dcon)
cs5535_gpio_set(OLPC_GPIO_DCON_BLANK, GPIO_EVENTS_ENABLE);
 
return 0;
-
-err_req_irq:
-   gpio_free(OLPC_GPIO_DCON_BLANK);
-err_gp_blank:
-   gpio_free(OLPC_GPIO_DCON_LOAD);
-err_gp_load:
-   gpio_free(OLPC_GPIO_DCON_IRQ);
-err_gp_irq:
-   gpio_free(OLPC_GPIO_DCON_STAT1);
-err_gp_stat1:
-   gpio_free(OLPC_GPIO_DCON_STAT0);
-   return -EIO;
 }
 
 static void dcon_wiggle_xo_1(void)
@@ -180,13 +170,13 @@ static void dcon_wiggle_xo_1(void)
 
 static void dcon_set_dconload_1(int val)
 {
-   gpio_set_value(OLPC_GPIO_DCON_LOAD, val);
+   gpiod_set_value(dcon_load, val);
 }
 
 static int dcon_read_status_xo_1(u8 *status)
 {
-   *status = gpio_get_value(OLPC_GPIO_DCON_STAT0);
-   *status |= gpio_get_value(OLPC_GPIO_DCON_STAT1) << 1;
+   *status = 

[RFC] doc: rcu: remove note on smp_mb during synchronize_rcu

[Joel Fernandes (Google)]

As per this thread [1], it seems this smp_mb isn't needed anymore:
"So the smp_mb() that I was trying to add doesn't need to be there."

So let us remove this part from the memory ordering documentation.

[1] https://lkml.org/lkml/2017/10/6/707

Signed-off-by: Joel Fernandes (Google) 
---
 .../Tree-RCU-Memory-Ordering.html | 32 +--
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git 
a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html 
b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
index a346ce0116eb..0fb1511763d4 100644
--- a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
+++ b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
@@ -77,7 +77,7 @@ The key point is that the lock-acquisition functions, 
including
 smp_mb__after_unlock_lock() immediately after successful
 acquisition of the lock.
 
-Therefore, for any given rcu_node struction, any access
+Therefore, for any given rcu_node structure, any access
 happening before one of the above lock-release functions will be seen
 by all CPUs as happening before any access happening after a later
 one of the above lock-acquisition functions.
@@ -162,36 +162,6 @@ an atomic_add_return() of zero) to detect idle 
CPUs.
 
 
 
-The approach must be extended to handle one final case, that
-of waking a task blocked in synchronize_rcu().
-This task might be affinitied to a CPU that is not yet aware that
-the grace period has ended, and thus might not yet be subject to
-the grace period's memory ordering.
-Therefore, there is an smp_mb() after the return from
-wait_for_completion() in the synchronize_rcu()
-code path.
-
-
-
-Quick Quiz:
-
-   What?  Where???
-   I don't see any smp_mb() after the return from
-   wait_for_completion()!!!
-
-Answer:
-
-   That would be because I spotted the need for that
-   smp_mb() during the creation of this documentation,
-   and it is therefore unlikely to hit mainline before v4.14.
-   Kudos to Lance Roy, Will Deacon, Peter Zijlstra, and
-   Jonathan Cameron for asking questions that sensitized me
-   to the rather elaborate sequence of events that demonstrate
-   the need for this memory barrier.
-
-
-
-
 Tree RCU's grace--period memory-ordering guarantees rely most
 heavily on the rcu_node structure's -lock
 field, so much so that it is necessary to abbreviate this pattern
-- 
2.19.1.568.g152ad8e336-goog

[RFC] doc: rcu: remove note on smp_mb during synchronize_rcu

[Joel Fernandes (Google)]

As per this thread [1], it seems this smp_mb isn't needed anymore:
"So the smp_mb() that I was trying to add doesn't need to be there."

So let us remove this part from the memory ordering documentation.

[1] https://lkml.org/lkml/2017/10/6/707

Signed-off-by: Joel Fernandes (Google) 
---
 .../Tree-RCU-Memory-Ordering.html | 32 +--
 1 file changed, 1 insertion(+), 31 deletions(-)

diff --git 
a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html 
b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
index a346ce0116eb..0fb1511763d4 100644
--- a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
+++ b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.html
@@ -77,7 +77,7 @@ The key point is that the lock-acquisition functions, 
including
 smp_mb__after_unlock_lock() immediately after successful
 acquisition of the lock.
 
-Therefore, for any given rcu_node struction, any access
+Therefore, for any given rcu_node structure, any access
 happening before one of the above lock-release functions will be seen
 by all CPUs as happening before any access happening after a later
 one of the above lock-acquisition functions.
@@ -162,36 +162,6 @@ an atomic_add_return() of zero) to detect idle 
CPUs.
 
 
 
-The approach must be extended to handle one final case, that
-of waking a task blocked in synchronize_rcu().
-This task might be affinitied to a CPU that is not yet aware that
-the grace period has ended, and thus might not yet be subject to
-the grace period's memory ordering.
-Therefore, there is an smp_mb() after the return from
-wait_for_completion() in the synchronize_rcu()
-code path.
-
-
-
-Quick Quiz:
-
-   What?  Where???
-   I don't see any smp_mb() after the return from
-   wait_for_completion()!!!
-
-Answer:
-
-   That would be because I spotted the need for that
-   smp_mb() during the creation of this documentation,
-   and it is therefore unlikely to hit mainline before v4.14.
-   Kudos to Lance Roy, Will Deacon, Peter Zijlstra, and
-   Jonathan Cameron for asking questions that sensitized me
-   to the rather elaborate sequence of events that demonstrate
-   the need for this memory barrier.
-
-
-
-
 Tree RCU's grace--period memory-ordering guarantees rely most
 heavily on the rcu_node structure's -lock
 field, so much so that it is necessary to abbreviate this pattern
-- 
2.19.1.568.g152ad8e336-goog

Re: [PATCH v3] libata: Apply NOLPM quirk for SAMSUNG MZ7TD256HAFV-000L9

[Diego Viola]

On Fri, Oct 26, 2018 at 5:36 PM Diego Viola  wrote:
>
> On Fri, Oct 26, 2018 at 11:21 AM Jens Axboe  wrote:
> >
> > On 10/26/18 7:45 AM, Diego Viola wrote:
> > > med_power_with_dipm causes my T450 to freeze with a SAMSUNG
> > > MZ7TD256HAFV-000L9 SSD (firmware DXT02L5Q).
> > >
> > > Switching the LPM to max_performance fixes this issue.
> >
> > Applied, thanks.
> >
> > --
> > Jens Axboe
> >
>
> Jens, Hans,
>
> Thank you.
>
> Diego

Hi Hans and Jens,

I just wanted to give you guys an update about this problem.

I've managed to update my SSD firmware to the latest version[1].

For running the update, I've had to install Windows 10, ran the
firmware update, remove Windows and reinstall Arch Linux.

The latest version of the firmware is DXT04L5Q, and it looks like
there hasn't been a new update since 2015.

I'll be running with med_power_with_dipm and hope this firmware update
fixes the problem, if it doesn't and I get another freeze, I'll send
another patch blacklisting the drive completely. Is that OK?

1. https://support.lenovo.com/br/en/downloads/ds038904

Thanks and Best Regards,
Diego

Re: [PATCH v3] libata: Apply NOLPM quirk for SAMSUNG MZ7TD256HAFV-000L9

[Diego Viola]

On Fri, Oct 26, 2018 at 5:36 PM Diego Viola  wrote:
>
> On Fri, Oct 26, 2018 at 11:21 AM Jens Axboe  wrote:
> >
> > On 10/26/18 7:45 AM, Diego Viola wrote:
> > > med_power_with_dipm causes my T450 to freeze with a SAMSUNG
> > > MZ7TD256HAFV-000L9 SSD (firmware DXT02L5Q).
> > >
> > > Switching the LPM to max_performance fixes this issue.
> >
> > Applied, thanks.
> >
> > --
> > Jens Axboe
> >
>
> Jens, Hans,
>
> Thank you.
>
> Diego

Hi Hans and Jens,

I just wanted to give you guys an update about this problem.

I've managed to update my SSD firmware to the latest version[1].

For running the update, I've had to install Windows 10, ran the
firmware update, remove Windows and reinstall Arch Linux.

The latest version of the firmware is DXT04L5Q, and it looks like
there hasn't been a new update since 2015.

I'll be running with med_power_with_dipm and hope this firmware update
fixes the problem, if it doesn't and I get another freeze, I'll send
another patch blacklisting the drive completely. Is that OK?

1. https://support.lenovo.com/br/en/downloads/ds038904

Thanks and Best Regards,
Diego

UI messages in event thread hangs perf top

[David Miller]

If I run perf top with a "make -j128" kernel build, I get ring buffer event
processing timeouts which results in:

ui__warning("Too slow to read ring buffer.\n"
"Please try increasing the period (-c) or\n"
"decreasing the freq (-F) or\n"
"limiting the number of CPUs (-C)\n");

from perf_top__mmap_read().

This hangs the main event thread.  Only the display thread runs after
this point.

We can't issue UI messages from the event thread, because those will
hang waiting for a keypress.  The display thread will eat any keys
we press and the event thread thus hangs forever.

I can tell this is what has happened because the histogram entries
continue to decay, yet the event count stops increasing.

If I put a gdb on the perf process, indeed the backtrace in the event
processing thread is in the select() call done by ui__getch().

Adding insult to injury, the display thread immediately overwrites the
warning message printed by the event thread, and thus the user has no
chance to even see it.

I really wonder how this was tested.

Perhaps we should mark the event thread in a special way and trigger
assertions if UI messages are printed from it.  Again, any such
operation will hang the thread and stop all event processing.

UI messages in event thread hangs perf top

[David Miller]

If I run perf top with a "make -j128" kernel build, I get ring buffer event
processing timeouts which results in:

ui__warning("Too slow to read ring buffer.\n"
"Please try increasing the period (-c) or\n"
"decreasing the freq (-F) or\n"
"limiting the number of CPUs (-C)\n");

from perf_top__mmap_read().

This hangs the main event thread.  Only the display thread runs after
this point.

We can't issue UI messages from the event thread, because those will
hang waiting for a keypress.  The display thread will eat any keys
we press and the event thread thus hangs forever.

I can tell this is what has happened because the histogram entries
continue to decay, yet the event count stops increasing.

If I put a gdb on the perf process, indeed the backtrace in the event
processing thread is in the select() call done by ui__getch().

Adding insult to injury, the display thread immediately overwrites the
warning message printed by the event thread, and thus the user has no
chance to even see it.

I really wonder how this was tested.

Perhaps we should mark the event thread in a special way and trigger
assertions if UI messages are printed from it.  Again, any such
operation will hang the thread and stop all event processing.

[PATCH] staging: rtl8712: fix CamelCase in fw_priv struct

[Robert Eshleman]

Rename fields in fw_priv struct from CamelCase to snake_case.
Reported by checkpatch.

Signed-off-by: Robert Eshleman 
---
 drivers/staging/rtl8712/hal_init.c| 10 +-
 drivers/staging/rtl8712/rtl8712_hal.h |  8 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8712/hal_init.c 
b/drivers/staging/rtl8712/hal_init.c
index 7cdd609..4c6519c 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -100,11 +100,11 @@ static void fill_fwpriv(struct _adapter *padapter, struct 
fw_priv *pfwpriv)
pfwpriv->rf_config = RTL8712_RFC_1T2R;
}
pfwpriv->mp_mode = (pregpriv->mp_mode == 1) ? 1 : 0;
-   pfwpriv->vcsType = pregpriv->vrtl_carrier_sense; /* 0:off 1:on 2:auto */
-   pfwpriv->vcsMode = pregpriv->vcs_type; /* 1:RTS/CTS 2:CTS to self */
-   /* default enable turboMode */
-   pfwpriv->turboMode = ((pregpriv->wifi_test == 1) ? 0 : 1);
-   pfwpriv->lowPowerMode = pregpriv->low_power;
+   pfwpriv->vcs_type = pregpriv->vrtl_carrier_sense; /* 0:off 1:on 2:auto 
*/
+   pfwpriv->vcs_mode = pregpriv->vcs_type; /* 1:RTS/CTS 2:CTS to self */
+   /* default enable turbo_mode */
+   pfwpriv->turbo_mode = ((pregpriv->wifi_test == 1) ? 0 : 1);
+   pfwpriv->low_power_mode = pregpriv->low_power;
 }
 
 static void update_fwhdr(struct fw_hdr *pfwhdr, const u8 *pmappedfw)
diff --git a/drivers/staging/rtl8712/rtl8712_hal.h 
b/drivers/staging/rtl8712/rtl8712_hal.h
index 42f5197..66cc464 100644
--- a/drivers/staging/rtl8712/rtl8712_hal.h
+++ b/drivers/staging/rtl8712/rtl8712_hal.h
@@ -72,13 +72,13 @@ struct fw_priv {   /*8-bytes alignment required*/
unsigned char regulatory_class_3; /*regulatory class bit map 3*/
unsigned char rfintfs;/* 0:SWSI, 1:HWSI, 2:HWPI*/
unsigned char def_nettype;
-   unsigned char turboMode;
-   unsigned char lowPowerMode;/* 0: normal mode, 1: low power mode*/
+   unsigned char turbo_mode;
+   unsigned char low_power_mode;/* 0: normal mode, 1: low power mode*/
/*--- long word 2 */
unsigned char lbk_mode; /*0x00: normal, 0x03: MACLBK, 0x01: PHYLBK*/
unsigned char mp_mode; /* 1: for MP use, 0: for normal driver */
-   unsigned char vcsType; /* 0:off 1:on 2:auto */
-   unsigned char vcsMode; /* 1:RTS/CTS 2:CTS to self */
+   unsigned char vcs_type; /* 0:off 1:on 2:auto */
+   unsigned char vcs_mode; /* 1:RTS/CTS 2:CTS to self */
unsigned char rsvd022;
unsigned char rsvd023;
unsigned char rsvd024;
-- 
2.7.4

[PATCH] staging: rtl8712: fix CamelCase in fw_priv struct

[Robert Eshleman]

Rename fields in fw_priv struct from CamelCase to snake_case.
Reported by checkpatch.

Signed-off-by: Robert Eshleman 
---
 drivers/staging/rtl8712/hal_init.c| 10 +-
 drivers/staging/rtl8712/rtl8712_hal.h |  8 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/rtl8712/hal_init.c 
b/drivers/staging/rtl8712/hal_init.c
index 7cdd609..4c6519c 100644
--- a/drivers/staging/rtl8712/hal_init.c
+++ b/drivers/staging/rtl8712/hal_init.c
@@ -100,11 +100,11 @@ static void fill_fwpriv(struct _adapter *padapter, struct 
fw_priv *pfwpriv)
pfwpriv->rf_config = RTL8712_RFC_1T2R;
}
pfwpriv->mp_mode = (pregpriv->mp_mode == 1) ? 1 : 0;
-   pfwpriv->vcsType = pregpriv->vrtl_carrier_sense; /* 0:off 1:on 2:auto */
-   pfwpriv->vcsMode = pregpriv->vcs_type; /* 1:RTS/CTS 2:CTS to self */
-   /* default enable turboMode */
-   pfwpriv->turboMode = ((pregpriv->wifi_test == 1) ? 0 : 1);
-   pfwpriv->lowPowerMode = pregpriv->low_power;
+   pfwpriv->vcs_type = pregpriv->vrtl_carrier_sense; /* 0:off 1:on 2:auto 
*/
+   pfwpriv->vcs_mode = pregpriv->vcs_type; /* 1:RTS/CTS 2:CTS to self */
+   /* default enable turbo_mode */
+   pfwpriv->turbo_mode = ((pregpriv->wifi_test == 1) ? 0 : 1);
+   pfwpriv->low_power_mode = pregpriv->low_power;
 }
 
 static void update_fwhdr(struct fw_hdr *pfwhdr, const u8 *pmappedfw)
diff --git a/drivers/staging/rtl8712/rtl8712_hal.h 
b/drivers/staging/rtl8712/rtl8712_hal.h
index 42f5197..66cc464 100644
--- a/drivers/staging/rtl8712/rtl8712_hal.h
+++ b/drivers/staging/rtl8712/rtl8712_hal.h
@@ -72,13 +72,13 @@ struct fw_priv {   /*8-bytes alignment required*/
unsigned char regulatory_class_3; /*regulatory class bit map 3*/
unsigned char rfintfs;/* 0:SWSI, 1:HWSI, 2:HWPI*/
unsigned char def_nettype;
-   unsigned char turboMode;
-   unsigned char lowPowerMode;/* 0: normal mode, 1: low power mode*/
+   unsigned char turbo_mode;
+   unsigned char low_power_mode;/* 0: normal mode, 1: low power mode*/
/*--- long word 2 */
unsigned char lbk_mode; /*0x00: normal, 0x03: MACLBK, 0x01: PHYLBK*/
unsigned char mp_mode; /* 1: for MP use, 0: for normal driver */
-   unsigned char vcsType; /* 0:off 1:on 2:auto */
-   unsigned char vcsMode; /* 1:RTS/CTS 2:CTS to self */
+   unsigned char vcs_type; /* 0:off 1:on 2:auto */
+   unsigned char vcs_mode; /* 1:RTS/CTS 2:CTS to self */
unsigned char rsvd022;
unsigned char rsvd023;
unsigned char rsvd024;
-- 
2.7.4

Re: [PATCH v2 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization

[Masahiro Yamada]

On Fri, Oct 19, 2018 at 9:50 PM Du Changbin  wrote:
>
> This will apply GCC '-Og' optimization level which is supported
> since GCC 4.8. This optimization level offers a reasonable level
> of optimization while maintaining fast compilation and a good
> debugging experience. It is similar to '-O1' while perferring
> to keep debug ability over runtime speed.
>
> If enabling this option breaks your kernel, you should either
> disable this or find a fix (mostly in the arch code). Currently
> this option has only been tested on x86_64 and arm platform.
>
> This option can satisfy people who was searching for a method
> to disable compiler optimizations so to achieve better kernel
> debugging experience with kgdb or qemu.
>
> The main problem of '-Og' is we must not use __attribute__((error(msg))).
> The compiler will report error though the call to error function
> still can be optimize out. So we must fallback to array tricky.
>
> Comparison of vmlinux size: a bit smaller.
>
> w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ size vmlinux
>textdata bss dec hex filename
> 22665554   9709674  2920908 3529613621a9388 vmlinux
>
> w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ size vmlinux
>textdata bss dec hex filename
> 21499032   10102758 2920908 3452269820ec64a vmlinux
>
> Comparison of system performance: a bit drop (~6%).
> This benchmark of kernel compilation is suggested by Ingo Molnar.
> https://lkml.org/lkml/2018/5/2/74
>
> Preparation: Set cpufreq to 'performance'.
> for ((cpu=0; cpu<120; cpu++)); do
>   G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
>   [ -f $G ] && echo performance > $G
> done
>
> w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ perf stat --repeat 5 --null --pre '\
> cp -a kernel ../kernel.copy.$(date +%s); \
> rm -rf *;\
> git checkout .;  \
> echo 1 > /proc/sys/vm/drop_caches;   \
> find ../kernel* -type f | xargs cat >/dev/null;  \
> make -j kernel >/dev/null;   \
> make clean >/dev/null 2>&1;  \
> sync'\
>  \
> make -j8 >/dev/null
>
> Performance counter stats for 'make -j8' (5 runs):
>
> 219.764246652 seconds time elapsed   ( +-  0.78% )
>
> w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ perf stat --repeat 5 --null --pre '\
> cp -a kernel ../kernel.copy.$(date +%s); \
> rm -rf *;\
> git checkout .;  \
> echo 1 > /proc/sys/vm/drop_caches;   \
> find ../kernel* -type f | xargs cat >/dev/null;  \
> make -j kernel >/dev/null;   \
> make clean >/dev/null 2>&1;  \
> sync'\
>  \
> make -j8 >/dev/null
>
> Performance counter stats for 'make -j8' (5 runs):
>
>  233.574187771 seconds time elapsed  ( +-  0.19% )
>
> Signed-off-by: Du Changbin 
> Acked-by: Steven Rostedt (VMware) 
> ---
>  Makefile |  5 +
>  include/linux/compiler-gcc.h |  2 +-
>  include/linux/compiler.h |  2 +-
>  init/Kconfig | 19 +++
>  4 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 757d6507cb5c..ea908cfe8594 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -657,6 +657,10 @@ KBUILD_CFLAGS  += $(call cc-disable-warning, 
> format-truncation)
>  KBUILD_CFLAGS  += $(call cc-disable-warning, format-overflow)
>  KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)
>
> +ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> +KBUILD_CFLAGS  += $(call cc-option, -Og)


This line is a problem.

As you stated in your patch description, -Og is supported on GCC 4.8+
but the minimum GCC version supported for Linux kernel is GCC 4.6


If you use either GCC 4.6 or 4.7,
$(call cc-option, -Og) is evaluated to empty.

It means no optimization flag is given to the compiler, then
the build fails.

I'd like to suggest to change as follows:




diff --git a/Makefile b/Makefile
index 122ab74..9af3dc4 100644
--- a/Makefile
+++ b/Makefile
@@ -663,7 +663,7 @@ KBUILD_CFLAGS   += $(call cc-disable-warning,
format-overflow)
 KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)

 ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
-KBUILD_CFLAGS  += $(call cc-option, -Og)
+KBUILD_CFLAGS  += -Og
 KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
 else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
diff --git a/init/Kconfig b/init/Kconfig
index 

Re: [PATCH v2 4/4] kernel hacking: new config CC_OPTIMIZE_FOR_DEBUGGING to apply GCC -Og optimization

[Masahiro Yamada]

On Fri, Oct 19, 2018 at 9:50 PM Du Changbin  wrote:
>
> This will apply GCC '-Og' optimization level which is supported
> since GCC 4.8. This optimization level offers a reasonable level
> of optimization while maintaining fast compilation and a good
> debugging experience. It is similar to '-O1' while perferring
> to keep debug ability over runtime speed.
>
> If enabling this option breaks your kernel, you should either
> disable this or find a fix (mostly in the arch code). Currently
> this option has only been tested on x86_64 and arm platform.
>
> This option can satisfy people who was searching for a method
> to disable compiler optimizations so to achieve better kernel
> debugging experience with kgdb or qemu.
>
> The main problem of '-Og' is we must not use __attribute__((error(msg))).
> The compiler will report error though the call to error function
> still can be optimize out. So we must fallback to array tricky.
>
> Comparison of vmlinux size: a bit smaller.
>
> w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ size vmlinux
>textdata bss dec hex filename
> 22665554   9709674  2920908 3529613621a9388 vmlinux
>
> w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ size vmlinux
>textdata bss dec hex filename
> 21499032   10102758 2920908 3452269820ec64a vmlinux
>
> Comparison of system performance: a bit drop (~6%).
> This benchmark of kernel compilation is suggested by Ingo Molnar.
> https://lkml.org/lkml/2018/5/2/74
>
> Preparation: Set cpufreq to 'performance'.
> for ((cpu=0; cpu<120; cpu++)); do
>   G=/sys/devices/system/cpu/cpu$cpu/cpufreq/scaling_governor
>   [ -f $G ] && echo performance > $G
> done
>
> w/o CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ perf stat --repeat 5 --null --pre '\
> cp -a kernel ../kernel.copy.$(date +%s); \
> rm -rf *;\
> git checkout .;  \
> echo 1 > /proc/sys/vm/drop_caches;   \
> find ../kernel* -type f | xargs cat >/dev/null;  \
> make -j kernel >/dev/null;   \
> make clean >/dev/null 2>&1;  \
> sync'\
>  \
> make -j8 >/dev/null
>
> Performance counter stats for 'make -j8' (5 runs):
>
> 219.764246652 seconds time elapsed   ( +-  0.78% )
>
> w/ CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> $ perf stat --repeat 5 --null --pre '\
> cp -a kernel ../kernel.copy.$(date +%s); \
> rm -rf *;\
> git checkout .;  \
> echo 1 > /proc/sys/vm/drop_caches;   \
> find ../kernel* -type f | xargs cat >/dev/null;  \
> make -j kernel >/dev/null;   \
> make clean >/dev/null 2>&1;  \
> sync'\
>  \
> make -j8 >/dev/null
>
> Performance counter stats for 'make -j8' (5 runs):
>
>  233.574187771 seconds time elapsed  ( +-  0.19% )
>
> Signed-off-by: Du Changbin 
> Acked-by: Steven Rostedt (VMware) 
> ---
>  Makefile |  5 +
>  include/linux/compiler-gcc.h |  2 +-
>  include/linux/compiler.h |  2 +-
>  init/Kconfig | 19 +++
>  4 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 757d6507cb5c..ea908cfe8594 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -657,6 +657,10 @@ KBUILD_CFLAGS  += $(call cc-disable-warning, 
> format-truncation)
>  KBUILD_CFLAGS  += $(call cc-disable-warning, format-overflow)
>  KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)
>
> +ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
> +KBUILD_CFLAGS  += $(call cc-option, -Og)


This line is a problem.

As you stated in your patch description, -Og is supported on GCC 4.8+
but the minimum GCC version supported for Linux kernel is GCC 4.6


If you use either GCC 4.6 or 4.7,
$(call cc-option, -Og) is evaluated to empty.

It means no optimization flag is given to the compiler, then
the build fails.

I'd like to suggest to change as follows:




diff --git a/Makefile b/Makefile
index 122ab74..9af3dc4 100644
--- a/Makefile
+++ b/Makefile
@@ -663,7 +663,7 @@ KBUILD_CFLAGS   += $(call cc-disable-warning,
format-overflow)
 KBUILD_CFLAGS  += $(call cc-disable-warning, int-in-bool-context)

 ifdef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
-KBUILD_CFLAGS  += $(call cc-option, -Og)
+KBUILD_CFLAGS  += -Og
 KBUILD_CFLAGS  += $(call cc-disable-warning,maybe-uninitialized,)
 else
 ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
diff --git a/init/Kconfig b/init/Kconfig
index 

[RFC] rcu: doc: update example about stale data

[Joel Fernandes (Google)]

The RCU example for 'rejecting stale data' on system-call auditting
stops iterating through the rules if a deleted one is found. It makes
more sense to continue looking at other rules once a deleted one is
rejected. Although the original example is fine, this makes it more
meaningful.

Signed-off-by: Joel Fernandes (Google) 
---
 Documentation/RCU/listRCU.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/RCU/listRCU.txt b/Documentation/RCU/listRCU.txt
index adb5a3782846..09e9a4fc723e 100644
--- a/Documentation/RCU/listRCU.txt
+++ b/Documentation/RCU/listRCU.txt
@@ -250,8 +250,7 @@ as follows:
spin_lock(>lock);
if (e->deleted) {
spin_unlock(>lock);
-   rcu_read_unlock();
-   return AUDIT_BUILD_CONTEXT;
+   continue;
}
rcu_read_unlock();
return state;
-- 
2.19.1.568.g152ad8e336-goog

[RFC] rcu: doc: update example about stale data

[Joel Fernandes (Google)]

The RCU example for 'rejecting stale data' on system-call auditting
stops iterating through the rules if a deleted one is found. It makes
more sense to continue looking at other rules once a deleted one is
rejected. Although the original example is fine, this makes it more
meaningful.

Signed-off-by: Joel Fernandes (Google) 
---
 Documentation/RCU/listRCU.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/RCU/listRCU.txt b/Documentation/RCU/listRCU.txt
index adb5a3782846..09e9a4fc723e 100644
--- a/Documentation/RCU/listRCU.txt
+++ b/Documentation/RCU/listRCU.txt
@@ -250,8 +250,7 @@ as follows:
spin_lock(>lock);
if (e->deleted) {
spin_unlock(>lock);
-   rcu_read_unlock();
-   return AUDIT_BUILD_CONTEXT;
+   continue;
}
rcu_read_unlock();
return state;
-- 
2.19.1.568.g152ad8e336-goog

Re: [PATCH v2 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif

[Masahiro Yamada]

On Fri, Oct 19, 2018 at 9:50 PM Du Changbin  wrote:
>
> The level4_kernel_pgt is only defined when X86_5LEVEL is enabled. So
> surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif to
> make code correct.

For clarification, is it better to mention
that this is a preparation for CONFIG_CC_OPTIMIZE_FOR_DEBUGGING ?


> Signed-off-by: Du Changbin 
> Acked-by: Steven Rostedt (VMware) 
> ---
>  arch/x86/include/asm/pgtable_64.h |  2 ++
>  arch/x86/kernel/head64.c  | 13 ++---
>  2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/pgtable_64.h 
> b/arch/x86/include/asm/pgtable_64.h
> index 9c85b54bf03c..9333f7fa5bdb 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -16,7 +16,9 @@
>  #include 
>  #include 
>
> +#ifdef CONFIG_X86_5LEVEL
>  extern p4d_t level4_kernel_pgt[512];
> +#endif


Is this #ifdef  necessary?

It is harmless to declaring unused stuff.



>  extern p4d_t level4_ident_pgt[512];
>  extern pud_t level3_kernel_pgt[512];
>  extern pud_t level3_ident_pgt[512];
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index ddee1f0870c4..4a59ef93c258 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -151,16 +151,15 @@ unsigned long __head __startup_64(unsigned long 
> physaddr,
>
> pgd = fixup_pointer(_top_pgt, physaddr);
> p = pgd + pgd_index(__START_KERNEL_map);
> -   if (la57)
> -   *p = (unsigned long)level4_kernel_pgt;
> -   else
> -   *p = (unsigned long)level3_kernel_pgt;
> -   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
> -
> +#ifdef CONFIG_X86_5LEVEL
> if (la57) {
> +   *p = (unsigned long)level4_kernel_pgt;
> p4d = fixup_pointer(_kernel_pgt, physaddr);
> p4d[511] += load_delta;
> -   }
> +   } else
> +#endif
> +   *p = (unsigned long)level3_kernel_pgt;
> +   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
>
> pud = fixup_pointer(_kernel_pgt, physaddr);
> pud[510] += load_delta;
> --
> 2.17.1
>


Hmm, this code looks a bit ugly...

Does the following one liner work with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING ?


diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 8047379..579847f 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -97,7 +97,7 @@ static bool __head check_la57_support(unsigned long physaddr)
return true;
 }
 #else
-static bool __head check_la57_support(unsigned long physaddr)
+static __always_inline bool __head check_la57_support(unsigned long physaddr)
 {
return false;
 }




--
Best Regards
Masahiro Yamada

Re: [PATCH v2 1/4] x86/mm: surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif

[Masahiro Yamada]

On Fri, Oct 19, 2018 at 9:50 PM Du Changbin  wrote:
>
> The level4_kernel_pgt is only defined when X86_5LEVEL is enabled. So
> surround level4_kernel_pgt with #ifdef CONFIG_X86_5LEVEL...#endif to
> make code correct.

For clarification, is it better to mention
that this is a preparation for CONFIG_CC_OPTIMIZE_FOR_DEBUGGING ?


> Signed-off-by: Du Changbin 
> Acked-by: Steven Rostedt (VMware) 
> ---
>  arch/x86/include/asm/pgtable_64.h |  2 ++
>  arch/x86/kernel/head64.c  | 13 ++---
>  2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/include/asm/pgtable_64.h 
> b/arch/x86/include/asm/pgtable_64.h
> index 9c85b54bf03c..9333f7fa5bdb 100644
> --- a/arch/x86/include/asm/pgtable_64.h
> +++ b/arch/x86/include/asm/pgtable_64.h
> @@ -16,7 +16,9 @@
>  #include 
>  #include 
>
> +#ifdef CONFIG_X86_5LEVEL
>  extern p4d_t level4_kernel_pgt[512];
> +#endif


Is this #ifdef  necessary?

It is harmless to declaring unused stuff.



>  extern p4d_t level4_ident_pgt[512];
>  extern pud_t level3_kernel_pgt[512];
>  extern pud_t level3_ident_pgt[512];
> diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
> index ddee1f0870c4..4a59ef93c258 100644
> --- a/arch/x86/kernel/head64.c
> +++ b/arch/x86/kernel/head64.c
> @@ -151,16 +151,15 @@ unsigned long __head __startup_64(unsigned long 
> physaddr,
>
> pgd = fixup_pointer(_top_pgt, physaddr);
> p = pgd + pgd_index(__START_KERNEL_map);
> -   if (la57)
> -   *p = (unsigned long)level4_kernel_pgt;
> -   else
> -   *p = (unsigned long)level3_kernel_pgt;
> -   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
> -
> +#ifdef CONFIG_X86_5LEVEL
> if (la57) {
> +   *p = (unsigned long)level4_kernel_pgt;
> p4d = fixup_pointer(_kernel_pgt, physaddr);
> p4d[511] += load_delta;
> -   }
> +   } else
> +#endif
> +   *p = (unsigned long)level3_kernel_pgt;
> +   *p += _PAGE_TABLE_NOENC - __START_KERNEL_map + load_delta;
>
> pud = fixup_pointer(_kernel_pgt, physaddr);
> pud[510] += load_delta;
> --
> 2.17.1
>


Hmm, this code looks a bit ugly...

Does the following one liner work with CONFIG_CC_OPTIMIZE_FOR_DEBUGGING ?


diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 8047379..579847f 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -97,7 +97,7 @@ static bool __head check_la57_support(unsigned long physaddr)
return true;
 }
 #else
-static bool __head check_la57_support(unsigned long physaddr)
+static __always_inline bool __head check_la57_support(unsigned long physaddr)
 {
return false;
 }




--
Best Regards
Masahiro Yamada

Re: [PATCH] HID: moving to group maintainership model

[Stephen Rothwell]

Hi Jiri,

On Sat, 27 Oct 2018 14:19:02 +0200 (CEST) Jiri Kosina  wrote:
>
> Stephen, could you please update the hid.git URL for linux-next as below? 
> Thanks.

Done, also added Benjamin as a contact.

[for Benjamin's benefit ...]

Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgement of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window. 

You will need to ensure that the patches/commits in your tree/series have
been:
 * submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
 * posted to the relevant mailing list,
 * reviewed by you (or another maintainer of your subsystem tree),
 * successfully unit tested, and 
 * destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.

-- 
Cheers,
Stephen Rothwell 
s...@canb.auug.org.au


pgpih4Wh1nF9g.pgp
Description: OpenPGP digital signature

Re: [PATCH] HID: moving to group maintainership model

[Stephen Rothwell]

Hi Jiri,

On Sat, 27 Oct 2018 14:19:02 +0200 (CEST) Jiri Kosina  wrote:
>
> Stephen, could you please update the hid.git URL for linux-next as below? 
> Thanks.

Done, also added Benjamin as a contact.

[for Benjamin's benefit ...]

Thanks for adding your subsystem tree as a participant of linux-next.  As
you may know, this is not a judgement of your code.  The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window. 

You will need to ensure that the patches/commits in your tree/series have
been:
 * submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
 * posted to the relevant mailing list,
 * reviewed by you (or another maintainer of your subsystem tree),
 * successfully unit tested, and 
 * destined for the current or next Linux merge window.

Basically, this should be just what you would send to Linus (or ask him
to fetch).  It is allowed to be rebased if you deem it necessary.

-- 
Cheers,
Stephen Rothwell 
s...@canb.auug.org.au


pgpih4Wh1nF9g.pgp
Description: OpenPGP digital signature

Re: [Ksummit-discuss] The linux devs can rescind their license grant.

[Al Viro]

On Sat, Oct 27, 2018 at 03:46:02PM -0700, Bruce Perens wrote:
> The anonymous person is generally thought to have appeared on the net
> previously as MikeeUSA. That entity has a well-recorded history of misogyny
> and other anti-social behaviour.

You are misreading it - behaviour of that...  member of our biological species
is entirely controlled by its desperate need to be noticed, no matter what.
Pathetic, but that's the social shmedia generation for you...  I wouldn't be
surprised if s/h/it maintains a sock puppet or two in the other bunch of
PETA-level wankers^W^W^W^Wculture warriors, BTW.

Re: [Ksummit-discuss] The linux devs can rescind their license grant.

[Al Viro]

On Sat, Oct 27, 2018 at 03:46:02PM -0700, Bruce Perens wrote:
> The anonymous person is generally thought to have appeared on the net
> previously as MikeeUSA. That entity has a well-recorded history of misogyny
> and other anti-social behaviour.

You are misreading it - behaviour of that...  member of our biological species
is entirely controlled by its desperate need to be noticed, no matter what.
Pathetic, but that's the social shmedia generation for you...  I wouldn't be
surprised if s/h/it maintains a sock puppet or two in the other bunch of
PETA-level wankers^W^W^W^Wculture warriors, BTW.

Group Imbalance Bug - performance drop by factor 10x on NUMA boxes with cgroups

[Jirka Hladky]

Hi Mel and Srikar,

I would like to ask you if you could look into the Group Imbalance Bug
described in this paper

http://www.ece.ubc.ca/~sasha/papers/eurosys16-final29.pdf

in chapter 3.1. See also comment [1]. The paper describes the bug on
workload which involves different ssh sessions and it assumes
kernel.sched_autogroup_enabled=1. We have found out that it can be
reproduced more easily with cgroups.

Reproducer consists of this workload
* 2 separate "stress --cpu 1" processes. Each stress process needs 1 CPU.
* NAS benchmark (https://www.nas.nasa.gov/publications/npb.html) from
which I use lu.C.x binary (Lower-Upper Gauss-Seidel solver) in the
Open Multi-Processing (OMP) mode.

We run the workload in two modes:

NORMAL - both stress and lu.C.x are run in the same control group
GROUP  - each binary is run in a separate control group:
stress, first instance: cpu:test_group_1
stress, seconds instance: cpu:test_group_2
lu.C.x : cpu:test_group_main

I run lu.C.x with a different number of threads - for example on 4
NUMA server with 4x Xeon Gold 6126 CPU (total 96 CPUs) - I run lu.C.x
with 72, 80, 88, and 92 threads. Since the server has 96 CPUs in
total, even with 92 threads for lu.C.x and two stress processes server
is still not fully loaded.

Here are the runtimes in seconds for lu.C.x for different number of threads

#Threads  NORMAL GROUP
72 21.2730.01
80 15.32 164
88 17.91 367
92 19.22 432

As you can see, already for 72 threads lu.C.x is significantly slower
when executed in dedicated cgroup. And it gets much worse with an
increasing number of threads (slowdown by the factor 10x and greater).

Some more details are below.

Please let me know if it sounds interesting and if you would like to
look into it. I can provide you with the reproducer plus some
supplementary python scripts to further analyze the results.

Thanks a lot!
Jirka

Some more details on the case with 80 threads for lu.C.x, 2 stress
processes run  on 96 CPUs server with 4 NUMA nodes.

Analyzing ps output is very interesting (here for 5 subsequent runs of
the workload):

Average number of threads scheduled for NUMA NODE  0  1  2  3

lu.C.x_80_NORMAL_1.ps.numa.hist Average21.25  21.00  19.75  18.00
lu.C.x_80_NORMAL_1.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_NORMAL_2.ps.numa.hist Average20.50  20.75  18.00  20.75
lu.C.x_80_NORMAL_2.stress.ps.numa.hist  Average1.00  0.75  0.25
lu.C.x_80_NORMAL_3.ps.numa.hist Average21.75  22.00  18.75  17.50
lu.C.x_80_NORMAL_3.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_NORMAL_4.ps.numa.hist Average21.50  21.00  18.75  18.75
lu.C.x_80_NORMAL_4.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_NORMAL_5.ps.numa.hist Average18.00  23.33  19.33  19.33
lu.C.x_80_NORMAL_5.stress.ps.numa.hist  Average1.00  1.00


As you can see, in NORMAL mode lu.C.x is uniformly scheduled over NUMA nodes.

Compare it with cgroups mode:

Average number of threads scheduled for NUMA NODE  0  1  2  3

lu.C.x_80_GROUP_1.ps.numa.hist Average13.05  13.54  27.65  25.76
lu.C.x_80_GROUP_1.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_2.ps.numa.hist Average12.18  14.85  27.56  25.41
lu.C.x_80_GROUP_2.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_3.ps.numa.hist Average15.32  13.23  26.52  24.94
lu.C.x_80_GROUP_3.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_4.ps.numa.hist Average13.82  14.86  25.64  25.68
lu.C.x_80_GROUP_4.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_5.ps.numa.hist Average15.12  13.03  25.12  26.73
lu.C.x_80_GROUP_5.stress.ps.numa.hist  Average1.00  1.00

In cgroup mode, the scheduler is moving lu.C.x away from the nodes #0
and #1 where stress processes are running. It does it to such extent
that NUMA nodes #2 and #3 are overcommitted - these NUMA nodes have
more NAS threads scheduled than CPUs available - there are 24 CPUs in
each NUMA node.

Here is the detailed report:
$more lu.C.x_80_GROUP_1.ps.numa.hist
#Date   NUMA 0  NUMA 1  NUMA 2  NUMA 3
2018-Oct-27_04h39m57s6   7   37  30
2018-Oct-27_04h40m02s16  15  23  26
2018-Oct-27_04h40m08s13  12  27  28
2018-Oct-27_04h40m13s9   15  29  27
2018-Oct-27_04h40m18s16  13  27  24
2018-Oct-27_04h40m23s16  14  25  25
2018-Oct-27_04h40m28s16  15  24  25
2018-Oct-27_04h40m33s10  11  34  25
2018-Oct-27_04h40m38s16  13  25  26
2018-Oct-27_04h40m43s10  10  32  28

Group Imbalance Bug - performance drop by factor 10x on NUMA boxes with cgroups

[Jirka Hladky]

Hi Mel and Srikar,

I would like to ask you if you could look into the Group Imbalance Bug
described in this paper

http://www.ece.ubc.ca/~sasha/papers/eurosys16-final29.pdf

in chapter 3.1. See also comment [1]. The paper describes the bug on
workload which involves different ssh sessions and it assumes
kernel.sched_autogroup_enabled=1. We have found out that it can be
reproduced more easily with cgroups.

Reproducer consists of this workload
* 2 separate "stress --cpu 1" processes. Each stress process needs 1 CPU.
* NAS benchmark (https://www.nas.nasa.gov/publications/npb.html) from
which I use lu.C.x binary (Lower-Upper Gauss-Seidel solver) in the
Open Multi-Processing (OMP) mode.

We run the workload in two modes:

NORMAL - both stress and lu.C.x are run in the same control group
GROUP  - each binary is run in a separate control group:
stress, first instance: cpu:test_group_1
stress, seconds instance: cpu:test_group_2
lu.C.x : cpu:test_group_main

I run lu.C.x with a different number of threads - for example on 4
NUMA server with 4x Xeon Gold 6126 CPU (total 96 CPUs) - I run lu.C.x
with 72, 80, 88, and 92 threads. Since the server has 96 CPUs in
total, even with 92 threads for lu.C.x and two stress processes server
is still not fully loaded.

Here are the runtimes in seconds for lu.C.x for different number of threads

#Threads  NORMAL GROUP
72 21.2730.01
80 15.32 164
88 17.91 367
92 19.22 432

As you can see, already for 72 threads lu.C.x is significantly slower
when executed in dedicated cgroup. And it gets much worse with an
increasing number of threads (slowdown by the factor 10x and greater).

Some more details are below.

Please let me know if it sounds interesting and if you would like to
look into it. I can provide you with the reproducer plus some
supplementary python scripts to further analyze the results.

Thanks a lot!
Jirka

Some more details on the case with 80 threads for lu.C.x, 2 stress
processes run  on 96 CPUs server with 4 NUMA nodes.

Analyzing ps output is very interesting (here for 5 subsequent runs of
the workload):

Average number of threads scheduled for NUMA NODE  0  1  2  3

lu.C.x_80_NORMAL_1.ps.numa.hist Average21.25  21.00  19.75  18.00
lu.C.x_80_NORMAL_1.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_NORMAL_2.ps.numa.hist Average20.50  20.75  18.00  20.75
lu.C.x_80_NORMAL_2.stress.ps.numa.hist  Average1.00  0.75  0.25
lu.C.x_80_NORMAL_3.ps.numa.hist Average21.75  22.00  18.75  17.50
lu.C.x_80_NORMAL_3.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_NORMAL_4.ps.numa.hist Average21.50  21.00  18.75  18.75
lu.C.x_80_NORMAL_4.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_NORMAL_5.ps.numa.hist Average18.00  23.33  19.33  19.33
lu.C.x_80_NORMAL_5.stress.ps.numa.hist  Average1.00  1.00


As you can see, in NORMAL mode lu.C.x is uniformly scheduled over NUMA nodes.

Compare it with cgroups mode:

Average number of threads scheduled for NUMA NODE  0  1  2  3

lu.C.x_80_GROUP_1.ps.numa.hist Average13.05  13.54  27.65  25.76
lu.C.x_80_GROUP_1.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_2.ps.numa.hist Average12.18  14.85  27.56  25.41
lu.C.x_80_GROUP_2.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_3.ps.numa.hist Average15.32  13.23  26.52  24.94
lu.C.x_80_GROUP_3.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_4.ps.numa.hist Average13.82  14.86  25.64  25.68
lu.C.x_80_GROUP_4.stress.ps.numa.hist  Average1.00  1.00
lu.C.x_80_GROUP_5.ps.numa.hist Average15.12  13.03  25.12  26.73
lu.C.x_80_GROUP_5.stress.ps.numa.hist  Average1.00  1.00

In cgroup mode, the scheduler is moving lu.C.x away from the nodes #0
and #1 where stress processes are running. It does it to such extent
that NUMA nodes #2 and #3 are overcommitted - these NUMA nodes have
more NAS threads scheduled than CPUs available - there are 24 CPUs in
each NUMA node.

Here is the detailed report:
$more lu.C.x_80_GROUP_1.ps.numa.hist
#Date   NUMA 0  NUMA 1  NUMA 2  NUMA 3
2018-Oct-27_04h39m57s6   7   37  30
2018-Oct-27_04h40m02s16  15  23  26
2018-Oct-27_04h40m08s13  12  27  28
2018-Oct-27_04h40m13s9   15  29  27
2018-Oct-27_04h40m18s16  13  27  24
2018-Oct-27_04h40m23s16  14  25  25
2018-Oct-27_04h40m28s16  15  24  25
2018-Oct-27_04h40m33s10  11  34  25
2018-Oct-27_04h40m38s16  13  25  26
2018-Oct-27_04h40m43s10  10  32  28

Re: [PATCH 4/5] staging: rtl8188eu: change return type of rtw_hal_xmit()

[Joe Perches]

On Sat, 2018-10-27 at 15:57 -0700, Joe Perches wrote:
> On Sat, 2018-10-27 at 22:28 +0200, Michael Straube wrote:
> > The function rtw_hal_xmit() returns true or false.
> > Change the return type from s32 to bool.
> [] 
> > diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
> > b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> []
> > @@ -598,7 +598,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
> >   * truedump packet directly
> >   * false   enqueue packet
> >   */
> > -s32 rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
> > +bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
> >  {
> > s32 res;
> 
> Does "s32 res" need changing to bool too?

Perhaps all the functions regardless of types
with returns of only _SUCCESS and _FAIL could be
converted to bool.

Re: [PATCH 4/5] staging: rtl8188eu: change return type of rtw_hal_xmit()

[Joe Perches]

On Sat, 2018-10-27 at 15:57 -0700, Joe Perches wrote:
> On Sat, 2018-10-27 at 22:28 +0200, Michael Straube wrote:
> > The function rtw_hal_xmit() returns true or false.
> > Change the return type from s32 to bool.
> [] 
> > diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
> > b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> []
> > @@ -598,7 +598,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
> >   * truedump packet directly
> >   * false   enqueue packet
> >   */
> > -s32 rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
> > +bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
> >  {
> > s32 res;
> 
> Does "s32 res" need changing to bool too?

Perhaps all the functions regardless of types
with returns of only _SUCCESS and _FAIL could be
converted to bool.

Re: [Ksummit-discuss] The linux devs can rescind their license grant.

[Eric S. Raymond]

Bruce Perens :
> The anonymous person is generally thought to have appeared on the net
> previously as MikeeUSA. That entity has a well-recorded history of misogyny
> and other anti-social behaviour. He's also complained to me recently that
> because of "people like me", the law prohibits him from marrying very young
> women. I mean single-digit young. Although he is not at all meritorious of
> your civil behavior, you may not wish to lower yourself to his level.

I strongly doubt it.  I've had my own run-in with MikeeUSA; I remember
it vividly and unpleasantly.  The prose style doesn't match.  MikeeUSA
could barely maintain coherent communication; this guy is using
language that indicates he's at least several degrees brighter.
-- 
http://www.catb.org/~esr/;>Eric S. Raymond

My work is funded by the Internet Civil Engineering Institute: https://icei.org
Please visit their site and donate: the civilization you save might be your own.

Re: [Ksummit-discuss] The linux devs can rescind their license grant.

[Eric S. Raymond]

Bruce Perens :
> The anonymous person is generally thought to have appeared on the net
> previously as MikeeUSA. That entity has a well-recorded history of misogyny
> and other anti-social behaviour. He's also complained to me recently that
> because of "people like me", the law prohibits him from marrying very young
> women. I mean single-digit young. Although he is not at all meritorious of
> your civil behavior, you may not wish to lower yourself to his level.

I strongly doubt it.  I've had my own run-in with MikeeUSA; I remember
it vividly and unpleasantly.  The prose style doesn't match.  MikeeUSA
could barely maintain coherent communication; this guy is using
language that indicates he's at least several degrees brighter.
-- 
http://www.catb.org/~esr/;>Eric S. Raymond

My work is funded by the Internet Civil Engineering Institute: https://icei.org
Please visit their site and donate: the civilization you save might be your own.

Re: [PATCH 4/5] staging: rtl8188eu: change return type of rtw_hal_xmit()

[Joe Perches]

On Sat, 2018-10-27 at 22:28 +0200, Michael Straube wrote:
> The function rtw_hal_xmit() returns true or false.
> Change the return type from s32 to bool.
[]
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
> b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
[]
> @@ -598,7 +598,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
>   *   truedump packet directly
>   *   false   enqueue packet
>   */
> -s32 rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
> +bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
>  {
>   s32 res;

Does "s32 res" need changing to bool too?

Re: [PATCH 4/5] staging: rtl8188eu: change return type of rtw_hal_xmit()

[Joe Perches]

On Sat, 2018-10-27 at 22:28 +0200, Michael Straube wrote:
> The function rtw_hal_xmit() returns true or false.
> Change the return type from s32 to bool.
[]
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
> b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
[]
> @@ -598,7 +598,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
>   *   truedump packet directly
>   *   false   enqueue packet
>   */
> -s32 rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
> +bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
>  {
>   s32 res;

Does "s32 res" need changing to bool too?

Re: [PATCH] arch/x86/boot/memory.c: Touched up coding-style issues

[Joe Perches]

On Sat, 2018-10-27 at 23:32 +0100, Jordan Borgner wrote:
> Added missing parentheses to sizeof() function in detect_memory_e820().
> 
> Removed unnecessary braces in detect_memory_e801().
> 
> Replaced three if-statements with a ternary if-statement and 
> removed an unnecessary integer variable in detect_memory().
> 
> This is my first patch I hope it is okay.

Hello Jordan.

While the first bit is generally OK, given some individual
maintainer preferences, it may not also be applied.

Whitespace only changes without logical fixes/upgrades are
generally discouraged outside of drivers/staging.

This bit below though changes behavior.

> @@ -121,16 +120,7 @@ static int detect_memory_88(void)
>  
>  int detect_memory(void)
>  {
> -   int err = -1;
> -
> -   if (detect_memory_e820() > 0)
> -   err = 0;
> -
> -   if (!detect_memory_e801())
> -   err = 0;
> -
> -   if (!detect_memory_88())
> -   err = 0;
> -
> -   return err;
> +   return (detect_memory_e820() > 0 ||
> +   !detect_memory_e801()||
> +   !detect_memory_88()) ? 0 : -1;
>  }


For instance:

If the first detect_memory_e820 > 0 is true,
the other detect_memory_ calls will
not be done.

The original will always perform all three tests.


Regardless, please try to make your first patches
to the linux kernel somewhere in drivers/staging
so get comfortable with the general mechanisms and
styles of kernel patching.

Re: [PATCH] arch/x86/boot/memory.c: Touched up coding-style issues

[Joe Perches]

On Sat, 2018-10-27 at 23:32 +0100, Jordan Borgner wrote:
> Added missing parentheses to sizeof() function in detect_memory_e820().
> 
> Removed unnecessary braces in detect_memory_e801().
> 
> Replaced three if-statements with a ternary if-statement and 
> removed an unnecessary integer variable in detect_memory().
> 
> This is my first patch I hope it is okay.

Hello Jordan.

While the first bit is generally OK, given some individual
maintainer preferences, it may not also be applied.

Whitespace only changes without logical fixes/upgrades are
generally discouraged outside of drivers/staging.

This bit below though changes behavior.

> @@ -121,16 +120,7 @@ static int detect_memory_88(void)
>  
>  int detect_memory(void)
>  {
> -   int err = -1;
> -
> -   if (detect_memory_e820() > 0)
> -   err = 0;
> -
> -   if (!detect_memory_e801())
> -   err = 0;
> -
> -   if (!detect_memory_88())
> -   err = 0;
> -
> -   return err;
> +   return (detect_memory_e820() > 0 ||
> +   !detect_memory_e801()||
> +   !detect_memory_88()) ? 0 : -1;
>  }


For instance:

If the first detect_memory_e820 > 0 is true,
the other detect_memory_ calls will
not be done.

The original will always perform all three tests.


Regardless, please try to make your first patches
to the linux kernel somewhere in drivers/staging
so get comfortable with the general mechanisms and
styles of kernel patching.

[PATCH] arch/x86/boot/memory.c: Touched up coding-style issues

[Jordan Borgner]

Added missing parentheses to sizeof() function in detect_memory_e820().

Removed unnecessary braces in detect_memory_e801().

Replaced three if-statements with a ternary if-statement and 
removed an unnecessary integer variable in detect_memory().

This is my first patch I hope it is okay.

Signed-off-by: Jordan Borgner 
---
 linux-4.19/arch/x86/boot/memory.c | 24 +++-
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/linux-4.19/arch/x86/boot/memory.c b/linux-4.19/arch/x86/boot/memory.c
index d9c28c8..a6124af 100644
--- a/linux-4.19/arch/x86/boot/memory.c
+++ b/linux-4.19/arch/x86/boot/memory.c
@@ -26,7 +26,7 @@ static int detect_memory_e820(void)
 
 	initregs();
 	ireg.ax  = 0xe820;
-	ireg.cx  = sizeof buf;
+	ireg.cx  = sizeof(buf);
 	ireg.edx = SMAP;
 	ireg.di  = (size_t)
 
@@ -88,11 +88,11 @@ static int detect_memory_e801(void)
 		oreg.bx = oreg.dx;
 	}
 
-	if (oreg.ax > 15*1024) {
+	if (oreg.ax > 15*1024)
 		return -1;	/* Bogus! */
-	} else if (oreg.ax == 15*1024) {
+	else if (oreg.ax == 15*1024)
 		boot_params.alt_mem_k = (oreg.bx << 6) + oreg.ax;
-	} else {
+	else
 		/*
 		 * This ignores memory above 16MB if we have a memory
 		 * hole there.  If someone actually finds a machine
@@ -101,7 +101,6 @@ static int detect_memory_e801(void)
 		 * map.
 		 */
 		boot_params.alt_mem_k = oreg.ax;
-	}
 
 	return 0;
 }
@@ -121,16 +120,7 @@ static int detect_memory_88(void)
 
 int detect_memory(void)
 {
-	int err = -1;
-
-	if (detect_memory_e820() > 0)
-		err = 0;
-
-	if (!detect_memory_e801())
-		err = 0;
-
-	if (!detect_memory_88())
-		err = 0;
-
-	return err;
+	return (detect_memory_e820() > 0 ||
+		!detect_memory_e801()||
+		!detect_memory_88()) ? 0 : -1;
 }
-- 
2.11.0

[PATCH] arch/x86/boot/memory.c: Touched up coding-style issues

[Jordan Borgner]

Added missing parentheses to sizeof() function in detect_memory_e820().

Removed unnecessary braces in detect_memory_e801().

Replaced three if-statements with a ternary if-statement and 
removed an unnecessary integer variable in detect_memory().

This is my first patch I hope it is okay.

Signed-off-by: Jordan Borgner 
---
 linux-4.19/arch/x86/boot/memory.c | 24 +++-
 1 file changed, 7 insertions(+), 17 deletions(-)

diff --git a/linux-4.19/arch/x86/boot/memory.c b/linux-4.19/arch/x86/boot/memory.c
index d9c28c8..a6124af 100644
--- a/linux-4.19/arch/x86/boot/memory.c
+++ b/linux-4.19/arch/x86/boot/memory.c
@@ -26,7 +26,7 @@ static int detect_memory_e820(void)
 
 	initregs();
 	ireg.ax  = 0xe820;
-	ireg.cx  = sizeof buf;
+	ireg.cx  = sizeof(buf);
 	ireg.edx = SMAP;
 	ireg.di  = (size_t)
 
@@ -88,11 +88,11 @@ static int detect_memory_e801(void)
 		oreg.bx = oreg.dx;
 	}
 
-	if (oreg.ax > 15*1024) {
+	if (oreg.ax > 15*1024)
 		return -1;	/* Bogus! */
-	} else if (oreg.ax == 15*1024) {
+	else if (oreg.ax == 15*1024)
 		boot_params.alt_mem_k = (oreg.bx << 6) + oreg.ax;
-	} else {
+	else
 		/*
 		 * This ignores memory above 16MB if we have a memory
 		 * hole there.  If someone actually finds a machine
@@ -101,7 +101,6 @@ static int detect_memory_e801(void)
 		 * map.
 		 */
 		boot_params.alt_mem_k = oreg.ax;
-	}
 
 	return 0;
 }
@@ -121,16 +120,7 @@ static int detect_memory_88(void)
 
 int detect_memory(void)
 {
-	int err = -1;
-
-	if (detect_memory_e820() > 0)
-		err = 0;
-
-	if (!detect_memory_e801())
-		err = 0;
-
-	if (!detect_memory_88())
-		err = 0;
-
-	return err;
+	return (detect_memory_e820() > 0 ||
+		!detect_memory_e801()||
+		!detect_memory_88()) ? 0 : -1;
 }
-- 
2.11.0

Re: [Ksummit-discuss] The linux devs can rescind their license grant.

[Jiri Kosina]

On Sat, 27 Oct 2018, tim.b...@sony.com wrote:

> Al,
>
> Can you please, even in the face of comments you find irritating, keep 
> your responses more civil?  Calling someone a "wankstain" is 
> unprofessional

Tim,

to be completely honest, communicating anonymously doesn't really match my 
"this is highly professional" standards either, so I don't think we should 
be losing too much sleep over this particular e-mail exchange.

CoC explicitly requires us to be reasonably nice to the human being on the 
other end of the wire, which I whole-heartedly believe is a very noble and 
nice goal. But you really have to know at least a little bit who's there 
on the other end. Otherwise failure to communicate might be sort of 
inevitable.

-- 
Jiri Kosina
SUSE Labs

Re: [Ksummit-discuss] The linux devs can rescind their license grant.

[Jiri Kosina]

On Sat, 27 Oct 2018, tim.b...@sony.com wrote:

> Al,
>
> Can you please, even in the face of comments you find irritating, keep 
> your responses more civil?  Calling someone a "wankstain" is 
> unprofessional

Tim,

to be completely honest, communicating anonymously doesn't really match my 
"this is highly professional" standards either, so I don't think we should 
be losing too much sleep over this particular e-mail exchange.

CoC explicitly requires us to be reasonably nice to the human being on the 
other end of the wire, which I whole-heartedly believe is a very noble and 
nice goal. But you really have to know at least a little bit who's there 
on the other end. Otherwise failure to communicate might be sort of 
inevitable.

-- 
Jiri Kosina
SUSE Labs

Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

[Joe Perches]

On Fri, 2018-10-26 at 22:54 +0200, Julia Lawall wrote:
> [Adding Joe Perches]
> 
> On Fri, 26 Oct 2018, Sasha Levin wrote:
> 
> > On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote:
> > > This change was suggested by checkpath.pl. Use unsigned int with bitfield
> > > allocate only one bit to the boolean variable.
> > > 
> > > CHECK: Avoid using bool structure members because of possible alignment
> > > issues
> > > 
> > > Signed-off-by: Shayenne da Luz Moura 
> > > ---
> > > drivers/staging/vboxvideo/vbox_drv.h| 14 +++---
> > > drivers/staging/vboxvideo/vboxvideo_guest.h |  2 +-
> > > 2 files changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> > > b/drivers/staging/vboxvideo/vbox_drv.h
> > > index 594f84272957..7d3e329a6b1c 100644
> > > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > > @@ -81,7 +81,7 @@ struct vbox_private {
> > >   u8 __iomem *vbva_buffers;
> > >   struct gen_pool *guest_pool;
> > >   struct vbva_buf_ctx *vbva_info;
> > > - bool any_pitch;
> > > + unsigned int any_pitch:1;
> > >   u32 num_crtcs;
> > >   /** Amount of available VRAM, including space used for buffers. */
> > >   u32 full_vram_size;
> > 
> > Using bitfields for booleans in these cases is less efficient than just
> > using "regular" booleans for two reasons:
> > 
> > 1. It will use the same amount of space. Due to alignment requirements,
> > the compiler can't squeeze in anything into the 7 bits that are now
> > "free". Each member, unless it's another bitfield, must start at a whole
> > byte.
> > 
> > 2. This is actually less efficient (slower) for the compiler to work
> > with. The smallest granularity we have to access memory is 1 byte; we
> > can't set individual bits directly in memory. For the original code, the
> > assembly for 'vbox_private.any_pitch = true' would look something like
> > this:
> > 
> > movl   $0x1,-0x10(%rsp)
> > 
> > As you can see, the compiler can directly write into the variable.
> > However, when we switch to using bitfields, the compiler must preserve
> > the original value of the other 7 bits, so it must first read them from
> > memory, manipulate the value and write it back. The assembly would
> > look something like this:
> > 
> > movzbl -0x10(%rsp),%eax
> > or $0x1,%eax
> > mov%al,-0x10(%rsp)
> > 
> > Which is less efficient than what was previously happening.
> 
> Maybe checkpatch could be more precise about what kind of bools should be
> changed?

Probably so, what verbiage would you suggest?

Also, any conversion from bool to int would
have to take care than any assigment uses !!
where appropriate.

Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

[Joe Perches]

On Fri, 2018-10-26 at 22:54 +0200, Julia Lawall wrote:
> [Adding Joe Perches]
> 
> On Fri, 26 Oct 2018, Sasha Levin wrote:
> 
> > On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote:
> > > This change was suggested by checkpath.pl. Use unsigned int with bitfield
> > > allocate only one bit to the boolean variable.
> > > 
> > > CHECK: Avoid using bool structure members because of possible alignment
> > > issues
> > > 
> > > Signed-off-by: Shayenne da Luz Moura 
> > > ---
> > > drivers/staging/vboxvideo/vbox_drv.h| 14 +++---
> > > drivers/staging/vboxvideo/vboxvideo_guest.h |  2 +-
> > > 2 files changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> > > b/drivers/staging/vboxvideo/vbox_drv.h
> > > index 594f84272957..7d3e329a6b1c 100644
> > > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > > @@ -81,7 +81,7 @@ struct vbox_private {
> > >   u8 __iomem *vbva_buffers;
> > >   struct gen_pool *guest_pool;
> > >   struct vbva_buf_ctx *vbva_info;
> > > - bool any_pitch;
> > > + unsigned int any_pitch:1;
> > >   u32 num_crtcs;
> > >   /** Amount of available VRAM, including space used for buffers. */
> > >   u32 full_vram_size;
> > 
> > Using bitfields for booleans in these cases is less efficient than just
> > using "regular" booleans for two reasons:
> > 
> > 1. It will use the same amount of space. Due to alignment requirements,
> > the compiler can't squeeze in anything into the 7 bits that are now
> > "free". Each member, unless it's another bitfield, must start at a whole
> > byte.
> > 
> > 2. This is actually less efficient (slower) for the compiler to work
> > with. The smallest granularity we have to access memory is 1 byte; we
> > can't set individual bits directly in memory. For the original code, the
> > assembly for 'vbox_private.any_pitch = true' would look something like
> > this:
> > 
> > movl   $0x1,-0x10(%rsp)
> > 
> > As you can see, the compiler can directly write into the variable.
> > However, when we switch to using bitfields, the compiler must preserve
> > the original value of the other 7 bits, so it must first read them from
> > memory, manipulate the value and write it back. The assembly would
> > look something like this:
> > 
> > movzbl -0x10(%rsp),%eax
> > or $0x1,%eax
> > mov%al,-0x10(%rsp)
> > 
> > Which is less efficient than what was previously happening.
> 
> Maybe checkpatch could be more precise about what kind of bools should be
> changed?

Probably so, what verbiage would you suggest?

Also, any conversion from bool to int would
have to take care than any assigment uses !!
where appropriate.

Re: Another HID problem this merge window..

[Jiri Kosina]

On Sat, 27 Oct 2018, Linus Torvalds wrote:

> I wonder if there is some truly old historical legacy there, ie the old 
> PC keyboard support would have been configurable out only for expert 
> users to avoid errors, and maybe the HID Kconfig file started getting 
> ideas from that...

This really goes waaay back to times when we extracted all the quirks from 
the generic driver (which became unmaintainable exactly because quirks 
being sprinkled left and right) into specialized drivers, but didn't want 
to cause too many user surprises that all of a sudden their configuration 
regressed when it comes to hardware support.

We've had exactly this discussion multiple times before, see for example

https://lkml.org/lkml/2010/5/20/227

So I guess there is no need for replaying it, I think we're in complete 
agreement.

That being said, benff Kconfig setting definitely escaped attention. That 
should never ever have been set to default y, I take blame for not 
noticing that while applying the patch.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: Another HID problem this merge window..

[Jiri Kosina]

On Sat, 27 Oct 2018, Linus Torvalds wrote:

> I wonder if there is some truly old historical legacy there, ie the old 
> PC keyboard support would have been configurable out only for expert 
> users to avoid errors, and maybe the HID Kconfig file started getting 
> ideas from that...

This really goes waaay back to times when we extracted all the quirks from 
the generic driver (which became unmaintainable exactly because quirks 
being sprinkled left and right) into specialized drivers, but didn't want 
to cause too many user surprises that all of a sudden their configuration 
regressed when it comes to hardware support.

We've had exactly this discussion multiple times before, see for example

https://lkml.org/lkml/2010/5/20/227

So I guess there is no need for replaying it, I think we're in complete 
agreement.

That being said, benff Kconfig setting definitely escaped attention. That 
should never ever have been set to default y, I take blame for not 
noticing that while applying the patch.

Thanks,

-- 
Jiri Kosina
SUSE Labs

linux-kernel@vger.kernel.org has password qwerty. Password must be changed

[linux-kernel]

Hello!

I'm a programmer who cracked your email account and device about half year ago.
You entered a password on one of the insecure site you visited, and I catched 
it.
Your password from linux-kernel@vger.kernel.org on moment of crack: qwerty

Of course you can will change your password, or already made it.
But it doesn't matter, my rat software update it every time.

Please don't try to contact me or find me, it is impossible, since I sent you 
an email from your email account.

Through your e-mail, I uploaded malicious code to your Operation System.
I saved all of your contacts with friends, colleagues, relatives and a complete 
history of visits to the Internet resources.
Also I installed a rat software on your device and long tome spying for you.

You are not my only victim, I usually lock devices and ask for a ransom.
But I was struck by the sites of intimate content that you very often visit.

I am in shock of your reach fantasies! Wow! I've never seen anything like this!
I did not even know that SUCH content could be so exciting!

So, when you had fun on intime sites (you know what I mean!)
I made screenshot with using my program from your camera of yours device.
After that, I jointed them to the content of the currently viewed site.

Will be funny when I send these photos to your contacts! And if your relatives 
see it?
BUT I'm sure you don't want it. I definitely would not want to ...

I will not do this if you pay me a little amount.
I think $814 is a nice price for it!

I accept only Bitcoins.
My BTC wallet: 1HQ7wGdA5G9qUtM8jyDt5obDv1x3vEvjCy

If you have difficulty with this - Ask Google "how to make a payment on a 
bitcoin wallet". It's easy.
After receiving the above amount, all your data will be immediately removed 
automatically.
My virus will also will be destroy itself from your operating system.

My Trojan have auto alert, after this email is looked, I will be know it!

You have 2 days (48 hours) for make a payment.
If this does not happen - all your contacts will get crazy shots with your 
dirty life!
And so that you do not obstruct me, your device will be locked (also after 48 
hours)

Do not take this frivolously! This is the last warning!
Various security services or antiviruses won't help you for sure (I have 
already collected all your data).

Here are the recommendations of a professional:
Antiviruses do not help against modern malicious code. Just do not enter your 
passwords on unsafe sites!

I hope you will be prudent.
Bye.

linux-kernel@vger.kernel.org has password qwerty. Password must be changed

[linux-kernel]

Hello!

I'm a programmer who cracked your email account and device about half year ago.
You entered a password on one of the insecure site you visited, and I catched 
it.
Your password from linux-kernel@vger.kernel.org on moment of crack: qwerty

Of course you can will change your password, or already made it.
But it doesn't matter, my rat software update it every time.

Please don't try to contact me or find me, it is impossible, since I sent you 
an email from your email account.

Through your e-mail, I uploaded malicious code to your Operation System.
I saved all of your contacts with friends, colleagues, relatives and a complete 
history of visits to the Internet resources.
Also I installed a rat software on your device and long tome spying for you.

You are not my only victim, I usually lock devices and ask for a ransom.
But I was struck by the sites of intimate content that you very often visit.

I am in shock of your reach fantasies! Wow! I've never seen anything like this!
I did not even know that SUCH content could be so exciting!

So, when you had fun on intime sites (you know what I mean!)
I made screenshot with using my program from your camera of yours device.
After that, I jointed them to the content of the currently viewed site.

Will be funny when I send these photos to your contacts! And if your relatives 
see it?
BUT I'm sure you don't want it. I definitely would not want to ...

I will not do this if you pay me a little amount.
I think $814 is a nice price for it!

I accept only Bitcoins.
My BTC wallet: 1HQ7wGdA5G9qUtM8jyDt5obDv1x3vEvjCy

If you have difficulty with this - Ask Google "how to make a payment on a 
bitcoin wallet". It's easy.
After receiving the above amount, all your data will be immediately removed 
automatically.
My virus will also will be destroy itself from your operating system.

My Trojan have auto alert, after this email is looked, I will be know it!

You have 2 days (48 hours) for make a payment.
If this does not happen - all your contacts will get crazy shots with your 
dirty life!
And so that you do not obstruct me, your device will be locked (also after 48 
hours)

Do not take this frivolously! This is the last warning!
Various security services or antiviruses won't help you for sure (I have 
already collected all your data).

Here are the recommendations of a professional:
Antiviruses do not help against modern malicious code. Just do not enter your 
passwords on unsafe sites!

I hope you will be prudent.
Bye.

drivers//regulator/bd718x7-regulator.c:364:16: error: implicit declaration of function 'of_match_ptr'; did you mean 'hash_ptr'?

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   69d5b97c597307773fe6c59775a5d5a88bb7e6b3
commit: 2ece646c90c5b45dd76c76ea207a3f3459f2c472 regulator: bd718xx: rename 
bd71837 to 718xx
date:   4 weeks ago
config: x86_64-randconfig-s4-10280315 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 2ece646c90c5b45dd76c76ea207a3f3459f2c472
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

>> drivers//regulator/bd718x7-regulator.c:364:16: error: implicit declaration 
>> of function 'of_match_ptr'; did you mean 'hash_ptr'? 
>> [-Werror=implicit-function-declaration]
   .of_match = of_match_ptr("BUCK1"),
   ^~~~
   hash_ptr
>> drivers//regulator/bd718x7-regulator.c:364:16: warning: initialization makes 
>> pointer from integer without a cast [-Wint-conversion]
   drivers//regulator/bd718x7-regulator.c:364:16: note: (near initialization 
for 'bd71847_regulators[0].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:364:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:364:16: note: (near initialization 
for 'bd71847_regulators[0].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:365:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:365:23: note: (near initialization 
for 'bd71847_regulators[0].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:365:23: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:365:23: note: (near initialization 
for 'bd71847_regulators[0].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:388:16: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .of_match = of_match_ptr("BUCK2"),
   ^~~~
   drivers//regulator/bd718x7-regulator.c:388:16: note: (near initialization 
for 'bd71847_regulators[1].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:388:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:388:16: note: (near initialization 
for 'bd71847_regulators[1].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:389:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:389:23: note: (near initialization 
for 'bd71847_regulators[1].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:389:23: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:389:23: note: (near initialization 
for 'bd71847_regulators[1].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:411:16: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .of_match = of_match_ptr("BUCK3"),
   ^~~~
   drivers//regulator/bd718x7-regulator.c:411:16: note: (near initialization 
for 'bd71847_regulators[2].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:411:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:411:16: note: (near initialization 
for 'bd71847_regulators[2].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:412:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:412:23: note: (near initialization 
for 'bd71847_regulators[2].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:412:23: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:412:23: note: (near initialization 
for 'bd71847_regulators[2].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:438:16: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .of_match = of_match_ptr("BUCK4"),
   ^~~~
   drivers//regulator/bd718x7-regulator.c:438:16: note: (near initialization 
for 'bd71847_regulators[3].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:438:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:438:16: note: (near initialization 
for 'bd71847_regulators[3].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:439:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:439:23: note: (near initialization 
for 

drivers//regulator/bd718x7-regulator.c:364:16: error: implicit declaration of function 'of_match_ptr'; did you mean 'hash_ptr'?

[kbuild test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   69d5b97c597307773fe6c59775a5d5a88bb7e6b3
commit: 2ece646c90c5b45dd76c76ea207a3f3459f2c472 regulator: bd718xx: rename 
bd71837 to 718xx
date:   4 weeks ago
config: x86_64-randconfig-s4-10280315 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
git checkout 2ece646c90c5b45dd76c76ea207a3f3459f2c472
# save the attached .config to linux build tree
make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

>> drivers//regulator/bd718x7-regulator.c:364:16: error: implicit declaration 
>> of function 'of_match_ptr'; did you mean 'hash_ptr'? 
>> [-Werror=implicit-function-declaration]
   .of_match = of_match_ptr("BUCK1"),
   ^~~~
   hash_ptr
>> drivers//regulator/bd718x7-regulator.c:364:16: warning: initialization makes 
>> pointer from integer without a cast [-Wint-conversion]
   drivers//regulator/bd718x7-regulator.c:364:16: note: (near initialization 
for 'bd71847_regulators[0].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:364:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:364:16: note: (near initialization 
for 'bd71847_regulators[0].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:365:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:365:23: note: (near initialization 
for 'bd71847_regulators[0].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:365:23: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:365:23: note: (near initialization 
for 'bd71847_regulators[0].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:388:16: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .of_match = of_match_ptr("BUCK2"),
   ^~~~
   drivers//regulator/bd718x7-regulator.c:388:16: note: (near initialization 
for 'bd71847_regulators[1].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:388:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:388:16: note: (near initialization 
for 'bd71847_regulators[1].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:389:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:389:23: note: (near initialization 
for 'bd71847_regulators[1].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:389:23: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:389:23: note: (near initialization 
for 'bd71847_regulators[1].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:411:16: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .of_match = of_match_ptr("BUCK3"),
   ^~~~
   drivers//regulator/bd718x7-regulator.c:411:16: note: (near initialization 
for 'bd71847_regulators[2].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:411:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:411:16: note: (near initialization 
for 'bd71847_regulators[2].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:412:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:412:23: note: (near initialization 
for 'bd71847_regulators[2].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:412:23: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:412:23: note: (near initialization 
for 'bd71847_regulators[2].desc.regulators_node')
   drivers//regulator/bd718x7-regulator.c:438:16: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .of_match = of_match_ptr("BUCK4"),
   ^~~~
   drivers//regulator/bd718x7-regulator.c:438:16: note: (near initialization 
for 'bd71847_regulators[3].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:438:16: error: initializer element is 
not constant
   drivers//regulator/bd718x7-regulator.c:438:16: note: (near initialization 
for 'bd71847_regulators[3].desc.of_match')
   drivers//regulator/bd718x7-regulator.c:439:23: warning: initialization makes 
pointer from integer without a cast [-Wint-conversion]
   .regulators_node = of_match_ptr("regulators"),
  ^~~~
   drivers//regulator/bd718x7-regulator.c:439:23: note: (near initialization 
for 

[PATCH 5/5] staging: rtl8188eu: cleanup long line in rtw_hal_xmit()

[Michael Straube]

Cleanup a line over 80 characters in rtw_hal_xmit() by using
if(x) instead of if(x == true). Also clears a missing spaces
around '|' checkpatch issue.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index 35bc8fc32db1..a72e069269b8 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -611,7 +611,7 @@ bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame 
*pxmitframe)
if (rtw_txframes_sta_ac_pending(adapt, pattrib) > 0)
goto enqueue;
 
-   if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY|_FW_UNDER_LINKING) == 
true)
+   if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY | _FW_UNDER_LINKING))
goto enqueue;
 
pxmitbuf = rtw_alloc_xmitbuf(pxmitpriv);
-- 
2.19.1

[PATCH 5/5] staging: rtl8188eu: cleanup long line in rtw_hal_xmit()

[Michael Straube]

Cleanup a line over 80 characters in rtw_hal_xmit() by using
if(x) instead of if(x == true). Also clears a missing spaces
around '|' checkpatch issue.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index 35bc8fc32db1..a72e069269b8 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -611,7 +611,7 @@ bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame 
*pxmitframe)
if (rtw_txframes_sta_ac_pending(adapt, pattrib) > 0)
goto enqueue;
 
-   if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY|_FW_UNDER_LINKING) == 
true)
+   if (check_fwstate(pmlmepriv, _FW_UNDER_SURVEY | _FW_UNDER_LINKING))
goto enqueue;
 
pxmitbuf = rtw_alloc_xmitbuf(pxmitpriv);
-- 
2.19.1

[PATCH 2/5] staging: rtl8188eu: change type of a struct field

[Michael Straube]

The field enable of struct recv_reorder_ctrl is only used for boolean
values, so change the type from u8 to bool.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/include/rtw_recv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/include/rtw_recv.h 
b/drivers/staging/rtl8188eu/include/rtw_recv.h
index 54b7ba367293..8fc500496f92 100644
--- a/drivers/staging/rtl8188eu/include/rtw_recv.h
+++ b/drivers/staging/rtl8188eu/include/rtw_recv.h
@@ -27,7 +27,7 @@
 /* for Rx reordering buffer control */
 struct recv_reorder_ctrl {
struct adapter  *padapter;
-   u8 enable;
+   bool enable;
u16 indicate_seq;/* wstart_b, init_value=0x */
u16 wend_b;
u8 wsize_b;
-- 
2.19.1

[PATCH 2/5] staging: rtl8188eu: change type of a struct field

[Michael Straube]

The field enable of struct recv_reorder_ctrl is only used for boolean
values, so change the type from u8 to bool.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/include/rtw_recv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/include/rtw_recv.h 
b/drivers/staging/rtl8188eu/include/rtw_recv.h
index 54b7ba367293..8fc500496f92 100644
--- a/drivers/staging/rtl8188eu/include/rtw_recv.h
+++ b/drivers/staging/rtl8188eu/include/rtw_recv.h
@@ -27,7 +27,7 @@
 /* for Rx reordering buffer control */
 struct recv_reorder_ctrl {
struct adapter  *padapter;
-   u8 enable;
+   bool enable;
u16 indicate_seq;/* wstart_b, init_value=0x */
u16 wend_b;
u8 wsize_b;
-- 
2.19.1

[PATCH 3/5] staging: rtl8188eu: change return type of rtl8188eu_xmitframe_complete()

[Michael Straube]

The function rtl8188eu_xmitframe_complete() returns true or false.
Change the return type from s32 to bool.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c| 3 ++-
 drivers/staging/rtl8188eu/include/rtl8188e_xmit.h | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index a11bee16d070..69a0edd27710 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -412,7 +412,8 @@ static u32 xmitframe_need_length(struct xmit_frame 
*pxmitframe)
return len;
 }
 
-s32 rtl8188eu_xmitframe_complete(struct adapter *adapt, struct xmit_priv 
*pxmitpriv)
+bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
+ struct xmit_priv *pxmitpriv)
 {
struct xmit_frame *pxmitframe = NULL;
struct xmit_frame *pfirstframe = NULL;
diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h 
b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
index 20d35480dab8..421e9f45306f 100644
--- a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
+++ b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
@@ -149,8 +149,8 @@ s32 rtl8188eu_init_xmit_priv(struct adapter *padapter);
 s32 rtl8188eu_xmit_buf_handler(struct adapter *padapter);
 #define hal_xmit_handler rtl8188eu_xmit_buf_handler
 void rtl8188eu_xmit_tasklet(void *priv);
-s32 rtl8188eu_xmitframe_complete(struct adapter *padapter,
-struct xmit_priv *pxmitpriv);
+bool rtl8188eu_xmitframe_complete(struct adapter *padapter,
+ struct xmit_priv *pxmitpriv);
 
 void handle_txrpt_ccx_88e(struct adapter *adapter, u8 *buf);
 
-- 
2.19.1

[PATCH 1/5] staging: rtl8188eu: remove unnecessary ternary operator

[Michael Straube]

The field accept_addba_req of struct mlme_ext_info has type bool.
Use the value of accept_addba_req directly instead of the ternary
operator in an asignment.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c 
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 3e05e2c7f61b..d678dcee7312 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1504,8 +1504,7 @@ void process_addba_req(struct adapter *padapter, u8 
*paddba_req, u8 *addr)
tid = (param>>2)&0x0f;
preorder_ctrl = >recvreorder_ctrl[tid];
preorder_ctrl->indicate_seq = 0x;
-   preorder_ctrl->enable = (pmlmeinfo->accept_addba_req) ? true
- : false;
+   preorder_ctrl->enable = pmlmeinfo->accept_addba_req;
}
 }
 
-- 
2.19.1

[PATCH 4/5] staging: rtl8188eu: change return type of rtw_hal_xmit()

[Michael Straube]

The function rtw_hal_xmit() returns true or false.
Change the return type from s32 to bool.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 2 +-
 drivers/staging/rtl8188eu/include/hal_intf.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index 69a0edd27710..35bc8fc32db1 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -598,7 +598,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
  * truedump packet directly
  * false   enqueue packet
  */
-s32 rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
+bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
 {
s32 res;
struct xmit_buf *pxmitbuf = NULL;
diff --git a/drivers/staging/rtl8188eu/include/hal_intf.h 
b/drivers/staging/rtl8188eu/include/hal_intf.h
index e5be27af7bf5..8b65fcba1967 100644
--- a/drivers/staging/rtl8188eu/include/hal_intf.h
+++ b/drivers/staging/rtl8188eu/include/hal_intf.h
@@ -185,7 +185,7 @@ u32 rtw_hal_inirp_init(struct adapter *padapter);
 void   rtw_hal_inirp_deinit(struct adapter *padapter);
 void usb_intf_stop(struct adapter *padapter);
 
-s32rtw_hal_xmit(struct adapter *padapter, struct xmit_frame *pxmitframe);
+bool rtw_hal_xmit(struct adapter *padapter, struct xmit_frame *pxmitframe);
 s32rtw_hal_mgnt_xmit(struct adapter *padapter,
  struct xmit_frame *pmgntframe);
 
-- 
2.19.1

[PATCH 3/5] staging: rtl8188eu: change return type of rtl8188eu_xmitframe_complete()

[Michael Straube]

The function rtl8188eu_xmitframe_complete() returns true or false.
Change the return type from s32 to bool.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c| 3 ++-
 drivers/staging/rtl8188eu/include/rtl8188e_xmit.h | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index a11bee16d070..69a0edd27710 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -412,7 +412,8 @@ static u32 xmitframe_need_length(struct xmit_frame 
*pxmitframe)
return len;
 }
 
-s32 rtl8188eu_xmitframe_complete(struct adapter *adapt, struct xmit_priv 
*pxmitpriv)
+bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
+ struct xmit_priv *pxmitpriv)
 {
struct xmit_frame *pxmitframe = NULL;
struct xmit_frame *pfirstframe = NULL;
diff --git a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h 
b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
index 20d35480dab8..421e9f45306f 100644
--- a/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
+++ b/drivers/staging/rtl8188eu/include/rtl8188e_xmit.h
@@ -149,8 +149,8 @@ s32 rtl8188eu_init_xmit_priv(struct adapter *padapter);
 s32 rtl8188eu_xmit_buf_handler(struct adapter *padapter);
 #define hal_xmit_handler rtl8188eu_xmit_buf_handler
 void rtl8188eu_xmit_tasklet(void *priv);
-s32 rtl8188eu_xmitframe_complete(struct adapter *padapter,
-struct xmit_priv *pxmitpriv);
+bool rtl8188eu_xmitframe_complete(struct adapter *padapter,
+ struct xmit_priv *pxmitpriv);
 
 void handle_txrpt_ccx_88e(struct adapter *adapter, u8 *buf);
 
-- 
2.19.1

[PATCH 1/5] staging: rtl8188eu: remove unnecessary ternary operator

[Michael Straube]

The field accept_addba_req of struct mlme_ext_info has type bool.
Use the value of accept_addba_req directly instead of the ternary
operator in an asignment.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c 
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 3e05e2c7f61b..d678dcee7312 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1504,8 +1504,7 @@ void process_addba_req(struct adapter *padapter, u8 
*paddba_req, u8 *addr)
tid = (param>>2)&0x0f;
preorder_ctrl = >recvreorder_ctrl[tid];
preorder_ctrl->indicate_seq = 0x;
-   preorder_ctrl->enable = (pmlmeinfo->accept_addba_req) ? true
- : false;
+   preorder_ctrl->enable = pmlmeinfo->accept_addba_req;
}
 }
 
-- 
2.19.1

[PATCH 4/5] staging: rtl8188eu: change return type of rtw_hal_xmit()

[Michael Straube]

The function rtw_hal_xmit() returns true or false.
Change the return type from s32 to bool.

Signed-off-by: Michael Straube 
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 2 +-
 drivers/staging/rtl8188eu/include/hal_intf.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c 
b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index 69a0edd27710..35bc8fc32db1 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -598,7 +598,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
  * truedump packet directly
  * false   enqueue packet
  */
-s32 rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
+bool rtw_hal_xmit(struct adapter *adapt, struct xmit_frame *pxmitframe)
 {
s32 res;
struct xmit_buf *pxmitbuf = NULL;
diff --git a/drivers/staging/rtl8188eu/include/hal_intf.h 
b/drivers/staging/rtl8188eu/include/hal_intf.h
index e5be27af7bf5..8b65fcba1967 100644
--- a/drivers/staging/rtl8188eu/include/hal_intf.h
+++ b/drivers/staging/rtl8188eu/include/hal_intf.h
@@ -185,7 +185,7 @@ u32 rtw_hal_inirp_init(struct adapter *padapter);
 void   rtw_hal_inirp_deinit(struct adapter *padapter);
 void usb_intf_stop(struct adapter *padapter);
 
-s32rtw_hal_xmit(struct adapter *padapter, struct xmit_frame *pxmitframe);
+bool rtw_hal_xmit(struct adapter *padapter, struct xmit_frame *pxmitframe);
 s32rtw_hal_mgnt_xmit(struct adapter *padapter,
  struct xmit_frame *pmgntframe);
 
-- 
2.19.1

Re: Another HID problem this merge window..

[Linus Torvalds]

On Sat, Oct 27, 2018 at 12:36 PM Joe Perches  wrote:
>
> In fairness, it seems many of the HID drivers do exactly that
> and this could have been a "copy from example" addition.

Interesting, and I think you're right.

I wonder why I haven't noticed this before. Some of those might be
hidden by other dependencies - what I do is to just check how "make
oldconfig" changes my normal fairly minimal configuration, and then
complain when I notice that somebody enables some odd new hardware.

Anyway, those other "!EXPERT" defaults look pretty questionable too.
HID itself beind default y and the HID_GENERIC driver defaulting on
seems eminently sane. And HID_LOGITECH might fall under both "very
common" and "enables other config options".

But even then, the !EXPERT seems an odd choice. Why not just 'y' for
the truly common cases?

I wonder if there is some truly old historical legacy there, ie the
old PC keyboard support would have been configurable out only for
expert users to avoid errors, and maybe the HID Kconfig file started
getting ideas from that...

Linus

Re: Another HID problem this merge window..

[Linus Torvalds]

On Sat, Oct 27, 2018 at 12:36 PM Joe Perches  wrote:
>
> In fairness, it seems many of the HID drivers do exactly that
> and this could have been a "copy from example" addition.

Interesting, and I think you're right.

I wonder why I haven't noticed this before. Some of those might be
hidden by other dependencies - what I do is to just check how "make
oldconfig" changes my normal fairly minimal configuration, and then
complain when I notice that somebody enables some odd new hardware.

Anyway, those other "!EXPERT" defaults look pretty questionable too.
HID itself beind default y and the HID_GENERIC driver defaulting on
seems eminently sane. And HID_LOGITECH might fall under both "very
common" and "enables other config options".

But even then, the !EXPERT seems an odd choice. Why not just 'y' for
the truly common cases?

I wonder if there is some truly old historical legacy there, ie the
old PC keyboard support would have been configurable out only for
expert users to avoid errors, and maybe the HID Kconfig file started
getting ideas from that...

Linus

Re: [PATCH 11/11] perf tools: Stop fallbacking to kallsyms for vdso symbols lookup

[Vinicius Costa Gomes]

Hi Jirka,

Jiri Olsa  writes:

> On Fri, Oct 26, 2018 at 04:19:52PM -0700, Vinicius Costa Gomes wrote:
>> Hi,
>> 
>> Adrian Hunter  writes:
>> 
>> > On 18/10/18 1:55 AM, Arnaldo Carvalho de Melo wrote:
>> >> From: Arnaldo Carvalho de Melo 
>> >> 
>> >> David reports that:
>> >> 
>> >> 
>> >> Perf has this hack where it uses the kernel symbol map as a backup when
>> >> a symbol can't be found in the user's symbol table(s).
>> >
>> > I don't think this is a complete fix because it exposes new problems.
>> 
>> This commit broke function name resolution for 'perf record -g' for me.
>> 
>> What I mean is, with this commit applied:
>> 
>> $ ./tools/perf/perf record -g -- sleep 1
>> 
>> $ ./tools/perf/perf report
>> 
>> 'perf report' doesn't seem to be able to show the function names of the
>> trace.
>> 
>> If I revert this commit, function names are resolved fine.
>
> that commit just showed up some places where we have the
> ip resolve wrong.. would attached patch fix it for you?

Thank you for your patch.

I can some difference in the output, but I wouldn't say that it's fixed.

Here are some samples, if it's useful somehow:

https://gist.github.com/vcgomes/985626705e0968b973e426964f86a4b0

The "ping" tests were done running

$ sudo ./tools/perf/perf record -g -- ping -f -c 1000 127.0.0.1

And the "sleep" tests were done running

$ sudo ./tools/perf/perf record -g -- /bin/sleep 1


--
Vinicius

Re: [PATCH 11/11] perf tools: Stop fallbacking to kallsyms for vdso symbols lookup

[Vinicius Costa Gomes]

Hi Jirka,

Jiri Olsa  writes:

> On Fri, Oct 26, 2018 at 04:19:52PM -0700, Vinicius Costa Gomes wrote:
>> Hi,
>> 
>> Adrian Hunter  writes:
>> 
>> > On 18/10/18 1:55 AM, Arnaldo Carvalho de Melo wrote:
>> >> From: Arnaldo Carvalho de Melo 
>> >> 
>> >> David reports that:
>> >> 
>> >> 
>> >> Perf has this hack where it uses the kernel symbol map as a backup when
>> >> a symbol can't be found in the user's symbol table(s).
>> >
>> > I don't think this is a complete fix because it exposes new problems.
>> 
>> This commit broke function name resolution for 'perf record -g' for me.
>> 
>> What I mean is, with this commit applied:
>> 
>> $ ./tools/perf/perf record -g -- sleep 1
>> 
>> $ ./tools/perf/perf report
>> 
>> 'perf report' doesn't seem to be able to show the function names of the
>> trace.
>> 
>> If I revert this commit, function names are resolved fine.
>
> that commit just showed up some places where we have the
> ip resolve wrong.. would attached patch fix it for you?

Thank you for your patch.

I can some difference in the output, but I wouldn't say that it's fixed.

Here are some samples, if it's useful somehow:

https://gist.github.com/vcgomes/985626705e0968b973e426964f86a4b0

The "ping" tests were done running

$ sudo ./tools/perf/perf record -g -- ping -f -c 1000 127.0.0.1

And the "sleep" tests were done running

$ sudo ./tools/perf/perf record -g -- /bin/sleep 1


--
Vinicius

Re: [PATCH] fs: proc: move linux_proc_banner to where it is used

[Alexey Dobriyan]

On Fri, Oct 26, 2018 at 11:20:34PM +0200, Rasmus Villemoes wrote:
> +#include 

> +#define linux_proc_banner \
> + "%s version %s" \
> + " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")" \
> + " (" LINUX_COMPILER ") %s\n"

Include doesn't work if compiling from scratch:

rm -rf ../obj
mkdir ../obj
make O=../obj defconfig
make O=../obj fs/proc/version.o

  CC  fs/proc/version.o
fs/proc/version.c:2:10: fatal error: generated/compile.h: No such file or 
directory

Re: [PATCH] fs: proc: move linux_proc_banner to where it is used

[Alexey Dobriyan]

On Fri, Oct 26, 2018 at 11:20:34PM +0200, Rasmus Villemoes wrote:
> +#include 

> +#define linux_proc_banner \
> + "%s version %s" \
> + " (" LINUX_COMPILE_BY "@" LINUX_COMPILE_HOST ")" \
> + " (" LINUX_COMPILER ") %s\n"

Include doesn't work if compiling from scratch:

rm -rf ../obj
mkdir ../obj
make O=../obj defconfig
make O=../obj fs/proc/version.o

  CC  fs/proc/version.o
fs/proc/version.c:2:10: fatal error: generated/compile.h: No such file or 
directory

Re: Another HID problem this merge window..

[Joe Perches]

On Sat, 2018-10-27 at 11:13 -0700, Linus Torvalds wrote:
> Ok, so this is a much smaller issue than the i2c one that cause boot
> problems, but it's annoying.
> 
> We do *not* enable new random drivers by default. And we most
> *definitely* don't do it when they are odd-ball ones that most people
> have never heard of.
> 
> Yet the new "BigBen Interactive" driver that was added this merge
> window did exactly that.

In fairness, it seems many of the HID drivers do exactly that
and this could have been a "copy from example" addition.

$ git grep -P -i -B2 'default\s+\!EXPERT' -- drivers/hid/Kconfig
drivers/hid/Kconfig-tristate "A4 tech mice"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Apple {i,Power,Mac}Books"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Belkin Flip KVM and Wireless keyboard"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Cherry Cymotion keyboard"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Chicony devices"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Cypress mouse and barcode readers"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Ezkey BTC 8193 keyboard"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "ITE devices"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Kensington Slimblade Trackball"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Logitech devices"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT

Re: Another HID problem this merge window..

[Joe Perches]

On Sat, 2018-10-27 at 11:13 -0700, Linus Torvalds wrote:
> Ok, so this is a much smaller issue than the i2c one that cause boot
> problems, but it's annoying.
> 
> We do *not* enable new random drivers by default. And we most
> *definitely* don't do it when they are odd-ball ones that most people
> have never heard of.
> 
> Yet the new "BigBen Interactive" driver that was added this merge
> window did exactly that.

In fairness, it seems many of the HID drivers do exactly that
and this could have been a "copy from example" addition.

$ git grep -P -i -B2 'default\s+\!EXPERT' -- drivers/hid/Kconfig
drivers/hid/Kconfig-tristate "A4 tech mice"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Apple {i,Power,Mac}Books"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Belkin Flip KVM and Wireless keyboard"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Cherry Cymotion keyboard"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Chicony devices"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Cypress mouse and barcode readers"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Ezkey BTC 8193 keyboard"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "ITE devices"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Kensington Slimblade Trackball"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT
--
drivers/hid/Kconfig-tristate "Logitech devices"
drivers/hid/Kconfig-depends on HID
drivers/hid/Kconfig:default !EXPERT

[PATCH] PCI: histb: constify dw_pcie_host_ops structure

[Julia Lawall]

The dw_pcie_host_ops structure is only stored in the ops field
of a pcie_port structure, and this field is const, so make the
dw_pcie_host_ops structure const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/pci/controller/dwc/pcie-histb.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/pci/controller/dwc/pcie-histb.c 
b/drivers/pci/controller/dwc/pcie-histb.c
--- a/drivers/pci/controller/dwc/pcie-histb.c
+++ b/drivers/pci/controller/dwc/pcie-histb.c
@@ -202,7 +202,7 @@ static int histb_pcie_host_init(struct p
return 0;
 }
 
-static struct dw_pcie_host_ops histb_pcie_host_ops = {
+static const struct dw_pcie_host_ops histb_pcie_host_ops = {
.rd_own_conf = histb_pcie_rd_own_conf,
.wr_own_conf = histb_pcie_wr_own_conf,
.host_init = histb_pcie_host_init,

[PATCH] PCI: histb: constify dw_pcie_host_ops structure

[Julia Lawall]

The dw_pcie_host_ops structure is only stored in the ops field
of a pcie_port structure, and this field is const, so make the
dw_pcie_host_ops structure const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/pci/controller/dwc/pcie-histb.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/pci/controller/dwc/pcie-histb.c 
b/drivers/pci/controller/dwc/pcie-histb.c
--- a/drivers/pci/controller/dwc/pcie-histb.c
+++ b/drivers/pci/controller/dwc/pcie-histb.c
@@ -202,7 +202,7 @@ static int histb_pcie_host_init(struct p
return 0;
 }
 
-static struct dw_pcie_host_ops histb_pcie_host_ops = {
+static const struct dw_pcie_host_ops histb_pcie_host_ops = {
.rd_own_conf = histb_pcie_rd_own_conf,
.wr_own_conf = histb_pcie_wr_own_conf,
.host_init = histb_pcie_host_init,

Another HID problem this merge window..

[Linus Torvalds]

Ok, so this is a much smaller issue than the i2c one that cause boot
problems, but it's annoying.

We do *not* enable new random drivers by default. And we most
*definitely* don't do it when they are odd-ball ones that most people
have never heard of.

Yet the new "BigBen Interactive" driver that was added this merge
window did exactly that.

Just don't do it.

Yes, yes, every developer always thinks that _their_ driver is so
special and so magically important that it should be enabled by
default. But no. When we have thousands of drivers, we don't randomly
pick one new driver to be enabled by default just because some
developer thinks it is special. It's not.

So the

default !EXPERT

was completely wrong in commit 256a90ed9e46 ("HID: hid-bigbenff:
driver for BigBen Interactive PS3OFMINIPAD gamepad"). Please don't do
things like this.

 Linus

Another HID problem this merge window..

[Linus Torvalds]

Ok, so this is a much smaller issue than the i2c one that cause boot
problems, but it's annoying.

We do *not* enable new random drivers by default. And we most
*definitely* don't do it when they are odd-ball ones that most people
have never heard of.

Yet the new "BigBen Interactive" driver that was added this merge
window did exactly that.

Just don't do it.

Yes, yes, every developer always thinks that _their_ driver is so
special and so magically important that it should be enabled by
default. But no. When we have thousands of drivers, we don't randomly
pick one new driver to be enabled by default just because some
developer thinks it is special. It's not.

So the

default !EXPERT

was completely wrong in commit 256a90ed9e46 ("HID: hid-bigbenff:
driver for BigBen Interactive PS3OFMINIPAD gamepad"). Please don't do
things like this.

 Linus

Re: [GIT PULL REQUEST] watchdog - v4.20 Merge window

[Linus Torvalds]

On Sat, Oct 27, 2018 at 4:50 AM Wim Van Sebroeck  
wrote:
>
> Please pull the watchdog changes for the v4.20 (or 5.00) release cycle.

Pulled,

 Linus

Re: [GIT PULL REQUEST] watchdog - v4.20 Merge window

[Linus Torvalds]

On Sat, Oct 27, 2018 at 4:50 AM Wim Van Sebroeck  
wrote:
>
> Please pull the watchdog changes for the v4.20 (or 5.00) release cycle.

Pulled,

 Linus

Re: [git pull] Input updates for v4.20-rc0

[Linus Torvalds]

On Fri, Oct 26, 2018 at 4:49 PM Dmitry Torokhov
 wrote:
>
> updates for the input subsystem. Just random driver fixups,
> nothing exiting.

Pulled,
 Linus

Re: [git pull] Input updates for v4.20-rc0

[Linus Torvalds]

On Fri, Oct 26, 2018 at 4:49 PM Dmitry Torokhov
 wrote:
>
> updates for the input subsystem. Just random driver fixups,
> nothing exiting.

Pulled,
 Linus

Re: [PATCH v3 2/3] iio: adc128s052: add ACPI _HID AANT1280

[Himanshu Jha]

Hi Dan,

On Thu, Oct 25, 2018 at 04:35:41PM +0100, Dan O'Donovan wrote:
> From: Nicola Lunghi 
> 
> ACPI _HID AANT1280 matches an ADC124S101 present on E3940 SKUs of the UP
> Squared board.
> 
> Add it to the driver.
> 
> Signed-off-by: Nicola Lunghi 
> [jav...@emutex.com: fix up commit message and one checkpatch warning]
> Signed-off-by: Javier Arteaga 
> Signed-off-by: Dan O'Donovan 
> ---
>  drivers/iio/adc/ti-adc128s052.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)

[]

> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id adc128_acpi_match[] = {
> + { "AANT1280", 2 }, /* ADC124S021 compatible ACPI ID */
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
> +#endif

I'm curious about the naming conventions used for selecting 
an ACPI ID.

AFAIK, ACPI or PNP ID consists of *Vendor* ID + Product ID.

PNP ID: PNP Vendor IDs consist of 3 characters, each character being
an uppercase letter (A-Z).

ACPI ID: ACPI Vendor IDs consist of 4 characters, each character being
either an uppercase letter (A-Z) or a numeral (0-9).

In your case,

Vendor ID -> AANT (AAEON TECHNOLOGY INC. registered ACPI ID prefix)
http://www.uefi.org/ACPI_ID_List?search=AANT

Product ID -> 1280

How did you come up with 1280 ? And why AANT ? why not TXNW which is
registered ACPI prefix for TEXAS INSTRUMENTS ?
http://www.uefi.org/ACPI_ID_List?search=Texas+

Latest ACPI manuals says:

6.1.5 _HID (Hardware ID)


This object is used to supply OSPM with the device’s Plug and Play
hardware ID.[1]

[1] "A Plug and Play ID or ACPI ID can be obtained by sending e-mail to
pn...@microsoft.com."

A _HID object evaluates to either a numeric 32-bit compressed EISA type ID or a 
string. If a
string, the format must be an alphanumeric PNP or ACPI ID with no asterisk or 
other leading
characters.

A valid PNP ID must be of the form "AAA" where A is an uppercase letter and 
# is a hex
digit. A valid ACPI ID must be of the form "" where N is an uppercase 
letter or a
digit ('0'-'9') and # is a hex digit. This specification reserves the string 
"ACPI" for use only
with devices defined herein. It further reserves all strings representing 4 HEX 
digits for
exclusive use with PCI-assigned Vendor IDs.


Next question:

There are a lot drivers in iio/ using ACPI for device enumeration using
these IDs. For now, let's take an example of Bosch Sensors:

drivers/iio/accel/bmc150-accel-i2c.c:static const struct acpi_device_id 
bmc150_accel_acpi_match[] = {
drivers/iio/accel/bmc150-accel-i2c.c-   {"BSBA0150",bmc150},
drivers/iio/accel/bmc150-accel-i2c.c-   {"BMC150A", bmc150},
drivers/iio/accel/bmc150-accel-i2c.c-   {"BMI055A", bmi055},
drivers/iio/accel/bmc150-accel-i2c.c-   {"BMA0255", bma255},

drivers/iio/gyro/bmg160_i2c.c:static const struct acpi_device_id 
bmg160_acpi_match[] = {
drivers/iio/gyro/bmg160_i2c.c-  {"BMG0160", 0},
drivers/iio/gyro/bmg160_i2c.c-  {"BMI055B", 0},
drivers/iio/gyro/bmg160_i2c.c-  {},
drivers/iio/gyro/bmg160_i2c.c-};

drivers/iio/imu/bmi160/bmi160_i2c.c:static const struct acpi_device_id 
bmi160_acpi_match[] = {
drivers/iio/imu/bmi160/bmi160_i2c.c-{"BMI0160", 0},
drivers/iio/imu/bmi160/bmi160_i2c.c-{ },
drivers/iio/imu/bmi160/bmi160_i2c.c-};
drivers/iio/imu/bmi160/bmi160_i2c.c-MODULE_DEVICE_TABLE(acpi, 
bmi160_acpi_match);

Bosch registered ACPI ID: BOSC
http://www.uefi.org/ACPI_ID_List?search=Bosch

Bosch registered PNP ID: BSG
http://www.uefi.org/PNP_ID_List?search=Bosch

So, how could we use PNP ID "BMI" which is registered prefix for
BENSON MEDICAL INSTRUMENTS COMPANY ?
http://www.uefi.org/PNP_ID_List?search=BMI

Product ID -> "160" is fine for bmi160 which uniquely identifies the
sensor device.

But shouldn't the prefix be "BSG0160" instead of "BMI0160" ?

When I wrote the driver for Bosch BME680, I followed the same guideline
as done everywhere else in the Bosch family:

drivers/iio/pressure/bmp280-i2c.c-  {"BMP0280", BMP280_CHIP_ID },
drivers/iio/pressure/bmp280-i2c.c-  {"BMP0180", BMP180_CHIP_ID },
drivers/iio/pressure/bmp280-i2c.c-  {"BMP0085", BMP180_CHIP_ID },
drivers/iio/pressure/bmp280-i2c.c-  {"BME0280", BME280_CHIP_ID },

Therefore, I used BME0680 for bosch bme680 sensor!

In OF matching, we use vendor prefixes and that looks more legimate:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/vendor-prefixes.txt

Dan,

These questions are not just for you but to rest of the community
members as well.

If there is something I misunderstood, then please let me know :)


Thanks
-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Re: [PATCH v3 2/3] iio: adc128s052: add ACPI _HID AANT1280

[Himanshu Jha]

Hi Dan,

On Thu, Oct 25, 2018 at 04:35:41PM +0100, Dan O'Donovan wrote:
> From: Nicola Lunghi 
> 
> ACPI _HID AANT1280 matches an ADC124S101 present on E3940 SKUs of the UP
> Squared board.
> 
> Add it to the driver.
> 
> Signed-off-by: Nicola Lunghi 
> [jav...@emutex.com: fix up commit message and one checkpatch warning]
> Signed-off-by: Javier Arteaga 
> Signed-off-by: Dan O'Donovan 
> ---
>  drivers/iio/adc/ti-adc128s052.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)

[]

> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id adc128_acpi_match[] = {
> + { "AANT1280", 2 }, /* ADC124S021 compatible ACPI ID */
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, adc128_acpi_match);
> +#endif

I'm curious about the naming conventions used for selecting 
an ACPI ID.

AFAIK, ACPI or PNP ID consists of *Vendor* ID + Product ID.

PNP ID: PNP Vendor IDs consist of 3 characters, each character being
an uppercase letter (A-Z).

ACPI ID: ACPI Vendor IDs consist of 4 characters, each character being
either an uppercase letter (A-Z) or a numeral (0-9).

In your case,

Vendor ID -> AANT (AAEON TECHNOLOGY INC. registered ACPI ID prefix)
http://www.uefi.org/ACPI_ID_List?search=AANT

Product ID -> 1280

How did you come up with 1280 ? And why AANT ? why not TXNW which is
registered ACPI prefix for TEXAS INSTRUMENTS ?
http://www.uefi.org/ACPI_ID_List?search=Texas+

Latest ACPI manuals says:

6.1.5 _HID (Hardware ID)


This object is used to supply OSPM with the device’s Plug and Play
hardware ID.[1]

[1] "A Plug and Play ID or ACPI ID can be obtained by sending e-mail to
pn...@microsoft.com."

A _HID object evaluates to either a numeric 32-bit compressed EISA type ID or a 
string. If a
string, the format must be an alphanumeric PNP or ACPI ID with no asterisk or 
other leading
characters.

A valid PNP ID must be of the form "AAA" where A is an uppercase letter and 
# is a hex
digit. A valid ACPI ID must be of the form "" where N is an uppercase 
letter or a
digit ('0'-'9') and # is a hex digit. This specification reserves the string 
"ACPI" for use only
with devices defined herein. It further reserves all strings representing 4 HEX 
digits for
exclusive use with PCI-assigned Vendor IDs.


Next question:

There are a lot drivers in iio/ using ACPI for device enumeration using
these IDs. For now, let's take an example of Bosch Sensors:

drivers/iio/accel/bmc150-accel-i2c.c:static const struct acpi_device_id 
bmc150_accel_acpi_match[] = {
drivers/iio/accel/bmc150-accel-i2c.c-   {"BSBA0150",bmc150},
drivers/iio/accel/bmc150-accel-i2c.c-   {"BMC150A", bmc150},
drivers/iio/accel/bmc150-accel-i2c.c-   {"BMI055A", bmi055},
drivers/iio/accel/bmc150-accel-i2c.c-   {"BMA0255", bma255},

drivers/iio/gyro/bmg160_i2c.c:static const struct acpi_device_id 
bmg160_acpi_match[] = {
drivers/iio/gyro/bmg160_i2c.c-  {"BMG0160", 0},
drivers/iio/gyro/bmg160_i2c.c-  {"BMI055B", 0},
drivers/iio/gyro/bmg160_i2c.c-  {},
drivers/iio/gyro/bmg160_i2c.c-};

drivers/iio/imu/bmi160/bmi160_i2c.c:static const struct acpi_device_id 
bmi160_acpi_match[] = {
drivers/iio/imu/bmi160/bmi160_i2c.c-{"BMI0160", 0},
drivers/iio/imu/bmi160/bmi160_i2c.c-{ },
drivers/iio/imu/bmi160/bmi160_i2c.c-};
drivers/iio/imu/bmi160/bmi160_i2c.c-MODULE_DEVICE_TABLE(acpi, 
bmi160_acpi_match);

Bosch registered ACPI ID: BOSC
http://www.uefi.org/ACPI_ID_List?search=Bosch

Bosch registered PNP ID: BSG
http://www.uefi.org/PNP_ID_List?search=Bosch

So, how could we use PNP ID "BMI" which is registered prefix for
BENSON MEDICAL INSTRUMENTS COMPANY ?
http://www.uefi.org/PNP_ID_List?search=BMI

Product ID -> "160" is fine for bmi160 which uniquely identifies the
sensor device.

But shouldn't the prefix be "BSG0160" instead of "BMI0160" ?

When I wrote the driver for Bosch BME680, I followed the same guideline
as done everywhere else in the Bosch family:

drivers/iio/pressure/bmp280-i2c.c-  {"BMP0280", BMP280_CHIP_ID },
drivers/iio/pressure/bmp280-i2c.c-  {"BMP0180", BMP180_CHIP_ID },
drivers/iio/pressure/bmp280-i2c.c-  {"BMP0085", BMP180_CHIP_ID },
drivers/iio/pressure/bmp280-i2c.c-  {"BME0280", BME280_CHIP_ID },

Therefore, I used BME0680 for bosch bme680 sensor!

In OF matching, we use vendor prefixes and that looks more legimate:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/vendor-prefixes.txt

Dan,

These questions are not just for you but to rest of the community
members as well.

If there is something I misunderstood, then please let me know :)


Thanks
-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Gooday To You,

[Ali Hamadu]

  Gooday To You,

  Please i need your kind Assistance. I will be very glad if you can

  assist me to receive this sum of ( $22. Million US dollars.) into 
your

  bank account for the benefit of our both families, reply me if you 
are

  ready to receive this fund.

Gooday To You,

[Ali Hamadu]

  Gooday To You,

  Please i need your kind Assistance. I will be very glad if you can

  assist me to receive this sum of ( $22. Million US dollars.) into 
your

  bank account for the benefit of our both families, reply me if you 
are

  ready to receive this fund.

Gooday To You,

[Ali Hamadu]

  Gooday To You,

  Please i need your kind Assistance. I will be very glad if you can

  assist me to receive this sum of ( $22. Million US dollars.) into 
your

  bank account for the benefit of our both families, reply me if you 
are

  ready to receive this fund.

Gooday To You,

[Ali Hamadu]

  Gooday To You,

  Please i need your kind Assistance. I will be very glad if you can

  assist me to receive this sum of ( $22. Million US dollars.) into 
your

  bank account for the benefit of our both families, reply me if you 
are

  ready to receive this fund.

Re: [GIT PULL] RTC for 4.20

[Linus Torvalds]

On Fri, Oct 26, 2018 at 1:55 PM Alexandre Belloni
 wrote:
>
> Here is the pull-request for the RTC subsystem for 4.20.
>
> This cycle, there were mostly non urgent fixes in drivers. I also
> finally unexported the non managed registration.

Pulled,

   Linus

Re: [GIT PULL] RTC for 4.20

[Linus Torvalds]

On Fri, Oct 26, 2018 at 1:55 PM Alexandre Belloni
 wrote:
>
> Here is the pull-request for the RTC subsystem for 4.20.
>
> This cycle, there were mostly non urgent fixes in drivers. I also
> finally unexported the non managed registration.

Pulled,

   Linus

Re: Oops in current tree in i2c

[Linus Torvalds]

On Sat, Oct 27, 2018 at 9:08 AM Linus Torvalds
 wrote:
>
> I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> isn't terminated by a NULL entry, and I will test that next.

Confirmed.  That makes my laptop boot cleanly.

See commit b59dfdaef173 ("i2c-hid: properly terminate
i2c_hid_dmi_desc_override_table[] array")

   Linus

Re: Oops in current tree in i2c

[Linus Torvalds]

On Sat, Oct 27, 2018 at 9:08 AM Linus Torvalds
 wrote:
>
> I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> isn't terminated by a NULL entry, and I will test that next.

Confirmed.  That makes my laptop boot cleanly.

See commit b59dfdaef173 ("i2c-hid: properly terminate
i2c_hid_dmi_desc_override_table[] array")

   Linus

Re: Oops in current tree in i2c

[Jiri Kosina]

On Sat, 27 Oct 2018, Linus Torvalds wrote:

> I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> isn't terminated by a NULL entry, and I will test that next.

Hm, that almost certainly is indeed the issue, thanks a lot for reporting 
it.

> What makes me *very* unhappy about this is that if I'm right, I think it 
> means that code was literally not tested at all by anybody who didn't 
> have one of the entries in that list.

Honestly, my autotesting of HID tree is running on HW that doesn't have 
i2c transport at all, only USB a Bluetooth. Something to improve I guess; 
will fix that next week.

Benjamin, do you have something for that in place already?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: Oops in current tree in i2c

[Jiri Kosina]

On Sat, 27 Oct 2018, Linus Torvalds wrote:

> I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> isn't terminated by a NULL entry, and I will test that next.

Hm, that almost certainly is indeed the issue, thanks a lot for reporting 
it.

> What makes me *very* unhappy about this is that if I'm right, I think it 
> means that code was literally not tested at all by anybody who didn't 
> have one of the entries in that list.

Honestly, my autotesting of HID tree is running on HW that doesn't have 
i2c transport at all, only USB a Bluetooth. Something to improve I guess; 
will fix that next week.

Benjamin, do you have something for that in place already?

Thanks,

-- 
Jiri Kosina
SUSE Labs

RE: [Ksummit-discuss] The linux devs can rescind their license grant.

[Tim.Bird]

> -Original Message-
> From: Al Viro
> 
> On Sat, Oct 27, 2018 at 06:52:44AM +, visionsofal...@redchan.it wrote:
> > Al: the FSF was so insistent on the adoption of the GPL version 3
> > because the GPL version 2 is not operative against the grantor.
> 
> Anonymous wankstain: sod off and learn to troll properly.  It *is* an art
> form, and the one you are clearly not up to.
> 
> D-.  For the effort and successful use of spellchecker.  The style and
> contents are about F to E-, unfortunately, so take that in the spirit
> in which it is offered.  As a participation award, that is.

Al,
Can you please, even in the face of comments you find irritating, keep your 
responses
more civil?  Calling someone a "wankstain" is unprofessional, and we're trying
to raise the level of discourse here.

> 
> *plonk*
I think this part of the response was sufficient to communicate that you do not
take the suggestions of the other party seriously.  And it communicates
to others the right approach.  If someone thinks that another person is acting 
in bad
faith, I think it's better to just stop listening to that person, and let that 
person know
it, and to let other community members know it.   De-escalation is preferable to
engagement when working with someone who is acting in bad faith.

Although I disagree with the approach used in the top portion of this message, 
I am
grateful that you are committed to protecting Linux and our development 
community.

Regards,
 -- Tim

RE: [Ksummit-discuss] The linux devs can rescind their license grant.

[Tim.Bird]

> -Original Message-
> From: Al Viro
> 
> On Sat, Oct 27, 2018 at 06:52:44AM +, visionsofal...@redchan.it wrote:
> > Al: the FSF was so insistent on the adoption of the GPL version 3
> > because the GPL version 2 is not operative against the grantor.
> 
> Anonymous wankstain: sod off and learn to troll properly.  It *is* an art
> form, and the one you are clearly not up to.
> 
> D-.  For the effort and successful use of spellchecker.  The style and
> contents are about F to E-, unfortunately, so take that in the spirit
> in which it is offered.  As a participation award, that is.

Al,
Can you please, even in the face of comments you find irritating, keep your 
responses
more civil?  Calling someone a "wankstain" is unprofessional, and we're trying
to raise the level of discourse here.

> 
> *plonk*
I think this part of the response was sufficient to communicate that you do not
take the suggestions of the other party seriously.  And it communicates
to others the right approach.  If someone thinks that another person is acting 
in bad
faith, I think it's better to just stop listening to that person, and let that 
person know
it, and to let other community members know it.   De-escalation is preferable to
engagement when working with someone who is acting in bad faith.

Although I disagree with the approach used in the top portion of this message, 
I am
grateful that you are committed to protecting Linux and our development 
community.

Regards,
 -- Tim

Oops in current tree in i2c

[Linus Torvalds]

Julian, Jiri,
 On my laptop I'm getting a kernel page fault with the current git
tree, and I'm tentatively blaming commit

  9ee3e06610fd ("HID: i2c-hid: override HID descriptors for certain devices")

but that's simply because it's the only thing that seems to touch this
particular area in this merge window.

The oops looks like this:

  BUG: unable to handle kernel paging request at 7a25d598
  PGD 0 P4D 0
  Oops:  [#1] SMP PTI
  CPU: 1 PID: 888 Comm: systemd-udevd Not tainted 4.19.0-07715-g345671ea0f92 #4
  Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.7.0 01/16/2018
  RIP: 0010:strstr+0x19/0x70

where the code disassembly (and the register contents) shows that the
wild pointer is the first argument to "strstr()", which just has a
bogus value that is not a valid kernel pointer (RDI: 7a25d598
- which is obviously also the address of the page fault)

The call trace is:

   dmi_matches+0x55/0xc0
   dmi_first_match+0x26/0x40
   i2c_hid_get_dmi_i2c_hid_desc_override+0x16/0x40 [i2c_hid]
   i2c_hid_probe+0x28c/0x760 [i2c_hid]
   i2c_device_probe+0x1e7/0x260
   really_probe+0xf8/0x3e0
   driver_probe_device+0x10f/0x120
   bus_for_each_drv+0x66/0xb0
   __device_attach+0xd9/0x150
   bus_probe_device+0x8a/0xa0
   device_add+0x48e/0x660
   i2c_new_device+0x162/0x350

which is why I suspect that new i2c_hid_get_dmi_hid_report_desc_override() code.

I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
isn't terminated by a NULL entry, and I will test that next.

What makes me *very* unhappy about this is that if I'm right, I think
it means that code was literally not tested at all by anybody who
didn't have one of the entries in that list.

  Linus

Oops in current tree in i2c

[Linus Torvalds]

Julian, Jiri,
 On my laptop I'm getting a kernel page fault with the current git
tree, and I'm tentatively blaming commit

  9ee3e06610fd ("HID: i2c-hid: override HID descriptors for certain devices")

but that's simply because it's the only thing that seems to touch this
particular area in this merge window.

The oops looks like this:

  BUG: unable to handle kernel paging request at 7a25d598
  PGD 0 P4D 0
  Oops:  [#1] SMP PTI
  CPU: 1 PID: 888 Comm: systemd-udevd Not tainted 4.19.0-07715-g345671ea0f92 #4
  Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.7.0 01/16/2018
  RIP: 0010:strstr+0x19/0x70

where the code disassembly (and the register contents) shows that the
wild pointer is the first argument to "strstr()", which just has a
bogus value that is not a valid kernel pointer (RDI: 7a25d598
- which is obviously also the address of the page fault)

The call trace is:

   dmi_matches+0x55/0xc0
   dmi_first_match+0x26/0x40
   i2c_hid_get_dmi_i2c_hid_desc_override+0x16/0x40 [i2c_hid]
   i2c_hid_probe+0x28c/0x760 [i2c_hid]
   i2c_device_probe+0x1e7/0x260
   really_probe+0xf8/0x3e0
   driver_probe_device+0x10f/0x120
   bus_for_each_drv+0x66/0xb0
   __device_attach+0xd9/0x150
   bus_probe_device+0x8a/0xa0
   device_add+0x48e/0x660
   i2c_new_device+0x162/0x350

which is why I suspect that new i2c_hid_get_dmi_hid_report_desc_override() code.

I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
isn't terminated by a NULL entry, and I will test that next.

What makes me *very* unhappy about this is that if I'm right, I think
it means that code was literally not tested at all by anybody who
didn't have one of the entries in that list.

  Linus

Re: [PATCH v5 2/2] staging: iio: ad2s1210: Add device tree support.

[Slawomir Stepien]

Hi

On paź 26, 2018 18:55, Nishad Kamdar wrote:
> Add device tree table for matching vendor ID
> and support for retrieving platform data
> from device tree.

So maybe you should make 2 commits?

> Signed-off-by: Nishad Kamdar 
> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 43 -
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
> b/drivers/staging/iio/resolver/ad2s1210.c
> index 93c3c70ce62e..9fd5461c4ed0 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state 
> *st)
>   return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct ad2s1210_platform_data *pdata;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return NULL;
> +
> + pdata->gpioin = of_property_read_bool(np, "adi,gpioin");

Why here you are adding "adi", but you are not adding it to the gpios in prev
commit?

I've also seen this: https://patchwork.kernel.org/patch/10355839/. However I do
not understand why adding vendor id to props is needed...

> +
> + return pdata;
> +}
> +#else
> +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)
> +{
> + return NULL;
> +}
> +#endif
> +
>  static int ad2s1210_probe(struct spi_device *spi)
>  {
>   struct iio_dev *indio_dev;
> @@ -682,7 +704,19 @@ static int ad2s1210_probe(struct spi_device *spi)
>   if (!indio_dev)
>   return -ENOMEM;
>   st = iio_priv(indio_dev);
> - st->pdata = spi->dev.platform_data;
> + if (spi->dev.of_node) {
> + st->pdata = ad2s1210_parse_dt(>dev);
> + if (!st->pdata)
> + return -EINVAL;
> + } else {
> + st->pdata = spi->dev.platform_data;
> + }
> +
> + if (!st->pdata) {
> + dev_err(>dev, "ad2s1210: no platform data supplied\n");
> + return -EINVAL;
> + }
> +

Why not just use only device-tree here? The ad2s1210_platform_data has now only
one entry... In that case remember to add "depends on OF" in Kconfig.

>   ret = ad2s1210_setup_gpios(st);
>   if (ret < 0)
>   return ret;
> @@ -724,6 +758,12 @@ static int ad2s1210_remove(struct spi_device *spi)
>   return 0;
>  }
>  
> +static const struct of_device_id ad2s1210_of_match[] = {
> + { .compatible = "adi,ad2s1210", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad2s1210_of_match);
> +
>  static const struct spi_device_id ad2s1210_id[] = {
>   { "ad2s1210" },
>   {}
> @@ -733,6 +773,7 @@ MODULE_DEVICE_TABLE(spi, ad2s1210_id);
>  static struct spi_driver ad2s1210_driver = {
>   .driver = {
>   .name = DRV_NAME,
> + .of_match_table = of_match_ptr(ad2s1210_of_match),
>   },
>   .probe = ad2s1210_probe,
>   .remove = ad2s1210_remove,

-- 
Slawomir Stepien

Re: [PATCH v5 2/2] staging: iio: ad2s1210: Add device tree support.

[Slawomir Stepien]

Hi

On paź 26, 2018 18:55, Nishad Kamdar wrote:
> Add device tree table for matching vendor ID
> and support for retrieving platform data
> from device tree.

So maybe you should make 2 commits?

> Signed-off-by: Nishad Kamdar 
> ---
>  drivers/staging/iio/resolver/ad2s1210.c | 43 -
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s1210.c 
> b/drivers/staging/iio/resolver/ad2s1210.c
> index 93c3c70ce62e..9fd5461c4ed0 100644
> --- a/drivers/staging/iio/resolver/ad2s1210.c
> +++ b/drivers/staging/iio/resolver/ad2s1210.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -669,6 +670,27 @@ static int ad2s1210_setup_gpios(struct ad2s1210_state 
> *st)
>   return 0;
>  }
>  
> +#ifdef CONFIG_OF
> +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct ad2s1210_platform_data *pdata;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata)
> + return NULL;
> +
> + pdata->gpioin = of_property_read_bool(np, "adi,gpioin");

Why here you are adding "adi", but you are not adding it to the gpios in prev
commit?

I've also seen this: https://patchwork.kernel.org/patch/10355839/. However I do
not understand why adding vendor id to props is needed...

> +
> + return pdata;
> +}
> +#else
> +static struct ad2s1210_platform_data *ad2s1210_parse_dt(struct device *dev)
> +{
> + return NULL;
> +}
> +#endif
> +
>  static int ad2s1210_probe(struct spi_device *spi)
>  {
>   struct iio_dev *indio_dev;
> @@ -682,7 +704,19 @@ static int ad2s1210_probe(struct spi_device *spi)
>   if (!indio_dev)
>   return -ENOMEM;
>   st = iio_priv(indio_dev);
> - st->pdata = spi->dev.platform_data;
> + if (spi->dev.of_node) {
> + st->pdata = ad2s1210_parse_dt(>dev);
> + if (!st->pdata)
> + return -EINVAL;
> + } else {
> + st->pdata = spi->dev.platform_data;
> + }
> +
> + if (!st->pdata) {
> + dev_err(>dev, "ad2s1210: no platform data supplied\n");
> + return -EINVAL;
> + }
> +

Why not just use only device-tree here? The ad2s1210_platform_data has now only
one entry... In that case remember to add "depends on OF" in Kconfig.

>   ret = ad2s1210_setup_gpios(st);
>   if (ret < 0)
>   return ret;
> @@ -724,6 +758,12 @@ static int ad2s1210_remove(struct spi_device *spi)
>   return 0;
>  }
>  
> +static const struct of_device_id ad2s1210_of_match[] = {
> + { .compatible = "adi,ad2s1210", },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, ad2s1210_of_match);
> +
>  static const struct spi_device_id ad2s1210_id[] = {
>   { "ad2s1210" },
>   {}
> @@ -733,6 +773,7 @@ MODULE_DEVICE_TABLE(spi, ad2s1210_id);
>  static struct spi_driver ad2s1210_driver = {
>   .driver = {
>   .name = DRV_NAME,
> + .of_match_table = of_match_ptr(ad2s1210_of_match),
>   },
>   .probe = ad2s1210_probe,
>   .remove = ad2s1210_remove,

-- 
Slawomir Stepien

Re: [PATCH v2] mtd: spi-nor: Add support for SPI boot flash access for AMD Family 16h

[Boris Brezillon]

Hi Brett,

On Tue, 16 Oct 2018 00:57:41 +
"Grandbois, Brett"  wrote:

> Add support to expose the SPI boot flash on AMD Family 16h CPUs as a
> standard mtd device to give userspace BIOS updaters greater feature
> support.  The BIOS and Kernel Developer's Guide refers to this as the
> 'SPI ROM' controller and so the driver follows that naming convention
> for consistency.
> 

We're currently trying to convert spi-nor controller drivers to the
spi-mem interface [1]. Can you look at this new interface and tell me if
you'd be able to implement it? If that's not possible, then I'd prefer
to have this driver implement the mtd_info interface directly.

Thanks,

Boris

[1]https://elixir.bootlin.com/linux/latest/source/include/linux/spi/spi-mem.h#L185

Re: [PATCH v2] mtd: spi-nor: Add support for SPI boot flash access for AMD Family 16h

[Boris Brezillon]

Hi Brett,

On Tue, 16 Oct 2018 00:57:41 +
"Grandbois, Brett"  wrote:

> Add support to expose the SPI boot flash on AMD Family 16h CPUs as a
> standard mtd device to give userspace BIOS updaters greater feature
> support.  The BIOS and Kernel Developer's Guide refers to this as the
> 'SPI ROM' controller and so the driver follows that naming convention
> for consistency.
> 

We're currently trying to convert spi-nor controller drivers to the
spi-mem interface [1]. Can you look at this new interface and tell me if
you'd be able to implement it? If that's not possible, then I'd prefer
to have this driver implement the mtd_info interface directly.

Thanks,

Boris

[1]https://elixir.bootlin.com/linux/latest/source/include/linux/spi/spi-mem.h#L185

Re: [PATCH v2 1/3] namei: implement O_BENEATH-style AT_* flags

[Aleksa Sarai]

On 2018-10-27, Al Viro  wrote:
> On Sat, Oct 27, 2018 at 06:17:29PM +1100, Aleksa Sarai wrote:
> 
> > I'm going to send out a v4 "soon" but I would like to know what folks
> > think about having resolveat(2) (or similar) to separate the scoping O_*
> > flags and produce an O_PATH -- since unsupported O_* flags are ignored
> > by older kernels userspace will have to do some plenty of checking after
> > each path operation.
> > 
> > Personally, I believe this (along with AT_EMPTY_PATH for openat(2))
> > would help with some other O_PATH issues.
> 
> The trouble with resolveat(2) is that for anything directory-modifying
> you really want directory locked before the lookup for last component.
> IOW, funlink(2) et.al. are hopeless.

Ah, right... In those cases I think that AT_SYMLINK_NOFOLLOW could help,
or maybe we would need to have some of the scoping flags for other
syscalls (though this would be an issue in either case for scoping
unlinkat(2) -- even if we used O_BENEATH). :/

But my main issue at the moment with O_PATH is that /proc/self/fd/...
re-opening allows for some very odd delayed-access-check attacks.
openat(2) doesn't give you an O_EMPTYPATH but that is what you get with
/proc.

For instance, take /proc/self/exe. Tautologically, it is impossible to
open it O_RDWR -- if you are resolving it through an "exe" magic symlink
then it is being used as a process's ->mm (and thus is locked from
writing). *However* you can open it O_PATH and then later re-open it
through /proc/self/fd. We had cases where a container runtime joining a
container would be able to do this and overwrite the container binary
*on the host*. This has been mitigated now (as part of CVE-2016-9962),
but I would be very shocked if there was no other places where this sort
of thing would happen.

My proposal for resolveat(2) would let us have some sort of "I want
these access bits" API for O_PATH. Of course there are some quite
not-nice changes I think you'd need to allow for this usecase -- my
back-of-the-envelope proposal would be to stash away the fmode bits
inside 'struct path' so that do_last() can see whether we are doing a
re-open of an existing 'struct file' (but I'm sure this sounds awful).

Is this a problem you think deserves solving / is there a better way of
going about it? Thanks.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature

Re: [PATCH v2 1/3] namei: implement O_BENEATH-style AT_* flags

[Aleksa Sarai]

On 2018-10-27, Al Viro  wrote:
> On Sat, Oct 27, 2018 at 06:17:29PM +1100, Aleksa Sarai wrote:
> 
> > I'm going to send out a v4 "soon" but I would like to know what folks
> > think about having resolveat(2) (or similar) to separate the scoping O_*
> > flags and produce an O_PATH -- since unsupported O_* flags are ignored
> > by older kernels userspace will have to do some plenty of checking after
> > each path operation.
> > 
> > Personally, I believe this (along with AT_EMPTY_PATH for openat(2))
> > would help with some other O_PATH issues.
> 
> The trouble with resolveat(2) is that for anything directory-modifying
> you really want directory locked before the lookup for last component.
> IOW, funlink(2) et.al. are hopeless.

Ah, right... In those cases I think that AT_SYMLINK_NOFOLLOW could help,
or maybe we would need to have some of the scoping flags for other
syscalls (though this would be an issue in either case for scoping
unlinkat(2) -- even if we used O_BENEATH). :/

But my main issue at the moment with O_PATH is that /proc/self/fd/...
re-opening allows for some very odd delayed-access-check attacks.
openat(2) doesn't give you an O_EMPTYPATH but that is what you get with
/proc.

For instance, take /proc/self/exe. Tautologically, it is impossible to
open it O_RDWR -- if you are resolving it through an "exe" magic symlink
then it is being used as a process's ->mm (and thus is locked from
writing). *However* you can open it O_PATH and then later re-open it
through /proc/self/fd. We had cases where a container runtime joining a
container would be able to do this and overwrite the container binary
*on the host*. This has been mitigated now (as part of CVE-2016-9962),
but I would be very shocked if there was no other places where this sort
of thing would happen.

My proposal for resolveat(2) would let us have some sort of "I want
these access bits" API for O_PATH. Of course there are some quite
not-nice changes I think you'd need to allow for this usecase -- my
back-of-the-envelope proposal would be to stash away the fmode bits
inside 'struct path' so that do_last() can see whether we are doing a
re-open of an existing 'struct file' (but I'm sure this sounds awful).

Is this a problem you think deserves solving / is there a better way of
going about it? Thanks.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature

[PATCH v7] i2c: Add PCI and platform drivers for the AMD MP2 I2C controller

[Elie Morisse]

This contains two drivers:
 * i2c-amd-plat-mp2: platform driver managing an i2c adapter (one of
the two busses of the MP2) and routing any i2c read/write command to
the PCI driver.
 * i2c-amd-pci-mp2: PCI driver communicating through the C2P/P2C
mailbox registers, or through DMA for more than 32 bytes transfers.

This is major rework of the patch submitted by Nehal-bakulchandra Shah
from AMD (https://patchwork.kernel.org/patch/10597369/).

Most of the event handling of v2/v3 was rewritten since it couldn't work
if more than one bus was enabled, and contains many more fixes listed
in the patch changelog.

With those changes both the touchpad and the touchscreen of the
Ryzen-based Lenovo Yoga 530 which lie in separate busses work beautifully.

Signed-off-by: Elie Morisse 
---
Changes since v1:(https://www.spinics.net/lists/linux-i2c/msg34650.html)
-> Add fix for IOMMU
-> Add depedency of ACPI
-> Add locks to avoid the crash

Changes since v2:(https://patchwork.ozlabs.org/patch/961270/)

-> fix for review comments
-> fix for more than 32 bytes write issue

Changes since v3 (https://patchwork.kernel.org/patch/10597369/) by Elie M.:

-> support more than one bus/adapter
-> support more than one slave per bus
-> use the bus speed specified by the slaves declared in the DSDT instead of
   assuming speed == 400kbits/s
-> instead of kzalloc'ing a buffer for every less than 32 bytes reads, simply
   use i2c_msg.buf
-> fix buffer overreads/overflows when (<=32 bytes) message lengths aren't a
   multiple of 4 by using memcpy_fromio and memcpy_toio
-> use streaming DMA mappings instead of allocating a coherent DMA buffer for
   every >32 bytes read/write
-> properly check for timeouts during i2c_amd_xfer and increase it from 50
   jiffies to 250 msecs (which is more in line with other drivers)
-> complete amd_i2c_dev.msg even if the device doesn't return a xxx_success
   event, instead of stalling i2c_amd_xfer
-> removed the spinlock and mdelay during i2c_amd_pci_configure, I didn't see
   the point since it's already waiting for a i2c_busenable_complete event
-> add an adapter-specific mutex lock for i2c_amd_xfer, since we don't want
   parallel calls writing to AMD_C2P_MSG0 (or AMD_C2P_MSG1)
-> add a global mutex lock for registers AMD_C2P_MSG2 to AMD_C2P_MSG9,  which
   are shared across the two busses/adapters
-> add MODULE_DEVICE_TABLE to automatically load i2c-amd-platdrv if the DSDT
   enumerates devices with the "AMDI0011" HID
-> set maximum length of reads/writes to 4095 (event's length field is 12 bits)
-> basic PM support
-> style corrections to match the kernel code style, and tried to reduce code
   duplication whenever possible

Changes since v4 (https://marc.info/?l=linux-kernel=154031133019835) by Elie 
M.:

-> fix missing typecast warning
-> removed the duplicated entry in Kconfig

Changes since v5 by Elie M.:

-> move DMA mapping from the platform driver to the PCI driver
-> attempt to find the platform device's PCI parent through the _DEP ACPI method
   (if not found take the first MP2 device registred in the i2c-amd-pci-mp2
   driver, like before)
-> do not assume anymore that the PCI device is owned by the i2c-amd-pci-mp2
   driver
-> address other review comments by Bjorn Helgaas (meant for v3)

Changes since v6 by Elie M.:

-> remove unnecessary memcpy from the DMA buffer during i2c_amd_read_completion

 MAINTAINERS   |   6 +
 drivers/i2c/busses/Kconfig|  15 +
 drivers/i2c/busses/Makefile   |   2 +
 drivers/i2c/busses/i2c-amd-pci-mp2.c  | 706 ++
 drivers/i2c/busses/i2c-amd-pci-mp2.h  | 224 
 drivers/i2c/busses/i2c-amd-plat-mp2.c | 373 ++
 6 files changed, 1326 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
 create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
 create mode 100644 drivers/i2c/busses/i2c-amd-plat-mp2.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6ac000cc006d..8ff2bddc1ac2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -791,6 +791,12 @@ F: drivers/gpu/drm/amd/include/vi_structs.h
 F: drivers/gpu/drm/amd/include/v9_structs.h
 F: include/uapi/linux/kfd_ioctl.h
 
+AMD MP2 I2C DRIVER
+M: Elie Morisse 
+L: linux-...@vger.kernel.org
+S: Maintained
+F: drivers/i2c/busses/i2c-amd-*-mp2*
+
 AMD POWERPLAY
 M: Rex Zhu 
 M: Evan Quan 
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 451d4ae50e66..e20f2d1ce381 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -77,6 +77,21 @@ config I2C_AMD8111
  This driver can also be built as a module.  If so, the module
  will be called i2c-amd8111.
 
+config I2C_AMD_MP2_PCI
+   tristate
+   depends on PCI
+
+config I2C_AMD_MP2_PLATFORM
+   tristate "AMD MP2 Platform"
+   select I2C_AMD_MP2_PCI
+   depends on ACPI
+   help
+ If you say yes to this option, support will be included for the AMD 
MP2
+ PCI I2C 

[PATCH v7] i2c: Add PCI and platform drivers for the AMD MP2 I2C controller

[Elie Morisse]

This contains two drivers:
 * i2c-amd-plat-mp2: platform driver managing an i2c adapter (one of
the two busses of the MP2) and routing any i2c read/write command to
the PCI driver.
 * i2c-amd-pci-mp2: PCI driver communicating through the C2P/P2C
mailbox registers, or through DMA for more than 32 bytes transfers.

This is major rework of the patch submitted by Nehal-bakulchandra Shah
from AMD (https://patchwork.kernel.org/patch/10597369/).

Most of the event handling of v2/v3 was rewritten since it couldn't work
if more than one bus was enabled, and contains many more fixes listed
in the patch changelog.

With those changes both the touchpad and the touchscreen of the
Ryzen-based Lenovo Yoga 530 which lie in separate busses work beautifully.

Signed-off-by: Elie Morisse 
---
Changes since v1:(https://www.spinics.net/lists/linux-i2c/msg34650.html)
-> Add fix for IOMMU
-> Add depedency of ACPI
-> Add locks to avoid the crash

Changes since v2:(https://patchwork.ozlabs.org/patch/961270/)

-> fix for review comments
-> fix for more than 32 bytes write issue

Changes since v3 (https://patchwork.kernel.org/patch/10597369/) by Elie M.:

-> support more than one bus/adapter
-> support more than one slave per bus
-> use the bus speed specified by the slaves declared in the DSDT instead of
   assuming speed == 400kbits/s
-> instead of kzalloc'ing a buffer for every less than 32 bytes reads, simply
   use i2c_msg.buf
-> fix buffer overreads/overflows when (<=32 bytes) message lengths aren't a
   multiple of 4 by using memcpy_fromio and memcpy_toio
-> use streaming DMA mappings instead of allocating a coherent DMA buffer for
   every >32 bytes read/write
-> properly check for timeouts during i2c_amd_xfer and increase it from 50
   jiffies to 250 msecs (which is more in line with other drivers)
-> complete amd_i2c_dev.msg even if the device doesn't return a xxx_success
   event, instead of stalling i2c_amd_xfer
-> removed the spinlock and mdelay during i2c_amd_pci_configure, I didn't see
   the point since it's already waiting for a i2c_busenable_complete event
-> add an adapter-specific mutex lock for i2c_amd_xfer, since we don't want
   parallel calls writing to AMD_C2P_MSG0 (or AMD_C2P_MSG1)
-> add a global mutex lock for registers AMD_C2P_MSG2 to AMD_C2P_MSG9,  which
   are shared across the two busses/adapters
-> add MODULE_DEVICE_TABLE to automatically load i2c-amd-platdrv if the DSDT
   enumerates devices with the "AMDI0011" HID
-> set maximum length of reads/writes to 4095 (event's length field is 12 bits)
-> basic PM support
-> style corrections to match the kernel code style, and tried to reduce code
   duplication whenever possible

Changes since v4 (https://marc.info/?l=linux-kernel=154031133019835) by Elie 
M.:

-> fix missing typecast warning
-> removed the duplicated entry in Kconfig

Changes since v5 by Elie M.:

-> move DMA mapping from the platform driver to the PCI driver
-> attempt to find the platform device's PCI parent through the _DEP ACPI method
   (if not found take the first MP2 device registred in the i2c-amd-pci-mp2
   driver, like before)
-> do not assume anymore that the PCI device is owned by the i2c-amd-pci-mp2
   driver
-> address other review comments by Bjorn Helgaas (meant for v3)

Changes since v6 by Elie M.:

-> remove unnecessary memcpy from the DMA buffer during i2c_amd_read_completion

 MAINTAINERS   |   6 +
 drivers/i2c/busses/Kconfig|  15 +
 drivers/i2c/busses/Makefile   |   2 +
 drivers/i2c/busses/i2c-amd-pci-mp2.c  | 706 ++
 drivers/i2c/busses/i2c-amd-pci-mp2.h  | 224 
 drivers/i2c/busses/i2c-amd-plat-mp2.c | 373 ++
 6 files changed, 1326 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
 create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
 create mode 100644 drivers/i2c/busses/i2c-amd-plat-mp2.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6ac000cc006d..8ff2bddc1ac2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -791,6 +791,12 @@ F: drivers/gpu/drm/amd/include/vi_structs.h
 F: drivers/gpu/drm/amd/include/v9_structs.h
 F: include/uapi/linux/kfd_ioctl.h
 
+AMD MP2 I2C DRIVER
+M: Elie Morisse 
+L: linux-...@vger.kernel.org
+S: Maintained
+F: drivers/i2c/busses/i2c-amd-*-mp2*
+
 AMD POWERPLAY
 M: Rex Zhu 
 M: Evan Quan 
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 451d4ae50e66..e20f2d1ce381 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -77,6 +77,21 @@ config I2C_AMD8111
  This driver can also be built as a module.  If so, the module
  will be called i2c-amd8111.
 
+config I2C_AMD_MP2_PCI
+   tristate
+   depends on PCI
+
+config I2C_AMD_MP2_PLATFORM
+   tristate "AMD MP2 Platform"
+   select I2C_AMD_MP2_PCI
+   depends on ACPI
+   help
+ If you say yes to this option, support will be included for the AMD 
MP2
+ PCI I2C