Re: [PATCH v1 1/4] dt-bindings: gpu: mali-utgard: add mediatek,mt7623-mali compatible

[Rob Herring]

On Wed, Apr 18, 2018 at 06:24:53PM +0800, sean.w...@mediatek.com wrote:
> From: Sean Wang 
> 
> The MediaTek MT7623 SoC contains a Mali-450, so add a compatible for it
> and define its own vendor-specific properties.
> 
> Signed-off-by: Sean Wang 
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 9 +
>  1 file changed, 9 insertions(+)

Reviewed-by: Rob Herring 

Re: [PATCH v1 1/4] dt-bindings: gpu: mali-utgard: add mediatek,mt7623-mali compatible

[Rob Herring]

On Wed, Apr 18, 2018 at 06:24:53PM +0800, sean.w...@mediatek.com wrote:
> From: Sean Wang 
> 
> The MediaTek MT7623 SoC contains a Mali-450, so add a compatible for it
> and define its own vendor-specific properties.
> 
> Signed-off-by: Sean Wang 
> ---
>  Documentation/devicetree/bindings/gpu/arm,mali-utgard.txt | 9 +
>  1 file changed, 9 insertions(+)

Reviewed-by: Rob Herring 

[PATCH 2/7] genirq/debugfs: Use irq_print_chip method if available

[Marc Zyngier]

If an irqchip has provided an irq_print_chip method, let's use
it instead of the name field directly.

Signed-off-by: Marc Zyngier 
---
 kernel/irq/debugfs.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index da41150322e1..50e324110271 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -69,7 +69,13 @@ irq_debug_show_chip(struct seq_file *m, struct irq_data 
*data, int ind)
seq_printf(m, "chip: None\n");
return;
}
-   seq_printf(m, "%*schip:%s\n", ind, "", chip->name);
+   seq_printf(m, "%*schip:", ind, "");
+   if (chip->irq_print_chip) {
+   chip->irq_print_chip(data, m);
+   seq_printf(m, "\n");
+   } else {
+   seq_printf(m, "%s\n", chip->name);
+   }
seq_printf(m, "%*sflags:   0x%lx\n", ind + 1, "", chip->flags);
irq_debug_show_bits(m, ind, chip->flags, irqchip_flags,
ARRAY_SIZE(irqchip_flags));
-- 
2.14.2

[PATCH 2/7] genirq/debugfs: Use irq_print_chip method if available

[Marc Zyngier]

If an irqchip has provided an irq_print_chip method, let's use
it instead of the name field directly.

Signed-off-by: Marc Zyngier 
---
 kernel/irq/debugfs.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/debugfs.c b/kernel/irq/debugfs.c
index da41150322e1..50e324110271 100644
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -69,7 +69,13 @@ irq_debug_show_chip(struct seq_file *m, struct irq_data 
*data, int ind)
seq_printf(m, "chip: None\n");
return;
}
-   seq_printf(m, "%*schip:%s\n", ind, "", chip->name);
+   seq_printf(m, "%*schip:", ind, "");
+   if (chip->irq_print_chip) {
+   chip->irq_print_chip(data, m);
+   seq_printf(m, "\n");
+   } else {
+   seq_printf(m, "%s\n", chip->name);
+   }
seq_printf(m, "%*sflags:   0x%lx\n", ind + 1, "", chip->flags);
irq_debug_show_bits(m, ind, chip->flags, irqchip_flags,
ARRAY_SIZE(irqchip_flags));
-- 
2.14.2

[PATCH 3/7] genirq/debugfs: Allow irq domain name to be overriden

[Marc Zyngier]

As for irqchip, it would be useful for a domain to generate its
name dynamically, and thus control the name that gets used in
debugfs.

Signed-off-by: Marc Zyngier 
---
 include/linux/irqdomain.h |  1 +
 kernel/irq/irqdomain.c| 14 ++
 2 files changed, 15 insertions(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 48c7e86bb556..a912b1cc1715 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -119,6 +119,7 @@ struct irq_domain_ops {
 unsigned long *out_hwirq, unsigned int *out_type);
 #endif
 #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+   char *(*override_name)(struct irq_domain *d);
void (*debug_show)(struct seq_file *m, struct irq_domain *d,
   struct irq_data *irqd, int ind);
 #endif
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 5d9fc01b60a6..521065c8bace 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -220,6 +220,20 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle 
*fwnode, int size,
domain->revmap_direct_max_irq = direct_max;
irq_domain_check_hierarchy(domain);
 
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+   if (domain->ops->override_name) {
+   char *new_name;
+
+   new_name = domain->ops->override_name(domain);
+   if (new_name) {
+   if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED)
+   kfree(domain->name);
+   domain->name = new_name;
+   domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
+   }
+   }
+#endif
+
mutex_lock(_domain_mutex);
debugfs_add_domain_dir(domain);
list_add(>link, _domain_list);
-- 
2.14.2

[PATCH 3/7] genirq/debugfs: Allow irq domain name to be overriden

[Marc Zyngier]

As for irqchip, it would be useful for a domain to generate its
name dynamically, and thus control the name that gets used in
debugfs.

Signed-off-by: Marc Zyngier 
---
 include/linux/irqdomain.h |  1 +
 kernel/irq/irqdomain.c| 14 ++
 2 files changed, 15 insertions(+)

diff --git a/include/linux/irqdomain.h b/include/linux/irqdomain.h
index 48c7e86bb556..a912b1cc1715 100644
--- a/include/linux/irqdomain.h
+++ b/include/linux/irqdomain.h
@@ -119,6 +119,7 @@ struct irq_domain_ops {
 unsigned long *out_hwirq, unsigned int *out_type);
 #endif
 #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+   char *(*override_name)(struct irq_domain *d);
void (*debug_show)(struct seq_file *m, struct irq_domain *d,
   struct irq_data *irqd, int ind);
 #endif
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 5d9fc01b60a6..521065c8bace 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -220,6 +220,20 @@ struct irq_domain *__irq_domain_add(struct fwnode_handle 
*fwnode, int size,
domain->revmap_direct_max_irq = direct_max;
irq_domain_check_hierarchy(domain);
 
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+   if (domain->ops->override_name) {
+   char *new_name;
+
+   new_name = domain->ops->override_name(domain);
+   if (new_name) {
+   if (domain->flags & IRQ_DOMAIN_NAME_ALLOCATED)
+   kfree(domain->name);
+   domain->name = new_name;
+   domain->flags |= IRQ_DOMAIN_NAME_ALLOCATED;
+   }
+   }
+#endif
+
mutex_lock(_domain_mutex);
debugfs_add_domain_dir(domain);
list_add(>link, _domain_list);
-- 
2.14.2

[PATCH 7/7] irqchip/partition-percpu: Move allocation of chained interrupt to activate

[Marc Zyngier]

The partition-percpu helpers work by tying the real interrupt to an
affinity-driven chained irqchip. So far, all the interrupt that can be
partitionned are being created at boot time.

This is a waste of resource, and means that we need to bypass some of
the critical checks (such as the trigger configuration).

A much better approach would be to create the backing interrupt when
one of the muxed interrupts gets activated, making sure we do things
on demand, and expose the right trigger information to the rest of
the kernel.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3.c | 31 +++
 drivers/irqchip/irq-partition-percpu.c   | 45 +---
 include/linux/irqchip/irq-partition-percpu.h |  4 +--
 3 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e5d101418390..f4c6d2223191 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -922,8 +922,6 @@ static int gic_irq_domain_map(struct irq_domain *d, 
unsigned int irq,
return 0;
 }
 
-#define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1)
-
 static int gic_irq_domain_translate(struct irq_domain *d,
struct irq_fwspec *fwspec,
unsigned long *hwirq,
@@ -938,7 +936,6 @@ static int gic_irq_domain_translate(struct irq_domain *d,
*hwirq = fwspec->param[1] + 32;
break;
case 1: /* PPI */
-   case GIC_IRQ_TYPE_PARTITION:
*hwirq = fwspec->param[1] + 16;
break;
case GIC_IRQ_TYPE_LPI:  /* LPI */
@@ -952,10 +949,8 @@ static int gic_irq_domain_translate(struct irq_domain *d,
 
/*
 * Make it clear that broken DTs are... broken.
-* Partitionned PPIs are an unfortunate exception.
 */
-   WARN_ON(*type == IRQ_TYPE_NONE &&
-   fwspec->param[0] != GIC_IRQ_TYPE_PARTITION);
+   WARN_ON(*type == IRQ_TYPE_NONE);
return 0;
}
 
@@ -1143,6 +1138,12 @@ static int __init gic_validate_dist_version(void __iomem 
*dist_base)
return 0;
 }
 
+static void partition_fwspec_convert(struct irq_fwspec *fwspec)
+{
+   BUG_ON(fwspec->param_count != 4);
+   fwspec->param_count = 3;
+}
+
 /* Create all possible partitions at boot time */
 static void __init gic_populate_ppi_partitions(struct device_node *gic_node)
 {
@@ -1207,23 +1208,11 @@ static void __init gic_populate_ppi_partitions(struct 
device_node *gic_node)
}
 
for (i = 0; i < 16; i++) {
-   unsigned int irq;
struct partition_desc *desc;
-   struct irq_fwspec ppi_fwspec = {
-   .fwnode = gic_data.fwnode,
-   .param_count= 3,
-   .param  = {
-   [0] = GIC_IRQ_TYPE_PARTITION,
-   [1] = i,
-   [2] = IRQ_TYPE_NONE,
-   },
-   };
-
-   irq = irq_create_fwspec_mapping(_fwspec);
-   if (WARN_ON(!irq))
-   continue;
+
desc = partition_create_desc(gic_data.fwnode, parts, nr_parts,
-irq, _domain_ops);
+partition_fwspec_convert,
+_domain_ops);
if (WARN_ON(!desc))
continue;
 
diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index 79a1ef0c0f73..009ce5c97b05 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -25,12 +25,15 @@
 #include 
 
 struct partition_desc {
+   struct mutexlock;
int nr_parts;
struct partition_affinity   *parts;
struct irq_domain   *domain;
struct irq_desc *chained_desc;
unsigned long   *bitmap;
struct irq_domain_ops   ops;
+   struct irq_fwspec   fwspec;
+   void (*convert)(struct irq_fwspec *fwspec);
 };
 
 static struct irq_data *partition_get_irqd_chip(struct partition_desc *part,
@@ -166,9 +169,14 @@ static int partition_domain_alloc(struct irq_domain 
*domain, unsigned int virq,
 
part = domain->host_data;
 
+   mutex_lock(>lock);
+   if (!part->fwspec.param_count) {
+   part->fwspec = *fwspec;
+   part->convert(>fwspec);
+   }
+   mutex_unlock(>lock);
+
set_bit(hwirq, part->bitmap);
-   irq_set_chained_handler_and_data(irq_desc_get_irq(part->chained_desc),

[PATCH 7/7] irqchip/partition-percpu: Move allocation of chained interrupt to activate

[Marc Zyngier]

The partition-percpu helpers work by tying the real interrupt to an
affinity-driven chained irqchip. So far, all the interrupt that can be
partitionned are being created at boot time.

This is a waste of resource, and means that we need to bypass some of
the critical checks (such as the trigger configuration).

A much better approach would be to create the backing interrupt when
one of the muxed interrupts gets activated, making sure we do things
on demand, and expose the right trigger information to the rest of
the kernel.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-gic-v3.c | 31 +++
 drivers/irqchip/irq-partition-percpu.c   | 45 +---
 include/linux/irqchip/irq-partition-percpu.h |  4 +--
 3 files changed, 53 insertions(+), 27 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e5d101418390..f4c6d2223191 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -922,8 +922,6 @@ static int gic_irq_domain_map(struct irq_domain *d, 
unsigned int irq,
return 0;
 }
 
-#define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1)
-
 static int gic_irq_domain_translate(struct irq_domain *d,
struct irq_fwspec *fwspec,
unsigned long *hwirq,
@@ -938,7 +936,6 @@ static int gic_irq_domain_translate(struct irq_domain *d,
*hwirq = fwspec->param[1] + 32;
break;
case 1: /* PPI */
-   case GIC_IRQ_TYPE_PARTITION:
*hwirq = fwspec->param[1] + 16;
break;
case GIC_IRQ_TYPE_LPI:  /* LPI */
@@ -952,10 +949,8 @@ static int gic_irq_domain_translate(struct irq_domain *d,
 
/*
 * Make it clear that broken DTs are... broken.
-* Partitionned PPIs are an unfortunate exception.
 */
-   WARN_ON(*type == IRQ_TYPE_NONE &&
-   fwspec->param[0] != GIC_IRQ_TYPE_PARTITION);
+   WARN_ON(*type == IRQ_TYPE_NONE);
return 0;
}
 
@@ -1143,6 +1138,12 @@ static int __init gic_validate_dist_version(void __iomem 
*dist_base)
return 0;
 }
 
+static void partition_fwspec_convert(struct irq_fwspec *fwspec)
+{
+   BUG_ON(fwspec->param_count != 4);
+   fwspec->param_count = 3;
+}
+
 /* Create all possible partitions at boot time */
 static void __init gic_populate_ppi_partitions(struct device_node *gic_node)
 {
@@ -1207,23 +1208,11 @@ static void __init gic_populate_ppi_partitions(struct 
device_node *gic_node)
}
 
for (i = 0; i < 16; i++) {
-   unsigned int irq;
struct partition_desc *desc;
-   struct irq_fwspec ppi_fwspec = {
-   .fwnode = gic_data.fwnode,
-   .param_count= 3,
-   .param  = {
-   [0] = GIC_IRQ_TYPE_PARTITION,
-   [1] = i,
-   [2] = IRQ_TYPE_NONE,
-   },
-   };
-
-   irq = irq_create_fwspec_mapping(_fwspec);
-   if (WARN_ON(!irq))
-   continue;
+
desc = partition_create_desc(gic_data.fwnode, parts, nr_parts,
-irq, _domain_ops);
+partition_fwspec_convert,
+_domain_ops);
if (WARN_ON(!desc))
continue;
 
diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index 79a1ef0c0f73..009ce5c97b05 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -25,12 +25,15 @@
 #include 
 
 struct partition_desc {
+   struct mutexlock;
int nr_parts;
struct partition_affinity   *parts;
struct irq_domain   *domain;
struct irq_desc *chained_desc;
unsigned long   *bitmap;
struct irq_domain_ops   ops;
+   struct irq_fwspec   fwspec;
+   void (*convert)(struct irq_fwspec *fwspec);
 };
 
 static struct irq_data *partition_get_irqd_chip(struct partition_desc *part,
@@ -166,9 +169,14 @@ static int partition_domain_alloc(struct irq_domain 
*domain, unsigned int virq,
 
part = domain->host_data;
 
+   mutex_lock(>lock);
+   if (!part->fwspec.param_count) {
+   part->fwspec = *fwspec;
+   part->convert(>fwspec);
+   }
+   mutex_unlock(>lock);
+
set_bit(hwirq, part->bitmap);
-   irq_set_chained_handler_and_data(irq_desc_get_irq(part->chained_desc),
-  

[PATCH 6/7] irqchip/partition-percpu: Allow chained_desc to be NULL

[Marc Zyngier]

As we're about to delay the binding of the partionned interrupt
to the underlying irqchip, we can end-up in a situation where
chained_desc will be NULL, and the operation cannot be carried
out.

Let's make sure we don't explode if that happens.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-partition-percpu.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index be0df6e0add9..79a1ef0c0f73 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -33,6 +33,18 @@ struct partition_desc {
struct irq_domain_ops   ops;
 };
 
+static struct irq_data *partition_get_irqd_chip(struct partition_desc *part,
+   struct irq_chip **chip)
+{
+   if (part->chained_desc) {
+   *chip = irq_desc_get_chip(part->chained_desc);
+   return irq_desc_get_irq_data(part->chained_desc);
+   }
+
+   *chip = NULL;
+   return NULL;
+}
+
 static bool partition_check_cpu(struct partition_desc *part,
unsigned int cpu, unsigned int hwirq)
 {
@@ -45,9 +57,9 @@ static bool partition_check_cpu(struct partition_desc *part,
struct irq_data *data;  \
\
part = irq_data_get_irq_chip_data(d);   \
-   chip = irq_desc_get_chip(part->chained_desc);   \
-   data = irq_desc_get_irq_data(part->chained_desc);   \
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&  \
+   data = partition_get_irqd_chip(part, );\
+   if (data && \
+   partition_check_cpu(part, smp_processor_id(), d->hwirq) &&  \
chip->method)
 
 #define PART_CALL_METHOD_VOID(method, d, ...)  \
@@ -96,10 +108,12 @@ static int partition_irq_set_type(struct irq_data *d, 
unsigned int type)
 static void partition_irq_print_chip(struct irq_data *d, struct seq_file *p)
 {
struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
+   struct irq_chip *chip;
+   struct irq_data *data;
 
-   seq_printf(p, " %5s-%lu", chip->name, data->hwirq);
+   data = partition_get_irqd_chip(part, );
+   if (data)
+   seq_printf(p, " %5s-%lu", chip->name, data->hwirq);
 }
 
 static struct irq_chip partition_irq_chip = {
-- 
2.14.2

[PATCH 6/7] irqchip/partition-percpu: Allow chained_desc to be NULL

[Marc Zyngier]

As we're about to delay the binding of the partionned interrupt
to the underlying irqchip, we can end-up in a situation where
chained_desc will be NULL, and the operation cannot be carried
out.

Let's make sure we don't explode if that happens.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-partition-percpu.c | 26 --
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index be0df6e0add9..79a1ef0c0f73 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -33,6 +33,18 @@ struct partition_desc {
struct irq_domain_ops   ops;
 };
 
+static struct irq_data *partition_get_irqd_chip(struct partition_desc *part,
+   struct irq_chip **chip)
+{
+   if (part->chained_desc) {
+   *chip = irq_desc_get_chip(part->chained_desc);
+   return irq_desc_get_irq_data(part->chained_desc);
+   }
+
+   *chip = NULL;
+   return NULL;
+}
+
 static bool partition_check_cpu(struct partition_desc *part,
unsigned int cpu, unsigned int hwirq)
 {
@@ -45,9 +57,9 @@ static bool partition_check_cpu(struct partition_desc *part,
struct irq_data *data;  \
\
part = irq_data_get_irq_chip_data(d);   \
-   chip = irq_desc_get_chip(part->chained_desc);   \
-   data = irq_desc_get_irq_data(part->chained_desc);   \
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&  \
+   data = partition_get_irqd_chip(part, );\
+   if (data && \
+   partition_check_cpu(part, smp_processor_id(), d->hwirq) &&  \
chip->method)
 
 #define PART_CALL_METHOD_VOID(method, d, ...)  \
@@ -96,10 +108,12 @@ static int partition_irq_set_type(struct irq_data *d, 
unsigned int type)
 static void partition_irq_print_chip(struct irq_data *d, struct seq_file *p)
 {
struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
+   struct irq_chip *chip;
+   struct irq_data *data;
 
-   seq_printf(p, " %5s-%lu", chip->name, data->hwirq);
+   data = partition_get_irqd_chip(part, );
+   if (data)
+   seq_printf(p, " %5s-%lu", chip->name, data->hwirq);
 }
 
 static struct irq_chip partition_irq_chip = {
-- 
2.14.2

[PATCH 0/7] Partitioned percpu IRQ rework

[Marc Zyngier]

Partitioned percpu interrupts are a very niche sport, even in the ARM
world. They occur when each CPU have a private device connected to a
per-CPU interrupt, but not all the devices are the same.

The best example of this occurs on the Rockchip rk3399 SoC, where all
the CPUs have a PMU connected to the same PPI (very good), except that
because this is a big-little system, we end-up with two different PMUs
(quite bad).

We already have some code to deal with this, but it integrates badly
with the debug infrastructure, and makes assumptions about the trigger
of the source interrupt (see the use of IRQ_TYPE_NONE in the GICv3
driver).

This series provides a better integration with debugfs, and cleans up
some of the most horrible parts of this code by simply moving the
plugging of the partition to the activate callback.

This has been tested on a (specially hack-up) KVM guest.

Thanks,

M.

Marc Zyngier (7):
  genirq/debugfs: Print percpu affinity for percpu_devid interrupts
  genirq/debugfs: Use irq_print_chip method if available
  genirq/debugfs: Allow irq domain name to be overriden
  irqchip/partition-percpu: Override domain name in debugfs
  irqchip/partition-percpu: Refactor backend method calling
  irqchip/partition-percpu: Allow chained_desc to be NULL
  irqchip/partition-percpu: Move allocation of chained interrupt to
activate

 drivers/irqchip/irq-gic-v3.c |  31 ++
 drivers/irqchip/irq-partition-percpu.c   | 147 ++-
 include/linux/irqchip/irq-partition-percpu.h |   4 +-
 include/linux/irqdomain.h|   1 +
 kernel/irq/debugfs.c |  13 ++-
 kernel/irq/irqdomain.c   |  14 +++
 6 files changed, 138 insertions(+), 72 deletions(-)

-- 
2.14.2

[PATCH 0/7] Partitioned percpu IRQ rework

[Marc Zyngier]

Partitioned percpu interrupts are a very niche sport, even in the ARM
world. They occur when each CPU have a private device connected to a
per-CPU interrupt, but not all the devices are the same.

The best example of this occurs on the Rockchip rk3399 SoC, where all
the CPUs have a PMU connected to the same PPI (very good), except that
because this is a big-little system, we end-up with two different PMUs
(quite bad).

We already have some code to deal with this, but it integrates badly
with the debug infrastructure, and makes assumptions about the trigger
of the source interrupt (see the use of IRQ_TYPE_NONE in the GICv3
driver).

This series provides a better integration with debugfs, and cleans up
some of the most horrible parts of this code by simply moving the
plugging of the partition to the activate callback.

This has been tested on a (specially hack-up) KVM guest.

Thanks,

M.

Marc Zyngier (7):
  genirq/debugfs: Print percpu affinity for percpu_devid interrupts
  genirq/debugfs: Use irq_print_chip method if available
  genirq/debugfs: Allow irq domain name to be overriden
  irqchip/partition-percpu: Override domain name in debugfs
  irqchip/partition-percpu: Refactor backend method calling
  irqchip/partition-percpu: Allow chained_desc to be NULL
  irqchip/partition-percpu: Move allocation of chained interrupt to
activate

 drivers/irqchip/irq-gic-v3.c |  31 ++
 drivers/irqchip/irq-partition-percpu.c   | 147 ++-
 include/linux/irqchip/irq-partition-percpu.h |   4 +-
 include/linux/irqdomain.h|   1 +
 kernel/irq/debugfs.c |  13 ++-
 kernel/irq/irqdomain.c   |  14 +++
 6 files changed, 138 insertions(+), 72 deletions(-)

-- 
2.14.2

[PATCH 4/7] irqchip/partition-percpu: Override domain name in debugfs

[Marc Zyngier]

So far, domain names that implement a percpu_devid partition
inherit the main domain name, which leads to conflicts in debugfs
and results in the absence of important data.

Let's provide a callback that will repaint the default name, and
keep all the data available.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-partition-percpu.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index ccd72c2cbc23..229f96ab3e26 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -205,6 +205,15 @@ int partition_translate_id(struct partition_desc *desc, 
void *partition_id)
return i;
 }
 
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+static atomic_t part_id;
+static char *partition_override_name(struct irq_domain *domain)
+{
+   return kasprintf(GFP_KERNEL, "%s-part-%d",
+domain->name, atomic_fetch_inc(_id));
+}
+#endif
+
 struct partition_desc *partition_create_desc(struct fwnode_handle *fwnode,
 struct partition_affinity *parts,
 int nr_parts,
@@ -223,6 +232,9 @@ struct partition_desc *partition_create_desc(struct 
fwnode_handle *fwnode,
desc->ops = *ops;
desc->ops.free = partition_domain_free;
desc->ops.alloc = partition_domain_alloc;
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+   desc->ops.override_name = partition_override_name;
+#endif
 
d = irq_domain_create_linear(fwnode, nr_parts, >ops, desc);
if (!d)
-- 
2.14.2

[PATCH 4/7] irqchip/partition-percpu: Override domain name in debugfs

[Marc Zyngier]

So far, domain names that implement a percpu_devid partition
inherit the main domain name, which leads to conflicts in debugfs
and results in the absence of important data.

Let's provide a callback that will repaint the default name, and
keep all the data available.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-partition-percpu.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index ccd72c2cbc23..229f96ab3e26 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -205,6 +205,15 @@ int partition_translate_id(struct partition_desc *desc, 
void *partition_id)
return i;
 }
 
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+static atomic_t part_id;
+static char *partition_override_name(struct irq_domain *domain)
+{
+   return kasprintf(GFP_KERNEL, "%s-part-%d",
+domain->name, atomic_fetch_inc(_id));
+}
+#endif
+
 struct partition_desc *partition_create_desc(struct fwnode_handle *fwnode,
 struct partition_affinity *parts,
 int nr_parts,
@@ -223,6 +232,9 @@ struct partition_desc *partition_create_desc(struct 
fwnode_handle *fwnode,
desc->ops = *ops;
desc->ops.free = partition_domain_free;
desc->ops.alloc = partition_domain_alloc;
+#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
+   desc->ops.override_name = partition_override_name;
+#endif
 
d = irq_domain_create_linear(fwnode, nr_parts, >ops, desc);
if (!d)
-- 
2.14.2

[PATCH 5/7] irqchip/partition-percpu: Refactor backend method calling

[Marc Zyngier]

The various methods provided by the partition-percpu irqchip are only
wrappers that end-up calling into the underlying irqchip. As we're
about to change the code a bit, let's start with introducing a set
of helpers that will make that transition much more painless.

No functionnal change.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-partition-percpu.c | 70 +++---
 1 file changed, 30 insertions(+), 40 deletions(-)

diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index 229f96ab3e26..be0df6e0add9 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -39,68 +39,58 @@ static bool partition_check_cpu(struct partition_desc *part,
return cpumask_test_cpu(cpu, >parts[hwirq].mask);
 }
 
+#define PART_IF_METHOD(method, d)  \
+   struct partition_desc *part;\
+   struct irq_chip *chip;  \
+   struct irq_data *data;  \
+   \
+   part = irq_data_get_irq_chip_data(d);   \
+   chip = irq_desc_get_chip(part->chained_desc);   \
+   data = irq_desc_get_irq_data(part->chained_desc);   \
+   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&  \
+   chip->method)
+
+#define PART_CALL_METHOD_VOID(method, d, ...)  \
+   do {\
+   PART_IF_METHOD(method, d)   \
+   chip->method(data, ##__VA_ARGS__);  \
+   } while(0)
+
+#define PART_CALL_METHOD_INT(retval, method, d, ...)   \
+   ({  \
+   int ret = retval;   \
+   PART_IF_METHOD(method, d)   \
+   ret = chip->method(data, ##__VA_ARGS__);\
+   ret;\
+   })
+
 static void partition_irq_mask(struct irq_data *d)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_mask)
-   chip->irq_mask(data);
+   PART_CALL_METHOD_VOID(irq_mask, d);
 }
 
 static void partition_irq_unmask(struct irq_data *d)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_unmask)
-   chip->irq_unmask(data);
+   PART_CALL_METHOD_VOID(irq_unmask, d);
 }
 
 static int partition_irq_set_irqchip_state(struct irq_data *d,
   enum irqchip_irq_state which,
   bool val)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_set_irqchip_state)
-   return chip->irq_set_irqchip_state(data, which, val);
-
-   return -EINVAL;
+   return PART_CALL_METHOD_INT(-EINVAL, irq_set_irqchip_state, d, which, 
val);
 }
 
 static int partition_irq_get_irqchip_state(struct irq_data *d,
   enum irqchip_irq_state which,
   bool *val)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_get_irqchip_state)
-   return chip->irq_get_irqchip_state(data, which, val);
-
-   return -EINVAL;
+   return PART_CALL_METHOD_INT(-EINVAL, irq_get_irqchip_state, d, which, 
val);
 }
 
 static int partition_irq_set_type(struct irq_data *d, unsigned int type)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (chip->irq_set_type)
-   

Re: [PATCH v2] mm: introduce memory.min

[Roman Gushchin]

Hi Vladimir!

> On Mon, Apr 23, 2018 at 01:36:10PM +0100, Roman Gushchin wrote:
> > +  memory.min
> > +   A read-write single value file which exists on non-root
> > +   cgroups.  The default is "0".
> > +
> > +   Hard memory protection.  If the memory usage of a cgroup
> > +   is within its effective min boundary, the cgroup's memory
> > +   won't be reclaimed under any conditions. If there is no
> > +   unprotected reclaimable memory available, OOM killer
> > +   is invoked.
> 
> What will happen if all tasks attached to a cgroup are killed by OOM,
> but its memory usage is still within memory.min? Will memory.min be
> ignored then?

Not really.

I don't think it's a big problem as long as a user isn't doing
something weird (e.g. moving processes with significant
amount of charged memory to other cgroups).

But what we can do here, is to ignore memory.min of empty cgroups
(patch below), it will resolve some edge cases like this.

Thanks!



>From 54a4f4fc9f8a6847c8a5814c53f0b575010808e5 Mon Sep 17 00:00:00 2001
From: Roman Gushchin 
Date: Tue, 24 Apr 2018 14:44:14 +0100
Subject: [PATCH] mm: ignore memory.min of abandoned memory cgroups

If a cgroup has no associated tasks, invoking the OOM killer
won't help release any memory, so respecting the memory.min
can lead to an infinite OOM loop or system stall.

Let's ignore memory.min of unpopulated cgroups.

Signed-off-by: Roman Gushchin 
Cc: Johannes Weiner 
Cc: Michal Hocko 
Cc: Vladimir Davydov 
Cc: Tejun Heo 
---
 mm/vmscan.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 0a42ab1ce42b..e563d67de787 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2549,8 +2549,11 @@ static bool shrink_node(pg_data_t *pgdat, struct 
scan_control *sc)
/*
 * Hard protection.
 * If there is no reclaimable memory, OOM.
+* Abandoned cgroups are loosing protection,
+* because OOM killer won't release any memory.
 */
-   continue;
+   if (cgroup_is_populated(memcg->css.cgroup))
+   continue;
case MEMCG_PROT_LOW:
/*
 * Soft protection.
-- 
2.14.3

[PATCH 5/7] irqchip/partition-percpu: Refactor backend method calling

[Marc Zyngier]

The various methods provided by the partition-percpu irqchip are only
wrappers that end-up calling into the underlying irqchip. As we're
about to change the code a bit, let's start with introducing a set
of helpers that will make that transition much more painless.

No functionnal change.

Signed-off-by: Marc Zyngier 
---
 drivers/irqchip/irq-partition-percpu.c | 70 +++---
 1 file changed, 30 insertions(+), 40 deletions(-)

diff --git a/drivers/irqchip/irq-partition-percpu.c 
b/drivers/irqchip/irq-partition-percpu.c
index 229f96ab3e26..be0df6e0add9 100644
--- a/drivers/irqchip/irq-partition-percpu.c
+++ b/drivers/irqchip/irq-partition-percpu.c
@@ -39,68 +39,58 @@ static bool partition_check_cpu(struct partition_desc *part,
return cpumask_test_cpu(cpu, >parts[hwirq].mask);
 }
 
+#define PART_IF_METHOD(method, d)  \
+   struct partition_desc *part;\
+   struct irq_chip *chip;  \
+   struct irq_data *data;  \
+   \
+   part = irq_data_get_irq_chip_data(d);   \
+   chip = irq_desc_get_chip(part->chained_desc);   \
+   data = irq_desc_get_irq_data(part->chained_desc);   \
+   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&  \
+   chip->method)
+
+#define PART_CALL_METHOD_VOID(method, d, ...)  \
+   do {\
+   PART_IF_METHOD(method, d)   \
+   chip->method(data, ##__VA_ARGS__);  \
+   } while(0)
+
+#define PART_CALL_METHOD_INT(retval, method, d, ...)   \
+   ({  \
+   int ret = retval;   \
+   PART_IF_METHOD(method, d)   \
+   ret = chip->method(data, ##__VA_ARGS__);\
+   ret;\
+   })
+
 static void partition_irq_mask(struct irq_data *d)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_mask)
-   chip->irq_mask(data);
+   PART_CALL_METHOD_VOID(irq_mask, d);
 }
 
 static void partition_irq_unmask(struct irq_data *d)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_unmask)
-   chip->irq_unmask(data);
+   PART_CALL_METHOD_VOID(irq_unmask, d);
 }
 
 static int partition_irq_set_irqchip_state(struct irq_data *d,
   enum irqchip_irq_state which,
   bool val)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_set_irqchip_state)
-   return chip->irq_set_irqchip_state(data, which, val);
-
-   return -EINVAL;
+   return PART_CALL_METHOD_INT(-EINVAL, irq_set_irqchip_state, d, which, 
val);
 }
 
 static int partition_irq_get_irqchip_state(struct irq_data *d,
   enum irqchip_irq_state which,
   bool *val)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (partition_check_cpu(part, smp_processor_id(), d->hwirq) &&
-   chip->irq_get_irqchip_state)
-   return chip->irq_get_irqchip_state(data, which, val);
-
-   return -EINVAL;
+   return PART_CALL_METHOD_INT(-EINVAL, irq_get_irqchip_state, d, which, 
val);
 }
 
 static int partition_irq_set_type(struct irq_data *d, unsigned int type)
 {
-   struct partition_desc *part = irq_data_get_irq_chip_data(d);
-   struct irq_chip *chip = irq_desc_get_chip(part->chained_desc);
-   struct irq_data *data = irq_desc_get_irq_data(part->chained_desc);
-
-   if (chip->irq_set_type)
-   return 

Re: [PATCH v2] mm: introduce memory.min

[Roman Gushchin]

Hi Vladimir!

> On Mon, Apr 23, 2018 at 01:36:10PM +0100, Roman Gushchin wrote:
> > +  memory.min
> > +   A read-write single value file which exists on non-root
> > +   cgroups.  The default is "0".
> > +
> > +   Hard memory protection.  If the memory usage of a cgroup
> > +   is within its effective min boundary, the cgroup's memory
> > +   won't be reclaimed under any conditions. If there is no
> > +   unprotected reclaimable memory available, OOM killer
> > +   is invoked.
> 
> What will happen if all tasks attached to a cgroup are killed by OOM,
> but its memory usage is still within memory.min? Will memory.min be
> ignored then?

Not really.

I don't think it's a big problem as long as a user isn't doing
something weird (e.g. moving processes with significant
amount of charged memory to other cgroups).

But what we can do here, is to ignore memory.min of empty cgroups
(patch below), it will resolve some edge cases like this.

Thanks!



>From 54a4f4fc9f8a6847c8a5814c53f0b575010808e5 Mon Sep 17 00:00:00 2001
From: Roman Gushchin 
Date: Tue, 24 Apr 2018 14:44:14 +0100
Subject: [PATCH] mm: ignore memory.min of abandoned memory cgroups

If a cgroup has no associated tasks, invoking the OOM killer
won't help release any memory, so respecting the memory.min
can lead to an infinite OOM loop or system stall.

Let's ignore memory.min of unpopulated cgroups.

Signed-off-by: Roman Gushchin 
Cc: Johannes Weiner 
Cc: Michal Hocko 
Cc: Vladimir Davydov 
Cc: Tejun Heo 
---
 mm/vmscan.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 0a42ab1ce42b..e563d67de787 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2549,8 +2549,11 @@ static bool shrink_node(pg_data_t *pgdat, struct 
scan_control *sc)
/*
 * Hard protection.
 * If there is no reclaimable memory, OOM.
+* Abandoned cgroups are loosing protection,
+* because OOM killer won't release any memory.
 */
-   continue;
+   if (cgroup_is_populated(memcg->css.cgroup))
+   continue;
case MEMCG_PROT_LOW:
/*
 * Soft protection.
-- 
2.14.3

Re: [PATCH 1/3] big key: get rid of stack array allocation

[Tycho Andersen]

Hi Eric,

On Mon, Apr 23, 2018 at 09:50:15PM -0700, Eric Biggers wrote:
> Hi Tycho,
> 
> On Mon, Apr 23, 2018 at 07:03:19PM -0600, Tycho Andersen wrote:
> > We're interested in getting rid of all of the stack allocated arrays in the
> > kernel [1]. This patch simply hardcodes the iv length to match that of the
> > hardcoded cipher.
> > 
> > [1]: https://lkml.org/lkml/2018/3/7/621
> > 
> > v2: hardcode the length of the nonce to be the GCM AES IV length, and do a
> > sanity check in init(), Eric Biggers
> > 
> > Signed-off-by: Tycho Andersen 
> > CC: David Howells 
> > CC: James Morris 
> > CC: "Serge E. Hallyn" 
> > CC: Jason A. Donenfeld 
> > CC: Eric Biggers 
> > ---
> >  security/keys/big_key.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/security/keys/big_key.c b/security/keys/big_key.c
> > index 933623784ccd..75c46786a166 100644
> > --- a/security/keys/big_key.c
> > +++ b/security/keys/big_key.c
> > @@ -22,6 +22,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  struct big_key_buf {
> > unsigned intnr_pages;
> > @@ -109,7 +110,7 @@ static int big_key_crypt(enum big_key_op op, struct 
> > big_key_buf *buf, size_t dat
> >  * an .update function, so there's no chance we'll wind up reusing the
> >  * key to encrypt updated data. Simply put: one key, one encryption.
> >  */
> > -   u8 zero_nonce[crypto_aead_ivsize(big_key_aead)];
> > +   u8 zero_nonce[GCM_AES_IV_SIZE];
> >  
> > aead_req = aead_request_alloc(big_key_aead, GFP_KERNEL);
> > if (!aead_req)
> > @@ -425,6 +426,12 @@ static int __init big_key_init(void)
> > pr_err("Can't alloc crypto: %d\n", ret);
> > return ret;
> > }
> > +
> > +   if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > +   WARN(1, "big key algorithm changed?");
> > +   return -EINVAL;
> > +   }
> > +
> 
> 'big_key_aead' needs to be freed on error.
> 
>   err = -EINVAL;
>   goto free_aead;

oof, yes, thanks.

> Also how about defining the IV size next to the algorithm name?
> Then all the algorithm details would be on adjacent lines:
> 
> static const char big_key_alg_name[] = "gcm(aes)";
> #define BIG_KEY_IV_SIZE GCM_AES_IV_SIZE

Sounds good, I'll fix both of these for v3.

Cheers,

Tycho

Re: [PATCH 1/3] big key: get rid of stack array allocation

[Tycho Andersen]

Hi Eric,

On Mon, Apr 23, 2018 at 09:50:15PM -0700, Eric Biggers wrote:
> Hi Tycho,
> 
> On Mon, Apr 23, 2018 at 07:03:19PM -0600, Tycho Andersen wrote:
> > We're interested in getting rid of all of the stack allocated arrays in the
> > kernel [1]. This patch simply hardcodes the iv length to match that of the
> > hardcoded cipher.
> > 
> > [1]: https://lkml.org/lkml/2018/3/7/621
> > 
> > v2: hardcode the length of the nonce to be the GCM AES IV length, and do a
> > sanity check in init(), Eric Biggers
> > 
> > Signed-off-by: Tycho Andersen 
> > CC: David Howells 
> > CC: James Morris 
> > CC: "Serge E. Hallyn" 
> > CC: Jason A. Donenfeld 
> > CC: Eric Biggers 
> > ---
> >  security/keys/big_key.c | 9 -
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/security/keys/big_key.c b/security/keys/big_key.c
> > index 933623784ccd..75c46786a166 100644
> > --- a/security/keys/big_key.c
> > +++ b/security/keys/big_key.c
> > @@ -22,6 +22,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  struct big_key_buf {
> > unsigned intnr_pages;
> > @@ -109,7 +110,7 @@ static int big_key_crypt(enum big_key_op op, struct 
> > big_key_buf *buf, size_t dat
> >  * an .update function, so there's no chance we'll wind up reusing the
> >  * key to encrypt updated data. Simply put: one key, one encryption.
> >  */
> > -   u8 zero_nonce[crypto_aead_ivsize(big_key_aead)];
> > +   u8 zero_nonce[GCM_AES_IV_SIZE];
> >  
> > aead_req = aead_request_alloc(big_key_aead, GFP_KERNEL);
> > if (!aead_req)
> > @@ -425,6 +426,12 @@ static int __init big_key_init(void)
> > pr_err("Can't alloc crypto: %d\n", ret);
> > return ret;
> > }
> > +
> > +   if (unlikely(crypto_aead_ivsize(big_key_aead) != GCM_AES_IV_SIZE)) {
> > +   WARN(1, "big key algorithm changed?");
> > +   return -EINVAL;
> > +   }
> > +
> 
> 'big_key_aead' needs to be freed on error.
> 
>   err = -EINVAL;
>   goto free_aead;

oof, yes, thanks.

> Also how about defining the IV size next to the algorithm name?
> Then all the algorithm details would be on adjacent lines:
> 
> static const char big_key_alg_name[] = "gcm(aes)";
> #define BIG_KEY_IV_SIZE GCM_AES_IV_SIZE

Sounds good, I'll fix both of these for v3.

Cheers,

Tycho

Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[Takashi Iwai]

On Tue, 24 Apr 2018 16:29:15 +0200,
Oleksandr Andrushchenko wrote:
> 
> On 04/24/2018 05:20 PM, Takashi Iwai wrote:
> > On Mon, 16 Apr 2018 08:24:51 +0200,
> > Oleksandr Andrushchenko wrote:
> >> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> >> +{
> >> +  struct xen_snd_front_evtchnl *channel = dev_id;
> >> +  struct xen_snd_front_info *front_info = channel->front_info;
> >> +  struct xensnd_resp *resp;
> >> +  RING_IDX i, rp;
> >> +  unsigned long flags;
> >> +
> >> +  if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> >> +  return IRQ_HANDLED;
> >> +
> >> +  spin_lock_irqsave(_info->io_lock, flags);
> >> +
> >> +again:
> >> +  rp = channel->u.req.ring.sring->rsp_prod;
> >> +  /* ensure we see queued responses up to rp */
> >> +  rmb();
> >> +
> >> +  for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
> > I'm not familiar with Xen stuff in general, but through a quick
> > glance, this kind of code worries me a bit.
> >
> > If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
> > very long loop, no?  Better to have a sanity check of the ring buffer
> > size.
> In this loop I have:
> resp = RING_GET_RESPONSE(>u.req.ring, i);
> and the RING_GET_RESPONSE macro is designed in the way that
> it wraps around when *i* in the question gets bigger than
> the ring size:
> 
> #define RING_GET_REQUEST(_r, _idx)                    \
>     (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
> 
> So, even if the counter has a bogus number it will not last long

Hm, this prevents from accessing outside the ring buffer, but does it
change the loop behavior?

Suppose channel->u.req.ring_rsp_cons = 1, and rp = 0, the loop below
would still consume the whole 32bit counts, no?

for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
resp = RING_GET_RESPONSE(>u.req.ring, i);
...
}


Takashi

Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[Takashi Iwai]

On Tue, 24 Apr 2018 16:29:15 +0200,
Oleksandr Andrushchenko wrote:
> 
> On 04/24/2018 05:20 PM, Takashi Iwai wrote:
> > On Mon, 16 Apr 2018 08:24:51 +0200,
> > Oleksandr Andrushchenko wrote:
> >> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> >> +{
> >> +  struct xen_snd_front_evtchnl *channel = dev_id;
> >> +  struct xen_snd_front_info *front_info = channel->front_info;
> >> +  struct xensnd_resp *resp;
> >> +  RING_IDX i, rp;
> >> +  unsigned long flags;
> >> +
> >> +  if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> >> +  return IRQ_HANDLED;
> >> +
> >> +  spin_lock_irqsave(_info->io_lock, flags);
> >> +
> >> +again:
> >> +  rp = channel->u.req.ring.sring->rsp_prod;
> >> +  /* ensure we see queued responses up to rp */
> >> +  rmb();
> >> +
> >> +  for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
> > I'm not familiar with Xen stuff in general, but through a quick
> > glance, this kind of code worries me a bit.
> >
> > If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
> > very long loop, no?  Better to have a sanity check of the ring buffer
> > size.
> In this loop I have:
> resp = RING_GET_RESPONSE(>u.req.ring, i);
> and the RING_GET_RESPONSE macro is designed in the way that
> it wraps around when *i* in the question gets bigger than
> the ring size:
> 
> #define RING_GET_REQUEST(_r, _idx)                    \
>     (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
> 
> So, even if the counter has a bogus number it will not last long

Hm, this prevents from accessing outside the ring buffer, but does it
change the loop behavior?

Suppose channel->u.req.ring_rsp_cons = 1, and rp = 0, the loop below
would still consume the whole 32bit counts, no?

for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
resp = RING_GET_RESPONSE(>u.req.ring, i);
...
}


Takashi

Re: [PATCH 9/9] Protect SELinux initialized state with pmalloc

[Igor Stoppa]

On 24/04/18 16:49, Stephen Smalley wrote:

On 04/23/2018 08:54 AM, Igor Stoppa wrote:


[...]


The patch is probably in need of rework, to make it fit better with the
new SELinux internal data structures, however it shows how to deny an
easy target to the attacker.


I know this is just an example, but not sure why you wouldn't just protect the
entire selinux_state.


Because I have much more to discuss about SELinux, which would involve 
the whole state, the policyDB and the AVC


I will start a separate thread about that. This was merely as simple as 
possible example of the use of the API.


I just wanted to have a feeling about how it would be received :-)


Note btw that the selinux_state encapsulation is preparatory work
for selinux namespaces [1], at which point the structure is in fact dynamically 
allocated
and there can be multiple instances of it.  That however is work-in-progress, 
highly experimental,
and might not ever make it upstream (if we can't resolve the various challenges 
it poses in a satisfactory
way).


Yes, I am aware of this and I would like to discuss also in the light of 
the future directions.


I just didn't want to waste too much time on something that you might 
want to change radically in a month :-)


I already was caught once by surprise when ss_initalized disappeared 
just when I had a patch ready for it :-)


--
igor

Re: [PATCH 9/9] Protect SELinux initialized state with pmalloc

[Igor Stoppa]

On 24/04/18 16:49, Stephen Smalley wrote:

On 04/23/2018 08:54 AM, Igor Stoppa wrote:


[...]


The patch is probably in need of rework, to make it fit better with the
new SELinux internal data structures, however it shows how to deny an
easy target to the attacker.


I know this is just an example, but not sure why you wouldn't just protect the
entire selinux_state.


Because I have much more to discuss about SELinux, which would involve 
the whole state, the policyDB and the AVC


I will start a separate thread about that. This was merely as simple as 
possible example of the use of the API.


I just wanted to have a feeling about how it would be received :-)


Note btw that the selinux_state encapsulation is preparatory work
for selinux namespaces [1], at which point the structure is in fact dynamically 
allocated
and there can be multiple instances of it.  That however is work-in-progress, 
highly experimental,
and might not ever make it upstream (if we can't resolve the various challenges 
it poses in a satisfactory
way).


Yes, I am aware of this and I would like to discuss also in the light of 
the future directions.


I just didn't want to waste too much time on something that you might 
want to change radically in a month :-)


I already was caught once by surprise when ss_initalized disappeared 
just when I had a patch ready for it :-)


--
igor

Re: [PATCH] ftrace/x86: Fix arch_syscall_match_sym_name for x86_64

[Steven Rostedt]

On Tue, 24 Apr 2018 12:17:38 +0200
Jiri Olsa  wrote:

> Recent commit changed the name prefix of syscalls
> for x86_64 (check the 'Fixes:' for commit number).
> 
> The names switch from "sys_" prefix to ""__x64_sys_",
> which made the default matching function always fail.
> 
> Consequently the ftrace syscall __init code could not
> match any syscall metadata so the "syscall" events
> vanished. This fix returns them back.
> 

Hi Jiri,

I already fixed it and it's queued it my tree. I'm waiting for some
other patches that are waiting for review so I can start testing and
push to Linus.

Also, this was half the fix, the total fix is here:

  http://lkml.kernel.org/r/20180418114509.5a768...@gandalf.local.home

I think I'm done waiting, I'll start testing and get my queue out. If
the other patches don't get the proper acks, I'm not taking them.

Thanks,

-- Steve

> Cc: Andy Lutomirski 
> Cc: Dominik Brodowski 
> Fixes: d5a00528b58c ("syscalls/core, syscalls/x86: Rename struct 
> pt_regs-based sys_*() to __x64_sys_*()")
> Signed-off-by: Jiri Olsa 
> ---
>  arch/x86/include/asm/ftrace.h | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index 09ad88572746..7b131ad39ee2 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -67,6 +67,18 @@ static inline bool arch_trace_is_compat_syscall(struct 
> pt_regs *regs)
>   return false;
>  }
>  #endif /* CONFIG_FTRACE_SYSCALLS && CONFIG_IA32_EMULATION */
> +
> +#ifdef CONFIG_X86_64
> +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME 1
> +static inline bool arch_syscall_match_sym_name(const char *sym, const char 
> *name)
> +{
> + /*
> +  * Skip the __x64_sys_ prefix and compare the
> +  * syscall name only.
> +  */
> + return !strcmp(sym + 9, name + 3);
> +}
> +#endif /* CONFIG_X86_64 */
>  #endif /* !__ASSEMBLY__  && !COMPILE_OFFSETS */
>  
>  #endif /* _ASM_X86_FTRACE_H */

Re: [PATCH] ftrace/x86: Fix arch_syscall_match_sym_name for x86_64

[Steven Rostedt]

On Tue, 24 Apr 2018 12:17:38 +0200
Jiri Olsa  wrote:

> Recent commit changed the name prefix of syscalls
> for x86_64 (check the 'Fixes:' for commit number).
> 
> The names switch from "sys_" prefix to ""__x64_sys_",
> which made the default matching function always fail.
> 
> Consequently the ftrace syscall __init code could not
> match any syscall metadata so the "syscall" events
> vanished. This fix returns them back.
> 

Hi Jiri,

I already fixed it and it's queued it my tree. I'm waiting for some
other patches that are waiting for review so I can start testing and
push to Linus.

Also, this was half the fix, the total fix is here:

  http://lkml.kernel.org/r/20180418114509.5a768...@gandalf.local.home

I think I'm done waiting, I'll start testing and get my queue out. If
the other patches don't get the proper acks, I'm not taking them.

Thanks,

-- Steve

> Cc: Andy Lutomirski 
> Cc: Dominik Brodowski 
> Fixes: d5a00528b58c ("syscalls/core, syscalls/x86: Rename struct 
> pt_regs-based sys_*() to __x64_sys_*()")
> Signed-off-by: Jiri Olsa 
> ---
>  arch/x86/include/asm/ftrace.h | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index 09ad88572746..7b131ad39ee2 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -67,6 +67,18 @@ static inline bool arch_trace_is_compat_syscall(struct 
> pt_regs *regs)
>   return false;
>  }
>  #endif /* CONFIG_FTRACE_SYSCALLS && CONFIG_IA32_EMULATION */
> +
> +#ifdef CONFIG_X86_64
> +#define ARCH_HAS_SYSCALL_MATCH_SYM_NAME 1
> +static inline bool arch_syscall_match_sym_name(const char *sym, const char 
> *name)
> +{
> + /*
> +  * Skip the __x64_sys_ prefix and compare the
> +  * syscall name only.
> +  */
> + return !strcmp(sym + 9, name + 3);
> +}
> +#endif /* CONFIG_X86_64 */
>  #endif /* !__ASSEMBLY__  && !COMPILE_OFFSETS */
>  
>  #endif /* _ASM_X86_FTRACE_H */

Re: [PATCH] clk: qcom: Add some missing gcc clks for msm8996

[Rob Herring]

On Tue, Apr 17, 2018 at 05:09:26PM -0700, Bjorn Andersson wrote:
> From: Rajendra Nayak 
> 
> Add a few missing gcc clks for msm8996
> 
> Signed-off-by: Rajendra Nayak 
> [bjorn: omit aggre0_noc_qosgen_extref_clk]
> Signed-off-by: Bjorn Andersson 
> ---
> 
> This patch was accompanied by one that allows the udelay to be defined per
> branch clock, used by the aggre0_noc_qosgen_extref_clk. I dropped this clock
> and the accompanying path in hope that we can get these clock in place.
> 
> Several of these clocks are needed by the MSS remoteproc driver.
> 
>  drivers/clk/qcom/gcc-msm8996.c   | 186 
> +++
>  include/dt-bindings/clock/qcom,gcc-msm8996.h |   9 ++
>  2 files changed, 195 insertions(+)

Reviewed-by: Rob Herring 

Re: [PATCH] clk: qcom: Add some missing gcc clks for msm8996

[Rob Herring]

On Tue, Apr 17, 2018 at 05:09:26PM -0700, Bjorn Andersson wrote:
> From: Rajendra Nayak 
> 
> Add a few missing gcc clks for msm8996
> 
> Signed-off-by: Rajendra Nayak 
> [bjorn: omit aggre0_noc_qosgen_extref_clk]
> Signed-off-by: Bjorn Andersson 
> ---
> 
> This patch was accompanied by one that allows the udelay to be defined per
> branch clock, used by the aggre0_noc_qosgen_extref_clk. I dropped this clock
> and the accompanying path in hope that we can get these clock in place.
> 
> Several of these clocks are needed by the MSS remoteproc driver.
> 
>  drivers/clk/qcom/gcc-msm8996.c   | 186 
> +++
>  include/dt-bindings/clock/qcom,gcc-msm8996.h |   9 ++
>  2 files changed, 195 insertions(+)

Reviewed-by: Rob Herring 

Re: [PATCH v2 13/13] media: imx274: add SELECTION support for cropping

[Luca Ceresoli]

Hi Sakari,

On 24/04/2018 11:59, Sakari Ailus wrote:
> Hi Luca,
> 
> Thank you for the patchset.
> 
> Some comments below... what I propose is that I apply the rest of the
> patches and then the comments to this one could be addressed separately.
> Would that work for you?

Yes, but I suggest you only apply patches 1-6. I noticed a warning in
patch 9, and patches 7-8 are not very useful without it. Will fix it in v3.

I'll wait for the outcome of the discussion below before sending v3.
Tell me if you prefer me to do it earlier.

> On Tue, Apr 24, 2018 at 10:24:18AM +0200, Luca Ceresoli wrote:
>> Currently this driver does not support cropping. The supported modes
>> are the following, all capturing the entire area:
>>
>>  - 3840x2160, 1:1 binning (native sensor resolution)
>>  - 1920x1080, 2:1 binning
>>  - 1280x720,  3:1 binning
>>
>> The set_fmt callback chooses among these 3 configurations the one that
>> matches the requested format.
>>
>> Adding support to VIDIOC_SUBDEV_G_SELECTION and
>> VIDIOC_SUBDEV_S_SELECTION involved a complete rewrite of set_fmt,
>> which now chooses the most appropriate binning based on the ratio
>> between the previously-set cropping size and the format size being
>> requested.
>>
>> Note that the names in enum imx274_mode change from being
>> resolution-based to being binning-based. Without cropping, the two
>> naming are equivalent. With cropping, the resolution could be
>> different, e.g. using 2:1 binning mode to crop 1200x960 and output a
>> 600x480 format. Using binning in the names avoids anny
>> misunderstanding.
>>
>> Signed-off-by: Luca Ceresoli 
>>
>> ---
>> Changed v1 -> v2:
>>  - add "media: " prefix to commit message
>> ---
>>  drivers/media/i2c/imx274.c | 266 
>> -
>>  1 file changed, 192 insertions(+), 74 deletions(-)
>>
>> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
>> index b6c54712f2aa..ceb5b3e498c6 100644
>> --- a/drivers/media/i2c/imx274.c
>> +++ b/drivers/media/i2c/imx274.c
>> @@ -19,6 +19,7 @@
>>   * along with this program.  If not, see .
>>   */
>>  
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -74,7 +75,7 @@
>>   */
>>  #define IMX274_MIN_EXPOSURE_TIME(4 * 260 / 72)
>>  
>> -#define IMX274_DEFAULT_MODE IMX274_MODE_3840X2160
>> +#define IMX274_DEFAULT_MODE IMX274_MODE_BINNING_OFF
>>  #define IMX274_MAX_WIDTH(3840)
>>  #define IMX274_MAX_HEIGHT   (2160)
>>  #define IMX274_MAX_FRAME_RATE   (120)
>> @@ -111,6 +112,20 @@
>>  #define IMX274_SHR_REG_LSB  0x300C /* SHR */
>>  #define IMX274_SVR_REG_MSB  0x300F /* SVR */
>>  #define IMX274_SVR_REG_LSB  0x300E /* SVR */
>> +#define IMX274_HTRIM_EN_REG 0x3037
>> +#define IMX274_HTRIM_START_REG_LSB  0x3038
>> +#define IMX274_HTRIM_START_REG_MSB  0x3039
>> +#define IMX274_HTRIM_END_REG_LSB0x303A
>> +#define IMX274_HTRIM_END_REG_MSB0x303B
>> +#define IMX274_VWIDCUTEN_REG0x30DD
>> +#define IMX274_VWIDCUT_REG_LSB  0x30DE
>> +#define IMX274_VWIDCUT_REG_MSB  0x30DF
>> +#define IMX274_VWINPOS_REG_LSB  0x30E0
>> +#define IMX274_VWINPOS_REG_MSB  0x30E1
>> +#define IMX274_WRITE_VSIZE_REG_LSB  0x3130
>> +#define IMX274_WRITE_VSIZE_REG_MSB  0x3131
>> +#define IMX274_Y_OUT_SIZE_REG_LSB   0x3132
>> +#define IMX274_Y_OUT_SIZE_REG_MSB   0x3133
>>  #define IMX274_VMAX_REG_1   0x30FA /* VMAX, MSB */
>>  #define IMX274_VMAX_REG_2   0x30F9 /* VMAX */
>>  #define IMX274_VMAX_REG_3   0x30F8 /* VMAX, LSB */
>> @@ -141,9 +156,9 @@ static const struct regmap_config imx274_regmap_config = 
>> {
>>  };
>>  
>>  enum imx274_mode {
>> -IMX274_MODE_3840X2160,
>> -IMX274_MODE_1920X1080,
>> -IMX274_MODE_1280X720,
>> +IMX274_MODE_BINNING_OFF,
>> +IMX274_MODE_BINNING_2_1,
>> +IMX274_MODE_BINNING_3_1,
>>  };
>>  
>>  /*
>> @@ -215,31 +230,14 @@ static const struct reg_8 
>> imx274_mode1_3840x2160_raw10[] = {
>>  {0x3004, 0x01},
>>  {0x3005, 0x01},
>>  {0x3006, 0x00},
>> -{0x3007, 0x02},
>> +{0x3007, 0xa2},
>>  
>>  {0x3018, 0xA2}, /* output XVS, HVS */
>>  
>>  {0x306B, 0x05},
>>  {0x30E2, 0x01},
>> -{0x30F6, 0x07}, /* HMAX, 263 */
>> -{0x30F7, 0x01}, /* HMAX */
>> -
>> -{0x30dd, 0x01}, /* crop to 2160 */
>> -{0x30de, 0x06},
>> -{0x30df, 0x00},
>> -{0x30e0, 0x12},
>> -{0x30e1, 0x00},
>> -{0x3037, 0x01}, /* to crop to 3840 */
>> -{0x3038, 0x0c},
>> -{0x3039, 0x00},
>> -{0x303a, 0x0c},
>> -{0x303b, 0x0f},
>>  
>>  {0x30EE, 0x01},
>> -{0x3130, 0x86},
>> -{0x3131, 0x08},
>> -{0x3132, 0x7E},
>> -{0x3133, 0x08},

Re: [PATCH v2 13/13] media: imx274: add SELECTION support for cropping

[Luca Ceresoli]

Hi Sakari,

On 24/04/2018 11:59, Sakari Ailus wrote:
> Hi Luca,
> 
> Thank you for the patchset.
> 
> Some comments below... what I propose is that I apply the rest of the
> patches and then the comments to this one could be addressed separately.
> Would that work for you?

Yes, but I suggest you only apply patches 1-6. I noticed a warning in
patch 9, and patches 7-8 are not very useful without it. Will fix it in v3.

I'll wait for the outcome of the discussion below before sending v3.
Tell me if you prefer me to do it earlier.

> On Tue, Apr 24, 2018 at 10:24:18AM +0200, Luca Ceresoli wrote:
>> Currently this driver does not support cropping. The supported modes
>> are the following, all capturing the entire area:
>>
>>  - 3840x2160, 1:1 binning (native sensor resolution)
>>  - 1920x1080, 2:1 binning
>>  - 1280x720,  3:1 binning
>>
>> The set_fmt callback chooses among these 3 configurations the one that
>> matches the requested format.
>>
>> Adding support to VIDIOC_SUBDEV_G_SELECTION and
>> VIDIOC_SUBDEV_S_SELECTION involved a complete rewrite of set_fmt,
>> which now chooses the most appropriate binning based on the ratio
>> between the previously-set cropping size and the format size being
>> requested.
>>
>> Note that the names in enum imx274_mode change from being
>> resolution-based to being binning-based. Without cropping, the two
>> naming are equivalent. With cropping, the resolution could be
>> different, e.g. using 2:1 binning mode to crop 1200x960 and output a
>> 600x480 format. Using binning in the names avoids anny
>> misunderstanding.
>>
>> Signed-off-by: Luca Ceresoli 
>>
>> ---
>> Changed v1 -> v2:
>>  - add "media: " prefix to commit message
>> ---
>>  drivers/media/i2c/imx274.c | 266 
>> -
>>  1 file changed, 192 insertions(+), 74 deletions(-)
>>
>> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
>> index b6c54712f2aa..ceb5b3e498c6 100644
>> --- a/drivers/media/i2c/imx274.c
>> +++ b/drivers/media/i2c/imx274.c
>> @@ -19,6 +19,7 @@
>>   * along with this program.  If not, see .
>>   */
>>  
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -74,7 +75,7 @@
>>   */
>>  #define IMX274_MIN_EXPOSURE_TIME(4 * 260 / 72)
>>  
>> -#define IMX274_DEFAULT_MODE IMX274_MODE_3840X2160
>> +#define IMX274_DEFAULT_MODE IMX274_MODE_BINNING_OFF
>>  #define IMX274_MAX_WIDTH(3840)
>>  #define IMX274_MAX_HEIGHT   (2160)
>>  #define IMX274_MAX_FRAME_RATE   (120)
>> @@ -111,6 +112,20 @@
>>  #define IMX274_SHR_REG_LSB  0x300C /* SHR */
>>  #define IMX274_SVR_REG_MSB  0x300F /* SVR */
>>  #define IMX274_SVR_REG_LSB  0x300E /* SVR */
>> +#define IMX274_HTRIM_EN_REG 0x3037
>> +#define IMX274_HTRIM_START_REG_LSB  0x3038
>> +#define IMX274_HTRIM_START_REG_MSB  0x3039
>> +#define IMX274_HTRIM_END_REG_LSB0x303A
>> +#define IMX274_HTRIM_END_REG_MSB0x303B
>> +#define IMX274_VWIDCUTEN_REG0x30DD
>> +#define IMX274_VWIDCUT_REG_LSB  0x30DE
>> +#define IMX274_VWIDCUT_REG_MSB  0x30DF
>> +#define IMX274_VWINPOS_REG_LSB  0x30E0
>> +#define IMX274_VWINPOS_REG_MSB  0x30E1
>> +#define IMX274_WRITE_VSIZE_REG_LSB  0x3130
>> +#define IMX274_WRITE_VSIZE_REG_MSB  0x3131
>> +#define IMX274_Y_OUT_SIZE_REG_LSB   0x3132
>> +#define IMX274_Y_OUT_SIZE_REG_MSB   0x3133
>>  #define IMX274_VMAX_REG_1   0x30FA /* VMAX, MSB */
>>  #define IMX274_VMAX_REG_2   0x30F9 /* VMAX */
>>  #define IMX274_VMAX_REG_3   0x30F8 /* VMAX, LSB */
>> @@ -141,9 +156,9 @@ static const struct regmap_config imx274_regmap_config = 
>> {
>>  };
>>  
>>  enum imx274_mode {
>> -IMX274_MODE_3840X2160,
>> -IMX274_MODE_1920X1080,
>> -IMX274_MODE_1280X720,
>> +IMX274_MODE_BINNING_OFF,
>> +IMX274_MODE_BINNING_2_1,
>> +IMX274_MODE_BINNING_3_1,
>>  };
>>  
>>  /*
>> @@ -215,31 +230,14 @@ static const struct reg_8 
>> imx274_mode1_3840x2160_raw10[] = {
>>  {0x3004, 0x01},
>>  {0x3005, 0x01},
>>  {0x3006, 0x00},
>> -{0x3007, 0x02},
>> +{0x3007, 0xa2},
>>  
>>  {0x3018, 0xA2}, /* output XVS, HVS */
>>  
>>  {0x306B, 0x05},
>>  {0x30E2, 0x01},
>> -{0x30F6, 0x07}, /* HMAX, 263 */
>> -{0x30F7, 0x01}, /* HMAX */
>> -
>> -{0x30dd, 0x01}, /* crop to 2160 */
>> -{0x30de, 0x06},
>> -{0x30df, 0x00},
>> -{0x30e0, 0x12},
>> -{0x30e1, 0x00},
>> -{0x3037, 0x01}, /* to crop to 3840 */
>> -{0x3038, 0x0c},
>> -{0x3039, 0x00},
>> -{0x303a, 0x0c},
>> -{0x303b, 0x0f},
>>  
>>  {0x30EE, 0x01},
>> -{0x3130, 0x86},
>> -{0x3131, 0x08},
>> -{0x3132, 0x7E},
>> -{0x3133, 0x08},
>>  {0x3342, 0x0A},

Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[Oleksandr Andrushchenko]

On 04/24/2018 05:20 PM, Takashi Iwai wrote:

On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:

+static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
+{
+   struct xen_snd_front_evtchnl *channel = dev_id;
+   struct xen_snd_front_info *front_info = channel->front_info;
+   struct xensnd_resp *resp;
+   RING_IDX i, rp;
+   unsigned long flags;
+
+   if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+   return IRQ_HANDLED;
+
+   spin_lock_irqsave(_info->io_lock, flags);
+
+again:
+   rp = channel->u.req.ring.sring->rsp_prod;
+   /* ensure we see queued responses up to rp */
+   rmb();
+
+   for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {

I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.

If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no?  Better to have a sanity check of the ring buffer
size.

In this loop I have:
resp = RING_GET_RESPONSE(>u.req.ring, i);
and the RING_GET_RESPONSE macro is designed in the way that
it wraps around when *i* in the question gets bigger than
the ring size:

#define RING_GET_REQUEST(_r, _idx)                    \
    (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))

So, even if the counter has a bogus number it will not last long


+static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id)
+{
+   struct xen_snd_front_evtchnl *channel = dev_id;
+   struct xen_snd_front_info *front_info = channel->front_info;
+   struct xensnd_event_page *page = channel->u.evt.page;
+   u32 cons, prod;
+   unsigned long flags;
+
+   if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+   return IRQ_HANDLED;
+
+   spin_lock_irqsave(_info->io_lock, flags);
+
+   prod = page->in_prod;
+   /* ensure we see ring contents up to prod */
+   virt_rmb();
+   if (prod == page->in_cons)
+   goto out;
+
+   for (cons = page->in_cons; cons != prod; cons++) {

Ditto.

Same as above


thanks,

Takashi

Thank you,
Oleksandr

Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[Oleksandr Andrushchenko]

On 04/24/2018 05:20 PM, Takashi Iwai wrote:

On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:

+static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
+{
+   struct xen_snd_front_evtchnl *channel = dev_id;
+   struct xen_snd_front_info *front_info = channel->front_info;
+   struct xensnd_resp *resp;
+   RING_IDX i, rp;
+   unsigned long flags;
+
+   if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+   return IRQ_HANDLED;
+
+   spin_lock_irqsave(_info->io_lock, flags);
+
+again:
+   rp = channel->u.req.ring.sring->rsp_prod;
+   /* ensure we see queued responses up to rp */
+   rmb();
+
+   for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {

I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.

If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no?  Better to have a sanity check of the ring buffer
size.

In this loop I have:
resp = RING_GET_RESPONSE(>u.req.ring, i);
and the RING_GET_RESPONSE macro is designed in the way that
it wraps around when *i* in the question gets bigger than
the ring size:

#define RING_GET_REQUEST(_r, _idx)                    \
    (&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))

So, even if the counter has a bogus number it will not last long


+static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id)
+{
+   struct xen_snd_front_evtchnl *channel = dev_id;
+   struct xen_snd_front_info *front_info = channel->front_info;
+   struct xensnd_event_page *page = channel->u.evt.page;
+   u32 cons, prod;
+   unsigned long flags;
+
+   if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+   return IRQ_HANDLED;
+
+   spin_lock_irqsave(_info->io_lock, flags);
+
+   prod = page->in_prod;
+   /* ensure we see ring contents up to prod */
+   virt_rmb();
+   if (prod == page->in_cons)
+   goto out;
+
+   for (cons = page->in_cons; cons != prod; cons++) {

Ditto.

Same as above


thanks,

Takashi

Thank you,
Oleksandr

Re: kernel BUG at fs/f2fs/node.c:LINE!

[Dmitry Vyukov]

On Tue, Apr 24, 2018 at 8:42 AM, syzbot
 wrote:
> Hello,
>
> syzbot tried to test the proposed patch but build/boot failed:
>
> failed to create VM pool: failed to create GCE image: failed to get create
> image operation operation-1524552040215-56a926ecc71d9-3edfeb8b-8abca81c:
> googleapi: Error 403: Rate Limit Exceeded, rateLimitExceeded
>
>
> Tested on git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/dev
> commit
> b549e322861b99f1862fb5f08dfb5e5753a38635 (Sat Apr 21 06:44:59 2018 +)
> f2fs: check cap_resource only for data blocks
>
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> Patch: https://syzkaller.appspot.com/x/patch.diff?id=5565179303559168
> Kernel config:
> https://syzkaller.appspot.com/x/.config?id=1808800213120130118

Hi Jaegeuk,

Re the last failure (rate limiting), now should be fixed with:
https://github.com/google/syzkaller/commit/e2f4bf8f3818d49baf0f3789add75d5fd506ad8d
which adds some backoff-and-retry logic.

Re the first one, do I understand it correctly that dev-test already
contains the fix that you want to test?
We did not foresee such case initially, but you are not the first one
to try this. I've added support for testing without a patch now:
https://github.com/google/syzkaller/commit/9366d03f001170479319878905031f63d4377c46
and updated the docs:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches

So now this should work:

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git
dev-test

Re: kernel BUG at fs/f2fs/node.c:LINE!

[Dmitry Vyukov]

On Tue, Apr 24, 2018 at 8:42 AM, syzbot
 wrote:
> Hello,
>
> syzbot tried to test the proposed patch but build/boot failed:
>
> failed to create VM pool: failed to create GCE image: failed to get create
> image operation operation-1524552040215-56a926ecc71d9-3edfeb8b-8abca81c:
> googleapi: Error 403: Rate Limit Exceeded, rateLimitExceeded
>
>
> Tested on git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/dev
> commit
> b549e322861b99f1862fb5f08dfb5e5753a38635 (Sat Apr 21 06:44:59 2018 +)
> f2fs: check cap_resource only for data blocks
>
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> Patch: https://syzkaller.appspot.com/x/patch.diff?id=5565179303559168
> Kernel config:
> https://syzkaller.appspot.com/x/.config?id=1808800213120130118

Hi Jaegeuk,

Re the last failure (rate limiting), now should be fixed with:
https://github.com/google/syzkaller/commit/e2f4bf8f3818d49baf0f3789add75d5fd506ad8d
which adds some backoff-and-retry logic.

Re the first one, do I understand it correctly that dev-test already
contains the fix that you want to test?
We did not foresee such case initially, but you are not the first one
to try this. I've added support for testing without a patch now:
https://github.com/google/syzkaller/commit/9366d03f001170479319878905031f63d4377c46
and updated the docs:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches

So now this should work:

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git
dev-test

Re: [PATCH 02/39] proc: introduce proc_create_seq{,_data}

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 09:41:06PM +0300, Alexey Dobriyan wrote:
> Should be oopsable.
> Once proc_create_data() returns, entry is live, ->open can be called.

Ok, switching to opencoding proc_create_data instead.

Re: [PATCH 02/39] proc: introduce proc_create_seq{,_data}

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 09:41:06PM +0300, Alexey Dobriyan wrote:
> Should be oopsable.
> Once proc_create_data() returns, entry is live, ->open can be called.

Ok, switching to opencoding proc_create_data instead.

Re: [PATCH 03/39] proc: introduce proc_create_seq_private

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 09:50:27PM +0300, Alexey Dobriyan wrote:
> On Thu, Apr 19, 2018 at 02:41:04PM +0200, Christoph Hellwig wrote:
> > Variant of proc_create_data that directly take a struct seq_operations
> 
> > --- a/fs/proc/internal.h
> > +++ b/fs/proc/internal.h
> > @@ -45,6 +45,7 @@ struct proc_dir_entry {
> > const struct inode_operations *proc_iops;
> > const struct file_operations *proc_fops;
> > const struct seq_operations *seq_ops;
> > +   size_t state_size;
> 
> "unsigned int" please.
> 
> Where have you seen 4GB priv states?

We're passing the result of sizeof, which happens to be a size_t.
But if it makes you happy I can switch to unsigned int.

Re: [PATCH 03/39] proc: introduce proc_create_seq_private

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 09:50:27PM +0300, Alexey Dobriyan wrote:
> On Thu, Apr 19, 2018 at 02:41:04PM +0200, Christoph Hellwig wrote:
> > Variant of proc_create_data that directly take a struct seq_operations
> 
> > --- a/fs/proc/internal.h
> > +++ b/fs/proc/internal.h
> > @@ -45,6 +45,7 @@ struct proc_dir_entry {
> > const struct inode_operations *proc_iops;
> > const struct file_operations *proc_fops;
> > const struct seq_operations *seq_ops;
> > +   size_t state_size;
> 
> "unsigned int" please.
> 
> Where have you seen 4GB priv states?

We're passing the result of sizeof, which happens to be a size_t.
But if it makes you happy I can switch to unsigned int.

Re: [PATCH] Bluetooth: btusb: add ID for LiteOn 04ca:301a

[Marcel Holtmann]

Hi Matthias,

> Contains a QCA6174A chipset, with USB BT. Let's support loading
> firmware on it.
> 
> From usb-devices:
> T:  Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=12   MxCh= 0
> D:  Ver= 2.01 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=04ca ProdID=301a Rev= 0.01
> C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> 
> Signed-off-by: Vic Wei 
> Signed-off-by: Matthias Kaehlcke 
> ---
> drivers/bluetooth/btusb.c | 1 +
> 1 file changed, 1 insertion(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH] Bluetooth: btusb: add ID for LiteOn 04ca:301a

[Marcel Holtmann]

Hi Matthias,

> Contains a QCA6174A chipset, with USB BT. Let's support loading
> firmware on it.
> 
> From usb-devices:
> T:  Bus=02 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#=  2 Spd=12   MxCh= 0
> D:  Ver= 2.01 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
> P:  Vendor=04ca ProdID=301a Rev= 0.01
> C:* #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
> I:* If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> I:* If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
> 
> Signed-off-by: Vic Wei 
> Signed-off-by: Matthias Kaehlcke 
> ---
> drivers/bluetooth/btusb.c | 1 +
> 1 file changed, 1 insertion(+)

patch has been applied to bluetooth-next tree.

Regards

Marcel

Re: [PATCH] drm/admgpu: fix mode_valid's return type

[Harry Wentland]

On 2018-04-24 09:14 AM, Luc Van Oostenryck wrote:
> The method struct drm_connector_helper_funcs::mode_valid is defined
> as returning an 'enum drm_mode_status' but the driver implementation
> for this method uses an 'int' for it.
> 
> Fix this by using 'enum drm_mode_status' in the driver too.
> 
> Signed-off-by: Luc Van Oostenryck 

amd-gfx mailing list should be sufficient for changes like these.

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c| 8 
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c  | 2 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 96501ff0e..8e66851eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -691,7 +691,7 @@ static int amdgpu_connector_lvds_get_modes(struct 
> drm_connector *connector)
>   return ret;
>  }
>  
> -static int amdgpu_connector_lvds_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_lvds_mode_valid(struct 
> drm_connector *connector,
>struct drm_display_mode *mode)
>  {
>   struct drm_encoder *encoder = 
> amdgpu_connector_best_single_encoder(connector);
> @@ -843,7 +843,7 @@ static int amdgpu_connector_vga_get_modes(struct 
> drm_connector *connector)
>   return ret;
>  }
>  
> -static int amdgpu_connector_vga_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_vga_mode_valid(struct 
> drm_connector *connector,
>   struct drm_display_mode *mode)
>  {
>   struct drm_device *dev = connector->dev;
> @@ -1172,7 +1172,7 @@ static void amdgpu_connector_dvi_force(struct 
> drm_connector *connector)
>   amdgpu_connector->use_digital = true;
>  }
>  
> -static int amdgpu_connector_dvi_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_dvi_mode_valid(struct 
> drm_connector *connector,
>   struct drm_display_mode *mode)
>  {
>   struct drm_device *dev = connector->dev;
> @@ -1448,7 +1448,7 @@ amdgpu_connector_dp_detect(struct drm_connector 
> *connector, bool force)
>   return ret;
>  }
>  
> -static int amdgpu_connector_dp_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_dp_mode_valid(struct 
> drm_connector *connector,
>  struct drm_display_mode *mode)
>  {
>   struct amdgpu_connector *amdgpu_connector = 
> to_amdgpu_connector(connector);
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index b51f05dc9..476c9b987 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -329,7 +329,7 @@ static int dce_virtual_get_modes(struct drm_connector 
> *connector)
>   return 0;
>  }
>  
> -static int dce_virtual_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status dce_virtual_mode_valid(struct drm_connector 
> *connector,
> struct drm_display_mode *mode)
>  {
>   return MODE_OK;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 4e2f379ce..d7e52c4f6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2855,7 +2855,7 @@ static void handle_edid_mgmt(struct amdgpu_dm_connector 
> *aconnector)
>   create_eml_sink(aconnector);
>  }
>  
> -int amdgpu_dm_connector_mode_valid(struct drm_connector *connector,
> +enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector 
> *connector,
>  struct drm_display_mode *mode)
>  {
>   int result = MODE_ERROR;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index b68400c11..cb7e20cb3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -246,7 +246,7 @@ void amdgpu_dm_connector_init_helper(struct 
> amdgpu_display_manager *dm,
>struct dc_link *link,
>int link_index);
>  
> -int amdgpu_dm_connector_mode_valid(struct drm_connector *connector,
> +enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector 
> *connector,
>  struct drm_display_mode *mode);
>  
>  void dm_restore_drm_connector_state(struct drm_device *dev,
> 

Re: [PATCH] drm/admgpu: fix mode_valid's return type

[Harry Wentland]

On 2018-04-24 09:14 AM, Luc Van Oostenryck wrote:
> The method struct drm_connector_helper_funcs::mode_valid is defined
> as returning an 'enum drm_mode_status' but the driver implementation
> for this method uses an 'int' for it.
> 
> Fix this by using 'enum drm_mode_status' in the driver too.
> 
> Signed-off-by: Luc Van Oostenryck 

amd-gfx mailing list should be sufficient for changes like these.

Reviewed-by: Harry Wentland 

Harry

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c| 8 
>  drivers/gpu/drm/amd/amdgpu/dce_virtual.c  | 2 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> index 96501ff0e..8e66851eb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> @@ -691,7 +691,7 @@ static int amdgpu_connector_lvds_get_modes(struct 
> drm_connector *connector)
>   return ret;
>  }
>  
> -static int amdgpu_connector_lvds_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_lvds_mode_valid(struct 
> drm_connector *connector,
>struct drm_display_mode *mode)
>  {
>   struct drm_encoder *encoder = 
> amdgpu_connector_best_single_encoder(connector);
> @@ -843,7 +843,7 @@ static int amdgpu_connector_vga_get_modes(struct 
> drm_connector *connector)
>   return ret;
>  }
>  
> -static int amdgpu_connector_vga_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_vga_mode_valid(struct 
> drm_connector *connector,
>   struct drm_display_mode *mode)
>  {
>   struct drm_device *dev = connector->dev;
> @@ -1172,7 +1172,7 @@ static void amdgpu_connector_dvi_force(struct 
> drm_connector *connector)
>   amdgpu_connector->use_digital = true;
>  }
>  
> -static int amdgpu_connector_dvi_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_dvi_mode_valid(struct 
> drm_connector *connector,
>   struct drm_display_mode *mode)
>  {
>   struct drm_device *dev = connector->dev;
> @@ -1448,7 +1448,7 @@ amdgpu_connector_dp_detect(struct drm_connector 
> *connector, bool force)
>   return ret;
>  }
>  
> -static int amdgpu_connector_dp_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status amdgpu_connector_dp_mode_valid(struct 
> drm_connector *connector,
>  struct drm_display_mode *mode)
>  {
>   struct amdgpu_connector *amdgpu_connector = 
> to_amdgpu_connector(connector);
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> index b51f05dc9..476c9b987 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
> @@ -329,7 +329,7 @@ static int dce_virtual_get_modes(struct drm_connector 
> *connector)
>   return 0;
>  }
>  
> -static int dce_virtual_mode_valid(struct drm_connector *connector,
> +static enum drm_mode_status dce_virtual_mode_valid(struct drm_connector 
> *connector,
> struct drm_display_mode *mode)
>  {
>   return MODE_OK;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 4e2f379ce..d7e52c4f6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2855,7 +2855,7 @@ static void handle_edid_mgmt(struct amdgpu_dm_connector 
> *aconnector)
>   create_eml_sink(aconnector);
>  }
>  
> -int amdgpu_dm_connector_mode_valid(struct drm_connector *connector,
> +enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector 
> *connector,
>  struct drm_display_mode *mode)
>  {
>   int result = MODE_ERROR;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index b68400c11..cb7e20cb3 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -246,7 +246,7 @@ void amdgpu_dm_connector_init_helper(struct 
> amdgpu_display_manager *dm,
>struct dc_link *link,
>int link_index);
>  
> -int amdgpu_dm_connector_mode_valid(struct drm_connector *connector,
> +enum drm_mode_status amdgpu_dm_connector_mode_valid(struct drm_connector 
> *connector,
>  struct drm_display_mode *mode);
>  
>  void dm_restore_drm_connector_state(struct drm_device *dev,
> 

Re: simplify procfs code for seq_file instances

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 09:57:50PM +0300, Alexey Dobriyan wrote:
> > git://git.infradead.org/users/hch/misc.git proc_create
> 
> 
> I want to ask if it is time to start using poorman function overloading
> with _b_c_e(). There are millions of allocation functions for example,
> all slightly difference, and people will add more. Seeing /proc interfaces
> doubled like this is painful.

Function overloading is totally unacceptable.

And I very much disagree with a tradeoff that keeps 5000 lines of 
code vs a few new helpers.

Re: simplify procfs code for seq_file instances

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 09:57:50PM +0300, Alexey Dobriyan wrote:
> > git://git.infradead.org/users/hch/misc.git proc_create
> 
> 
> I want to ask if it is time to start using poorman function overloading
> with _b_c_e(). There are millions of allocation functions for example,
> all slightly difference, and people will add more. Seeing /proc interfaces
> doubled like this is painful.

Function overloading is totally unacceptable.

And I very much disagree with a tradeoff that keeps 5000 lines of 
code vs a few new helpers.

Re: Early timeouts due to inaccurate jiffies during system suspend/resume

[Ville Syrjälä]

On Tue, Apr 24, 2018 at 05:07:41PM +0300, Imre Deak wrote:
> On Mon, Apr 23, 2018 at 08:01:28PM +0300, Imre Deak wrote:
> > On Thu, Apr 19, 2018 at 01:05:39PM +0200, Thomas Gleixner wrote:
> > > On Thu, 19 Apr 2018, Imre Deak wrote:
> > > > Hi,
> > > > 
> > > > while checking bug [1], I noticed that jiffies based timing loops like
> > > > 
> > > > expire = jiffies + timeout + 1;
> > > > while (!time_after(jiffies, expire))
> > > > do_something;
> > > > 
> > > > can last shorter than expected (that is less than timeout).
> > > 
> > > Yes, that can happen when the timer interrupt is delayed long enough for
> > > whatever reason. If you need accurate timing then you need to use
> > > ktime_get().
> > 
> > Thanks. I always regarded jiffies as non-accurate, but something that
> > gives a minimum time delay guarantee (when adjusted by +1 as above). I
> > wonder if there are other callers in kernel that don't expect an early
> > timeout.
> 
> msleep and any other schedule_timeout based waits are also affected. At the
> same time for example msleep's documentation says:
> "msleep - sleep safely even with waitqueue interruptions".
> 
> To me that suggests a wait with a minimum guaranteed delay.
> 
> Ville had an idea to make the behavior more deterministic by clamping
> the jiffies increment to 1 for each timer interrupt. Would that work?

Another observation is that this is basically the same problem
that we had with the drm vblank counter. I solved that by introducing
drm_accurate_vblank_count() which makes sure the counter is up to
date before sampling it. Then we can safely do stuff like:

count = drm_accurate_vblank_count();
while (drm_vblank_count() == count)
...;

As long as we don't lose all vblank interrupts that will work and never
complete prematurely. And we still allow the vblank counter to increment
by >1.

I suppose doing something similar for jiffies would be nice as well,
but I'm not sure how feasible that would be. At the very least it
would involve patching a lot of code.

> 
> > 
> > We switched now to using ktime_get_raw() in the i915 driver.
> > 
> > > 
> > > > After some ftracing it seems like jiffies gets stale due to a missed
> > > > LAPIC timer interrupt after the interrupt is armed in
> > > > lapic_next_deadline() and before jiffies is sampled at 2. above.
> > > > Eventually the interrupt does get delivered, at which point jiffies gets
> > > > updated via tick_do_update_jiffies64() with a >1 ticks increment.
> > > > Between lapic_next_deadline() and the - late - delivery of the interrupt
> > > > the CPU on which the interrupt is armed doesn't go idle.
> > > 
> > > That's odd. I have no real explanation for that.
> > 
> > Looks like the reason is IRQ latency. For reference here are the
> > longest ones I found with irqsoff ftracing, all running with IRQs disabled
> > during system resume:
> > 
> > hpet_rtc_interrupt()->hpet_rtc_timer_reinit():
> > do { ... } while(!hpet_cnt_ahead(...));
> > takes sometimes up to ~40msec for me.
> > 
> > hpet_rtc_interrupt()->mc146818_get_time():
> > if (mc146818_is_updating()) mdelay(20);
> > 
> > driver_probe_device->atkbd_connect()->i8042_port_close()->__i8042_command()->i8042_wait_write():
> > takes sometimes up to ~10msec for me.
> > 
> > All the above paired with asynchronous calling of the drivers' resume
> > hooks may result in the jumps in jiffies I saw.
> > 
> > --Imre

-- 
Ville Syrjälä
Intel

Re: Early timeouts due to inaccurate jiffies during system suspend/resume

[Ville Syrjälä]

On Tue, Apr 24, 2018 at 05:07:41PM +0300, Imre Deak wrote:
> On Mon, Apr 23, 2018 at 08:01:28PM +0300, Imre Deak wrote:
> > On Thu, Apr 19, 2018 at 01:05:39PM +0200, Thomas Gleixner wrote:
> > > On Thu, 19 Apr 2018, Imre Deak wrote:
> > > > Hi,
> > > > 
> > > > while checking bug [1], I noticed that jiffies based timing loops like
> > > > 
> > > > expire = jiffies + timeout + 1;
> > > > while (!time_after(jiffies, expire))
> > > > do_something;
> > > > 
> > > > can last shorter than expected (that is less than timeout).
> > > 
> > > Yes, that can happen when the timer interrupt is delayed long enough for
> > > whatever reason. If you need accurate timing then you need to use
> > > ktime_get().
> > 
> > Thanks. I always regarded jiffies as non-accurate, but something that
> > gives a minimum time delay guarantee (when adjusted by +1 as above). I
> > wonder if there are other callers in kernel that don't expect an early
> > timeout.
> 
> msleep and any other schedule_timeout based waits are also affected. At the
> same time for example msleep's documentation says:
> "msleep - sleep safely even with waitqueue interruptions".
> 
> To me that suggests a wait with a minimum guaranteed delay.
> 
> Ville had an idea to make the behavior more deterministic by clamping
> the jiffies increment to 1 for each timer interrupt. Would that work?

Another observation is that this is basically the same problem
that we had with the drm vblank counter. I solved that by introducing
drm_accurate_vblank_count() which makes sure the counter is up to
date before sampling it. Then we can safely do stuff like:

count = drm_accurate_vblank_count();
while (drm_vblank_count() == count)
...;

As long as we don't lose all vblank interrupts that will work and never
complete prematurely. And we still allow the vblank counter to increment
by >1.

I suppose doing something similar for jiffies would be nice as well,
but I'm not sure how feasible that would be. At the very least it
would involve patching a lot of code.

> 
> > 
> > We switched now to using ktime_get_raw() in the i915 driver.
> > 
> > > 
> > > > After some ftracing it seems like jiffies gets stale due to a missed
> > > > LAPIC timer interrupt after the interrupt is armed in
> > > > lapic_next_deadline() and before jiffies is sampled at 2. above.
> > > > Eventually the interrupt does get delivered, at which point jiffies gets
> > > > updated via tick_do_update_jiffies64() with a >1 ticks increment.
> > > > Between lapic_next_deadline() and the - late - delivery of the interrupt
> > > > the CPU on which the interrupt is armed doesn't go idle.
> > > 
> > > That's odd. I have no real explanation for that.
> > 
> > Looks like the reason is IRQ latency. For reference here are the
> > longest ones I found with irqsoff ftracing, all running with IRQs disabled
> > during system resume:
> > 
> > hpet_rtc_interrupt()->hpet_rtc_timer_reinit():
> > do { ... } while(!hpet_cnt_ahead(...));
> > takes sometimes up to ~40msec for me.
> > 
> > hpet_rtc_interrupt()->mc146818_get_time():
> > if (mc146818_is_updating()) mdelay(20);
> > 
> > driver_probe_device->atkbd_connect()->i8042_port_close()->__i8042_command()->i8042_wait_write():
> > takes sometimes up to ~10msec for me.
> > 
> > All the above paired with asynchronous calling of the drivers' resume
> > hooks may result in the jumps in jiffies I saw.
> > 
> > --Imre

-- 
Ville Syrjälä
Intel

Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[Takashi Iwai]

On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:
> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> +{
> + struct xen_snd_front_evtchnl *channel = dev_id;
> + struct xen_snd_front_info *front_info = channel->front_info;
> + struct xensnd_resp *resp;
> + RING_IDX i, rp;
> + unsigned long flags;
> +
> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(_info->io_lock, flags);
> +
> +again:
> + rp = channel->u.req.ring.sring->rsp_prod;
> + /* ensure we see queued responses up to rp */
> + rmb();
> +
> + for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {

I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.

If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no?  Better to have a sanity check of the ring buffer
size.

> +static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id)
> +{
> + struct xen_snd_front_evtchnl *channel = dev_id;
> + struct xen_snd_front_info *front_info = channel->front_info;
> + struct xensnd_event_page *page = channel->u.evt.page;
> + u32 cons, prod;
> + unsigned long flags;
> +
> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(_info->io_lock, flags);
> +
> + prod = page->in_prod;
> + /* ensure we see ring contents up to prod */
> + virt_rmb();
> + if (prod == page->in_cons)
> + goto out;
> +
> + for (cons = page->in_cons; cons != prod; cons++) {

Ditto.


thanks,

Takashi

Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling

[Takashi Iwai]

On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:
> +static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
> +{
> + struct xen_snd_front_evtchnl *channel = dev_id;
> + struct xen_snd_front_info *front_info = channel->front_info;
> + struct xensnd_resp *resp;
> + RING_IDX i, rp;
> + unsigned long flags;
> +
> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(_info->io_lock, flags);
> +
> +again:
> + rp = channel->u.req.ring.sring->rsp_prod;
> + /* ensure we see queued responses up to rp */
> + rmb();
> +
> + for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {

I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.

If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no?  Better to have a sanity check of the ring buffer
size.

> +static irqreturn_t evtchnl_interrupt_evt(int irq, void *dev_id)
> +{
> + struct xen_snd_front_evtchnl *channel = dev_id;
> + struct xen_snd_front_info *front_info = channel->front_info;
> + struct xensnd_event_page *page = channel->u.evt.page;
> + u32 cons, prod;
> + unsigned long flags;
> +
> + if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(_info->io_lock, flags);
> +
> + prod = page->in_prod;
> + /* ensure we see ring contents up to prod */
> + virt_rmb();
> + if (prod == page->in_cons)
> + goto out;
> +
> + for (cons = page->in_cons; cons != prod; cons++) {

Ditto.


thanks,

Takashi

[PATCH] powerpc/signal32: Use fault_in_pages_readable() to prefault user context

[Christophe Leroy]

Use fault_in_pages_readable() to prefault user context
instead of open coding

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/kernel/signal_32.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 492f03451877..cfacb2726152 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #ifdef CONFIG_PPC64
 #include 
@@ -1045,7 +1046,6 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
 struct ucontext __user *new_ctx,
 int ctx_size, int r6, int r7, int r8, struct pt_regs *regs)
 {
-   unsigned char tmp __maybe_unused;
int ctx_has_vsx_region = 0;
 
 #ifdef CONFIG_PPC64
@@ -1109,9 +1109,8 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
}
if (new_ctx == NULL)
return 0;
-   if (!access_ok(VERIFY_READ, new_ctx, ctx_size)
-   || __get_user(tmp, (u8 __user *) new_ctx)
-   || __get_user(tmp, (u8 __user *) new_ctx + ctx_size - 1))
+   if (!access_ok(VERIFY_READ, new_ctx, ctx_size) ||
+   fault_in_pages_readable((u8 __user *)new_ctx, ctx_size))
return -EFAULT;
 
/*
@@ -1231,7 +1230,6 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
 {
struct sig_dbg_op op;
int i;
-   unsigned char tmp __maybe_unused;
unsigned long new_msr = regs->msr;
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
unsigned long new_dbcr0 = current->thread.debug.dbcr0;
@@ -1287,9 +1285,8 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
current->thread.debug.dbcr0 = new_dbcr0;
 #endif
 
-   if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx))
-   || __get_user(tmp, (u8 __user *) ctx)
-   || __get_user(tmp, (u8 __user *) (ctx + 1) - 1))
+   if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx)) ||
+   fault_in_pages_readable((u8 __user *)ctx, 1))
return -EFAULT;
 
/*
-- 
2.13.3

[PATCH] powerpc/signal32: Use fault_in_pages_readable() to prefault user context

[Christophe Leroy]

Use fault_in_pages_readable() to prefault user context
instead of open coding

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/kernel/signal_32.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 492f03451877..cfacb2726152 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #ifdef CONFIG_PPC64
 #include 
@@ -1045,7 +1046,6 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
 struct ucontext __user *new_ctx,
 int ctx_size, int r6, int r7, int r8, struct pt_regs *regs)
 {
-   unsigned char tmp __maybe_unused;
int ctx_has_vsx_region = 0;
 
 #ifdef CONFIG_PPC64
@@ -1109,9 +1109,8 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
}
if (new_ctx == NULL)
return 0;
-   if (!access_ok(VERIFY_READ, new_ctx, ctx_size)
-   || __get_user(tmp, (u8 __user *) new_ctx)
-   || __get_user(tmp, (u8 __user *) new_ctx + ctx_size - 1))
+   if (!access_ok(VERIFY_READ, new_ctx, ctx_size) ||
+   fault_in_pages_readable((u8 __user *)new_ctx, ctx_size))
return -EFAULT;
 
/*
@@ -1231,7 +1230,6 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
 {
struct sig_dbg_op op;
int i;
-   unsigned char tmp __maybe_unused;
unsigned long new_msr = regs->msr;
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
unsigned long new_dbcr0 = current->thread.debug.dbcr0;
@@ -1287,9 +1285,8 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
current->thread.debug.dbcr0 = new_dbcr0;
 #endif
 
-   if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx))
-   || __get_user(tmp, (u8 __user *) ctx)
-   || __get_user(tmp, (u8 __user *) (ctx + 1) - 1))
+   if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx)) ||
+   fault_in_pages_readable((u8 __user *)ctx, 1))
return -EFAULT;
 
/*
-- 
2.13.3

Re: [PATCH 03/39] proc: introduce proc_create_seq_private

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 05:18:18PM +0300, Dan Carpenter wrote:
> > -static const struct file_operations cio_ignore_proc_fops = {
> > -   .open= cio_ignore_proc_open,
> > -   .read= seq_read,
> > -   .llseek  = seq_lseek,
> > -   .release = seq_release_private,
> > -   .write   = cio_ignore_write,
>
> The cio_ignore_write() function isn't used any more so compilers will
> complain.

No compiler in the buildboot farm complained, but neverless this

Re: [PATCH 03/39] proc: introduce proc_create_seq_private

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 05:18:18PM +0300, Dan Carpenter wrote:
> > -static const struct file_operations cio_ignore_proc_fops = {
> > -   .open= cio_ignore_proc_open,
> > -   .read= seq_read,
> > -   .llseek  = seq_lseek,
> > -   .release = seq_release_private,
> > -   .write   = cio_ignore_write,
>
> The cio_ignore_write() function isn't used any more so compilers will
> complain.

No compiler in the buildboot farm complained, but neverless this

Re: [PATCH] arm64: support __int128 with clang

[Mark Rutland]

On Tue, Apr 24, 2018 at 02:56:09PM +0100, Mark Rutland wrote:
> I belevie that currently it is not possible to use a released version of
> clang to build a working kernel without additional patches (at least
> 5.0.0, there don't seem to be any 6.0.0 binaries that can target
> AArch64). Issues include:
> 
> * Released versions of clang don't support the 's' constraint used in
>   some hyp code.
> 

> * Released versions of clang don't support 'rX' as a register name, as
>   is used by some workaround code shared between arm/arm64.

This case will apparently be fixed for 6.0.1 (and presumably 7.0.0):

https://reviews.llvm.org/rL328829
https://bugs.llvm.org/show_bug.cgi?id=36862

Neither have been released yet.

Mark.

Re: [PATCH] arm64: support __int128 with clang

[Mark Rutland]

On Tue, Apr 24, 2018 at 02:56:09PM +0100, Mark Rutland wrote:
> I belevie that currently it is not possible to use a released version of
> clang to build a working kernel without additional patches (at least
> 5.0.0, there don't seem to be any 6.0.0 binaries that can target
> AArch64). Issues include:
> 
> * Released versions of clang don't support the 's' constraint used in
>   some hyp code.
> 

> * Released versions of clang don't support 'rX' as a register name, as
>   is used by some workaround code shared between arm/arm64.

This case will apparently be fixed for 6.0.1 (and presumably 7.0.0):

https://reviews.llvm.org/rL328829
https://bugs.llvm.org/show_bug.cgi?id=36862

Neither have been released yet.

Mark.

Re: [RFC] [PATCH 0/5] procfs: reduce duplication by using symlinks

[Eric W. Biederman]

je...@suse.com writes:

> From: Jeff Mahoney 
>
> Hi all -
>
> I recently encountered a customer issue where, on a machine with many TiB
> of memory and a few hundred cores, after a task with a few thousand threads
> and hundreds of files open exited, the system would softlockup.  That
> issue was (is still) being addressed by Nik Borisov's patch to add a
> cond_resched call to shrink_dentry_list.  The underlying issue is still
> there, though.  We just don't complain as loudly.  When a huge task
> exits, now the system is more or less unresponsive for about eight
> minutes.  All CPUs are pinned and every one of them is going through
> dentry and inode eviction for the procfs files associated with each
> thread.  It's made worse by every CPU contending on the super's
> inode list lock.
>
> The numbers get big.  My test case was 4096 threads with 16384 files
> open.  It's a contrived example, but not that far off from the actual
> customer case.  In this case, a simple "find /proc" would create around
> 300 million dentry/inode pairs.  More practically, lsof(1) does it too,
> it just takes longer.  On smaller systems, memory pressure starts pushing
> them out. Memory pressure isn't really an issue on this machine, so we
> end up using well over 100GB for proc files.  It's the combination of
> the wasted CPU cycles in teardown and the wasted memory at runtime that
> pushed me to take this approach.
>
> The biggest culprit is the "fd" and "fdinfo" directories, but those are
> made worse by there being multiple copies of them even for the same
> task without threads getting involved:
>
> - /proc/pid/fd and /proc/pid/task/pid/fd are identical but share no
>   resources.
>
> - Every /proc/pid/task/*/fd directory in a thread group has identical
>   contents (unless unshare(CLONE_FILES) was called), but share no
>   resources.
>
> - If we do a lookup like /proc/pid/fd on a member of a thread group,
>   we'll get a valid directory.  Inside, there will be a complete
>   copy of /proc/pid/task/* just like in /proc/tgid/task.  Again,
>   nothing is shared.
>
> This patch set reduces some (most) of the duplication by conditionally
> replacing some of the directories with symbolic links to copies that are
> identical.
>
> 1) Eliminate the duplication of the task directories between threads.
>The task directory belongs to the thread leader and the threads
>link to it: e.g. /proc/915/task -> ../910/task  This mainly
>reduces duplication when individual threads are looked up directly
>at the tgid level.  The impact varies based on the number of threads.
>The user has to go out of their way in order to mess up their system
>in this way.  But if they were so inclined, they could create ~550
>billion inodes and dentries using the test case.
>
> 2) Eliminate the duplication of directories that are created identically
>between the tgid-level pid directory and its task directory: fd,
>fdinfo, ns, net, attr.  There is obviously more duplication between
>the two directories, but replacing a file with a symbolic link
>doesn't get us anything.  This reduces the number of files associated
>with fd and fdinfo by half if threads aren't involved.
>
> 3) Eliminate the duplication of fd and fdinfo directories among threads
>that share a files_struct.  We check at directory creation time if
>the task is a group leader and if not, whether it shares ->files with
>the group leader.  If so, we create a symbolic link to ../tgid/fd*.
>We use a d_revalidate callback to check whether the thread has called
>unshare(CLONE_FILES) and, if so, fail the revalidation for the symlink.
>Upon re-lookup, a directory will be created in its place.  This is
>pretty simple, so if the thread group leader calls unshare, all threads
>get directories.
>
> With these patches applied, running the same testcase, the proc_inode
> cache only gets to about 600k objects, which is about 99.7% fewer.  I
> get that procfs isn't supposed to be scalable, but this is kind of
> extreme. :)
>
> Finally, I'm not a procfs expert.  I'm posting this as an RFC for folks
> with more knowledge of the details to pick it apart.  The biggest is that
> I'm not sure if any tools depend on any of these things being directories
> instead of symlinks.  I'd hope not, but I don't have the answer.  I'm
> sure there are corner cases I'm missing.  Hopefully, it's not just flat
> out broken since this is a problem that does need solving.
>
> Now I'll go put on the fireproof suit.

This needs to be tested against at least apparmor to see if this breaks
common policies.  Changing files to symlinks in proc has a bad habit of
either breaking apparmor policies or userspace assumptions.   Symbolic
links are unfortunately visible to userspace.

Further the proc structure is tgid/task/tid where the leaf directories
are per thread.

We more likely could get away with some magic symlinks (that would not
be user 

Re: [RFC] [PATCH 0/5] procfs: reduce duplication by using symlinks

[Eric W. Biederman]

je...@suse.com writes:

> From: Jeff Mahoney 
>
> Hi all -
>
> I recently encountered a customer issue where, on a machine with many TiB
> of memory and a few hundred cores, after a task with a few thousand threads
> and hundreds of files open exited, the system would softlockup.  That
> issue was (is still) being addressed by Nik Borisov's patch to add a
> cond_resched call to shrink_dentry_list.  The underlying issue is still
> there, though.  We just don't complain as loudly.  When a huge task
> exits, now the system is more or less unresponsive for about eight
> minutes.  All CPUs are pinned and every one of them is going through
> dentry and inode eviction for the procfs files associated with each
> thread.  It's made worse by every CPU contending on the super's
> inode list lock.
>
> The numbers get big.  My test case was 4096 threads with 16384 files
> open.  It's a contrived example, but not that far off from the actual
> customer case.  In this case, a simple "find /proc" would create around
> 300 million dentry/inode pairs.  More practically, lsof(1) does it too,
> it just takes longer.  On smaller systems, memory pressure starts pushing
> them out. Memory pressure isn't really an issue on this machine, so we
> end up using well over 100GB for proc files.  It's the combination of
> the wasted CPU cycles in teardown and the wasted memory at runtime that
> pushed me to take this approach.
>
> The biggest culprit is the "fd" and "fdinfo" directories, but those are
> made worse by there being multiple copies of them even for the same
> task without threads getting involved:
>
> - /proc/pid/fd and /proc/pid/task/pid/fd are identical but share no
>   resources.
>
> - Every /proc/pid/task/*/fd directory in a thread group has identical
>   contents (unless unshare(CLONE_FILES) was called), but share no
>   resources.
>
> - If we do a lookup like /proc/pid/fd on a member of a thread group,
>   we'll get a valid directory.  Inside, there will be a complete
>   copy of /proc/pid/task/* just like in /proc/tgid/task.  Again,
>   nothing is shared.
>
> This patch set reduces some (most) of the duplication by conditionally
> replacing some of the directories with symbolic links to copies that are
> identical.
>
> 1) Eliminate the duplication of the task directories between threads.
>The task directory belongs to the thread leader and the threads
>link to it: e.g. /proc/915/task -> ../910/task  This mainly
>reduces duplication when individual threads are looked up directly
>at the tgid level.  The impact varies based on the number of threads.
>The user has to go out of their way in order to mess up their system
>in this way.  But if they were so inclined, they could create ~550
>billion inodes and dentries using the test case.
>
> 2) Eliminate the duplication of directories that are created identically
>between the tgid-level pid directory and its task directory: fd,
>fdinfo, ns, net, attr.  There is obviously more duplication between
>the two directories, but replacing a file with a symbolic link
>doesn't get us anything.  This reduces the number of files associated
>with fd and fdinfo by half if threads aren't involved.
>
> 3) Eliminate the duplication of fd and fdinfo directories among threads
>that share a files_struct.  We check at directory creation time if
>the task is a group leader and if not, whether it shares ->files with
>the group leader.  If so, we create a symbolic link to ../tgid/fd*.
>We use a d_revalidate callback to check whether the thread has called
>unshare(CLONE_FILES) and, if so, fail the revalidation for the symlink.
>Upon re-lookup, a directory will be created in its place.  This is
>pretty simple, so if the thread group leader calls unshare, all threads
>get directories.
>
> With these patches applied, running the same testcase, the proc_inode
> cache only gets to about 600k objects, which is about 99.7% fewer.  I
> get that procfs isn't supposed to be scalable, but this is kind of
> extreme. :)
>
> Finally, I'm not a procfs expert.  I'm posting this as an RFC for folks
> with more knowledge of the details to pick it apart.  The biggest is that
> I'm not sure if any tools depend on any of these things being directories
> instead of symlinks.  I'd hope not, but I don't have the answer.  I'm
> sure there are corner cases I'm missing.  Hopefully, it's not just flat
> out broken since this is a problem that does need solving.
>
> Now I'll go put on the fireproof suit.

This needs to be tested against at least apparmor to see if this breaks
common policies.  Changing files to symlinks in proc has a bad habit of
either breaking apparmor policies or userspace assumptions.   Symbolic
links are unfortunately visible to userspace.

Further the proc structure is tgid/task/tid where the leaf directories
are per thread.

We more likely could get away with some magic symlinks (that would not
be user visible) rather than 

Re: [RFC v4 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Yang Shi]

On 4/24/18 5:23 AM, Kirill A. Shutemov wrote:

On Tue, Apr 24, 2018 at 12:00:50PM +0800, Yang Shi wrote:

Since tmpfs THP was supported in 4.8, hugetlbfs is not the only
filesystem with huge page support anymore. tmpfs can use huge page via
THP when mounting by "huge=" mount option.

When applications use huge page on hugetlbfs, it just need check the
filesystem magic number, but it is not enough for tmpfs. Make
stat.st_blksize return huge page size if it is mounted by appropriate
"huge=" option to give applications a hint to optimize the behavior with
THP.

Some applications may not do wisely with THP. For example, QEMU may mmap
file on non huge page aligned hint address with MAP_FIXED, which results
in no pages are PMD mapped even though THP is used. Some applications
may mmap file with non huge page aligned offset. Both behaviors make THP
pointless.

statfs.f_bsize still returns 4KB for tmpfs since THP could be split, and it
also may fallback to 4KB page silently if there is not enough huge page.
Furthermore, different f_bsize makes max_blocks and free_blocks
calculation harder but without too much benefit. Returning huge page
size via stat.st_blksize sounds good enough.

Since PUD size huge page for THP has not been supported, now it just
returns HPAGE_PMD_SIZE.

Signed-off-by: Yang Shi 
Cc: "Kirill A. Shutemov" 
Cc: Hugh Dickins 
Cc: Michal Hocko 
Cc: Alexander Viro 
Suggested-by: Christoph Hellwig 
---
v3 --> v4:
* Rework the commit log per the education from Michal and Kirill
* Fix build error if CONFIG_TRANSPARENT_HUGEPAGE is disabled
v2 --> v3:
* Use shmem_sb_info.huge instead of global variable per Michal's comment
v2 --> v1:
* Adopted the suggestion from hch to return huge page size via st_blksize
   instead of creating a new flag.

  mm/shmem.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index b859192..19b8055 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -988,6 +988,7 @@ static int shmem_getattr(const struct path *path, struct 
kstat *stat,
  {
struct inode *inode = path->dentry->d_inode;
struct shmem_inode_info *info = SHMEM_I(inode);
+   struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
  
  	if (info->alloced - info->swapped != inode->i_mapping->nrpages) {

spin_lock_irq(>lock);
@@ -995,6 +996,11 @@ static int shmem_getattr(const struct path *path, struct 
kstat *stat,
spin_unlock_irq(>lock);
}
generic_fillattr(inode, stat);
+#ifdef CONFIG_TRANSPARENT_HUGE_PAGECACHE
+   if (sbinfo->huge > 0)

No ifdeffery, please.

And we probably want to check if shmem_huge is 'force'.

Something like this?

if (IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
 (shmem_huge == SHMEM_HUGE_FORCE || sbinfo->huge))


BTW, it sounds we also need check shmem_huge != SHMEM_HUGE_DENY, right? 
Otherwise it still return huge page size, but in fact no huge page can 
be allocated with 'deny'.


So, how about:

if (IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
         (shmem_huge == SHMEM_HUGE_FORCE || sbinfo->huge) &&
 shmem_huge != SHMEM_HUGE_DENY)

Thanks,
Yang




+   stat->blksize = HPAGE_PMD_SIZE;
+#endif
+   
return 0;
  }
  
--

1.8.3.1

Re: [RFC v4 PATCH] mm: shmem: make stat.st_blksize return huge page size if THP is on

[Yang Shi]

On 4/24/18 5:23 AM, Kirill A. Shutemov wrote:

On Tue, Apr 24, 2018 at 12:00:50PM +0800, Yang Shi wrote:

Since tmpfs THP was supported in 4.8, hugetlbfs is not the only
filesystem with huge page support anymore. tmpfs can use huge page via
THP when mounting by "huge=" mount option.

When applications use huge page on hugetlbfs, it just need check the
filesystem magic number, but it is not enough for tmpfs. Make
stat.st_blksize return huge page size if it is mounted by appropriate
"huge=" option to give applications a hint to optimize the behavior with
THP.

Some applications may not do wisely with THP. For example, QEMU may mmap
file on non huge page aligned hint address with MAP_FIXED, which results
in no pages are PMD mapped even though THP is used. Some applications
may mmap file with non huge page aligned offset. Both behaviors make THP
pointless.

statfs.f_bsize still returns 4KB for tmpfs since THP could be split, and it
also may fallback to 4KB page silently if there is not enough huge page.
Furthermore, different f_bsize makes max_blocks and free_blocks
calculation harder but without too much benefit. Returning huge page
size via stat.st_blksize sounds good enough.

Since PUD size huge page for THP has not been supported, now it just
returns HPAGE_PMD_SIZE.

Signed-off-by: Yang Shi 
Cc: "Kirill A. Shutemov" 
Cc: Hugh Dickins 
Cc: Michal Hocko 
Cc: Alexander Viro 
Suggested-by: Christoph Hellwig 
---
v3 --> v4:
* Rework the commit log per the education from Michal and Kirill
* Fix build error if CONFIG_TRANSPARENT_HUGEPAGE is disabled
v2 --> v3:
* Use shmem_sb_info.huge instead of global variable per Michal's comment
v2 --> v1:
* Adopted the suggestion from hch to return huge page size via st_blksize
   instead of creating a new flag.

  mm/shmem.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index b859192..19b8055 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -988,6 +988,7 @@ static int shmem_getattr(const struct path *path, struct 
kstat *stat,
  {
struct inode *inode = path->dentry->d_inode;
struct shmem_inode_info *info = SHMEM_I(inode);
+   struct shmem_sb_info *sbinfo = SHMEM_SB(inode->i_sb);
  
  	if (info->alloced - info->swapped != inode->i_mapping->nrpages) {

spin_lock_irq(>lock);
@@ -995,6 +996,11 @@ static int shmem_getattr(const struct path *path, struct 
kstat *stat,
spin_unlock_irq(>lock);
}
generic_fillattr(inode, stat);
+#ifdef CONFIG_TRANSPARENT_HUGE_PAGECACHE
+   if (sbinfo->huge > 0)

No ifdeffery, please.

And we probably want to check if shmem_huge is 'force'.

Something like this?

if (IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
 (shmem_huge == SHMEM_HUGE_FORCE || sbinfo->huge))


BTW, it sounds we also need check shmem_huge != SHMEM_HUGE_DENY, right? 
Otherwise it still return huge page size, but in fact no huge page can 
be allocated with 'deny'.


So, how about:

if (IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
         (shmem_huge == SHMEM_HUGE_FORCE || sbinfo->huge) &&
 shmem_huge != SHMEM_HUGE_DENY)

Thanks,
Yang




+   stat->blksize = HPAGE_PMD_SIZE;
+#endif
+   
return 0;
  }
  
--

1.8.3.1

Re: [PATCH 16/39] ipmi: simplify procfs code

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 10:29:29AM -0500, Corey Minyard wrote:
> On 04/19/2018 07:41 AM, Christoph Hellwig wrote:
>> Use remove_proc_subtree to remove the whole subtree on cleanup instead
>> of a hand rolled list of proc entries, unwind the registration loop into
>> individual calls.  Switch to use proc_create_single to further simplify
>> the code.
>
> I'm yanking all the proc code out of the IPMI driver in 3.18.  So this is 
> probably
> not necessary.

Ok, I'll drop this patch.

Re: [PATCH 16/39] ipmi: simplify procfs code

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 10:29:29AM -0500, Corey Minyard wrote:
> On 04/19/2018 07:41 AM, Christoph Hellwig wrote:
>> Use remove_proc_subtree to remove the whole subtree on cleanup instead
>> of a hand rolled list of proc entries, unwind the registration loop into
>> individual calls.  Switch to use proc_create_single to further simplify
>> the code.
>
> I'm yanking all the proc code out of the IPMI driver in 3.18.  So this is 
> probably
> not necessary.

Ok, I'll drop this patch.

Re: [PATCH 26/39] rtc/proc: switch to proc_create_single_data

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 03:10:27PM +0200, Alexandre Belloni wrote:
> On 19/04/2018 14:41:27+0200, Christoph Hellwig wrote:
> > And stop trying to get a reference on the submodule, procfs code deals
> > with release after and unloaded module and thus removed proc entry.
> > 
> 
> Are you sure about that? The rtc module is not the one adding the procfs
> file so I'm not sure how the procfs code can handle it.

The proc file is removed from this call chain:

  _exit (module_exit handler)
-> rtc_device_unregister
  -> rtc_proc_del_device
-> remove_proc_entry

remove_proc_entry takes care of waiting for currently active file
operation instances and makes sure every new operation never calls
into the actual proc file ops.  Same behavior as in RTC exists all
over the kernel.

Re: [PATCH 26/39] rtc/proc: switch to proc_create_single_data

[Christoph Hellwig]

On Thu, Apr 19, 2018 at 03:10:27PM +0200, Alexandre Belloni wrote:
> On 19/04/2018 14:41:27+0200, Christoph Hellwig wrote:
> > And stop trying to get a reference on the submodule, procfs code deals
> > with release after and unloaded module and thus removed proc entry.
> > 
> 
> Are you sure about that? The rtc module is not the one adding the procfs
> file so I'm not sure how the procfs code can handle it.

The proc file is removed from this call chain:

  _exit (module_exit handler)
-> rtc_device_unregister
  -> rtc_proc_del_device
-> remove_proc_entry

remove_proc_entry takes care of waiting for currently active file
operation instances and makes sure every new operation never calls
into the actual proc file ops.  Same behavior as in RTC exists all
over the kernel.

Re: [PATCH v3 01/19] powerpc/powermac: Mark variable x as unused

[Christophe LEROY]

Le 04/04/2018 à 22:07, Mathieu Malaterre a écrit :

Since the value of x is never intended to be read, declare it with gcc
attribute as unused. Fix warning treated as error with W=1:

   arch/powerpc/platforms/powermac/bootx_init.c:471:21: error: variable ‘x’ set 
but not used [-Werror=unused-but-set-variable]

Suggested-by: Christophe Leroy 
Signed-off-by: Mathieu Malaterre 
---
v3: style: add missing empty line after declaration
v2: move x variable within its local scope

  arch/powerpc/platforms/powermac/bootx_init.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powermac/bootx_init.c 
b/arch/powerpc/platforms/powermac/bootx_init.c
index c3c9bbb3573a..ba0964c17620 100644
--- a/arch/powerpc/platforms/powermac/bootx_init.c
+++ b/arch/powerpc/platforms/powermac/bootx_init.c
@@ -468,7 +468,7 @@ void __init bootx_init(unsigned long r3, unsigned long r4)
boot_infos_t *bi = (boot_infos_t *) r4;
unsigned long hdr;
unsigned long space;
-   unsigned long ptr, x;
+   unsigned long ptr;
char *model;
unsigned long offset = reloc_offset();
  
@@ -562,6 +562,8 @@ void __init bootx_init(unsigned long r3, unsigned long r4)

 * MMU switched OFF, so this should not be useful anymore.
 */
if (bi->version < 4) {
+   unsigned long x __maybe_unused;
+


That's detail, but shouldn't it be marked __always_unused instead ?

Christophe


bootx_printf("Touching pages...\n");
  
  		/*

Re: [PATCH v3 01/19] powerpc/powermac: Mark variable x as unused

[Christophe LEROY]

Le 04/04/2018 à 22:07, Mathieu Malaterre a écrit :

Since the value of x is never intended to be read, declare it with gcc
attribute as unused. Fix warning treated as error with W=1:

   arch/powerpc/platforms/powermac/bootx_init.c:471:21: error: variable ‘x’ set 
but not used [-Werror=unused-but-set-variable]

Suggested-by: Christophe Leroy 
Signed-off-by: Mathieu Malaterre 
---
v3: style: add missing empty line after declaration
v2: move x variable within its local scope

  arch/powerpc/platforms/powermac/bootx_init.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powermac/bootx_init.c 
b/arch/powerpc/platforms/powermac/bootx_init.c
index c3c9bbb3573a..ba0964c17620 100644
--- a/arch/powerpc/platforms/powermac/bootx_init.c
+++ b/arch/powerpc/platforms/powermac/bootx_init.c
@@ -468,7 +468,7 @@ void __init bootx_init(unsigned long r3, unsigned long r4)
boot_infos_t *bi = (boot_infos_t *) r4;
unsigned long hdr;
unsigned long space;
-   unsigned long ptr, x;
+   unsigned long ptr;
char *model;
unsigned long offset = reloc_offset();
  
@@ -562,6 +562,8 @@ void __init bootx_init(unsigned long r3, unsigned long r4)

 * MMU switched OFF, so this should not be useful anymore.
 */
if (bi->version < 4) {
+   unsigned long x __maybe_unused;
+


That's detail, but shouldn't it be marked __always_unused instead ?

Christophe


bootx_printf("Touching pages...\n");
  
  		/*

[PATCH 05/10] ia64: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Benefit from the generic softirq mask implementation that rely on per-CPU
mutators instead of working with raw operators on top of this_cpu_ptr().

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/ia64/include/asm/hardirq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/hardirq.h b/arch/ia64/include/asm/hardirq.h
index 22fae71..ccde7c2 100644
--- a/arch/ia64/include/asm/hardirq.h
+++ b/arch/ia64/include/asm/hardirq.h
@@ -13,7 +13,7 @@
 
 #define __ARCH_IRQ_STAT1
 
-#define local_softirq_pending()
(*this_cpu_ptr(_cpu_info.softirq_pending))
+#define local_softirq_pending_ref  ia64_cpu_info.softirq_pending
 
 #include 
 #include 
-- 
2.7.4

[PATCH 05/10] ia64: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Benefit from the generic softirq mask implementation that rely on per-CPU
mutators instead of working with raw operators on top of this_cpu_ptr().

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/ia64/include/asm/hardirq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/hardirq.h b/arch/ia64/include/asm/hardirq.h
index 22fae71..ccde7c2 100644
--- a/arch/ia64/include/asm/hardirq.h
+++ b/arch/ia64/include/asm/hardirq.h
@@ -13,7 +13,7 @@
 
 #define __ARCH_IRQ_STAT1
 
-#define local_softirq_pending()
(*this_cpu_ptr(_cpu_info.softirq_pending))
+#define local_softirq_pending_ref  ia64_cpu_info.softirq_pending
 
 #include 
 #include 
-- 
2.7.4

Re: [PATCH 20/39] afs: simplify procfs code

[Christoph Hellwig]

On Fri, Apr 20, 2018 at 01:29:34PM +0100, David Howells wrote:
> David Howells  wrote:
> 
> > > Use remove_proc_subtree to remove the whole subtree on cleanup, and
> > > unwind the registration loop into individual calls.  Switch to use
> > > proc_create_seq where applicable.
> > 
> > Note that this is likely going to clash with my patch to net-namespace all 
> > of
> > the afs proc files:
> > 
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2
> > 
> > If it helps, I should be able to disentangle this from the mount-api changes
> > since the subsequent patch connects the dots to propagate the network
> > namespace over automount using the new fs_context to do it.
> 
> Okay, I'll follow up this mail with a pair of patches to just use network
> namespacing in AFS.  The first exports a function from core code only; the
> second is the actual modifications to AFS.

I don't think you should need any of these.  seq_file_net or
seq_file_single_net will return you the net_ns based on a struct
seq_file.  And even from your write routines you can reach the
seq_file in file->private pretty easily.

Re: [PATCH 20/39] afs: simplify procfs code

[Christoph Hellwig]

On Fri, Apr 20, 2018 at 01:29:34PM +0100, David Howells wrote:
> David Howells  wrote:
> 
> > > Use remove_proc_subtree to remove the whole subtree on cleanup, and
> > > unwind the registration loop into individual calls.  Switch to use
> > > proc_create_seq where applicable.
> > 
> > Note that this is likely going to clash with my patch to net-namespace all 
> > of
> > the afs proc files:
> > 
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-context=f60c26c827c073583107ebf19d87bc5c0e71c3d2
> > 
> > If it helps, I should be able to disentangle this from the mount-api changes
> > since the subsequent patch connects the dots to propagate the network
> > namespace over automount using the new fs_context to do it.
> 
> Okay, I'll follow up this mail with a pair of patches to just use network
> namespacing in AFS.  The first exports a function from core code only; the
> second is the actual modifications to AFS.

I don't think you should need any of these.  seq_file_net or
seq_file_single_net will return you the net_ns based on a struct
seq_file.  And even from your write routines you can reach the
seq_file in file->private pretty easily.

[PATCH 06/10] parisc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/parisc/include/asm/hardirq.h | 8 
 1 file changed, 8 deletions(-)

diff --git a/arch/parisc/include/asm/hardirq.h 
b/arch/parisc/include/asm/hardirq.h
index 0778151..1a1235a 100644
--- a/arch/parisc/include/asm/hardirq.h
+++ b/arch/parisc/include/asm/hardirq.h
@@ -34,14 +34,6 @@ DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 #define __IRQ_STAT(cpu, member) (irq_stat[cpu].member)
 #define inc_irq_stat(member)   this_cpu_inc(irq_stat.member)
 #define __inc_irq_stat(member) __this_cpu_inc(irq_stat.member)
-#define local_softirq_pending()
this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
-
-#define set_softirq_pending(x) \
-   this_cpu_write(irq_stat.__softirq_pending, (x))
-#define or_softirq_pending(x)  this_cpu_or(irq_stat.__softirq_pending, (x))
-
 #define ack_bad_irq(irq) WARN(1, "unexpected IRQ trap at vector %02x\n", irq)
 
 #endif /* _PARISC_HARDIRQ_H */
-- 
2.7.4

[PATCH 06/10] parisc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/parisc/include/asm/hardirq.h | 8 
 1 file changed, 8 deletions(-)

diff --git a/arch/parisc/include/asm/hardirq.h 
b/arch/parisc/include/asm/hardirq.h
index 0778151..1a1235a 100644
--- a/arch/parisc/include/asm/hardirq.h
+++ b/arch/parisc/include/asm/hardirq.h
@@ -34,14 +34,6 @@ DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 #define __IRQ_STAT(cpu, member) (irq_stat[cpu].member)
 #define inc_irq_stat(member)   this_cpu_inc(irq_stat.member)
 #define __inc_irq_stat(member) __this_cpu_inc(irq_stat.member)
-#define local_softirq_pending()
this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
-
-#define set_softirq_pending(x) \
-   this_cpu_write(irq_stat.__softirq_pending, (x))
-#define or_softirq_pending(x)  this_cpu_or(irq_stat.__softirq_pending, (x))
-
 #define ack_bad_irq(irq) WARN(1, "unexpected IRQ trap at vector %02x\n", irq)
 
 #endif /* _PARISC_HARDIRQ_H */
-- 
2.7.4

c9e97a1997 ("mm: initialize pages on demand during boot"): BUG: kernel reboot-without-warning in early-boot stage, last printk: early console in setup code

[kernel test robot]

Greetings,

0day kernel testing robot got the below dmesg and the first bad commit is

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

commit c9e97a1997fbf3a1d18d4065c2ca381f0704d7e5
Author: Pavel Tatashin <pasha.tatas...@oracle.com>
AuthorDate: Thu Apr 5 16:22:31 2018 -0700
Commit: Linus Torvalds <torva...@linux-foundation.org>
CommitDate: Thu Apr 5 21:36:25 2018 -0700

mm: initialize pages on demand during boot

Deferred page initialization allows the boot cpu to initialize a small
subset of the system's pages early in boot, with other cpus doing the
rest later on.

It is, however, problematic to know how many pages the kernel needs
during boot.  Different modules and kernel parameters may change the
requirement, so the boot cpu either initializes too many pages or runs
out of memory.

To fix that, initialize early pages on demand.  This ensures the kernel
does the minimum amount of work to initialize pages during boot and
leaves the rest to be divided in the multithreaded initialization path
(deferred_init_memmap).

The on-demand code is permanently disabled using static branching once
deferred pages are initialized.  After the static branch is changed to
false, the overhead is up-to two branch-always instructions if the zone
watermark check fails or if rmqueue fails.

Sergey Senozhatsky noticed that while deferred pages currently make
sense only on NUMA machines (we start one thread per latency node),
CONFIG_NUMA is not a requirement for CONFIG_DEFERRED_STRUCT_PAGE_INIT,
so that is also must be addressed in the patch.

[a...@linux-foundation.org: fix typo in comment, make deferred_pages static]
[pasha.tatas...@oracle.com: fix min() type mismatch warning]
  Link: 
http://lkml.kernel.org/r/20180212164543.26592-1-pasha.tatas...@oracle.com
[pasha.tatas...@oracle.com: use zone_to_nid() in deferred_grow_zone()]
  Link: 
http://lkml.kernel.org/r/20180214163343.21234-2-pasha.tatas...@oracle.com
[pasha.tatas...@oracle.com: might_sleep warning]
  Link: 
http://lkml.kernel.org/r/20180306192022.28289-1-pasha.tatas...@oracle.com
[a...@linux-foundation.org: s/spin_lock/spin_lock_irq/ in 
page_alloc_init_late()]
[pasha.tatas...@oracle.com: v5]
  Link: 
http://lkml.kernel.org/r/20180309220807.24961-3-pasha.tatas...@oracle.com
[a...@linux-foundation.org: tweak comments]
[pasha.tatas...@oracle.com: v6]
  Link: 
http://lkml.kernel.org/r/20180313182355.17669-3-pasha.tatas...@oracle.com
[a...@linux-foundation.org: coding-style fixes]
Link: 
http://lkml.kernel.org/r/20180209192216.20509-2-pasha.tatas...@oracle.com
Signed-off-by: Pavel Tatashin <pasha.tatas...@oracle.com>
Reviewed-by: Daniel Jordan <daniel.m.jor...@oracle.com>
Reviewed-by: Steven Sistare <steven.sist...@oracle.com>
Reviewed-by: Andrew Morton <a...@linux-foundation.org>
Tested-by: Masayoshi Mizuma <m.miz...@jp.fujitsu.com>
Acked-by: Mel Gorman <mgor...@suse.de>
Cc: Michal Hocko <mho...@suse.com>
Cc: Catalin Marinas <catalin.mari...@arm.com>
Cc: AKASHI Takahiro <takahiro.aka...@linaro.org>
Cc: Gioh Kim <gi-oh@profitbricks.com>
Cc: Heiko Carstens <heiko.carst...@de.ibm.com>
Cc: Yaowei Bai <baiyao...@cmss.chinamobile.com>
Cc: Wei Yang <richard.weiy...@gmail.com>
Cc: Paul Burton <paul.bur...@mips.com>
Cc: Miles Chen <miles.c...@mediatek.com>
Cc: Vlastimil Babka <vba...@suse.cz>
Cc: Johannes Weiner <han...@cmpxchg.org>
Signed-off-by: Andrew Morton <a...@linux-foundation.org>
Signed-off-by: Linus Torvalds <torva...@linux-foundation.org>

3a2d7fa8a3  mm: disable interrupts while initializing deferred pages
c9e97a1997  mm: initialize pages on demand during boot
6d08b06e67  Linux 4.17-rc2
43cd1f4979  Add linux-next specific files for 20180424
+---+++---+---+
|   
| 3a2d7fa8a3 | c9e97a1997 | v4.17-rc2 | next-20180424 |
+---+++---+---+
| boot_successes
| 45 | 10 | 1 | 0 |
| boot_failures 
| 0  | 8  | 17| 10|
| 
BUG:kernel_reboot-without-warning_in_early-boot_stage,last_printk:early_console_in_setup_code
 | 0  | 8  | 12| 10|

c9e97a1997 ("mm: initialize pages on demand during boot"): BUG: kernel reboot-without-warning in early-boot stage, last printk: early console in setup code

[kernel test robot]

Greetings,

0day kernel testing robot got the below dmesg and the first bad commit is

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

commit c9e97a1997fbf3a1d18d4065c2ca381f0704d7e5
Author: Pavel Tatashin 
AuthorDate: Thu Apr 5 16:22:31 2018 -0700
Commit: Linus Torvalds 
CommitDate: Thu Apr 5 21:36:25 2018 -0700

mm: initialize pages on demand during boot

Deferred page initialization allows the boot cpu to initialize a small
subset of the system's pages early in boot, with other cpus doing the
rest later on.

It is, however, problematic to know how many pages the kernel needs
during boot.  Different modules and kernel parameters may change the
requirement, so the boot cpu either initializes too many pages or runs
out of memory.

To fix that, initialize early pages on demand.  This ensures the kernel
does the minimum amount of work to initialize pages during boot and
leaves the rest to be divided in the multithreaded initialization path
(deferred_init_memmap).

The on-demand code is permanently disabled using static branching once
deferred pages are initialized.  After the static branch is changed to
false, the overhead is up-to two branch-always instructions if the zone
watermark check fails or if rmqueue fails.

Sergey Senozhatsky noticed that while deferred pages currently make
sense only on NUMA machines (we start one thread per latency node),
CONFIG_NUMA is not a requirement for CONFIG_DEFERRED_STRUCT_PAGE_INIT,
so that is also must be addressed in the patch.

[a...@linux-foundation.org: fix typo in comment, make deferred_pages static]
[pasha.tatas...@oracle.com: fix min() type mismatch warning]
  Link: 
http://lkml.kernel.org/r/20180212164543.26592-1-pasha.tatas...@oracle.com
[pasha.tatas...@oracle.com: use zone_to_nid() in deferred_grow_zone()]
  Link: 
http://lkml.kernel.org/r/20180214163343.21234-2-pasha.tatas...@oracle.com
[pasha.tatas...@oracle.com: might_sleep warning]
  Link: 
http://lkml.kernel.org/r/20180306192022.28289-1-pasha.tatas...@oracle.com
[a...@linux-foundation.org: s/spin_lock/spin_lock_irq/ in 
page_alloc_init_late()]
[pasha.tatas...@oracle.com: v5]
  Link: 
http://lkml.kernel.org/r/20180309220807.24961-3-pasha.tatas...@oracle.com
[a...@linux-foundation.org: tweak comments]
[pasha.tatas...@oracle.com: v6]
  Link: 
http://lkml.kernel.org/r/20180313182355.17669-3-pasha.tatas...@oracle.com
[a...@linux-foundation.org: coding-style fixes]
Link: 
http://lkml.kernel.org/r/20180209192216.20509-2-pasha.tatas...@oracle.com
Signed-off-by: Pavel Tatashin 
Reviewed-by: Daniel Jordan 
Reviewed-by: Steven Sistare 
Reviewed-by: Andrew Morton 
Tested-by: Masayoshi Mizuma 
Acked-by: Mel Gorman 
Cc: Michal Hocko 
Cc: Catalin Marinas 
Cc: AKASHI Takahiro 
Cc: Gioh Kim 
Cc: Heiko Carstens 
Cc: Yaowei Bai 
Cc: Wei Yang 
Cc: Paul Burton 
Cc: Miles Chen 
Cc: Vlastimil Babka 
Cc: Johannes Weiner 
Signed-off-by: Andrew Morton 
Signed-off-by: Linus Torvalds 

3a2d7fa8a3  mm: disable interrupts while initializing deferred pages
c9e97a1997  mm: initialize pages on demand during boot
6d08b06e67  Linux 4.17-rc2
43cd1f4979  Add linux-next specific files for 20180424
+---+++---+---+
|   
| 3a2d7fa8a3 | c9e97a1997 | v4.17-rc2 | next-20180424 |
+---+++---+---+
| boot_successes
| 45 | 10 | 1 | 0 |
| boot_failures 
| 0  | 8  | 17| 10|
| 
BUG:kernel_reboot-without-warning_in_early-boot_stage,last_printk:early_console_in_setup_code
 | 0  | 8  | 12| 10|
| BUG:unable_to_handle_kernel   
| 0  | 0  | 5 |   |
| Oops:#[##]
| 0  | 0  | 5 |   |
| RIP:llc_ui_release
| 0  | 0  | 5 |   |
| Kernel_panic-not_syncing:Fatal_exception  
| 0  | 0  | 5

[PATCH 09/10] x86: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/x86/include/asm/hardirq.h | 8 
 1 file changed, 8 deletions(-)

diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index 5ea2afd..740a428a 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -50,14 +50,6 @@ DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 
 #define inc_irq_stat(member)   this_cpu_inc(irq_stat.member)
 
-#define local_softirq_pending()
this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
-
-#define set_softirq_pending(x) \
-   this_cpu_write(irq_stat.__softirq_pending, (x))
-#define or_softirq_pending(x)  this_cpu_or(irq_stat.__softirq_pending, (x))
-
 extern void ack_bad_irq(unsigned int irq);
 
 extern u64 arch_irq_stat_cpu(unsigned int cpu);
-- 
2.7.4

[PATCH 09/10] x86: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/x86/include/asm/hardirq.h | 8 
 1 file changed, 8 deletions(-)

diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index 5ea2afd..740a428a 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -50,14 +50,6 @@ DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 
 #define inc_irq_stat(member)   this_cpu_inc(irq_stat.member)
 
-#define local_softirq_pending()
this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
-
-#define set_softirq_pending(x) \
-   this_cpu_write(irq_stat.__softirq_pending, (x))
-#define or_softirq_pending(x)  this_cpu_or(irq_stat.__softirq_pending, (x))
-
 extern void ack_bad_irq(unsigned int irq);
 
 extern u64 arch_irq_stat_cpu(unsigned int cpu);
-- 
2.7.4

Re: [PATCH] ALSA: hda: Request driver probe from an async task

[Takashi Iwai]

On Tue, 24 Apr 2018 16:03:53 +0200,
Paul Menzel wrote:
> 
> Dear Takashi,
> 
> 
> On 04/24/18 14:15, Takashi Iwai wrote:
> > On Tue, 24 Apr 2018 13:59:58 +0200,
> > Paul Menzel wrote:
> 
> >> On 04/23/18 14:33, Takashi Iwai wrote:
> >>> On Mon, 23 Apr 2018 14:30:36 +0200, Paul Menzel wrote:
> >>
>  On 04/23/18 14:21, Takashi Iwai wrote:
> > On Mon, 23 Apr 2018 14:05:52 +0200, Paul Menzel wrote:
> >>
> >> From: Paul Menzel 
> >> Date: Sat, 24 Mar 2018 09:28:43 +0100
> >>
> >> On an ASRock E350M1, with Linux 4.17-rc1 according to `initcall_debug`
> >> calling `azx_driver_init` takes sometimes more than a few milliseconds,
> >> and up to 200 ms.
> >>
> >> ```
> >> [2.892598] calling  azx_driver_init+0x0/0xfe4 [snd_hda_intel] @ 218
> >> [2.943002] initcall azx_driver_init+0x0/0xfe4 [snd_hda_intel]
> >> returned 0 after 49195 usecs
> >> ```
> >>
> >> Trying to execute the Linux kernel in less than 500 ms, this is quite a
> >> hold-up, and therefore request the probe from an async task.
> >>
> >> With this change, the test shows, that the function returns earlier.
> >>
> >> ```
> >> [3.254800] calling  azx_driver_init+0x0/0xfe4 [snd_hda_intel] @ 227
> >> [3.254887] initcall azx_driver_init+0x0/0xfe4 [snd_hda_intel]
> >> returned 0 after 66 usecs
> >> ```
> >>
> >> The same behavior is visible on a Dell OptiPlex 7010. The longer times
> >> seem to happen, when the module *e1000e* is probed during the same 
> >> time.
> >>
> >> Signed-off-by: Paul Menzel 
> >
> > What actually took so long?  Could you analyze further instead of
> > blindly putting the flag?
> 
>  Well, I am not sure. Could you please give me hints, how to debug this
>  further? Is there some debug flag?
> >>>
> >>> Usually perf would help, but even a simple printk() should suffice to
> >>> see what's going on there :)
> >>
> >> Please find the messages for a 23 ms run below, and the debug patch
> >> attached.
> >>
> >>> [2.996238] calling  azx_driver_init+0x0/0xfe4 [snd_hda_intel] @ 214
> >>> [3.009838] calling  rtl8169_pci_driver_init+0x0/0x1000 [r8169] @ 217
> >>> [3.009904] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
> >>> [3.010721] random: get_random_bytes called from 
> >>> ipv6_regen_rndid+0x2c/0x100 with crng_init=1
> >>> [3.011283] r8169 :03:00.0 eth0: RTL8168e/8111e at 0x(ptrval), 
> >>> bc:5f:f4:c8:d3:98, XID 0c20 IRQ 26
> >>> [3.011289] r8169 :03:00.0 eth0: jumbo features [frames: 9200 
> >>> bytes, tx checksumming: ko]
> >>> [3.013876] initcall rtl8169_pci_driver_init+0x0/0x1000 [r8169] 
> >>> returned 0 after 3917 usecs
> >>> [3.031754] calling  pcspkr_platform_driver_init+0x0/0x1000 [pcspkr] @ 
> >>> 221
> >>> [3.031904] input: PC Speaker as /devices/platform/pcspkr/input/input4
> >>> [3.032288] initcall pcspkr_platform_driver_init+0x0/0x1000 [pcspkr] 
> >>> returned 0 after 508 usecs
> >>> [3.034795] calling  psmouse_init+0x0/0x7c [psmouse] @ 220
> >>> [3.034903] initcall psmouse_init+0x0/0x7c [psmouse] returned 0 after 
> >>> 87 usecs
> >>> [3.043051] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043154] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043187] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043198] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043229] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043265] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043429] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043479] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043749] random: get_random_bytes called from key_alloc+0x1fc/0x5e0 
> >>> with crng_init=1
> >>> [3.043973] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.045483] random: get_random_u32 called from 
> >>> arch_rnd.part.2+0x18/0x40 with crng_init=1
> >>> [3.045501] random: get_random_u32 called from 
> >>> load_elf_binary+0x76a/0x1d20 with crng_init=1
> >>> [3.045507] random: get_random_u32 called from 
> >>> arch_align_stack+0x45/0x70 with crng_init=1
> >>> [3.045528] random: get_random_u32 called from 
> >>> arch_rnd.part.2+0x18/0x40 with crng_init=1
> >>> [3.045558] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.045771] random: get_random_u32 called from 
> >>> 

Re: [PATCH] ALSA: hda: Request driver probe from an async task

[Takashi Iwai]

On Tue, 24 Apr 2018 16:03:53 +0200,
Paul Menzel wrote:
> 
> Dear Takashi,
> 
> 
> On 04/24/18 14:15, Takashi Iwai wrote:
> > On Tue, 24 Apr 2018 13:59:58 +0200,
> > Paul Menzel wrote:
> 
> >> On 04/23/18 14:33, Takashi Iwai wrote:
> >>> On Mon, 23 Apr 2018 14:30:36 +0200, Paul Menzel wrote:
> >>
>  On 04/23/18 14:21, Takashi Iwai wrote:
> > On Mon, 23 Apr 2018 14:05:52 +0200, Paul Menzel wrote:
> >>
> >> From: Paul Menzel 
> >> Date: Sat, 24 Mar 2018 09:28:43 +0100
> >>
> >> On an ASRock E350M1, with Linux 4.17-rc1 according to `initcall_debug`
> >> calling `azx_driver_init` takes sometimes more than a few milliseconds,
> >> and up to 200 ms.
> >>
> >> ```
> >> [2.892598] calling  azx_driver_init+0x0/0xfe4 [snd_hda_intel] @ 218
> >> [2.943002] initcall azx_driver_init+0x0/0xfe4 [snd_hda_intel]
> >> returned 0 after 49195 usecs
> >> ```
> >>
> >> Trying to execute the Linux kernel in less than 500 ms, this is quite a
> >> hold-up, and therefore request the probe from an async task.
> >>
> >> With this change, the test shows, that the function returns earlier.
> >>
> >> ```
> >> [3.254800] calling  azx_driver_init+0x0/0xfe4 [snd_hda_intel] @ 227
> >> [3.254887] initcall azx_driver_init+0x0/0xfe4 [snd_hda_intel]
> >> returned 0 after 66 usecs
> >> ```
> >>
> >> The same behavior is visible on a Dell OptiPlex 7010. The longer times
> >> seem to happen, when the module *e1000e* is probed during the same 
> >> time.
> >>
> >> Signed-off-by: Paul Menzel 
> >
> > What actually took so long?  Could you analyze further instead of
> > blindly putting the flag?
> 
>  Well, I am not sure. Could you please give me hints, how to debug this
>  further? Is there some debug flag?
> >>>
> >>> Usually perf would help, but even a simple printk() should suffice to
> >>> see what's going on there :)
> >>
> >> Please find the messages for a 23 ms run below, and the debug patch
> >> attached.
> >>
> >>> [2.996238] calling  azx_driver_init+0x0/0xfe4 [snd_hda_intel] @ 214
> >>> [3.009838] calling  rtl8169_pci_driver_init+0x0/0x1000 [r8169] @ 217
> >>> [3.009904] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
> >>> [3.010721] random: get_random_bytes called from 
> >>> ipv6_regen_rndid+0x2c/0x100 with crng_init=1
> >>> [3.011283] r8169 :03:00.0 eth0: RTL8168e/8111e at 0x(ptrval), 
> >>> bc:5f:f4:c8:d3:98, XID 0c20 IRQ 26
> >>> [3.011289] r8169 :03:00.0 eth0: jumbo features [frames: 9200 
> >>> bytes, tx checksumming: ko]
> >>> [3.013876] initcall rtl8169_pci_driver_init+0x0/0x1000 [r8169] 
> >>> returned 0 after 3917 usecs
> >>> [3.031754] calling  pcspkr_platform_driver_init+0x0/0x1000 [pcspkr] @ 
> >>> 221
> >>> [3.031904] input: PC Speaker as /devices/platform/pcspkr/input/input4
> >>> [3.032288] initcall pcspkr_platform_driver_init+0x0/0x1000 [pcspkr] 
> >>> returned 0 after 508 usecs
> >>> [3.034795] calling  psmouse_init+0x0/0x7c [psmouse] @ 220
> >>> [3.034903] initcall psmouse_init+0x0/0x7c [psmouse] returned 0 after 
> >>> 87 usecs
> >>> [3.043051] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043154] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043187] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043198] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043229] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043265] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043429] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.043479] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.043749] random: get_random_bytes called from key_alloc+0x1fc/0x5e0 
> >>> with crng_init=1
> >>> [3.043973] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.045483] random: get_random_u32 called from 
> >>> arch_rnd.part.2+0x18/0x40 with crng_init=1
> >>> [3.045501] random: get_random_u32 called from 
> >>> load_elf_binary+0x76a/0x1d20 with crng_init=1
> >>> [3.045507] random: get_random_u32 called from 
> >>> arch_align_stack+0x45/0x70 with crng_init=1
> >>> [3.045528] random: get_random_u32 called from 
> >>> arch_rnd.part.2+0x18/0x40 with crng_init=1
> >>> [3.045558] random: get_random_u32 called from 
> >>> cache_alloc_refill+0x5bb/0x13d0 with crng_init=1
> >>> [3.045771] random: get_random_u32 called from 
> >>> cache_random_seq_create+0xa3/0x1f0 with crng_init=1
> >>> [3.046127] 

[PATCH 10/10] softirq/s390: Move default mutators of overwritten softirq mask to s390

[Frederic Weisbecker]

s390 is now the last architecture that entirely overwrites
local_softirq_pending() and uses the according default definitions of
set_softirq_pending() and or_softirq_pending().

Just move these to s390 to debloat the generic code complexity.

Suggested-by: Peter Zijlstra 
Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/s390/include/asm/hardirq.h | 2 ++
 include/linux/interrupt.h   | 7 ---
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/s390/include/asm/hardirq.h b/arch/s390/include/asm/hardirq.h
index a296c6a..dfbc3c6c0 100644
--- a/arch/s390/include/asm/hardirq.h
+++ b/arch/s390/include/asm/hardirq.h
@@ -14,6 +14,8 @@
 #include 
 
 #define local_softirq_pending() (S390_lowcore.softirq_pending)
+#define set_softirq_pending(x) (S390_lowcore.softirq_pending = (x))
+#define or_softirq_pending(x)  (S390_lowcore.softirq_pending |= (x))
 
 #define __ARCH_IRQ_STAT
 #define __ARCH_HAS_DO_SOFTIRQ
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 7a11f73..eeceac3 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -442,13 +442,6 @@ extern bool force_irqthreads;
 #define set_softirq_pending(x) (__this_cpu_write(local_softirq_pending_ref, 
(x)))
 #define or_softirq_pending(x)  (__this_cpu_or(local_softirq_pending_ref, (x)))
 
-#else /* local_softirq_pending */
-
-#ifndef __ARCH_SET_SOFTIRQ_PENDING
-#define set_softirq_pending(x) (local_softirq_pending() = (x))
-#define or_softirq_pending(x)  (local_softirq_pending() |= (x))
-#endif
-
 #endif /* local_softirq_pending */
 
 /* Some architectures might implement lazy enabling/disabling of
-- 
2.7.4

[PATCH 07/10] powerpc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Acked-by: Michael Ellerman 
Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/powerpc/include/asm/hardirq.h | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/include/asm/hardirq.h 
b/arch/powerpc/include/asm/hardirq.h
index 5986d47..383f628 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -25,15 +25,8 @@ typedef struct {
 DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 
 #define __ARCH_IRQ_STAT
-
-#define local_softirq_pending()
__this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
 #define __ARCH_IRQ_EXIT_IRQS_DISABLED
 
-#define set_softirq_pending(x) __this_cpu_write(irq_stat.__softirq_pending, 
(x))
-#define or_softirq_pending(x) __this_cpu_or(irq_stat.__softirq_pending, (x))
-
 static inline void ack_bad_irq(unsigned int irq)
 {
printk(KERN_CRIT "unexpected IRQ trap at vector %02x\n", irq);
-- 
2.7.4

[PATCH 10/10] softirq/s390: Move default mutators of overwritten softirq mask to s390

[Frederic Weisbecker]

s390 is now the last architecture that entirely overwrites
local_softirq_pending() and uses the according default definitions of
set_softirq_pending() and or_softirq_pending().

Just move these to s390 to debloat the generic code complexity.

Suggested-by: Peter Zijlstra 
Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/s390/include/asm/hardirq.h | 2 ++
 include/linux/interrupt.h   | 7 ---
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/s390/include/asm/hardirq.h b/arch/s390/include/asm/hardirq.h
index a296c6a..dfbc3c6c0 100644
--- a/arch/s390/include/asm/hardirq.h
+++ b/arch/s390/include/asm/hardirq.h
@@ -14,6 +14,8 @@
 #include 
 
 #define local_softirq_pending() (S390_lowcore.softirq_pending)
+#define set_softirq_pending(x) (S390_lowcore.softirq_pending = (x))
+#define or_softirq_pending(x)  (S390_lowcore.softirq_pending |= (x))
 
 #define __ARCH_IRQ_STAT
 #define __ARCH_HAS_DO_SOFTIRQ
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 7a11f73..eeceac3 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -442,13 +442,6 @@ extern bool force_irqthreads;
 #define set_softirq_pending(x) (__this_cpu_write(local_softirq_pending_ref, 
(x)))
 #define or_softirq_pending(x)  (__this_cpu_or(local_softirq_pending_ref, (x)))
 
-#else /* local_softirq_pending */
-
-#ifndef __ARCH_SET_SOFTIRQ_PENDING
-#define set_softirq_pending(x) (local_softirq_pending() = (x))
-#define or_softirq_pending(x)  (local_softirq_pending() |= (x))
-#endif
-
 #endif /* local_softirq_pending */
 
 /* Some architectures might implement lazy enabling/disabling of
-- 
2.7.4

[PATCH 07/10] powerpc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Acked-by: Michael Ellerman 
Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/powerpc/include/asm/hardirq.h | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/include/asm/hardirq.h 
b/arch/powerpc/include/asm/hardirq.h
index 5986d47..383f628 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -25,15 +25,8 @@ typedef struct {
 DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 
 #define __ARCH_IRQ_STAT
-
-#define local_softirq_pending()
__this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
 #define __ARCH_IRQ_EXIT_IRQS_DISABLED
 
-#define set_softirq_pending(x) __this_cpu_write(irq_stat.__softirq_pending, 
(x))
-#define or_softirq_pending(x) __this_cpu_or(irq_stat.__softirq_pending, (x))
-
 static inline void ack_bad_irq(unsigned int irq)
 {
printk(KERN_CRIT "unexpected IRQ trap at vector %02x\n", irq);
-- 
2.7.4

[PATCH 08/10] sparc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Benefit from the generic softirq mask implementation that rely on per-CPU
mutators instead of working with raw operators on top of this_cpu_ptr().

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/sparc/include/asm/hardirq_64.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/hardirq_64.h 
b/arch/sparc/include/asm/hardirq_64.h
index 6aba904..75b92bf 100644
--- a/arch/sparc/include/asm/hardirq_64.h
+++ b/arch/sparc/include/asm/hardirq_64.h
@@ -10,8 +10,9 @@
 #include 
 
 #define __ARCH_IRQ_STAT
-#define local_softirq_pending() \
-   (*this_cpu_ptr(&__cpu_data.__softirq_pending))
+
+#define local_softirq_pending_ref \
+   __cpu_data.__softirq_pending
 
 void ack_bad_irq(unsigned int irq);
 
-- 
2.7.4

[PATCH 08/10] sparc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Benefit from the generic softirq mask implementation that rely on per-CPU
mutators instead of working with raw operators on top of this_cpu_ptr().

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/sparc/include/asm/hardirq_64.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/hardirq_64.h 
b/arch/sparc/include/asm/hardirq_64.h
index 6aba904..75b92bf 100644
--- a/arch/sparc/include/asm/hardirq_64.h
+++ b/arch/sparc/include/asm/hardirq_64.h
@@ -10,8 +10,9 @@
 #include 
 
 #define __ARCH_IRQ_STAT
-#define local_softirq_pending() \
-   (*this_cpu_ptr(&__cpu_data.__softirq_pending))
+
+#define local_softirq_pending_ref \
+   __cpu_data.__softirq_pending
 
 void ack_bad_irq(unsigned int irq);
 
-- 
2.7.4

[PATCH 03/10] softirq: Turn default irq_cpustat_t to standard per-cpu

[Frederic Weisbecker]

In order to optimize and consolidate softirq mask accesses, let's
convert the default irq_cpustat_t implementation to per-CPU standard API.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 include/linux/irq_cpustat.h | 4 ++--
 kernel/softirq.c| 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq_cpustat.h b/include/linux/irq_cpustat.h
index 4954948..ddea03c 100644
--- a/include/linux/irq_cpustat.h
+++ b/include/linux/irq_cpustat.h
@@ -18,8 +18,8 @@
  */
 
 #ifndef __ARCH_IRQ_STAT
-extern irq_cpustat_t irq_stat[];   /* defined in asm/hardirq.h */
-#define __IRQ_STAT(cpu, member)(irq_stat[cpu].member)
+DECLARE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);  /* defined in 
asm/hardirq.h */
+#define __IRQ_STAT(cpu, member)(per_cpu(irq_stat.member, cpu))
 #endif
 
   /* arch independent irq_stat fields */
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 177de36..c5fafd7 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -49,8 +49,8 @@
  */
 
 #ifndef __ARCH_IRQ_STAT
-irq_cpustat_t irq_stat[NR_CPUS] cacheline_aligned;
-EXPORT_SYMBOL(irq_stat);
+DEFINE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);
+EXPORT_PER_CPU_SYMBOL(irq_stat);
 #endif
 
 static struct softirq_action softirq_vec[NR_SOFTIRQS] 
__cacheline_aligned_in_smp;
-- 
2.7.4

[PATCH 03/10] softirq: Turn default irq_cpustat_t to standard per-cpu

[Frederic Weisbecker]

In order to optimize and consolidate softirq mask accesses, let's
convert the default irq_cpustat_t implementation to per-CPU standard API.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 include/linux/irq_cpustat.h | 4 ++--
 kernel/softirq.c| 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq_cpustat.h b/include/linux/irq_cpustat.h
index 4954948..ddea03c 100644
--- a/include/linux/irq_cpustat.h
+++ b/include/linux/irq_cpustat.h
@@ -18,8 +18,8 @@
  */
 
 #ifndef __ARCH_IRQ_STAT
-extern irq_cpustat_t irq_stat[];   /* defined in asm/hardirq.h */
-#define __IRQ_STAT(cpu, member)(irq_stat[cpu].member)
+DECLARE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);  /* defined in 
asm/hardirq.h */
+#define __IRQ_STAT(cpu, member)(per_cpu(irq_stat.member, cpu))
 #endif
 
   /* arch independent irq_stat fields */
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 177de36..c5fafd7 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -49,8 +49,8 @@
  */
 
 #ifndef __ARCH_IRQ_STAT
-irq_cpustat_t irq_stat[NR_CPUS] cacheline_aligned;
-EXPORT_SYMBOL(irq_stat);
+DEFINE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);
+EXPORT_PER_CPU_SYMBOL(irq_stat);
 #endif
 
 static struct softirq_action softirq_vec[NR_SOFTIRQS] 
__cacheline_aligned_in_smp;
-- 
2.7.4

[GIT PULL] softirq: Consolidate and optimize softirq mask v3

[Frederic Weisbecker]

Thomas, Ingo,

Please pull the softirq/mask-v3 branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
softirq/mask-v3

HEAD: e225bb16aa5eb0ab83ddd0de423aca3057b1cfee

(No change since last version, just a rebase against v4.17-rc1 and
add acks from Michael Ellerman)

--
The softirq mask and its accessors/mutators have many implementations
scattered around many architectures. Most do the same things consisting
in a field in a per-cpu struct (often irq_cpustat_t) accessed through
per-cpu ops. We can provide instead a generic efficient version that
most of them can use. In fact s390 is the only exception because the
field is stored in lowcore.

Thanks,
Frederic
---

Frederic Weisbecker (10):
  ia64: Convert local_softirq_pending() to per-cpu ops
  sparc: Convert local_softirq_pending() to use per-cpu op
  softirq: Turn default irq_cpustat_t to standard per-cpu
  softirq: Consolidate default local_softirq_pending() implementations
  ia64: Switch to generic local_softirq_pending() implementation
  parisc: Switch to generic local_softirq_pending() implementation
  powerpc: Switch to generic local_softirq_pending() implementation
  sparc: Switch to generic local_softirq_pending() implementation
  x86: Switch to generic local_softirq_pending() implementation
  softirq/s390: Move default mutators of overwritten softirq mask to s390


 arch/ia64/include/asm/hardirq.h |  2 +-
 arch/parisc/include/asm/hardirq.h   |  8 
 arch/powerpc/include/asm/hardirq.h  |  7 ---
 arch/s390/include/asm/hardirq.h |  2 ++
 arch/sparc/include/asm/hardirq_64.h |  5 +++--
 arch/x86/include/asm/hardirq.h  |  8 
 include/linux/interrupt.h   | 13 ++---
 include/linux/irq_cpustat.h | 10 +++---
 kernel/softirq.c|  4 ++--
 9 files changed, 21 insertions(+), 38 deletions(-)

[GIT PULL] softirq: Consolidate and optimize softirq mask v3

[Frederic Weisbecker]

Thomas, Ingo,

Please pull the softirq/mask-v3 branch that can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
softirq/mask-v3

HEAD: e225bb16aa5eb0ab83ddd0de423aca3057b1cfee

(No change since last version, just a rebase against v4.17-rc1 and
add acks from Michael Ellerman)

--
The softirq mask and its accessors/mutators have many implementations
scattered around many architectures. Most do the same things consisting
in a field in a per-cpu struct (often irq_cpustat_t) accessed through
per-cpu ops. We can provide instead a generic efficient version that
most of them can use. In fact s390 is the only exception because the
field is stored in lowcore.

Thanks,
Frederic
---

Frederic Weisbecker (10):
  ia64: Convert local_softirq_pending() to per-cpu ops
  sparc: Convert local_softirq_pending() to use per-cpu op
  softirq: Turn default irq_cpustat_t to standard per-cpu
  softirq: Consolidate default local_softirq_pending() implementations
  ia64: Switch to generic local_softirq_pending() implementation
  parisc: Switch to generic local_softirq_pending() implementation
  powerpc: Switch to generic local_softirq_pending() implementation
  sparc: Switch to generic local_softirq_pending() implementation
  x86: Switch to generic local_softirq_pending() implementation
  softirq/s390: Move default mutators of overwritten softirq mask to s390


 arch/ia64/include/asm/hardirq.h |  2 +-
 arch/parisc/include/asm/hardirq.h   |  8 
 arch/powerpc/include/asm/hardirq.h  |  7 ---
 arch/s390/include/asm/hardirq.h |  2 ++
 arch/sparc/include/asm/hardirq_64.h |  5 +++--
 arch/x86/include/asm/hardirq.h  |  8 
 include/linux/interrupt.h   | 13 ++---
 include/linux/irq_cpustat.h | 10 +++---
 kernel/softirq.c|  4 ++--
 9 files changed, 21 insertions(+), 38 deletions(-)

[PATCH 04/10] softirq: Consolidate default local_softirq_pending() implementations

[Frederic Weisbecker]

Consolidate and optimize default softirq mask API implementations.
Per-CPU operations are expected to be faster and a few architectures
already rely on them to implement local_softirq_pending() and related
accessors/mutators. Those will be migrated to the new generic code.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 include/linux/interrupt.h   | 14 ++
 include/linux/irq_cpustat.h |  6 +-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 5426627..7a11f73 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -432,11 +432,25 @@ extern bool force_irqthreads;
 #define force_irqthreads   (0)
 #endif
 
+#ifndef local_softirq_pending
+
+#ifndef local_softirq_pending_ref
+#define local_softirq_pending_ref irq_stat.__softirq_pending
+#endif
+
+#define local_softirq_pending()
(__this_cpu_read(local_softirq_pending_ref))
+#define set_softirq_pending(x) (__this_cpu_write(local_softirq_pending_ref, 
(x)))
+#define or_softirq_pending(x)  (__this_cpu_or(local_softirq_pending_ref, (x)))
+
+#else /* local_softirq_pending */
+
 #ifndef __ARCH_SET_SOFTIRQ_PENDING
 #define set_softirq_pending(x) (local_softirq_pending() = (x))
 #define or_softirq_pending(x)  (local_softirq_pending() |= (x))
 #endif
 
+#endif /* local_softirq_pending */
+
 /* Some architectures might implement lazy enabling/disabling of
  * interrupts. In some cases, such as stop_machine, we might want
  * to ensure that after a local_irq_disable(), interrupts have
diff --git a/include/linux/irq_cpustat.h b/include/linux/irq_cpustat.h
index ddea03c..6e8895c 100644
--- a/include/linux/irq_cpustat.h
+++ b/include/linux/irq_cpustat.h
@@ -22,11 +22,7 @@ DECLARE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);/* 
defined in asm/hardirq.h */
 #define __IRQ_STAT(cpu, member)(per_cpu(irq_stat.member, cpu))
 #endif
 
-  /* arch independent irq_stat fields */
-#define local_softirq_pending() \
-   __IRQ_STAT(smp_processor_id(), __softirq_pending)
-
-  /* arch dependent irq_stat fields */
+/* arch dependent irq_stat fields */
 #define nmi_count(cpu) __IRQ_STAT((cpu), __nmi_count)  /* i386 */
 
 #endif /* __irq_cpustat_h */
-- 
2.7.4

[PATCH 04/10] softirq: Consolidate default local_softirq_pending() implementations

[Frederic Weisbecker]

Consolidate and optimize default softirq mask API implementations.
Per-CPU operations are expected to be faster and a few architectures
already rely on them to implement local_softirq_pending() and related
accessors/mutators. Those will be migrated to the new generic code.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 include/linux/interrupt.h   | 14 ++
 include/linux/irq_cpustat.h |  6 +-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 5426627..7a11f73 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -432,11 +432,25 @@ extern bool force_irqthreads;
 #define force_irqthreads   (0)
 #endif
 
+#ifndef local_softirq_pending
+
+#ifndef local_softirq_pending_ref
+#define local_softirq_pending_ref irq_stat.__softirq_pending
+#endif
+
+#define local_softirq_pending()
(__this_cpu_read(local_softirq_pending_ref))
+#define set_softirq_pending(x) (__this_cpu_write(local_softirq_pending_ref, 
(x)))
+#define or_softirq_pending(x)  (__this_cpu_or(local_softirq_pending_ref, (x)))
+
+#else /* local_softirq_pending */
+
 #ifndef __ARCH_SET_SOFTIRQ_PENDING
 #define set_softirq_pending(x) (local_softirq_pending() = (x))
 #define or_softirq_pending(x)  (local_softirq_pending() |= (x))
 #endif
 
+#endif /* local_softirq_pending */
+
 /* Some architectures might implement lazy enabling/disabling of
  * interrupts. In some cases, such as stop_machine, we might want
  * to ensure that after a local_irq_disable(), interrupts have
diff --git a/include/linux/irq_cpustat.h b/include/linux/irq_cpustat.h
index ddea03c..6e8895c 100644
--- a/include/linux/irq_cpustat.h
+++ b/include/linux/irq_cpustat.h
@@ -22,11 +22,7 @@ DECLARE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);/* 
defined in asm/hardirq.h */
 #define __IRQ_STAT(cpu, member)(per_cpu(irq_stat.member, cpu))
 #endif
 
-  /* arch independent irq_stat fields */
-#define local_softirq_pending() \
-   __IRQ_STAT(smp_processor_id(), __softirq_pending)
-
-  /* arch dependent irq_stat fields */
+/* arch dependent irq_stat fields */
 #define nmi_count(cpu) __IRQ_STAT((cpu), __nmi_count)  /* i386 */
 
 #endif /* __irq_cpustat_h */
-- 
2.7.4

[PATCH 02/10] sparc: Convert local_softirq_pending() to use per-cpu op

[Frederic Weisbecker]

In order to consolidate and optimize generic softirq mask accesses, we
first need to convert architectures to use per-cpu operations when
possible.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/sparc/include/asm/hardirq_64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sparc/include/asm/hardirq_64.h 
b/arch/sparc/include/asm/hardirq_64.h
index f565402..6aba904 100644
--- a/arch/sparc/include/asm/hardirq_64.h
+++ b/arch/sparc/include/asm/hardirq_64.h
@@ -11,7 +11,7 @@
 
 #define __ARCH_IRQ_STAT
 #define local_softirq_pending() \
-   (local_cpu_data().__softirq_pending)
+   (*this_cpu_ptr(&__cpu_data.__softirq_pending))
 
 void ack_bad_irq(unsigned int irq);
 
-- 
2.7.4

[PATCH 02/10] sparc: Convert local_softirq_pending() to use per-cpu op

[Frederic Weisbecker]

In order to consolidate and optimize generic softirq mask accesses, we
first need to convert architectures to use per-cpu operations when
possible.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/sparc/include/asm/hardirq_64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sparc/include/asm/hardirq_64.h 
b/arch/sparc/include/asm/hardirq_64.h
index f565402..6aba904 100644
--- a/arch/sparc/include/asm/hardirq_64.h
+++ b/arch/sparc/include/asm/hardirq_64.h
@@ -11,7 +11,7 @@
 
 #define __ARCH_IRQ_STAT
 #define local_softirq_pending() \
-   (local_cpu_data().__softirq_pending)
+   (*this_cpu_ptr(&__cpu_data.__softirq_pending))
 
 void ack_bad_irq(unsigned int irq);
 
-- 
2.7.4

[PATCH 01/10] ia64: Convert local_softirq_pending() to per-cpu ops

[Frederic Weisbecker]

In order to consolidate and optimize generic softirq mask accesses, we
first need to convert architectures to use per-cpu operations when
possible.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/ia64/include/asm/hardirq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/hardirq.h b/arch/ia64/include/asm/hardirq.h
index bdc4669..22fae71 100644
--- a/arch/ia64/include/asm/hardirq.h
+++ b/arch/ia64/include/asm/hardirq.h
@@ -13,7 +13,7 @@
 
 #define __ARCH_IRQ_STAT1
 
-#define local_softirq_pending()
(local_cpu_data->softirq_pending)
+#define local_softirq_pending()
(*this_cpu_ptr(_cpu_info.softirq_pending))
 
 #include 
 #include 
-- 
2.7.4

[PATCH 01/10] ia64: Convert local_softirq_pending() to per-cpu ops

[Frederic Weisbecker]

In order to consolidate and optimize generic softirq mask accesses, we
first need to convert architectures to use per-cpu operations when
possible.

Signed-off-by: Frederic Weisbecker 
Cc: Thomas Gleixner 
Cc: Peter Zijlstra 
Cc: Ingo Molnar 
Cc: Sebastian Andrzej Siewior 
Cc: David S. Miller 
Cc: Benjamin Herrenschmidt 
Cc: Paul Mackerras 
Cc: Michael Ellerman 
Cc: James E.J. Bottomley 
Cc: Helge Deller 
Cc: Tony Luck 
Cc: Fenghua Yu 
Cc: Martin Schwidefsky 
Cc: Heiko Carstens 
---
 arch/ia64/include/asm/hardirq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/hardirq.h b/arch/ia64/include/asm/hardirq.h
index bdc4669..22fae71 100644
--- a/arch/ia64/include/asm/hardirq.h
+++ b/arch/ia64/include/asm/hardirq.h
@@ -13,7 +13,7 @@
 
 #define __ARCH_IRQ_STAT1
 
-#define local_softirq_pending()
(local_cpu_data->softirq_pending)
+#define local_softirq_pending()
(*this_cpu_ptr(_cpu_info.softirq_pending))
 
 #include 
 #include 
-- 
2.7.4

Re: [RFC 00/13] arm64: allwinner: Add A64 DE2 pipeline support

[Maxime Ripard]

On Tue, Apr 24, 2018 at 07:04:12PM +0530, Jagan Teki wrote:
> Allwinner A64 has display engine pipeline like other Allwinner SOC's 
> A83T/H3/H5.
> 
> A64 DE2 behaviour similar to Allwinner A83T where mixer0, connected to tcon0 
> with
> RGB, LVDS MIPI-DSI and mixer1, connected to tcon1 with HDMI.
> This series merely concentrated on HDMI pipeline and rest will add eventually.
> 
> patch 1: dt-bindings for a64 DE2 CCU
> 
> patch 2: a64 DE2 CCU node addition
> 
> patch 3: dt-bindings for a64 DE2 pipeline
> 
> patch 4 - 5: dt-bindings for a64 mixer0 and tcon-lcd
> 
> patch 6: a64 DE2 pipeline node addition
> 
> patch 7 - 8: dt-bindings for a64 HDMI and HDMI PHY
> 
> patch 9: a64 HDMI nodes addition
> 
> patch 10 - 11: dt-bindings for a64 mixer1 and tcon-tv
> 
> patch 12: a64 HDMI pipeline
> 
> patch 13: enable HDMI out on bananpi-m64
> 
> Tested HDMI on bananapi-m64 (along with DE2 SRAM C changes from [1]
> thread), able to detect the HDMI but, no penguins on screen.
> 
> Request for any suggestions.
> 
> Test log on Bananpi-m64:
> [0.247631] sun4i-drm display-engine: bound 110.mixer (ops 
> sun8i_mixer_ops)
> [0.256717] sun4i-drm display-engine: bound 120.mixer (ops 
> sun8i_mixer_ops)
> [0.256783] sun4i-tcon 1c0c000.lcd-controller: Missing LVDS properties, 
> Please upgrade your DT
> [0.256792] sun4i-tcon 1c0c000.lcd-controller: LVDS output disabled

That doesn't seem to work so well for LVDS.

> [0.257081] sun4i-drm display-engine: No panel or bridge found... RGB 
> output disabled
> [0.257099] sun4i-drm display-engine: bound 1c0c000.lcd-controller (ops 
> sun4i_tcon_ops)
> [0.257273] sun4i-drm display-engine: No panel or bridge found... RGB 
> output disabled
> [0.257288] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops 
> sun4i_tcon_ops)
> [0.258176] sun8i-dw-hdmi 1ee.hdmi: Detected HDMI TX controller v1.32a 
> with HDCP (sun8i_dw_hdmi_p)
> [0.258596] sun8i-dw-hdmi 1ee.hdmi: registered DesignWare HDMI I2C bus 
> driver
> [0.259188] sun4i-drm display-engine: bound 1ee.hdmi (ops 
> sun8i_dw_hdmi_ops)
> [0.259199] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [0.259205] [drm] No driver support for vblank timestamp query.
> [0.259308] [drm] Cannot find any crtc or sizes

A good guess would be that you can't get the EDIDs for some
reason. Have you tried forcing a mode to see if the display part
already works?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: [RFC 00/13] arm64: allwinner: Add A64 DE2 pipeline support

[Maxime Ripard]

On Tue, Apr 24, 2018 at 07:04:12PM +0530, Jagan Teki wrote:
> Allwinner A64 has display engine pipeline like other Allwinner SOC's 
> A83T/H3/H5.
> 
> A64 DE2 behaviour similar to Allwinner A83T where mixer0, connected to tcon0 
> with
> RGB, LVDS MIPI-DSI and mixer1, connected to tcon1 with HDMI.
> This series merely concentrated on HDMI pipeline and rest will add eventually.
> 
> patch 1: dt-bindings for a64 DE2 CCU
> 
> patch 2: a64 DE2 CCU node addition
> 
> patch 3: dt-bindings for a64 DE2 pipeline
> 
> patch 4 - 5: dt-bindings for a64 mixer0 and tcon-lcd
> 
> patch 6: a64 DE2 pipeline node addition
> 
> patch 7 - 8: dt-bindings for a64 HDMI and HDMI PHY
> 
> patch 9: a64 HDMI nodes addition
> 
> patch 10 - 11: dt-bindings for a64 mixer1 and tcon-tv
> 
> patch 12: a64 HDMI pipeline
> 
> patch 13: enable HDMI out on bananpi-m64
> 
> Tested HDMI on bananapi-m64 (along with DE2 SRAM C changes from [1]
> thread), able to detect the HDMI but, no penguins on screen.
> 
> Request for any suggestions.
> 
> Test log on Bananpi-m64:
> [0.247631] sun4i-drm display-engine: bound 110.mixer (ops 
> sun8i_mixer_ops)
> [0.256717] sun4i-drm display-engine: bound 120.mixer (ops 
> sun8i_mixer_ops)
> [0.256783] sun4i-tcon 1c0c000.lcd-controller: Missing LVDS properties, 
> Please upgrade your DT
> [0.256792] sun4i-tcon 1c0c000.lcd-controller: LVDS output disabled

That doesn't seem to work so well for LVDS.

> [0.257081] sun4i-drm display-engine: No panel or bridge found... RGB 
> output disabled
> [0.257099] sun4i-drm display-engine: bound 1c0c000.lcd-controller (ops 
> sun4i_tcon_ops)
> [0.257273] sun4i-drm display-engine: No panel or bridge found... RGB 
> output disabled
> [0.257288] sun4i-drm display-engine: bound 1c0d000.lcd-controller (ops 
> sun4i_tcon_ops)
> [0.258176] sun8i-dw-hdmi 1ee.hdmi: Detected HDMI TX controller v1.32a 
> with HDCP (sun8i_dw_hdmi_p)
> [0.258596] sun8i-dw-hdmi 1ee.hdmi: registered DesignWare HDMI I2C bus 
> driver
> [0.259188] sun4i-drm display-engine: bound 1ee.hdmi (ops 
> sun8i_dw_hdmi_ops)
> [0.259199] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013).
> [0.259205] [drm] No driver support for vblank timestamp query.
> [0.259308] [drm] Cannot find any crtc or sizes

A good guess would be that you can't get the EDIDs for some
reason. Have you tried forcing a mode to see if the display part
already works?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: Early timeouts due to inaccurate jiffies during system suspend/resume

[Imre Deak]

On Mon, Apr 23, 2018 at 08:01:28PM +0300, Imre Deak wrote:
> On Thu, Apr 19, 2018 at 01:05:39PM +0200, Thomas Gleixner wrote:
> > On Thu, 19 Apr 2018, Imre Deak wrote:
> > > Hi,
> > > 
> > > while checking bug [1], I noticed that jiffies based timing loops like
> > > 
> > >   expire = jiffies + timeout + 1;
> > >   while (!time_after(jiffies, expire))
> > >   do_something;
> > > 
> > > can last shorter than expected (that is less than timeout).
> > 
> > Yes, that can happen when the timer interrupt is delayed long enough for
> > whatever reason. If you need accurate timing then you need to use
> > ktime_get().
> 
> Thanks. I always regarded jiffies as non-accurate, but something that
> gives a minimum time delay guarantee (when adjusted by +1 as above). I
> wonder if there are other callers in kernel that don't expect an early
> timeout.

msleep and any other schedule_timeout based waits are also affected. At the
same time for example msleep's documentation says:
"msleep - sleep safely even with waitqueue interruptions".

To me that suggests a wait with a minimum guaranteed delay.

Ville had an idea to make the behavior more deterministic by clamping
the jiffies increment to 1 for each timer interrupt. Would that work?

> 
> We switched now to using ktime_get_raw() in the i915 driver.
> 
> > 
> > > After some ftracing it seems like jiffies gets stale due to a missed
> > > LAPIC timer interrupt after the interrupt is armed in
> > > lapic_next_deadline() and before jiffies is sampled at 2. above.
> > > Eventually the interrupt does get delivered, at which point jiffies gets
> > > updated via tick_do_update_jiffies64() with a >1 ticks increment.
> > > Between lapic_next_deadline() and the - late - delivery of the interrupt
> > > the CPU on which the interrupt is armed doesn't go idle.
> > 
> > That's odd. I have no real explanation for that.
> 
> Looks like the reason is IRQ latency. For reference here are the
> longest ones I found with irqsoff ftracing, all running with IRQs disabled
> during system resume:
> 
> hpet_rtc_interrupt()->hpet_rtc_timer_reinit():
> do { ... } while(!hpet_cnt_ahead(...));
> takes sometimes up to ~40msec for me.
> 
> hpet_rtc_interrupt()->mc146818_get_time():
> if (mc146818_is_updating()) mdelay(20);
> 
> driver_probe_device->atkbd_connect()->i8042_port_close()->__i8042_command()->i8042_wait_write():
> takes sometimes up to ~10msec for me.
> 
> All the above paired with asynchronous calling of the drivers' resume
> hooks may result in the jumps in jiffies I saw.
> 
> --Imre

Re: Early timeouts due to inaccurate jiffies during system suspend/resume

[Imre Deak]

On Mon, Apr 23, 2018 at 08:01:28PM +0300, Imre Deak wrote:
> On Thu, Apr 19, 2018 at 01:05:39PM +0200, Thomas Gleixner wrote:
> > On Thu, 19 Apr 2018, Imre Deak wrote:
> > > Hi,
> > > 
> > > while checking bug [1], I noticed that jiffies based timing loops like
> > > 
> > >   expire = jiffies + timeout + 1;
> > >   while (!time_after(jiffies, expire))
> > >   do_something;
> > > 
> > > can last shorter than expected (that is less than timeout).
> > 
> > Yes, that can happen when the timer interrupt is delayed long enough for
> > whatever reason. If you need accurate timing then you need to use
> > ktime_get().
> 
> Thanks. I always regarded jiffies as non-accurate, but something that
> gives a minimum time delay guarantee (when adjusted by +1 as above). I
> wonder if there are other callers in kernel that don't expect an early
> timeout.

msleep and any other schedule_timeout based waits are also affected. At the
same time for example msleep's documentation says:
"msleep - sleep safely even with waitqueue interruptions".

To me that suggests a wait with a minimum guaranteed delay.

Ville had an idea to make the behavior more deterministic by clamping
the jiffies increment to 1 for each timer interrupt. Would that work?

> 
> We switched now to using ktime_get_raw() in the i915 driver.
> 
> > 
> > > After some ftracing it seems like jiffies gets stale due to a missed
> > > LAPIC timer interrupt after the interrupt is armed in
> > > lapic_next_deadline() and before jiffies is sampled at 2. above.
> > > Eventually the interrupt does get delivered, at which point jiffies gets
> > > updated via tick_do_update_jiffies64() with a >1 ticks increment.
> > > Between lapic_next_deadline() and the - late - delivery of the interrupt
> > > the CPU on which the interrupt is armed doesn't go idle.
> > 
> > That's odd. I have no real explanation for that.
> 
> Looks like the reason is IRQ latency. For reference here are the
> longest ones I found with irqsoff ftracing, all running with IRQs disabled
> during system resume:
> 
> hpet_rtc_interrupt()->hpet_rtc_timer_reinit():
> do { ... } while(!hpet_cnt_ahead(...));
> takes sometimes up to ~40msec for me.
> 
> hpet_rtc_interrupt()->mc146818_get_time():
> if (mc146818_is_updating()) mdelay(20);
> 
> driver_probe_device->atkbd_connect()->i8042_port_close()->__i8042_command()->i8042_wait_write():
> takes sometimes up to ~10msec for me.
> 
> All the above paired with asynchronous calling of the drivers' resume
> hooks may result in the jumps in jiffies I saw.
> 
> --Imre

RE: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Alan Douglas]

Hi Kishon,

On 24 April 2018 10:36 Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 24/04/2018 08:07, Kishon Vijay Abraham I wrote:
> > Hi,
> >
> > On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote:
> >> Hi Kishon,
> >>
> >> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
> >>> Hi Gustavo,
> >>>
> >>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>  Changes the pcie_raise_irq function signature, namely the
>  interrupt_num variable type from u8 to u16 to accommodate the MSI-X
>  maximum interrupts of 2048.
> 
>  Implements a PCIe config space capability iterator function to
>  search and save the MSI and MSI-X pointers. With this method the
>  code becomes more generic and flexible.
> 
>  Implements MSI-X set/get functions for sysfs interface in order to
>  change the EP entries number.
> 
>  Implements EP MSI-X interface for triggering interruptions.
> 
>  Signed-off-by: Gustavo Pimentel 
>  ---
>   drivers/pci/dwc/pci-dra7xx.c   |   2 +-
>   drivers/pci/dwc/pcie-artpec6.c |   2 +-
>   drivers/pci/dwc/pcie-designware-ep.c   | 145
> -
>   drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>   drivers/pci/dwc/pcie-designware.h  |  23 +-
>   5 files changed, 173 insertions(+), 5 deletions(-)
> 
>  diff --git a/drivers/pci/dwc/pci-dra7xx.c
>  b/drivers/pci/dwc/pci-dra7xx.c index ed8558d..5265725 100644
>  --- a/drivers/pci/dwc/pci-dra7xx.c
>  +++ b/drivers/pci/dwc/pci-dra7xx.c
>  @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct
>  dra7xx_pcie *dra7xx,  }
> 
>   static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  - enum pci_epc_irq_type type, u8
> interrupt_num)
>  + enum pci_epc_irq_type type, u16
> interrupt_num)
>   {
>   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>   struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci); diff --git
>  a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
>  index e66cede..96dc259 100644
>  --- a/drivers/pci/dwc/pcie-artpec6.c
>  +++ b/drivers/pci/dwc/pcie-artpec6.c
>  @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct
>  dw_pcie_ep *ep)  }
> 
>   static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  -  enum pci_epc_irq_type type, u8
> interrupt_num)
>  +  enum pci_epc_irq_type type, u16
> interrupt_num)
>   {
>   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> 
>  diff --git a/drivers/pci/dwc/pcie-designware-ep.c
>  b/drivers/pci/dwc/pcie-designware-ep.c
>  index 15b22a6..874d4c2 100644
>  --- a/drivers/pci/dwc/pcie-designware-ep.c
>  +++ b/drivers/pci/dwc/pcie-designware-ep.c
>  @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie
> *pci, enum pci_barno bar)
>   __dw_pcie_ep_reset_bar(pci, bar, 0);  }
> 
>  +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep) {
> >>>
> >>> This should be implemented in a generic way similar to
> pci_find_capability().
> >>> It'll be useful when we try to implement other capabilities as well.
> >>
> >> Hum, what you suggest? Something implemented on the pci-epf-core?
> >
> > yeah, Initially thought it could be implemented as a helper function
> > in pci-epc-core so that both designware and cadence can use it.
> 
> That would be nice, however I couldn't find out how to access the config
> space, through the pci_epf or pci_epc structs.
> 
> So, I reworked the functions like this:
> 
> (on pcie-designware-ep.c)
> 
> u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
>   u8 cap)
> {
> u8 cap_id, next_cap_ptr;
> u16 reg;
> 
> reg = dw_pcie_readw_dbi(pci, cap_ptr);
> next_cap_ptr = (reg & 0xff00) >> 8;
> cap_id = (reg & 0x00ff);
> 
> if (!next_cap_ptr || cap_id > PCI_CAP_ID_MAX)
> return 0;
> 
> if (cap_id == cap)
> return cap_ptr;
> 
> return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); }
> 
> u8 dw_pcie_ep_find_capability(struct dw_pcie *pci, u8 cap) {
> u8 next_cap_ptr;
> u16 reg;
> 
> reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
> next_cap_ptr = (reg & 0x00ff);
> 
> if (!next_cap_ptr)
> return 0;
> 
> return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); }
> 
> int dw_pcie_ep_init(struct dw_pcie_ep *ep) { [...]
> ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
> ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); [...]
> }
> 
> >
> > But do we really have to find the address like this? since all
> > designware IP's will 

RE: [RFC 01/10] PCI: dwc: Add MSI-X callbacks handler

[Alan Douglas]

Hi Kishon,

On 24 April 2018 10:36 Gustavo Pimentel wrote:
> Hi Kishon,
> 
> On 24/04/2018 08:07, Kishon Vijay Abraham I wrote:
> > Hi,
> >
> > On Monday 23 April 2018 03:06 PM, Gustavo Pimentel wrote:
> >> Hi Kishon,
> >>
> >> On 16/04/2018 10:29, Kishon Vijay Abraham I wrote:
> >>> Hi Gustavo,
> >>>
> >>> On Tuesday 10 April 2018 10:44 PM, Gustavo Pimentel wrote:
>  Changes the pcie_raise_irq function signature, namely the
>  interrupt_num variable type from u8 to u16 to accommodate the MSI-X
>  maximum interrupts of 2048.
> 
>  Implements a PCIe config space capability iterator function to
>  search and save the MSI and MSI-X pointers. With this method the
>  code becomes more generic and flexible.
> 
>  Implements MSI-X set/get functions for sysfs interface in order to
>  change the EP entries number.
> 
>  Implements EP MSI-X interface for triggering interruptions.
> 
>  Signed-off-by: Gustavo Pimentel 
>  ---
>   drivers/pci/dwc/pci-dra7xx.c   |   2 +-
>   drivers/pci/dwc/pcie-artpec6.c |   2 +-
>   drivers/pci/dwc/pcie-designware-ep.c   | 145
> -
>   drivers/pci/dwc/pcie-designware-plat.c |   6 +-
>   drivers/pci/dwc/pcie-designware.h  |  23 +-
>   5 files changed, 173 insertions(+), 5 deletions(-)
> 
>  diff --git a/drivers/pci/dwc/pci-dra7xx.c
>  b/drivers/pci/dwc/pci-dra7xx.c index ed8558d..5265725 100644
>  --- a/drivers/pci/dwc/pci-dra7xx.c
>  +++ b/drivers/pci/dwc/pci-dra7xx.c
>  @@ -369,7 +369,7 @@ static void dra7xx_pcie_raise_msi_irq(struct
>  dra7xx_pcie *dra7xx,  }
> 
>   static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  - enum pci_epc_irq_type type, u8
> interrupt_num)
>  + enum pci_epc_irq_type type, u16
> interrupt_num)
>   {
>   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>   struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci); diff --git
>  a/drivers/pci/dwc/pcie-artpec6.c b/drivers/pci/dwc/pcie-artpec6.c
>  index e66cede..96dc259 100644
>  --- a/drivers/pci/dwc/pcie-artpec6.c
>  +++ b/drivers/pci/dwc/pcie-artpec6.c
>  @@ -428,7 +428,7 @@ static void artpec6_pcie_ep_init(struct
>  dw_pcie_ep *ep)  }
> 
>   static int artpec6_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
>  -  enum pci_epc_irq_type type, u8
> interrupt_num)
>  +  enum pci_epc_irq_type type, u16
> interrupt_num)
>   {
>   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> 
>  diff --git a/drivers/pci/dwc/pcie-designware-ep.c
>  b/drivers/pci/dwc/pcie-designware-ep.c
>  index 15b22a6..874d4c2 100644
>  --- a/drivers/pci/dwc/pcie-designware-ep.c
>  +++ b/drivers/pci/dwc/pcie-designware-ep.c
>  @@ -40,6 +40,44 @@ void dw_pcie_ep_reset_bar(struct dw_pcie
> *pci, enum pci_barno bar)
>   __dw_pcie_ep_reset_bar(pci, bar, 0);  }
> 
>  +void dw_pcie_ep_find_cap_addr(struct dw_pcie_ep *ep) {
> >>>
> >>> This should be implemented in a generic way similar to
> pci_find_capability().
> >>> It'll be useful when we try to implement other capabilities as well.
> >>
> >> Hum, what you suggest? Something implemented on the pci-epf-core?
> >
> > yeah, Initially thought it could be implemented as a helper function
> > in pci-epc-core so that both designware and cadence can use it.
> 
> That would be nice, however I couldn't find out how to access the config
> space, through the pci_epf or pci_epc structs.
> 
> So, I reworked the functions like this:
> 
> (on pcie-designware-ep.c)
> 
> u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8 cap_ptr,
>   u8 cap)
> {
> u8 cap_id, next_cap_ptr;
> u16 reg;
> 
> reg = dw_pcie_readw_dbi(pci, cap_ptr);
> next_cap_ptr = (reg & 0xff00) >> 8;
> cap_id = (reg & 0x00ff);
> 
> if (!next_cap_ptr || cap_id > PCI_CAP_ID_MAX)
> return 0;
> 
> if (cap_id == cap)
> return cap_ptr;
> 
> return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); }
> 
> u8 dw_pcie_ep_find_capability(struct dw_pcie *pci, u8 cap) {
> u8 next_cap_ptr;
> u16 reg;
> 
> reg = dw_pcie_readw_dbi(pci, PCI_CAPABILITY_LIST);
> next_cap_ptr = (reg & 0x00ff);
> 
> if (!next_cap_ptr)
> return 0;
> 
> return __dw_pcie_ep_find_next_cap(pci, next_cap_ptr, cap); }
> 
> int dw_pcie_ep_init(struct dw_pcie_ep *ep) { [...]
> ep->msi_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSI);
> ep->msix_cap = dw_pcie_ep_find_capability(pci, PCI_CAP_ID_MSIX); [...]
> }
> 
> >
> > But do we really have to find the address like this? since all
> > designware IP's will have a particular capability at a 

Re: [PATCH 1/2] clk: honor CLK_MUX_ROUND_CLOSEST in generic clk mux

[Ezequiel Garcia]

On 9 April 2018 at 10:59, Jerome Brunet  wrote:
>
> CLK_MUX_ROUND_CLOSEST is part of the clk_mux documentation but clk_mux
> directly calls __clk_mux_determine_rate(), which overrides the flag.
> As result, if clk_mux is instantiated with CLK_MUX_ROUND_CLOSEST, the
> flag will be ignored and the clock rounded down.
>
> To solve this, this patch expose clk_mux_determine_rate_flags() in the
> clk-provider API and uses it in the determine_rate() callback of clk_mux.
>
> Fixes: 15a02c1f6dd7 ("clk: Add __clk_mux_determine_rate_closest")
> Signed-off-by: Jerome Brunet 
> ---
>  drivers/clk/clk-mux.c| 10 +-
>  drivers/clk/clk.c|  7 ---
>  include/linux/clk-provider.h |  3 +++
>  3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index ac4a042f8658..1628b93655ed 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -112,10 +112,18 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 
> index)
> return 0;
>  }
>
> +static int clk_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> +   struct clk_mux *mux = to_clk_mux(hw);
> +
> +   return clk_mux_determine_rate_flags(hw, req, mux->flags);
> +}
> +
>  const struct clk_ops clk_mux_ops = {
> .get_parent = clk_mux_get_parent,
> .set_parent = clk_mux_set_parent,
> -   .determine_rate = __clk_mux_determine_rate,
> +   .determine_rate = clk_mux_determine_rate,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ops);
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ea67ac81c6f9..7af555f0e60c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -426,9 +426,9 @@ static bool mux_is_better_rate(unsigned long rate, 
> unsigned long now,
> return now <= rate && now > best;
>  }
>
> -static int
> -clk_mux_determine_rate_flags(struct clk_hw *hw, struct clk_rate_request *req,
> -unsigned long flags)
> +int clk_mux_determine_rate_flags(struct clk_hw *hw,
> +struct clk_rate_request *req,
> +unsigned long flags)
>  {
> struct clk_core *core = hw->core, *parent, *best_parent = NULL;
> int i, num_parents, ret;
> @@ -488,6 +488,7 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, struct 
> clk_rate_request *req,
>
> return 0;
>  }
> +EXPORT_SYMBOL_GPL(clk_mux_determine_rate_flags);
>

Why do we need to export this?

>  struct clk *__clk_lookup(const char *name)
>  {
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 210a890008f9..1d25e149c1c5 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -765,6 +765,9 @@ int __clk_mux_determine_rate(struct clk_hw *hw,
>  int __clk_determine_rate(struct clk_hw *core, struct clk_rate_request *req);
>  int __clk_mux_determine_rate_closest(struct clk_hw *hw,
>  struct clk_rate_request *req);
> +int clk_mux_determine_rate_flags(struct clk_hw *hw,
> +struct clk_rate_request *req,
> +unsigned long flags);
>  void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent);
>  void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
>unsigned long max_rate);
> --
> 2.14.3
>

Reviewed-by: Ezequiel Garcia 

Thanks!
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar

Re: [PATCH 1/2] clk: honor CLK_MUX_ROUND_CLOSEST in generic clk mux

[Ezequiel Garcia]

On 9 April 2018 at 10:59, Jerome Brunet  wrote:
>
> CLK_MUX_ROUND_CLOSEST is part of the clk_mux documentation but clk_mux
> directly calls __clk_mux_determine_rate(), which overrides the flag.
> As result, if clk_mux is instantiated with CLK_MUX_ROUND_CLOSEST, the
> flag will be ignored and the clock rounded down.
>
> To solve this, this patch expose clk_mux_determine_rate_flags() in the
> clk-provider API and uses it in the determine_rate() callback of clk_mux.
>
> Fixes: 15a02c1f6dd7 ("clk: Add __clk_mux_determine_rate_closest")
> Signed-off-by: Jerome Brunet 
> ---
>  drivers/clk/clk-mux.c| 10 +-
>  drivers/clk/clk.c|  7 ---
>  include/linux/clk-provider.h |  3 +++
>  3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index ac4a042f8658..1628b93655ed 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -112,10 +112,18 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 
> index)
> return 0;
>  }
>
> +static int clk_mux_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> +   struct clk_mux *mux = to_clk_mux(hw);
> +
> +   return clk_mux_determine_rate_flags(hw, req, mux->flags);
> +}
> +
>  const struct clk_ops clk_mux_ops = {
> .get_parent = clk_mux_get_parent,
> .set_parent = clk_mux_set_parent,
> -   .determine_rate = __clk_mux_determine_rate,
> +   .determine_rate = clk_mux_determine_rate,
>  };
>  EXPORT_SYMBOL_GPL(clk_mux_ops);
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index ea67ac81c6f9..7af555f0e60c 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -426,9 +426,9 @@ static bool mux_is_better_rate(unsigned long rate, 
> unsigned long now,
> return now <= rate && now > best;
>  }
>
> -static int
> -clk_mux_determine_rate_flags(struct clk_hw *hw, struct clk_rate_request *req,
> -unsigned long flags)
> +int clk_mux_determine_rate_flags(struct clk_hw *hw,
> +struct clk_rate_request *req,
> +unsigned long flags)
>  {
> struct clk_core *core = hw->core, *parent, *best_parent = NULL;
> int i, num_parents, ret;
> @@ -488,6 +488,7 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, struct 
> clk_rate_request *req,
>
> return 0;
>  }
> +EXPORT_SYMBOL_GPL(clk_mux_determine_rate_flags);
>

Why do we need to export this?

>  struct clk *__clk_lookup(const char *name)
>  {
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 210a890008f9..1d25e149c1c5 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -765,6 +765,9 @@ int __clk_mux_determine_rate(struct clk_hw *hw,
>  int __clk_determine_rate(struct clk_hw *core, struct clk_rate_request *req);
>  int __clk_mux_determine_rate_closest(struct clk_hw *hw,
>  struct clk_rate_request *req);
> +int clk_mux_determine_rate_flags(struct clk_hw *hw,
> +struct clk_rate_request *req,
> +unsigned long flags);
>  void clk_hw_reparent(struct clk_hw *hw, struct clk_hw *new_parent);
>  void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
>unsigned long max_rate);
> --
> 2.14.3
>

Reviewed-by: Ezequiel Garcia 

Thanks!
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar