Re: Sed-opal fixups

[Scott Bauer]

On Thu, Feb 09, 2017 at 05:43:20PM +, David Laight wrote:
> From: Scott Bauer
> > Sent: 09 February 2017 17:20
> > It may be too late to change anyhting in the uapi header. When we
> > switched over to using IOC_SIZE I found a bug where I had switched
> > up a structure in one of the series from v4 to v5 but never changed
> > the structure in the IOW. The structure that was in there was to small
> > so when we kzalloc on it we don't request enough space. It worked before
> > because we were using the cmd strictly as a command #, not using the IOC
> > and friends.
> > 
> > If it's too late to modify that IOW, I can work around it by reallocing
> > on the correct size for that command only. I verified the rest of the
> > commands and the structures are the same.
> > 
> > Let me know what you think, please.
> 
> Maybe define IOC_OPAL_ACTIVATE_LSP_OLD to the incorrect value and
> IOC_OPAL_ACTIVATE_LSP to the correct one.
> But that relies on any users specifying the correct structure.
> I wouldn't guarantee that.

I think I'm the only userspace user right now, this went in on monday,
so I can can change my tooling easily. I just wasnt sure if there was a
set time where the user ABI cannot be changed.

> 
> At the top of the driver's ioctl path add:
>   if (cmd == IOC_OPAL_ACTIVATE_LSP_OLD) cmd = IOC_OPAL_ACTIVATE_LSP;
>

I think it would have to be the other way around the correct sized one would
be IOC_OPAL_ACTIAVE_LSP_NEW so the check would be:
if (cmd == IOC_OPAL_ACTIVATE_LSP) cmd = IOC_OPAL_ACTIVATE_LSP_NEW. If we're
allowed to change it (the bad sized one) from LSP to LSP_OLD then we should
just change the structure. If we have to leave it we need to introduce a _NEW
with the correct size.


> For some code I added a userspace wrapper on ioctl() to check the
> size of the supplied arg matched that required by the 'cmd'.
> I've also done the same in the kernel.
> (all as compile time checks).
> 
>   David
> 
> 

Re: Sed-opal fixups

[Scott Bauer]

On Thu, Feb 09, 2017 at 05:43:20PM +, David Laight wrote:
> From: Scott Bauer
> > Sent: 09 February 2017 17:20
> > It may be too late to change anyhting in the uapi header. When we
> > switched over to using IOC_SIZE I found a bug where I had switched
> > up a structure in one of the series from v4 to v5 but never changed
> > the structure in the IOW. The structure that was in there was to small
> > so when we kzalloc on it we don't request enough space. It worked before
> > because we were using the cmd strictly as a command #, not using the IOC
> > and friends.
> > 
> > If it's too late to modify that IOW, I can work around it by reallocing
> > on the correct size for that command only. I verified the rest of the
> > commands and the structures are the same.
> > 
> > Let me know what you think, please.
> 
> Maybe define IOC_OPAL_ACTIVATE_LSP_OLD to the incorrect value and
> IOC_OPAL_ACTIVATE_LSP to the correct one.
> But that relies on any users specifying the correct structure.
> I wouldn't guarantee that.

I think I'm the only userspace user right now, this went in on monday,
so I can can change my tooling easily. I just wasnt sure if there was a
set time where the user ABI cannot be changed.

> 
> At the top of the driver's ioctl path add:
>   if (cmd == IOC_OPAL_ACTIVATE_LSP_OLD) cmd = IOC_OPAL_ACTIVATE_LSP;
>

I think it would have to be the other way around the correct sized one would
be IOC_OPAL_ACTIAVE_LSP_NEW so the check would be:
if (cmd == IOC_OPAL_ACTIVATE_LSP) cmd = IOC_OPAL_ACTIVATE_LSP_NEW. If we're
allowed to change it (the bad sized one) from LSP to LSP_OLD then we should
just change the structure. If we have to leave it we need to introduce a _NEW
with the correct size.


> For some code I added a userspace wrapper on ioctl() to check the
> size of the supplied arg matched that required by the 'cmd'.
> I've also done the same in the kernel.
> (all as compile time checks).
> 
>   David
> 
> 

[RFC] mm/hugetlb: use mem policy when allocating surplus huge pages

[Grzegorz Andrejczuk]

Application allocating overcommitted hugepages behave differently when
its mempolicy is set to bind with NUMA nodes containing CPUs and not
containing CPUs. When memory is allocated on node with CPUs everything
work as expected, when memory is allocated on CPU-less node:
1. Some memory is allocated from node with CPUs.
2. Application is terminated with SIGBUS (due to touching not allocated
   page).

Reproduction (Node0: 90GB, 272 logical CPUs; Node1: 16GB, No CPUs):
int
main()
{
  char *p = (char*)mmap(0, 4*1024*1024, PROT_READ|PROT_WRITE,
 MAP_PRIVATE|MAP_ANONYMOUS|MAP_HUGETLB, 0, 0);
  *p = 0;
  p += 2*1024*1024;
  *p=0;
  return  0;
}

echo 2 > /proc/sys/vm/nr_overcommit_hugepages
numactl -m 0 ./test #works
numactl -m 1 ./test #sigbus

The reason for this behavior is hugetlb_reserve_pages(...) omits
struct vm_area when calling hugetlb_acct_pages(..) and later allocation is
unable to determine memory policy.

To fix this issue memory policy is forwarded from hugetlb_reserved_pages
to allocation routine.
When policy is interleave, NUMA Node is computed by:
  page address >> huge_page_shift() % interleaved nodes count.

This algorithm assumes that address is known, but in this case address
is not known so to keep interleave working without it, dummy address is
computed as vm_start + (1 << huge_page_shift())*n, where n is allocated
page number.

Signed-off-by: Grzegorz Andrejczuk 
---
 mm/hugetlb.c | 49 +++--
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 418bf01..3913066 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -67,7 +67,8 @@ static int num_fault_mutexes;
 struct mutex *hugetlb_fault_mutex_table cacheline_aligned_in_smp;
 
 /* Forward declaration */
-static int hugetlb_acct_memory(struct hstate *h, long delta);
+static int hugetlb_acct_memory(struct hstate *h, long delta,
+  struct vm_area_struct *vma);
 
 static inline void unlock_or_release_subpool(struct hugepage_subpool *spool)
 {
@@ -81,7 +82,7 @@ static inline void unlock_or_release_subpool(struct 
hugepage_subpool *spool)
if (free) {
if (spool->min_hpages != -1)
hugetlb_acct_memory(spool->hstate,
-   -spool->min_hpages);
+   -spool->min_hpages, NULL);
kfree(spool);
}
 }
@@ -101,7 +102,7 @@ struct hugepage_subpool *hugepage_new_subpool(struct hstate 
*h, long max_hpages,
spool->hstate = h;
spool->min_hpages = min_hpages;
 
-   if (min_hpages != -1 && hugetlb_acct_memory(h, min_hpages)) {
+   if (min_hpages != -1 && hugetlb_acct_memory(h, min_hpages, NULL)) {
kfree(spool);
return NULL;
}
@@ -576,7 +577,7 @@ void hugetlb_fix_reserve_counts(struct inode *inode)
if (rsv_adjust) {
struct hstate *h = hstate_inode(inode);
 
-   hugetlb_acct_memory(h, 1);
+   hugetlb_acct_memory(h, 1, NULL);
}
 }
 
@@ -1690,10 +1691,12 @@ struct page *alloc_huge_page_node(struct hstate *h, int 
nid)
  * Increase the hugetlb pool such that it can accommodate a reservation
  * of size 'delta'.
  */
-static int gather_surplus_pages(struct hstate *h, int delta)
+static int gather_surplus_pages(struct hstate *h, int delta,
+   struct vm_area_struct *vma)
 {
struct list_head surplus_list;
struct page *page, *tmp;
+   unsigned long address_offset = 0;
int ret, i;
int needed, allocated;
bool alloc_ok = true;
@@ -1711,7 +1714,20 @@ static int gather_surplus_pages(struct hstate *h, int 
delta)
 retry:
spin_unlock(_lock);
for (i = 0; i < needed; i++) {
-   page = __alloc_buddy_huge_page_no_mpol(h, NUMA_NO_NODE);
+   if (vma) {
+   unsigned long dummy_addr = vma->vm_start +
+   (address_offset << huge_page_shift(h));
+
+   if (dummy_addr >= vma->vm_end) {
+   address_offset = 0;
+   dummy_addr = vma->vm_start;
+   }
+   page = __alloc_buddy_huge_page_with_mpol(h, vma,
+dummy_addr);
+   address_offset++;
+   } else {
+   page = __alloc_buddy_huge_page_no_mpol(h, NUMA_NO_NODE);
+   }
if (!page) {
alloc_ok = false;
break;
@@ -2057,7 +2073,7 @@ struct page *alloc_huge_page(struct vm_area_struct *vma,
long rsv_adjust;
 
rsv_adjust = hugepage_subpool_put_pages(spool, 1);
-   hugetlb_acct_memory(h, -rsv_adjust);
+   

[RFC] mm/hugetlb: use mem policy when allocating surplus huge pages

[Grzegorz Andrejczuk]

Application allocating overcommitted hugepages behave differently when
its mempolicy is set to bind with NUMA nodes containing CPUs and not
containing CPUs. When memory is allocated on node with CPUs everything
work as expected, when memory is allocated on CPU-less node:
1. Some memory is allocated from node with CPUs.
2. Application is terminated with SIGBUS (due to touching not allocated
   page).

Reproduction (Node0: 90GB, 272 logical CPUs; Node1: 16GB, No CPUs):
int
main()
{
  char *p = (char*)mmap(0, 4*1024*1024, PROT_READ|PROT_WRITE,
 MAP_PRIVATE|MAP_ANONYMOUS|MAP_HUGETLB, 0, 0);
  *p = 0;
  p += 2*1024*1024;
  *p=0;
  return  0;
}

echo 2 > /proc/sys/vm/nr_overcommit_hugepages
numactl -m 0 ./test #works
numactl -m 1 ./test #sigbus

The reason for this behavior is hugetlb_reserve_pages(...) omits
struct vm_area when calling hugetlb_acct_pages(..) and later allocation is
unable to determine memory policy.

To fix this issue memory policy is forwarded from hugetlb_reserved_pages
to allocation routine.
When policy is interleave, NUMA Node is computed by:
  page address >> huge_page_shift() % interleaved nodes count.

This algorithm assumes that address is known, but in this case address
is not known so to keep interleave working without it, dummy address is
computed as vm_start + (1 << huge_page_shift())*n, where n is allocated
page number.

Signed-off-by: Grzegorz Andrejczuk 
---
 mm/hugetlb.c | 49 +++--
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 418bf01..3913066 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -67,7 +67,8 @@ static int num_fault_mutexes;
 struct mutex *hugetlb_fault_mutex_table cacheline_aligned_in_smp;
 
 /* Forward declaration */
-static int hugetlb_acct_memory(struct hstate *h, long delta);
+static int hugetlb_acct_memory(struct hstate *h, long delta,
+  struct vm_area_struct *vma);
 
 static inline void unlock_or_release_subpool(struct hugepage_subpool *spool)
 {
@@ -81,7 +82,7 @@ static inline void unlock_or_release_subpool(struct 
hugepage_subpool *spool)
if (free) {
if (spool->min_hpages != -1)
hugetlb_acct_memory(spool->hstate,
-   -spool->min_hpages);
+   -spool->min_hpages, NULL);
kfree(spool);
}
 }
@@ -101,7 +102,7 @@ struct hugepage_subpool *hugepage_new_subpool(struct hstate 
*h, long max_hpages,
spool->hstate = h;
spool->min_hpages = min_hpages;
 
-   if (min_hpages != -1 && hugetlb_acct_memory(h, min_hpages)) {
+   if (min_hpages != -1 && hugetlb_acct_memory(h, min_hpages, NULL)) {
kfree(spool);
return NULL;
}
@@ -576,7 +577,7 @@ void hugetlb_fix_reserve_counts(struct inode *inode)
if (rsv_adjust) {
struct hstate *h = hstate_inode(inode);
 
-   hugetlb_acct_memory(h, 1);
+   hugetlb_acct_memory(h, 1, NULL);
}
 }
 
@@ -1690,10 +1691,12 @@ struct page *alloc_huge_page_node(struct hstate *h, int 
nid)
  * Increase the hugetlb pool such that it can accommodate a reservation
  * of size 'delta'.
  */
-static int gather_surplus_pages(struct hstate *h, int delta)
+static int gather_surplus_pages(struct hstate *h, int delta,
+   struct vm_area_struct *vma)
 {
struct list_head surplus_list;
struct page *page, *tmp;
+   unsigned long address_offset = 0;
int ret, i;
int needed, allocated;
bool alloc_ok = true;
@@ -1711,7 +1714,20 @@ static int gather_surplus_pages(struct hstate *h, int 
delta)
 retry:
spin_unlock(_lock);
for (i = 0; i < needed; i++) {
-   page = __alloc_buddy_huge_page_no_mpol(h, NUMA_NO_NODE);
+   if (vma) {
+   unsigned long dummy_addr = vma->vm_start +
+   (address_offset << huge_page_shift(h));
+
+   if (dummy_addr >= vma->vm_end) {
+   address_offset = 0;
+   dummy_addr = vma->vm_start;
+   }
+   page = __alloc_buddy_huge_page_with_mpol(h, vma,
+dummy_addr);
+   address_offset++;
+   } else {
+   page = __alloc_buddy_huge_page_no_mpol(h, NUMA_NO_NODE);
+   }
if (!page) {
alloc_ok = false;
break;
@@ -2057,7 +2073,7 @@ struct page *alloc_huge_page(struct vm_area_struct *vma,
long rsv_adjust;
 
rsv_adjust = hugepage_subpool_put_pages(spool, 1);
-   hugetlb_acct_memory(h, -rsv_adjust);
+   hugetlb_acct_memory(h, 

Re: [PATCH v4] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Andy Shevchenko]

On Thu, Feb 9, 2017 at 7:36 PM, Ben Gardner  wrote:
> Allow the at24 driver to get configuration information from both OF and
> ACPI by using the more generic device_property functions.
> This change was inspired by the at25.c driver.
>
> I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
> With the following ACPI construct, this patch instantiates a working
> instance of the driver.
>
> Device (EEP0) {
>  Name (_HID, "PRP0001")
>  Name (_DSD, Package () {
>   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>   Package () {
>Package () {"compatible", Package () {"st,24c02"}},
>Package () {"pagesize", 16},
>   },
>  })
>  Name (_CRS, ResourceTemplate () {
>   I2cSerialBus (
>0x0057, ControllerInitiated, 40,
>AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
>ResourceConsumer,,)
>  })
> }
>
> Note: Matching the driver to the I2C device requires another patch.
>  http://www.spinics.net/lists/linux-acpi/msg71914.html

Reviewed-by: Andy Shevchenko 

> Signed-off-by: Ben Gardner 
> ---
>  drivers/misc/eeprom/at24.c | 45 +++--
>  1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 051b147..764ff5df 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -19,7 +19,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, 
> void *val, size_t count)
> return 0;
>  }
>
> -#ifdef CONFIG_OF
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> +static void at24_get_pdata(struct device *dev, struct at24_platform_data 
> *chip)
>  {
> -   const __be32 *val;
> -   struct device_node *node = client->dev.of_node;
> -
> -   if (node) {
> -   if (of_get_property(node, "read-only", NULL))
> -   chip->flags |= AT24_FLAG_READONLY;
> -   val = of_get_property(node, "pagesize", NULL);
> -   if (val)
> -   chip->page_size = be32_to_cpup(val);
> +   int err;
> +   u32 val;
> +
> +   if (device_property_present(dev, "read-only"))
> +   chip->flags |= AT24_FLAG_READONLY;
> +
> +   err = device_property_read_u32(dev, "pagesize", );
> +   if (!err) {
> +   chip->page_size = val;
> +   } else {
> +   /*
> +* This is slow, but we can't know all eeproms, so we better
> +* play safe. Specifying custom eeprom-types via platform_data
> +* is recommended anyhow.
> +*/
> +   chip->page_size = 1;
> }
>  }
> -#else
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> -{ }
> -#endif /* CONFIG_OF */
>
>  static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
> *id)
>  {
> @@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
> struct i2c_device_id *id)
> chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
> magic >>= AT24_SIZE_BYTELEN;
> chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
> -   /*
> -* This is slow, but we can't know all eeproms, so we better
> -* play safe. Specifying custom eeprom-types via platform_data
> -* is recommended anyhow.
> -*/
> -   chip.page_size = 1;
>
> -   /* update chipdata if OF is present */
> -   at24_get_ofdata(client, );
> +   at24_get_pdata(>dev, );
>
> chip.setup = NULL;
> chip.context = NULL;
> --
> 2.7.4
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Andy Shevchenko]

On Thu, Feb 9, 2017 at 7:36 PM, Ben Gardner  wrote:
> Allow the at24 driver to get configuration information from both OF and
> ACPI by using the more generic device_property functions.
> This change was inspired by the at25.c driver.
>
> I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
> With the following ACPI construct, this patch instantiates a working
> instance of the driver.
>
> Device (EEP0) {
>  Name (_HID, "PRP0001")
>  Name (_DSD, Package () {
>   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>   Package () {
>Package () {"compatible", Package () {"st,24c02"}},
>Package () {"pagesize", 16},
>   },
>  })
>  Name (_CRS, ResourceTemplate () {
>   I2cSerialBus (
>0x0057, ControllerInitiated, 40,
>AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
>ResourceConsumer,,)
>  })
> }
>
> Note: Matching the driver to the I2C device requires another patch.
>  http://www.spinics.net/lists/linux-acpi/msg71914.html

Reviewed-by: Andy Shevchenko 

> Signed-off-by: Ben Gardner 
> ---
>  drivers/misc/eeprom/at24.c | 45 +++--
>  1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 051b147..764ff5df 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -19,7 +19,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, 
> void *val, size_t count)
> return 0;
>  }
>
> -#ifdef CONFIG_OF
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> +static void at24_get_pdata(struct device *dev, struct at24_platform_data 
> *chip)
>  {
> -   const __be32 *val;
> -   struct device_node *node = client->dev.of_node;
> -
> -   if (node) {
> -   if (of_get_property(node, "read-only", NULL))
> -   chip->flags |= AT24_FLAG_READONLY;
> -   val = of_get_property(node, "pagesize", NULL);
> -   if (val)
> -   chip->page_size = be32_to_cpup(val);
> +   int err;
> +   u32 val;
> +
> +   if (device_property_present(dev, "read-only"))
> +   chip->flags |= AT24_FLAG_READONLY;
> +
> +   err = device_property_read_u32(dev, "pagesize", );
> +   if (!err) {
> +   chip->page_size = val;
> +   } else {
> +   /*
> +* This is slow, but we can't know all eeproms, so we better
> +* play safe. Specifying custom eeprom-types via platform_data
> +* is recommended anyhow.
> +*/
> +   chip->page_size = 1;
> }
>  }
> -#else
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> -{ }
> -#endif /* CONFIG_OF */
>
>  static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
> *id)
>  {
> @@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
> struct i2c_device_id *id)
> chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
> magic >>= AT24_SIZE_BYTELEN;
> chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
> -   /*
> -* This is slow, but we can't know all eeproms, so we better
> -* play safe. Specifying custom eeprom-types via platform_data
> -* is recommended anyhow.
> -*/
> -   chip.page_size = 1;
>
> -   /* update chipdata if OF is present */
> -   at24_get_ofdata(client, );
> +   at24_get_pdata(>dev, );
>
> chip.setup = NULL;
> chip.context = NULL;
> --
> 2.7.4
>



-- 
With Best Regards,
Andy Shevchenko

Re: mm: deadlock between get_online_cpus/pcpu_alloc

[Thomas Gleixner]

On Thu, 9 Feb 2017, Christoph Lameter wrote:
> On Thu, 9 Feb 2017, Thomas Gleixner wrote:
> 
> > You are just not getting it, really.
> >
> > The problem is that this for_each_online_cpu() is racy against a concurrent
> > hot unplug and therefor can queue stuff for a not longer online cpu. That's
> > what the mm folks tried to avoid by preventing a CPU hotplug operation
> > before entering that loop.
> 
> With a stop machine action it is NOT racy because the machine goes into a
> special kernel state that guarantees that key operating system structures
> are not touched. See mm/page_alloc.c's use of that characteristic to build
> zonelists. Thus it cannot be executing for_each_online_cpu and related
> tasks (unless one does not disable preempt  but that is a given if a
> spinlock has been taken)..

drain_all_pages() is called from preemptible context. So what are you
talking about again?

> > > Lets get rid of get_online_cpus() etc.
> >
> > And that solves what?
> 
> It gets rid of future issues with serialization in paths were we need to
> lock and still do for_each_online_cpu().

There are code pathes which might sleep inside the loop so
get_online_cpus() is the only way to serialize against hotplug.

Just because the only tool you know is stop machine it does not make
everything an atomic context where it can be applied.

> > Can you please start to understand the scope of the whole hotplug machinery
> > including the requirements for get_online_cpus() before you waste
> > everybodys time with your uninformed and halfbaken proposals?
> 
> Its an obvious solution to the issues that have arisen multiple times with
> get_online_cpus() within the slab allocators. The hotplug machinery should
> make things as easy as possible for other people and having these
> get_online_cpus() everywhere does complicate things.

It's no obvious solution to everything. It's context dependend and people
have to think hard how to solve their problem within the context they are
dealing with.

Your 'get rid of get_online_cpus()' mantra does make all of this magically
simple. Relying on the fact, that the CPU online bit is cleared via
stomp_machine(), which is a horrible operation in various aspects, only
applies to a very small subset of problems. You can repeat your mantra
another thousand times and that will not make the way larger set of
problems magically disappear.

Thanks,

tglx

Re: mm: deadlock between get_online_cpus/pcpu_alloc

[Thomas Gleixner]

On Thu, 9 Feb 2017, Christoph Lameter wrote:
> On Thu, 9 Feb 2017, Thomas Gleixner wrote:
> 
> > You are just not getting it, really.
> >
> > The problem is that this for_each_online_cpu() is racy against a concurrent
> > hot unplug and therefor can queue stuff for a not longer online cpu. That's
> > what the mm folks tried to avoid by preventing a CPU hotplug operation
> > before entering that loop.
> 
> With a stop machine action it is NOT racy because the machine goes into a
> special kernel state that guarantees that key operating system structures
> are not touched. See mm/page_alloc.c's use of that characteristic to build
> zonelists. Thus it cannot be executing for_each_online_cpu and related
> tasks (unless one does not disable preempt  but that is a given if a
> spinlock has been taken)..

drain_all_pages() is called from preemptible context. So what are you
talking about again?

> > > Lets get rid of get_online_cpus() etc.
> >
> > And that solves what?
> 
> It gets rid of future issues with serialization in paths were we need to
> lock and still do for_each_online_cpu().

There are code pathes which might sleep inside the loop so
get_online_cpus() is the only way to serialize against hotplug.

Just because the only tool you know is stop machine it does not make
everything an atomic context where it can be applied.

> > Can you please start to understand the scope of the whole hotplug machinery
> > including the requirements for get_online_cpus() before you waste
> > everybodys time with your uninformed and halfbaken proposals?
> 
> Its an obvious solution to the issues that have arisen multiple times with
> get_online_cpus() within the slab allocators. The hotplug machinery should
> make things as easy as possible for other people and having these
> get_online_cpus() everywhere does complicate things.

It's no obvious solution to everything. It's context dependend and people
have to think hard how to solve their problem within the context they are
dealing with.

Your 'get rid of get_online_cpus()' mantra does make all of this magically
simple. Relying on the fact, that the CPU online bit is cleared via
stomp_machine(), which is a horrible operation in various aspects, only
applies to a very small subset of problems. You can repeat your mantra
another thousand times and that will not make the way larger set of
problems magically disappear.

Thanks,

tglx

Re: [PATCH v3 13/14] mm: migrate: move_pages() supports thp migration

[Zi Yan]

On 9 Feb 2017, at 3:16, Naoya Horiguchi wrote:

> On Sun, Feb 05, 2017 at 11:12:51AM -0500, Zi Yan wrote:
>> From: Naoya Horiguchi 
>>
>> This patch enables thp migration for move_pages(2).
>>
>> Signed-off-by: Naoya Horiguchi 
>> ---
>>  mm/migrate.c | 37 -
>>  1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 84181a3668c6..9bcaccb481ac 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -1413,7 +1413,17 @@ static struct page *new_page_node(struct page *p, 
>> unsigned long private,
>>  if (PageHuge(p))
>>  return alloc_huge_page_node(page_hstate(compound_head(p)),
>>  pm->node);
>> -else
>> +else if (thp_migration_supported() && PageTransHuge(p)) {
>> +struct page *thp;
>> +
>> +thp = alloc_pages_node(pm->node,
>> +(GFP_TRANSHUGE | __GFP_THISNODE) & ~__GFP_RECLAIM,
>> +HPAGE_PMD_ORDER);
>> +if (!thp)
>> +return NULL;
>> +prep_transhuge_page(thp);
>> +return thp;
>> +} else
>>  return __alloc_pages_node(pm->node,
>>  GFP_HIGHUSER_MOVABLE | __GFP_THISNODE, 0);
>>  }
>> @@ -1440,6 +1450,8 @@ static int do_move_page_to_node_array(struct mm_struct 
>> *mm,
>>  for (pp = pm; pp->node != MAX_NUMNODES; pp++) {
>>  struct vm_area_struct *vma;
>>  struct page *page;
>> +struct page *head;
>> +unsigned int follflags;
>>
>>  err = -EFAULT;
>>  vma = find_vma(mm, pp->addr);
>> @@ -1447,8 +1459,10 @@ static int do_move_page_to_node_array(struct 
>> mm_struct *mm,
>>  goto set_status;
>>
>>  /* FOLL_DUMP to ignore special (like zero) pages */
>> -page = follow_page(vma, pp->addr,
>> -FOLL_GET | FOLL_SPLIT | FOLL_DUMP);
>> +follflags = FOLL_GET | FOLL_SPLIT | FOLL_DUMP;
>
> FOLL_SPLIT should be added depending on thp_migration_supported().

Sure. I will fix it.


--
Best Regards
Yan Zi


signature.asc
Description: OpenPGP digital signature

Re: [PATCH v3 13/14] mm: migrate: move_pages() supports thp migration

[Zi Yan]

On 9 Feb 2017, at 3:16, Naoya Horiguchi wrote:

> On Sun, Feb 05, 2017 at 11:12:51AM -0500, Zi Yan wrote:
>> From: Naoya Horiguchi 
>>
>> This patch enables thp migration for move_pages(2).
>>
>> Signed-off-by: Naoya Horiguchi 
>> ---
>>  mm/migrate.c | 37 -
>>  1 file changed, 28 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 84181a3668c6..9bcaccb481ac 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -1413,7 +1413,17 @@ static struct page *new_page_node(struct page *p, 
>> unsigned long private,
>>  if (PageHuge(p))
>>  return alloc_huge_page_node(page_hstate(compound_head(p)),
>>  pm->node);
>> -else
>> +else if (thp_migration_supported() && PageTransHuge(p)) {
>> +struct page *thp;
>> +
>> +thp = alloc_pages_node(pm->node,
>> +(GFP_TRANSHUGE | __GFP_THISNODE) & ~__GFP_RECLAIM,
>> +HPAGE_PMD_ORDER);
>> +if (!thp)
>> +return NULL;
>> +prep_transhuge_page(thp);
>> +return thp;
>> +} else
>>  return __alloc_pages_node(pm->node,
>>  GFP_HIGHUSER_MOVABLE | __GFP_THISNODE, 0);
>>  }
>> @@ -1440,6 +1450,8 @@ static int do_move_page_to_node_array(struct mm_struct 
>> *mm,
>>  for (pp = pm; pp->node != MAX_NUMNODES; pp++) {
>>  struct vm_area_struct *vma;
>>  struct page *page;
>> +struct page *head;
>> +unsigned int follflags;
>>
>>  err = -EFAULT;
>>  vma = find_vma(mm, pp->addr);
>> @@ -1447,8 +1459,10 @@ static int do_move_page_to_node_array(struct 
>> mm_struct *mm,
>>  goto set_status;
>>
>>  /* FOLL_DUMP to ignore special (like zero) pages */
>> -page = follow_page(vma, pp->addr,
>> -FOLL_GET | FOLL_SPLIT | FOLL_DUMP);
>> +follflags = FOLL_GET | FOLL_SPLIT | FOLL_DUMP;
>
> FOLL_SPLIT should be added depending on thp_migration_supported().

Sure. I will fix it.


--
Best Regards
Yan Zi


signature.asc
Description: OpenPGP digital signature

Re: [PATCH v3 09/14] mm: thp: check pmd migration entry in common path

[Zi Yan]

On 9 Feb 2017, at 3:16, Naoya Horiguchi wrote:

> On Sun, Feb 05, 2017 at 11:12:47AM -0500, Zi Yan wrote:
>> From: Naoya Horiguchi 
>>
>> If one of callers of page migration starts to handle thp,
>> memory management code start to see pmd migration entry, so we need
>> to prepare for it before enabling. This patch changes various code
>> point which checks the status of given pmds in order to prevent race
>> between thp migration and the pmd-related works.
>>
>> ChangeLog v1 -> v2:
>> - introduce pmd_related() (I know the naming is not good, but can't
>>   think up no better name. Any suggesntion is welcomed.)
>>
>> Signed-off-by: Naoya Horiguchi 
>>
>> ChangeLog v2 -> v3:
>> - add is_swap_pmd()
>> - a pmd entry should be is_swap_pmd(), pmd_trans_huge(), pmd_devmap(),
>>   or pmd_none()
>
> (nitpick) ... or normal pmd pointing to pte pages?

Sure, I will add it.

>
>> - use pmdp_huge_clear_flush() instead of pmdp_huge_get_and_clear()
>> - flush_cache_range() while set_pmd_migration_entry()
>> - pmd_none_or_trans_huge_or_clear_bad() and pmd_trans_unstable() return
>>   true on pmd_migration_entry, so that migration entries are not
>>   treated as pmd page table entries.
>>
>> Signed-off-by: Zi Yan 
>> ---
>>  arch/x86/mm/gup.c |  4 +--
>>  fs/proc/task_mmu.c| 22 -
>>  include/asm-generic/pgtable.h | 71 
>>  include/linux/huge_mm.h   | 21 ++--
>>  include/linux/swapops.h   | 74 +
>>  mm/gup.c  | 20 ++--
>>  mm/huge_memory.c  | 76 
>> ---
>>  mm/madvise.c  |  2 ++
>>  mm/memcontrol.c   |  2 ++
>>  mm/memory.c   |  9 +++--
>>  mm/memory_hotplug.c   | 13 +++-
>>  mm/mempolicy.c|  1 +
>>  mm/mprotect.c |  6 ++--
>>  mm/mremap.c   |  2 +-
>>  mm/pagewalk.c |  2 ++
>>  15 files changed, 221 insertions(+), 104 deletions(-)
>>
>> diff --git a/arch/x86/mm/gup.c b/arch/x86/mm/gup.c
>> index 0d4fb3ebbbac..78a153d90064 100644
>> --- a/arch/x86/mm/gup.c
>> +++ b/arch/x86/mm/gup.c
>> @@ -222,9 +222,9 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, 
>> unsigned long end,
>>  pmd_t pmd = *pmdp;
>>
>>  next = pmd_addr_end(addr, end);
>> -if (pmd_none(pmd))
>> +if (!pmd_present(pmd))
>>  return 0;
>> -if (unlikely(pmd_large(pmd) || !pmd_present(pmd))) {
>> +if (unlikely(pmd_large(pmd))) {
>>  /*
>>   * NUMA hinting faults need to be handled in the GUP
>>   * slowpath for accounting purposes and so that they
>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> index 6c07c7813b26..1e64d6898c68 100644
>> --- a/fs/proc/task_mmu.c
>> +++ b/fs/proc/task_mmu.c
>> @@ -596,7 +596,8 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long 
>> addr, unsigned long end,
>>
>>  ptl = pmd_trans_huge_lock(pmd, vma);
>>  if (ptl) {
>> -smaps_pmd_entry(pmd, addr, walk);
>> +if (pmd_present(*pmd))
>> +smaps_pmd_entry(pmd, addr, walk);
>>  spin_unlock(ptl);
>>  return 0;
>>  }
>> @@ -929,6 +930,9 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned 
>> long addr,
>>  goto out;
>>  }
>>
>> +if (!pmd_present(*pmd))
>> +goto out;
>> +
>>  page = pmd_page(*pmd);
>>
>>  /* Clear accessed and referenced bits. */
>> @@ -1208,19 +1212,19 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned 
>> long addr, unsigned long end,
>>  if (ptl) {
>>  u64 flags = 0, frame = 0;
>>  pmd_t pmd = *pmdp;
>> +struct page *page;
>>
>>  if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(pmd))
>>  flags |= PM_SOFT_DIRTY;
>>
>> -/*
>> - * Currently pmd for thp is always present because thp
>> - * can not be swapped-out, migrated, or HWPOISONed
>> - * (split in such cases instead.)
>> - * This if-check is just to prepare for future implementation.
>> - */
>> -if (pmd_present(pmd)) {
>> -struct page *page = pmd_page(pmd);
>> +if (is_pmd_migration_entry(pmd)) {
>> +swp_entry_t entry = pmd_to_swp_entry(pmd);
>>
>> +frame = swp_type(entry) |
>> +(swp_offset(entry) << MAX_SWAPFILES_SHIFT);
>> +page = migration_entry_to_page(entry);
>> +} else if (pmd_present(pmd)) {
>> +page = pmd_page(pmd);
>>  if (page_mapcount(page) 

Re: [PATCH v3 09/14] mm: thp: check pmd migration entry in common path

[Zi Yan]

On 9 Feb 2017, at 3:16, Naoya Horiguchi wrote:

> On Sun, Feb 05, 2017 at 11:12:47AM -0500, Zi Yan wrote:
>> From: Naoya Horiguchi 
>>
>> If one of callers of page migration starts to handle thp,
>> memory management code start to see pmd migration entry, so we need
>> to prepare for it before enabling. This patch changes various code
>> point which checks the status of given pmds in order to prevent race
>> between thp migration and the pmd-related works.
>>
>> ChangeLog v1 -> v2:
>> - introduce pmd_related() (I know the naming is not good, but can't
>>   think up no better name. Any suggesntion is welcomed.)
>>
>> Signed-off-by: Naoya Horiguchi 
>>
>> ChangeLog v2 -> v3:
>> - add is_swap_pmd()
>> - a pmd entry should be is_swap_pmd(), pmd_trans_huge(), pmd_devmap(),
>>   or pmd_none()
>
> (nitpick) ... or normal pmd pointing to pte pages?

Sure, I will add it.

>
>> - use pmdp_huge_clear_flush() instead of pmdp_huge_get_and_clear()
>> - flush_cache_range() while set_pmd_migration_entry()
>> - pmd_none_or_trans_huge_or_clear_bad() and pmd_trans_unstable() return
>>   true on pmd_migration_entry, so that migration entries are not
>>   treated as pmd page table entries.
>>
>> Signed-off-by: Zi Yan 
>> ---
>>  arch/x86/mm/gup.c |  4 +--
>>  fs/proc/task_mmu.c| 22 -
>>  include/asm-generic/pgtable.h | 71 
>>  include/linux/huge_mm.h   | 21 ++--
>>  include/linux/swapops.h   | 74 +
>>  mm/gup.c  | 20 ++--
>>  mm/huge_memory.c  | 76 
>> ---
>>  mm/madvise.c  |  2 ++
>>  mm/memcontrol.c   |  2 ++
>>  mm/memory.c   |  9 +++--
>>  mm/memory_hotplug.c   | 13 +++-
>>  mm/mempolicy.c|  1 +
>>  mm/mprotect.c |  6 ++--
>>  mm/mremap.c   |  2 +-
>>  mm/pagewalk.c |  2 ++
>>  15 files changed, 221 insertions(+), 104 deletions(-)
>>
>> diff --git a/arch/x86/mm/gup.c b/arch/x86/mm/gup.c
>> index 0d4fb3ebbbac..78a153d90064 100644
>> --- a/arch/x86/mm/gup.c
>> +++ b/arch/x86/mm/gup.c
>> @@ -222,9 +222,9 @@ static int gup_pmd_range(pud_t pud, unsigned long addr, 
>> unsigned long end,
>>  pmd_t pmd = *pmdp;
>>
>>  next = pmd_addr_end(addr, end);
>> -if (pmd_none(pmd))
>> +if (!pmd_present(pmd))
>>  return 0;
>> -if (unlikely(pmd_large(pmd) || !pmd_present(pmd))) {
>> +if (unlikely(pmd_large(pmd))) {
>>  /*
>>   * NUMA hinting faults need to be handled in the GUP
>>   * slowpath for accounting purposes and so that they
>> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
>> index 6c07c7813b26..1e64d6898c68 100644
>> --- a/fs/proc/task_mmu.c
>> +++ b/fs/proc/task_mmu.c
>> @@ -596,7 +596,8 @@ static int smaps_pte_range(pmd_t *pmd, unsigned long 
>> addr, unsigned long end,
>>
>>  ptl = pmd_trans_huge_lock(pmd, vma);
>>  if (ptl) {
>> -smaps_pmd_entry(pmd, addr, walk);
>> +if (pmd_present(*pmd))
>> +smaps_pmd_entry(pmd, addr, walk);
>>  spin_unlock(ptl);
>>  return 0;
>>  }
>> @@ -929,6 +930,9 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned 
>> long addr,
>>  goto out;
>>  }
>>
>> +if (!pmd_present(*pmd))
>> +goto out;
>> +
>>  page = pmd_page(*pmd);
>>
>>  /* Clear accessed and referenced bits. */
>> @@ -1208,19 +1212,19 @@ static int pagemap_pmd_range(pmd_t *pmdp, unsigned 
>> long addr, unsigned long end,
>>  if (ptl) {
>>  u64 flags = 0, frame = 0;
>>  pmd_t pmd = *pmdp;
>> +struct page *page;
>>
>>  if ((vma->vm_flags & VM_SOFTDIRTY) || pmd_soft_dirty(pmd))
>>  flags |= PM_SOFT_DIRTY;
>>
>> -/*
>> - * Currently pmd for thp is always present because thp
>> - * can not be swapped-out, migrated, or HWPOISONed
>> - * (split in such cases instead.)
>> - * This if-check is just to prepare for future implementation.
>> - */
>> -if (pmd_present(pmd)) {
>> -struct page *page = pmd_page(pmd);
>> +if (is_pmd_migration_entry(pmd)) {
>> +swp_entry_t entry = pmd_to_swp_entry(pmd);
>>
>> +frame = swp_type(entry) |
>> +(swp_offset(entry) << MAX_SWAPFILES_SHIFT);
>> +page = migration_entry_to_page(entry);
>> +} else if (pmd_present(pmd)) {
>> +page = pmd_page(pmd);
>>  if (page_mapcount(page) == 1)
>>  flags |= PM_MMAP_EXCLUSIVE;
>>
>> diff 

[PATCH v4] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Ben Gardner]

Allow the at24 driver to get configuration information from both OF and
ACPI by using the more generic device_property functions.
This change was inspired by the at25.c driver.

I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
With the following ACPI construct, this patch instantiates a working
instance of the driver.

Device (EEP0) {
 Name (_HID, "PRP0001")
 Name (_DSD, Package () {
  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
  Package () {
   Package () {"compatible", Package () {"st,24c02"}},
   Package () {"pagesize", 16},
  },
 })
 Name (_CRS, ResourceTemplate () {
  I2cSerialBus (
   0x0057, ControllerInitiated, 40,
   AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
   ResourceConsumer,,)
 })
}

Note: Matching the driver to the I2C device requires another patch.
 http://www.spinics.net/lists/linux-acpi/msg71914.html

Signed-off-by: Ben Gardner 
---
 drivers/misc/eeprom/at24.c | 45 +++--
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 051b147..764ff5df 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -19,7 +19,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, void 
*val, size_t count)
return 0;
 }
 
-#ifdef CONFIG_OF
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
+static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
-   const __be32 *val;
-   struct device_node *node = client->dev.of_node;
-
-   if (node) {
-   if (of_get_property(node, "read-only", NULL))
-   chip->flags |= AT24_FLAG_READONLY;
-   val = of_get_property(node, "pagesize", NULL);
-   if (val)
-   chip->page_size = be32_to_cpup(val);
+   int err;
+   u32 val;
+
+   if (device_property_present(dev, "read-only"))
+   chip->flags |= AT24_FLAG_READONLY;
+
+   err = device_property_read_u32(dev, "pagesize", );
+   if (!err) {
+   chip->page_size = val;
+   } else {
+   /*
+* This is slow, but we can't know all eeproms, so we better
+* play safe. Specifying custom eeprom-types via platform_data
+* is recommended anyhow.
+*/
+   chip->page_size = 1;
}
 }
-#else
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
-{ }
-#endif /* CONFIG_OF */
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
*id)
 {
@@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
magic >>= AT24_SIZE_BYTELEN;
chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
-   /*
-* This is slow, but we can't know all eeproms, so we better
-* play safe. Specifying custom eeprom-types via platform_data
-* is recommended anyhow.
-*/
-   chip.page_size = 1;
 
-   /* update chipdata if OF is present */
-   at24_get_ofdata(client, );
+   at24_get_pdata(>dev, );
 
chip.setup = NULL;
chip.context = NULL;
-- 
2.7.4

[PATCH v4] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Ben Gardner]

Allow the at24 driver to get configuration information from both OF and
ACPI by using the more generic device_property functions.
This change was inspired by the at25.c driver.

I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
With the following ACPI construct, this patch instantiates a working
instance of the driver.

Device (EEP0) {
 Name (_HID, "PRP0001")
 Name (_DSD, Package () {
  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
  Package () {
   Package () {"compatible", Package () {"st,24c02"}},
   Package () {"pagesize", 16},
  },
 })
 Name (_CRS, ResourceTemplate () {
  I2cSerialBus (
   0x0057, ControllerInitiated, 40,
   AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
   ResourceConsumer,,)
 })
}

Note: Matching the driver to the I2C device requires another patch.
 http://www.spinics.net/lists/linux-acpi/msg71914.html

Signed-off-by: Ben Gardner 
---
 drivers/misc/eeprom/at24.c | 45 +++--
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 051b147..764ff5df 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -19,7 +19,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, void 
*val, size_t count)
return 0;
 }
 
-#ifdef CONFIG_OF
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
+static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
-   const __be32 *val;
-   struct device_node *node = client->dev.of_node;
-
-   if (node) {
-   if (of_get_property(node, "read-only", NULL))
-   chip->flags |= AT24_FLAG_READONLY;
-   val = of_get_property(node, "pagesize", NULL);
-   if (val)
-   chip->page_size = be32_to_cpup(val);
+   int err;
+   u32 val;
+
+   if (device_property_present(dev, "read-only"))
+   chip->flags |= AT24_FLAG_READONLY;
+
+   err = device_property_read_u32(dev, "pagesize", );
+   if (!err) {
+   chip->page_size = val;
+   } else {
+   /*
+* This is slow, but we can't know all eeproms, so we better
+* play safe. Specifying custom eeprom-types via platform_data
+* is recommended anyhow.
+*/
+   chip->page_size = 1;
}
 }
-#else
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
-{ }
-#endif /* CONFIG_OF */
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
*id)
 {
@@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
magic >>= AT24_SIZE_BYTELEN;
chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
-   /*
-* This is slow, but we can't know all eeproms, so we better
-* play safe. Specifying custom eeprom-types via platform_data
-* is recommended anyhow.
-*/
-   chip.page_size = 1;
 
-   /* update chipdata if OF is present */
-   at24_get_ofdata(client, );
+   at24_get_pdata(>dev, );
 
chip.setup = NULL;
chip.context = NULL;
-- 
2.7.4

Re: [PATCH] drm: Add helper macro for duplicating custom drm_*_state

[Daniel Vetter]

On Thu, Feb 09, 2017 at 03:41:42PM +, Mihail Atanassov wrote:
> Assuming a derived struct of the form:
> 
> struct foo_bar_state
> {
>   struct drm_bar_state bar_state;
>   struct foo_private priv;
>   struct foo_private2 *priv2;
> };
> 
> memcpy priv and priv2 to the new instance of foo_bar_state. The
> intention is to use this macro in ->atomic_duplicate_state in conjunction with
> __drm_atomic_helper_*_duplicate_state, which already copies the relevant
> drm_*_state struct.
> 
> There's an equality check for new_state and old_state to ensure that
> they are distinct instances of the same type, and a BUILD_BUG if the
> base struct (bar_state in the above example) is not first in the derived
> struct, to avoid missing any data before it and corrupting the base's data.
> 
> Signed-off-by: Mihail Atanassov 

Seems reasonable, but I don't have any strong opinions about it's
usefulness. Converting a few drivers (to really establish it as a
pattern), or at least a few acks from maintainers, to prove that it's a
good idea, and I'll merge this.
-Daniel

> ---
>  include/drm/drm_atomic_helper.h | 33 +
>  1 file changed, 33 insertions(+)
> 
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d066e94..ecc6a82 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc 
> *crtc,
>  uint32_t size);
>  
>  /**
> + * drm_atomic_duplicate_custom_state - helper macro for duplicating
> + * driver-private additions to drm_*_state.
> + * @new_state: pointer to destination state struct
> + * @old_state: pointer to source state struct
> + * @basename: name of the drm_*_state member of the new_state/old_state 
> struct
> + *
> + * Copies the data after the base struct until the end of the custom struct,
> + * e.g. given a structure
> + *
> + * struct foo_bar_state {
> + *   struct drm_bar_state base;
> + *   struct foo_private priv;
> + *   struct foo_private2 *priv2;
> + * };
> + *
> + * this copies priv and priv2. NB: the base struct *must* be the first 
> element
> + * of the derived struct, and new_state and old_state have to be two distinct
> + * instances.
> + */
> +#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, 
> basename) \
> + do { \
> + size_t base_size = sizeof(new_state->basename); \
> + size_t base_offset = offsetof(typeof(*new_state), basename); \
> + \
> + BUILD_BUG_ON(base_offset != 0); \
> + if (new_state == old_state) /* Type-check */ \
> + break; \
> + memcpy((char *)new_state + base_size, \
> +(char *)old_state + base_size, \
> +sizeof(*new_state) - base_size); \
> + } while(0)
> +
> +/**
>   * drm_atomic_crtc_for_each_plane - iterate over planes currently attached 
> to CRTC
>   * @plane: the loop cursor
>   * @crtc:  the crtc whose planes are iterated
> -- 
> 1.9.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm: Add helper macro for duplicating custom drm_*_state

[Daniel Vetter]

On Thu, Feb 09, 2017 at 03:41:42PM +, Mihail Atanassov wrote:
> Assuming a derived struct of the form:
> 
> struct foo_bar_state
> {
>   struct drm_bar_state bar_state;
>   struct foo_private priv;
>   struct foo_private2 *priv2;
> };
> 
> memcpy priv and priv2 to the new instance of foo_bar_state. The
> intention is to use this macro in ->atomic_duplicate_state in conjunction with
> __drm_atomic_helper_*_duplicate_state, which already copies the relevant
> drm_*_state struct.
> 
> There's an equality check for new_state and old_state to ensure that
> they are distinct instances of the same type, and a BUILD_BUG if the
> base struct (bar_state in the above example) is not first in the derived
> struct, to avoid missing any data before it and corrupting the base's data.
> 
> Signed-off-by: Mihail Atanassov 

Seems reasonable, but I don't have any strong opinions about it's
usefulness. Converting a few drivers (to really establish it as a
pattern), or at least a few acks from maintainers, to prove that it's a
good idea, and I'll merge this.
-Daniel

> ---
>  include/drm/drm_atomic_helper.h | 33 +
>  1 file changed, 33 insertions(+)
> 
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d066e94..ecc6a82 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -171,6 +171,39 @@ int drm_atomic_helper_legacy_gamma_set(struct drm_crtc 
> *crtc,
>  uint32_t size);
>  
>  /**
> + * drm_atomic_duplicate_custom_state - helper macro for duplicating
> + * driver-private additions to drm_*_state.
> + * @new_state: pointer to destination state struct
> + * @old_state: pointer to source state struct
> + * @basename: name of the drm_*_state member of the new_state/old_state 
> struct
> + *
> + * Copies the data after the base struct until the end of the custom struct,
> + * e.g. given a structure
> + *
> + * struct foo_bar_state {
> + *   struct drm_bar_state base;
> + *   struct foo_private priv;
> + *   struct foo_private2 *priv2;
> + * };
> + *
> + * this copies priv and priv2. NB: the base struct *must* be the first 
> element
> + * of the derived struct, and new_state and old_state have to be two distinct
> + * instances.
> + */
> +#define drm_atomic_helper_duplicate_custom_state(new_state, old_state, 
> basename) \
> + do { \
> + size_t base_size = sizeof(new_state->basename); \
> + size_t base_offset = offsetof(typeof(*new_state), basename); \
> + \
> + BUILD_BUG_ON(base_offset != 0); \
> + if (new_state == old_state) /* Type-check */ \
> + break; \
> + memcpy((char *)new_state + base_size, \
> +(char *)old_state + base_size, \
> +sizeof(*new_state) - base_size); \
> + } while(0)
> +
> +/**
>   * drm_atomic_crtc_for_each_plane - iterate over planes currently attached 
> to CRTC
>   * @plane: the loop cursor
>   * @crtc:  the crtc whose planes are iterated
> -- 
> 1.9.1
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

make pdfdoc errors with 4.10-rc7

[Jim Davis]

On a Fedora 25 system, make pdfdocs is failing with

[jim@krebstar linux-rc]$ grep -v -i 'warning:' /tmp/make-pdfdocs.err
/data/linux-rc/Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown
target name: "sphinx c domain".
./include/net/cfg80211.h:3154: ERROR: Unexpected indentation.
./include/net/mac80211.h:3214: ERROR: Unexpected indentation.
./include/net/mac80211.h:3219: ERROR: Unexpected indentation.
./include/net/mac80211.h:1773: ERROR: Unexpected indentation.
./kernel/time/timer.c:1240: ERROR: Unexpected indentation.
./kernel/time/timer.c:1242: ERROR: Unexpected indentation.
./include/linux/wait.h:124: ERROR: Unexpected indentation.
./include/linux/spi/spi.h:369: ERROR: Unexpected indentation.
./drivers/usb/core/message.c:481: ERROR: Unexpected indentation.
/data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown
target name: "usb_type".
/data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown
target name: "usb_dir".
/data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown
target name: "usb_recip".
/data/linux-rc/Documentation/driver-api/usb.rst:689: ERROR: Unknown
target name: "usbdevfs_urb_type".
./sound/soc/soc-core.c:2508: ERROR: Unknown target name: "snd_soc_daifmt".
./sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn".
make[2]: *** [linux-user.pdf] Error 1
make[1]: *** [pdfdocs] Error 2
make: *** [pdfdocs] Error 2


-- 
Jim

[PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

[Scott Bauer]

When CONFIG_KASAN is enabled, compilation fails:

block/sed-opal.c: In function 'sed_ioctl':
block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 
2048 bytes [-Werror=frame-larger-than=]

Moved all the ioctl structures off the stack and dynamically activate
using _IOC_SIZE()

Fixes: 455a7b238cd6 ("block: Add Sed-opal library")

Reported-by: Arnd Bergmann 
Signed-off-by: Scott Bauer 
---
 block/sed-opal.c | 134 +--
 1 file changed, 50 insertions(+), 84 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index bf1406e..4985d95 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2346,7 +2346,10 @@ EXPORT_SYMBOL(opal_unlock_from_suspend);
 
 int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
 {
+   void *ioctl_ptr;
+   int ret = -ENOTTY;
void __user *arg = (void __user *)ptr;
+   unsigned int cmd_size = _IOC_SIZE(cmd);
 
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -2355,94 +2358,57 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, 
unsigned long ptr)
return -ENOTSUPP;
}
 
-   switch (cmd) {
-   case IOC_OPAL_SAVE: {
-   struct opal_lock_unlock lk_unlk;
-
-   if (copy_from_user(_unlk, arg, sizeof(lk_unlk)))
-   return -EFAULT;
-   return opal_save(dev, _unlk);
-   }
-   case IOC_OPAL_LOCK_UNLOCK: {
-   struct opal_lock_unlock lk_unlk;
-
-   if (copy_from_user(_unlk, arg, sizeof(lk_unlk)))
-   return -EFAULT;
-   return opal_lock_unlock(dev, _unlk);
-   }
-   case IOC_OPAL_TAKE_OWNERSHIP: {
-   struct opal_key opal_key;
-
-   if (copy_from_user(_key, arg, sizeof(opal_key)))
-   return -EFAULT;
-   return opal_take_ownership(dev, _key);
-   }
-   case IOC_OPAL_ACTIVATE_LSP: {
-   struct opal_lr_act opal_lr_act;
-
-   if (copy_from_user(_lr_act, arg, sizeof(opal_lr_act)))
-   return -EFAULT;
-   return opal_activate_lsp(dev, _lr_act);
-   }
-   case IOC_OPAL_SET_PW: {
-   struct opal_new_pw opal_pw;
-
-   if (copy_from_user(_pw, arg, sizeof(opal_pw)))
-   return -EFAULT;
-   return opal_set_new_pw(dev, _pw);
-   }
-   case IOC_OPAL_ACTIVATE_USR: {
-   struct opal_session_info session;
-
-   if (copy_from_user(, arg, sizeof(session)))
-   return -EFAULT;
-   return opal_activate_user(dev, );
-   }
-   case IOC_OPAL_REVERT_TPR: {
-   struct opal_key opal_key;
-
-   if (copy_from_user(_key, arg, sizeof(opal_key)))
-   return -EFAULT;
-   return opal_reverttper(dev, _key);
-   }
-   case IOC_OPAL_LR_SETUP: {
-   struct opal_user_lr_setup lrs;
-
-   if (copy_from_user(, arg, sizeof(lrs)))
-   return -EFAULT;
-   return opal_setup_locking_range(dev, );
-   }
-   case IOC_OPAL_ADD_USR_TO_LR: {
-   struct opal_lock_unlock lk_unlk;
-
-   if (copy_from_user(_unlk, arg, sizeof(lk_unlk)))
-   return -EFAULT;
-   return opal_add_user_to_lr(dev, _unlk);
-   }
-   case IOC_OPAL_ENABLE_DISABLE_MBR: {
-   struct opal_mbr_data mbr;
-
-   if (copy_from_user(, arg, sizeof(mbr)))
-   return -EFAULT;
-   return opal_enable_disable_shadow_mbr(dev, );
-   }
-   case IOC_OPAL_ERASE_LR: {
-   struct opal_session_info session;
-
-   if (copy_from_user(, arg, sizeof(session)))
-   return -EFAULT;
-   return opal_erase_locking_range(dev, );
+   ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
+   if (!ioctl_ptr)
+   return -ENOMEM;
+   if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
+   ret = -EFAULT;
+   goto out;
}
-   case IOC_OPAL_SECURE_ERASE_LR: {
-   struct opal_session_info session;
 
-   if (copy_from_user(, arg, sizeof(session)))
-   return -EFAULT;
-   return opal_secure_erase_locking_range(dev, );
-   }
+   switch (cmd) {
+   case IOC_OPAL_SAVE:
+   ret = opal_save(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_LOCK_UNLOCK:
+   ret = opal_lock_unlock(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_TAKE_OWNERSHIP:
+   ret = opal_take_ownership(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_ACTIVATE_LSP:
+   ret = opal_activate_lsp(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_SET_PW:
+ 

[PATCH V3 2/2] Move stack parameters for sed_ioctl to prevent oversized stack with CONFIG_KASAN

[Scott Bauer]

When CONFIG_KASAN is enabled, compilation fails:

block/sed-opal.c: In function 'sed_ioctl':
block/sed-opal.c:2447:1: error: the frame size of 2256 bytes is larger than 
2048 bytes [-Werror=frame-larger-than=]

Moved all the ioctl structures off the stack and dynamically activate
using _IOC_SIZE()

Fixes: 455a7b238cd6 ("block: Add Sed-opal library")

Reported-by: Arnd Bergmann 
Signed-off-by: Scott Bauer 
---
 block/sed-opal.c | 134 +--
 1 file changed, 50 insertions(+), 84 deletions(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index bf1406e..4985d95 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2346,7 +2346,10 @@ EXPORT_SYMBOL(opal_unlock_from_suspend);
 
 int sed_ioctl(struct opal_dev *dev, unsigned int cmd, unsigned long ptr)
 {
+   void *ioctl_ptr;
+   int ret = -ENOTTY;
void __user *arg = (void __user *)ptr;
+   unsigned int cmd_size = _IOC_SIZE(cmd);
 
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -2355,94 +2358,57 @@ int sed_ioctl(struct opal_dev *dev, unsigned int cmd, 
unsigned long ptr)
return -ENOTSUPP;
}
 
-   switch (cmd) {
-   case IOC_OPAL_SAVE: {
-   struct opal_lock_unlock lk_unlk;
-
-   if (copy_from_user(_unlk, arg, sizeof(lk_unlk)))
-   return -EFAULT;
-   return opal_save(dev, _unlk);
-   }
-   case IOC_OPAL_LOCK_UNLOCK: {
-   struct opal_lock_unlock lk_unlk;
-
-   if (copy_from_user(_unlk, arg, sizeof(lk_unlk)))
-   return -EFAULT;
-   return opal_lock_unlock(dev, _unlk);
-   }
-   case IOC_OPAL_TAKE_OWNERSHIP: {
-   struct opal_key opal_key;
-
-   if (copy_from_user(_key, arg, sizeof(opal_key)))
-   return -EFAULT;
-   return opal_take_ownership(dev, _key);
-   }
-   case IOC_OPAL_ACTIVATE_LSP: {
-   struct opal_lr_act opal_lr_act;
-
-   if (copy_from_user(_lr_act, arg, sizeof(opal_lr_act)))
-   return -EFAULT;
-   return opal_activate_lsp(dev, _lr_act);
-   }
-   case IOC_OPAL_SET_PW: {
-   struct opal_new_pw opal_pw;
-
-   if (copy_from_user(_pw, arg, sizeof(opal_pw)))
-   return -EFAULT;
-   return opal_set_new_pw(dev, _pw);
-   }
-   case IOC_OPAL_ACTIVATE_USR: {
-   struct opal_session_info session;
-
-   if (copy_from_user(, arg, sizeof(session)))
-   return -EFAULT;
-   return opal_activate_user(dev, );
-   }
-   case IOC_OPAL_REVERT_TPR: {
-   struct opal_key opal_key;
-
-   if (copy_from_user(_key, arg, sizeof(opal_key)))
-   return -EFAULT;
-   return opal_reverttper(dev, _key);
-   }
-   case IOC_OPAL_LR_SETUP: {
-   struct opal_user_lr_setup lrs;
-
-   if (copy_from_user(, arg, sizeof(lrs)))
-   return -EFAULT;
-   return opal_setup_locking_range(dev, );
-   }
-   case IOC_OPAL_ADD_USR_TO_LR: {
-   struct opal_lock_unlock lk_unlk;
-
-   if (copy_from_user(_unlk, arg, sizeof(lk_unlk)))
-   return -EFAULT;
-   return opal_add_user_to_lr(dev, _unlk);
-   }
-   case IOC_OPAL_ENABLE_DISABLE_MBR: {
-   struct opal_mbr_data mbr;
-
-   if (copy_from_user(, arg, sizeof(mbr)))
-   return -EFAULT;
-   return opal_enable_disable_shadow_mbr(dev, );
-   }
-   case IOC_OPAL_ERASE_LR: {
-   struct opal_session_info session;
-
-   if (copy_from_user(, arg, sizeof(session)))
-   return -EFAULT;
-   return opal_erase_locking_range(dev, );
+   ioctl_ptr = kzalloc(cmd_size, GFP_KERNEL);
+   if (!ioctl_ptr)
+   return -ENOMEM;
+   if (copy_from_user(ioctl_ptr, arg, cmd_size)) {
+   ret = -EFAULT;
+   goto out;
}
-   case IOC_OPAL_SECURE_ERASE_LR: {
-   struct opal_session_info session;
 
-   if (copy_from_user(, arg, sizeof(session)))
-   return -EFAULT;
-   return opal_secure_erase_locking_range(dev, );
-   }
+   switch (cmd) {
+   case IOC_OPAL_SAVE:
+   ret = opal_save(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_LOCK_UNLOCK:
+   ret = opal_lock_unlock(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_TAKE_OWNERSHIP:
+   ret = opal_take_ownership(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_ACTIVATE_LSP:
+   ret = opal_activate_lsp(dev, ioctl_ptr);
+   break;
+   case IOC_OPAL_SET_PW:
+   ret = opal_set_new_pw(dev, 

make pdfdoc errors with 4.10-rc7

[Jim Davis]

On a Fedora 25 system, make pdfdocs is failing with

[jim@krebstar linux-rc]$ grep -v -i 'warning:' /tmp/make-pdfdocs.err
/data/linux-rc/Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown
target name: "sphinx c domain".
./include/net/cfg80211.h:3154: ERROR: Unexpected indentation.
./include/net/mac80211.h:3214: ERROR: Unexpected indentation.
./include/net/mac80211.h:3219: ERROR: Unexpected indentation.
./include/net/mac80211.h:1773: ERROR: Unexpected indentation.
./kernel/time/timer.c:1240: ERROR: Unexpected indentation.
./kernel/time/timer.c:1242: ERROR: Unexpected indentation.
./include/linux/wait.h:124: ERROR: Unexpected indentation.
./include/linux/spi/spi.h:369: ERROR: Unexpected indentation.
./drivers/usb/core/message.c:481: ERROR: Unexpected indentation.
/data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown
target name: "usb_type".
/data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown
target name: "usb_dir".
/data/linux-rc/Documentation/driver-api/usb.rst:623: ERROR: Unknown
target name: "usb_recip".
/data/linux-rc/Documentation/driver-api/usb.rst:689: ERROR: Unknown
target name: "usbdevfs_urb_type".
./sound/soc/soc-core.c:2508: ERROR: Unknown target name: "snd_soc_daifmt".
./sound/core/jack.c:312: ERROR: Unknown target name: "snd_jack_btn".
make[2]: *** [linux-user.pdf] Error 1
make[1]: *** [pdfdocs] Error 2
make: *** [pdfdocs] Error 2


-- 
Jim

[PATCH V3 1/2] uapi: sed-opal fix IOW for activate lsp to use correct struct

[Scott Bauer]

the IOW for the IOC_OPAL_ACTIVATE_LSP took the wrong strcure which
would give us the wrong size when using _IOC_SIZE, switch it to the
right structure.

Fixes: 058f8a2 ("Include: Uapi: Add user ABI for Sed/Opal")

Signed-off-by: Scott Bauer 
---
 include/uapi/linux/sed-opal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index fc06e3a..c72e073 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -106,7 +106,7 @@ struct opal_mbr_data {
 #define IOC_OPAL_SAVE  _IOW('p', 220, struct opal_lock_unlock)
 #define IOC_OPAL_LOCK_UNLOCK   _IOW('p', 221, struct opal_lock_unlock)
 #define IOC_OPAL_TAKE_OWNERSHIP_IOW('p', 222, struct opal_key)
-#define IOC_OPAL_ACTIVATE_LSP   _IOW('p', 223, struct opal_key)
+#define IOC_OPAL_ACTIVATE_LSP   _IOW('p', 223, struct opal_lr_act)
 #define IOC_OPAL_SET_PW _IOW('p', 224, struct opal_new_pw)
 #define IOC_OPAL_ACTIVATE_USR   _IOW('p', 225, struct opal_session_info)
 #define IOC_OPAL_REVERT_TPR _IOW('p', 226, struct opal_key)
-- 
2.7.4

[PATCH V3 1/2] uapi: sed-opal fix IOW for activate lsp to use correct struct

[Scott Bauer]

the IOW for the IOC_OPAL_ACTIVATE_LSP took the wrong strcure which
would give us the wrong size when using _IOC_SIZE, switch it to the
right structure.

Fixes: 058f8a2 ("Include: Uapi: Add user ABI for Sed/Opal")

Signed-off-by: Scott Bauer 
---
 include/uapi/linux/sed-opal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index fc06e3a..c72e073 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -106,7 +106,7 @@ struct opal_mbr_data {
 #define IOC_OPAL_SAVE  _IOW('p', 220, struct opal_lock_unlock)
 #define IOC_OPAL_LOCK_UNLOCK   _IOW('p', 221, struct opal_lock_unlock)
 #define IOC_OPAL_TAKE_OWNERSHIP_IOW('p', 222, struct opal_key)
-#define IOC_OPAL_ACTIVATE_LSP   _IOW('p', 223, struct opal_key)
+#define IOC_OPAL_ACTIVATE_LSP   _IOW('p', 223, struct opal_lr_act)
 #define IOC_OPAL_SET_PW _IOW('p', 224, struct opal_new_pw)
 #define IOC_OPAL_ACTIVATE_USR   _IOW('p', 225, struct opal_session_info)
 #define IOC_OPAL_REVERT_TPR _IOW('p', 226, struct opal_key)
-- 
2.7.4

Sed-opal fixups

[Scott Bauer]

It may be too late to change anyhting in the uapi header. When we
switched over to using IOC_SIZE I found a bug where I had switched
up a structure in one of the series from v4 to v5 but never changed
the structure in the IOW. The structure that was in there was to small
so when we kzalloc on it we don't request enough space. It worked before
because we were using the cmd strictly as a command #, not using the IOC
and friends.

If it's too late to modify that IOW, I can work around it by reallocing
on the correct size for that command only. I verified the rest of the
commands and the structures are the same.

Let me know what you think, please.

Sed-opal fixups

[Scott Bauer]

It may be too late to change anyhting in the uapi header. When we
switched over to using IOC_SIZE I found a bug where I had switched
up a structure in one of the series from v4 to v5 but never changed
the structure in the IOW. The structure that was in there was to small
so when we kzalloc on it we don't request enough space. It worked before
because we were using the cmd strictly as a command #, not using the IOC
and friends.

If it's too late to modify that IOW, I can work around it by reallocing
on the correct size for that command only. I verified the rest of the
commands and the structures are the same.

Let me know what you think, please.

Re: [PATCH v3 01/14] Documentation: dt/bindings: Document pinctrl-ingenic

[Paul Cercueil]

Le 2017-01-31 14:09, Linus Walleij a écrit :
On Tue, Jan 31, 2017 at 11:31 AM, Paul Cercueil  
wrote:

[Rob]:

From the overlapping register addresses in the examples and this
description, it looks like the pinctrlr and gpio controller are 1 
block.

If so, then there should only be 1 node.


Well, that's what I had until Linus W. just told me to do the 
opposite:



Just pull all these down two levels and make them one device
each instead of having them inside the pin controller node
like this.


I guess the argument is that they are in the same coherent memory
range so they should be one device node. That is how we handle
e.g. system controllers so it makes some sense.

So can the two GPIO controllers be modeled as two subnodes of
the pin controller then?

Subnodes are certainly OK, we have that for many other devices
such as interrupt controllers on PCI bridges and what not.

So when the probing of the pin controller is ready it can just
walk down and populate the GPIO subdevices with
of_platform_default_populate() or simply by registering the
device directly with platform_device_add_data() just like an
MFD device does?

This is nice because we want to use the standard gpio ranges
to map pins to GPIO lines.

I'm sorry about the unclarities here, but it's essentially an intrinsic
problem with GPIO that has been with us for years: do we model
each "bank" as a device or do we just register each bank as a
gpiochip, or do we even make one gpiochip to cover all the banks.
All solutions can be found in the kernel... also the different DT 
bindings:

one node for a whole slew of GPIO controllers, or seveal nodes
and I bet also several nodes for memory ranges in close proximity.

I don't know for sure what is the most elegant solution, we might
need to build some consensus here for the future so it doesn't
get to heterogeneous.

Yours,
Linus Walleij


I was thinking that instead of having one pinctrl-ingenic instance 
covering
0x600 of register space, and 6 instances of gpio-ingenic having 0x100 
each,
I could just have 6 instances of pinctrl-ingenic, each one with an 
instance
of gpio-ingenic declared as a sub-node, each handling just 0x100 of 
memory space.


Then I can make pinctrl-ingenic and gpio-ingenic share a regmap (through 
syscon),
which would be a good idea anyway since the two drivers poke to the very 
same
registers (in theory not at the same time, but it's never safe to assume 
things

like this).

Problem is, that in that case the pin functions/groups (and 
ingenic,pull-ups)
would have to be in DTS because we would have 6 instances with different 
pin

groups, and I know you hate that.

Thoughts?

Re: [PATCH v3 01/14] Documentation: dt/bindings: Document pinctrl-ingenic

[Paul Cercueil]

Le 2017-01-31 14:09, Linus Walleij a écrit :
On Tue, Jan 31, 2017 at 11:31 AM, Paul Cercueil  
wrote:

[Rob]:

From the overlapping register addresses in the examples and this
description, it looks like the pinctrlr and gpio controller are 1 
block.

If so, then there should only be 1 node.


Well, that's what I had until Linus W. just told me to do the 
opposite:



Just pull all these down two levels and make them one device
each instead of having them inside the pin controller node
like this.


I guess the argument is that they are in the same coherent memory
range so they should be one device node. That is how we handle
e.g. system controllers so it makes some sense.

So can the two GPIO controllers be modeled as two subnodes of
the pin controller then?

Subnodes are certainly OK, we have that for many other devices
such as interrupt controllers on PCI bridges and what not.

So when the probing of the pin controller is ready it can just
walk down and populate the GPIO subdevices with
of_platform_default_populate() or simply by registering the
device directly with platform_device_add_data() just like an
MFD device does?

This is nice because we want to use the standard gpio ranges
to map pins to GPIO lines.

I'm sorry about the unclarities here, but it's essentially an intrinsic
problem with GPIO that has been with us for years: do we model
each "bank" as a device or do we just register each bank as a
gpiochip, or do we even make one gpiochip to cover all the banks.
All solutions can be found in the kernel... also the different DT 
bindings:

one node for a whole slew of GPIO controllers, or seveal nodes
and I bet also several nodes for memory ranges in close proximity.

I don't know for sure what is the most elegant solution, we might
need to build some consensus here for the future so it doesn't
get to heterogeneous.

Yours,
Linus Walleij


I was thinking that instead of having one pinctrl-ingenic instance 
covering
0x600 of register space, and 6 instances of gpio-ingenic having 0x100 
each,
I could just have 6 instances of pinctrl-ingenic, each one with an 
instance
of gpio-ingenic declared as a sub-node, each handling just 0x100 of 
memory space.


Then I can make pinctrl-ingenic and gpio-ingenic share a regmap (through 
syscon),
which would be a good idea anyway since the two drivers poke to the very 
same
registers (in theory not at the same time, but it's never safe to assume 
things

like this).

Problem is, that in that case the pin functions/groups (and 
ingenic,pull-ups)
would have to be in DTS because we would have 6 instances with different 
pin

groups, and I know you hate that.

Thoughts?

Re: mm: deadlock between get_online_cpus/pcpu_alloc

[Christoph Lameter]

On Thu, 9 Feb 2017, Thomas Gleixner wrote:

> You are just not getting it, really.
>
> The problem is that this for_each_online_cpu() is racy against a concurrent
> hot unplug and therefor can queue stuff for a not longer online cpu. That's
> what the mm folks tried to avoid by preventing a CPU hotplug operation
> before entering that loop.

With a stop machine action it is NOT racy because the machine goes into a
special kernel state that guarantees that key operating system structures
are not touched. See mm/page_alloc.c's use of that characteristic to build
zonelists. Thus it cannot be executing for_each_online_cpu and related
tasks (unless one does not disable preempt  but that is a given if a
spinlock has been taken)..

> > Lets get rid of get_online_cpus() etc.
>
> And that solves what?

It gets rid of future issues with serialization in paths were we need to
lock and still do for_each_online_cpu().

> Can you please start to understand the scope of the whole hotplug machinery
> including the requirements for get_online_cpus() before you waste
> everybodys time with your uninformed and halfbaken proposals?

Its an obvious solution to the issues that have arisen multiple times with
get_online_cpus() within the slab allocators. The hotplug machinery should
make things as easy as possible for other people and having these
get_online_cpus() everywhere does complicate things.

Re: mm: deadlock between get_online_cpus/pcpu_alloc

[Christoph Lameter]

On Thu, 9 Feb 2017, Thomas Gleixner wrote:

> You are just not getting it, really.
>
> The problem is that this for_each_online_cpu() is racy against a concurrent
> hot unplug and therefor can queue stuff for a not longer online cpu. That's
> what the mm folks tried to avoid by preventing a CPU hotplug operation
> before entering that loop.

With a stop machine action it is NOT racy because the machine goes into a
special kernel state that guarantees that key operating system structures
are not touched. See mm/page_alloc.c's use of that characteristic to build
zonelists. Thus it cannot be executing for_each_online_cpu and related
tasks (unless one does not disable preempt  but that is a given if a
spinlock has been taken)..

> > Lets get rid of get_online_cpus() etc.
>
> And that solves what?

It gets rid of future issues with serialization in paths were we need to
lock and still do for_each_online_cpu().

> Can you please start to understand the scope of the whole hotplug machinery
> including the requirements for get_online_cpus() before you waste
> everybodys time with your uninformed and halfbaken proposals?

Its an obvious solution to the issues that have arisen multiple times with
get_online_cpus() within the slab allocators. The hotplug machinery should
make things as easy as possible for other people and having these
get_online_cpus() everywhere does complicate things.

Re: [RFC] drm: Helper macro for drm state duplication

[Daniel Vetter]

On Thu, Feb 09, 2017 at 03:41:41PM +, Mihail Atanassov wrote:
> Hi,
> 
> I was working on a few patches adding fields to struct malidp_crtc_state and
> found myself writing memcpy multiple times in the ->atomic_duplicate_state
> hook because I wanted to avoid copying the drm_crtc_state twice
> (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> also applies to the other drm_*_state derivatives, so I concocted a macro
> helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
> one). I'd appreciate some comments on whether anyone else might find this
> macro useful. Thanks!
> 
> Mihail Atanassov (1):
>   drm: Add helper macro for duplicating custom drm_*_state

I don't have your patch here, did something go wrong with the submission?
Only the cover letter seems to have made it through ...
-Daniel

> 
>  include/drm/drm_atomic_helper.h | 33 +
>  1 file changed, 33 insertions(+)
> 
> -- 
> Mihail Atanassov
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC] drm: Helper macro for drm state duplication

[Daniel Vetter]

On Thu, Feb 09, 2017 at 03:41:41PM +, Mihail Atanassov wrote:
> Hi,
> 
> I was working on a few patches adding fields to struct malidp_crtc_state and
> found myself writing memcpy multiple times in the ->atomic_duplicate_state
> hook because I wanted to avoid copying the drm_crtc_state twice
> (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> also applies to the other drm_*_state derivatives, so I concocted a macro
> helper to do the copy in one chunk (two if you count the __drm_atomic_helper*
> one). I'd appreciate some comments on whether anyone else might find this
> macro useful. Thanks!
> 
> Mihail Atanassov (1):
>   drm: Add helper macro for duplicating custom drm_*_state

I don't have your patch here, did something go wrong with the submission?
Only the cover letter seems to have made it through ...
-Daniel

> 
>  include/drm/drm_atomic_helper.h | 33 +
>  1 file changed, 33 insertions(+)
> 
> -- 
> Mihail Atanassov
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: net/sctp: null-ptr-deref in sctp_put_port/sctp_endpoint_destroy

[Xin Long]

On Thu, Feb 9, 2017 at 3:00 AM, Andrey Konovalov  wrote:
> Hi,
>
> I've got the following error report while fuzzing the kernel with syzkaller.
>
> On commit 926af6273fc683cd98cd0ce7bf0d04a02eed6742.
>
> A reproducer and .config are attached.
>
> general protection fault:  [#1] SMP KASAN
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Modules linked in:
> CPU: 3 PID: 19506 Comm: syz-executor0 Not tainted 4.10.0-rc7+ #126
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: 88003d7c1700 task.stack: 88003799
> RIP: 0010:__write_once_size include/linux/compiler.h:272 [inline]
> RIP: 0010:__hlist_del include/linux/list.h:635 [inline]
> RIP: 0010:__sk_del_bind_node include/net/sock.h:685 [inline]
> RIP: 0010:__sctp_put_port net/sctp/socket.c:6994 [inline]
> RIP: 0010:sctp_put_port+0x210/0x620 net/sctp/socket.c:7004
> RSP: 0018:8800379969e0 EFLAGS: 00010246
> RAX: dc10 RBX: 110006f32d41 RCX: 880037996a28
> RDX:  RSI:  RDI: dc00
> RBP: 880037996b10 R08:  R09: 
> R10: 1c8b9dfca637effe R11: dc00 R12: 0005
> R13: 880037996ae8 R14: 88003a066c40 R15: 88003a067178
> FS:  7fd314dba700() GS:88006df0() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 20002ffc CR3: 3b10e000 CR4: 06e0
> Call Trace:
>  sctp_endpoint_destroy net/sctp/endpointola.c:275 [inline]
>  sctp_endpoint_put+0x1b2/0x260 net/sctp/endpointola.c:296
>  sctp_endpoint_free+0x97/0xc0 net/sctp/endpointola.c:238
>  sctp_destroy_sock+0xd6/0x470 net/sctp/socket.c:4254
>  sctp_v6_destroy_sock+0x15/0x20 net/sctp/socket.c:7871
>  sk_common_release+0xbf/0x4e0 net/core/sock.c:2747
>  sctp_close+0x764/0x9f0 net/sctp/socket.c:1550
>  inet_release+0xed/0x1c0 net/ipv4/af_inet.c:425
>  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:431
>  sock_release+0x8d/0x1e0 net/socket.c:599
>  sock_close+0x16/0x20 net/socket.c:1063
>  __fput+0x332/0x7f0 fs/file_table.c:208
>  fput+0x15/0x20 fs/file_table.c:244
>  task_work_run+0x19b/0x270 kernel/task_work.c:116
>  exit_task_work include/linux/task_work.h:21 [inline]
>  do_exit+0x186b/0x2800 kernel/exit.c:839
>  do_group_exit+0x149/0x420 kernel/exit.c:943
>  get_signal+0x76d/0x17e0 kernel/signal.c:2313
>  do_signal+0xd2/0x2190 arch/x86/kernel/signal.c:807
>  exit_to_usermode_loop+0x170/0x200 arch/x86/entry/common.c:156
>  prepare_exit_to_usermode arch/x86/entry/common.c:190 [inline]
>  syscall_return_slowpath+0x3d3/0x420 arch/x86/entry/common.c:259
>  entry_SYSCALL_64_fastpath+0xc0/0xc2
> RIP: 0033:0x4458b9
> RSP: 002b:7fd314db9cf8 EFLAGS: 0246 ORIG_RAX: 00ca
> RAX: fe00 RBX: 00708170 RCX: 004458b9
> RDX:  RSI:  RDI: 00708170
> RBP: 00708150 R08:  R09: 
> R10:  R11: 0246 R12: 
> R13:  R14: 7fd314dba9c0 R15: 7fd314dba700
> Code: ff ff ff 49 8b 76 20 48 bf 00 00 00 00 00 fc ff df 49 89 c8 49
> c1 e8 03 41 c6 04 38 00 49 89 f0 49 89 95 40 ff ff ff 49 c1 e8 03 <41>
> 80 3c 38 00 0f 85 f6 02 00 00 48 c1 e9 03 49 b8 00 00 00 00
> RIP: __write_once_size include/linux/compiler.h:272 [inline] RSP:
> 8800379969e0
> RIP: __hlist_del include/linux/list.h:635 [inline] RSP: 8800379969e0
> RIP: __sk_del_bind_node include/net/sock.h:685 [inline] RSP: 8800379969e0
> RIP: __sctp_put_port net/sctp/socket.c:6994 [inline] RSP: 8800379969e0
> RIP: sctp_put_port+0x210/0x620 net/sctp/socket.c:7004 RSP: 8800379969e0
> ---[ end trace 54c316f8731a5a88 ]---
> Kernel panic - not syncing: Fatal exception in interrupt
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
the script is trying to add a new mrtable and set raw6_sk(sk)->ip6mr_table,
but it's a sctp_sock. to set raw6_sk(sk)->ip6mr_table will cover
sctp_sock(sk)->bind_hash, as [1] and [2] are at the same
offset, this issue then is triggered when release this sk.

struct sctp_sock {
[0x0] struct inet_sock inet;
  [0x3d0] sctp_socket_type_t type;
  [0x3d8] struct sctp_pf *pf;
  [0x3e0] struct crypto_hash *hmac;
  [0x3e8] char *sctp_hmac_alg;
  [0x3f0] struct sctp_endpoint *ep;
  [0x3f8] struct sctp_bind_bucket *bind_hash; <[1]
  ...

struct raw6_sock {
[0x0] struct inet_sock inet;
  [0x3d0] __u32 checksum;
  [0x3d4] __u32 offset;
  [0x3d8] struct icmp6_filter filter;
  [0x3f8] __u32 ip6mr_table; <--[2]
  ...

We may need to check sk->sk_type != SOCK_RAW in the case
MRT6_TABLE in ip6_mroute_setsockopt just as in case MRT6_INIT.
The similar issue may also exists in other protocol sk.

Re: net/sctp: null-ptr-deref in sctp_put_port/sctp_endpoint_destroy

[Xin Long]

On Thu, Feb 9, 2017 at 3:00 AM, Andrey Konovalov  wrote:
> Hi,
>
> I've got the following error report while fuzzing the kernel with syzkaller.
>
> On commit 926af6273fc683cd98cd0ce7bf0d04a02eed6742.
>
> A reproducer and .config are attached.
>
> general protection fault:  [#1] SMP KASAN
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Modules linked in:
> CPU: 3 PID: 19506 Comm: syz-executor0 Not tainted 4.10.0-rc7+ #126
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> task: 88003d7c1700 task.stack: 88003799
> RIP: 0010:__write_once_size include/linux/compiler.h:272 [inline]
> RIP: 0010:__hlist_del include/linux/list.h:635 [inline]
> RIP: 0010:__sk_del_bind_node include/net/sock.h:685 [inline]
> RIP: 0010:__sctp_put_port net/sctp/socket.c:6994 [inline]
> RIP: 0010:sctp_put_port+0x210/0x620 net/sctp/socket.c:7004
> RSP: 0018:8800379969e0 EFLAGS: 00010246
> RAX: dc10 RBX: 110006f32d41 RCX: 880037996a28
> RDX:  RSI:  RDI: dc00
> RBP: 880037996b10 R08:  R09: 
> R10: 1c8b9dfca637effe R11: dc00 R12: 0005
> R13: 880037996ae8 R14: 88003a066c40 R15: 88003a067178
> FS:  7fd314dba700() GS:88006df0() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 20002ffc CR3: 3b10e000 CR4: 06e0
> Call Trace:
>  sctp_endpoint_destroy net/sctp/endpointola.c:275 [inline]
>  sctp_endpoint_put+0x1b2/0x260 net/sctp/endpointola.c:296
>  sctp_endpoint_free+0x97/0xc0 net/sctp/endpointola.c:238
>  sctp_destroy_sock+0xd6/0x470 net/sctp/socket.c:4254
>  sctp_v6_destroy_sock+0x15/0x20 net/sctp/socket.c:7871
>  sk_common_release+0xbf/0x4e0 net/core/sock.c:2747
>  sctp_close+0x764/0x9f0 net/sctp/socket.c:1550
>  inet_release+0xed/0x1c0 net/ipv4/af_inet.c:425
>  inet6_release+0x50/0x70 net/ipv6/af_inet6.c:431
>  sock_release+0x8d/0x1e0 net/socket.c:599
>  sock_close+0x16/0x20 net/socket.c:1063
>  __fput+0x332/0x7f0 fs/file_table.c:208
>  fput+0x15/0x20 fs/file_table.c:244
>  task_work_run+0x19b/0x270 kernel/task_work.c:116
>  exit_task_work include/linux/task_work.h:21 [inline]
>  do_exit+0x186b/0x2800 kernel/exit.c:839
>  do_group_exit+0x149/0x420 kernel/exit.c:943
>  get_signal+0x76d/0x17e0 kernel/signal.c:2313
>  do_signal+0xd2/0x2190 arch/x86/kernel/signal.c:807
>  exit_to_usermode_loop+0x170/0x200 arch/x86/entry/common.c:156
>  prepare_exit_to_usermode arch/x86/entry/common.c:190 [inline]
>  syscall_return_slowpath+0x3d3/0x420 arch/x86/entry/common.c:259
>  entry_SYSCALL_64_fastpath+0xc0/0xc2
> RIP: 0033:0x4458b9
> RSP: 002b:7fd314db9cf8 EFLAGS: 0246 ORIG_RAX: 00ca
> RAX: fe00 RBX: 00708170 RCX: 004458b9
> RDX:  RSI:  RDI: 00708170
> RBP: 00708150 R08:  R09: 
> R10:  R11: 0246 R12: 
> R13:  R14: 7fd314dba9c0 R15: 7fd314dba700
> Code: ff ff ff 49 8b 76 20 48 bf 00 00 00 00 00 fc ff df 49 89 c8 49
> c1 e8 03 41 c6 04 38 00 49 89 f0 49 89 95 40 ff ff ff 49 c1 e8 03 <41>
> 80 3c 38 00 0f 85 f6 02 00 00 48 c1 e9 03 49 b8 00 00 00 00
> RIP: __write_once_size include/linux/compiler.h:272 [inline] RSP:
> 8800379969e0
> RIP: __hlist_del include/linux/list.h:635 [inline] RSP: 8800379969e0
> RIP: __sk_del_bind_node include/net/sock.h:685 [inline] RSP: 8800379969e0
> RIP: __sctp_put_port net/sctp/socket.c:6994 [inline] RSP: 8800379969e0
> RIP: sctp_put_port+0x210/0x620 net/sctp/socket.c:7004 RSP: 8800379969e0
> ---[ end trace 54c316f8731a5a88 ]---
> Kernel panic - not syncing: Fatal exception in interrupt
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..
the script is trying to add a new mrtable and set raw6_sk(sk)->ip6mr_table,
but it's a sctp_sock. to set raw6_sk(sk)->ip6mr_table will cover
sctp_sock(sk)->bind_hash, as [1] and [2] are at the same
offset, this issue then is triggered when release this sk.

struct sctp_sock {
[0x0] struct inet_sock inet;
  [0x3d0] sctp_socket_type_t type;
  [0x3d8] struct sctp_pf *pf;
  [0x3e0] struct crypto_hash *hmac;
  [0x3e8] char *sctp_hmac_alg;
  [0x3f0] struct sctp_endpoint *ep;
  [0x3f8] struct sctp_bind_bucket *bind_hash; <[1]
  ...

struct raw6_sock {
[0x0] struct inet_sock inet;
  [0x3d0] __u32 checksum;
  [0x3d4] __u32 offset;
  [0x3d8] struct icmp6_filter filter;
  [0x3f8] __u32 ip6mr_table; <--[2]
  ...

We may need to check sk->sk_type != SOCK_RAW in the case
MRT6_TABLE in ip6_mroute_setsockopt just as in case MRT6_INIT.
The similar issue may also exists in other protocol sk.

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Andrew Lunn]

> Adding a 'select' statement to something as broad as NETDEVICES sounds
> really bad, it has a significant risk of introducing dependency loops
> and may be confusing if you want to build a multiplatform config without
> networking support (note that NETDEVICES in turn depends on NET, which
> can also be disabled).

O.K, so overall it is not simple. So lets drop my idea.
 
> One possibility would be to have a special Kconfig symbol that controls
> mdiobus_register_board_info() being present and have that symbol
> force PHYLIB to never be "=m". Then we can either have no networking
> support and no phylib, turning mdiobus_register_board_info() into a
> stub, or we have the function built-in and reachable from the board
> code.

FYI: Florian is working on splitting MDIO out of PHYLIB. There will be
two separate symbols, so it will be possible to have MDIO without
PHYLIB. When this happens, i expect mdiobus_register_board_info() will
be in the MDIO part, not PHYLIB.

   Andrew

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Andrew Lunn]

> Adding a 'select' statement to something as broad as NETDEVICES sounds
> really bad, it has a significant risk of introducing dependency loops
> and may be confusing if you want to build a multiplatform config without
> networking support (note that NETDEVICES in turn depends on NET, which
> can also be disabled).

O.K, so overall it is not simple. So lets drop my idea.
 
> One possibility would be to have a special Kconfig symbol that controls
> mdiobus_register_board_info() being present and have that symbol
> force PHYLIB to never be "=m". Then we can either have no networking
> support and no phylib, turning mdiobus_register_board_info() into a
> stub, or we have the function built-in and reachable from the board
> code.

FYI: Florian is working on splitting MDIO out of PHYLIB. There will be
two separate symbols, so it will be possible to have MDIO without
PHYLIB. When this happens, i expect mdiobus_register_board_info() will
be in the MDIO part, not PHYLIB.

   Andrew

Alpha Kernel Regression [was Re: BRSGP relocation truncations in linking kernel for Alpha.]

[Michael Cree]

On Tue, Oct 25, 2016 at 09:26:38PM +1300, Michael Cree wrote:
> And while I mention libc I am seeing (rather rare) random segfaults
> in programs such as cp, tar, install and dpkg ever since the upgrade
> to glibc 2.23 (or maybe it was 2.24).  I am struggling to get a
> backtrace because it only happens very occassionally (but often enough
> that it is almost impossible for a build and install of large software
> packages such as libreoffice to complete without failure at some
> random point) and when I rerun the failing program manually it then
> always works.  I'll keep trying to narrow this one down.

It's not glibc.  Downgrading to previously known working versions does
not solve the random segfaults.  But downgrading the kernel does fix
the problem on Alpha.  Noted that 4.6 is good but 4.6.7 is bad so
bisected the 4.6.y stable kernel branch to get the first bad commit as
0784672d05684de901fc2aa56150d7ea9a475a2d, i.e.:

commit 0784672d05684de901fc2aa56150d7ea9a475a2d
Author: Chen Feng 
Date:   Fri May 20 16:59:02 2016 -0700

mm/compaction.c: fix zoneindex in kcompactd()

commit 6cd9dc3e75078ef646076fa63adfb9b85ced0b66 upstream.

While testing the kcompactd in my platform 3G MEM only DMA ZONE.  I
found the kcompactd never wakeup.  It seems the zoneindex has already
minus 1 before.  So the traverse here should be <=.

It fixes a regression where kswapd could previously compact, but
kcompactd not.  Not a crash fix though.

Cheers
Michael.

Alpha Kernel Regression [was Re: BRSGP relocation truncations in linking kernel for Alpha.]

[Michael Cree]

On Tue, Oct 25, 2016 at 09:26:38PM +1300, Michael Cree wrote:
> And while I mention libc I am seeing (rather rare) random segfaults
> in programs such as cp, tar, install and dpkg ever since the upgrade
> to glibc 2.23 (or maybe it was 2.24).  I am struggling to get a
> backtrace because it only happens very occassionally (but often enough
> that it is almost impossible for a build and install of large software
> packages such as libreoffice to complete without failure at some
> random point) and when I rerun the failing program manually it then
> always works.  I'll keep trying to narrow this one down.

It's not glibc.  Downgrading to previously known working versions does
not solve the random segfaults.  But downgrading the kernel does fix
the problem on Alpha.  Noted that 4.6 is good but 4.6.7 is bad so
bisected the 4.6.y stable kernel branch to get the first bad commit as
0784672d05684de901fc2aa56150d7ea9a475a2d, i.e.:

commit 0784672d05684de901fc2aa56150d7ea9a475a2d
Author: Chen Feng 
Date:   Fri May 20 16:59:02 2016 -0700

mm/compaction.c: fix zoneindex in kcompactd()

commit 6cd9dc3e75078ef646076fa63adfb9b85ced0b66 upstream.

While testing the kcompactd in my platform 3G MEM only DMA ZONE.  I
found the kcompactd never wakeup.  It seems the zoneindex has already
minus 1 before.  So the traverse here should be <=.

It fixes a regression where kswapd could previously compact, but
kcompactd not.  Not a crash fix though.

Cheers
Michael.

Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible deadlock case in kretprobe

[Russell King - ARM Linux]

On Fri, Feb 10, 2017 at 01:32:22AM +0900, Masami Hiramatsu wrote:
> Fix a possibility of deadlock case in kretprobe on arm
> implementation. There may be a chance that the kretprobe
> hash table lock can cause a dead lock.
> 
> The senario is that a user puts 2 kretprobes, one on normal
> function and one on a function which can be called from
> somewhare which can interrupt in irq disabled critical
> section like FIQ.

If we:
- hit a kernel tracing feature from FIQ context
- the tracing feature takes a lock
- the lock is also taken elsewhere on the same CPU with IRQs disabled

we will quite simply deadlock.

In this case, kretprobe_hash_lock() takes the hlist_lock using
raw_spin_lock_irqsave().

Now, from what I can see in the kprobes code, this lock is taken in
other contexts (eg, kprobe_flush_task()), which means even with this
fix, it's still risky if a kprobe is placed on a FIQ-called function.

> In this case, if the kernel hits the 1st kretprobe on a
> normal function return which calls trampoline_handler(),
> acquire a spinlock on the hash table in kretprobe_hash_lock()
> and disable irqs. After that, if the 2nd kretprobe is kicked
> from FIQ, it also calls trampoline_handler() and tries to
> acquire the same spinlock (since the hash is based on
> current task, same as the 1st kretprobe), it causes
> a deadlock.

So my deadlock scenario is:

- we're in the middle of kprobe_flush_task()
- FIQ happens, calls trampoline_handler()
- deadlock in kretprobe_hash_lock()

>From what I can see, kretprobes in FIQ are just unsafe.

I suspect that avoiding these deadlocks means that we have to deny
kprobes from FIQ context - making trampoline_handler() return
immediately if in_nmi() is true.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [BUGFIX PATCH tip/master V2 3/3] kprobes/arm: Fix a possible deadlock case in kretprobe

[Russell King - ARM Linux]

On Fri, Feb 10, 2017 at 01:32:22AM +0900, Masami Hiramatsu wrote:
> Fix a possibility of deadlock case in kretprobe on arm
> implementation. There may be a chance that the kretprobe
> hash table lock can cause a dead lock.
> 
> The senario is that a user puts 2 kretprobes, one on normal
> function and one on a function which can be called from
> somewhare which can interrupt in irq disabled critical
> section like FIQ.

If we:
- hit a kernel tracing feature from FIQ context
- the tracing feature takes a lock
- the lock is also taken elsewhere on the same CPU with IRQs disabled

we will quite simply deadlock.

In this case, kretprobe_hash_lock() takes the hlist_lock using
raw_spin_lock_irqsave().

Now, from what I can see in the kprobes code, this lock is taken in
other contexts (eg, kprobe_flush_task()), which means even with this
fix, it's still risky if a kprobe is placed on a FIQ-called function.

> In this case, if the kernel hits the 1st kretprobe on a
> normal function return which calls trampoline_handler(),
> acquire a spinlock on the hash table in kretprobe_hash_lock()
> and disable irqs. After that, if the 2nd kretprobe is kicked
> from FIQ, it also calls trampoline_handler() and tries to
> acquire the same spinlock (since the hash is based on
> current task, same as the 1st kretprobe), it causes
> a deadlock.

So my deadlock scenario is:

- we're in the middle of kprobe_flush_task()
- FIQ happens, calls trampoline_handler()
- deadlock in kretprobe_hash_lock()

>From what I can see, kretprobes in FIQ are just unsafe.

I suspect that avoiding these deadlocks means that we have to deny
kprobes from FIQ context - making trampoline_handler() return
immediately if in_nmi() is true.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH 0/3] KEYS: Fixes

[David Howells]

Hi James,

Can you pull these patches into your next tree please?  They include the
following:

 (1) Fix sign-file for use with libressl.

 (2) Fix error production in request_master_key().

 (3) Explicitly zero-out secret data before freeing it in case gcc
 optimises memset() away in future.

I don't think there's anything urgent enough here to warrant handing
directly to Linus.

The patches can be found here also:


http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-fixes

Tagged thusly:

git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
keys-fixes-20170209

David
---
Dan Carpenter (2):
  KEYS: Fix an error code in request_master_key()
  KEYS: Use memzero_explicit() for secret data

Felix Fietkau (1):
  sign-file: fix build error in sign-file.c with libressl


 scripts/sign-file.c  |4 +++-
 security/keys/encrypted-keys/encrypted.c |4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

[PATCH 2/3] KEYS: Fix an error code in request_master_key()

[David Howells]

From: Dan Carpenter 

This function has two callers and neither are able to handle a NULL
return.  Really, -EINVAL is the correct thing return here anyway.  This
fixes some static checker warnings like:

security/keys/encrypted-keys/encrypted.c:709 encrypted_key_decrypt()
error: uninitialized symbol 'master_key'.

Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
Signed-off-by: Dan Carpenter 
Acked-by: Mimi Zohar 
---

 security/keys/encrypted-keys/encrypted.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 17a06105ccb6..d7a4969b2dd3 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -437,7 +437,7 @@ static struct skcipher_request *init_skcipher_req(const u8 
*key,
 static struct key *request_master_key(struct encrypted_key_payload *epayload,
  const u8 **master_key, size_t 
*master_keylen)
 {
-   struct key *mkey = NULL;
+   struct key *mkey = ERR_PTR(-EINVAL);
 
if (!strncmp(epayload->master_desc, KEY_TRUSTED_PREFIX,
 KEY_TRUSTED_PREFIX_LEN)) {

[PATCH 0/3] KEYS: Fixes

[David Howells]

Hi James,

Can you pull these patches into your next tree please?  They include the
following:

 (1) Fix sign-file for use with libressl.

 (2) Fix error production in request_master_key().

 (3) Explicitly zero-out secret data before freeing it in case gcc
 optimises memset() away in future.

I don't think there's anything urgent enough here to warrant handing
directly to Linus.

The patches can be found here also:


http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-fixes

Tagged thusly:

git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
keys-fixes-20170209

David
---
Dan Carpenter (2):
  KEYS: Fix an error code in request_master_key()
  KEYS: Use memzero_explicit() for secret data

Felix Fietkau (1):
  sign-file: fix build error in sign-file.c with libressl


 scripts/sign-file.c  |4 +++-
 security/keys/encrypted-keys/encrypted.c |4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

[PATCH 2/3] KEYS: Fix an error code in request_master_key()

[David Howells]

From: Dan Carpenter 

This function has two callers and neither are able to handle a NULL
return.  Really, -EINVAL is the correct thing return here anyway.  This
fixes some static checker warnings like:

security/keys/encrypted-keys/encrypted.c:709 encrypted_key_decrypt()
error: uninitialized symbol 'master_key'.

Fixes: 7e70cb497850 ("keys: add new key-type encrypted")
Signed-off-by: Dan Carpenter 
Acked-by: Mimi Zohar 
---

 security/keys/encrypted-keys/encrypted.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index 17a06105ccb6..d7a4969b2dd3 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -437,7 +437,7 @@ static struct skcipher_request *init_skcipher_req(const u8 
*key,
 static struct key *request_master_key(struct encrypted_key_payload *epayload,
  const u8 **master_key, size_t 
*master_keylen)
 {
-   struct key *mkey = NULL;
+   struct key *mkey = ERR_PTR(-EINVAL);
 
if (!strncmp(epayload->master_desc, KEY_TRUSTED_PREFIX,
 KEY_TRUSTED_PREFIX_LEN)) {

Re: [RFC][PATCH 00/21] tracing: Inter-event (e.g. latency) support

[Tom Zanussi]

On Thu, 2017-02-09 at 23:18 +0900, Masami Hiramatsu wrote:
> Hi Tom,
> 
> On Wed, 08 Feb 2017 19:14:22 -0600
> Tom Zanussi  wrote:
> 
> > > > I'm submitting the patchset (based on tracing/for-next) as an RFC not
> > > > only to get comments, but because there are still some problems I
> > > > haven't fixed yet...
> > > > 
> > > > Here are some examples that should make things less abstract.
> > > > 
> > > >   
> > > >   Example - wakeup latency
> > > >   
> > > > 
> > > >   This basically implements the -RT latency_hist 'wakeup_latency'
> > > >   histogram using the synthetic events, variables, and actions
> > > >   described.  The output below is from a run of cyclictest using the
> > > >   following command:
> > > > 
> > > > # rt-tests/cyclictest -p 80 -n -s -t 2
> > > > 
> > > >   What we're measuring the latency of is the time between when a
> > > >   thread (of cyclictest) is awakened and when it's scheduled in.  To
> > > >   do that we add triggers to sched_wakeup and sched_switch with the
> > > >   appropriate variables, and on a matching sched_switch event,
> > > >   generate a synthetic 'wakeup_latency' event.  Since it's just
> > > >   another trace event like any other, we can also define a histogram
> > > >   on that event, the output of which is what we see displayed when
> > > >   reading the wakeup_latency 'hist' file.
> > > > 
> > > >   First, we create a synthetic event called wakeup_latency, that
> > > >   references 3 variables from other events:
> > > > 
> > > > # echo 'wakeup_latency lat=sched_switch:wakeup_lat \
> > > >pid=sched_switch:woken_pid \
> > > >prio=sched_switch:woken_prio' >> \
> > > > /sys/kernel/debug/tracing/synthetic_events
> > > > 
> > > >   Next we add a trigger to sched_wakeup, which saves the value of the
> > > >   'common_timestamp' when that event is hit in a variable, ts0.  Note
> > > >   that this happens only when 'comm==cyclictest'.
> > > > 
> > > >   Also, 'common_timestamp' is a new field defined on every event (if
> > > >   needed - if there are no users of timestamps in a trace, timestamps
> > > >   won't be saved and there's no additional overhead from that).
> > > > 
> > > > #  echo 'hist:keys=pid:ts0=common_timestamp.usecs if \
> > > >  comm=="cyclictest"' >> \
> > > >  /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
> > > > 
> > > >   Next, we add a trigger to sched_switch.  When the pid being switched
> > > >   to matches the pid woken up by a previous sched_wakeup event, this
> > > >   event grabs the ts0 saved on that event, takes the difference
> > > >   between it and the current sched_switch's common_timestamp, and
> > > >   assigns it to a new 'wakeup_lat' variable.  It also saves a couple
> > > >   other variables and then invokes the onmatch().trace() action which
> > > >   generates a new wakeup_latency event using those variables.
> > > > 
> > > > # echo 'hist:keys=woken_pid=next_pid:woken_prio=next_prio:\
> > > >
> > > > wakeup_lat=common_timestamp.usecs-ts0:onmatch().trace(wakeup_latency) \
> > > > if next_comm=="cyclictest"' >> \
> > > > /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
> > > 
> > > Hmm, this looks a bit hard to understand, I guess that onmatch() means
> > > "if there is an event which has ts0 variable and the event's key matches
> > > this key, take some action".
> > 
> > Yes, that's pretty much it. It's essentially shorthand for this kind of
> > common idiom, where timestamp[] is an associative array, which in our
> > case is the tracing_map of the histogram: 
> > 
> > event sched_wakeup()
> > {
> > ts0[wakeup_pid] = now()
> > }
> > 
> > event sched_switch()
> > {
> > if (ts0[next_pid])
> > latency = now() - ts0[next_pid] /* next_pid == wakeup_pid */
> > }
> > 
> > Only if ts0 has already been set does the onmatch() get invoked - if ts0
> > hasn't been set, there's no match and the trace(wakeup_latency) doesn't
> > happen.
> 
> OK, it reminds me other questions.
> 
> - Even if there is no matched ts0, sched_switch's hist will store
>   woken_pid etc on its histogram map?

Yes, the match is just to invoke the onmatch() action, but the variables
are set regardless.

> - If there is matched ts0 and wakeup_latency event has been kicked,
>   the matched entry on ts0 is removed? and also in that case what
>   happens on sched_switch's hist?
> 

The entry isn't actually removed, but as far as ts0 goes, the result is
the same as if it had been - ts0 is a read-once variable, so once it's
used by the latency calculation, it's reset to an 'unset' after reading.
This essentially is how the 'if ts0[next_pid]' gets implemented -
actually, I should have added the implied removal to the pseudocode
above:

if (ts0[next_pid])
latency = now() - ts0[next_pid]
ts0[next_pid] = null

The 

Re: [RFC][PATCH 00/21] tracing: Inter-event (e.g. latency) support

[Tom Zanussi]

On Thu, 2017-02-09 at 23:18 +0900, Masami Hiramatsu wrote:
> Hi Tom,
> 
> On Wed, 08 Feb 2017 19:14:22 -0600
> Tom Zanussi  wrote:
> 
> > > > I'm submitting the patchset (based on tracing/for-next) as an RFC not
> > > > only to get comments, but because there are still some problems I
> > > > haven't fixed yet...
> > > > 
> > > > Here are some examples that should make things less abstract.
> > > > 
> > > >   
> > > >   Example - wakeup latency
> > > >   
> > > > 
> > > >   This basically implements the -RT latency_hist 'wakeup_latency'
> > > >   histogram using the synthetic events, variables, and actions
> > > >   described.  The output below is from a run of cyclictest using the
> > > >   following command:
> > > > 
> > > > # rt-tests/cyclictest -p 80 -n -s -t 2
> > > > 
> > > >   What we're measuring the latency of is the time between when a
> > > >   thread (of cyclictest) is awakened and when it's scheduled in.  To
> > > >   do that we add triggers to sched_wakeup and sched_switch with the
> > > >   appropriate variables, and on a matching sched_switch event,
> > > >   generate a synthetic 'wakeup_latency' event.  Since it's just
> > > >   another trace event like any other, we can also define a histogram
> > > >   on that event, the output of which is what we see displayed when
> > > >   reading the wakeup_latency 'hist' file.
> > > > 
> > > >   First, we create a synthetic event called wakeup_latency, that
> > > >   references 3 variables from other events:
> > > > 
> > > > # echo 'wakeup_latency lat=sched_switch:wakeup_lat \
> > > >pid=sched_switch:woken_pid \
> > > >prio=sched_switch:woken_prio' >> \
> > > > /sys/kernel/debug/tracing/synthetic_events
> > > > 
> > > >   Next we add a trigger to sched_wakeup, which saves the value of the
> > > >   'common_timestamp' when that event is hit in a variable, ts0.  Note
> > > >   that this happens only when 'comm==cyclictest'.
> > > > 
> > > >   Also, 'common_timestamp' is a new field defined on every event (if
> > > >   needed - if there are no users of timestamps in a trace, timestamps
> > > >   won't be saved and there's no additional overhead from that).
> > > > 
> > > > #  echo 'hist:keys=pid:ts0=common_timestamp.usecs if \
> > > >  comm=="cyclictest"' >> \
> > > >  /sys/kernel/debug/tracing/events/sched/sched_wakeup/trigger
> > > > 
> > > >   Next, we add a trigger to sched_switch.  When the pid being switched
> > > >   to matches the pid woken up by a previous sched_wakeup event, this
> > > >   event grabs the ts0 saved on that event, takes the difference
> > > >   between it and the current sched_switch's common_timestamp, and
> > > >   assigns it to a new 'wakeup_lat' variable.  It also saves a couple
> > > >   other variables and then invokes the onmatch().trace() action which
> > > >   generates a new wakeup_latency event using those variables.
> > > > 
> > > > # echo 'hist:keys=woken_pid=next_pid:woken_prio=next_prio:\
> > > >
> > > > wakeup_lat=common_timestamp.usecs-ts0:onmatch().trace(wakeup_latency) \
> > > > if next_comm=="cyclictest"' >> \
> > > > /sys/kernel/debug/tracing/events/sched/sched_switch/trigger
> > > 
> > > Hmm, this looks a bit hard to understand, I guess that onmatch() means
> > > "if there is an event which has ts0 variable and the event's key matches
> > > this key, take some action".
> > 
> > Yes, that's pretty much it. It's essentially shorthand for this kind of
> > common idiom, where timestamp[] is an associative array, which in our
> > case is the tracing_map of the histogram: 
> > 
> > event sched_wakeup()
> > {
> > ts0[wakeup_pid] = now()
> > }
> > 
> > event sched_switch()
> > {
> > if (ts0[next_pid])
> > latency = now() - ts0[next_pid] /* next_pid == wakeup_pid */
> > }
> > 
> > Only if ts0 has already been set does the onmatch() get invoked - if ts0
> > hasn't been set, there's no match and the trace(wakeup_latency) doesn't
> > happen.
> 
> OK, it reminds me other questions.
> 
> - Even if there is no matched ts0, sched_switch's hist will store
>   woken_pid etc on its histogram map?

Yes, the match is just to invoke the onmatch() action, but the variables
are set regardless.

> - If there is matched ts0 and wakeup_latency event has been kicked,
>   the matched entry on ts0 is removed? and also in that case what
>   happens on sched_switch's hist?
> 

The entry isn't actually removed, but as far as ts0 goes, the result is
the same as if it had been - ts0 is a read-once variable, so once it's
used by the latency calculation, it's reset to an 'unset' after reading.
This essentially is how the 'if ts0[next_pid]' gets implemented -
actually, I should have added the implied removal to the pseudocode
above:

if (ts0[next_pid])
latency = now() - ts0[next_pid]
ts0[next_pid] = null

The variables on sched_switch, 

Re: [RFC] drm: Helper macro for drm state duplication

[Liviu Dudau]

On Thu, Feb 09, 2017 at 05:57:04PM +0100, Daniel Vetter wrote:
> On Thu, Feb 09, 2017 at 03:41:41PM +, Mihail Atanassov wrote:
> > Hi,
> > 
> > I was working on a few patches adding fields to struct malidp_crtc_state and
> > found myself writing memcpy multiple times in the ->atomic_duplicate_state
> > hook because I wanted to avoid copying the drm_crtc_state twice
> > (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> > also applies to the other drm_*_state derivatives, so I concocted a macro
> > helper to do the copy in one chunk (two if you count the 
> > __drm_atomic_helper*
> > one). I'd appreciate some comments on whether anyone else might find this
> > macro useful. Thanks!
> > 
> > Mihail Atanassov (1):
> >   drm: Add helper macro for duplicating custom drm_*_state
> 
> I don't have your patch here, did something go wrong with the submission?
> Only the cover letter seems to have made it through ...

There is another patch that should come through, I can see it in my home 
account.

Liviu

> -Daniel
> 
> > 
> >  include/drm/drm_atomic_helper.h | 33 +
> >  1 file changed, 33 insertions(+)
> > 
> > -- 
> > Mihail Atanassov
> > 
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯

Re: [RFC] drm: Helper macro for drm state duplication

[Liviu Dudau]

On Thu, Feb 09, 2017 at 05:57:04PM +0100, Daniel Vetter wrote:
> On Thu, Feb 09, 2017 at 03:41:41PM +, Mihail Atanassov wrote:
> > Hi,
> > 
> > I was working on a few patches adding fields to struct malidp_crtc_state and
> > found myself writing memcpy multiple times in the ->atomic_duplicate_state
> > hook because I wanted to avoid copying the drm_crtc_state twice
> > (__drm_atomic_helper_crtc_duplicate_state copies it already). I figured this
> > also applies to the other drm_*_state derivatives, so I concocted a macro
> > helper to do the copy in one chunk (two if you count the 
> > __drm_atomic_helper*
> > one). I'd appreciate some comments on whether anyone else might find this
> > macro useful. Thanks!
> > 
> > Mihail Atanassov (1):
> >   drm: Add helper macro for duplicating custom drm_*_state
> 
> I don't have your patch here, did something go wrong with the submission?
> Only the cover letter seems to have made it through ...

There is another patch that should come through, I can see it in my home 
account.

Liviu

> -Daniel
> 
> > 
> >  include/drm/drm_atomic_helper.h | 33 +
> >  1 file changed, 33 insertions(+)
> > 
> > -- 
> > Mihail Atanassov
> > 
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯

Re: [PATCH v3] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Ben Gardner]

Ignore this last patch. I should remember to test even the simplest of
cut-and-paste ops.

On Thu, Feb 9, 2017 at 11:09 AM, Ben Gardner  wrote:
> Allow the at24 driver to get configuration information from both OF and
> ACPI by using the more generic device_property functions.
> This change was inspired by the at25.c driver.
>
> I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
> With the following ACPI construct, this patch instantiates a working
> instance of the driver.
>
> Device (EEP0) {
>  Name (_HID, "PRP0001")
>  Name (_DSD, Package () {
>   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>   Package () {
>Package () {"compatible", Package () {"st,24c02"}},
>Package () {"pagesize", 16},
>   },
>  })
>  Name (_CRS, ResourceTemplate () {
>   I2cSerialBus (
>0x0057, ControllerInitiated, 40,
>AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
>ResourceConsumer,,)
>  })
> }
>
> Note: Matching the driver to the I2C device requires another patch.
>  http://www.spinics.net/lists/linux-acpi/msg71914.html
>
> Signed-off-by: Ben Gardner 
> ---
>  drivers/misc/eeprom/at24.c | 45 +++--
>  1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 051b147..c2d9969 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -19,7 +19,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, 
> void *val, size_t count)
> return 0;
>  }
>
> -#ifdef CONFIG_OF
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> +static void at24_get_pdata(struct device *dev, struct at24_platform_data 
> *chip)
>  {
> -   const __be32 *val;
> -   struct device_node *node = client->dev.of_node;
> -
> -   if (node) {
> -   if (of_get_property(node, "read-only", NULL))
> -   chip->flags |= AT24_FLAG_READONLY;
> -   val = of_get_property(node, "pagesize", NULL);
> -   if (val)
> -   chip->page_size = be32_to_cpup(val);
> +   int err;
> +   u32 val;
> +
> +   if (device_property_present(dev, "read-only"))
> +   chip->flags |= AT24_FLAG_READONLY;
> +
> +   err = device_property_read_u32(dev, "pagesize", );
> +   if (!err) {
> +   chip->page_size = val;
> +   } else {
> +   /*
> +* This is slow, but we can't know all eeproms, so we better
> +* play safe. Specifying custom eeprom-types via platform_data
> +* is recommended anyhow.
> +*/
> +   chip.page_size = 1;
> }
>  }
> -#else
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> -{ }
> -#endif /* CONFIG_OF */
>
>  static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
> *id)
>  {
> @@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
> struct i2c_device_id *id)
> chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
> magic >>= AT24_SIZE_BYTELEN;
> chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
> -   /*
> -* This is slow, but we can't know all eeproms, so we better
> -* play safe. Specifying custom eeprom-types via platform_data
> -* is recommended anyhow.
> -*/
> -   chip.page_size = 1;
>
> -   /* update chipdata if OF is present */
> -   at24_get_ofdata(client, );
> +   at24_get_pdata(>dev, );
>
> chip.setup = NULL;
> chip.context = NULL;
> --
> 2.7.4
>

Re: [PATCH v3] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Ben Gardner]

Ignore this last patch. I should remember to test even the simplest of
cut-and-paste ops.

On Thu, Feb 9, 2017 at 11:09 AM, Ben Gardner  wrote:
> Allow the at24 driver to get configuration information from both OF and
> ACPI by using the more generic device_property functions.
> This change was inspired by the at25.c driver.
>
> I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
> With the following ACPI construct, this patch instantiates a working
> instance of the driver.
>
> Device (EEP0) {
>  Name (_HID, "PRP0001")
>  Name (_DSD, Package () {
>   ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>   Package () {
>Package () {"compatible", Package () {"st,24c02"}},
>Package () {"pagesize", 16},
>   },
>  })
>  Name (_CRS, ResourceTemplate () {
>   I2cSerialBus (
>0x0057, ControllerInitiated, 40,
>AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
>ResourceConsumer,,)
>  })
> }
>
> Note: Matching the driver to the I2C device requires another patch.
>  http://www.spinics.net/lists/linux-acpi/msg71914.html
>
> Signed-off-by: Ben Gardner 
> ---
>  drivers/misc/eeprom/at24.c | 45 +++--
>  1 file changed, 19 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
> index 051b147..c2d9969 100644
> --- a/drivers/misc/eeprom/at24.c
> +++ b/drivers/misc/eeprom/at24.c
> @@ -19,7 +19,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, 
> void *val, size_t count)
> return 0;
>  }
>
> -#ifdef CONFIG_OF
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> +static void at24_get_pdata(struct device *dev, struct at24_platform_data 
> *chip)
>  {
> -   const __be32 *val;
> -   struct device_node *node = client->dev.of_node;
> -
> -   if (node) {
> -   if (of_get_property(node, "read-only", NULL))
> -   chip->flags |= AT24_FLAG_READONLY;
> -   val = of_get_property(node, "pagesize", NULL);
> -   if (val)
> -   chip->page_size = be32_to_cpup(val);
> +   int err;
> +   u32 val;
> +
> +   if (device_property_present(dev, "read-only"))
> +   chip->flags |= AT24_FLAG_READONLY;
> +
> +   err = device_property_read_u32(dev, "pagesize", );
> +   if (!err) {
> +   chip->page_size = val;
> +   } else {
> +   /*
> +* This is slow, but we can't know all eeproms, so we better
> +* play safe. Specifying custom eeprom-types via platform_data
> +* is recommended anyhow.
> +*/
> +   chip.page_size = 1;
> }
>  }
> -#else
> -static void at24_get_ofdata(struct i2c_client *client,
> -   struct at24_platform_data *chip)
> -{ }
> -#endif /* CONFIG_OF */
>
>  static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
> *id)
>  {
> @@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
> struct i2c_device_id *id)
> chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
> magic >>= AT24_SIZE_BYTELEN;
> chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
> -   /*
> -* This is slow, but we can't know all eeproms, so we better
> -* play safe. Specifying custom eeprom-types via platform_data
> -* is recommended anyhow.
> -*/
> -   chip.page_size = 1;
>
> -   /* update chipdata if OF is present */
> -   at24_get_ofdata(client, );
> +   at24_get_pdata(>dev, );
>
> chip.setup = NULL;
> chip.context = NULL;
> --
> 2.7.4
>

[PATCH 1/3] sign-file: fix build error in sign-file.c with libressl

[David Howells]

From: Felix Fietkau 

The sign-file tool failed to build against libressl. Fix this by extending
the PKCS7 check and thus making sign-file link against libressl without an
error.

Signed-off-by: John Crispin 
Signed-off-by: Felix Fietkau 
Signed-off-by: David Howells 
---

 scripts/sign-file.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 19ec468b1168..fbd34b8e8f57 100644
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -41,7 +41,9 @@
  * signing with anything other than SHA1 - so we're stuck with that if such is
  * the case.
  */
-#if OPENSSL_VERSION_NUMBER < 0x1000L || defined(OPENSSL_NO_CMS)
+#if defined(LIBRESSL_VERSION_NUMBER) || \
+   OPENSSL_VERSION_NUMBER < 0x1000L || \
+   defined(OPENSSL_NO_CMS)
 #define USE_PKCS7
 #endif
 #ifndef USE_PKCS7

[PATCH 1/3] sign-file: fix build error in sign-file.c with libressl

[David Howells]

From: Felix Fietkau 

The sign-file tool failed to build against libressl. Fix this by extending
the PKCS7 check and thus making sign-file link against libressl without an
error.

Signed-off-by: John Crispin 
Signed-off-by: Felix Fietkau 
Signed-off-by: David Howells 
---

 scripts/sign-file.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/sign-file.c b/scripts/sign-file.c
index 19ec468b1168..fbd34b8e8f57 100644
--- a/scripts/sign-file.c
+++ b/scripts/sign-file.c
@@ -41,7 +41,9 @@
  * signing with anything other than SHA1 - so we're stuck with that if such is
  * the case.
  */
-#if OPENSSL_VERSION_NUMBER < 0x1000L || defined(OPENSSL_NO_CMS)
+#if defined(LIBRESSL_VERSION_NUMBER) || \
+   OPENSSL_VERSION_NUMBER < 0x1000L || \
+   defined(OPENSSL_NO_CMS)
 #define USE_PKCS7
 #endif
 #ifndef USE_PKCS7

[PATCH 3/3] KEYS: Use memzero_explicit() for secret data

[David Howells]

From: Dan Carpenter 

I don't think GCC has figured out how to optimize the memset() away, but
they might eventually so let's future proof this code a bit.

Signed-off-by: Dan Carpenter 
Signed-off-by: David Howells 
Acked-by: Mimi Zohar 
---

 security/keys/encrypted-keys/encrypted.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index d7a4969b2dd3..4fb315cddf5b 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -985,7 +985,7 @@ static void encrypted_destroy(struct key *key)
if (!epayload)
return;
 
-   memset(epayload->decrypted_data, 0, epayload->decrypted_datalen);
+   memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen);
kfree(key->payload.data[0]);
 }
 

[PATCH 3/3] KEYS: Use memzero_explicit() for secret data

[David Howells]

From: Dan Carpenter 

I don't think GCC has figured out how to optimize the memset() away, but
they might eventually so let's future proof this code a bit.

Signed-off-by: Dan Carpenter 
Signed-off-by: David Howells 
Acked-by: Mimi Zohar 
---

 security/keys/encrypted-keys/encrypted.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/encrypted-keys/encrypted.c 
b/security/keys/encrypted-keys/encrypted.c
index d7a4969b2dd3..4fb315cddf5b 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -985,7 +985,7 @@ static void encrypted_destroy(struct key *key)
if (!epayload)
return;
 
-   memset(epayload->decrypted_data, 0, epayload->decrypted_datalen);
+   memzero_explicit(epayload->decrypted_data, epayload->decrypted_datalen);
kfree(key->payload.data[0]);
 }
 

[PATCH v2 4/4] iio: chemical: add dsm501 particle sensor driver bindings

[Tomasz Duszynski]

This patch adds bindings for dsm501 particle sensor driver.

Signed-off-by: Tomasz Duszynski 
---
 Changes in v2:
  o add 'data' prefix to gpios property

 .../devicetree/bindings/iio/chemical/dsm501.txt  | 16 
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/dsm501.txt

diff --git a/Documentation/devicetree/bindings/iio/chemical/dsm501.txt 
b/Documentation/devicetree/bindings/iio/chemical/dsm501.txt
new file mode 100644
index ..126ce528f5eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/dsm501.txt
@@ -0,0 +1,16 @@
+* DSM501 particle sensor
+
+Required properties:
+
+- compatible: should be one of:
+ "samyoung,dsm501"
+ "shinyei,ppd42ns"
+- data-gpios: should specify the GPIO connected to the sensor's data line, see
+ "gpios property" in 
Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+
+dsm501 {
+   compatible = "samyoung,dsm501";
+   data-gpios = < 28 0>;
+};
--
2.11.1

[PATCH v3 2/4] iio: chemical: add driver for dsm501/ppd42ns particle sensors

[Tomasz Duszynski]

This patch adds support for dsm501 and ppd42ns particle sensors.

Both sensors work on the same principle. Heater (resistor) heats up air
in sensor chamber which induces upward flow. Particles convect up through
a light beam provided by internal infra-red LED. Light scattered by
particles is picked by photodiode and internal electronics outputs PWM
signal.

Measuring low time occupancy of the signal during 30s measurement period
particles number in cubic meter can be computed.

Signed-off-by: Tomasz Duszynski 
---
 Changes in v3:
  o use devm_gpiod_get() instead of indexed version. This is due to bindings
change.

 Changes in v2:
  o whitespace fix
  o drop excessive comments about conversion to pcs/0.01cf
  o fix 'concentartion' typo in dsm501_number_concentartion handler name

 drivers/iio/chemical/Kconfig  |  10 ++
 drivers/iio/chemical/Makefile |   1 +
 drivers/iio/chemical/dsm501.c | 225 ++
 3 files changed, 236 insertions(+)
 create mode 100644 drivers/iio/chemical/dsm501.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index cea7f9857a1f..986d612aa77f 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -21,6 +21,16 @@ config ATLAS_PH_SENSOR
 To compile this driver as module, choose M here: the
 module will be called atlas-ph-sensor.

+config DSM501
+   tristate "Samyoung DSM501 particle sensor"
+   depends on GPIOLIB
+   help
+Say yes here to build support for the Samyoung DSM501
+particle sensor.
+
+To compile this driver as a module, choose M here: the module
+will be called dsm501.
+
 config IAQCORE
tristate "AMS iAQ-Core VOC sensors"
depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index b02202b41289..76f50ff8ba7d 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -4,5 +4,6 @@

 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ATLAS_PH_SENSOR)  += atlas-ph-sensor.o
+obj-$(CONFIG_DSM501)   += dsm501.o
 obj-$(CONFIG_IAQCORE)  += ams-iaq-core.o
 obj-$(CONFIG_VZ89X)+= vz89x.o
diff --git a/drivers/iio/chemical/dsm501.c b/drivers/iio/chemical/dsm501.c
new file mode 100644
index ..7c53b8305508
--- /dev/null
+++ b/drivers/iio/chemical/dsm501.c
@@ -0,0 +1,225 @@
+/*
+ * Samyoung DSM501 particle sensor driver
+ *
+ * Copyright (C) 2017 Tomasz Duszynski 
+ *
+ * Datasheets:
+ *  http://www.samyoungsnc.com/products/3-1%20Specification%20DSM501.pdf
+ *  http://wiki.timelab.org/images/f/f9/PPD42NS.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DSM501_DRV_NAME "dsm501"
+#define DSM501_IRQ_NAME "dsm501_irq"
+
+#define DSM501_DEFAULT_MEASUREMENT_TIME 30 /* seconds */
+
+struct dsm501_data {
+   ktime_t ts;
+   ktime_t low_time;
+   ktime_t meas_time;
+
+   int irq;
+   struct gpio_desc *gpio;
+
+   struct mutex lock;
+
+   int (*number_concentration)(struct dsm501_data *data);
+};
+
+/*
+ * Series of data points in Fig. 8-3 (Low Ratio vs Particle)
+ * can be approximated by following polynomials:
+ *
+ * p(r) = 0 (undefined) for r < 4
+ * p(r) = 2353564.2r - 4373814.7 for 4 <= r < 20
+ * p(r) = 4788112.4r - 53581390 for r >= 20
+ */
+static int dsm501_number_concentration(struct dsm501_data *data)
+{
+   s64 retval = 0, r = div64_s64(ktime_to_ns(data->low_time) * 100,
+ ktime_to_ns(data->meas_time));
+
+   if (r >= 4 && r < 20)
+   retval = 23535642 * r - 43738147;
+   else if (r >= 20)
+   retval = 47881124 * r - 535813900;
+
+   return div_s64(retval, 10);
+}
+
+/*
+ * Series of data points in Fig. 2 (Lo Pulse Occupancy Time vs Concentration)
+ * can be approximated by following polynomial:
+ *
+ * p(r) = 3844.2r^3 - 16201.3r^2 + 1848746.1r + 52497.2
+ */
+static int ppd42ns_number_concentration(struct dsm501_data *data)
+{
+   s64 retval, r3, r2, r = div64_s64(ktime_to_ns(data->low_time) * 100,
+ ktime_to_ns(data->meas_time));
+
+   r2 = r * r;
+   r3 = r2 * r;
+
+   retval = 38442 * r3;
+   retval -= 162013 * r2;
+   retval += 18487461 * r;
+   retval += 524972;
+
+   return div_s64(retval, 10);

[PATCH v2 4/4] iio: chemical: add dsm501 particle sensor driver bindings

[Tomasz Duszynski]

This patch adds bindings for dsm501 particle sensor driver.

Signed-off-by: Tomasz Duszynski 
---
 Changes in v2:
  o add 'data' prefix to gpios property

 .../devicetree/bindings/iio/chemical/dsm501.txt  | 16 
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/dsm501.txt

diff --git a/Documentation/devicetree/bindings/iio/chemical/dsm501.txt 
b/Documentation/devicetree/bindings/iio/chemical/dsm501.txt
new file mode 100644
index ..126ce528f5eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/dsm501.txt
@@ -0,0 +1,16 @@
+* DSM501 particle sensor
+
+Required properties:
+
+- compatible: should be one of:
+ "samyoung,dsm501"
+ "shinyei,ppd42ns"
+- data-gpios: should specify the GPIO connected to the sensor's data line, see
+ "gpios property" in 
Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+
+dsm501 {
+   compatible = "samyoung,dsm501";
+   data-gpios = < 28 0>;
+};
--
2.11.1

[PATCH v3 2/4] iio: chemical: add driver for dsm501/ppd42ns particle sensors

[Tomasz Duszynski]

This patch adds support for dsm501 and ppd42ns particle sensors.

Both sensors work on the same principle. Heater (resistor) heats up air
in sensor chamber which induces upward flow. Particles convect up through
a light beam provided by internal infra-red LED. Light scattered by
particles is picked by photodiode and internal electronics outputs PWM
signal.

Measuring low time occupancy of the signal during 30s measurement period
particles number in cubic meter can be computed.

Signed-off-by: Tomasz Duszynski 
---
 Changes in v3:
  o use devm_gpiod_get() instead of indexed version. This is due to bindings
change.

 Changes in v2:
  o whitespace fix
  o drop excessive comments about conversion to pcs/0.01cf
  o fix 'concentartion' typo in dsm501_number_concentartion handler name

 drivers/iio/chemical/Kconfig  |  10 ++
 drivers/iio/chemical/Makefile |   1 +
 drivers/iio/chemical/dsm501.c | 225 ++
 3 files changed, 236 insertions(+)
 create mode 100644 drivers/iio/chemical/dsm501.c

diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index cea7f9857a1f..986d612aa77f 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -21,6 +21,16 @@ config ATLAS_PH_SENSOR
 To compile this driver as module, choose M here: the
 module will be called atlas-ph-sensor.

+config DSM501
+   tristate "Samyoung DSM501 particle sensor"
+   depends on GPIOLIB
+   help
+Say yes here to build support for the Samyoung DSM501
+particle sensor.
+
+To compile this driver as a module, choose M here: the module
+will be called dsm501.
+
 config IAQCORE
tristate "AMS iAQ-Core VOC sensors"
depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index b02202b41289..76f50ff8ba7d 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -4,5 +4,6 @@

 # When adding new entries keep the list in alphabetical order
 obj-$(CONFIG_ATLAS_PH_SENSOR)  += atlas-ph-sensor.o
+obj-$(CONFIG_DSM501)   += dsm501.o
 obj-$(CONFIG_IAQCORE)  += ams-iaq-core.o
 obj-$(CONFIG_VZ89X)+= vz89x.o
diff --git a/drivers/iio/chemical/dsm501.c b/drivers/iio/chemical/dsm501.c
new file mode 100644
index ..7c53b8305508
--- /dev/null
+++ b/drivers/iio/chemical/dsm501.c
@@ -0,0 +1,225 @@
+/*
+ * Samyoung DSM501 particle sensor driver
+ *
+ * Copyright (C) 2017 Tomasz Duszynski 
+ *
+ * Datasheets:
+ *  http://www.samyoungsnc.com/products/3-1%20Specification%20DSM501.pdf
+ *  http://wiki.timelab.org/images/f/f9/PPD42NS.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DSM501_DRV_NAME "dsm501"
+#define DSM501_IRQ_NAME "dsm501_irq"
+
+#define DSM501_DEFAULT_MEASUREMENT_TIME 30 /* seconds */
+
+struct dsm501_data {
+   ktime_t ts;
+   ktime_t low_time;
+   ktime_t meas_time;
+
+   int irq;
+   struct gpio_desc *gpio;
+
+   struct mutex lock;
+
+   int (*number_concentration)(struct dsm501_data *data);
+};
+
+/*
+ * Series of data points in Fig. 8-3 (Low Ratio vs Particle)
+ * can be approximated by following polynomials:
+ *
+ * p(r) = 0 (undefined) for r < 4
+ * p(r) = 2353564.2r - 4373814.7 for 4 <= r < 20
+ * p(r) = 4788112.4r - 53581390 for r >= 20
+ */
+static int dsm501_number_concentration(struct dsm501_data *data)
+{
+   s64 retval = 0, r = div64_s64(ktime_to_ns(data->low_time) * 100,
+ ktime_to_ns(data->meas_time));
+
+   if (r >= 4 && r < 20)
+   retval = 23535642 * r - 43738147;
+   else if (r >= 20)
+   retval = 47881124 * r - 535813900;
+
+   return div_s64(retval, 10);
+}
+
+/*
+ * Series of data points in Fig. 2 (Lo Pulse Occupancy Time vs Concentration)
+ * can be approximated by following polynomial:
+ *
+ * p(r) = 3844.2r^3 - 16201.3r^2 + 1848746.1r + 52497.2
+ */
+static int ppd42ns_number_concentration(struct dsm501_data *data)
+{
+   s64 retval, r3, r2, r = div64_s64(ktime_to_ns(data->low_time) * 100,
+ ktime_to_ns(data->meas_time));
+
+   r2 = r * r;
+   r3 = r2 * r;
+
+   retval = 38442 * r3;
+   retval -= 162013 * r2;
+   retval += 18487461 * r;
+   retval += 524972;
+
+   return div_s64(retval, 10);
+}
+
+static irqreturn_t dsm501_irq(int 

[PATCH][V2] Smack: fix a dereference before null check on sock->sk

[Colin King]

From: Colin Ian King 

The initialisation of pointer ssp is from a dereference on sock->sk
before sock-sk is null checked, hence there is a potential for a
null pointer deference.  Fix this by moving the assignment of ssp
to just before it is used in the call to smk_ipv6_check.

Also minor clean up of code to reduce #ifdef noise.
Detected with CoverityScan, CID#1324196 ("Dereference before null check")

Signed-off-by: Colin Ian King 
---
 security/smack/smack_lsm.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index fc8fb31..0c5656d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2897,10 +2897,6 @@ static int smack_socket_connect(struct socket *sock, 
struct sockaddr *sap,
 #if IS_ENABLED(CONFIG_IPV6)
struct sockaddr_in6 *sip = (struct sockaddr_in6 *)sap;
 #endif
-#ifdef SMACK_IPV6_SECMARK_LABELING
-   struct smack_known *rsp;
-   struct socket_smack *ssp = sock->sk->sk_security;
-#endif
 
if (sock->sk == NULL)
return 0;
@@ -2915,10 +2911,17 @@ static int smack_socket_connect(struct socket *sock, 
struct sockaddr *sap,
if (addrlen < sizeof(struct sockaddr_in6))
return -EINVAL;
 #ifdef SMACK_IPV6_SECMARK_LABELING
-   rsp = smack_ipv6host_label(sip);
-   if (rsp != NULL)
-   rc = smk_ipv6_check(ssp->smk_out, rsp, sip,
-   SMK_CONNECTING);
+   {
+   struct smack_known *rsp = smack_ipv6host_label(sip);
+
+   if (rsp != NULL) {
+   struct socket_smack *ssp =
+   sock->sk->sk_security;
+
+   rc = smk_ipv6_check(ssp->smk_out, rsp, sip,
+   SMK_CONNECTING);
+   }
+   }
 #endif
 #ifdef SMACK_IPV6_PORT_LABELING
rc = smk_ipv6_port_check(sock->sk, sip, SMK_CONNECTING);
-- 
2.10.2

[PATCH][V2] Smack: fix a dereference before null check on sock->sk

[Colin King]

From: Colin Ian King 

The initialisation of pointer ssp is from a dereference on sock->sk
before sock-sk is null checked, hence there is a potential for a
null pointer deference.  Fix this by moving the assignment of ssp
to just before it is used in the call to smk_ipv6_check.

Also minor clean up of code to reduce #ifdef noise.
Detected with CoverityScan, CID#1324196 ("Dereference before null check")

Signed-off-by: Colin Ian King 
---
 security/smack/smack_lsm.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index fc8fb31..0c5656d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2897,10 +2897,6 @@ static int smack_socket_connect(struct socket *sock, 
struct sockaddr *sap,
 #if IS_ENABLED(CONFIG_IPV6)
struct sockaddr_in6 *sip = (struct sockaddr_in6 *)sap;
 #endif
-#ifdef SMACK_IPV6_SECMARK_LABELING
-   struct smack_known *rsp;
-   struct socket_smack *ssp = sock->sk->sk_security;
-#endif
 
if (sock->sk == NULL)
return 0;
@@ -2915,10 +2911,17 @@ static int smack_socket_connect(struct socket *sock, 
struct sockaddr *sap,
if (addrlen < sizeof(struct sockaddr_in6))
return -EINVAL;
 #ifdef SMACK_IPV6_SECMARK_LABELING
-   rsp = smack_ipv6host_label(sip);
-   if (rsp != NULL)
-   rc = smk_ipv6_check(ssp->smk_out, rsp, sip,
-   SMK_CONNECTING);
+   {
+   struct smack_known *rsp = smack_ipv6host_label(sip);
+
+   if (rsp != NULL) {
+   struct socket_smack *ssp =
+   sock->sk->sk_security;
+
+   rc = smk_ipv6_check(ssp->smk_out, rsp, sip,
+   SMK_CONNECTING);
+   }
+   }
 #endif
 #ifdef SMACK_IPV6_PORT_LABELING
rc = smk_ipv6_port_check(sock->sk, sip, SMK_CONNECTING);
-- 
2.10.2

Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric

[Andi Kleen]

On Thu, Feb 09, 2017 at 12:39:37PM +0100, Jiri Olsa wrote:
> and this makes me think, that this is not the right approach
> 
> adding extra copy of an event when you want to add new expression?

I don't want to add new expressions.

I don't even need arbitrary expressions, just DividedBy
to get percentages, you just forced me to do the expressions.


> why can't we have another list/file of those expressions

The last time I proposed separate files Ingo vetoed it.
He wanted everything built in.

> from which point we could point and configure events we need

If you want full flexibility you can use your perf stat report
approach, or what most people do is to just run a script/spreadsheet
over the the -x; output. This all continues to work.

This is just a minimum approach to provide some convenience
integrated with the event list to provide something similar
as the built in expressions in stat-shadow. 

It's not trying to build the great perf scripting language.

-Andi

Re: [PATCH 10/10] perf, tools, stat: Output JSON MetricExpr metric

[Andi Kleen]

On Thu, Feb 09, 2017 at 12:39:37PM +0100, Jiri Olsa wrote:
> and this makes me think, that this is not the right approach
> 
> adding extra copy of an event when you want to add new expression?

I don't want to add new expressions.

I don't even need arbitrary expressions, just DividedBy
to get percentages, you just forced me to do the expressions.


> why can't we have another list/file of those expressions

The last time I proposed separate files Ingo vetoed it.
He wanted everything built in.

> from which point we could point and configure events we need

If you want full flexibility you can use your perf stat report
approach, or what most people do is to just run a script/spreadsheet
over the the -x; output. This all continues to work.

This is just a minimum approach to provide some convenience
integrated with the event list to provide something similar
as the built in expressions in stat-shadow. 

It's not trying to build the great perf scripting language.

-Andi

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

On Thursday, February 9, 2017 4:57:51 PM CET Andrew Lunn wrote:
> On Thu, Feb 09, 2017 at 04:08:11PM +0100, Arnd Bergmann wrote:
> > The newly introduced mdiobus_register_board_info() function is only 
> > available
> > as part of PHYLIB, so we get a link error when we call that from a board 
> > while
> > phylib is disabled:
> > 
> > arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
> > common.c:(.init.text+0x6a4): undefined reference to 
> > `mdiobus_register_board_info'
> > 
> > This adds a workaround that is made up of three parts:
> > 
> > - in plat-orion, the function for declaring the switch is hidden without
> >   PHYLIB.
> > - in mach-orion5x, the caller conditionally stubs out the call to
> >   the removed function, so we can still build other orion5x boards
> >   without PHYLIB
> > - For the boards that actually declare the switch, we select PHYLIB
> >   explicitly from Kconfig if NETDEVICES is set. Without NETDEVICES,
> >   we cannot enable PHYLIB, but we also wouldn't need it.
> 
> Hi Arnd
> 
> Although all correct, would it not be simpler to just select PHYLIB
> and NETDEVICES? These devices are all NAS boxes and WiFi access
> points. What sense does it make to build a kernel without working
> networking for these classes of devices?

Adding a 'select' statement to something as broad as NETDEVICES sounds
really bad, it has a significant risk of introducing dependency loops
and may be confusing if you want to build a multiplatform config without
networking support (note that NETDEVICES in turn depends on NET, which
can also be disabled).

One possibility would be to have a special Kconfig symbol that controls
mdiobus_register_board_info() being present and have that symbol
force PHYLIB to never be "=m". Then we can either have no networking
support and no phylib, turning mdiobus_register_board_info() into a
stub, or we have the function built-in and reachable from the board
code.

Arnd

Re: [PATCH] [net-next] ARM: orion: fix PHYLIB dependency

[Arnd Bergmann]

On Thursday, February 9, 2017 4:57:51 PM CET Andrew Lunn wrote:
> On Thu, Feb 09, 2017 at 04:08:11PM +0100, Arnd Bergmann wrote:
> > The newly introduced mdiobus_register_board_info() function is only 
> > available
> > as part of PHYLIB, so we get a link error when we call that from a board 
> > while
> > phylib is disabled:
> > 
> > arch/arm/plat-orion/common.o: In function `orion_ge00_switch_init':
> > common.c:(.init.text+0x6a4): undefined reference to 
> > `mdiobus_register_board_info'
> > 
> > This adds a workaround that is made up of three parts:
> > 
> > - in plat-orion, the function for declaring the switch is hidden without
> >   PHYLIB.
> > - in mach-orion5x, the caller conditionally stubs out the call to
> >   the removed function, so we can still build other orion5x boards
> >   without PHYLIB
> > - For the boards that actually declare the switch, we select PHYLIB
> >   explicitly from Kconfig if NETDEVICES is set. Without NETDEVICES,
> >   we cannot enable PHYLIB, but we also wouldn't need it.
> 
> Hi Arnd
> 
> Although all correct, would it not be simpler to just select PHYLIB
> and NETDEVICES? These devices are all NAS boxes and WiFi access
> points. What sense does it make to build a kernel without working
> networking for these classes of devices?

Adding a 'select' statement to something as broad as NETDEVICES sounds
really bad, it has a significant risk of introducing dependency loops
and may be confusing if you want to build a multiplatform config without
networking support (note that NETDEVICES in turn depends on NET, which
can also be disabled).

One possibility would be to have a special Kconfig symbol that controls
mdiobus_register_board_info() being present and have that symbol
force PHYLIB to never be "=m". Then we can either have no networking
support and no phylib, turning mdiobus_register_board_info() into a
stub, or we have the function built-in and reachable from the board
code.

Arnd

Re: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver

[Paul Cercueil]

Hi,


What some drivers do when they just get/set a bit in a register
to get/set or set the direction of a GPIO, is to select GPIO_GENERIC
and just bgpio_init() with the right iomem pointers, then the core
will register handlers for get, set, set_direcition callback and
get_direction and your driver can just focus on the remainders.


GPIO_GENERIC and bgpio_init() would work for my .set() / .get() 
callbacks,

not for my .direction_input() / .direction_output() callbacks which need
to set more than one register.


If you're not just replacing these with GPIO_GENERIC, please also
include a .get_direction() callback.


My .direction_input() and .direction_output() callbacks just call into
the pinctrl driver, using pinctrl_gpio_direction_[in,out]put().
I didn't find a way to get the direction info from the pinctrl driver,
is that something that the core should provide?

Thanks,
-Paul

Re: [PATCH v3 04/14] GPIO: Add gpio-ingenic driver

[Paul Cercueil]

Hi,


What some drivers do when they just get/set a bit in a register
to get/set or set the direction of a GPIO, is to select GPIO_GENERIC
and just bgpio_init() with the right iomem pointers, then the core
will register handlers for get, set, set_direcition callback and
get_direction and your driver can just focus on the remainders.


GPIO_GENERIC and bgpio_init() would work for my .set() / .get() 
callbacks,

not for my .direction_input() / .direction_output() callbacks which need
to set more than one register.


If you're not just replacing these with GPIO_GENERIC, please also
include a .get_direction() callback.


My .direction_input() and .direction_output() callbacks just call into
the pinctrl driver, using pinctrl_gpio_direction_[in,out]put().
I didn't find a way to get the direction info from the pinctrl driver,
is that something that the core should provide?

Thanks,
-Paul

Re: [PATCH v2 1/2] drm/cma-helper: Add multi buffer support for cma fbdev

[Daniel Vetter]

On Thu, Feb 02, 2017 at 11:31:56AM +0100, Maxime Ripard wrote:
> From: Xinliang Liu 
> 
> This patch add a config to support to create multi buffer for cma fbdev.
> Such as double buffer and triple buffer.
> 
> Cma fbdev is convient to add a legency fbdev. And still many Android
> devices use fbdev now and at least double buffer is needed for these
> Android devices, so that a buffer flip can be operated. It will need
> some time for Android device vendors to abondon legency fbdev. So multi
> buffer for fbdev is needed.
> 
> Signed-off-by: Xinliang Liu 
> [s.chr...@phytec.de: Picking patch from
>  https://lkml.org/lkml/2015/9/14/188]
> Signed-off-by: Stefan Christ 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/Kconfig | 8 
>  drivers/gpu/drm/drm_fb_cma_helper.c | 8 +++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index ebfe8404c25f..2ca9bb26a4e4 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -121,6 +121,14 @@ config DRM_KMS_CMA_HELPER
>   help
> Choose this if you need the KMS CMA helper functions
>  
> +config DRM_CMA_FBDEV_BUFFER_NUM
> + int "Cma Fbdev Buffer Number"
> + depends on DRM_KMS_CMA_HELPER
> + default 1
> + help
> +   Defines the buffer number of cma fbdev.  Default is one buffer.
> +   For double buffer please set to 2 and 3 for triple buffer.
> +
>  source "drivers/gpu/drm/i2c/Kconfig"
>  
>  source "drivers/gpu/drm/arm/Kconfig"
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 81b3558302b5..e3f8b9e720a0 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -411,6 +411,12 @@ static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
>   kfree(fbi->fbops);
>  }
>  
> +static int fbdev_num_buffers = CONFIG_DRM_CMA_FBDEV_BUFFER_NUM;
> +module_param(fbdev_num_buffers, int, 0444);
> +MODULE_PARM_DESC(fbdev_num_buffers,
> +  "Number of frame buffers to allocate [default="
> +  __MODULE_STRING(CONFIG_DRM_CMA_FBDEV_BUFFER_NUM) "]");

Pure bikshed: Should this be an overallocation %? 200 for
double-buffering, 100 as the default?

Slightly less bikesheddy: Can't we do this in the core helpers somehow?
I'd be nice if this is not cma specific. If it's not possible, I'd at
least move the symbol to drm_fb_helper.c, and add some kernel-doc around
it. That allows any other non-cma driver to at least implement support for
this. That also has the benefit of featuring it more prominently, in our
docs.

Besides these bikesheds/question, looks all reasonable to me. If you can
get some more acks would be great, but will merge anyway.
-Daniel

> +
>  /*
>   * For use in a (struct drm_fb_helper_funcs *)->fb_probe callback function 
> that
>   * needs custom struct drm_framebuffer_funcs, like dirty() for deferred_io 
> use.
> @@ -437,7 +443,7 @@ int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper 
> *helper,
>   bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>  
>   mode_cmd.width = sizes->surface_width;
> - mode_cmd.height = sizes->surface_height;
> + mode_cmd.height = sizes->surface_height * fbdev_num_buffers;
>   mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel;
>   mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
>   sizes->surface_depth);
> -- 
> git-series 0.8.11
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v2 1/2] drm/cma-helper: Add multi buffer support for cma fbdev

[Daniel Vetter]

On Thu, Feb 02, 2017 at 11:31:56AM +0100, Maxime Ripard wrote:
> From: Xinliang Liu 
> 
> This patch add a config to support to create multi buffer for cma fbdev.
> Such as double buffer and triple buffer.
> 
> Cma fbdev is convient to add a legency fbdev. And still many Android
> devices use fbdev now and at least double buffer is needed for these
> Android devices, so that a buffer flip can be operated. It will need
> some time for Android device vendors to abondon legency fbdev. So multi
> buffer for fbdev is needed.
> 
> Signed-off-by: Xinliang Liu 
> [s.chr...@phytec.de: Picking patch from
>  https://lkml.org/lkml/2015/9/14/188]
> Signed-off-by: Stefan Christ 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/Kconfig | 8 
>  drivers/gpu/drm/drm_fb_cma_helper.c | 8 +++-
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index ebfe8404c25f..2ca9bb26a4e4 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -121,6 +121,14 @@ config DRM_KMS_CMA_HELPER
>   help
> Choose this if you need the KMS CMA helper functions
>  
> +config DRM_CMA_FBDEV_BUFFER_NUM
> + int "Cma Fbdev Buffer Number"
> + depends on DRM_KMS_CMA_HELPER
> + default 1
> + help
> +   Defines the buffer number of cma fbdev.  Default is one buffer.
> +   For double buffer please set to 2 and 3 for triple buffer.
> +
>  source "drivers/gpu/drm/i2c/Kconfig"
>  
>  source "drivers/gpu/drm/arm/Kconfig"
> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
> b/drivers/gpu/drm/drm_fb_cma_helper.c
> index 81b3558302b5..e3f8b9e720a0 100644
> --- a/drivers/gpu/drm/drm_fb_cma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c
> @@ -411,6 +411,12 @@ static void drm_fbdev_cma_defio_fini(struct fb_info *fbi)
>   kfree(fbi->fbops);
>  }
>  
> +static int fbdev_num_buffers = CONFIG_DRM_CMA_FBDEV_BUFFER_NUM;
> +module_param(fbdev_num_buffers, int, 0444);
> +MODULE_PARM_DESC(fbdev_num_buffers,
> +  "Number of frame buffers to allocate [default="
> +  __MODULE_STRING(CONFIG_DRM_CMA_FBDEV_BUFFER_NUM) "]");

Pure bikshed: Should this be an overallocation %? 200 for
double-buffering, 100 as the default?

Slightly less bikesheddy: Can't we do this in the core helpers somehow?
I'd be nice if this is not cma specific. If it's not possible, I'd at
least move the symbol to drm_fb_helper.c, and add some kernel-doc around
it. That allows any other non-cma driver to at least implement support for
this. That also has the benefit of featuring it more prominently, in our
docs.

Besides these bikesheds/question, looks all reasonable to me. If you can
get some more acks would be great, but will merge anyway.
-Daniel

> +
>  /*
>   * For use in a (struct drm_fb_helper_funcs *)->fb_probe callback function 
> that
>   * needs custom struct drm_framebuffer_funcs, like dirty() for deferred_io 
> use.
> @@ -437,7 +443,7 @@ int drm_fbdev_cma_create_with_funcs(struct drm_fb_helper 
> *helper,
>   bytes_per_pixel = DIV_ROUND_UP(sizes->surface_bpp, 8);
>  
>   mode_cmd.width = sizes->surface_width;
> - mode_cmd.height = sizes->surface_height;
> + mode_cmd.height = sizes->surface_height * fbdev_num_buffers;
>   mode_cmd.pitches[0] = sizes->surface_width * bytes_per_pixel;
>   mode_cmd.pixel_format = drm_mode_legacy_fb_format(sizes->surface_bpp,
>   sizes->surface_depth);
> -- 
> git-series 0.8.11
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCHv3] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Steven Rostedt]

On Thu, 9 Feb 2017 16:29:56 +
Russell King - ARM Linux  wrote:

> On Tue, Feb 07, 2017 at 10:57:55PM +, Abel Vesa wrote:
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +
> > +.macro __ftrace_regs_caller
> > +
> > +   add ip, sp, #4  @ move in IP the value of SP as it was
> > +   @ before the push {lr} of the mcount mechanism
> > +   stmdb   sp!, {ip,lr,pc}
> > +   stmdb   sp!, {r0-r11,lr}
> > +
> > +   @ stack content at this point:
> > +   @ 0  4  4448   52   56   60   64
> > +   @ R0 | R1 | ... | R11 | LR | SP + 4 | LR | PC | previous LR |  
> 
> How important is this to be close to "struct pt_regs" ?  Do we care about
> r12 being "wrong" ?  The other issue is that pt_regs is actually 72
> bytes in size, not 68 bytes.  So, does that mean we end up inappropriately
> leaking some of the kernel stack to userspace through ftrace?

The regs passed to the ftrace code isn't passed to userspace. It's used
by kprobes as a "fake breakpoint" (like fake news?), and by kernel live
patching to modify what function actually gets called after ftrace
returns.


> 
> It's possible to save all the registers like this if we need to provide
> a complete picture of the register set at function entry:
> 
>   str ip, [sp, #-16]!
>   add ip, sp, #20
>   stmia   sp, {ip, lr, pc}
>   stmdb   sp!, {r0 - r11}
> 
> However, is that even correct - don't we want pt_regs' LR and PC to be
> related to the function call itself?  The "previous LR" as you describe
> it is where the called function (the one that is being traced) will
> return to.  The current LR at this point is the address within the
> traced function.  So actually I think this is more strictly correct, if
> I'm understanding the intention here correctly:
> 
>   str ip, [sp, #S_IP - PT_REGS_SIZE]! @ save current IP
>   ldr ip, [sp, #PT_REGS_SIZE - S_IP]  @ get LR at traced function 
> entry
>   str lr, [sp, #S_PC - S_IP]  @ save current LR as PC
>   str ip, [sp, #S_LR - S_IP]  @ save traced function return
>   add ip, sp, #PT_REGS_SIZE - S_IP + 4
>   str ip, [sp, #S_SP - SP_IP] @ save stack pointer at 
> function entry
>   stmdb   sp!, {r0 - r11}
>   @ clear CPSR and old_r0 words
>   mov r3, #0
>   str r3, [sp, #S_PSR]
>   str r3, [sp, #S_OLD_R0]
> 
> However, that has the side effect of misaligning the stack (the stack
> needs to be aligned to 8 bytes).  So, if we decide we don't care about
> the saved LR value (except as a mechanism to preserve it across the
> call into the ftrace code):
> 
>   str ip, [sp, #S_IP - PT_REGS_SIZE + 4]!
>   str lr, [sp, #S_PC - S_IP]
>   ldr lr, [sp, #PT_REGS_SIZE - 4 - S_IP]
>   add ip, sp, #PT_REGS_SIZE - S_IP
>   stmib   sp, {ip, lr}
>   stmdb   sp!, {r0 - r11}
>   @ clear CPSR and old_r0 words
>   mov r3, #0
>   str r3, [sp, #S_PSR]
>   str r3, [sp, #S_OLD_R0]
> 
> and the return would be:
> 
>   ldmia   sp, {r0 - pc}
> 
> That all said - maybe someone from the ftrace community can comment on
> how much of pt_regs is actually necessary here?
> 

Matters about the users. The REGS was originally created for kprobes,
to simulate a kprobe breakpoint. As calling kprobes directly is much
faster than going through the breakpoint mechanism. As adding a kprobe
to the start of a function is a very common practice, it made sense to
have ftrace give it a boost.

Then came along live kernel patching, which I believe this series is
trying to support. What is needed by pt_regs is a way to "hijack" the
function being called to instead call the patched function. That is,
ftrace is not being used for tracing, but in reality, being used to
modify the running kernel. It is being used to change what function
gets called. ftrace is just a hook for that mechanism.

-- Steve

Re: [PATCHv3] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Steven Rostedt]

On Thu, 9 Feb 2017 16:29:56 +
Russell King - ARM Linux  wrote:

> On Tue, Feb 07, 2017 at 10:57:55PM +, Abel Vesa wrote:
> > +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> > +
> > +.macro __ftrace_regs_caller
> > +
> > +   add ip, sp, #4  @ move in IP the value of SP as it was
> > +   @ before the push {lr} of the mcount mechanism
> > +   stmdb   sp!, {ip,lr,pc}
> > +   stmdb   sp!, {r0-r11,lr}
> > +
> > +   @ stack content at this point:
> > +   @ 0  4  4448   52   56   60   64
> > +   @ R0 | R1 | ... | R11 | LR | SP + 4 | LR | PC | previous LR |  
> 
> How important is this to be close to "struct pt_regs" ?  Do we care about
> r12 being "wrong" ?  The other issue is that pt_regs is actually 72
> bytes in size, not 68 bytes.  So, does that mean we end up inappropriately
> leaking some of the kernel stack to userspace through ftrace?

The regs passed to the ftrace code isn't passed to userspace. It's used
by kprobes as a "fake breakpoint" (like fake news?), and by kernel live
patching to modify what function actually gets called after ftrace
returns.


> 
> It's possible to save all the registers like this if we need to provide
> a complete picture of the register set at function entry:
> 
>   str ip, [sp, #-16]!
>   add ip, sp, #20
>   stmia   sp, {ip, lr, pc}
>   stmdb   sp!, {r0 - r11}
> 
> However, is that even correct - don't we want pt_regs' LR and PC to be
> related to the function call itself?  The "previous LR" as you describe
> it is where the called function (the one that is being traced) will
> return to.  The current LR at this point is the address within the
> traced function.  So actually I think this is more strictly correct, if
> I'm understanding the intention here correctly:
> 
>   str ip, [sp, #S_IP - PT_REGS_SIZE]! @ save current IP
>   ldr ip, [sp, #PT_REGS_SIZE - S_IP]  @ get LR at traced function 
> entry
>   str lr, [sp, #S_PC - S_IP]  @ save current LR as PC
>   str ip, [sp, #S_LR - S_IP]  @ save traced function return
>   add ip, sp, #PT_REGS_SIZE - S_IP + 4
>   str ip, [sp, #S_SP - SP_IP] @ save stack pointer at 
> function entry
>   stmdb   sp!, {r0 - r11}
>   @ clear CPSR and old_r0 words
>   mov r3, #0
>   str r3, [sp, #S_PSR]
>   str r3, [sp, #S_OLD_R0]
> 
> However, that has the side effect of misaligning the stack (the stack
> needs to be aligned to 8 bytes).  So, if we decide we don't care about
> the saved LR value (except as a mechanism to preserve it across the
> call into the ftrace code):
> 
>   str ip, [sp, #S_IP - PT_REGS_SIZE + 4]!
>   str lr, [sp, #S_PC - S_IP]
>   ldr lr, [sp, #PT_REGS_SIZE - 4 - S_IP]
>   add ip, sp, #PT_REGS_SIZE - S_IP
>   stmib   sp, {ip, lr}
>   stmdb   sp!, {r0 - r11}
>   @ clear CPSR and old_r0 words
>   mov r3, #0
>   str r3, [sp, #S_PSR]
>   str r3, [sp, #S_OLD_R0]
> 
> and the return would be:
> 
>   ldmia   sp, {r0 - pc}
> 
> That all said - maybe someone from the ftrace community can comment on
> how much of pt_regs is actually necessary here?
> 

Matters about the users. The REGS was originally created for kprobes,
to simulate a kprobe breakpoint. As calling kprobes directly is much
faster than going through the breakpoint mechanism. As adding a kprobe
to the start of a function is a very common practice, it made sense to
have ftrace give it a boost.

Then came along live kernel patching, which I believe this series is
trying to support. What is needed by pt_regs is a way to "hijack" the
function being called to instead call the patched function. That is,
ftrace is not being used for tracing, but in reality, being used to
modify the running kernel. It is being used to change what function
gets called. ftrace is just a hook for that mechanism.

-- Steve

[PATCH v4 2/2] staging: omap4iss: fix coding style issue

[Avraham Shukron]

Broke argument list so that it won't exceed 80 characters

Signed-off-by: Avraham Shukron 
---
 drivers/staging/media/omap4iss/iss_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index e21811a..0bac582 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -301,7 +301,8 @@ iss_video_check_format(struct iss_video *video, struct 
iss_video_fh *vfh)

 static int iss_video_queue_setup(struct vb2_queue *vq,
 unsigned int *count, unsigned int *num_planes,
-unsigned int sizes[], struct device 
*alloc_devs[])
+unsigned int sizes[],
+struct device *alloc_devs[])
 {
struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
struct iss_video *video = vfh->video;
-- 
2.7.4

[PATCH v4 2/2] staging: omap4iss: fix coding style issue

[Avraham Shukron]

Broke argument list so that it won't exceed 80 characters

Signed-off-by: Avraham Shukron 
---
 drivers/staging/media/omap4iss/iss_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index e21811a..0bac582 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -301,7 +301,8 @@ iss_video_check_format(struct iss_video *video, struct 
iss_video_fh *vfh)

 static int iss_video_queue_setup(struct vb2_queue *vq,
 unsigned int *count, unsigned int *num_planes,
-unsigned int sizes[], struct device 
*alloc_devs[])
+unsigned int sizes[],
+struct device *alloc_devs[])
 {
struct iss_video_fh *vfh = vb2_get_drv_priv(vq);
struct iss_video *video = vfh->video;
-- 
2.7.4

Re: [PATCH] drm/vc4: Drop debug print at boot with DPI enabled.

[Daniel Vetter]

On Wed, Feb 08, 2017 at 12:47:01PM -0800, Eric Anholt wrote:
> Unlike the other encoders in the driver, I've also dropped the debug
> dump function.  There's only really one register to this device, and
> we have the debugfs reg entry still.
> 
> Signed-off-by: Eric Anholt 

Yeah, dmesg spew by default isn't cool. Btw if you ever want to have fancy
debug printers, there's drm_printer, which allows you to spam both dmesg
and debugfs with the same code. Rob did that, I recently converted drm_mm,
it's pretty cool.

Acked-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/vc4/vc4_dpi.c | 13 -
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index 1e1f6b8184d0..3f360cf6cf5a 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -144,17 +144,6 @@ static const struct {
>   DPI_REG(DPI_ID),
>  };
>  
> -static void vc4_dpi_dump_regs(struct vc4_dpi *dpi)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(dpi_regs); i++) {
> - DRM_INFO("0x%04x (%s): 0x%08x\n",
> -  dpi_regs[i].reg, dpi_regs[i].name,
> -  DPI_READ(dpi_regs[i].reg));
> - }
> -}
> -
>  #ifdef CONFIG_DEBUG_FS
>  int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused)
>  {
> @@ -416,8 +405,6 @@ static int vc4_dpi_bind(struct device *dev, struct device 
> *master, void *data)
>   if (IS_ERR(dpi->regs))
>   return PTR_ERR(dpi->regs);
>  
> - vc4_dpi_dump_regs(dpi);
> -
>   if (DPI_READ(DPI_ID) != DPI_ID_VALUE) {
>   dev_err(dev, "Port returned 0x%08x for ID instead of 0x%08x\n",
>   DPI_READ(DPI_ID), DPI_ID_VALUE);
> -- 
> 2.11.0
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/vc4: Drop debug print at boot with DPI enabled.

[Daniel Vetter]

On Wed, Feb 08, 2017 at 12:47:01PM -0800, Eric Anholt wrote:
> Unlike the other encoders in the driver, I've also dropped the debug
> dump function.  There's only really one register to this device, and
> we have the debugfs reg entry still.
> 
> Signed-off-by: Eric Anholt 

Yeah, dmesg spew by default isn't cool. Btw if you ever want to have fancy
debug printers, there's drm_printer, which allows you to spam both dmesg
and debugfs with the same code. Rob did that, I recently converted drm_mm,
it's pretty cool.

Acked-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/vc4/vc4_dpi.c | 13 -
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
> index 1e1f6b8184d0..3f360cf6cf5a 100644
> --- a/drivers/gpu/drm/vc4/vc4_dpi.c
> +++ b/drivers/gpu/drm/vc4/vc4_dpi.c
> @@ -144,17 +144,6 @@ static const struct {
>   DPI_REG(DPI_ID),
>  };
>  
> -static void vc4_dpi_dump_regs(struct vc4_dpi *dpi)
> -{
> - int i;
> -
> - for (i = 0; i < ARRAY_SIZE(dpi_regs); i++) {
> - DRM_INFO("0x%04x (%s): 0x%08x\n",
> -  dpi_regs[i].reg, dpi_regs[i].name,
> -  DPI_READ(dpi_regs[i].reg));
> - }
> -}
> -
>  #ifdef CONFIG_DEBUG_FS
>  int vc4_dpi_debugfs_regs(struct seq_file *m, void *unused)
>  {
> @@ -416,8 +405,6 @@ static int vc4_dpi_bind(struct device *dev, struct device 
> *master, void *data)
>   if (IS_ERR(dpi->regs))
>   return PTR_ERR(dpi->regs);
>  
> - vc4_dpi_dump_regs(dpi);
> -
>   if (DPI_READ(DPI_ID) != DPI_ID_VALUE) {
>   dev_err(dev, "Port returned 0x%08x for ID instead of 0x%08x\n",
>   DPI_READ(DPI_ID), DPI_ID_VALUE);
> -- 
> 2.11.0
> 
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v3] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Ben Gardner]

Allow the at24 driver to get configuration information from both OF and
ACPI by using the more generic device_property functions.
This change was inspired by the at25.c driver.

I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
With the following ACPI construct, this patch instantiates a working
instance of the driver.

Device (EEP0) {
 Name (_HID, "PRP0001")
 Name (_DSD, Package () {
  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
  Package () {
   Package () {"compatible", Package () {"st,24c02"}},
   Package () {"pagesize", 16},
  },
 })
 Name (_CRS, ResourceTemplate () {
  I2cSerialBus (
   0x0057, ControllerInitiated, 40,
   AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
   ResourceConsumer,,)
 })
}

Note: Matching the driver to the I2C device requires another patch.
 http://www.spinics.net/lists/linux-acpi/msg71914.html

Signed-off-by: Ben Gardner 
---
 drivers/misc/eeprom/at24.c | 45 +++--
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 051b147..c2d9969 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -19,7 +19,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, void 
*val, size_t count)
return 0;
 }
 
-#ifdef CONFIG_OF
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
+static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
-   const __be32 *val;
-   struct device_node *node = client->dev.of_node;
-
-   if (node) {
-   if (of_get_property(node, "read-only", NULL))
-   chip->flags |= AT24_FLAG_READONLY;
-   val = of_get_property(node, "pagesize", NULL);
-   if (val)
-   chip->page_size = be32_to_cpup(val);
+   int err;
+   u32 val;
+
+   if (device_property_present(dev, "read-only"))
+   chip->flags |= AT24_FLAG_READONLY;
+
+   err = device_property_read_u32(dev, "pagesize", );
+   if (!err) {
+   chip->page_size = val;
+   } else {
+   /*
+* This is slow, but we can't know all eeproms, so we better
+* play safe. Specifying custom eeprom-types via platform_data
+* is recommended anyhow.
+*/
+   chip.page_size = 1;
}
 }
-#else
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
-{ }
-#endif /* CONFIG_OF */
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
*id)
 {
@@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
magic >>= AT24_SIZE_BYTELEN;
chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
-   /*
-* This is slow, but we can't know all eeproms, so we better
-* play safe. Specifying custom eeprom-types via platform_data
-* is recommended anyhow.
-*/
-   chip.page_size = 1;
 
-   /* update chipdata if OF is present */
-   at24_get_ofdata(client, );
+   at24_get_pdata(>dev, );
 
chip.setup = NULL;
chip.context = NULL;
-- 
2.7.4

[PATCH v3] eeprom/at24: use device_property_*() functions instead of of_get_property()

[Ben Gardner]

Allow the at24 driver to get configuration information from both OF and
ACPI by using the more generic device_property functions.
This change was inspired by the at25.c driver.

I have a custom board with a ST M24C02 EEPROM attached to an I2C bus.
With the following ACPI construct, this patch instantiates a working
instance of the driver.

Device (EEP0) {
 Name (_HID, "PRP0001")
 Name (_DSD, Package () {
  ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
  Package () {
   Package () {"compatible", Package () {"st,24c02"}},
   Package () {"pagesize", 16},
  },
 })
 Name (_CRS, ResourceTemplate () {
  I2cSerialBus (
   0x0057, ControllerInitiated, 40,
   AddressingMode7Bit, "\\_SB.PCI0.I2C3", 0x00,
   ResourceConsumer,,)
 })
}

Note: Matching the driver to the I2C device requires another patch.
 http://www.spinics.net/lists/linux-acpi/msg71914.html

Signed-off-by: Ben Gardner 
---
 drivers/misc/eeprom/at24.c | 45 +++--
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 051b147..c2d9969 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -19,7 +19,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -562,26 +562,26 @@ static int at24_write(void *priv, unsigned int off, void 
*val, size_t count)
return 0;
 }
 
-#ifdef CONFIG_OF
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
+static void at24_get_pdata(struct device *dev, struct at24_platform_data *chip)
 {
-   const __be32 *val;
-   struct device_node *node = client->dev.of_node;
-
-   if (node) {
-   if (of_get_property(node, "read-only", NULL))
-   chip->flags |= AT24_FLAG_READONLY;
-   val = of_get_property(node, "pagesize", NULL);
-   if (val)
-   chip->page_size = be32_to_cpup(val);
+   int err;
+   u32 val;
+
+   if (device_property_present(dev, "read-only"))
+   chip->flags |= AT24_FLAG_READONLY;
+
+   err = device_property_read_u32(dev, "pagesize", );
+   if (!err) {
+   chip->page_size = val;
+   } else {
+   /*
+* This is slow, but we can't know all eeproms, so we better
+* play safe. Specifying custom eeprom-types via platform_data
+* is recommended anyhow.
+*/
+   chip.page_size = 1;
}
 }
-#else
-static void at24_get_ofdata(struct i2c_client *client,
-   struct at24_platform_data *chip)
-{ }
-#endif /* CONFIG_OF */
 
 static int at24_probe(struct i2c_client *client, const struct i2c_device_id 
*id)
 {
@@ -613,15 +613,8 @@ static int at24_probe(struct i2c_client *client, const 
struct i2c_device_id *id)
chip.byte_len = BIT(magic & AT24_BITMASK(AT24_SIZE_BYTELEN));
magic >>= AT24_SIZE_BYTELEN;
chip.flags = magic & AT24_BITMASK(AT24_SIZE_FLAGS);
-   /*
-* This is slow, but we can't know all eeproms, so we better
-* play safe. Specifying custom eeprom-types via platform_data
-* is recommended anyhow.
-*/
-   chip.page_size = 1;
 
-   /* update chipdata if OF is present */
-   at24_get_ofdata(client, );
+   at24_get_pdata(>dev, );
 
chip.setup = NULL;
chip.context = NULL;
-- 
2.7.4

Re: [PATCH 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method

[Thomas Gleixner]

On Thu, 9 Feb 2017, Vitaly Kuznetsov wrote:
> +#ifdef CONFIG_HYPERV_TSCPAGE
> +static notrace u64 vread_hvclock(int *mode)
> +{
> + const struct ms_hyperv_tsc_page *tsc_pg =
> + (const struct ms_hyperv_tsc_page *)_page;
> + u64 sequence, scale, offset, current_tick, cur_tsc;
> +
> + while (1) {
> + sequence = READ_ONCE(tsc_pg->tsc_sequence);
> + if (!sequence)
> + break;
> +
> + scale = READ_ONCE(tsc_pg->tsc_scale);
> + offset = READ_ONCE(tsc_pg->tsc_offset);
> + rdtscll(cur_tsc);
> +
> + current_tick = mul_u64_u64_shr(cur_tsc, scale, 64) + offset;
> +
> + if (READ_ONCE(tsc_pg->tsc_sequence) == sequence)
> + return current_tick;

That sequence stuff lacks still a sensible explanation. It's fundamentally
different from the sequence counting we do in the kernel, so documentation
for it is really required.

Thanks,

tglx

Re: [PATCH 2/2] x86/vdso: Add VCLOCK_HVCLOCK vDSO clock read method

[Thomas Gleixner]

On Thu, 9 Feb 2017, Vitaly Kuznetsov wrote:
> +#ifdef CONFIG_HYPERV_TSCPAGE
> +static notrace u64 vread_hvclock(int *mode)
> +{
> + const struct ms_hyperv_tsc_page *tsc_pg =
> + (const struct ms_hyperv_tsc_page *)_page;
> + u64 sequence, scale, offset, current_tick, cur_tsc;
> +
> + while (1) {
> + sequence = READ_ONCE(tsc_pg->tsc_sequence);
> + if (!sequence)
> + break;
> +
> + scale = READ_ONCE(tsc_pg->tsc_scale);
> + offset = READ_ONCE(tsc_pg->tsc_offset);
> + rdtscll(cur_tsc);
> +
> + current_tick = mul_u64_u64_shr(cur_tsc, scale, 64) + offset;
> +
> + if (READ_ONCE(tsc_pg->tsc_sequence) == sequence)
> + return current_tick;

That sequence stuff lacks still a sensible explanation. It's fundamentally
different from the sequence counting we do in the kernel, so documentation
for it is really required.

Thanks,

tglx

Re: [PATCH] netlink: move nla_put_{u8,u16,u32} out of line

[Arnd Bergmann]

On Thursday, February 9, 2017 4:51:25 PM CET Dmitry Vyukov wrote:
> On Thu, Feb 9, 2017 at 3:33 PM, Arnd Bergmann  wrote:
> >>>
> >>> Thanks for the list, that looks very helpful. The ones I found myself
> >>> seem to be a strict (and small) subset of those, using gcc-7.0.1 on x86-64
> >>> with allmodconfig and a few hundred randconfig builds. Which compiler
> >>> version did you use for your testing? If new versions are better than old
> >>> ones, we could start by fixing the ones that are still present in gcc-6 
> >>> and
> >>> gcc-7, and making the warning conditionally on the compiler version.
> >>>
> >>> Another idea might be to separate out asan_stack=1 into a separate
> >>> Kconfig option and warn if that is enabled with compilers that are known
> >>> to be relatively bad it keeping the stack small.
> >>
> >>
> >> Mine is gcc version 7.0.0 20161208. Make sure you enable KASAN_INLINE
> >> and I also enabled CONFIG_KCOV. Other than that I just did
> >> allyesconfig + make -k.
> >
> > Ok, I see my mistake now: On the allmodconfig build, I had KASAN_INLINE
> > disabled, which made the problem go away for almost all files (almost all
> > frame sizes are below 2048 bytes, except for the two issues I posted patches
> > for (hisilicon ethernet driver, and nla_put_* users).
> >
> > On the randconfig build test, I have a long series of patches applied that
> > address all known warnings, including my earlier "kasan: turn off
> > -fsanitize-address-use-after-scope for now" (see
> > https://patchwork.kernel.org/patch/9442323/). Without
> > -fsanitize-address-use-after-scope, the problem is also mostly gone
> > (a few cases show up in drivers/media/, and also in block/sed-opal.c
> > and drivers/tty/vt/keyboard.c). I now have initial patches for all of
> > them to bring the stack size below 2048 bytes.
> >
> > As far as I can see, the remaining problems with scary stack frame sizes
> > you found only show up with the combination of all three: KASAN_INLINE,
> > asan_stack=1 and -fsanitize-address-use-after-scope. If all three were
> > separately configurable and we merge the patches I made, I think we could
> > enable the normal 2048 byte warning in all configurations that have at least
> > one of the turned off, but I don't know which of those combinations would
> > actually be sensible for production kernels.
> 
> FWIW I use all of them (+KCOV which is additional instrumentation).
> If you ask which is the least important one, it is
> -fsanitize-address-use-after-scope. I have not see any single report
> due to it (probably due to kernel coding style).

Ok, I'm testing with this patch now and will let you know if I run
into any other warnings on my x86 randconfig build. I'll also try
to get some results from other compiler versions and architectures.

Let me know what you think of this approach.

Arnd

>From 119968322aa83f8039531d704e5f1cf24e680680 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann 
Date: Thu, 9 Feb 2017 17:45:59 +0100
Subject: [PATCH] kasan: make use-after-scope sanitizer optional

We get a lot of very large stack frames when combining CONFIG_KASAN_INLINE
with the default -fsanitize-address-use-after-scope --param asan-stack=1
options, which can easily cause an overflow of the kernel stack, e.g.

drivers/acpi/nfit/core.c:2686:1: warning: the frame size of 4080 bytes is 
larger than 2048 bytes [-Wframe-larger-than=]
drivers/gpu/drm/amd/amdgpu/si.c:1756:1: warning: the frame size of 7304 bytes 
is larger than 2048 bytes [-Wframe-larger-than=]
drivers/gpu/drm/i915/gvt/handlers.c:2200:1: warning: the frame size of 43752 
bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:952:1: warning: the frame size of 6032 
bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/isdn/hardware/avm/b1.c:637:1: warning: the frame size of 13200 bytes is 
larger than 2048 bytes [-Wframe-larger-than=]
drivers/media/dvb-frontends/stv090x.c:3089:1: warning: the frame size of 5880 
bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/media/i2c/cx25840/cx25840-core.c:4964:1: warning: the frame size of 
93992 bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4994:1: warning: the frame size 
of 23928 bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/staging/dgnc/dgnc_tty.c:2788:1: warning: the frame size of 7072 bytes 
is larger than 2048 bytes [-Wframe-larger-than=]
fs/ntfs/mft.c:2762:1: warning: the frame size of 7432 bytes is larger than 2048 
bytes [-Wframe-larger-than=]
lib/atomic64_test.c:242:1: warning: the frame size of 12648 bytes is larger 
than 2048 bytes [-Wframe-larger-than=]

To reduce this risk, -fsanitize-address-use-after-scope is now split out
into a separate Kconfig option, which cannot be selected at the same time
as CONFIG_KASAN_INLINE, leading to stack frames that are smaller than 2
kilobytes most of the time on x86_64. Now we can turn on the warning 

Re: [PATCH] netlink: move nla_put_{u8,u16,u32} out of line

[Arnd Bergmann]

On Thursday, February 9, 2017 4:51:25 PM CET Dmitry Vyukov wrote:
> On Thu, Feb 9, 2017 at 3:33 PM, Arnd Bergmann  wrote:
> >>>
> >>> Thanks for the list, that looks very helpful. The ones I found myself
> >>> seem to be a strict (and small) subset of those, using gcc-7.0.1 on x86-64
> >>> with allmodconfig and a few hundred randconfig builds. Which compiler
> >>> version did you use for your testing? If new versions are better than old
> >>> ones, we could start by fixing the ones that are still present in gcc-6 
> >>> and
> >>> gcc-7, and making the warning conditionally on the compiler version.
> >>>
> >>> Another idea might be to separate out asan_stack=1 into a separate
> >>> Kconfig option and warn if that is enabled with compilers that are known
> >>> to be relatively bad it keeping the stack small.
> >>
> >>
> >> Mine is gcc version 7.0.0 20161208. Make sure you enable KASAN_INLINE
> >> and I also enabled CONFIG_KCOV. Other than that I just did
> >> allyesconfig + make -k.
> >
> > Ok, I see my mistake now: On the allmodconfig build, I had KASAN_INLINE
> > disabled, which made the problem go away for almost all files (almost all
> > frame sizes are below 2048 bytes, except for the two issues I posted patches
> > for (hisilicon ethernet driver, and nla_put_* users).
> >
> > On the randconfig build test, I have a long series of patches applied that
> > address all known warnings, including my earlier "kasan: turn off
> > -fsanitize-address-use-after-scope for now" (see
> > https://patchwork.kernel.org/patch/9442323/). Without
> > -fsanitize-address-use-after-scope, the problem is also mostly gone
> > (a few cases show up in drivers/media/, and also in block/sed-opal.c
> > and drivers/tty/vt/keyboard.c). I now have initial patches for all of
> > them to bring the stack size below 2048 bytes.
> >
> > As far as I can see, the remaining problems with scary stack frame sizes
> > you found only show up with the combination of all three: KASAN_INLINE,
> > asan_stack=1 and -fsanitize-address-use-after-scope. If all three were
> > separately configurable and we merge the patches I made, I think we could
> > enable the normal 2048 byte warning in all configurations that have at least
> > one of the turned off, but I don't know which of those combinations would
> > actually be sensible for production kernels.
> 
> FWIW I use all of them (+KCOV which is additional instrumentation).
> If you ask which is the least important one, it is
> -fsanitize-address-use-after-scope. I have not see any single report
> due to it (probably due to kernel coding style).

Ok, I'm testing with this patch now and will let you know if I run
into any other warnings on my x86 randconfig build. I'll also try
to get some results from other compiler versions and architectures.

Let me know what you think of this approach.

Arnd

>From 119968322aa83f8039531d704e5f1cf24e680680 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann 
Date: Thu, 9 Feb 2017 17:45:59 +0100
Subject: [PATCH] kasan: make use-after-scope sanitizer optional

We get a lot of very large stack frames when combining CONFIG_KASAN_INLINE
with the default -fsanitize-address-use-after-scope --param asan-stack=1
options, which can easily cause an overflow of the kernel stack, e.g.

drivers/acpi/nfit/core.c:2686:1: warning: the frame size of 4080 bytes is 
larger than 2048 bytes [-Wframe-larger-than=]
drivers/gpu/drm/amd/amdgpu/si.c:1756:1: warning: the frame size of 7304 bytes 
is larger than 2048 bytes [-Wframe-larger-than=]
drivers/gpu/drm/i915/gvt/handlers.c:2200:1: warning: the frame size of 43752 
bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:952:1: warning: the frame size of 6032 
bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/isdn/hardware/avm/b1.c:637:1: warning: the frame size of 13200 bytes is 
larger than 2048 bytes [-Wframe-larger-than=]
drivers/media/dvb-frontends/stv090x.c:3089:1: warning: the frame size of 5880 
bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/media/i2c/cx25840/cx25840-core.c:4964:1: warning: the frame size of 
93992 bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4994:1: warning: the frame size 
of 23928 bytes is larger than 2048 bytes [-Wframe-larger-than=]
drivers/staging/dgnc/dgnc_tty.c:2788:1: warning: the frame size of 7072 bytes 
is larger than 2048 bytes [-Wframe-larger-than=]
fs/ntfs/mft.c:2762:1: warning: the frame size of 7432 bytes is larger than 2048 
bytes [-Wframe-larger-than=]
lib/atomic64_test.c:242:1: warning: the frame size of 12648 bytes is larger 
than 2048 bytes [-Wframe-larger-than=]

To reduce this risk, -fsanitize-address-use-after-scope is now split out
into a separate Kconfig option, which cannot be selected at the same time
as CONFIG_KASAN_INLINE, leading to stack frames that are smaller than 2
kilobytes most of the time on x86_64. Now we can turn on the warning again
that was disabled in 

Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC

[Daniel Vetter]

On Thu, Feb 02, 2017 at 11:31:57AM +0100, Maxime Ripard wrote:
> From: Stefan Christ 
> 
> Implement legacy framebuffer ioctl FBIO_WAITFORVSYNC in the generic
> framebuffer emulation driver. Legacy framebuffer users like non kms/drm
> based OpenGL(ES)/EGL implementations may require the ioctl to
> synchronize drawing or buffer flip for double buffering. It is tested on
> the i.MX6.
> 
> Code is based on
> 
> https://github.com/Xilinx/linux-xlnx/blob/master/drivers/gpu/drm/xilinx/xilinx_drm_fb.c#L196
> 
> Signed-off-by: Stefan Christ 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 55 ++-
>  include/drm/drm_fb_helper.h |  5 ++-
>  2 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e934b541feea..39a3532e311c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1234,6 +1234,61 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
> fb_info *info)
>  EXPORT_SYMBOL(drm_fb_helper_setcmap);
>  
>  /**
> + * drm_fb_helper_ioctl - legacy ioctl implementation
> + * @info: fbdev registered by the helper
> + * @cmd: ioctl command
> + * @arg: ioctl argument
> + *
> + * A helper to implement the standard fbdev ioctl. Only
> + * FBIO_WAITFORVSYNC is implemented for now.
> + */
> +int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, unsigned 
> long arg)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> + struct drm_device *dev = fb_helper->dev;
> + unsigned int i;
> + int ret = 0;
> +
> + drm_modeset_lock_all(dev);
> + if (!drm_fb_helper_is_bound(fb_helper)) {
> + drm_modeset_unlock_all(dev);
> + return -EBUSY;
> + }
> +
> + switch (cmd) {
> + case FBIO_WAITFORVSYNC:
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + struct drm_mode_set *mode_set;
> + struct drm_crtc *crtc;
> +
> + mode_set = _helper->crtc_info[i].mode_set;
> + crtc = mode_set->crtc;
> +
> + /*
> +  * Only call drm_crtc_wait_one_vblank for crtcs that
> +  * are currently enabled.
> +  */
> + if (!crtc->enabled)
> + continue;

This needs locking, and the interface spec for vblank_get says you'll get
an -EINVAL if the pipe is off. I'd say drop this, and instead eat the
-EINVAL from vblank_get and explain in a comment why you do that.

The trouble is that ->enabled is legacy state, atomic drivers don't need
to update it (helpers do it by default, but only for transition reasons),
and it's not synchronized to the real state changes at all. vblank_get
otoh is.

Otherwise lgtm.
-Daniel

> +
> + ret = drm_crtc_vblank_get(crtc);
> + if (!ret) {
> + drm_crtc_wait_one_vblank(crtc);
> + drm_crtc_vblank_put(crtc);
> + }
> + }
> + goto unlock;
> + default:
> + ret = -ENOTTY;
> + }
> +
> +unlock:
> + drm_modeset_unlock_all(dev);
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_ioctl);
> +
> +/**
>   * drm_fb_helper_check_var - implementation for ->fb_check_var
>   * @var: screeninfo to check
>   * @info: fbdev registered by the helper
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 975deedd593e..460be9d9fa98 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -230,7 +230,8 @@ struct drm_fb_helper {
>   .fb_blank   = drm_fb_helper_blank, \
>   .fb_pan_display = drm_fb_helper_pan_display, \
>   .fb_debug_enter = drm_fb_helper_debug_enter, \
> - .fb_debug_leave = drm_fb_helper_debug_leave
> + .fb_debug_leave = drm_fb_helper_debug_leave, \
> + .fb_ioctl   = drm_fb_helper_ioctl
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>  void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper 
> *helper,
> @@ -286,6 +287,8 @@ void drm_fb_helper_set_suspend_unlocked(struct 
> drm_fb_helper *fb_helper,
>  
>  int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info);
>  
> +int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, unsigned 
> long arg);
> +
>  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
>  int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int 
> bpp_sel);
>  int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper);
> -- 
> git-series 0.8.11
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel 

Re: [PATCH v2 2/2] drm/fb_helper: implement ioctl FBIO_WAITFORVSYNC

[Daniel Vetter]

On Thu, Feb 02, 2017 at 11:31:57AM +0100, Maxime Ripard wrote:
> From: Stefan Christ 
> 
> Implement legacy framebuffer ioctl FBIO_WAITFORVSYNC in the generic
> framebuffer emulation driver. Legacy framebuffer users like non kms/drm
> based OpenGL(ES)/EGL implementations may require the ioctl to
> synchronize drawing or buffer flip for double buffering. It is tested on
> the i.MX6.
> 
> Code is based on
> 
> https://github.com/Xilinx/linux-xlnx/blob/master/drivers/gpu/drm/xilinx/xilinx_drm_fb.c#L196
> 
> Signed-off-by: Stefan Christ 
> Signed-off-by: Maxime Ripard 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 55 ++-
>  include/drm/drm_fb_helper.h |  5 ++-
>  2 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index e934b541feea..39a3532e311c 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1234,6 +1234,61 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct 
> fb_info *info)
>  EXPORT_SYMBOL(drm_fb_helper_setcmap);
>  
>  /**
> + * drm_fb_helper_ioctl - legacy ioctl implementation
> + * @info: fbdev registered by the helper
> + * @cmd: ioctl command
> + * @arg: ioctl argument
> + *
> + * A helper to implement the standard fbdev ioctl. Only
> + * FBIO_WAITFORVSYNC is implemented for now.
> + */
> +int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, unsigned 
> long arg)
> +{
> + struct drm_fb_helper *fb_helper = info->par;
> + struct drm_device *dev = fb_helper->dev;
> + unsigned int i;
> + int ret = 0;
> +
> + drm_modeset_lock_all(dev);
> + if (!drm_fb_helper_is_bound(fb_helper)) {
> + drm_modeset_unlock_all(dev);
> + return -EBUSY;
> + }
> +
> + switch (cmd) {
> + case FBIO_WAITFORVSYNC:
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + struct drm_mode_set *mode_set;
> + struct drm_crtc *crtc;
> +
> + mode_set = _helper->crtc_info[i].mode_set;
> + crtc = mode_set->crtc;
> +
> + /*
> +  * Only call drm_crtc_wait_one_vblank for crtcs that
> +  * are currently enabled.
> +  */
> + if (!crtc->enabled)
> + continue;

This needs locking, and the interface spec for vblank_get says you'll get
an -EINVAL if the pipe is off. I'd say drop this, and instead eat the
-EINVAL from vblank_get and explain in a comment why you do that.

The trouble is that ->enabled is legacy state, atomic drivers don't need
to update it (helpers do it by default, but only for transition reasons),
and it's not synchronized to the real state changes at all. vblank_get
otoh is.

Otherwise lgtm.
-Daniel

> +
> + ret = drm_crtc_vblank_get(crtc);
> + if (!ret) {
> + drm_crtc_wait_one_vblank(crtc);
> + drm_crtc_vblank_put(crtc);
> + }
> + }
> + goto unlock;
> + default:
> + ret = -ENOTTY;
> + }
> +
> +unlock:
> + drm_modeset_unlock_all(dev);
> + return ret;
> +}
> +EXPORT_SYMBOL(drm_fb_helper_ioctl);
> +
> +/**
>   * drm_fb_helper_check_var - implementation for ->fb_check_var
>   * @var: screeninfo to check
>   * @info: fbdev registered by the helper
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 975deedd593e..460be9d9fa98 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -230,7 +230,8 @@ struct drm_fb_helper {
>   .fb_blank   = drm_fb_helper_blank, \
>   .fb_pan_display = drm_fb_helper_pan_display, \
>   .fb_debug_enter = drm_fb_helper_debug_enter, \
> - .fb_debug_leave = drm_fb_helper_debug_leave
> + .fb_debug_leave = drm_fb_helper_debug_leave, \
> + .fb_ioctl   = drm_fb_helper_ioctl
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>  void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper 
> *helper,
> @@ -286,6 +287,8 @@ void drm_fb_helper_set_suspend_unlocked(struct 
> drm_fb_helper *fb_helper,
>  
>  int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info);
>  
> +int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd, unsigned 
> long arg);
> +
>  int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);
>  int drm_fb_helper_initial_config(struct drm_fb_helper *fb_helper, int 
> bpp_sel);
>  int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper);
> -- 
> git-series 0.8.11
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v4 1/2] staging: omap4iss: fix multiline comment style

[Avraham Shukron]

Fixed multi-line comments to their preferred style (First line empty)

Signed-off-by: Avraham Shukron 
---
 drivers/staging/media/omap4iss/iss_video.c | 38 --
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index bb0e3b4..e21811a 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -128,7 +128,8 @@ static unsigned int iss_video_mbus_to_pix(const struct 
iss_video *video,
pix->width = mbus->width;
pix->height = mbus->height;

-   /* Skip the last format in the loop so that it will be selected if no
+   /*
+* Skip the last format in the loop so that it will be selected if no
 * match is found.
 */
for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -138,7 +139,8 @@ static unsigned int iss_video_mbus_to_pix(const struct 
iss_video *video,

min_bpl = pix->width * ALIGN(formats[i].bpp, 8) / 8;

-   /* Clamp the requested bytes per line value. If the maximum bytes per
+   /*
+* Clamp the requested bytes per line value. If the maximum bytes per
 * line value is zero, the module doesn't support user configurable line
 * sizes. Override the requested value with the minimum in that case.
 */
@@ -172,7 +174,8 @@ static void iss_video_pix_to_mbus(const struct 
v4l2_pix_format *pix,
mbus->width = pix->width;
mbus->height = pix->height;

-   /* Skip the last format in the loop so that it will be selected if no
+   /*
+* Skip the last format in the loop so that it will be selected if no
 * match is found.
 */
for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -360,7 +363,8 @@ static void iss_video_buf_queue(struct vb2_buffer *vb)

spin_lock_irqsave(>qlock, flags);

-   /* Mark the buffer is faulty and give it back to the queue immediately
+   /*
+* Mark the buffer is faulty and give it back to the queue immediately
 * if the video node has registered an error. vb2 will perform the same
 * check when preparing the buffer, but that is inherently racy, so we
 * need to handle the race condition with an authoritative check here.
@@ -443,7 +447,8 @@ struct iss_buffer *omap4iss_video_buffer_next(struct 
iss_video *video)

buf->vb.vb2_buf.timestamp = ktime_get_ns();

-   /* Do frame number propagation only if this is the output video node.
+   /*
+* Do frame number propagation only if this is the output video node.
 * Frame number either comes from the CSI receivers or it gets
 * incremented here if H3A is not active.
 * Note: There is no guarantee that the output buffer will finish
@@ -605,7 +610,8 @@ iss_video_set_format(struct file *file, void *fh, struct 
v4l2_format *format)

mutex_lock(>mutex);

-   /* Fill the bytesperline and sizeimage fields by converting to media bus
+   /*
+* Fill the bytesperline and sizeimage fields by converting to media bus
 * format and back to pixel format.
 */
iss_video_pix_to_mbus(>fmt.pix, );
@@ -678,8 +684,9 @@ iss_video_get_selection(struct file *file, void *fh, struct 
v4l2_selection *sel)
if (subdev == NULL)
return -EINVAL;

-   /* Try the get selection operation first and fallback to get format if 
not
-* implemented.
+   /*
+* Try the get selection operation first and fallback to get format if
+* not implemented.
 */
sdsel.pad = pad;
ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, );
@@ -867,7 +874,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)

mutex_lock(>stream_lock);

-   /* Start streaming on the pipeline. No link touching an entity in the
+   /*
+* Start streaming on the pipeline. No link touching an entity in the
 * pipeline can be activated or deactivated once streaming is started.
 */
pipe = entity->pipe
@@ -895,7 +903,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)
while ((entity = media_graph_walk_next()))
media_entity_enum_set(>ent_enum, entity);

-   /* Verify that the currently configured format matches the output of
+   /*
+* Verify that the currently configured format matches the output of
 * the connected subdev.
 */
ret = iss_video_check_format(video, vfh);
@@ -905,7 +914,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)
video->bpl_padding = ret;
video->bpl_value = vfh->format.fmt.pix.bytesperline;

-   /* Find the ISS video node connected at the far end of the pipeline and
+   /*
+* Find the ISS video node connected at the far end of the 

[PATCH RT 1/2] workqueue: use rcu_readlock() in put_pwq_unlocked()

[Steven Rostedt]

3.12.70-rt94-rc1 stable review patch.
If anyone has any objections, please let me know.

--

From: Sebastian Andrzej Siewior 

The RCU sched protection was changed to RCU only and so all IRQ-off and
preempt-off disabled region were changed to the relevant rcu-read-lock
primitives. One was missed and triggered:
|[ BUG: bad unlock balance detected! ]
|4.4.30-rt41 #51 Tainted: GW
|btattach/345 is trying to release lock (
|Unable to handle kernel paging request at virtual address 6b6b6bbb
|Backtrace:
|[] (lock_release) from [] (rt_spin_unlock+0x20/0x30)
|[] (rt_spin_unlock) from [] (put_pwq_unlocked+0xa4/0x118)
|[] (put_pwq_unlocked) from [] 
(destroy_workqueue+0x164/0x1b0)
|[] (destroy_workqueue) from [] 
(hci_unregister_dev+0x120/0x21c)
|[] (hci_unregister_dev) from [] 
(hci_uart_tty_close+0x90/0xbc)
|[] (hci_uart_tty_close) from [] (tty_ldisc_close+0x50/0x58)
|[] (tty_ldisc_close) from [] (tty_ldisc_kill+0x18/0x78)
|[] (tty_ldisc_kill) from [] (tty_ldisc_release+0x100/0x134)
|[] (tty_ldisc_release) from [] (tty_release+0x3bc/0x460)
|[] (tty_release) from [] (__fput+0xe0/0x1b4)
|[] (__fput) from [] (fput+0x10/0x14)
|[] (fput) from [] (task_work_run+0xa4/0xb8)
|[] (task_work_run) from [] (do_exit+0x40c/0x8b0)
|[] (do_exit) from [] (do_group_exit+0x54/0xc4)

Cc: stable...@vger.kernel.org
Reported-by: John Keeping 
Tested-by: John Keeping 
Signed-off-by: Sebastian Andrzej Siewior 
Signed-off-by: Steven Rostedt (VMware) 
---
 kernel/workqueue.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5e7cd0dcf6a4..6d36faa3b666 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1135,9 +1135,11 @@ static void put_pwq_unlocked(struct pool_workqueue *pwq)
 * As both pwqs and pools are RCU protected, the
 * following lock operations are safe.
 */
+   rcu_read_lock();
local_spin_lock_irq(pendingb_lock, >pool->lock);
put_pwq(pwq);
local_spin_unlock_irq(pendingb_lock, >pool->lock);
+   rcu_read_unlock();
}
 }
 
-- 
2.10.2

[PATCH v4 1/2] staging: omap4iss: fix multiline comment style

[Avraham Shukron]

Fixed multi-line comments to their preferred style (First line empty)

Signed-off-by: Avraham Shukron 
---
 drivers/staging/media/omap4iss/iss_video.c | 38 --
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/media/omap4iss/iss_video.c 
b/drivers/staging/media/omap4iss/iss_video.c
index bb0e3b4..e21811a 100644
--- a/drivers/staging/media/omap4iss/iss_video.c
+++ b/drivers/staging/media/omap4iss/iss_video.c
@@ -128,7 +128,8 @@ static unsigned int iss_video_mbus_to_pix(const struct 
iss_video *video,
pix->width = mbus->width;
pix->height = mbus->height;

-   /* Skip the last format in the loop so that it will be selected if no
+   /*
+* Skip the last format in the loop so that it will be selected if no
 * match is found.
 */
for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -138,7 +139,8 @@ static unsigned int iss_video_mbus_to_pix(const struct 
iss_video *video,

min_bpl = pix->width * ALIGN(formats[i].bpp, 8) / 8;

-   /* Clamp the requested bytes per line value. If the maximum bytes per
+   /*
+* Clamp the requested bytes per line value. If the maximum bytes per
 * line value is zero, the module doesn't support user configurable line
 * sizes. Override the requested value with the minimum in that case.
 */
@@ -172,7 +174,8 @@ static void iss_video_pix_to_mbus(const struct 
v4l2_pix_format *pix,
mbus->width = pix->width;
mbus->height = pix->height;

-   /* Skip the last format in the loop so that it will be selected if no
+   /*
+* Skip the last format in the loop so that it will be selected if no
 * match is found.
 */
for (i = 0; i < ARRAY_SIZE(formats) - 1; ++i) {
@@ -360,7 +363,8 @@ static void iss_video_buf_queue(struct vb2_buffer *vb)

spin_lock_irqsave(>qlock, flags);

-   /* Mark the buffer is faulty and give it back to the queue immediately
+   /*
+* Mark the buffer is faulty and give it back to the queue immediately
 * if the video node has registered an error. vb2 will perform the same
 * check when preparing the buffer, but that is inherently racy, so we
 * need to handle the race condition with an authoritative check here.
@@ -443,7 +447,8 @@ struct iss_buffer *omap4iss_video_buffer_next(struct 
iss_video *video)

buf->vb.vb2_buf.timestamp = ktime_get_ns();

-   /* Do frame number propagation only if this is the output video node.
+   /*
+* Do frame number propagation only if this is the output video node.
 * Frame number either comes from the CSI receivers or it gets
 * incremented here if H3A is not active.
 * Note: There is no guarantee that the output buffer will finish
@@ -605,7 +610,8 @@ iss_video_set_format(struct file *file, void *fh, struct 
v4l2_format *format)

mutex_lock(>mutex);

-   /* Fill the bytesperline and sizeimage fields by converting to media bus
+   /*
+* Fill the bytesperline and sizeimage fields by converting to media bus
 * format and back to pixel format.
 */
iss_video_pix_to_mbus(>fmt.pix, );
@@ -678,8 +684,9 @@ iss_video_get_selection(struct file *file, void *fh, struct 
v4l2_selection *sel)
if (subdev == NULL)
return -EINVAL;

-   /* Try the get selection operation first and fallback to get format if 
not
-* implemented.
+   /*
+* Try the get selection operation first and fallback to get format if
+* not implemented.
 */
sdsel.pad = pad;
ret = v4l2_subdev_call(subdev, pad, get_selection, NULL, );
@@ -867,7 +874,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)

mutex_lock(>stream_lock);

-   /* Start streaming on the pipeline. No link touching an entity in the
+   /*
+* Start streaming on the pipeline. No link touching an entity in the
 * pipeline can be activated or deactivated once streaming is started.
 */
pipe = entity->pipe
@@ -895,7 +903,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)
while ((entity = media_graph_walk_next()))
media_entity_enum_set(>ent_enum, entity);

-   /* Verify that the currently configured format matches the output of
+   /*
+* Verify that the currently configured format matches the output of
 * the connected subdev.
 */
ret = iss_video_check_format(video, vfh);
@@ -905,7 +914,8 @@ iss_video_streamon(struct file *file, void *fh, enum 
v4l2_buf_type type)
video->bpl_padding = ret;
video->bpl_value = vfh->format.fmt.pix.bytesperline;

-   /* Find the ISS video node connected at the far end of the pipeline and
+   /*
+* Find the ISS video node connected at the far end of the pipeline and
 * 

[PATCH RT 1/2] workqueue: use rcu_readlock() in put_pwq_unlocked()

[Steven Rostedt]

3.12.70-rt94-rc1 stable review patch.
If anyone has any objections, please let me know.

--

From: Sebastian Andrzej Siewior 

The RCU sched protection was changed to RCU only and so all IRQ-off and
preempt-off disabled region were changed to the relevant rcu-read-lock
primitives. One was missed and triggered:
|[ BUG: bad unlock balance detected! ]
|4.4.30-rt41 #51 Tainted: GW
|btattach/345 is trying to release lock (
|Unable to handle kernel paging request at virtual address 6b6b6bbb
|Backtrace:
|[] (lock_release) from [] (rt_spin_unlock+0x20/0x30)
|[] (rt_spin_unlock) from [] (put_pwq_unlocked+0xa4/0x118)
|[] (put_pwq_unlocked) from [] 
(destroy_workqueue+0x164/0x1b0)
|[] (destroy_workqueue) from [] 
(hci_unregister_dev+0x120/0x21c)
|[] (hci_unregister_dev) from [] 
(hci_uart_tty_close+0x90/0xbc)
|[] (hci_uart_tty_close) from [] (tty_ldisc_close+0x50/0x58)
|[] (tty_ldisc_close) from [] (tty_ldisc_kill+0x18/0x78)
|[] (tty_ldisc_kill) from [] (tty_ldisc_release+0x100/0x134)
|[] (tty_ldisc_release) from [] (tty_release+0x3bc/0x460)
|[] (tty_release) from [] (__fput+0xe0/0x1b4)
|[] (__fput) from [] (fput+0x10/0x14)
|[] (fput) from [] (task_work_run+0xa4/0xb8)
|[] (task_work_run) from [] (do_exit+0x40c/0x8b0)
|[] (do_exit) from [] (do_group_exit+0x54/0xc4)

Cc: stable...@vger.kernel.org
Reported-by: John Keeping 
Tested-by: John Keeping 
Signed-off-by: Sebastian Andrzej Siewior 
Signed-off-by: Steven Rostedt (VMware) 
---
 kernel/workqueue.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 5e7cd0dcf6a4..6d36faa3b666 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1135,9 +1135,11 @@ static void put_pwq_unlocked(struct pool_workqueue *pwq)
 * As both pwqs and pools are RCU protected, the
 * following lock operations are safe.
 */
+   rcu_read_lock();
local_spin_lock_irq(pendingb_lock, >pool->lock);
put_pwq(pwq);
local_spin_unlock_irq(pendingb_lock, >pool->lock);
+   rcu_read_unlock();
}
 }
 
-- 
2.10.2

Re: [PATCHv6 01/37] mm, shmem: swich huge tmpfs to multi-order radix-tree entries

[Kirill A. Shutemov]

On Wed, Feb 08, 2017 at 07:57:27PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 26, 2017 at 02:57:43PM +0300, Kirill A. Shutemov wrote:
> > +++ b/include/linux/pagemap.h
> > @@ -332,6 +332,15 @@ static inline struct page 
> > *grab_cache_page_nowait(struct address_space *mapping,
> > mapping_gfp_mask(mapping));
> >  }
> >  
> > +static inline struct page *find_subpage(struct page *page, pgoff_t offset)
> > +{
> > +   VM_BUG_ON_PAGE(PageTail(page), page);
> > +   VM_BUG_ON_PAGE(page->index > offset, page);
> > +   VM_BUG_ON_PAGE(page->index + (1 << compound_order(page)) < offset,
> > +   page);
> > +   return page - page->index + offset;
> > +}
> 
> What would you think to:
> 
> static inline void check_page_index(struct page *page, pgoff_t offset)
> {
>   VM_BUG_ON_PAGE(PageTail(page), page);
>   VM_BUG_ON_PAGE(page->index > offset, page);
>   VM_BUG_ON_PAGE(page->index + (1 << compound_order(page)) <= offset,
>   page);
> }
> 
> (I think I fixed an off-by-one error up there ...  if
> index + (1 << order) == offset, this is also a bug, right?
> because offset would then refer to the next page, not this page)

Right, thanks.

> 
> static inline struct page *find_subpage(struct page *page, pgoff_t offset)
> {
>   check_page_index(page, offset);
>   return page + (offset - page->index);
> }
> 
> ... then you can use check_page_index down ...

Okay, makes sense.

> 
> > @@ -1250,7 +1233,6 @@ struct page *find_lock_entry(struct address_space 
> > *mapping, pgoff_t offset)
> > put_page(page);
> > goto repeat;
> > }
> > -   VM_BUG_ON_PAGE(page_to_pgoff(page) != offset, page);
> 
> ... here?

Ok.

> > @@ -1472,25 +1451,35 @@ unsigned find_get_pages(struct address_space 
> > *mapping, pgoff_t start,
> > goto repeat;
> > }
> >  
> > +   /* For multi-order entries, find relevant subpage */
> > +   if (PageTransHuge(page)) {
> > +   VM_BUG_ON(index - page->index < 0);
> > +   VM_BUG_ON(index - page->index >= HPAGE_PMD_NR);
> > +   page += index - page->index;
> > +   }
> 
> Use find_subpage() here?

Ok.

> > pages[ret] = page;
> > if (++ret == nr_pages)
> > break;
> > +   if (!PageTransCompound(page))
> > +   continue;
> > +   for (refs = 0; ret < nr_pages &&
> > +   (index + 1) % HPAGE_PMD_NR;
> > +   ret++, refs++, index++)
> > +   pages[ret] = ++page;
> > +   if (refs)
> > +   page_ref_add(compound_head(page), refs);
> > +   if (ret == nr_pages)
> > +   break;
> 
> Can we avoid referencing huge pages specifically in the page cache?  I'd
> like us to get to the point where we can put arbitrary compound pages into
> the page cache.  For example, I think this can be written as:
> 
>   if (!PageCompound(page))
>   continue;
>   for (refs = 0; ret < nr_pages; refs++, index++) {
>   if (index > page->index + (1 << compound_order(page)))
>   break;
>   pages[ret++] = ++page;
>   }
>   if (refs)
>   page_ref_add(compound_head(page), refs);
>   if (ret == nr_pages)
>   break;

That's slightly more costly, but I guess that's fine.

> > @@ -1541,19 +1533,12 @@ unsigned find_get_pages_contig(struct address_space 
> > *mapping, pgoff_t index,
> >  
> > +   /* For multi-order entries, find relevant subpage */
> > +   if (PageTransHuge(page)) {
> > +   VM_BUG_ON(index - page->index < 0);
> > +   VM_BUG_ON(index - page->index >= HPAGE_PMD_NR);
> > +   page += index - page->index;
> > +   }
> > +
> > pages[ret] = page;
> > if (++ret == nr_pages)
> > break;
> > +   if (!PageTransCompound(page))
> > +   continue;
> > +   for (refs = 0; ret < nr_pages &&
> > +   (index + 1) % HPAGE_PMD_NR;
> > +   ret++, refs++, index++)
> > +   pages[ret] = ++page;
> > +   if (refs)
> > +   page_ref_add(compound_head(page), refs);
> > +   if (ret == nr_pages)
> > +   break;
> > }
> > rcu_read_unlock();
> > return ret;
> 
> Ugh, the same code again.  Hmm ... we only need to modify 'ret' as a result
> of this ... so could we split it out like this?
> 
> static unsigned populate_pages(struct page **pages, unsigned i, unsigned max,
> struct page *page)
> {
> unsigned refs = 0;
> for (;;) {
> pages[i++] = page;
>   

Re: [PATCHv6 01/37] mm, shmem: swich huge tmpfs to multi-order radix-tree entries

[Kirill A. Shutemov]

On Wed, Feb 08, 2017 at 07:57:27PM -0800, Matthew Wilcox wrote:
> On Thu, Jan 26, 2017 at 02:57:43PM +0300, Kirill A. Shutemov wrote:
> > +++ b/include/linux/pagemap.h
> > @@ -332,6 +332,15 @@ static inline struct page 
> > *grab_cache_page_nowait(struct address_space *mapping,
> > mapping_gfp_mask(mapping));
> >  }
> >  
> > +static inline struct page *find_subpage(struct page *page, pgoff_t offset)
> > +{
> > +   VM_BUG_ON_PAGE(PageTail(page), page);
> > +   VM_BUG_ON_PAGE(page->index > offset, page);
> > +   VM_BUG_ON_PAGE(page->index + (1 << compound_order(page)) < offset,
> > +   page);
> > +   return page - page->index + offset;
> > +}
> 
> What would you think to:
> 
> static inline void check_page_index(struct page *page, pgoff_t offset)
> {
>   VM_BUG_ON_PAGE(PageTail(page), page);
>   VM_BUG_ON_PAGE(page->index > offset, page);
>   VM_BUG_ON_PAGE(page->index + (1 << compound_order(page)) <= offset,
>   page);
> }
> 
> (I think I fixed an off-by-one error up there ...  if
> index + (1 << order) == offset, this is also a bug, right?
> because offset would then refer to the next page, not this page)

Right, thanks.

> 
> static inline struct page *find_subpage(struct page *page, pgoff_t offset)
> {
>   check_page_index(page, offset);
>   return page + (offset - page->index);
> }
> 
> ... then you can use check_page_index down ...

Okay, makes sense.

> 
> > @@ -1250,7 +1233,6 @@ struct page *find_lock_entry(struct address_space 
> > *mapping, pgoff_t offset)
> > put_page(page);
> > goto repeat;
> > }
> > -   VM_BUG_ON_PAGE(page_to_pgoff(page) != offset, page);
> 
> ... here?

Ok.

> > @@ -1472,25 +1451,35 @@ unsigned find_get_pages(struct address_space 
> > *mapping, pgoff_t start,
> > goto repeat;
> > }
> >  
> > +   /* For multi-order entries, find relevant subpage */
> > +   if (PageTransHuge(page)) {
> > +   VM_BUG_ON(index - page->index < 0);
> > +   VM_BUG_ON(index - page->index >= HPAGE_PMD_NR);
> > +   page += index - page->index;
> > +   }
> 
> Use find_subpage() here?

Ok.

> > pages[ret] = page;
> > if (++ret == nr_pages)
> > break;
> > +   if (!PageTransCompound(page))
> > +   continue;
> > +   for (refs = 0; ret < nr_pages &&
> > +   (index + 1) % HPAGE_PMD_NR;
> > +   ret++, refs++, index++)
> > +   pages[ret] = ++page;
> > +   if (refs)
> > +   page_ref_add(compound_head(page), refs);
> > +   if (ret == nr_pages)
> > +   break;
> 
> Can we avoid referencing huge pages specifically in the page cache?  I'd
> like us to get to the point where we can put arbitrary compound pages into
> the page cache.  For example, I think this can be written as:
> 
>   if (!PageCompound(page))
>   continue;
>   for (refs = 0; ret < nr_pages; refs++, index++) {
>   if (index > page->index + (1 << compound_order(page)))
>   break;
>   pages[ret++] = ++page;
>   }
>   if (refs)
>   page_ref_add(compound_head(page), refs);
>   if (ret == nr_pages)
>   break;

That's slightly more costly, but I guess that's fine.

> > @@ -1541,19 +1533,12 @@ unsigned find_get_pages_contig(struct address_space 
> > *mapping, pgoff_t index,
> >  
> > +   /* For multi-order entries, find relevant subpage */
> > +   if (PageTransHuge(page)) {
> > +   VM_BUG_ON(index - page->index < 0);
> > +   VM_BUG_ON(index - page->index >= HPAGE_PMD_NR);
> > +   page += index - page->index;
> > +   }
> > +
> > pages[ret] = page;
> > if (++ret == nr_pages)
> > break;
> > +   if (!PageTransCompound(page))
> > +   continue;
> > +   for (refs = 0; ret < nr_pages &&
> > +   (index + 1) % HPAGE_PMD_NR;
> > +   ret++, refs++, index++)
> > +   pages[ret] = ++page;
> > +   if (refs)
> > +   page_ref_add(compound_head(page), refs);
> > +   if (ret == nr_pages)
> > +   break;
> > }
> > rcu_read_unlock();
> > return ret;
> 
> Ugh, the same code again.  Hmm ... we only need to modify 'ret' as a result
> of this ... so could we split it out like this?
> 
> static unsigned populate_pages(struct page **pages, unsigned i, unsigned max,
> struct page *page)
> {
> unsigned refs = 0;
> for (;;) {
> pages[i++] = page;
>   

[PATCH RT 0/2] Linux 3.12.70-rt94-rc1

[Steven Rostedt]

Dear RT Folks,

This is the RT stable review cycle of patch 3.12.70-rt94-rc1.

Please scream at me if I messed something up. Please test the patches too.

The -rc release will be uploaded to kernel.org and will be deleted when
the final release is out. This is just a review release (or release candidate).

The pre-releases will not be pushed to the git repository, only the
final release is.

If all goes well, this patch will be converted to the next main release
on 2/13/2017.

Enjoy,

-- Steve


To build 3.12.70-rt94-rc1 directly, the following patches should be applied:

  http://www.kernel.org/pub/linux/kernel/v3.x/linux-3.12.tar.xz

  http://www.kernel.org/pub/linux/kernel/v3.x/patch-3.12.70.xz

  
http://www.kernel.org/pub/linux/kernel/projects/rt/3.12/patch-3.12.70-rt94-rc1.patch.xz

You can also build from 3.12.70-rt93 by applying the incremental patch:

http://www.kernel.org/pub/linux/kernel/projects/rt/3.12/incr/patch-3.12.70-rt93-rt94-rc1.patch.xz


Changes from 3.12.70-rt93:

---


Sebastian Andrzej Siewior (1):
  workqueue: use rcu_readlock() in put_pwq_unlocked()

Steven Rostedt (VMware) (1):
  Linux 3.12.70-rt94-rc1


 kernel/workqueue.c | 2 ++
 localversion-rt| 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

[PATCH RT 2/2] Linux 3.12.70-rt94-rc1

[Steven Rostedt]

3.12.70-rt94-rc1 stable review patch.
If anyone has any objections, please let me know.

--

From: "Steven Rostedt (VMware)" 

---
 localversion-rt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/localversion-rt b/localversion-rt
index e98a1fe050bd..dcc2fd2ca155 100644
--- a/localversion-rt
+++ b/localversion-rt
@@ -1 +1 @@
--rt93
+-rt94-rc1
-- 
2.10.2

[PATCH RT 2/2] Linux 3.12.70-rt94-rc1

[Steven Rostedt]

3.12.70-rt94-rc1 stable review patch.
If anyone has any objections, please let me know.

--

From: "Steven Rostedt (VMware)" 

---
 localversion-rt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/localversion-rt b/localversion-rt
index e98a1fe050bd..dcc2fd2ca155 100644
--- a/localversion-rt
+++ b/localversion-rt
@@ -1 +1 @@
--rt93
+-rt94-rc1
-- 
2.10.2

[PATCH RT 0/2] Linux 3.12.70-rt94-rc1

[Steven Rostedt]

Dear RT Folks,

This is the RT stable review cycle of patch 3.12.70-rt94-rc1.

Please scream at me if I messed something up. Please test the patches too.

The -rc release will be uploaded to kernel.org and will be deleted when
the final release is out. This is just a review release (or release candidate).

The pre-releases will not be pushed to the git repository, only the
final release is.

If all goes well, this patch will be converted to the next main release
on 2/13/2017.

Enjoy,

-- Steve


To build 3.12.70-rt94-rc1 directly, the following patches should be applied:

  http://www.kernel.org/pub/linux/kernel/v3.x/linux-3.12.tar.xz

  http://www.kernel.org/pub/linux/kernel/v3.x/patch-3.12.70.xz

  
http://www.kernel.org/pub/linux/kernel/projects/rt/3.12/patch-3.12.70-rt94-rc1.patch.xz

You can also build from 3.12.70-rt93 by applying the incremental patch:

http://www.kernel.org/pub/linux/kernel/projects/rt/3.12/incr/patch-3.12.70-rt93-rt94-rc1.patch.xz


Changes from 3.12.70-rt93:

---


Sebastian Andrzej Siewior (1):
  workqueue: use rcu_readlock() in put_pwq_unlocked()

Steven Rostedt (VMware) (1):
  Linux 3.12.70-rt94-rc1


 kernel/workqueue.c | 2 ++
 localversion-rt| 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

Re: [PATCH] netlink: move nla_put_{u8,u16,u32} out of line

[Dmitry Vyukov]

On Thu, Feb 9, 2017 at 3:33 PM, Arnd Bergmann  wrote:
>>>
>>> Thanks for the list, that looks very helpful. The ones I found myself
>>> seem to be a strict (and small) subset of those, using gcc-7.0.1 on x86-64
>>> with allmodconfig and a few hundred randconfig builds. Which compiler
>>> version did you use for your testing? If new versions are better than old
>>> ones, we could start by fixing the ones that are still present in gcc-6 and
>>> gcc-7, and making the warning conditionally on the compiler version.
>>>
>>> Another idea might be to separate out asan_stack=1 into a separate
>>> Kconfig option and warn if that is enabled with compilers that are known
>>> to be relatively bad it keeping the stack small.
>>
>>
>> Mine is gcc version 7.0.0 20161208. Make sure you enable KASAN_INLINE
>> and I also enabled CONFIG_KCOV. Other than that I just did
>> allyesconfig + make -k.
>
> Ok, I see my mistake now: On the allmodconfig build, I had KASAN_INLINE
> disabled, which made the problem go away for almost all files (almost all
> frame sizes are below 2048 bytes, except for the two issues I posted patches
> for (hisilicon ethernet driver, and nla_put_* users).
>
> On the randconfig build test, I have a long series of patches applied that
> address all known warnings, including my earlier "kasan: turn off
> -fsanitize-address-use-after-scope for now" (see
> https://patchwork.kernel.org/patch/9442323/). Without
> -fsanitize-address-use-after-scope, the problem is also mostly gone
> (a few cases show up in drivers/media/, and also in block/sed-opal.c
> and drivers/tty/vt/keyboard.c). I now have initial patches for all of
> them to bring the stack size below 2048 bytes.
>
> As far as I can see, the remaining problems with scary stack frame sizes
> you found only show up with the combination of all three: KASAN_INLINE,
> asan_stack=1 and -fsanitize-address-use-after-scope. If all three were
> separately configurable and we merge the patches I made, I think we could
> enable the normal 2048 byte warning in all configurations that have at least
> one of the turned off, but I don't know which of those combinations would
> actually be sensible for production kernels.

FWIW I use all of them (+KCOV which is additional instrumentation).
If you ask which is the least important one, it is
-fsanitize-address-use-after-scope. I have not see any single report
due to it (probably due to kernel coding style).

Re: [PATCH] netlink: move nla_put_{u8,u16,u32} out of line

[Dmitry Vyukov]

On Thu, Feb 9, 2017 at 3:33 PM, Arnd Bergmann  wrote:
>>>
>>> Thanks for the list, that looks very helpful. The ones I found myself
>>> seem to be a strict (and small) subset of those, using gcc-7.0.1 on x86-64
>>> with allmodconfig and a few hundred randconfig builds. Which compiler
>>> version did you use for your testing? If new versions are better than old
>>> ones, we could start by fixing the ones that are still present in gcc-6 and
>>> gcc-7, and making the warning conditionally on the compiler version.
>>>
>>> Another idea might be to separate out asan_stack=1 into a separate
>>> Kconfig option and warn if that is enabled with compilers that are known
>>> to be relatively bad it keeping the stack small.
>>
>>
>> Mine is gcc version 7.0.0 20161208. Make sure you enable KASAN_INLINE
>> and I also enabled CONFIG_KCOV. Other than that I just did
>> allyesconfig + make -k.
>
> Ok, I see my mistake now: On the allmodconfig build, I had KASAN_INLINE
> disabled, which made the problem go away for almost all files (almost all
> frame sizes are below 2048 bytes, except for the two issues I posted patches
> for (hisilicon ethernet driver, and nla_put_* users).
>
> On the randconfig build test, I have a long series of patches applied that
> address all known warnings, including my earlier "kasan: turn off
> -fsanitize-address-use-after-scope for now" (see
> https://patchwork.kernel.org/patch/9442323/). Without
> -fsanitize-address-use-after-scope, the problem is also mostly gone
> (a few cases show up in drivers/media/, and also in block/sed-opal.c
> and drivers/tty/vt/keyboard.c). I now have initial patches for all of
> them to bring the stack size below 2048 bytes.
>
> As far as I can see, the remaining problems with scary stack frame sizes
> you found only show up with the combination of all three: KASAN_INLINE,
> asan_stack=1 and -fsanitize-address-use-after-scope. If all three were
> separately configurable and we merge the patches I made, I think we could
> enable the normal 2048 byte warning in all configurations that have at least
> one of the turned off, but I don't know which of those combinations would
> actually be sensible for production kernels.

FWIW I use all of them (+KCOV which is additional instrumentation).
If you ask which is the least important one, it is
-fsanitize-address-use-after-scope. I have not see any single report
due to it (probably due to kernel coding style).

[PATCH next 1/2] crypto: atmel-sha: fix missing "return" instructions

[Cyrille Pitchen]

This patch fixes a previous patch: "crypto: atmel-sha - update request
queue management to make it more generic".

Indeed the patch above should have replaced the "return -EINVAL;" lines by
"return atmel_sha_complete(dd, -EINVAL);" but instead replaced them by a
simple call of "atmel_sha_complete(dd, -EINVAL);".
Hence all "return" instructions were missing.

Reported-by: Dan Carpenter 
Signed-off-by: Cyrille Pitchen 
---
 drivers/crypto/atmel-sha.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 22d0c0c118da..d6c3d9529d36 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -668,7 +668,7 @@ static int atmel_sha_xmit_dma(struct atmel_sha_dev *dd, 
dma_addr_t dma_addr1,
DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
}
if (!in_desc)
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
 
in_desc->callback = atmel_sha_dma_callback;
in_desc->callback_param = dd;
@@ -725,7 +725,7 @@ static int atmel_sha_xmit_dma_map(struct atmel_sha_dev *dd,
if (dma_mapping_error(dd->dev, ctx->dma_addr)) {
dev_err(dd->dev, "dma %u bytes error\n", ctx->buflen +
ctx->block_size);
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
ctx->flags &= ~SHA_FLAGS_SG;
@@ -816,7 +816,7 @@ static int atmel_sha_update_dma_start(struct atmel_sha_dev 
*dd)
if (dma_mapping_error(dd->dev, ctx->dma_addr)) {
dev_err(dd->dev, "dma %u bytes error\n",
ctx->buflen + ctx->block_size);
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
if (length == 0) {
@@ -830,7 +830,7 @@ static int atmel_sha_update_dma_start(struct atmel_sha_dev 
*dd)
if (!dma_map_sg(dd->dev, ctx->sg, 1,
DMA_TO_DEVICE)) {
dev_err(dd->dev, "dma_map_sg  error\n");
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
ctx->flags |= SHA_FLAGS_SG;
@@ -844,7 +844,7 @@ static int atmel_sha_update_dma_start(struct atmel_sha_dev 
*dd)
 
if (!dma_map_sg(dd->dev, ctx->sg, 1, DMA_TO_DEVICE)) {
dev_err(dd->dev, "dma_map_sg  error\n");
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
ctx->flags |= SHA_FLAGS_SG;
-- 
2.7.4

[PATCH next 1/2] crypto: atmel-sha: fix missing "return" instructions

[Cyrille Pitchen]

This patch fixes a previous patch: "crypto: atmel-sha - update request
queue management to make it more generic".

Indeed the patch above should have replaced the "return -EINVAL;" lines by
"return atmel_sha_complete(dd, -EINVAL);" but instead replaced them by a
simple call of "atmel_sha_complete(dd, -EINVAL);".
Hence all "return" instructions were missing.

Reported-by: Dan Carpenter 
Signed-off-by: Cyrille Pitchen 
---
 drivers/crypto/atmel-sha.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index 22d0c0c118da..d6c3d9529d36 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -668,7 +668,7 @@ static int atmel_sha_xmit_dma(struct atmel_sha_dev *dd, 
dma_addr_t dma_addr1,
DMA_MEM_TO_DEV, DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
}
if (!in_desc)
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
 
in_desc->callback = atmel_sha_dma_callback;
in_desc->callback_param = dd;
@@ -725,7 +725,7 @@ static int atmel_sha_xmit_dma_map(struct atmel_sha_dev *dd,
if (dma_mapping_error(dd->dev, ctx->dma_addr)) {
dev_err(dd->dev, "dma %u bytes error\n", ctx->buflen +
ctx->block_size);
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
ctx->flags &= ~SHA_FLAGS_SG;
@@ -816,7 +816,7 @@ static int atmel_sha_update_dma_start(struct atmel_sha_dev 
*dd)
if (dma_mapping_error(dd->dev, ctx->dma_addr)) {
dev_err(dd->dev, "dma %u bytes error\n",
ctx->buflen + ctx->block_size);
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
if (length == 0) {
@@ -830,7 +830,7 @@ static int atmel_sha_update_dma_start(struct atmel_sha_dev 
*dd)
if (!dma_map_sg(dd->dev, ctx->sg, 1,
DMA_TO_DEVICE)) {
dev_err(dd->dev, "dma_map_sg  error\n");
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
ctx->flags |= SHA_FLAGS_SG;
@@ -844,7 +844,7 @@ static int atmel_sha_update_dma_start(struct atmel_sha_dev 
*dd)
 
if (!dma_map_sg(dd->dev, ctx->sg, 1, DMA_TO_DEVICE)) {
dev_err(dd->dev, "dma_map_sg  error\n");
-   atmel_sha_complete(dd, -EINVAL);
+   return atmel_sha_complete(dd, -EINVAL);
}
 
ctx->flags |= SHA_FLAGS_SG;
-- 
2.7.4

[PATCH next 0/2] crypto: atmel-sha: fix error management

[Cyrille Pitchen]

Hi all,

this series is based on next-20170209.

The first patch fixes a bug reported by Dan Carpenter. I didn't put a
Fixes tag since the buggy patch is only in linux-next for now so its
commit ID is likely to change when entering Linus' tree.
It fixes a wrong 'sed' command: many "return -EINVAL;" lines should have
been replaced by "return atmel_sha_complete(dd, -EINVAL);" but instead
were replaced by direct calls of "atmel_sha_complete(dd, -EINVAL);".
My bad, sorry for that!

The second patch fixes the way error cases are handled from
atmel_sha_start(). For instance, when atmel_sha_update_req() returned an
error, atmel_sha_final_req() may have been called after anyway. This issue
was present even before my rework of the request queue management, which
introduced atmel_sha_start(), so I guess this is a long time issue.

Finally, for driver maintainance purpose, I'm preparing other patches to
fix the very same and very unlikely issue in both atmel-aes.c and
atmel-sha.c:

atmel_{aes|sha}_hw_init() may fail, for instance if clk_enable() fails.
If so, atmel_{aes|sha}_complete() is called to release the hardware and
report the error. Indeed this _complete() function should be called to
report and handle any error. However it also incondionnally calls
clk_disable(). Hence the following sequence may be buggy:

err = atmel_{aes|sha}_hw_init(dd); /* clk_enable() may have failed. */
if (err)
return atmel_{aes|sha}_hw_init(dd, err);
/* clk_disable() is called anyway. */

I didn't finalize my fixes yet for this unlikely bug. Besides the bug was
already present in v4.9 and before, so before introducing
atmel_sha_complete().

from atmel_sha_handle_queue(), the older sequence was:

err = atmel_sha_hw_init(dd);

if (err)
goto err1;

[...]

err1:
if (err != -EINPROGRESS)
/* done_task will not finish it, so do it here */
atmel_sha_finish_req(req, err);

Best regards,

Cyrille

Cyrille Pitchen (2):
  crypto: atmel-sha: fix missing "return" instructions
  crypto: atmel-sha: fix error management in atmel_sha_start()

 drivers/crypto/atmel-sha.c | 37 +++--
 1 file changed, 27 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH next 0/2] crypto: atmel-sha: fix error management

[Cyrille Pitchen]

Hi all,

this series is based on next-20170209.

The first patch fixes a bug reported by Dan Carpenter. I didn't put a
Fixes tag since the buggy patch is only in linux-next for now so its
commit ID is likely to change when entering Linus' tree.
It fixes a wrong 'sed' command: many "return -EINVAL;" lines should have
been replaced by "return atmel_sha_complete(dd, -EINVAL);" but instead
were replaced by direct calls of "atmel_sha_complete(dd, -EINVAL);".
My bad, sorry for that!

The second patch fixes the way error cases are handled from
atmel_sha_start(). For instance, when atmel_sha_update_req() returned an
error, atmel_sha_final_req() may have been called after anyway. This issue
was present even before my rework of the request queue management, which
introduced atmel_sha_start(), so I guess this is a long time issue.

Finally, for driver maintainance purpose, I'm preparing other patches to
fix the very same and very unlikely issue in both atmel-aes.c and
atmel-sha.c:

atmel_{aes|sha}_hw_init() may fail, for instance if clk_enable() fails.
If so, atmel_{aes|sha}_complete() is called to release the hardware and
report the error. Indeed this _complete() function should be called to
report and handle any error. However it also incondionnally calls
clk_disable(). Hence the following sequence may be buggy:

err = atmel_{aes|sha}_hw_init(dd); /* clk_enable() may have failed. */
if (err)
return atmel_{aes|sha}_hw_init(dd, err);
/* clk_disable() is called anyway. */

I didn't finalize my fixes yet for this unlikely bug. Besides the bug was
already present in v4.9 and before, so before introducing
atmel_sha_complete().

from atmel_sha_handle_queue(), the older sequence was:

err = atmel_sha_hw_init(dd);

if (err)
goto err1;

[...]

err1:
if (err != -EINPROGRESS)
/* done_task will not finish it, so do it here */
atmel_sha_finish_req(req, err);

Best regards,

Cyrille

Cyrille Pitchen (2):
  crypto: atmel-sha: fix missing "return" instructions
  crypto: atmel-sha: fix error management in atmel_sha_start()

 drivers/crypto/atmel-sha.c | 37 +++--
 1 file changed, 27 insertions(+), 10 deletions(-)

-- 
2.7.4

[PATCH next 2/2] crypto: atmel-sha: fix error management in atmel_sha_start()

[Cyrille Pitchen]

This patch clarifies and fixes how errors should be handled by
atmel_sha_start().

For update operations, the previous code wrongly assumed that
(err != -EINPROGRESS) implies (err == 0). It's wrong because that doesn't
take the error cases (err < 0) into account.

This patch also adds many comments to detail all the possible returned
values and what should be done in each case.

Especially, when an error occurs, since atmel_sha_complete() has already
been called, hence releasing the hardware, atmel_sha_start() must not call
atmel_sha_finish_req() later otherwise atmel_sha_complete() would be
called a second time.

Signed-off-by: Cyrille Pitchen 
---
 drivers/crypto/atmel-sha.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index d6c3d9529d36..0d207dac9aa2 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -1106,22 +1106,39 @@ static int atmel_sha_start(struct atmel_sha_dev *dd)
ctx->op, req->nbytes);
 
err = atmel_sha_hw_init(dd);
-
if (err)
-   goto err1;
+   return atmel_sha_complete(dd, err);
+
+   /*
+* atmel_sha_update_req() and atmel_sha_final_req() can return either:
+*  -EINPROGRESS: the hardware is busy and the SHA driver will resume
+*its job later in the done_task.
+*This is the main path.
+*
+* 0: the SHA driver can continue its job then release the hardware
+*later, if needed, with atmel_sha_finish_req().
+*This is the alternate path.
+*
+* < 0: an error has occurred so atmel_sha_complete(dd, err) has already
+*  been called, hence the hardware has been released.
+*  The SHA driver must stop its job without calling
+*  atmel_sha_finish_req(), otherwise atmel_sha_complete() would be
+*  called a second time.
+*
+* Please note that currently, atmel_sha_final_req() never returns 0.
+*/
 
dd->resume = atmel_sha_done;
if (ctx->op == SHA_OP_UPDATE) {
err = atmel_sha_update_req(dd);
-   if (err != -EINPROGRESS && (ctx->flags & SHA_FLAGS_FINUP))
+   if (!err && (ctx->flags & SHA_FLAGS_FINUP))
/* no final() after finup() */
err = atmel_sha_final_req(dd);
} else if (ctx->op == SHA_OP_FINAL) {
err = atmel_sha_final_req(dd);
}
 
-err1:
-   if (err != -EINPROGRESS)
+   if (!err)
/* done_task will not finish it, so do it here */
atmel_sha_finish_req(req, err);
 
-- 
2.7.4

[PATCH next 2/2] crypto: atmel-sha: fix error management in atmel_sha_start()

[Cyrille Pitchen]

This patch clarifies and fixes how errors should be handled by
atmel_sha_start().

For update operations, the previous code wrongly assumed that
(err != -EINPROGRESS) implies (err == 0). It's wrong because that doesn't
take the error cases (err < 0) into account.

This patch also adds many comments to detail all the possible returned
values and what should be done in each case.

Especially, when an error occurs, since atmel_sha_complete() has already
been called, hence releasing the hardware, atmel_sha_start() must not call
atmel_sha_finish_req() later otherwise atmel_sha_complete() would be
called a second time.

Signed-off-by: Cyrille Pitchen 
---
 drivers/crypto/atmel-sha.c | 27 ++-
 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/atmel-sha.c b/drivers/crypto/atmel-sha.c
index d6c3d9529d36..0d207dac9aa2 100644
--- a/drivers/crypto/atmel-sha.c
+++ b/drivers/crypto/atmel-sha.c
@@ -1106,22 +1106,39 @@ static int atmel_sha_start(struct atmel_sha_dev *dd)
ctx->op, req->nbytes);
 
err = atmel_sha_hw_init(dd);
-
if (err)
-   goto err1;
+   return atmel_sha_complete(dd, err);
+
+   /*
+* atmel_sha_update_req() and atmel_sha_final_req() can return either:
+*  -EINPROGRESS: the hardware is busy and the SHA driver will resume
+*its job later in the done_task.
+*This is the main path.
+*
+* 0: the SHA driver can continue its job then release the hardware
+*later, if needed, with atmel_sha_finish_req().
+*This is the alternate path.
+*
+* < 0: an error has occurred so atmel_sha_complete(dd, err) has already
+*  been called, hence the hardware has been released.
+*  The SHA driver must stop its job without calling
+*  atmel_sha_finish_req(), otherwise atmel_sha_complete() would be
+*  called a second time.
+*
+* Please note that currently, atmel_sha_final_req() never returns 0.
+*/
 
dd->resume = atmel_sha_done;
if (ctx->op == SHA_OP_UPDATE) {
err = atmel_sha_update_req(dd);
-   if (err != -EINPROGRESS && (ctx->flags & SHA_FLAGS_FINUP))
+   if (!err && (ctx->flags & SHA_FLAGS_FINUP))
/* no final() after finup() */
err = atmel_sha_final_req(dd);
} else if (ctx->op == SHA_OP_FINAL) {
err = atmel_sha_final_req(dd);
}
 
-err1:
-   if (err != -EINPROGRESS)
+   if (!err)
/* done_task will not finish it, so do it here */
atmel_sha_finish_req(req, err);
 
-- 
2.7.4

Intel PT decoder switch case fallthrough cases reported by gcc 7

[Arnaldo Carvalho de Melo]

Hi,

I've updated the container with Fedora Rawhide I use to build
tools/perf/ and samples/bcc/ and it now comes with gcc 7, where I get
things like:

  CC   /tmp/build/perf/tests/code-reading.o
util/intel-pt-decoder/intel-pt-decoder.c: In function 'intel_pt_walk_psb':
util/intel-pt-decoder/intel-pt-decoder.c:1748:31: error: this statement may 
fall through [-Werror=implicit-fallthrough=]
decoder->continuous_period = false;
   ^
util/intel-pt-decoder/intel-pt-decoder.c:1749:3: note: here
   case INTEL_PT_TIP_PGE:
   ^~~~
util/intel-pt-decoder/intel-pt-decoder.c:1801:4: error: this statement may fall 
through [-Werror=implicit-fallthrough=]
intel_pt_clear_tx_flags(decoder);
^~~~
util/intel-pt-decoder/intel-pt-decoder.c:1802:3: note: here
   case INTEL_PT_TNT:
   ^~~~
util/intel-pt-decoder/intel-pt-decoder.c: In function 'intel_pt_walk_to_ip':
util/intel-pt-decoder/intel-pt-decoder.c:1841:31: error: this statement may 
fall through [-Werror=implicit-fallthrough=]
decoder->continuous_period = false;
   ^
util/intel-pt-decoder/intel-pt-decoder.c:1842:3: note: here
   case INTEL_PT_TIP_PGE:
   ^~~~
  MKDIR/tmp/build/perf/util/scripting-engines/


This gets solved with a new attribute, that you have to add where in the past
we added:

/* Fall through */

To indicate that the fall through to the next case statement block is
intentional, so now I have a __fallthrough and I am addressing all the cases,
please check if the ones below are the ones intended for the Intel PT parts,
please Ack or advise, only one seemed like a bug, i.e. a break should be used,
but I'm not sure.

- Arnaldo


diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c 
b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index e4e7dc781d21..d4ed327a4908 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -1746,6 +1746,7 @@ static int intel_pt_walk_psb(struct intel_pt_decoder 
*decoder)
switch (decoder->packet.type) {
case INTEL_PT_TIP_PGD:
decoder->continuous_period = false;
+   __fallthrough;
case INTEL_PT_TIP_PGE:
case INTEL_PT_TIP:
intel_pt_log("ERROR: Unexpected packet\n");
@@ -1799,6 +1800,8 @@ static int intel_pt_walk_psb(struct intel_pt_decoder 
*decoder)
decoder->pge = false;
decoder->continuous_period = false;
intel_pt_clear_tx_flags(decoder);
+   break;
+
case INTEL_PT_TNT:
decoder->have_tma = false;
intel_pt_log("ERROR: Unexpected packet\n");
@@ -1839,6 +1842,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder 
*decoder)
switch (decoder->packet.type) {
case INTEL_PT_TIP_PGD:
decoder->continuous_period = false;
+   __fallthrough;
case INTEL_PT_TIP_PGE:
case INTEL_PT_TIP:
decoder->pge = decoder->packet.type != INTEL_PT_TIP_PGD;
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c 
b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
index 4f7b32020487..7528ae4f7e28 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "intel-pt-pkt-decoder.h"
 
@@ -498,6 +499,7 @@ int intel_pt_pkt_desc(const struct intel_pt_pkt *packet, 
char *buf,
case INTEL_PT_FUP:
if (!(packet->count))
return snprintf(buf, buf_len, "%s no ip", name);
+   __fallthrough;
case INTEL_PT_CYC:
case INTEL_PT_VMCS:
case INTEL_PT_MTC:

Intel PT decoder switch case fallthrough cases reported by gcc 7

[Arnaldo Carvalho de Melo]

Hi,

I've updated the container with Fedora Rawhide I use to build
tools/perf/ and samples/bcc/ and it now comes with gcc 7, where I get
things like:

  CC   /tmp/build/perf/tests/code-reading.o
util/intel-pt-decoder/intel-pt-decoder.c: In function 'intel_pt_walk_psb':
util/intel-pt-decoder/intel-pt-decoder.c:1748:31: error: this statement may 
fall through [-Werror=implicit-fallthrough=]
decoder->continuous_period = false;
   ^
util/intel-pt-decoder/intel-pt-decoder.c:1749:3: note: here
   case INTEL_PT_TIP_PGE:
   ^~~~
util/intel-pt-decoder/intel-pt-decoder.c:1801:4: error: this statement may fall 
through [-Werror=implicit-fallthrough=]
intel_pt_clear_tx_flags(decoder);
^~~~
util/intel-pt-decoder/intel-pt-decoder.c:1802:3: note: here
   case INTEL_PT_TNT:
   ^~~~
util/intel-pt-decoder/intel-pt-decoder.c: In function 'intel_pt_walk_to_ip':
util/intel-pt-decoder/intel-pt-decoder.c:1841:31: error: this statement may 
fall through [-Werror=implicit-fallthrough=]
decoder->continuous_period = false;
   ^
util/intel-pt-decoder/intel-pt-decoder.c:1842:3: note: here
   case INTEL_PT_TIP_PGE:
   ^~~~
  MKDIR/tmp/build/perf/util/scripting-engines/


This gets solved with a new attribute, that you have to add where in the past
we added:

/* Fall through */

To indicate that the fall through to the next case statement block is
intentional, so now I have a __fallthrough and I am addressing all the cases,
please check if the ones below are the ones intended for the Intel PT parts,
please Ack or advise, only one seemed like a bug, i.e. a break should be used,
but I'm not sure.

- Arnaldo


diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c 
b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
index e4e7dc781d21..d4ed327a4908 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-decoder.c
@@ -1746,6 +1746,7 @@ static int intel_pt_walk_psb(struct intel_pt_decoder 
*decoder)
switch (decoder->packet.type) {
case INTEL_PT_TIP_PGD:
decoder->continuous_period = false;
+   __fallthrough;
case INTEL_PT_TIP_PGE:
case INTEL_PT_TIP:
intel_pt_log("ERROR: Unexpected packet\n");
@@ -1799,6 +1800,8 @@ static int intel_pt_walk_psb(struct intel_pt_decoder 
*decoder)
decoder->pge = false;
decoder->continuous_period = false;
intel_pt_clear_tx_flags(decoder);
+   break;
+
case INTEL_PT_TNT:
decoder->have_tma = false;
intel_pt_log("ERROR: Unexpected packet\n");
@@ -1839,6 +1842,7 @@ static int intel_pt_walk_to_ip(struct intel_pt_decoder 
*decoder)
switch (decoder->packet.type) {
case INTEL_PT_TIP_PGD:
decoder->continuous_period = false;
+   __fallthrough;
case INTEL_PT_TIP_PGE:
case INTEL_PT_TIP:
decoder->pge = decoder->packet.type != INTEL_PT_TIP_PGD;
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c 
b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
index 4f7b32020487..7528ae4f7e28 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-pkt-decoder.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "intel-pt-pkt-decoder.h"
 
@@ -498,6 +499,7 @@ int intel_pt_pkt_desc(const struct intel_pt_pkt *packet, 
char *buf,
case INTEL_PT_FUP:
if (!(packet->count))
return snprintf(buf, buf_len, "%s no ip", name);
+   __fallthrough;
case INTEL_PT_CYC:
case INTEL_PT_VMCS:
case INTEL_PT_MTC: