Re: [PATCH 4/9] virtio_pci: simplify MSI-X setup

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

Try to grab the MSI-X vectors early and fall back to the shared one
before doing lots of allocations.

Signed-off-by: Christoph Hellwig 
---


Reviewed-by: Jason Wang 


  drivers/virtio/virtio_pci_common.c | 58 +++---
  1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c 
b/drivers/virtio/virtio_pci_common.c
index 9c4ad7d3f..74ff74c0 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -142,32 +142,39 @@ void vp_del_vqs(struct virtio_device *vdev)
  }
  
  static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,

- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[],
- bool per_vq_vectors)
+   struct virtqueue *vqs[], vq_callback_t *callbacks[],
+   const char * const names[])
  {
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
const char *name = dev_name(_dev->vdev.dev);
int i, err = -ENOMEM, allocated_vectors, nvectors;
+   bool shared = false;
u16 msix_vec;
  
-	if (per_vq_vectors) {

-   /* Best option: one for change interrupt, one per vq. */
-   nvectors = 1;
-   for (i = 0; i < nvqs; ++i)
-   if (callbacks[i])
-   ++nvectors;
-   } else {
-   /* Second best: one for change, shared for all vqs. */
+   nvectors = 1;
+   for (i = 0; i < nvqs; i++) {
+   if (callbacks[i])
+   nvectors++;
+   }
+
+   /* Try one vector per queue first. */
+   err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
+   PCI_IRQ_MSIX);
+   if (err < 0) {
+   /* Fallback to one vector for config, one shared for queues. */
+   shared = true;
nvectors = 2;
+   err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
+   PCI_IRQ_MSIX);
+   if (err < 0)
+   return err;
}
  
  	vp_dev->msix_vectors = nvectors;

vp_dev->msix_names = kmalloc_array(nvectors,
sizeof(*vp_dev->msix_names), GFP_KERNEL);
if (!vp_dev->msix_names)
-   return err;
+   goto out_free_irq_vectors;
  
  	vp_dev->msix_affinity_masks = kcalloc(nvectors,

sizeof(*vp_dev->msix_affinity_masks), GFP_KERNEL);
@@ -180,18 +187,13 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, 
unsigned nvqs,
goto out_free_msix_affinity_masks;
}
  
-	err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,

-   PCI_IRQ_MSIX);
-   if (err < 0)
-   goto out_free_msix_affinity_masks;
-
/* Set the vector used for configuration */
snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
 "%s-config", name);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
0, vp_dev->msix_names[0], vp_dev);
if (err)
-   goto out_free_irq_vectors;
+   goto out_free_msix_affinity_masks;
  
  	/* Verify we had enough resources to assign the vector */

if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
@@ -241,7 +243,11 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, 
unsigned nvqs,
}
vp_dev->msix_vector_map[i] = msix_vec;
  
-		if (per_vq_vectors)

+   /*
+* Use a different vector for each queue if they are available,
+* else share the same vector for all VQs.
+*/
+   if (!shared)
allocated_vectors++;
}
  
@@ -254,8 +260,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,

vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
  out_free_config_irq:
free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
-out_free_irq_vectors:
-   pci_free_irq_vectors(vp_dev->pci_dev);
  out_free_msix_affinity_masks:
for (i = 0; i < nvectors; i++) {
if (vp_dev->msix_affinity_masks[i])
@@ -264,6 +268,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, 
unsigned nvqs,
kfree(vp_dev->msix_affinity_masks);
  out_free_msix_names:
kfree(vp_dev->msix_names);
+out_free_irq_vectors:
+   pci_free_irq_vectors(vp_dev->pci_dev);
return err;
  }
  
@@ -308,15 +314,9 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,

  {
int err;
  
-	/* Try MSI-X with one vector per queue. */

-   err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, 

Re: [PATCH 4/9] virtio_pci: simplify MSI-X setup

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

Try to grab the MSI-X vectors early and fall back to the shared one
before doing lots of allocations.

Signed-off-by: Christoph Hellwig 
---


Reviewed-by: Jason Wang 


  drivers/virtio/virtio_pci_common.c | 58 +++---
  1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c 
b/drivers/virtio/virtio_pci_common.c
index 9c4ad7d3f..74ff74c0 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -142,32 +142,39 @@ void vp_del_vqs(struct virtio_device *vdev)
  }
  
  static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,

- struct virtqueue *vqs[],
- vq_callback_t *callbacks[],
- const char * const names[],
- bool per_vq_vectors)
+   struct virtqueue *vqs[], vq_callback_t *callbacks[],
+   const char * const names[])
  {
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
const char *name = dev_name(_dev->vdev.dev);
int i, err = -ENOMEM, allocated_vectors, nvectors;
+   bool shared = false;
u16 msix_vec;
  
-	if (per_vq_vectors) {

-   /* Best option: one for change interrupt, one per vq. */
-   nvectors = 1;
-   for (i = 0; i < nvqs; ++i)
-   if (callbacks[i])
-   ++nvectors;
-   } else {
-   /* Second best: one for change, shared for all vqs. */
+   nvectors = 1;
+   for (i = 0; i < nvqs; i++) {
+   if (callbacks[i])
+   nvectors++;
+   }
+
+   /* Try one vector per queue first. */
+   err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
+   PCI_IRQ_MSIX);
+   if (err < 0) {
+   /* Fallback to one vector for config, one shared for queues. */
+   shared = true;
nvectors = 2;
+   err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,
+   PCI_IRQ_MSIX);
+   if (err < 0)
+   return err;
}
  
  	vp_dev->msix_vectors = nvectors;

vp_dev->msix_names = kmalloc_array(nvectors,
sizeof(*vp_dev->msix_names), GFP_KERNEL);
if (!vp_dev->msix_names)
-   return err;
+   goto out_free_irq_vectors;
  
  	vp_dev->msix_affinity_masks = kcalloc(nvectors,

sizeof(*vp_dev->msix_affinity_masks), GFP_KERNEL);
@@ -180,18 +187,13 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, 
unsigned nvqs,
goto out_free_msix_affinity_masks;
}
  
-	err = pci_alloc_irq_vectors(vp_dev->pci_dev, nvectors, nvectors,

-   PCI_IRQ_MSIX);
-   if (err < 0)
-   goto out_free_msix_affinity_masks;
-
/* Set the vector used for configuration */
snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
 "%s-config", name);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
0, vp_dev->msix_names[0], vp_dev);
if (err)
-   goto out_free_irq_vectors;
+   goto out_free_msix_affinity_masks;
  
  	/* Verify we had enough resources to assign the vector */

if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
@@ -241,7 +243,11 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, 
unsigned nvqs,
}
vp_dev->msix_vector_map[i] = msix_vec;
  
-		if (per_vq_vectors)

+   /*
+* Use a different vector for each queue if they are available,
+* else share the same vector for all VQs.
+*/
+   if (!shared)
allocated_vectors++;
}
  
@@ -254,8 +260,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,

vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
  out_free_config_irq:
free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
-out_free_irq_vectors:
-   pci_free_irq_vectors(vp_dev->pci_dev);
  out_free_msix_affinity_masks:
for (i = 0; i < nvectors; i++) {
if (vp_dev->msix_affinity_masks[i])
@@ -264,6 +268,8 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, 
unsigned nvqs,
kfree(vp_dev->msix_affinity_masks);
  out_free_msix_names:
kfree(vp_dev->msix_names);
+out_free_irq_vectors:
+   pci_free_irq_vectors(vp_dev->pci_dev);
return err;
  }
  
@@ -308,15 +314,9 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs,

  {
int err;
  
-	/* Try MSI-X with one vector per queue. */

-   err = vp_find_vqs_msix(vdev, nvqs, vqs, callbacks, names, true);
-   if (!err)
-  

Re: [PATCH 3/9] virtio_pci: don't duplicate the msix_enable flag in struct pci_dev

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

Signed-off-by: Christoph Hellwig 
---


Reviewed-by: Jason Wang 


  drivers/virtio/virtio_pci_common.c | 5 ++---
  drivers/virtio/virtio_pci_common.h | 2 --
  drivers/virtio/virtio_pci_legacy.c | 2 +-
  drivers/virtio/virtio_pci_modern.c | 2 +-
  include/uapi/linux/virtio_pci.h| 2 +-
  5 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c 
b/drivers/virtio/virtio_pci_common.c
index 5880e86..9c4ad7d3f 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -125,7 +125,7 @@ void vp_del_vqs(struct virtio_device *vdev)
  
  	vp_remove_vqs(vdev);
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
for (i = 0; i < vp_dev->msix_vectors; i++)
free_cpumask_var(vp_dev->msix_affinity_masks[i]);
  
@@ -245,7 +245,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,

allocated_vectors++;
}
  
-	vp_dev->msix_enabled = 1;

return 0;
  
  out_remove_vqs:

@@ -341,7 +340,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
if (!vq->callback)
return -EINVAL;
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
int vec = vp_dev->msix_vector_map[vq->index];
struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
diff --git a/drivers/virtio/virtio_pci_common.h 
b/drivers/virtio/virtio_pci_common.h
index 8559386..217ca87 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -64,8 +64,6 @@ struct virtio_pci_device {
/* the IO mapping for the PCI config space */
void __iomem *ioaddr;
  
-	/* MSI-X support */

-   int msix_enabled;
cpumask_var_t *msix_affinity_masks;
/* Name strings for interrupts. This size should be enough,
 * and I'm too lazy to allocate each name separately. */
diff --git a/drivers/virtio/virtio_pci_legacy.c 
b/drivers/virtio/virtio_pci_legacy.c
index 47292da..2ab6aee 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -165,7 +165,7 @@ static void del_vq(struct virtqueue *vq)
  
  	iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
iowrite16(VIRTIO_MSI_NO_VECTOR,
  vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
/* Flush the write out to device */
diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c
index 00e6fc1..e5ce310 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -412,7 +412,7 @@ static void del_vq(struct virtqueue *vq)
  
  	vp_iowrite16(vq->index, _dev->common->queue_select);
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
 _dev->common->queue_msix_vector);
/* Flush the write out to device */
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 90007a1..15b4385 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -79,7 +79,7 @@
   * configuration space */
  #define VIRTIO_PCI_CONFIG_OFF(msix_enabled)   ((msix_enabled) ? 24 : 20)
  /* Deprecated: please use VIRTIO_PCI_CONFIG_OFF instead */
-#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->msix_enabled)
+#define VIRTIO_PCI_CONFIG(dev) 
VIRTIO_PCI_CONFIG_OFF((dev)->pci_dev->msix_enabled)
  
  /* Virtio ABI version, this must match exactly */

  #define VIRTIO_PCI_ABI_VERSION0

Re: [PATCH 3/9] virtio_pci: don't duplicate the msix_enable flag in struct pci_dev

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

Signed-off-by: Christoph Hellwig 
---


Reviewed-by: Jason Wang 


  drivers/virtio/virtio_pci_common.c | 5 ++---
  drivers/virtio/virtio_pci_common.h | 2 --
  drivers/virtio/virtio_pci_legacy.c | 2 +-
  drivers/virtio/virtio_pci_modern.c | 2 +-
  include/uapi/linux/virtio_pci.h| 2 +-
  5 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c 
b/drivers/virtio/virtio_pci_common.c
index 5880e86..9c4ad7d3f 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -125,7 +125,7 @@ void vp_del_vqs(struct virtio_device *vdev)
  
  	vp_remove_vqs(vdev);
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
for (i = 0; i < vp_dev->msix_vectors; i++)
free_cpumask_var(vp_dev->msix_affinity_masks[i]);
  
@@ -245,7 +245,6 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,

allocated_vectors++;
}
  
-	vp_dev->msix_enabled = 1;

return 0;
  
  out_remove_vqs:

@@ -341,7 +340,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
if (!vq->callback)
return -EINVAL;
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
int vec = vp_dev->msix_vector_map[vq->index];
struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
diff --git a/drivers/virtio/virtio_pci_common.h 
b/drivers/virtio/virtio_pci_common.h
index 8559386..217ca87 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -64,8 +64,6 @@ struct virtio_pci_device {
/* the IO mapping for the PCI config space */
void __iomem *ioaddr;
  
-	/* MSI-X support */

-   int msix_enabled;
cpumask_var_t *msix_affinity_masks;
/* Name strings for interrupts. This size should be enough,
 * and I'm too lazy to allocate each name separately. */
diff --git a/drivers/virtio/virtio_pci_legacy.c 
b/drivers/virtio/virtio_pci_legacy.c
index 47292da..2ab6aee 100644
--- a/drivers/virtio/virtio_pci_legacy.c
+++ b/drivers/virtio/virtio_pci_legacy.c
@@ -165,7 +165,7 @@ static void del_vq(struct virtqueue *vq)
  
  	iowrite16(vq->index, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_SEL);
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
iowrite16(VIRTIO_MSI_NO_VECTOR,
  vp_dev->ioaddr + VIRTIO_MSI_QUEUE_VECTOR);
/* Flush the write out to device */
diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c
index 00e6fc1..e5ce310 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -412,7 +412,7 @@ static void del_vq(struct virtqueue *vq)
  
  	vp_iowrite16(vq->index, _dev->common->queue_select);
  
-	if (vp_dev->msix_enabled) {

+   if (vp_dev->pci_dev->msix_enabled) {
vp_iowrite16(VIRTIO_MSI_NO_VECTOR,
 _dev->common->queue_msix_vector);
/* Flush the write out to device */
diff --git a/include/uapi/linux/virtio_pci.h b/include/uapi/linux/virtio_pci.h
index 90007a1..15b4385 100644
--- a/include/uapi/linux/virtio_pci.h
+++ b/include/uapi/linux/virtio_pci.h
@@ -79,7 +79,7 @@
   * configuration space */
  #define VIRTIO_PCI_CONFIG_OFF(msix_enabled)   ((msix_enabled) ? 24 : 20)
  /* Deprecated: please use VIRTIO_PCI_CONFIG_OFF instead */
-#define VIRTIO_PCI_CONFIG(dev) VIRTIO_PCI_CONFIG_OFF((dev)->msix_enabled)
+#define VIRTIO_PCI_CONFIG(dev) 
VIRTIO_PCI_CONFIG_OFF((dev)->pci_dev->msix_enabled)
  
  /* Virtio ABI version, this must match exactly */

  #define VIRTIO_PCI_ABI_VERSION0

Re: [PATCH 1/9] virtio_pci: remove struct virtio_pci_vq_info

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

We don't really need struct virtio_pci_vq_info, as most field in there
are redundant:

  - the vq backpointer is not strictly neede to start with
  - the entry in the vqs list is not needed - the generic virtqueue already
has list, we only need to check if it has a callback to get the same
semantics
  - we can use a simple array to look up the MSI-X vec if needed.
  - That simple array now also duoble serves to replace the per_vq_vectors
flag

Signed-off-by: Christoph Hellwig 
---
  drivers/virtio/virtio_pci_common.c | 117 +++--
  drivers/virtio/virtio_pci_common.h |  25 +---
  drivers/virtio/virtio_pci_legacy.c |   6 +-
  drivers/virtio/virtio_pci_modern.c |   6 +-
  4 files changed, 39 insertions(+), 115 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c 
b/drivers/virtio/virtio_pci_common.c
index 186cbab..a3376731 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -62,16 +62,13 @@ static irqreturn_t vp_config_changed(int irq, void *opaque)
  static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
  {
struct virtio_pci_device *vp_dev = opaque;
-   struct virtio_pci_vq_info *info;
irqreturn_t ret = IRQ_NONE;
-   unsigned long flags;
+   struct virtqueue *vq;
  
-	spin_lock_irqsave(_dev->lock, flags);

-   list_for_each_entry(info, _dev->virtqueues, node) {
-   if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+   list_for_each_entry(vq, _dev->vdev.vqs, list) {
+   if (vq->callback && vring_interrupt(irq, vq) == IRQ_HANDLED)


The check of vq->callback seems redundant, we will check it soon in 
vring_interrupt().


Thanks

Re: [PATCH 1/9] virtio_pci: remove struct virtio_pci_vq_info

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

We don't really need struct virtio_pci_vq_info, as most field in there
are redundant:

  - the vq backpointer is not strictly neede to start with
  - the entry in the vqs list is not needed - the generic virtqueue already
has list, we only need to check if it has a callback to get the same
semantics
  - we can use a simple array to look up the MSI-X vec if needed.
  - That simple array now also duoble serves to replace the per_vq_vectors
flag

Signed-off-by: Christoph Hellwig 
---
  drivers/virtio/virtio_pci_common.c | 117 +++--
  drivers/virtio/virtio_pci_common.h |  25 +---
  drivers/virtio/virtio_pci_legacy.c |   6 +-
  drivers/virtio/virtio_pci_modern.c |   6 +-
  4 files changed, 39 insertions(+), 115 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c 
b/drivers/virtio/virtio_pci_common.c
index 186cbab..a3376731 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -62,16 +62,13 @@ static irqreturn_t vp_config_changed(int irq, void *opaque)
  static irqreturn_t vp_vring_interrupt(int irq, void *opaque)
  {
struct virtio_pci_device *vp_dev = opaque;
-   struct virtio_pci_vq_info *info;
irqreturn_t ret = IRQ_NONE;
-   unsigned long flags;
+   struct virtqueue *vq;
  
-	spin_lock_irqsave(_dev->lock, flags);

-   list_for_each_entry(info, _dev->virtqueues, node) {
-   if (vring_interrupt(irq, info->vq) == IRQ_HANDLED)
+   list_for_each_entry(vq, _dev->vdev.vqs, list) {
+   if (vq->callback && vring_interrupt(irq, vq) == IRQ_HANDLED)


The check of vq->callback seems redundant, we will check it soon in 
vring_interrupt().


Thanks

Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

+   snprintf(vp_dev->msix_names[i + 1],
+sizeof(*vp_dev->msix_names), "%s-%s",
 dev_name(_dev->vdev.dev), names[i]);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
- vring_interrupt, 0,
- vp_dev->msix_names[msix_vec],
- vqs[i]);
+ vring_interrupt, IRQF_SHARED,
+ vp_dev->msix_names[i + 1], vqs[i]);


Do we need to check per_vq_vectors before dereferencing msix_names[i + 1] ?

Thanks

Re: [PATCH 2/9] virtio_pci: use shared interrupts for virtqueues

[Jason Wang]

On 2017年01月27日 16:16, Christoph Hellwig wrote:

+   snprintf(vp_dev->msix_names[i + 1],
+sizeof(*vp_dev->msix_names), "%s-%s",
 dev_name(_dev->vdev.dev), names[i]);
err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
- vring_interrupt, 0,
- vp_dev->msix_names[msix_vec],
- vqs[i]);
+ vring_interrupt, IRQF_SHARED,
+ vp_dev->msix_names[i + 1], vqs[i]);


Do we need to check per_vq_vectors before dereferencing msix_names[i + 1] ?

Thanks

ATTENTION,

[Administrator]

ATTENTION,

Your mailbox has exceeded the storage limit, which is 5 GB as defined by the 
administrator, who is currently running on 10.9GB, you may not be able to send 
or receive new mail until you re-validate your mailbox mail. To revalidate your 
mailbox, send the following information below:

name:
Username:
password:
Confirm Password:
E-mail:
phone:

If you are unable to revalidate your mailbox, your mailbox will be disabled!

Sorry for the inconvenience.
Verification code: en: 006,524
Mail Technical Support© 2017

thank you
Systems Administrator.

ATTENTION,

[Administrator]

ATTENTION,

Your mailbox has exceeded the storage limit, which is 5 GB as defined by the 
administrator, who is currently running on 10.9GB, you may not be able to send 
or receive new mail until you re-validate your mailbox mail. To revalidate your 
mailbox, send the following information below:

name:
Username:
password:
Confirm Password:
E-mail:
phone:

If you are unable to revalidate your mailbox, your mailbox will be disabled!

Sorry for the inconvenience.
Verification code: en: 006,524
Mail Technical Support© 2017

thank you
Systems Administrator.

Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call

[Christoph Hellwig]

On Thu, Feb 02, 2017 at 08:00:52AM -0500, Jeff Layton wrote:
> Yeah, that might work. You could kmalloc the buffer array according to
> the maxsize value. For small ones we could even consider using an on-
> stack buffer.

For the block direct I/O code we defintively want to avoid any
allocations for small I/O a that shows up in the performance numbers.
And we'd like to reuse the on-stack bio_vec so that the defintion of
a small I/O can be as big as possible without blowing up the stack.

Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call

[Christoph Hellwig]

On Thu, Feb 02, 2017 at 08:00:52AM -0500, Jeff Layton wrote:
> Yeah, that might work. You could kmalloc the buffer array according to
> the maxsize value. For small ones we could even consider using an on-
> stack buffer.

For the block direct I/O code we defintively want to avoid any
allocations for small I/O a that shows up in the performance numbers.
And we'd like to reuse the on-stack bio_vec so that the defintion of
a small I/O can be as big as possible without blowing up the stack.

Re: [PATCH 6/7] mm: vmscan: move dirty pages out of the way until they're flushed

[Hillf Danton]

On February 03, 2017 3:20 AM Johannes Weiner wrote: 
> @@ -1063,7 +1063,7 @@ static unsigned long shrink_page_list(struct list_head 
> *page_list,
>   PageReclaim(page) &&
>   test_bit(PGDAT_WRITEBACK, >flags)) {
>   nr_immediate++;
> - goto keep_locked;
> + goto activate_locked;

Out of topic but relevant IMHO, I can't find where it is cleared by grepping:

$ grep -nr PGDAT_WRITEBACK  linux-4.9/mm
linux-4.9/mm/vmscan.c:1019: test_bit(PGDAT_WRITEBACK, >flags)) {
linux-4.9/mm/vmscan.c:1777: set_bit(PGDAT_WRITEBACK, >flags);

It was removed in commit 1d82de618dd 
("mm, vmscan: make kswapd reclaim in terms of nodes")

Is it currently maintained somewhere else, Mel and John?

thanks
Hillf

Re: [PATCH 6/7] mm: vmscan: move dirty pages out of the way until they're flushed

[Hillf Danton]

On February 03, 2017 3:20 AM Johannes Weiner wrote: 
> @@ -1063,7 +1063,7 @@ static unsigned long shrink_page_list(struct list_head 
> *page_list,
>   PageReclaim(page) &&
>   test_bit(PGDAT_WRITEBACK, >flags)) {
>   nr_immediate++;
> - goto keep_locked;
> + goto activate_locked;

Out of topic but relevant IMHO, I can't find where it is cleared by grepping:

$ grep -nr PGDAT_WRITEBACK  linux-4.9/mm
linux-4.9/mm/vmscan.c:1019: test_bit(PGDAT_WRITEBACK, >flags)) {
linux-4.9/mm/vmscan.c:1777: set_bit(PGDAT_WRITEBACK, >flags);

It was removed in commit 1d82de618dd 
("mm, vmscan: make kswapd reclaim in terms of nodes")

Is it currently maintained somewhere else, Mel and John?

thanks
Hillf

RE: READ

[Monica Martinez]

Hello, 

I need your assistance in a deal that will be of mutual benefit for the both of 
us from Camp Stanley, Stationed in Uijeongbu, South Korea. Please get back to 
me for more info. 

Thank you for your time.

Sgt. Monica Martinez.

RE: READ

[Monica Martinez]

Hello, 

I need your assistance in a deal that will be of mutual benefit for the both of 
us from Camp Stanley, Stationed in Uijeongbu, South Korea. Please get back to 
me for more info. 

Thank you for your time.

Sgt. Monica Martinez.

Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call

[Al Viro]

On Thu, Feb 02, 2017 at 08:00:52AM -0500, Jeff Layton wrote:
> > I'm not sure we need to touch any get_user_pages_fast() at all; let it
> > fill a medium-sized array and use that as a buffer.  In particular,
> > I *really* don't like the idea of having the callbacks done in an
> > inconsistent locking environment - sometimes under ->mmap_sem, sometimes
> > not.
> > 
> 
> Yeah, that might work. You could kmalloc the buffer array according to
> the maxsize value. For small ones we could even consider using an on-
> stack buffer.

Yes.  FWIW, I'm not proposing to kill iov_iter_get_pages{,_alloc}()
immediately - the new primitive would initially use the damn thing.
That can be backported without any problems, and conversions would be
one-by-one.  If/when we get to the point where most of the users have
been switched to the new helper, we could try and see if what remains
could be dealt with; if that works, it might be possible to fold
iov_iter_get_pages() into it.  In particular, for ITER_BVEC and
ITER_PIPE we don't need to bother with any intermediate arrays at all, etc.
But that's only after several cycles when everyone is asked to try and use
the new primitive instead of iov_iter_get_pages().

The calling conventions of iov_iter_get_pages() are ugly and I would love to
get rid of them, but doing that in a flagday conversion is a lot of PITA
for no good reason.

Speaking of get_user_pages() and conversions:

get_user_pages() relies upon find_extend_vma() to reject kernel
addresses; the fast side of get_user_pages_fast() doesn't have anything
of that sort in case e.g. HAVE_GENERIC_RCU_GUP.  Sure, usually we have
that verified and rejected earlier anyway, but it's a fairly subtle difference
that doesn't seem to be documented anywhere.

For example, /dev/st* reads and writes (st_read()/st_write()) feed the
address of buffer to get_user_pages_unlocked().  If somebody replaced
those with get_user_pages_fast(), we'd be in trouble as soon as
some code got tricked into using kernel_write() on /dev/st*.

access_ok() in HAVE_GENERIC_RCU_GUP {__,}get_user_pages_fast() obviously
doesn't help in that scenario.  What am I missing here?

Re: [PATCH v3 0/2] iov_iter: allow iov_iter_get_pages_alloc to allocate more pages per call

[Al Viro]

On Thu, Feb 02, 2017 at 08:00:52AM -0500, Jeff Layton wrote:
> > I'm not sure we need to touch any get_user_pages_fast() at all; let it
> > fill a medium-sized array and use that as a buffer.  In particular,
> > I *really* don't like the idea of having the callbacks done in an
> > inconsistent locking environment - sometimes under ->mmap_sem, sometimes
> > not.
> > 
> 
> Yeah, that might work. You could kmalloc the buffer array according to
> the maxsize value. For small ones we could even consider using an on-
> stack buffer.

Yes.  FWIW, I'm not proposing to kill iov_iter_get_pages{,_alloc}()
immediately - the new primitive would initially use the damn thing.
That can be backported without any problems, and conversions would be
one-by-one.  If/when we get to the point where most of the users have
been switched to the new helper, we could try and see if what remains
could be dealt with; if that works, it might be possible to fold
iov_iter_get_pages() into it.  In particular, for ITER_BVEC and
ITER_PIPE we don't need to bother with any intermediate arrays at all, etc.
But that's only after several cycles when everyone is asked to try and use
the new primitive instead of iov_iter_get_pages().

The calling conventions of iov_iter_get_pages() are ugly and I would love to
get rid of them, but doing that in a flagday conversion is a lot of PITA
for no good reason.

Speaking of get_user_pages() and conversions:

get_user_pages() relies upon find_extend_vma() to reject kernel
addresses; the fast side of get_user_pages_fast() doesn't have anything
of that sort in case e.g. HAVE_GENERIC_RCU_GUP.  Sure, usually we have
that verified and rejected earlier anyway, but it's a fairly subtle difference
that doesn't seem to be documented anywhere.

For example, /dev/st* reads and writes (st_read()/st_write()) feed the
address of buffer to get_user_pages_unlocked().  If somebody replaced
those with get_user_pages_fast(), we'd be in trouble as soon as
some code got tricked into using kernel_write() on /dev/st*.

access_ok() in HAVE_GENERIC_RCU_GUP {__,}get_user_pages_fast() obviously
doesn't help in that scenario.  What am I missing here?

Re: [RFC v2] [ALSA][CONTROL] 1. Added 2 ioctls for reading/writing values of multiple controls in one go (at once) 2. Restructured the code for read/write of a single control

[Takashi Iwai]

On Fri, 03 Feb 2017 06:28:08 +0100,
Satendra Singh Thakur wrote:
> 
> From: satendra singh thakur 
> 
> 1.Added 2 ioctls in alsa driver's control interface
> -Added an ioctl to read values of multiple elements at once
> -Added an ioctl to write values of multiple elements at once
> -In the absence of above ioctls user needs to call N ioctls to
>  read/write value of N elements which requires N context switches
> -Proposed ioctls will allow accessing N elements' values in a single
>  context switch
> -Above mentioned ioctl will be useful for alsa utils such as amixer
>  which reads all controls of given sound card
> 2. Restructured/Combined two functions snd_ctl_elem_read_user and
> snd_ctl_elem_write_user into a single function snd_ctl_elem_rw_user
> -These functions were having most of the code which was similar
>  to each other
> -Thus, there was redundant/duplicate code which was removed
>  and a single function was formed/defined.
> -Removed functions snd_ctl_elem_read_user and snd_ctl_elem_write_user
>  having redundant/duplicate code
> 3. This is version 2 of RFC, here we combine previous 2 patches
>  submitted yesterday (02-Feb) with prefix [RFC] [ALSA][CONTROL]
> -Fixed doxygen comments related warnings
> -Fixed stack frame related warning
> 
> Signed-off-by: Satendra Singh Thakur 

Thanks for the patch.

But, could you answer to the question Clemens posted for v1 patch?
Namely, do N-times context switching really matter?  Does it give the
severe performance issue?

If we have a measured number, this thing is worth to consider.
OTOH, without the actual measurement but "just because it must be
better", it's no good reason for adding a new API/ABI.


thanks,

Takashi

Re: [RFC v2] [ALSA][CONTROL] 1. Added 2 ioctls for reading/writing values of multiple controls in one go (at once) 2. Restructured the code for read/write of a single control

[Takashi Iwai]

On Fri, 03 Feb 2017 06:28:08 +0100,
Satendra Singh Thakur wrote:
> 
> From: satendra singh thakur 
> 
> 1.Added 2 ioctls in alsa driver's control interface
> -Added an ioctl to read values of multiple elements at once
> -Added an ioctl to write values of multiple elements at once
> -In the absence of above ioctls user needs to call N ioctls to
>  read/write value of N elements which requires N context switches
> -Proposed ioctls will allow accessing N elements' values in a single
>  context switch
> -Above mentioned ioctl will be useful for alsa utils such as amixer
>  which reads all controls of given sound card
> 2. Restructured/Combined two functions snd_ctl_elem_read_user and
> snd_ctl_elem_write_user into a single function snd_ctl_elem_rw_user
> -These functions were having most of the code which was similar
>  to each other
> -Thus, there was redundant/duplicate code which was removed
>  and a single function was formed/defined.
> -Removed functions snd_ctl_elem_read_user and snd_ctl_elem_write_user
>  having redundant/duplicate code
> 3. This is version 2 of RFC, here we combine previous 2 patches
>  submitted yesterday (02-Feb) with prefix [RFC] [ALSA][CONTROL]
> -Fixed doxygen comments related warnings
> -Fixed stack frame related warning
> 
> Signed-off-by: Satendra Singh Thakur 

Thanks for the patch.

But, could you answer to the question Clemens posted for v1 patch?
Namely, do N-times context switching really matter?  Does it give the
severe performance issue?

If we have a measured number, this thing is worth to consider.
OTOH, without the actual measurement but "just because it must be
better", it's no good reason for adding a new API/ABI.


thanks,

Takashi

Re: [RFC] [ALSA][CONTROL] Added 2 ioctls for reading/writing values of multiple controls in one go (at once)

[Takashi Iwai]

On Thu, 02 Feb 2017 04:45:48 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Feb 2 2017 12:14, Satendra Singh Thakur wrote:
> > From: satendra singh thakur 
> >
> > -Added 2 ioctls in alsa driver's control interface
> > -Added an ioctl to read values of multiple elements at once
> > -Added an ioctl to write values of multiple elements at once
> > -In the absence of above ioctls user needs to call N ioctls to
> >  read/write value of N elements which requires N context switches
> > -Proposed ioctls will allow accessing N elements' values in a single
> >  context switch
> > -Above mentioned ioctl will be useful for alsa utils such as amixer
> >  which reads all controls of given sound card
> >
> > Signed-off-by: Satendra Singh Thakur 
> > ---
> >  include/uapi/sound/asound.h |8 +
> >  sound/core/control.c|   69 
> > +++
> >  2 files changed, 77 insertions(+)
> 
> I'm _strongly_ interested in your two patches, because it has a
> potentiality to purge ASoC abuse of TLV feature, which was introduced
> in  2014 with a bad reviewing process.

I don't think it can be a replacement for the extended TLV usages.
The proposed API is nothing but a loop of ctl elem read/write, and I'm
not sure whether it worth to introduce the new ioctls just for that.


Takashi

Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest

[Juergen Gross]

On 26/01/17 20:41, Boris Ostrovsky wrote:
> Start PVH guest at XEN_ELFNOTE_PHYS32_ENTRY address. Setup hypercall
> page, initialize boot_params, enable early page tables.
> 
> Since this stub is executed before kernel entry point we cannot use
> variables in .bss which is cleared by kernel. We explicitly place
> variables that are initialized here into .data.
> 
> Signed-off-by: Boris Ostrovsky 
> ---
> Changes in v2:
> * Assembly cleanup
> * Check for e820 size in init_pvh_bootparams()
> * Check XEN_HVM_START_MAGIC_VALUE in start_info
> 
> 
>  arch/x86/xen/Kconfig |   2 +-
>  arch/x86/xen/Makefile|   1 +
>  arch/x86/xen/enlighten.c |  98 -
>  arch/x86/xen/xen-pvh.S   | 137 
> +++
>  include/xen/xen.h|   5 ++
>  5 files changed, 241 insertions(+), 2 deletions(-)
>  create mode 100644 arch/x86/xen/xen-pvh.S
> 
> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> index c7b15f3..76b6dbd 100644
> --- a/arch/x86/xen/Kconfig
> +++ b/arch/x86/xen/Kconfig
> @@ -53,5 +53,5 @@ config XEN_DEBUG_FS
>  
>  config XEN_PVH
>   bool "Support for running as a PVH guest"
> - depends on X86_64 && XEN && XEN_PVHVM
> + depends on XEN && XEN_PVHVM && ACPI
>   def_bool n
> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
> index e47e527..cb0164a 100644
> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_XEN_DEBUG_FS)  += debugfs.o
>  obj-$(CONFIG_XEN_DOM0)   += vga.o
>  obj-$(CONFIG_SWIOTLB_XEN)+= pci-swiotlb-xen.o
>  obj-$(CONFIG_XEN_EFI)+= efi.o
> +obj-$(CONFIG_XEN_PVH)+= xen-pvh.o
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 828f1b2..c82fe14 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -45,6 +45,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -121,7 +122,8 @@
>  DEFINE_PER_CPU(uint32_t, xen_vcpu_id);
>  EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>  
> -enum xen_domain_type xen_domain_type = XEN_NATIVE;
> +enum xen_domain_type xen_domain_type
> + __attribute__((section(".data"))) = XEN_NATIVE;
>  EXPORT_SYMBOL_GPL(xen_domain_type);
>  
>  unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
> @@ -176,6 +178,17 @@ struct tls_descs {
>   */
>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>  
> +#ifdef CONFIG_XEN_PVH
> +/*
> + * PVH variables. These need to live in data segment since they are
> + * initialized before startup_{32|64}, which clear .bss, are invoked.
> + */
> +bool xen_pvh __attribute__((section(".data"))) = 0;
> +struct hvm_start_info pvh_start_info __attribute__((section(".data")));
> +unsigned int pvh_start_info_sz = sizeof(pvh_start_info);

While I believe this can live in .bss as it isn't used after clearing
.bss there should either be a comment why this is save or you should
attribute it as .data, too.

> +struct boot_params pvh_bootparams __attribute__((section(".data")));
> +#endif
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -1656,6 +1669,89 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  #endif
>  }
>  
> +#ifdef CONFIG_XEN_PVH
> +static void __init init_pvh_bootparams(void)
> +{
> + struct xen_memory_map memmap;
> + unsigned int i;
> + int rc;
> +
> + memset(_bootparams, 0, sizeof(pvh_bootparams));
> +
> + memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_map);
> + set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_map);
> + rc = HYPERVISOR_memory_op(XENMEM_memory_map, );
> + if (rc) {
> + xen_raw_printk("XENMEM_memory_map failed (%d)\n", rc);
> + BUG();
> + }
> +
> + if (memmap.nr_entries < E820MAX) {

Shouldn't this be E820MAX - 1?
What happens if memmap.nr_entries is already
ARRAY_SIZE(pvh_bootparams.e820_map) ?

> + pvh_bootparams.e820_map[memmap.nr_entries].addr =
> + ISA_START_ADDRESS;
> + pvh_bootparams.e820_map[memmap.nr_entries].size =
> + ISA_END_ADDRESS - ISA_START_ADDRESS;
> + pvh_bootparams.e820_map[memmap.nr_entries++].type =
> + E820_RESERVED;

I'd rather split out the '++' to a separate statement.

> + } else
> + xen_raw_printk("Warning: Can fit ISA range into e820\n");
> +
> + sanitize_e820_map(pvh_bootparams.e820_map,
> +   ARRAY_SIZE(pvh_bootparams.e820_map),
> +   _entries);
> +
> + pvh_bootparams.e820_entries = memmap.nr_entries;
> + for (i = 0; i < pvh_bootparams.e820_entries; i++)
> + e820_add_region(pvh_bootparams.e820_map[i].addr,
> + pvh_bootparams.e820_map[i].size,
> + pvh_bootparams.e820_map[i].type);
> +
> + pvh_bootparams.hdr.cmd_line_ptr =
> + 

Re: [RFC] [ALSA][CONTROL] Added 2 ioctls for reading/writing values of multiple controls in one go (at once)

[Takashi Iwai]

On Thu, 02 Feb 2017 04:45:48 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Feb 2 2017 12:14, Satendra Singh Thakur wrote:
> > From: satendra singh thakur 
> >
> > -Added 2 ioctls in alsa driver's control interface
> > -Added an ioctl to read values of multiple elements at once
> > -Added an ioctl to write values of multiple elements at once
> > -In the absence of above ioctls user needs to call N ioctls to
> >  read/write value of N elements which requires N context switches
> > -Proposed ioctls will allow accessing N elements' values in a single
> >  context switch
> > -Above mentioned ioctl will be useful for alsa utils such as amixer
> >  which reads all controls of given sound card
> >
> > Signed-off-by: Satendra Singh Thakur 
> > ---
> >  include/uapi/sound/asound.h |8 +
> >  sound/core/control.c|   69 
> > +++
> >  2 files changed, 77 insertions(+)
> 
> I'm _strongly_ interested in your two patches, because it has a
> potentiality to purge ASoC abuse of TLV feature, which was introduced
> in  2014 with a bad reviewing process.

I don't think it can be a replacement for the extended TLV usages.
The proposed API is nothing but a loop of ctl elem read/write, and I'm
not sure whether it worth to introduce the new ioctls just for that.


Takashi

Re: [PATCH v2 4/9] xen/pvh: Bootstrap PVH guest

[Juergen Gross]

On 26/01/17 20:41, Boris Ostrovsky wrote:
> Start PVH guest at XEN_ELFNOTE_PHYS32_ENTRY address. Setup hypercall
> page, initialize boot_params, enable early page tables.
> 
> Since this stub is executed before kernel entry point we cannot use
> variables in .bss which is cleared by kernel. We explicitly place
> variables that are initialized here into .data.
> 
> Signed-off-by: Boris Ostrovsky 
> ---
> Changes in v2:
> * Assembly cleanup
> * Check for e820 size in init_pvh_bootparams()
> * Check XEN_HVM_START_MAGIC_VALUE in start_info
> 
> 
>  arch/x86/xen/Kconfig |   2 +-
>  arch/x86/xen/Makefile|   1 +
>  arch/x86/xen/enlighten.c |  98 -
>  arch/x86/xen/xen-pvh.S   | 137 
> +++
>  include/xen/xen.h|   5 ++
>  5 files changed, 241 insertions(+), 2 deletions(-)
>  create mode 100644 arch/x86/xen/xen-pvh.S
> 
> diff --git a/arch/x86/xen/Kconfig b/arch/x86/xen/Kconfig
> index c7b15f3..76b6dbd 100644
> --- a/arch/x86/xen/Kconfig
> +++ b/arch/x86/xen/Kconfig
> @@ -53,5 +53,5 @@ config XEN_DEBUG_FS
>  
>  config XEN_PVH
>   bool "Support for running as a PVH guest"
> - depends on X86_64 && XEN && XEN_PVHVM
> + depends on XEN && XEN_PVHVM && ACPI
>   def_bool n
> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
> index e47e527..cb0164a 100644
> --- a/arch/x86/xen/Makefile
> +++ b/arch/x86/xen/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_XEN_DEBUG_FS)  += debugfs.o
>  obj-$(CONFIG_XEN_DOM0)   += vga.o
>  obj-$(CONFIG_SWIOTLB_XEN)+= pci-swiotlb-xen.o
>  obj-$(CONFIG_XEN_EFI)+= efi.o
> +obj-$(CONFIG_XEN_PVH)+= xen-pvh.o
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 828f1b2..c82fe14 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -45,6 +45,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -121,7 +122,8 @@
>  DEFINE_PER_CPU(uint32_t, xen_vcpu_id);
>  EXPORT_PER_CPU_SYMBOL(xen_vcpu_id);
>  
> -enum xen_domain_type xen_domain_type = XEN_NATIVE;
> +enum xen_domain_type xen_domain_type
> + __attribute__((section(".data"))) = XEN_NATIVE;
>  EXPORT_SYMBOL_GPL(xen_domain_type);
>  
>  unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
> @@ -176,6 +178,17 @@ struct tls_descs {
>   */
>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>  
> +#ifdef CONFIG_XEN_PVH
> +/*
> + * PVH variables. These need to live in data segment since they are
> + * initialized before startup_{32|64}, which clear .bss, are invoked.
> + */
> +bool xen_pvh __attribute__((section(".data"))) = 0;
> +struct hvm_start_info pvh_start_info __attribute__((section(".data")));
> +unsigned int pvh_start_info_sz = sizeof(pvh_start_info);

While I believe this can live in .bss as it isn't used after clearing
.bss there should either be a comment why this is save or you should
attribute it as .data, too.

> +struct boot_params pvh_bootparams __attribute__((section(".data")));
> +#endif
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -1656,6 +1669,89 @@ asmlinkage __visible void __init xen_start_kernel(void)
>  #endif
>  }
>  
> +#ifdef CONFIG_XEN_PVH
> +static void __init init_pvh_bootparams(void)
> +{
> + struct xen_memory_map memmap;
> + unsigned int i;
> + int rc;
> +
> + memset(_bootparams, 0, sizeof(pvh_bootparams));
> +
> + memmap.nr_entries = ARRAY_SIZE(pvh_bootparams.e820_map);
> + set_xen_guest_handle(memmap.buffer, pvh_bootparams.e820_map);
> + rc = HYPERVISOR_memory_op(XENMEM_memory_map, );
> + if (rc) {
> + xen_raw_printk("XENMEM_memory_map failed (%d)\n", rc);
> + BUG();
> + }
> +
> + if (memmap.nr_entries < E820MAX) {

Shouldn't this be E820MAX - 1?
What happens if memmap.nr_entries is already
ARRAY_SIZE(pvh_bootparams.e820_map) ?

> + pvh_bootparams.e820_map[memmap.nr_entries].addr =
> + ISA_START_ADDRESS;
> + pvh_bootparams.e820_map[memmap.nr_entries].size =
> + ISA_END_ADDRESS - ISA_START_ADDRESS;
> + pvh_bootparams.e820_map[memmap.nr_entries++].type =
> + E820_RESERVED;

I'd rather split out the '++' to a separate statement.

> + } else
> + xen_raw_printk("Warning: Can fit ISA range into e820\n");
> +
> + sanitize_e820_map(pvh_bootparams.e820_map,
> +   ARRAY_SIZE(pvh_bootparams.e820_map),
> +   _entries);
> +
> + pvh_bootparams.e820_entries = memmap.nr_entries;
> + for (i = 0; i < pvh_bootparams.e820_entries; i++)
> + e820_add_region(pvh_bootparams.e820_map[i].addr,
> + pvh_bootparams.e820_map[i].size,
> + pvh_bootparams.e820_map[i].type);
> +
> + pvh_bootparams.hdr.cmd_line_ptr =
> + 

Re: [PATCH 4.9 00/51] 4.9.8-stable review

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 09:14:30PM -0800, Guenter Roeck wrote:
> On 02/02/2017 10:37 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.9.8 release.
> > There are 51 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sat Feb  4 18:33:31 UTC 2017.
> > Anything received after that time might be too late.
> > 
> 
> Build results:
>   total: 149 pass: 149 fail: 0
> Qemu test results:
>   total: 122 pass: 122 fail: 0
> 
> Details are available at http://kerneltests.org/builders.

Great, thanks for testing these and letting me know.

greg k-h

Re: [PATCH 4.9 00/51] 4.9.8-stable review

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 09:14:30PM -0800, Guenter Roeck wrote:
> On 02/02/2017 10:37 AM, Greg Kroah-Hartman wrote:
> > This is the start of the stable review cycle for the 4.9.8 release.
> > There are 51 patches in this series, all will be posted as a response
> > to this one.  If anyone has any issues with these being applied, please
> > let me know.
> > 
> > Responses should be made by Sat Feb  4 18:33:31 UTC 2017.
> > Anything received after that time might be too late.
> > 
> 
> Build results:
>   total: 149 pass: 149 fail: 0
> Qemu test results:
>   total: 122 pass: 122 fail: 0
> 
> Details are available at http://kerneltests.org/builders.

Great, thanks for testing these and letting me know.

greg k-h

Re: [PATCH 00/10] android: binder: support for domains and scatter-gather.

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 08:56:04PM -0800, John Stultz wrote:
> On Mon, Oct 24, 2016 at 6:20 AM, Martijn Coenen  wrote:
> > Android userspace will start using binder IPC for communication with HAL
> > modules. To clearly separate this IPC domain from the existing framework
> > IPC domain, this patch series adds support for multiple "binder domains".
> > This is implemented by each domain having its own binder context manager,
> > and then having a separate device node per domain.
> >
> > The other change introduced by this series is scatter-gather; currently
> > all objects passed through binder IPC must be serialized in userspace, with
> > with the exception of binder objects and file descriptors. By adding 
> > scatter-
> > gather support for memory buffers, we can avoid the serialization copy,
> > thereby increasing performance for larger transaction sizes.
> >
> > The two patches from Arve are security patches that we've already applied
> > in android-common. I included them in front of the series because my changes
> > touch quite some of that code.
> 
> Hey Greg,
>Curious what the status of these patches are? Did you have any
> outstanding objection, or were you just waiting for them to be resent?
> 
> AOSP is now making use of this feature, so mainline kernels have
> trouble booting w/o it, so it would be good to get merged. :)

The patches that ended up in AOSP are a bit different from this series
from what I remember, which is why I didn't end up applying this series
at the time (it needed more work.)  If you could dig up the latest
versions, I'll be glad to review/merge them as well.

There's also the 'hwbinder' work that should be in AOSP also, that
should get merged too.

thanks,

greg k-h

Re: [PATCH 00/10] android: binder: support for domains and scatter-gather.

[Greg Kroah-Hartman]

On Thu, Feb 02, 2017 at 08:56:04PM -0800, John Stultz wrote:
> On Mon, Oct 24, 2016 at 6:20 AM, Martijn Coenen  wrote:
> > Android userspace will start using binder IPC for communication with HAL
> > modules. To clearly separate this IPC domain from the existing framework
> > IPC domain, this patch series adds support for multiple "binder domains".
> > This is implemented by each domain having its own binder context manager,
> > and then having a separate device node per domain.
> >
> > The other change introduced by this series is scatter-gather; currently
> > all objects passed through binder IPC must be serialized in userspace, with
> > with the exception of binder objects and file descriptors. By adding 
> > scatter-
> > gather support for memory buffers, we can avoid the serialization copy,
> > thereby increasing performance for larger transaction sizes.
> >
> > The two patches from Arve are security patches that we've already applied
> > in android-common. I included them in front of the series because my changes
> > touch quite some of that code.
> 
> Hey Greg,
>Curious what the status of these patches are? Did you have any
> outstanding objection, or were you just waiting for them to be resent?
> 
> AOSP is now making use of this feature, so mainline kernels have
> trouble booting w/o it, so it would be good to get merged. :)

The patches that ended up in AOSP are a bit different from this series
from what I remember, which is why I didn't end up applying this series
at the time (it needed more work.)  If you could dig up the latest
versions, I'll be glad to review/merge them as well.

There's also the 'hwbinder' work that should be in AOSP also, that
should get merged too.

thanks,

greg k-h

Re: [PATCH V2 0/4] efi/x86: move efi bgrt init code to early init

[Dave Young]

On 01/27/17 at 05:03pm, Ard Biesheuvel wrote:
> On 16 January 2017 at 02:45, Dave Young  wrote:
> > Hi,
> >
> > Here the the update of the series for moving bgrt init code to early init.
> >
> > Main changes is:
> > - Move the 1st patch to the last because it does not block the 2nd patch
> > any more with Peter's patch to prune invlid memmap entries:
> > https://git.kernel.org/cgit/linux/kernel/git/efi/efi.git/commit/?h=next=b2a91
> > a35445229
> > But it is still tood to have since efi_mem_reserve only cares about boot 
> > related
> > mem ranges.
> >
> > - Other comments about code itself, details please the the patches 
> > themselves.
> >
> >  arch/x86/include/asm/efi.h   |1
> >  arch/x86/kernel/acpi/boot.c  |   12 +++
> >  arch/x86/platform/efi/efi-bgrt.c |   59 
> > ---
> >  arch/x86/platform/efi/efi.c  |   26 +++--
> >  arch/x86/platform/efi/quirks.c   |2 -
> >  drivers/acpi/bgrt.c  |   28 +-
> >  drivers/firmware/efi/fake_mem.c  |3 +
> >  drivers/firmware/efi/memmap.c|   22 +-
> >  include/linux/efi-bgrt.h |7 +---
> >  include/linux/efi.h  |5 +--
> >  init/main.c  |1
> >  11 files changed, 92 insertions(+), 74 deletions(-)
> >
> 
> Dave,
> 
> I have pushed these to efi/next: please double check if I did it
> correctly. I had some trouble applying these given that I rebased
> efi/next onto -rc4. However, the fact that you are not using standard
> git cover letters and emails doesn't help things, so could you
> *please* use standard git send-email to post to linux-efi in the
> future? Thanks.

Ard, apologize for late reply due to a one week holiday. I double-checked
the efi-next commits, I think they are correct. As for the email format
I use quilt to send the series so that the cover letter is not git
formatted. The patches are based on mainline linus tree, maybe this is
the reason of the trouble I will check if it is efi/next mergeble with
"git am" before sending out next time and will switch to git-send-email
if it does not work.

Thanks
Dave

Re: [PATCH V2 0/4] efi/x86: move efi bgrt init code to early init

[Dave Young]

On 01/27/17 at 05:03pm, Ard Biesheuvel wrote:
> On 16 January 2017 at 02:45, Dave Young  wrote:
> > Hi,
> >
> > Here the the update of the series for moving bgrt init code to early init.
> >
> > Main changes is:
> > - Move the 1st patch to the last because it does not block the 2nd patch
> > any more with Peter's patch to prune invlid memmap entries:
> > https://git.kernel.org/cgit/linux/kernel/git/efi/efi.git/commit/?h=next=b2a91
> > a35445229
> > But it is still tood to have since efi_mem_reserve only cares about boot 
> > related
> > mem ranges.
> >
> > - Other comments about code itself, details please the the patches 
> > themselves.
> >
> >  arch/x86/include/asm/efi.h   |1
> >  arch/x86/kernel/acpi/boot.c  |   12 +++
> >  arch/x86/platform/efi/efi-bgrt.c |   59 
> > ---
> >  arch/x86/platform/efi/efi.c  |   26 +++--
> >  arch/x86/platform/efi/quirks.c   |2 -
> >  drivers/acpi/bgrt.c  |   28 +-
> >  drivers/firmware/efi/fake_mem.c  |3 +
> >  drivers/firmware/efi/memmap.c|   22 +-
> >  include/linux/efi-bgrt.h |7 +---
> >  include/linux/efi.h  |5 +--
> >  init/main.c  |1
> >  11 files changed, 92 insertions(+), 74 deletions(-)
> >
> 
> Dave,
> 
> I have pushed these to efi/next: please double check if I did it
> correctly. I had some trouble applying these given that I rebased
> efi/next onto -rc4. However, the fact that you are not using standard
> git cover letters and emails doesn't help things, so could you
> *please* use standard git send-email to post to linux-efi in the
> future? Thanks.

Ard, apologize for late reply due to a one week holiday. I double-checked
the efi-next commits, I think they are correct. As for the email format
I use quilt to send the series so that the cover letter is not git
formatted. The patches are based on mainline linus tree, maybe this is
the reason of the trouble I will check if it is efi/next mergeble with
"git am" before sending out next time and will switch to git-send-email
if it does not work.

Thanks
Dave

ATTENZIONE;

[amministratore]

ATTENZIONE;

La cassetta postale ha superato il limite di archiviazione, che è 5 GB come 
definiti dall'amministratore, che è attualmente in esecuzione su 10.9GB, non si 
può essere in grado di inviare o ricevere nuovi messaggi fino a ri-convalidare 
la tua mailbox. Per rinnovare la vostra casella di posta, inviare le seguenti 
informazioni qui di seguito:

nome:
Nome utente:
Password:
Conferma Password:
E-mail:
telefono:

Se non si riesce a rinnovare la vostra casella di posta, la vostra
caselladi posta sarà disabilitato!

Ci dispiace per l'inconvenienza.
Codice di verifica: en:0085362LK.ft6789000.2017
Mail Technical Support ©2017

grazie
Sistemi amministratore

ATTENZIONE;

[amministratore]

ATTENZIONE;

La cassetta postale ha superato il limite di archiviazione, che è 5 GB come 
definiti dall'amministratore, che è attualmente in esecuzione su 10.9GB, non si 
può essere in grado di inviare o ricevere nuovi messaggi fino a ri-convalidare 
la tua mailbox. Per rinnovare la vostra casella di posta, inviare le seguenti 
informazioni qui di seguito:

nome:
Nome utente:
Password:
Conferma Password:
E-mail:
telefono:

Se non si riesce a rinnovare la vostra casella di posta, la vostra
caselladi posta sarà disabilitato!

Ci dispiace per l'inconvenienza.
Codice di verifica: en:0085362LK.ft6789000.2017
Mail Technical Support ©2017

grazie
Sistemi amministratore

[PATCH net] ipv6: Fix IPv6 packet loss in scenarios involving roaming + snooping switches

[Linus Lüssing]

When for instance a mobile Linux device roams from one access point to
another with both APs sharing the same broadcast domain and a
multicast snooping switch in between:

1)(c) <~~~> (AP1) <--[SSW]--> (AP2)

2)  (AP1) <--[SSW]--> (AP2) <~~~> (c)

Then currently IPv6 multicast packets will get lost for (c) until an
MLD Querier sends its next query message. The packet loss occurs
because upon roaming the Linux host so far stayed silent regarding
MLD and the snooping switch will therefore be unaware of the
multicast topology change for a while.

This patch fixes this by always resending MLD reports when an interface
change happens, for instance from NO-CARRIER to CARRIER state.

Signed-off-by: Linus Lüssing 

---

Initial problem report was sent to the bridge mailing list a while ago:
- https://lists.linuxfoundation.org/pipermail/bridge/2015-September/009754.html

The RFCs concerning IGMP, MLD and snooping switches seem a have a hole
concerning roaming. A request for clarification to mcast-w...@ietf.org
was left unanswered, unfortunately:
- https://mailarchive.ietf.org/arch/msg/mcast-wifi/Ghn2cGy1oN2ZwG1qaQO9SE13g6g

However, simply resending reports seems to be the straight forward way
to me to fix the issue mentioned above.
---
 net/ipv6/addrconf.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f60e88e..81f7b4e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3386,9 +3386,15 @@ static int addrconf_notify(struct notifier_block *this, 
unsigned long event,
}
 
if (idev) {
-   if (idev->if_flags & IF_READY)
-   /* device is already configured. */
+   if (idev->if_flags & IF_READY) {
+   /* device is already configured -
+* but resend MLD reports, we might
+* have roamed and need to update
+* multicast snooping switches
+*/
+   ipv6_mc_up(idev);
break;
+   }
idev->if_flags |= IF_READY;
}
 
-- 
2.1.4

[PATCH net] ipv6: Fix IPv6 packet loss in scenarios involving roaming + snooping switches

[Linus Lüssing]

When for instance a mobile Linux device roams from one access point to
another with both APs sharing the same broadcast domain and a
multicast snooping switch in between:

1)(c) <~~~> (AP1) <--[SSW]--> (AP2)

2)  (AP1) <--[SSW]--> (AP2) <~~~> (c)

Then currently IPv6 multicast packets will get lost for (c) until an
MLD Querier sends its next query message. The packet loss occurs
because upon roaming the Linux host so far stayed silent regarding
MLD and the snooping switch will therefore be unaware of the
multicast topology change for a while.

This patch fixes this by always resending MLD reports when an interface
change happens, for instance from NO-CARRIER to CARRIER state.

Signed-off-by: Linus Lüssing 

---

Initial problem report was sent to the bridge mailing list a while ago:
- https://lists.linuxfoundation.org/pipermail/bridge/2015-September/009754.html

The RFCs concerning IGMP, MLD and snooping switches seem a have a hole
concerning roaming. A request for clarification to mcast-w...@ietf.org
was left unanswered, unfortunately:
- https://mailarchive.ietf.org/arch/msg/mcast-wifi/Ghn2cGy1oN2ZwG1qaQO9SE13g6g

However, simply resending reports seems to be the straight forward way
to me to fix the issue mentioned above.
---
 net/ipv6/addrconf.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index f60e88e..81f7b4e 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3386,9 +3386,15 @@ static int addrconf_notify(struct notifier_block *this, 
unsigned long event,
}
 
if (idev) {
-   if (idev->if_flags & IF_READY)
-   /* device is already configured. */
+   if (idev->if_flags & IF_READY) {
+   /* device is already configured -
+* but resend MLD reports, we might
+* have roamed and need to update
+* multicast snooping switches
+*/
+   ipv6_mc_up(idev);
break;
+   }
idev->if_flags |= IF_READY;
}
 
-- 
2.1.4

Re: [PATCH 1/4] selftests, x86, protection_keys: fix unused variable compile warnings

[Ingo Molnar]

* Shuah Khan  wrote:

> On 02/02/2017 04:45 PM, Dave Hansen wrote:
> > On 02/02/2017 03:36 PM, Shuah Khan wrote:
> >> This patches is already in linux-kselftest next for 4.11
> >>
> >> Is there a reason why you chose to resend these.
> > 
> > Oh, my apologies!  I didn't realize it had been picked up elsewhere.
> > Ingo had mentioned a few times in the last few days that he'd noticed
> > the warnings.
> > 
> > So, x86 maintainers, do we want these to go through the kselftest tree
> > or the x86 tree?
> > 
> 
> I usually let many of the new tests that depend on new features go through
> x86 tree. Patches that fix problems that are already in kselftest, I send
> them up in via kselftest.
> 
> I noticed these when very early on when 4.10-rc1 came out and send the patches
> out. SO they have been in linux-kselftest next for a while now.

That's OK, but please Cc: me (or at least lkml) next time around.

Also, these should have been sent to Linus as fixes once the warnings were 
noticed, not queued up for v4.11 - but it's probably too late for that.

Are you going to pick up the other two fixes as well? They look good to me:

Acked-by: Ingo Molnar 

Thanks,

Ingo

Re: [PATCH 1/4] selftests, x86, protection_keys: fix unused variable compile warnings

[Ingo Molnar]

* Shuah Khan  wrote:

> On 02/02/2017 04:45 PM, Dave Hansen wrote:
> > On 02/02/2017 03:36 PM, Shuah Khan wrote:
> >> This patches is already in linux-kselftest next for 4.11
> >>
> >> Is there a reason why you chose to resend these.
> > 
> > Oh, my apologies!  I didn't realize it had been picked up elsewhere.
> > Ingo had mentioned a few times in the last few days that he'd noticed
> > the warnings.
> > 
> > So, x86 maintainers, do we want these to go through the kselftest tree
> > or the x86 tree?
> > 
> 
> I usually let many of the new tests that depend on new features go through
> x86 tree. Patches that fix problems that are already in kselftest, I send
> them up in via kselftest.
> 
> I noticed these when very early on when 4.10-rc1 came out and send the patches
> out. SO they have been in linux-kselftest next for a while now.

That's OK, but please Cc: me (or at least lkml) next time around.

Also, these should have been sent to Linus as fixes once the warnings were 
noticed, not queued up for v4.11 - but it's probably too late for that.

Are you going to pick up the other two fixes as well? They look good to me:

Acked-by: Ingo Molnar 

Thanks,

Ingo

Re: [PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 03일 15:38에 Inki Dae 이(가) 쓴 글:
> 
> 
> 2017년 02월 01일 17:29에 Andrzej Hajda 이(가) 쓴 글:
>> On TM2/TM2e platforms HDMI output is connected to MHL bridge
>> SiI8620. To allow configure UltraHD modes on the bridge
>> and to eliminate unsupported modes this bridge should be
>> attached to drm_encoder implemented in exynos_hdmi.
>>
>> Signed-off-by: Andrzej Hajda 
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 
>> +---
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index a73b192..41fb894 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -35,6 +35,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -133,6 +134,7 @@ struct hdmi_context {
>>  struct regulator_bulk_data  regul_bulk[ARRAY_SIZE(supply)];
>>  struct regulator*reg_hdmi_en;
>>  struct exynos_drm_clk   phy_clk;
>> +struct drm_bridge   *bridge;
>>  };
>>  
>>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
>> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder 
>> *encoder)
>>  drm_connector_register(connector);
>>  drm_mode_connector_attach_encoder(connector, encoder);
>>  
>> -return 0;
>> +if (hdata->bridge) {
>> +encoder->bridge = hdata->bridge;
>> +hdata->bridge->encoder = encoder;
>> +ret = drm_bridge_attach(encoder->dev, hdata->bridge);
> 
> arguments of drm_bridge_attach function has been changed so fixed it - 
> trivial thing.
> Applied it including other patches.

The argument was changed by below patch,
drm: bridge: Link encoder and bridge in core code

Thanks.

> 
> Thanks,
> Inki Dae
> 
>> +if (ret)
>> +DRM_ERROR("Failed to attach bridge\n");
>> +}
>> +
>> +return ret;
>>  }
>>  
>>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
>> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk 
>> *clk, bool enable)
>>  hdmiphy_disable(hdata);
>>  }
>>  
>> +static int hdmi_bridge_init(struct hdmi_context *hdata)
>> +{
>> +struct device *dev = hdata->dev;
>> +struct device_node *ep, *np;
>> +
>> +ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>> +if (!ep)
>> +return 0;
>> +
>> +np = of_graph_get_remote_port_parent(ep);
>> +of_node_put(ep);
>> +if (!np) {
>> +DRM_ERROR("failed to get remote port parent");
>> +return -EINVAL;
>> +}
>> +
>> +hdata->bridge = of_drm_find_bridge(np);
>> +of_node_put(np);
>> +
>> +if (!hdata->bridge)
>> +return -EPROBE_DEFER;
>> +
>> +return 0;
>> +}
>> +
>>  static int hdmi_resources_init(struct hdmi_context *hdata)
>>  {
>>  struct device *dev = hdata->dev;
>> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context 
>> *hdata)
>>  
>>  hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>>  
>> -if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
>> -return 0;
>> +if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
>> +if (IS_ERR(hdata->reg_hdmi_en))
>> +return PTR_ERR(hdata->reg_hdmi_en);
>>  
>> -if (IS_ERR(hdata->reg_hdmi_en))
>> -return PTR_ERR(hdata->reg_hdmi_en);
>> -
>> -ret = regulator_enable(hdata->reg_hdmi_en);
>> -if (ret)
>> -DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +ret = regulator_enable(hdata->reg_hdmi_en);
>> +if (ret) {
>> +DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +return ret;
>> +}
>> +}
>>  
>> -return ret;
>> +return hdmi_bridge_init(hdata);
>>  }
>>  
>>  static struct of_device_id hdmi_match_types[] = {
>>

Re: [PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 03일 15:38에 Inki Dae 이(가) 쓴 글:
> 
> 
> 2017년 02월 01일 17:29에 Andrzej Hajda 이(가) 쓴 글:
>> On TM2/TM2e platforms HDMI output is connected to MHL bridge
>> SiI8620. To allow configure UltraHD modes on the bridge
>> and to eliminate unsupported modes this bridge should be
>> attached to drm_encoder implemented in exynos_hdmi.
>>
>> Signed-off-by: Andrzej Hajda 
>> ---
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 
>> +---
>>  1 file changed, 46 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index a73b192..41fb894 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -35,6 +35,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -133,6 +134,7 @@ struct hdmi_context {
>>  struct regulator_bulk_data  regul_bulk[ARRAY_SIZE(supply)];
>>  struct regulator*reg_hdmi_en;
>>  struct exynos_drm_clk   phy_clk;
>> +struct drm_bridge   *bridge;
>>  };
>>  
>>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
>> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder 
>> *encoder)
>>  drm_connector_register(connector);
>>  drm_mode_connector_attach_encoder(connector, encoder);
>>  
>> -return 0;
>> +if (hdata->bridge) {
>> +encoder->bridge = hdata->bridge;
>> +hdata->bridge->encoder = encoder;
>> +ret = drm_bridge_attach(encoder->dev, hdata->bridge);
> 
> arguments of drm_bridge_attach function has been changed so fixed it - 
> trivial thing.
> Applied it including other patches.

The argument was changed by below patch,
drm: bridge: Link encoder and bridge in core code

Thanks.

> 
> Thanks,
> Inki Dae
> 
>> +if (ret)
>> +DRM_ERROR("Failed to attach bridge\n");
>> +}
>> +
>> +return ret;
>>  }
>>  
>>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
>> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk 
>> *clk, bool enable)
>>  hdmiphy_disable(hdata);
>>  }
>>  
>> +static int hdmi_bridge_init(struct hdmi_context *hdata)
>> +{
>> +struct device *dev = hdata->dev;
>> +struct device_node *ep, *np;
>> +
>> +ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
>> +if (!ep)
>> +return 0;
>> +
>> +np = of_graph_get_remote_port_parent(ep);
>> +of_node_put(ep);
>> +if (!np) {
>> +DRM_ERROR("failed to get remote port parent");
>> +return -EINVAL;
>> +}
>> +
>> +hdata->bridge = of_drm_find_bridge(np);
>> +of_node_put(np);
>> +
>> +if (!hdata->bridge)
>> +return -EPROBE_DEFER;
>> +
>> +return 0;
>> +}
>> +
>>  static int hdmi_resources_init(struct hdmi_context *hdata)
>>  {
>>  struct device *dev = hdata->dev;
>> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context 
>> *hdata)
>>  
>>  hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>>  
>> -if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
>> -return 0;
>> +if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
>> +if (IS_ERR(hdata->reg_hdmi_en))
>> +return PTR_ERR(hdata->reg_hdmi_en);
>>  
>> -if (IS_ERR(hdata->reg_hdmi_en))
>> -return PTR_ERR(hdata->reg_hdmi_en);
>> -
>> -ret = regulator_enable(hdata->reg_hdmi_en);
>> -if (ret)
>> -DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +ret = regulator_enable(hdata->reg_hdmi_en);
>> +if (ret) {
>> +DRM_ERROR("failed to enable hdmi-en regulator\n");
>> +return ret;
>> +}
>> +}
>>  
>> -return ret;
>> +return hdmi_bridge_init(hdata);
>>  }
>>  
>>  static struct of_device_id hdmi_match_types[] = {
>>

Re: [PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 01일 17:29에 Andrzej Hajda 이(가) 쓴 글:
> On TM2/TM2e platforms HDMI output is connected to MHL bridge
> SiI8620. To allow configure UltraHD modes on the bridge
> and to eliminate unsupported modes this bridge should be
> attached to drm_encoder implemented in exynos_hdmi.
> 
> Signed-off-by: Andrzej Hajda 
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 
> +---
>  1 file changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index a73b192..41fb894 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -133,6 +134,7 @@ struct hdmi_context {
>   struct regulator_bulk_data  regul_bulk[ARRAY_SIZE(supply)];
>   struct regulator*reg_hdmi_en;
>   struct exynos_drm_clk   phy_clk;
> + struct drm_bridge   *bridge;
>  };
>  
>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder 
> *encoder)
>   drm_connector_register(connector);
>   drm_mode_connector_attach_encoder(connector, encoder);
>  
> - return 0;
> + if (hdata->bridge) {
> + encoder->bridge = hdata->bridge;
> + hdata->bridge->encoder = encoder;
> + ret = drm_bridge_attach(encoder->dev, hdata->bridge);

arguments of drm_bridge_attach function has been changed so fixed it - trivial 
thing.
Applied it including other patches.

Thanks,
Inki Dae

> + if (ret)
> + DRM_ERROR("Failed to attach bridge\n");
> + }
> +
> + return ret;
>  }
>  
>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk 
> *clk, bool enable)
>   hdmiphy_disable(hdata);
>  }
>  
> +static int hdmi_bridge_init(struct hdmi_context *hdata)
> +{
> + struct device *dev = hdata->dev;
> + struct device_node *ep, *np;
> +
> + ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> + if (!ep)
> + return 0;
> +
> + np = of_graph_get_remote_port_parent(ep);
> + of_node_put(ep);
> + if (!np) {
> + DRM_ERROR("failed to get remote port parent");
> + return -EINVAL;
> + }
> +
> + hdata->bridge = of_drm_find_bridge(np);
> + of_node_put(np);
> +
> + if (!hdata->bridge)
> + return -EPROBE_DEFER;
> +
> + return 0;
> +}
> +
>  static int hdmi_resources_init(struct hdmi_context *hdata)
>  {
>   struct device *dev = hdata->dev;
> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context 
> *hdata)
>  
>   hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>  
> - if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
> - return 0;
> + if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
> + if (IS_ERR(hdata->reg_hdmi_en))
> + return PTR_ERR(hdata->reg_hdmi_en);
>  
> - if (IS_ERR(hdata->reg_hdmi_en))
> - return PTR_ERR(hdata->reg_hdmi_en);
> -
> - ret = regulator_enable(hdata->reg_hdmi_en);
> - if (ret)
> - DRM_ERROR("failed to enable hdmi-en regulator\n");
> + ret = regulator_enable(hdata->reg_hdmi_en);
> + if (ret) {
> + DRM_ERROR("failed to enable hdmi-en regulator\n");
> + return ret;
> + }
> + }
>  
> - return ret;
> + return hdmi_bridge_init(hdata);
>  }
>  
>  static struct of_device_id hdmi_match_types[] = {
> 

Re: [PATCH v3 4/7] drm/exynos/hdmi: add bridge support

[Inki Dae]

2017년 02월 01일 17:29에 Andrzej Hajda 이(가) 쓴 글:
> On TM2/TM2e platforms HDMI output is connected to MHL bridge
> SiI8620. To allow configure UltraHD modes on the bridge
> and to eliminate unsupported modes this bridge should be
> attached to drm_encoder implemented in exynos_hdmi.
> 
> Signed-off-by: Andrzej Hajda 
> ---
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 56 
> +---
>  1 file changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c 
> b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index a73b192..41fb894 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -133,6 +134,7 @@ struct hdmi_context {
>   struct regulator_bulk_data  regul_bulk[ARRAY_SIZE(supply)];
>   struct regulator*reg_hdmi_en;
>   struct exynos_drm_clk   phy_clk;
> + struct drm_bridge   *bridge;
>  };
>  
>  static inline struct hdmi_context *encoder_to_hdmi(struct drm_encoder *e)
> @@ -922,7 +924,15 @@ static int hdmi_create_connector(struct drm_encoder 
> *encoder)
>   drm_connector_register(connector);
>   drm_mode_connector_attach_encoder(connector, encoder);
>  
> - return 0;
> + if (hdata->bridge) {
> + encoder->bridge = hdata->bridge;
> + hdata->bridge->encoder = encoder;
> + ret = drm_bridge_attach(encoder->dev, hdata->bridge);

arguments of drm_bridge_attach function has been changed so fixed it - trivial 
thing.
Applied it including other patches.

Thanks,
Inki Dae

> + if (ret)
> + DRM_ERROR("Failed to attach bridge\n");
> + }
> +
> + return ret;
>  }
>  
>  static bool hdmi_mode_fixup(struct drm_encoder *encoder,
> @@ -1591,6 +1601,31 @@ static void hdmiphy_clk_enable(struct exynos_drm_clk 
> *clk, bool enable)
>   hdmiphy_disable(hdata);
>  }
>  
> +static int hdmi_bridge_init(struct hdmi_context *hdata)
> +{
> + struct device *dev = hdata->dev;
> + struct device_node *ep, *np;
> +
> + ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, -1);
> + if (!ep)
> + return 0;
> +
> + np = of_graph_get_remote_port_parent(ep);
> + of_node_put(ep);
> + if (!np) {
> + DRM_ERROR("failed to get remote port parent");
> + return -EINVAL;
> + }
> +
> + hdata->bridge = of_drm_find_bridge(np);
> + of_node_put(np);
> +
> + if (!hdata->bridge)
> + return -EPROBE_DEFER;
> +
> + return 0;
> +}
> +
>  static int hdmi_resources_init(struct hdmi_context *hdata)
>  {
>   struct device *dev = hdata->dev;
> @@ -1630,17 +1665,18 @@ static int hdmi_resources_init(struct hdmi_context 
> *hdata)
>  
>   hdata->reg_hdmi_en = devm_regulator_get_optional(dev, "hdmi-en");
>  
> - if (PTR_ERR(hdata->reg_hdmi_en) == -ENODEV)
> - return 0;
> + if (PTR_ERR(hdata->reg_hdmi_en) != -ENODEV) {
> + if (IS_ERR(hdata->reg_hdmi_en))
> + return PTR_ERR(hdata->reg_hdmi_en);
>  
> - if (IS_ERR(hdata->reg_hdmi_en))
> - return PTR_ERR(hdata->reg_hdmi_en);
> -
> - ret = regulator_enable(hdata->reg_hdmi_en);
> - if (ret)
> - DRM_ERROR("failed to enable hdmi-en regulator\n");
> + ret = regulator_enable(hdata->reg_hdmi_en);
> + if (ret) {
> + DRM_ERROR("failed to enable hdmi-en regulator\n");
> + return ret;
> + }
> + }
>  
> - return ret;
> + return hdmi_bridge_init(hdata);
>  }
>  
>  static struct of_device_id hdmi_match_types[] = {
> 

Re: [PATCH 1/2 v3] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

On Thu, Feb 02, 2017 at 11:44:22AM +0100, Michal Hocko wrote:
> On Tue 31-01-17 14:32:08, Vinayak Menon wrote:
> > During global reclaim, the nr_reclaimed passed to vmpressure
> > includes the pages reclaimed from slab. But the corresponding
> > scanned slab pages is not passed. This can cause total reclaimed
> > pages to be greater than scanned, causing an unsigned underflow
> > in vmpressure resulting in a critical event being sent to root
> > cgroup. So do not consider reclaimed slab pages for vmpressure
> > calculation. The reclaimed pages from slab can be excluded because
> > the freeing of a page by slab shrinking depends on each slab's
> > object population, making the cost model (i.e. scan:free) different
> > from that of LRU.
> 
> This might be true but what happens if the slab reclaim contributes
> significantly to the overal reclaim? This would be quite rare but not
> impossible.

Of course, it is better for vmpressure to cover slab but it's not
easy without page-based shrinking model, I think. It wold make
vmpressure higher easily due to low reclaim efficiency compared to
LRU pages. Yeah, vmpressure is not a perfect but no need to add
more noises, either. It's regression since 6b4f7799c6a5 so I think
this patch should go first and if someone want to cover slab really,
he should spend a time to work it well. It's too much that Vinayak
shuld make a effort for that.

Re: [PATCH 1/2 v3] mm: vmscan: do not pass reclaimed slab to vmpressure

[Minchan Kim]

On Thu, Feb 02, 2017 at 11:44:22AM +0100, Michal Hocko wrote:
> On Tue 31-01-17 14:32:08, Vinayak Menon wrote:
> > During global reclaim, the nr_reclaimed passed to vmpressure
> > includes the pages reclaimed from slab. But the corresponding
> > scanned slab pages is not passed. This can cause total reclaimed
> > pages to be greater than scanned, causing an unsigned underflow
> > in vmpressure resulting in a critical event being sent to root
> > cgroup. So do not consider reclaimed slab pages for vmpressure
> > calculation. The reclaimed pages from slab can be excluded because
> > the freeing of a page by slab shrinking depends on each slab's
> > object population, making the cost model (i.e. scan:free) different
> > from that of LRU.
> 
> This might be true but what happens if the slab reclaim contributes
> significantly to the overal reclaim? This would be quite rare but not
> impossible.

Of course, it is better for vmpressure to cover slab but it's not
easy without page-based shrinking model, I think. It wold make
vmpressure higher easily due to low reclaim efficiency compared to
LRU pages. Yeah, vmpressure is not a perfect but no need to add
more noises, either. It's regression since 6b4f7799c6a5 so I think
this patch should go first and if someone want to cover slab really,
he should spend a time to work it well. It's too much that Vinayak
shuld make a effort for that.

Re: [PATCH] pciehp: Fix race condition handling surprise link-down

[Raj, Ashok]

Hi Bjorn

On Thu, Feb 02, 2017 at 08:59:01PM -0600, Bjorn Helgaas wrote:
> Hi Ashok,
> 
> Sorry it took me so long to review this.  I never felt like I really
> understood it, and it took me a long time to try to figure out a more
> useful response.

No worries. Agree its a litte tricky, and took me several iterations before
doing someting that was simple enough, without a complete overhaul of
state management. 

Thanks a ton for capturing the sequence, I did capture
some debug output along at that time. My apologies for not adding it
along. But this becomes excellant notes and perhaps would be good to 
capture in commit or in the documentation. Going through this isn't fun :-)


Responses below:
> > 
> > This patch fixes that by setting the p_slot->state only when the work to
> > handle the power event is executing, protected by the p_slot->hotplug_lock.
> 
> So let me first try to understand what's going on with the current
> code.  In the normal case where a device is removed or turned off and
> pciehp can complete everything before another device appears, I think
> the flow is like this:

You got this problem part right. Spot on!
> 
>   p_slot->state == STATIC_STATE (powered on, link up)
> 
> <-- surprise link down interrupt
>   pciehp_isr()
> queue INT_LINK_DOWN work
> 
>   interrupt_event_handler(INT_LINK_DOWN)
> set p_slot->state = POWEROFF_STATE
> queue DISABLE_REQ work
> 
>   pciehp_power_thread(DISABLE_REQ)
> send PCI_EXP_SLTCTL_PWR_OFF command
> wait for power-off to complete
> set p_slot->state = STATIC_STATE
> 
>   p_slot->state == STATIC_STATE (powered off)
> 
> In the problem case, the link goes down, and while pciehp is still
> dealing with that, the link comes back up.  So I think one possible
> sequence is like this:
> 
>   p_slot->state == STATIC_STATE (powered on, link up)
> 
> <-- surprise link down interrupt
>   1a  pciehp_isr()
> queue INT_LINK_DOWN work # queued: 1-LD
> 
>   1b  interrupt_event_handler(INT_LINK_DOWN) # process 1-LD
> # handle_link_event() sees case STATIC_STATE
> set p_slot->state = POWEROFF_STATE
> queue DISABLE_REQ work   # queued: 1-DR
> 
> <-- surprise link up interrupt
>   2a  pciehp_isr()
> queue INT_LINK_UP work   # queued: 1-DR 2-LU
> 
>   1c  pciehp_power_thread(DISABLE_REQ)   # process 1-DR
> send PCI_EXP_SLTCTL_PWR_OFF command
> wait for power-off to complete
> set p_slot->state = STATIC_STATE
> 
> <-- link down interrupt (result of PWR_OFF)
>   3a  pciehp_isr()
> queue INT_LINK_DOWN work # queued: 2-LU 3-LD
> 
>   2b  interrupt_event_handler(INT_LINK_UP)   # process 2-LU
> # handle_link_event() sees case STATIC_STATE
> set p_slot->state = POWERON_STATE
> queue ENABLE_REQ work# queued: 3-LD 2-ER
> 
>   3b  interrupt_event_handler(INT_LINK_DOWN) # process 3-LD
> # handle_link_event() sees case POWERON_STATE, so we emit
> # "Link Down event queued; currently getting powered on"
> set p_slot->state = POWEROFF_STATE
> queue DISABLE_REQ work   # queued: 2-ER 3-DR
> 
>   2c  pciehp_power_thread(ENABLE_REQ)# process 2-ER
> send PCI_EXP_SLTCTL_PWR_ON command
> wait for power-on to complete
> set p_slot->state = STATIC_STATE
> 
> <-- link up interrupt (result of PWR_ON)
>   4a  pciehp_isr()
> queue INT_LINK_UP work   # queued: 3-DR 4-LU
> 
>   3c  pciehp_power_thread(DISABLE_REQ)   # process 3-DR
> send PCI_EXP_SLTCTL_PWR_OFF command
> wait for power-off to complete
> set p_slot->state = STATIC_STATE
> 
> <-- link down interrupt (result of PWR_OFF)
>   5a  pciehp_isr()
> queue INT_LINK_DOWN work # queued: 4-LU 5-LD
> 
> State 5a is the same as 3a (we're in STATIC_STATE with Link Up and
> Link Down work items queued), so the whole cycle can repeat.
> 
> Now let's assume we apply this patch and see what changes.  The patch
> changes where we set p_slot->state.  Currently we set POWEROFF_STATE
> or POWERON_STATE in the interrupt_event_handler() work item.  The
> patch moves that to the pciehp_power_thread() work item, where the
> power commands are actually sent.

Right. The difference with this patch is when we set the state to 
POWERON_STATE or POWEROFF_STATE, we only do that when the previous
POWER* operation has entirely completed. Since now its protected with the
hotplug_lock mutex.

In the problem case, since we set the state before the pciehp_power_thread,
we end up changing the state to POWER*_STATE before the previous POWER*
action 

Re: [PATCH] pciehp: Fix race condition handling surprise link-down

[Raj, Ashok]

Hi Bjorn

On Thu, Feb 02, 2017 at 08:59:01PM -0600, Bjorn Helgaas wrote:
> Hi Ashok,
> 
> Sorry it took me so long to review this.  I never felt like I really
> understood it, and it took me a long time to try to figure out a more
> useful response.

No worries. Agree its a litte tricky, and took me several iterations before
doing someting that was simple enough, without a complete overhaul of
state management. 

Thanks a ton for capturing the sequence, I did capture
some debug output along at that time. My apologies for not adding it
along. But this becomes excellant notes and perhaps would be good to 
capture in commit or in the documentation. Going through this isn't fun :-)


Responses below:
> > 
> > This patch fixes that by setting the p_slot->state only when the work to
> > handle the power event is executing, protected by the p_slot->hotplug_lock.
> 
> So let me first try to understand what's going on with the current
> code.  In the normal case where a device is removed or turned off and
> pciehp can complete everything before another device appears, I think
> the flow is like this:

You got this problem part right. Spot on!
> 
>   p_slot->state == STATIC_STATE (powered on, link up)
> 
> <-- surprise link down interrupt
>   pciehp_isr()
> queue INT_LINK_DOWN work
> 
>   interrupt_event_handler(INT_LINK_DOWN)
> set p_slot->state = POWEROFF_STATE
> queue DISABLE_REQ work
> 
>   pciehp_power_thread(DISABLE_REQ)
> send PCI_EXP_SLTCTL_PWR_OFF command
> wait for power-off to complete
> set p_slot->state = STATIC_STATE
> 
>   p_slot->state == STATIC_STATE (powered off)
> 
> In the problem case, the link goes down, and while pciehp is still
> dealing with that, the link comes back up.  So I think one possible
> sequence is like this:
> 
>   p_slot->state == STATIC_STATE (powered on, link up)
> 
> <-- surprise link down interrupt
>   1a  pciehp_isr()
> queue INT_LINK_DOWN work # queued: 1-LD
> 
>   1b  interrupt_event_handler(INT_LINK_DOWN) # process 1-LD
> # handle_link_event() sees case STATIC_STATE
> set p_slot->state = POWEROFF_STATE
> queue DISABLE_REQ work   # queued: 1-DR
> 
> <-- surprise link up interrupt
>   2a  pciehp_isr()
> queue INT_LINK_UP work   # queued: 1-DR 2-LU
> 
>   1c  pciehp_power_thread(DISABLE_REQ)   # process 1-DR
> send PCI_EXP_SLTCTL_PWR_OFF command
> wait for power-off to complete
> set p_slot->state = STATIC_STATE
> 
> <-- link down interrupt (result of PWR_OFF)
>   3a  pciehp_isr()
> queue INT_LINK_DOWN work # queued: 2-LU 3-LD
> 
>   2b  interrupt_event_handler(INT_LINK_UP)   # process 2-LU
> # handle_link_event() sees case STATIC_STATE
> set p_slot->state = POWERON_STATE
> queue ENABLE_REQ work# queued: 3-LD 2-ER
> 
>   3b  interrupt_event_handler(INT_LINK_DOWN) # process 3-LD
> # handle_link_event() sees case POWERON_STATE, so we emit
> # "Link Down event queued; currently getting powered on"
> set p_slot->state = POWEROFF_STATE
> queue DISABLE_REQ work   # queued: 2-ER 3-DR
> 
>   2c  pciehp_power_thread(ENABLE_REQ)# process 2-ER
> send PCI_EXP_SLTCTL_PWR_ON command
> wait for power-on to complete
> set p_slot->state = STATIC_STATE
> 
> <-- link up interrupt (result of PWR_ON)
>   4a  pciehp_isr()
> queue INT_LINK_UP work   # queued: 3-DR 4-LU
> 
>   3c  pciehp_power_thread(DISABLE_REQ)   # process 3-DR
> send PCI_EXP_SLTCTL_PWR_OFF command
> wait for power-off to complete
> set p_slot->state = STATIC_STATE
> 
> <-- link down interrupt (result of PWR_OFF)
>   5a  pciehp_isr()
> queue INT_LINK_DOWN work # queued: 4-LU 5-LD
> 
> State 5a is the same as 3a (we're in STATIC_STATE with Link Up and
> Link Down work items queued), so the whole cycle can repeat.
> 
> Now let's assume we apply this patch and see what changes.  The patch
> changes where we set p_slot->state.  Currently we set POWEROFF_STATE
> or POWERON_STATE in the interrupt_event_handler() work item.  The
> patch moves that to the pciehp_power_thread() work item, where the
> power commands are actually sent.

Right. The difference with this patch is when we set the state to 
POWERON_STATE or POWEROFF_STATE, we only do that when the previous
POWER* operation has entirely completed. Since now its protected with the
hotplug_lock mutex.

In the problem case, since we set the state before the pciehp_power_thread,
we end up changing the state to POWER*_STATE before the previous POWER*
action 

Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

[Inki Dae]

2017년 02월 02일 04:03에 Sean Paul 이(가) 쓴 글:
> On Wed, Feb 01, 2017 at 03:29:40PM +, Emil Velikov wrote:
>> On 1 February 2017 at 14:52, Thierry Reding  wrote:
>>> On Tue, Jan 31, 2017 at 02:54:53PM -0800, Eric Anholt wrote:
 Thierry Reding  writes:

> [ Unknown signature status ]
> On Tue, Jan 31, 2017 at 10:15:10AM -0800, Eric Anholt wrote:
>> Thierry Reding  writes:
>>
>>> [ Unknown signature status ]
>>> On Tue, Jan 31, 2017 at 09:38:53AM -0500, Sean Paul wrote:
 On Tue, Jan 31, 2017 at 09:54:49AM +0100, Thierry Reding wrote:
> On Tue, Jan 31, 2017 at 09:01:07AM +0900, Inki Dae wrote:
>>
>>
>> 2017년 01월 24일 10:50에 Hoegeun Kwon 이(가) 쓴 글:
>>> Dear Thierry,
>>>
>>> Could you please review this patch?
>>
>> Thierry, I think this patch has been reviewed enough but no comment
>> from you. Seems you are busy. I will pick up this.
>
> Sorry, but that's not how it works. This patch has gone through 8
> revisions within 4 weeks, and I tend to ignore patches like that until
> the dust settles.
>

 Seems like the dust was pretty settled. It was posted on 1/11, pinged 
 on 1/24,
 and picked up on 1/31. I don't think it's unreasonable to take it 
 through
 another tree after that.

 I wonder if drm_panel would benefit from the -misc group 
 maintainership model
 as drm_bridge does. By spreading out the workload, the high-maintenance
 patches would hopefully find someone to shepherd them through.
>>>
>>> Except that nobody except me really cares. If we let people take patches
>>> through separate trees or group-maintained trees they'll likely go in
>>> without too much thought. DRM panel is somewhat different from core DRM
>>> in this regard because its infrastructure is minimal and there's little
>>> outside the panel-simple driver. So we're still at a stage where we need
>>> to fine-tune what drivers should look like and how we can improve.
>>
>> I would love to care and participate in review, but with the structure
>> of your tree you're the only one whose review counts, so I don't
>> participate.
>
> Really? What exactly do you think is special about the structure of my
> tree? I require patches to be on dri-devel (I pick them up from the
> patchwork instance at freedesktop.org), the tree is publicly available
> and reviewed-by tags get picked up automatically by patchwork.
>
> The panel tree works exactly like any other maintainer tree. And my
> review is *not* the only one that counts. I appreciate every Reviewed-by
> tag I see on panel patches because it means that I don't have to look as
> closely as I have to otherwise.
>
> It is true that I am responsible for those patches, that's why I get to
> have the final word on whether or not a patch gets applied. And that's
> no different from any other maintainer tree either.

 If me reviewing a patch isn't part of unblocking that patch getting in,
 then I won't bother because all I could end up doing is punishing the
 developer of the patch.  Contributors have a hard enough time already.
>>>
>>> Maybe you should go and read my previous reply again more carefully.
>>> Perhaps then you'll realize that reviews are in fact helping in getting
>>> patches merged.
>>>
>>> Interestingly my inbox doesn't show you ever bothering to review panel
>>> patches, so maybe you should be more careful about your assumptions.
>>>
>> Gents, it's understandable that emotions might be running high.
>>
>> What's the point in pointing fingers at each other - there is enough
>> to go in each direction.
>> Let us all step back for a second and consider how we can make things better.
>>
> 
> Seems like I kicked up some dust here, for that I apologize. I certainly did 
> not
> intend to diminish Thierry's (or anyone else's) role as maintainer.
> 
> To put this as concisely as I can, I thought drm_panel would be a good 
> candidate
> for -misc given:
> - drm_bridge is already maintained there
> - the drivers are small, and we just resolved to maintain small 
> drivers
>   in -misc
> - new patches are blocked on a single reviewer/committer as opposed 
> to a
>   qualified committee (which I have come to understand is a feature in
>   this instance)

Agree. drm_panel is not large enough to require another maintainer.
However, we had already agreed that Thierry manages drm_panel, either 
implicitly or explicitly.
Seems he's tired now and he wants to talk about this issue again on next Monday.
At the meeting, I think we could decide whether going to group maintainership 
model, stay as-is or other better way, 

Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

[Inki Dae]

2017년 02월 02일 04:03에 Sean Paul 이(가) 쓴 글:
> On Wed, Feb 01, 2017 at 03:29:40PM +, Emil Velikov wrote:
>> On 1 February 2017 at 14:52, Thierry Reding  wrote:
>>> On Tue, Jan 31, 2017 at 02:54:53PM -0800, Eric Anholt wrote:
 Thierry Reding  writes:

> [ Unknown signature status ]
> On Tue, Jan 31, 2017 at 10:15:10AM -0800, Eric Anholt wrote:
>> Thierry Reding  writes:
>>
>>> [ Unknown signature status ]
>>> On Tue, Jan 31, 2017 at 09:38:53AM -0500, Sean Paul wrote:
 On Tue, Jan 31, 2017 at 09:54:49AM +0100, Thierry Reding wrote:
> On Tue, Jan 31, 2017 at 09:01:07AM +0900, Inki Dae wrote:
>>
>>
>> 2017년 01월 24일 10:50에 Hoegeun Kwon 이(가) 쓴 글:
>>> Dear Thierry,
>>>
>>> Could you please review this patch?
>>
>> Thierry, I think this patch has been reviewed enough but no comment
>> from you. Seems you are busy. I will pick up this.
>
> Sorry, but that's not how it works. This patch has gone through 8
> revisions within 4 weeks, and I tend to ignore patches like that until
> the dust settles.
>

 Seems like the dust was pretty settled. It was posted on 1/11, pinged 
 on 1/24,
 and picked up on 1/31. I don't think it's unreasonable to take it 
 through
 another tree after that.

 I wonder if drm_panel would benefit from the -misc group 
 maintainership model
 as drm_bridge does. By spreading out the workload, the high-maintenance
 patches would hopefully find someone to shepherd them through.
>>>
>>> Except that nobody except me really cares. If we let people take patches
>>> through separate trees or group-maintained trees they'll likely go in
>>> without too much thought. DRM panel is somewhat different from core DRM
>>> in this regard because its infrastructure is minimal and there's little
>>> outside the panel-simple driver. So we're still at a stage where we need
>>> to fine-tune what drivers should look like and how we can improve.
>>
>> I would love to care and participate in review, but with the structure
>> of your tree you're the only one whose review counts, so I don't
>> participate.
>
> Really? What exactly do you think is special about the structure of my
> tree? I require patches to be on dri-devel (I pick them up from the
> patchwork instance at freedesktop.org), the tree is publicly available
> and reviewed-by tags get picked up automatically by patchwork.
>
> The panel tree works exactly like any other maintainer tree. And my
> review is *not* the only one that counts. I appreciate every Reviewed-by
> tag I see on panel patches because it means that I don't have to look as
> closely as I have to otherwise.
>
> It is true that I am responsible for those patches, that's why I get to
> have the final word on whether or not a patch gets applied. And that's
> no different from any other maintainer tree either.

 If me reviewing a patch isn't part of unblocking that patch getting in,
 then I won't bother because all I could end up doing is punishing the
 developer of the patch.  Contributors have a hard enough time already.
>>>
>>> Maybe you should go and read my previous reply again more carefully.
>>> Perhaps then you'll realize that reviews are in fact helping in getting
>>> patches merged.
>>>
>>> Interestingly my inbox doesn't show you ever bothering to review panel
>>> patches, so maybe you should be more careful about your assumptions.
>>>
>> Gents, it's understandable that emotions might be running high.
>>
>> What's the point in pointing fingers at each other - there is enough
>> to go in each direction.
>> Let us all step back for a second and consider how we can make things better.
>>
> 
> Seems like I kicked up some dust here, for that I apologize. I certainly did 
> not
> intend to diminish Thierry's (or anyone else's) role as maintainer.
> 
> To put this as concisely as I can, I thought drm_panel would be a good 
> candidate
> for -misc given:
> - drm_bridge is already maintained there
> - the drivers are small, and we just resolved to maintain small 
> drivers
>   in -misc
> - new patches are blocked on a single reviewer/committer as opposed 
> to a
>   qualified committee (which I have come to understand is a feature in
>   this instance)

Agree. drm_panel is not large enough to require another maintainer.
However, we had already agreed that Thierry manages drm_panel, either 
implicitly or explicitly.
Seems he's tired now and he wants to talk about this issue again on next Monday.
At the meeting, I think we could decide whether going to group maintainership 
model, stay as-is or other better way, including reaching consensus.

Thanks,
Inki Dae

> 
> So if we can't migrate it to 

[PATCH v2 3/4] seccomp: Create an action to log before allowing

[Tyler Hicks]

Add a new action, SECCOMP_RET_LOG, that logs a syscall before allowing
the syscall. At the implementation level, this action is identical to
the existing SECCOMP_RET_ALLOW action. However, it can be very useful when
initially developing a seccomp filter for an application. The developer
can set the default action to be SECCOMP_RET_LOG, maybe mark any
obviously needed syscalls with SECCOMP_RET_ALLOW, and then put the
application through its paces. A list of syscalls that triggered the
default action (SECCOMP_RET_LOG) can be easily gleaned from the logs and
that list can be used to build the syscall whitelist. Finally, the
developer can change the default action to the desired value.

This provides a more friendly experience than seeing the application get
killed, then updating the filter and rebuilding the app, seeing the
application get killed due to a different syscall, then updating the
filter and rebuilding the app, etc.

The functionality is similar to what's supported by the various LSMs.
SELinux has permissive mode, AppArmor has complain mode, SMACK has
bring-up mode, etc.

SECCOMP_RET_LOG is given a lower value than SECCOMP_RET_ALLOW so that
"allow" can be written to the max_action_to_log sysctl in order to get a
list of logged actions without the, potentially larger, set of allowed
actions.

Signed-off-by: Tyler Hicks 
---
 Documentation/prctl/seccomp_filter.txt | 6 ++
 include/uapi/linux/seccomp.h   | 1 +
 kernel/seccomp.c   | 4 
 3 files changed, 11 insertions(+)

diff --git a/Documentation/prctl/seccomp_filter.txt 
b/Documentation/prctl/seccomp_filter.txt
index 1e469ef..ba55a91 100644
--- a/Documentation/prctl/seccomp_filter.txt
+++ b/Documentation/prctl/seccomp_filter.txt
@@ -138,6 +138,12 @@ SECCOMP_RET_TRACE:
allow use of ptrace, even of other sandboxed processes, without
extreme care; ptracers can use this mechanism to escape.)
 
+SECCOMP_RET_LOG:
+   Results in the system call being executed after it is logged. This
+   should be used by application developers to learn which syscalls their
+   application needs without having to iterate through multiple test and
+   development cycles to build the list.
+
 SECCOMP_RET_ALLOW:
Results in the system call being executed.
 
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 0f238a4..67f72cd 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -29,6 +29,7 @@
 #define SECCOMP_RET_TRAP   0x0003U /* disallow and force a SIGSYS */
 #define SECCOMP_RET_ERRNO  0x0005U /* returns an errno */
 #define SECCOMP_RET_TRACE  0x7ff0U /* pass to a tracer or disallow */
+#define SECCOMP_RET_LOG0x7ffeU /* allow after logging */
 #define SECCOMP_RET_ALLOW  0x7fffU /* allow */
 
 /* Masks for the return value sections. */
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 548fb89..8627481 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -650,6 +650,7 @@ static int __seccomp_filter(int this_syscall, const struct 
seccomp_data *sd,
 
return 0;
 
+   case SECCOMP_RET_LOG:
case SECCOMP_RET_ALLOW:
seccomp_log(this_syscall, 0, action);
return 0;
@@ -934,6 +935,7 @@ long seccomp_get_filter(struct task_struct *task, unsigned 
long filter_off,
 #define SECCOMP_RET_TRAP_NAME  "trap"
 #define SECCOMP_RET_ERRNO_NAME "errno"
 #define SECCOMP_RET_TRACE_NAME "trace"
+#define SECCOMP_RET_LOG_NAME   "log"
 #define SECCOMP_RET_ALLOW_NAME "allow"
 
 /* Largest strlen() of all action names */
@@ -943,6 +945,7 @@ static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME 
" "
  SECCOMP_RET_TRAP_NAME " "
  SECCOMP_RET_ERRNO_NAME" "
  SECCOMP_RET_TRACE_NAME" "
+ SECCOMP_RET_LOG_NAME  " "
  SECCOMP_RET_ALLOW_NAME;
 
 struct seccomp_action_name {
@@ -955,6 +958,7 @@ static struct seccomp_action_name seccomp_action_names[] = {
{ SECCOMP_RET_TRAP, SECCOMP_RET_TRAP_NAME },
{ SECCOMP_RET_ERRNO, SECCOMP_RET_ERRNO_NAME },
{ SECCOMP_RET_TRACE, SECCOMP_RET_TRACE_NAME },
+   { SECCOMP_RET_LOG, SECCOMP_RET_LOG_NAME },
{ SECCOMP_RET_ALLOW, SECCOMP_RET_ALLOW_NAME },
{ }
 };
-- 
2.7.4

[PATCH v2 1/4] seccomp: Add sysctl to display available actions

[Tyler Hicks]

This patch creates a read-only sysctl containing an ordered list of
seccomp actions that the kernel supports. The ordering, from left to
right, is the lowest action value (kill) to the highest action value
(allow). Currently, a read of the sysctl file would return "kill trap
errno trace allow". The contents of this sysctl file can be useful for
userspace code as well as the system administrator.

The path to the sysctl is:

  /proc/sys/kernel/seccomp/actions_avail

libseccomp and other userspace code can easily determine which actions
the current kernel supports. The set of actions supported by the current
kernel may be different than the set of action macros found in kernel
headers that were installed where the userspace code was built.

In addition, this sysctl will allow system administrators to know which
actions are supported by the kernel and make it easier to configure
exactly what seccomp logs through the audit subsystem. Support for this
level of logging configuration will come in a future patch.

Signed-off-by: Tyler Hicks 
---
 kernel/seccomp.c | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index f7ce79a..919ad9f 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -16,10 +16,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
 #include 
@@ -905,3 +907,51 @@ long seccomp_get_filter(struct task_struct *task, unsigned 
long filter_off,
return ret;
 }
 #endif
+
+#ifdef CONFIG_SYSCTL
+
+#define SECCOMP_RET_KILL_NAME  "kill"
+#define SECCOMP_RET_TRAP_NAME  "trap"
+#define SECCOMP_RET_ERRNO_NAME "errno"
+#define SECCOMP_RET_TRACE_NAME "trace"
+#define SECCOMP_RET_ALLOW_NAME "allow"
+
+static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME" "
+ SECCOMP_RET_TRAP_NAME " "
+ SECCOMP_RET_ERRNO_NAME" "
+ SECCOMP_RET_TRACE_NAME" "
+ SECCOMP_RET_ALLOW_NAME;
+
+static struct ctl_path seccomp_sysctl_path[] = {
+   { .procname = "kernel", },
+   { .procname = "seccomp", },
+   { }
+};
+
+static struct ctl_table seccomp_sysctl_table[] = {
+   {
+   .procname   = "actions_avail",
+   .data   = _actions_avail,
+   .maxlen = sizeof(seccomp_actions_avail),
+   .mode   = 0444,
+   .proc_handler   = proc_dostring,
+   },
+   { }
+};
+
+static int __init seccomp_sysctl_init(void)
+{
+   struct ctl_table_header *hdr;
+
+   hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table);
+   kmemleak_not_leak(hdr);
+   return 0;
+}
+
+#else /* CONFIG_SYSCTL */
+
+static __init int seccomp_sysctl_init(void) { return 0; }
+
+#endif /* CONFIG_SYSCTL */
+
+device_initcall(seccomp_sysctl_init)
-- 
2.7.4

[PATCH v2 3/4] seccomp: Create an action to log before allowing

[Tyler Hicks]

Add a new action, SECCOMP_RET_LOG, that logs a syscall before allowing
the syscall. At the implementation level, this action is identical to
the existing SECCOMP_RET_ALLOW action. However, it can be very useful when
initially developing a seccomp filter for an application. The developer
can set the default action to be SECCOMP_RET_LOG, maybe mark any
obviously needed syscalls with SECCOMP_RET_ALLOW, and then put the
application through its paces. A list of syscalls that triggered the
default action (SECCOMP_RET_LOG) can be easily gleaned from the logs and
that list can be used to build the syscall whitelist. Finally, the
developer can change the default action to the desired value.

This provides a more friendly experience than seeing the application get
killed, then updating the filter and rebuilding the app, seeing the
application get killed due to a different syscall, then updating the
filter and rebuilding the app, etc.

The functionality is similar to what's supported by the various LSMs.
SELinux has permissive mode, AppArmor has complain mode, SMACK has
bring-up mode, etc.

SECCOMP_RET_LOG is given a lower value than SECCOMP_RET_ALLOW so that
"allow" can be written to the max_action_to_log sysctl in order to get a
list of logged actions without the, potentially larger, set of allowed
actions.

Signed-off-by: Tyler Hicks 
---
 Documentation/prctl/seccomp_filter.txt | 6 ++
 include/uapi/linux/seccomp.h   | 1 +
 kernel/seccomp.c   | 4 
 3 files changed, 11 insertions(+)

diff --git a/Documentation/prctl/seccomp_filter.txt 
b/Documentation/prctl/seccomp_filter.txt
index 1e469ef..ba55a91 100644
--- a/Documentation/prctl/seccomp_filter.txt
+++ b/Documentation/prctl/seccomp_filter.txt
@@ -138,6 +138,12 @@ SECCOMP_RET_TRACE:
allow use of ptrace, even of other sandboxed processes, without
extreme care; ptracers can use this mechanism to escape.)
 
+SECCOMP_RET_LOG:
+   Results in the system call being executed after it is logged. This
+   should be used by application developers to learn which syscalls their
+   application needs without having to iterate through multiple test and
+   development cycles to build the list.
+
 SECCOMP_RET_ALLOW:
Results in the system call being executed.
 
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 0f238a4..67f72cd 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -29,6 +29,7 @@
 #define SECCOMP_RET_TRAP   0x0003U /* disallow and force a SIGSYS */
 #define SECCOMP_RET_ERRNO  0x0005U /* returns an errno */
 #define SECCOMP_RET_TRACE  0x7ff0U /* pass to a tracer or disallow */
+#define SECCOMP_RET_LOG0x7ffeU /* allow after logging */
 #define SECCOMP_RET_ALLOW  0x7fffU /* allow */
 
 /* Masks for the return value sections. */
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 548fb89..8627481 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -650,6 +650,7 @@ static int __seccomp_filter(int this_syscall, const struct 
seccomp_data *sd,
 
return 0;
 
+   case SECCOMP_RET_LOG:
case SECCOMP_RET_ALLOW:
seccomp_log(this_syscall, 0, action);
return 0;
@@ -934,6 +935,7 @@ long seccomp_get_filter(struct task_struct *task, unsigned 
long filter_off,
 #define SECCOMP_RET_TRAP_NAME  "trap"
 #define SECCOMP_RET_ERRNO_NAME "errno"
 #define SECCOMP_RET_TRACE_NAME "trace"
+#define SECCOMP_RET_LOG_NAME   "log"
 #define SECCOMP_RET_ALLOW_NAME "allow"
 
 /* Largest strlen() of all action names */
@@ -943,6 +945,7 @@ static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME 
" "
  SECCOMP_RET_TRAP_NAME " "
  SECCOMP_RET_ERRNO_NAME" "
  SECCOMP_RET_TRACE_NAME" "
+ SECCOMP_RET_LOG_NAME  " "
  SECCOMP_RET_ALLOW_NAME;
 
 struct seccomp_action_name {
@@ -955,6 +958,7 @@ static struct seccomp_action_name seccomp_action_names[] = {
{ SECCOMP_RET_TRAP, SECCOMP_RET_TRAP_NAME },
{ SECCOMP_RET_ERRNO, SECCOMP_RET_ERRNO_NAME },
{ SECCOMP_RET_TRACE, SECCOMP_RET_TRACE_NAME },
+   { SECCOMP_RET_LOG, SECCOMP_RET_LOG_NAME },
{ SECCOMP_RET_ALLOW, SECCOMP_RET_ALLOW_NAME },
{ }
 };
-- 
2.7.4

[PATCH v2 1/4] seccomp: Add sysctl to display available actions

[Tyler Hicks]

This patch creates a read-only sysctl containing an ordered list of
seccomp actions that the kernel supports. The ordering, from left to
right, is the lowest action value (kill) to the highest action value
(allow). Currently, a read of the sysctl file would return "kill trap
errno trace allow". The contents of this sysctl file can be useful for
userspace code as well as the system administrator.

The path to the sysctl is:

  /proc/sys/kernel/seccomp/actions_avail

libseccomp and other userspace code can easily determine which actions
the current kernel supports. The set of actions supported by the current
kernel may be different than the set of action macros found in kernel
headers that were installed where the userspace code was built.

In addition, this sysctl will allow system administrators to know which
actions are supported by the kernel and make it easier to configure
exactly what seccomp logs through the audit subsystem. Support for this
level of logging configuration will come in a future patch.

Signed-off-by: Tyler Hicks 
---
 kernel/seccomp.c | 50 ++
 1 file changed, 50 insertions(+)

diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index f7ce79a..919ad9f 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -16,10 +16,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_HAVE_ARCH_SECCOMP_FILTER
 #include 
@@ -905,3 +907,51 @@ long seccomp_get_filter(struct task_struct *task, unsigned 
long filter_off,
return ret;
 }
 #endif
+
+#ifdef CONFIG_SYSCTL
+
+#define SECCOMP_RET_KILL_NAME  "kill"
+#define SECCOMP_RET_TRAP_NAME  "trap"
+#define SECCOMP_RET_ERRNO_NAME "errno"
+#define SECCOMP_RET_TRACE_NAME "trace"
+#define SECCOMP_RET_ALLOW_NAME "allow"
+
+static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME" "
+ SECCOMP_RET_TRAP_NAME " "
+ SECCOMP_RET_ERRNO_NAME" "
+ SECCOMP_RET_TRACE_NAME" "
+ SECCOMP_RET_ALLOW_NAME;
+
+static struct ctl_path seccomp_sysctl_path[] = {
+   { .procname = "kernel", },
+   { .procname = "seccomp", },
+   { }
+};
+
+static struct ctl_table seccomp_sysctl_table[] = {
+   {
+   .procname   = "actions_avail",
+   .data   = _actions_avail,
+   .maxlen = sizeof(seccomp_actions_avail),
+   .mode   = 0444,
+   .proc_handler   = proc_dostring,
+   },
+   { }
+};
+
+static int __init seccomp_sysctl_init(void)
+{
+   struct ctl_table_header *hdr;
+
+   hdr = register_sysctl_paths(seccomp_sysctl_path, seccomp_sysctl_table);
+   kmemleak_not_leak(hdr);
+   return 0;
+}
+
+#else /* CONFIG_SYSCTL */
+
+static __init int seccomp_sysctl_init(void) { return 0; }
+
+#endif /* CONFIG_SYSCTL */
+
+device_initcall(seccomp_sysctl_init)
-- 
2.7.4

[PATCH v2 2/4] seccomp: Add sysctl to configure actions that should be logged

[Tyler Hicks]

Administrators can write to this sysctl to set the maximum seccomp
action that should be logged. Any actions with values greater than
what's written to the sysctl will not be logged.

For example, all SECCOMP_RET_KILL, SECCOMP_RET_TRAP, and
SECCOMP_RET_ERRNO actions would be logged if "errno" were written to the
sysctl. SECCOMP_RET_TRACE and SECCOMP_RET_ALLOW actions would not be
logged since their values are higher than SECCOMP_RET_ERRNO.

The path to the sysctl is:

 /proc/sys/kernel/seccomp/max_action_to_log

The actions_avail sysctl can be read to discover the valid action names
that can be written to the max_action_to_log sysctl. The actions_avail
sysctl is also useful in understanding the ordering of actions used when
deciding the maximum action to log.

The default setting for the sysctl is to only log SECCOMP_RET_KILL
actions which matches the existing behavior.

There's one important exception to this sysctl. If a task is
specifically being audited, meaning that an audit context has been
allocated for the task, seccomp will log all actions other than
SECCOMP_RET_ALLOW despite the value of max_action_to_log. This exception
preserves the existing auditing behavior of tasks with an allocated
audit context.

Signed-off-by: Tyler Hicks 
---
 include/linux/audit.h |   6 +--
 kernel/seccomp.c  | 114 --
 2 files changed, 112 insertions(+), 8 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index f51fca8d..e0d95fc 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -315,11 +315,7 @@ void audit_core_dumps(long signr);
 
 static inline void audit_seccomp(unsigned long syscall, long signr, int code)
 {
-   if (!audit_enabled)
-   return;
-
-   /* Force a record to be reported if a signal was delivered. */
-   if (signr || unlikely(!audit_dummy_context()))
+   if (audit_enabled && unlikely(!audit_dummy_context()))
__audit_seccomp(syscall, signr, code);
 }
 
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 919ad9f..548fb89 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -509,6 +509,24 @@ static void seccomp_send_sigsys(int syscall, int reason)
 }
 #endif /* CONFIG_SECCOMP_FILTER */
 
+static u32 seccomp_max_action_to_log = SECCOMP_RET_KILL;
+
+static void seccomp_log(unsigned long syscall, long signr, u32 action)
+{
+   /* Force an audit message to be emitted when the action is not greater
+* than the configured maximum action.
+*/
+   if (action <= seccomp_max_action_to_log)
+   return __audit_seccomp(syscall, signr, action);
+
+   /* If the action is not an ALLOW action, let the audit subsystem decide
+* if it should be audited based on whether the current task itself is
+* being audited.
+*/
+   if (action != SECCOMP_RET_ALLOW)
+   return audit_seccomp(syscall, signr, action);
+}
+
 /*
  * Secure computing mode 1 allows only read/write/exit/sigreturn.
  * To be fully secure this must be combined with rlimit
@@ -534,7 +552,7 @@ static void __secure_computing_strict(int this_syscall)
 #ifdef SECCOMP_DEBUG
dump_stack();
 #endif
-   audit_seccomp(this_syscall, SIGKILL, SECCOMP_RET_KILL);
+   seccomp_log(this_syscall, SIGKILL, SECCOMP_RET_KILL);
do_exit(SIGKILL);
 }
 
@@ -633,18 +651,19 @@ static int __seccomp_filter(int this_syscall, const 
struct seccomp_data *sd,
return 0;
 
case SECCOMP_RET_ALLOW:
+   seccomp_log(this_syscall, 0, action);
return 0;
 
case SECCOMP_RET_KILL:
default:
-   audit_seccomp(this_syscall, SIGSYS, action);
+   seccomp_log(this_syscall, SIGSYS, action);
do_exit(SIGSYS);
}
 
unreachable();
 
 skip:
-   audit_seccomp(this_syscall, 0, action);
+   seccomp_log(this_syscall, 0, action);
return -1;
 }
 #else
@@ -910,18 +929,102 @@ long seccomp_get_filter(struct task_struct *task, 
unsigned long filter_off,
 
 #ifdef CONFIG_SYSCTL
 
+/* Human readable action names for friendly sysctl interaction */
 #define SECCOMP_RET_KILL_NAME  "kill"
 #define SECCOMP_RET_TRAP_NAME  "trap"
 #define SECCOMP_RET_ERRNO_NAME "errno"
 #define SECCOMP_RET_TRACE_NAME "trace"
 #define SECCOMP_RET_ALLOW_NAME "allow"
 
+/* Largest strlen() of all action names */
+#define SECCOMP_RET_MAX_NAME_LEN   5
+
 static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME" "
  SECCOMP_RET_TRAP_NAME " "
  SECCOMP_RET_ERRNO_NAME" "
  SECCOMP_RET_TRACE_NAME" "
  SECCOMP_RET_ALLOW_NAME;
 
+struct seccomp_action_name {
+   u32 action;
+   const char  *name;
+};
+
+static struct seccomp_action_name 

[PATCH v2 4/4] seccomp: Add tests for SECCOMP_RET_LOG

[Tyler Hicks]

Extend the kernel selftests for seccomp to test the newly added
SECCOMP_RET_LOG action. The added tests follow the example of existing
tests.

Unfortunately, the tests are not capable of inspecting the audit log to
verify that the syscall was logged.

Signed-off-by: Tyler Hicks 
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 94 +++
 1 file changed, 94 insertions(+)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c 
b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 03f1fa4..a39f620 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -87,6 +87,10 @@ struct seccomp_data {
 };
 #endif
 
+#ifndef SECCOMP_RET_LOG
+#define SECCOMP_RET_LOG   0x7ffeU /* allow after logging */
+#endif
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 #define syscall_arg(_n) (offsetof(struct seccomp_data, args[_n]))
 #elif __BYTE_ORDER == __BIG_ENDIAN
@@ -342,6 +346,28 @@ TEST(empty_prog)
EXPECT_EQ(EINVAL, errno);
 }
 
+TEST(log_all)
+{
+   struct sock_filter filter[] = {
+   BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_LOG),
+   };
+   struct sock_fprog prog = {
+   .len = (unsigned short)ARRAY_SIZE(filter),
+   .filter = filter,
+   };
+   long ret;
+   pid_t parent = getppid();
+
+   ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+   ASSERT_EQ(0, ret);
+
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, );
+   ASSERT_EQ(0, ret);
+
+   /* getppid() should succeed and be logged (no check for logging) */
+   EXPECT_EQ(parent, syscall(__NR_getppid));
+}
+
 TEST_SIGNAL(unknown_ret_is_kill_inside, SIGSYS)
 {
struct sock_filter filter[] = {
@@ -735,6 +761,7 @@ TEST_F(TRAP, handler)
 
 FIXTURE_DATA(precedence) {
struct sock_fprog allow;
+   struct sock_fprog log;
struct sock_fprog trace;
struct sock_fprog error;
struct sock_fprog trap;
@@ -746,6 +773,13 @@ FIXTURE_SETUP(precedence)
struct sock_filter allow_insns[] = {
BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
};
+   struct sock_filter log_insns[] = {
+   BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+   offsetof(struct seccomp_data, nr)),
+   BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getpid, 1, 0),
+   BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+   BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_LOG),
+   };
struct sock_filter trace_insns[] = {
BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
offsetof(struct seccomp_data, nr)),
@@ -782,6 +816,7 @@ FIXTURE_SETUP(precedence)
memcpy(self->_x.filter, &_x##_insns, sizeof(_x##_insns)); \
self->_x.len = (unsigned short)ARRAY_SIZE(_x##_insns)
FILTER_ALLOC(allow);
+   FILTER_ALLOC(log);
FILTER_ALLOC(trace);
FILTER_ALLOC(error);
FILTER_ALLOC(trap);
@@ -792,6 +827,7 @@ FIXTURE_TEARDOWN(precedence)
 {
 #define FILTER_FREE(_x) if (self->_x.filter) free(self->_x.filter)
FILTER_FREE(allow);
+   FILTER_FREE(log);
FILTER_FREE(trace);
FILTER_FREE(error);
FILTER_FREE(trap);
@@ -809,6 +845,8 @@ TEST_F(precedence, allow_ok)
 
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >allow);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
@@ -833,6 +871,8 @@ TEST_F_SIGNAL(precedence, kill_is_highest, SIGSYS)
 
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >allow);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
@@ -864,6 +904,8 @@ TEST_F_SIGNAL(precedence, kill_is_highest_in_any_order, 
SIGSYS)
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trap);
@@ -885,6 +927,8 @@ TEST_F_SIGNAL(precedence, trap_is_second, SIGSYS)
 
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >allow);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
@@ -910,6 +954,8 @@ TEST_F_SIGNAL(precedence, trap_is_second_in_any_order, 
SIGSYS)

[PATCH v2 2/4] seccomp: Add sysctl to configure actions that should be logged

[Tyler Hicks]

Administrators can write to this sysctl to set the maximum seccomp
action that should be logged. Any actions with values greater than
what's written to the sysctl will not be logged.

For example, all SECCOMP_RET_KILL, SECCOMP_RET_TRAP, and
SECCOMP_RET_ERRNO actions would be logged if "errno" were written to the
sysctl. SECCOMP_RET_TRACE and SECCOMP_RET_ALLOW actions would not be
logged since their values are higher than SECCOMP_RET_ERRNO.

The path to the sysctl is:

 /proc/sys/kernel/seccomp/max_action_to_log

The actions_avail sysctl can be read to discover the valid action names
that can be written to the max_action_to_log sysctl. The actions_avail
sysctl is also useful in understanding the ordering of actions used when
deciding the maximum action to log.

The default setting for the sysctl is to only log SECCOMP_RET_KILL
actions which matches the existing behavior.

There's one important exception to this sysctl. If a task is
specifically being audited, meaning that an audit context has been
allocated for the task, seccomp will log all actions other than
SECCOMP_RET_ALLOW despite the value of max_action_to_log. This exception
preserves the existing auditing behavior of tasks with an allocated
audit context.

Signed-off-by: Tyler Hicks 
---
 include/linux/audit.h |   6 +--
 kernel/seccomp.c  | 114 --
 2 files changed, 112 insertions(+), 8 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index f51fca8d..e0d95fc 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -315,11 +315,7 @@ void audit_core_dumps(long signr);
 
 static inline void audit_seccomp(unsigned long syscall, long signr, int code)
 {
-   if (!audit_enabled)
-   return;
-
-   /* Force a record to be reported if a signal was delivered. */
-   if (signr || unlikely(!audit_dummy_context()))
+   if (audit_enabled && unlikely(!audit_dummy_context()))
__audit_seccomp(syscall, signr, code);
 }
 
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 919ad9f..548fb89 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -509,6 +509,24 @@ static void seccomp_send_sigsys(int syscall, int reason)
 }
 #endif /* CONFIG_SECCOMP_FILTER */
 
+static u32 seccomp_max_action_to_log = SECCOMP_RET_KILL;
+
+static void seccomp_log(unsigned long syscall, long signr, u32 action)
+{
+   /* Force an audit message to be emitted when the action is not greater
+* than the configured maximum action.
+*/
+   if (action <= seccomp_max_action_to_log)
+   return __audit_seccomp(syscall, signr, action);
+
+   /* If the action is not an ALLOW action, let the audit subsystem decide
+* if it should be audited based on whether the current task itself is
+* being audited.
+*/
+   if (action != SECCOMP_RET_ALLOW)
+   return audit_seccomp(syscall, signr, action);
+}
+
 /*
  * Secure computing mode 1 allows only read/write/exit/sigreturn.
  * To be fully secure this must be combined with rlimit
@@ -534,7 +552,7 @@ static void __secure_computing_strict(int this_syscall)
 #ifdef SECCOMP_DEBUG
dump_stack();
 #endif
-   audit_seccomp(this_syscall, SIGKILL, SECCOMP_RET_KILL);
+   seccomp_log(this_syscall, SIGKILL, SECCOMP_RET_KILL);
do_exit(SIGKILL);
 }
 
@@ -633,18 +651,19 @@ static int __seccomp_filter(int this_syscall, const 
struct seccomp_data *sd,
return 0;
 
case SECCOMP_RET_ALLOW:
+   seccomp_log(this_syscall, 0, action);
return 0;
 
case SECCOMP_RET_KILL:
default:
-   audit_seccomp(this_syscall, SIGSYS, action);
+   seccomp_log(this_syscall, SIGSYS, action);
do_exit(SIGSYS);
}
 
unreachable();
 
 skip:
-   audit_seccomp(this_syscall, 0, action);
+   seccomp_log(this_syscall, 0, action);
return -1;
 }
 #else
@@ -910,18 +929,102 @@ long seccomp_get_filter(struct task_struct *task, 
unsigned long filter_off,
 
 #ifdef CONFIG_SYSCTL
 
+/* Human readable action names for friendly sysctl interaction */
 #define SECCOMP_RET_KILL_NAME  "kill"
 #define SECCOMP_RET_TRAP_NAME  "trap"
 #define SECCOMP_RET_ERRNO_NAME "errno"
 #define SECCOMP_RET_TRACE_NAME "trace"
 #define SECCOMP_RET_ALLOW_NAME "allow"
 
+/* Largest strlen() of all action names */
+#define SECCOMP_RET_MAX_NAME_LEN   5
+
 static char seccomp_actions_avail[] = SECCOMP_RET_KILL_NAME" "
  SECCOMP_RET_TRAP_NAME " "
  SECCOMP_RET_ERRNO_NAME" "
  SECCOMP_RET_TRACE_NAME" "
  SECCOMP_RET_ALLOW_NAME;
 
+struct seccomp_action_name {
+   u32 action;
+   const char  *name;
+};
+
+static struct seccomp_action_name seccomp_action_names[] = {

[PATCH v2 4/4] seccomp: Add tests for SECCOMP_RET_LOG

[Tyler Hicks]

Extend the kernel selftests for seccomp to test the newly added
SECCOMP_RET_LOG action. The added tests follow the example of existing
tests.

Unfortunately, the tests are not capable of inspecting the audit log to
verify that the syscall was logged.

Signed-off-by: Tyler Hicks 
---
 tools/testing/selftests/seccomp/seccomp_bpf.c | 94 +++
 1 file changed, 94 insertions(+)

diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c 
b/tools/testing/selftests/seccomp/seccomp_bpf.c
index 03f1fa4..a39f620 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -87,6 +87,10 @@ struct seccomp_data {
 };
 #endif
 
+#ifndef SECCOMP_RET_LOG
+#define SECCOMP_RET_LOG   0x7ffeU /* allow after logging */
+#endif
+
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 #define syscall_arg(_n) (offsetof(struct seccomp_data, args[_n]))
 #elif __BYTE_ORDER == __BIG_ENDIAN
@@ -342,6 +346,28 @@ TEST(empty_prog)
EXPECT_EQ(EINVAL, errno);
 }
 
+TEST(log_all)
+{
+   struct sock_filter filter[] = {
+   BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_LOG),
+   };
+   struct sock_fprog prog = {
+   .len = (unsigned short)ARRAY_SIZE(filter),
+   .filter = filter,
+   };
+   long ret;
+   pid_t parent = getppid();
+
+   ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0);
+   ASSERT_EQ(0, ret);
+
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, );
+   ASSERT_EQ(0, ret);
+
+   /* getppid() should succeed and be logged (no check for logging) */
+   EXPECT_EQ(parent, syscall(__NR_getppid));
+}
+
 TEST_SIGNAL(unknown_ret_is_kill_inside, SIGSYS)
 {
struct sock_filter filter[] = {
@@ -735,6 +761,7 @@ TEST_F(TRAP, handler)
 
 FIXTURE_DATA(precedence) {
struct sock_fprog allow;
+   struct sock_fprog log;
struct sock_fprog trace;
struct sock_fprog error;
struct sock_fprog trap;
@@ -746,6 +773,13 @@ FIXTURE_SETUP(precedence)
struct sock_filter allow_insns[] = {
BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
};
+   struct sock_filter log_insns[] = {
+   BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
+   offsetof(struct seccomp_data, nr)),
+   BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, __NR_getpid, 1, 0),
+   BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+   BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_LOG),
+   };
struct sock_filter trace_insns[] = {
BPF_STMT(BPF_LD|BPF_W|BPF_ABS,
offsetof(struct seccomp_data, nr)),
@@ -782,6 +816,7 @@ FIXTURE_SETUP(precedence)
memcpy(self->_x.filter, &_x##_insns, sizeof(_x##_insns)); \
self->_x.len = (unsigned short)ARRAY_SIZE(_x##_insns)
FILTER_ALLOC(allow);
+   FILTER_ALLOC(log);
FILTER_ALLOC(trace);
FILTER_ALLOC(error);
FILTER_ALLOC(trap);
@@ -792,6 +827,7 @@ FIXTURE_TEARDOWN(precedence)
 {
 #define FILTER_FREE(_x) if (self->_x.filter) free(self->_x.filter)
FILTER_FREE(allow);
+   FILTER_FREE(log);
FILTER_FREE(trace);
FILTER_FREE(error);
FILTER_FREE(trap);
@@ -809,6 +845,8 @@ TEST_F(precedence, allow_ok)
 
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >allow);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
@@ -833,6 +871,8 @@ TEST_F_SIGNAL(precedence, kill_is_highest, SIGSYS)
 
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >allow);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
@@ -864,6 +904,8 @@ TEST_F_SIGNAL(precedence, kill_is_highest_in_any_order, 
SIGSYS)
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trap);
@@ -885,6 +927,8 @@ TEST_F_SIGNAL(precedence, trap_is_second, SIGSYS)
 
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >allow);
ASSERT_EQ(0, ret);
+   ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >log);
+   ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >trace);
ASSERT_EQ(0, ret);
ret = prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, >error);
@@ -910,6 +954,8 @@ TEST_F_SIGNAL(precedence, trap_is_second_in_any_order, 
SIGSYS)
ASSERT_EQ(0, ret);
  

[PATCH v2 0/4] Improved seccomp logging

[Tyler Hicks]

This patch set is the second revision of the following two previously
submitted patch sets:

http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhi...@canonical.com
http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhi...@canonical.com

The patch set aims to address some known deficiencies in seccomp's current
logging capabilities:

  1. Inability to log all filter actions.
  2. Inability to selectively enable filtering; e.g. devs want noisy logging,
 users want relative quiet.
  3. Consistent behavior with audit enabled and disabled.
  4. Inability to easily develop a filter due to the lack of a
 permissive/complain mode.

The first three items were outlined by Paul Moore and are issues that I agree
with. The last one is one that I'm particularly interested in.

I deviated a little from the plan that he laid out to address the third issue.
Looking back at Paul's feedback, he wanted a way to log seccomp actions even
when the audit subsystem is disabled at build time. I felt like the bigger
problem is that, while it is common for kernels to be built with audit support,
it is far less common to actually have auditd running. Therefore, my approach
was to improve the situation when kernel audit support is enabled at build time
but audit_enabled is false at runtime. The audit subsystem forwards messages
onto syslog in that situation.

Tyler

[PATCH v2 0/4] Improved seccomp logging

[Tyler Hicks]

This patch set is the second revision of the following two previously
submitted patch sets:

http://lkml.kernel.org/r/1483375990-14948-1-git-send-email-tyhi...@canonical.com
http://lkml.kernel.org/r/1483377999-15019-2-git-send-email-tyhi...@canonical.com

The patch set aims to address some known deficiencies in seccomp's current
logging capabilities:

  1. Inability to log all filter actions.
  2. Inability to selectively enable filtering; e.g. devs want noisy logging,
 users want relative quiet.
  3. Consistent behavior with audit enabled and disabled.
  4. Inability to easily develop a filter due to the lack of a
 permissive/complain mode.

The first three items were outlined by Paul Moore and are issues that I agree
with. The last one is one that I'm particularly interested in.

I deviated a little from the plan that he laid out to address the third issue.
Looking back at Paul's feedback, he wanted a way to log seccomp actions even
when the audit subsystem is disabled at build time. I felt like the bigger
problem is that, while it is common for kernels to be built with audit support,
it is far less common to actually have auditd running. Therefore, my approach
was to improve the situation when kernel audit support is enabled at build time
but audit_enabled is false at runtime. The audit subsystem forwards messages
onto syslog in that situation.

Tyler

[PATCH v4] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.
[Patch v1] is rejected as the changes done is not following the linux
coding style and [Patch v2] is rejected because forgot to mention the
cause of rejection of [Patch v1].The cause of rejection of [Patch v3] is
that the mention of the cause was not in the correct format.

Signed-off-by: Arushi 

---

In the [patch v1] to allign the * on each line in block was not correct
as it is also not following the correct coding style of kernel so it got
rejected and the details of all the other patch versions is given above.
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char *skip_conversion(char *input)
 /*
  * Function: find_specifier_end
  * Return a pointer to the end of the format specifier.
-*/
+ */
 static char *find_specifier_end(char *input)
 {
input++;/* Advance over %. */
@@ -478,7 +478,7 @@ static char *find_specifier_end(char *input)
  * Compare the format specifiers pointed to by *input1 and *input2.
  * Return 1 if they are the same, 0 otherwise.  Advance *input1 and *input2
  * so that they point to the character following the end of the specifier.
-*/
+ */
 static int compare_specifiers(char **input1, char **input2)
 {
int same = 0;
@@ -500,7 +500,7 @@ static int compare_specifiers(char **input1, char **input2)
  * Check that two format strings contain the same number of format specifiers,
  * and that the order of specifiers is the same in both strings.
  * Return 1 if the condition holds, 0 if it doesn't.
-*/
+ */
 static int fmt_validate(char *template, char *user)
 {
int valid = 1;
@@ -537,7 +537,7 @@ static int fmt_validate(char *template, char *user)
  * Failure conditions:
  * -EINVAL -  Invalid format specifiers in formatted message or illegal index.
  * -ENOMEM -  Unable to allocate memory.
-*/
+ */
 ssize_t spk_msg_set(enum msg_index_t index, char *text, size_t length)
 {
int rc = 0;
@@ -573,7 +573,7 @@ ssize_t spk_msg_set(enum msg_index_t index, char *text, 
size_t length)
 /*
  * Find a message group, given its name.  Return a pointer to the structure
  * if found, or NULL otherwise.
-*/
+ */
 struct msg_group_t *spk_find_msg_group(const char *group_name)
 {
struct msg_group_t *group = NULL;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index bf539d38..c2f70ef5b9b3 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -16,7 +16,7 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
-*/
+ */
 
 #include 
 #include 
diff --git 

[PATCH v4] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.
[Patch v1] is rejected as the changes done is not following the linux
coding style and [Patch v2] is rejected because forgot to mention the
cause of rejection of [Patch v1].The cause of rejection of [Patch v3] is
that the mention of the cause was not in the correct format.

Signed-off-by: Arushi 

---

In the [patch v1] to allign the * on each line in block was not correct
as it is also not following the correct coding style of kernel so it got
rejected and the details of all the other patch versions is given above.
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char *skip_conversion(char *input)
 /*
  * Function: find_specifier_end
  * Return a pointer to the end of the format specifier.
-*/
+ */
 static char *find_specifier_end(char *input)
 {
input++;/* Advance over %. */
@@ -478,7 +478,7 @@ static char *find_specifier_end(char *input)
  * Compare the format specifiers pointed to by *input1 and *input2.
  * Return 1 if they are the same, 0 otherwise.  Advance *input1 and *input2
  * so that they point to the character following the end of the specifier.
-*/
+ */
 static int compare_specifiers(char **input1, char **input2)
 {
int same = 0;
@@ -500,7 +500,7 @@ static int compare_specifiers(char **input1, char **input2)
  * Check that two format strings contain the same number of format specifiers,
  * and that the order of specifiers is the same in both strings.
  * Return 1 if the condition holds, 0 if it doesn't.
-*/
+ */
 static int fmt_validate(char *template, char *user)
 {
int valid = 1;
@@ -537,7 +537,7 @@ static int fmt_validate(char *template, char *user)
  * Failure conditions:
  * -EINVAL -  Invalid format specifiers in formatted message or illegal index.
  * -ENOMEM -  Unable to allocate memory.
-*/
+ */
 ssize_t spk_msg_set(enum msg_index_t index, char *text, size_t length)
 {
int rc = 0;
@@ -573,7 +573,7 @@ ssize_t spk_msg_set(enum msg_index_t index, char *text, 
size_t length)
 /*
  * Find a message group, given its name.  Return a pointer to the structure
  * if found, or NULL otherwise.
-*/
+ */
 struct msg_group_t *spk_find_msg_group(const char *group_name)
 {
struct msg_group_t *group = NULL;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index bf539d38..c2f70ef5b9b3 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -16,7 +16,7 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
-*/
+ */
 
 #include 
 #include 
diff --git 

linux-next: Tree for Feb 3

[Stephen Rothwell]

Hi all,

Changes since 20170202:

Dropped tree: vfs-miklos (build failure and out of date)

The powerpc tree gained a conflict against the powerpc-fixes tree.

The vfs-miklos tree still had its build failure, so I just dropped it
again for today.

The v4l-dvb tree still had its build failure so I used the version from
next-20170130.

The regulator tree still had its build failure so I used the version from
next-20170131.

The rcu tree gained conflicts against the powerpc and netfilter-next
trees.

The akpm tree gained a conflict against the block tree.

Non-merge commits (relative to Linus' tree): 6884
 7479 files changed, 292247 insertions(+), 145339 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc and an allmodconfig (with
CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a
native build of tools/perf. After the final fixups (if any), I do an
x86_64 modules_install followed by builds for x86_64 allnoconfig,
powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig
and pseries_le_defconfig and i386, sparc and sparc64 defconfig.

Below is a summary of the state of the merge.

I am currently merging 254 trees (counting Linus' and 36 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (6d04dfc89660 Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging fixes/master (30066ce675d3 Merge branch 'linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6)
Merging kbuild-current/rc-fixes (c7858bf16c0b asm-prototypes: Clear any CPP 
defines before declaring the functions)
Merging arc-current/for-curr (566cf877a1fc Linux 4.10-rc6)
Merging arm-current/fixes (228dbbfb5d77 ARM: 8643/3: arm/ptrace: Preserve 
previous registers for short regset write)
Merging m68k-current/for-linus (ad595b77c4a8 m68k/atari: Use seq_puts() in 
atari_get_hardware_list())
Merging metag-fixes/fixes (35d04077ad96 metag: Only define 
atomic_dec_if_positive conditionally)
Merging powerpc-fixes/fixes (a0615a16f7d0 powerpc/mm: Use the correct pointer 
when setting a 2MB pte)
Merging sparc/master (f9a42e0d58cf Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc)
Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and 
linking special files)
Merging net/master (6d04dfc89660 Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging ipsec/master (4e5da369df64 Documentation/networking: fix typo in 
mpls-sysctl)
Merging netfilter/master (a47b70ea86bd ravb: unmap descriptors when freeing 
rings)
Merging ipvs/master (045169816b31 Merge branch 'linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6)
Merging wireless-drivers/master (52f5631a4c05 rtlwifi: rtl8192ce: Fix loading 
of incorrect firmware)
Merging mac80211/master (115865fa0826 mac80211: don't try to sleep in 
rate_control_rate_init())
Merging sound-current/for-linus (6cf4569ce356 Merge tag 'asoc-fix-v4.10-rc3' of 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus)
Merging pci-current/for-linus (030305d69fc6 PCI/ASPM: Handle PCI-to-PCIe 
bridges as roots of PCIe hierarchies)
Merging driver-core.current/driver-core-linus (49def1853334 Linux 4.10-rc4)
Merging tty.current/tty-linus (49def1853334 Linux 4.10-rc4)
Merging usb.current/usb-linus (a3683e0c1410 Merge tag 'usb-serial-4.10-rc6' of 
git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into usb-linus)
Merging usb-gadget-fixes/fixes (efe357f4633a usb: dwc2: host: fix 
Wmaybe-uninitialized warning)
Merging usb-serial-fixes/usb-linus (d07830db1bdb USB: serial: pl2303: add ATEN 
device ID)
Merging usb-chipidea-fixes/ci-for-usb-stable (c7fbb09b2ea1 usb: chipidea: move 
the lock initialization to core file)
Merging phy/fixes (7ce7d89f4883 Linux 4.10-rc1)
Merging staging.c

linux-next: Tree for Feb 3

[Stephen Rothwell]

Hi all,

Changes since 20170202:

Dropped tree: vfs-miklos (build failure and out of date)

The powerpc tree gained a conflict against the powerpc-fixes tree.

The vfs-miklos tree still had its build failure, so I just dropped it
again for today.

The v4l-dvb tree still had its build failure so I used the version from
next-20170130.

The regulator tree still had its build failure so I used the version from
next-20170131.

The rcu tree gained conflicts against the powerpc and netfilter-next
trees.

The akpm tree gained a conflict against the block tree.

Non-merge commits (relative to Linus' tree): 6884
 7479 files changed, 292247 insertions(+), 145339 deletions(-)



I have created today's linux-next tree at
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
(patches at http://www.kernel.org/pub/linux/kernel/next/ ).  If you
are tracking the linux-next tree using git, you should not use "git pull"
to do so as that will try to merge the new linux-next release with the
old one.  You should use "git fetch" and checkout or reset to the new
master.

You can see which trees have been included by looking in the Next/Trees
file in the source.  There are also quilt-import.log and merge.log
files in the Next directory.  Between each merge, the tree was built
with a ppc64_defconfig for powerpc and an allmodconfig (with
CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a
native build of tools/perf. After the final fixups (if any), I do an
x86_64 modules_install followed by builds for x86_64 allnoconfig,
powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig
and pseries_le_defconfig and i386, sparc and sparc64 defconfig.

Below is a summary of the state of the merge.

I am currently merging 254 trees (counting Linus' and 36 trees of bug
fix patches pending for the current merge release).

Stats about the size of the tree over time can be seen at
http://neuling.org/linux-next-size.html .

Status of my local build tests will be at
http://kisskb.ellerman.id.au/linux-next .  If maintainers want to give
advice about cross compilers/configs that work, we are always open to add
more builds.

Thanks to Randy Dunlap for doing many randconfig builds.  And to Paul
Gortmaker for triage and bug fixes.

-- 
Cheers,
Stephen Rothwell

$ git checkout master
$ git reset --hard stable
Merging origin/master (6d04dfc89660 Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging fixes/master (30066ce675d3 Merge branch 'linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6)
Merging kbuild-current/rc-fixes (c7858bf16c0b asm-prototypes: Clear any CPP 
defines before declaring the functions)
Merging arc-current/for-curr (566cf877a1fc Linux 4.10-rc6)
Merging arm-current/fixes (228dbbfb5d77 ARM: 8643/3: arm/ptrace: Preserve 
previous registers for short regset write)
Merging m68k-current/for-linus (ad595b77c4a8 m68k/atari: Use seq_puts() in 
atari_get_hardware_list())
Merging metag-fixes/fixes (35d04077ad96 metag: Only define 
atomic_dec_if_positive conditionally)
Merging powerpc-fixes/fixes (a0615a16f7d0 powerpc/mm: Use the correct pointer 
when setting a 2MB pte)
Merging sparc/master (f9a42e0d58cf Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/sparc)
Merging fscrypt-current/for-stable (42d97eb0ade3 fscrypt: fix renaming and 
linking special files)
Merging net/master (6d04dfc89660 Merge 
git://git.kernel.org/pub/scm/linux/kernel/git/davem/net)
Merging ipsec/master (4e5da369df64 Documentation/networking: fix typo in 
mpls-sysctl)
Merging netfilter/master (a47b70ea86bd ravb: unmap descriptors when freeing 
rings)
Merging ipvs/master (045169816b31 Merge branch 'linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6)
Merging wireless-drivers/master (52f5631a4c05 rtlwifi: rtl8192ce: Fix loading 
of incorrect firmware)
Merging mac80211/master (115865fa0826 mac80211: don't try to sleep in 
rate_control_rate_init())
Merging sound-current/for-linus (6cf4569ce356 Merge tag 'asoc-fix-v4.10-rc3' of 
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into for-linus)
Merging pci-current/for-linus (030305d69fc6 PCI/ASPM: Handle PCI-to-PCIe 
bridges as roots of PCIe hierarchies)
Merging driver-core.current/driver-core-linus (49def1853334 Linux 4.10-rc4)
Merging tty.current/tty-linus (49def1853334 Linux 4.10-rc4)
Merging usb.current/usb-linus (a3683e0c1410 Merge tag 'usb-serial-4.10-rc6' of 
git://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into usb-linus)
Merging usb-gadget-fixes/fixes (efe357f4633a usb: dwc2: host: fix 
Wmaybe-uninitialized warning)
Merging usb-serial-fixes/usb-linus (d07830db1bdb USB: serial: pl2303: add ATEN 
device ID)
Merging usb-chipidea-fixes/ci-for-usb-stable (c7fbb09b2ea1 usb: chipidea: move 
the lock initialization to core file)
Merging phy/fixes (7ce7d89f4883 Linux 4.10-rc1)
Merging staging.c

[RFC v2] [ALSA][CONTROL] 1. Added 2 ioctls for reading/writing values of multiple controls in one go (at once) 2. Restructured the code for read/write of a single control

[Satendra Singh Thakur]

From: satendra singh thakur 

1.Added 2 ioctls in alsa driver's control interface
-Added an ioctl to read values of multiple elements at once
-Added an ioctl to write values of multiple elements at once
-In the absence of above ioctls user needs to call N ioctls to
 read/write value of N elements which requires N context switches
-Proposed ioctls will allow accessing N elements' values in a single
 context switch
-Above mentioned ioctl will be useful for alsa utils such as amixer
 which reads all controls of given sound card
2. Restructured/Combined two functions snd_ctl_elem_read_user and
snd_ctl_elem_write_user into a single function snd_ctl_elem_rw_user
-These functions were having most of the code which was similar
 to each other
-Thus, there was redundant/duplicate code which was removed
 and a single function was formed/defined.
-Removed functions snd_ctl_elem_read_user and snd_ctl_elem_write_user
 having redundant/duplicate code
3. This is version 2 of RFC, here we combine previous 2 patches
 submitted yesterday (02-Feb) with prefix [RFC] [ALSA][CONTROL]
-Fixed doxygen comments related warnings
-Fixed stack frame related warning

Signed-off-by: Satendra Singh Thakur 
---
 include/uapi/sound/asound.h |8 +++
 sound/core/control.c|  130 +--
 2 files changed, 109 insertions(+), 29 deletions(-)

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index be353a7..a0dcee7 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -948,6 +948,11 @@ struct snd_ctl_tlv {
unsigned int length;/* in bytes aligned to 4 */
unsigned int tlv[0];/* first TLV */
 };
+/*Struct to read/write multiple element values */
+struct snd_ctl_elem_values {
+   unsigned int num_vals;/* Number of values*/
+   struct snd_ctl_elem_value *pvals;/* Pointer to the array of values */
+};
 
 #define SNDRV_CTL_IOCTL_PVERSION   _IOR('U', 0x00, int)
 #define SNDRV_CTL_IOCTL_CARD_INFO  _IOR('U', 0x01, struct 
snd_ctl_card_info)
@@ -974,6 +979,9 @@ struct snd_ctl_tlv {
 #define SNDRV_CTL_IOCTL_RAWMIDI_PREFER_SUBDEVICE _IOW('U', 0x42, int)
 #define SNDRV_CTL_IOCTL_POWER  _IOWR('U', 0xd0, int)
 #define SNDRV_CTL_IOCTL_POWER_STATE_IOR('U', 0xd1, int)
+/*Multipe controls' values read and write*/
+#define SNDRV_CTL_IOCTL_ELEMS_READ _IOWR('U', 0xe0, struct 
snd_ctl_elem_values)
+#define SNDRV_CTL_IOCTL_ELEMS_WRITE_IOWR('U', 0xe1, struct 
snd_ctl_elem_values)
 
 /*
  *  Read interface.
diff --git a/sound/core/control.c b/sound/core/control.c
index fb096cb..980046c 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -920,27 +920,6 @@ static int snd_ctl_elem_read(struct snd_card *card,
return result;
 }
 
-static int snd_ctl_elem_read_user(struct snd_card *card,
- struct snd_ctl_elem_value __user *_control)
-{
-   struct snd_ctl_elem_value *control;
-   int result;
-
-   control = memdup_user(_control, sizeof(*control));
-   if (IS_ERR(control))
-   return PTR_ERR(control);
-
-   snd_power_lock(card);
-   result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-   if (result >= 0)
-   result = snd_ctl_elem_read(card, control);
-   snd_power_unlock(card);
-   if (result >= 0)
-   if (copy_to_user(_control, control, sizeof(*control)))
-   result = -EFAULT;
-   kfree(control);
-   return result;
-}
 
 static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file,
  struct snd_ctl_elem_value *control)
@@ -976,22 +955,38 @@ static int snd_ctl_elem_write(struct snd_card *card, 
struct snd_ctl_file *file,
return result;
 }
 
-static int snd_ctl_elem_write_user(struct snd_ctl_file *file,
-  struct snd_ctl_elem_value __user *_control)
+/**
+ * snd_ctl_elem_rw_user - Read/write value of an element
+ * @file: pointer to control file object
+ * @_control: user pointer to struct snd_ctl_elem_value
+ * @write_flag: flag, true for write, false for read operation
+ *
+ * This function reads the value of controls with the given IDs
+ * of the same card
+ * Return: On success, it returns positive number returned by
+ * snd_ctl_elem_write/snd_ctl_elem_read
+ * On failure, it returns appropriate error
+ */
+
+static int snd_ctl_elem_rw_user(struct snd_ctl_file *file,
+ struct snd_ctl_elem_value __user *_control,
+   bool write_flag)
 {
struct snd_ctl_elem_value *control;
struct snd_card *card;
int result;
-
control = memdup_user(_control, sizeof(*control));
if (IS_ERR(control))
return PTR_ERR(control);
-
card = file->card;
snd_power_lock(card);
result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-   if (result >= 0)

[RFC v2] [ALSA][CONTROL] 1. Added 2 ioctls for reading/writing values of multiple controls in one go (at once) 2. Restructured the code for read/write of a single control

[Satendra Singh Thakur]

From: satendra singh thakur 

1.Added 2 ioctls in alsa driver's control interface
-Added an ioctl to read values of multiple elements at once
-Added an ioctl to write values of multiple elements at once
-In the absence of above ioctls user needs to call N ioctls to
 read/write value of N elements which requires N context switches
-Proposed ioctls will allow accessing N elements' values in a single
 context switch
-Above mentioned ioctl will be useful for alsa utils such as amixer
 which reads all controls of given sound card
2. Restructured/Combined two functions snd_ctl_elem_read_user and
snd_ctl_elem_write_user into a single function snd_ctl_elem_rw_user
-These functions were having most of the code which was similar
 to each other
-Thus, there was redundant/duplicate code which was removed
 and a single function was formed/defined.
-Removed functions snd_ctl_elem_read_user and snd_ctl_elem_write_user
 having redundant/duplicate code
3. This is version 2 of RFC, here we combine previous 2 patches
 submitted yesterday (02-Feb) with prefix [RFC] [ALSA][CONTROL]
-Fixed doxygen comments related warnings
-Fixed stack frame related warning

Signed-off-by: Satendra Singh Thakur 
---
 include/uapi/sound/asound.h |8 +++
 sound/core/control.c|  130 +--
 2 files changed, 109 insertions(+), 29 deletions(-)

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index be353a7..a0dcee7 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -948,6 +948,11 @@ struct snd_ctl_tlv {
unsigned int length;/* in bytes aligned to 4 */
unsigned int tlv[0];/* first TLV */
 };
+/*Struct to read/write multiple element values */
+struct snd_ctl_elem_values {
+   unsigned int num_vals;/* Number of values*/
+   struct snd_ctl_elem_value *pvals;/* Pointer to the array of values */
+};
 
 #define SNDRV_CTL_IOCTL_PVERSION   _IOR('U', 0x00, int)
 #define SNDRV_CTL_IOCTL_CARD_INFO  _IOR('U', 0x01, struct 
snd_ctl_card_info)
@@ -974,6 +979,9 @@ struct snd_ctl_tlv {
 #define SNDRV_CTL_IOCTL_RAWMIDI_PREFER_SUBDEVICE _IOW('U', 0x42, int)
 #define SNDRV_CTL_IOCTL_POWER  _IOWR('U', 0xd0, int)
 #define SNDRV_CTL_IOCTL_POWER_STATE_IOR('U', 0xd1, int)
+/*Multipe controls' values read and write*/
+#define SNDRV_CTL_IOCTL_ELEMS_READ _IOWR('U', 0xe0, struct 
snd_ctl_elem_values)
+#define SNDRV_CTL_IOCTL_ELEMS_WRITE_IOWR('U', 0xe1, struct 
snd_ctl_elem_values)
 
 /*
  *  Read interface.
diff --git a/sound/core/control.c b/sound/core/control.c
index fb096cb..980046c 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -920,27 +920,6 @@ static int snd_ctl_elem_read(struct snd_card *card,
return result;
 }
 
-static int snd_ctl_elem_read_user(struct snd_card *card,
- struct snd_ctl_elem_value __user *_control)
-{
-   struct snd_ctl_elem_value *control;
-   int result;
-
-   control = memdup_user(_control, sizeof(*control));
-   if (IS_ERR(control))
-   return PTR_ERR(control);
-
-   snd_power_lock(card);
-   result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-   if (result >= 0)
-   result = snd_ctl_elem_read(card, control);
-   snd_power_unlock(card);
-   if (result >= 0)
-   if (copy_to_user(_control, control, sizeof(*control)))
-   result = -EFAULT;
-   kfree(control);
-   return result;
-}
 
 static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file,
  struct snd_ctl_elem_value *control)
@@ -976,22 +955,38 @@ static int snd_ctl_elem_write(struct snd_card *card, 
struct snd_ctl_file *file,
return result;
 }
 
-static int snd_ctl_elem_write_user(struct snd_ctl_file *file,
-  struct snd_ctl_elem_value __user *_control)
+/**
+ * snd_ctl_elem_rw_user - Read/write value of an element
+ * @file: pointer to control file object
+ * @_control: user pointer to struct snd_ctl_elem_value
+ * @write_flag: flag, true for write, false for read operation
+ *
+ * This function reads the value of controls with the given IDs
+ * of the same card
+ * Return: On success, it returns positive number returned by
+ * snd_ctl_elem_write/snd_ctl_elem_read
+ * On failure, it returns appropriate error
+ */
+
+static int snd_ctl_elem_rw_user(struct snd_ctl_file *file,
+ struct snd_ctl_elem_value __user *_control,
+   bool write_flag)
 {
struct snd_ctl_elem_value *control;
struct snd_card *card;
int result;
-
control = memdup_user(_control, sizeof(*control));
if (IS_ERR(control))
return PTR_ERR(control);
-
card = file->card;
snd_power_lock(card);
result = snd_power_wait(card, SNDRV_CTL_POWER_D0);
-   if (result >= 0)
-   result = 

Re: [PATCH 1/2 v3] mm: vmscan: do not pass reclaimed slab to vmpressure

[vinayak menon]

On Thu, Feb 2, 2017 at 9:31 PM, Michal Hocko  wrote:
>
> Why would you like to chose and kill a task when the slab reclaim can
> still make sufficient progres? Are you sure that the slab contribution
> to the stats makes all the above happening?
>
I agree that a task need not be killed if sufficient progress is made
in reclaiming
memory say from slab. But here it looks like we have an impact because of just
increasing the reclaimed without touching the scanned. It could be because of
disimilar costs or not adding adding cost. I agree that vmpressure is
only a reasonable
estimate which does not already include few other costs, but I am not
sure whether it is ok
to add another element which further increases that disparity.
We noticed this problem when moving from 3.18 to 4.4 kernel version. With the
same workload, the vmpressure events differ between 3.18 and 4.4 causing the
above mentioned problem. And with this patch on 4.4 we get the same results
as in 3,18. So the slab contribution to stats is making a difference.

>> This increases the memory pressure and
>> finally result in late critical events, but by that time the task
>> launch latencies are impacted.
>

> I have seen vmpressure hitting critical events really quickly but that
> is mostly because the vmpressure uses only very simplistic
> approximation. Usually the reclaim goes well, until you hit to dirty
> or pinned pages. Then it can get really bad, so you can get from high
> effectiveness to 0 pretty quickly.
> --
> Michal Hocko
> SUSE Labs

Re: [PATCH 1/2 v3] mm: vmscan: do not pass reclaimed slab to vmpressure

[vinayak menon]

On Thu, Feb 2, 2017 at 9:31 PM, Michal Hocko  wrote:
>
> Why would you like to chose and kill a task when the slab reclaim can
> still make sufficient progres? Are you sure that the slab contribution
> to the stats makes all the above happening?
>
I agree that a task need not be killed if sufficient progress is made
in reclaiming
memory say from slab. But here it looks like we have an impact because of just
increasing the reclaimed without touching the scanned. It could be because of
disimilar costs or not adding adding cost. I agree that vmpressure is
only a reasonable
estimate which does not already include few other costs, but I am not
sure whether it is ok
to add another element which further increases that disparity.
We noticed this problem when moving from 3.18 to 4.4 kernel version. With the
same workload, the vmpressure events differ between 3.18 and 4.4 causing the
above mentioned problem. And with this patch on 4.4 we get the same results
as in 3,18. So the slab contribution to stats is making a difference.

>> This increases the memory pressure and
>> finally result in late critical events, but by that time the task
>> launch latencies are impacted.
>

> I have seen vmpressure hitting critical events really quickly but that
> is mostly because the vmpressure uses only very simplistic
> approximation. Usually the reclaim goes well, until you hit to dirty
> or pinned pages. Then it can get really bad, so you can get from high
> effectiveness to 0 pretty quickly.
> --
> Michal Hocko
> SUSE Labs

[PATCH v4] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.
[Patch v1] is rejected as the changes done is not following the linux
coding style and [Patch v2] is rejected because forgot to mention the
cause of rejection of [Patch v1].The cause of rejection of [Patch v3] is
that the mention of the cause was not in the correct format.

Signed-off-by: Arushi 

---

In the [patch v1] to allign the * on each line in block was not correct
as it is also not following the correct coding style of kernel so it got
rejected and the details of all the other patch versions is given above.
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char *skip_conversion(char *input)
 /*
  * Function: find_specifier_end
  * Return a pointer to the end of the format specifier.
-*/
+ */
 static char *find_specifier_end(char *input)
 {
input++;/* Advance over %. */
@@ -478,7 +478,7 @@ static char *find_specifier_end(char *input)
  * Compare the format specifiers pointed to by *input1 and *input2.
  * Return 1 if they are the same, 0 otherwise.  Advance *input1 and *input2
  * so that they point to the character following the end of the specifier.
-*/
+ */
 static int compare_specifiers(char **input1, char **input2)
 {
int same = 0;
@@ -500,7 +500,7 @@ static int compare_specifiers(char **input1, char **input2)
  * Check that two format strings contain the same number of format specifiers,
  * and that the order of specifiers is the same in both strings.
  * Return 1 if the condition holds, 0 if it doesn't.
-*/
+ */
 static int fmt_validate(char *template, char *user)
 {
int valid = 1;
@@ -537,7 +537,7 @@ static int fmt_validate(char *template, char *user)
  * Failure conditions:
  * -EINVAL -  Invalid format specifiers in formatted message or illegal index.
  * -ENOMEM -  Unable to allocate memory.
-*/
+ */
 ssize_t spk_msg_set(enum msg_index_t index, char *text, size_t length)
 {
int rc = 0;
@@ -573,7 +573,7 @@ ssize_t spk_msg_set(enum msg_index_t index, char *text, 
size_t length)
 /*
  * Find a message group, given its name.  Return a pointer to the structure
  * if found, or NULL otherwise.
-*/
+ */
 struct msg_group_t *spk_find_msg_group(const char *group_name)
 {
struct msg_group_t *group = NULL;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index bf539d38..c2f70ef5b9b3 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -16,7 +16,7 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
-*/
+ */
 
 #include 
 #include 
diff --git 

[PATCH v4] staging: rtl8192e: Aligning the * on each line in block comments

[Arushi]

This patch fixes the issue by aligning the * on each line in block comments.
[Patch v1] is rejected as the changes done is not following the linux
coding style and [Patch v2] is rejected because forgot to mention the
cause of rejection of [Patch v1].The cause of rejection of [Patch v3] is
that the mention of the cause was not in the correct format.

Signed-off-by: Arushi 

---

In the [patch v1] to allign the * on each line in block was not correct
as it is also not following the correct coding style of kernel so it got
rejected and the details of all the other patch versions is given above.
---
 drivers/staging/speakup/fakekey.c| 10 +-
 drivers/staging/speakup/i18n.c   | 14 +++---
 drivers/staging/speakup/main.c   |  2 +-
 drivers/staging/speakup/speakup_acntsa.c |  2 +-
 drivers/staging/speakup/speakup_apollo.c |  2 +-
 drivers/staging/speakup/speakup_decext.c |  2 +-
 drivers/staging/speakup/speakup_decpc.c  |  4 ++--
 drivers/staging/speakup/speakup_dtlk.c   |  2 +-
 drivers/staging/speakup/speakup_dtlk.h   | 10 +-
 drivers/staging/speakup/speakup_ltlk.c   |  2 +-
 10 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/speakup/fakekey.c 
b/drivers/staging/speakup/fakekey.c
index 8f058b42f68d..d76da0a1382c 100644
--- a/drivers/staging/speakup/fakekey.c
+++ b/drivers/staging/speakup/fakekey.c
@@ -63,8 +63,8 @@ void speakup_remove_virtual_keyboard(void)
 }
 
 /*
-* Send a simulated down-arrow to the application.
-*/
+ * Send a simulated down-arrow to the application.
+ */
 void speakup_fake_down_arrow(void)
 {
unsigned long flags;
@@ -87,9 +87,9 @@ void speakup_fake_down_arrow(void)
 }
 
 /*
-* Are we handling a simulated keypress on the current CPU?
-* Returns a boolean.
-*/
+ * Are we handling a simulated keypress on the current CPU?
+ * Returns a boolean.
+ */
 bool speakup_fake_key_pressed(void)
 {
return this_cpu_read(reporting_keystroke);
diff --git a/drivers/staging/speakup/i18n.c b/drivers/staging/speakup/i18n.c
index 8960079e4d60..2f9b3df7f78d 100644
--- a/drivers/staging/speakup/i18n.c
+++ b/drivers/staging/speakup/i18n.c
@@ -401,7 +401,7 @@ char *spk_msg_get(enum msg_index_t index)
  * Finds the start of the next format specifier in the argument string.
  * Return value: pointer to start of format
  * specifier, or NULL if no specifier exists.
-*/
+ */
 static char *next_specifier(char *input)
 {
int found = 0;
@@ -450,7 +450,7 @@ static char *skip_width(char *input)
  * Note that this code only accepts a handful of conversion specifiers:
  * c d s x and ld.  Not accidental; these are exactly the ones used in
  * the default group of formatted messages.
-*/
+ */
 static char *skip_conversion(char *input)
 {
if ((input[0] == 'l') && (input[1] == 'd'))
@@ -463,7 +463,7 @@ static char *skip_conversion(char *input)
 /*
  * Function: find_specifier_end
  * Return a pointer to the end of the format specifier.
-*/
+ */
 static char *find_specifier_end(char *input)
 {
input++;/* Advance over %. */
@@ -478,7 +478,7 @@ static char *find_specifier_end(char *input)
  * Compare the format specifiers pointed to by *input1 and *input2.
  * Return 1 if they are the same, 0 otherwise.  Advance *input1 and *input2
  * so that they point to the character following the end of the specifier.
-*/
+ */
 static int compare_specifiers(char **input1, char **input2)
 {
int same = 0;
@@ -500,7 +500,7 @@ static int compare_specifiers(char **input1, char **input2)
  * Check that two format strings contain the same number of format specifiers,
  * and that the order of specifiers is the same in both strings.
  * Return 1 if the condition holds, 0 if it doesn't.
-*/
+ */
 static int fmt_validate(char *template, char *user)
 {
int valid = 1;
@@ -537,7 +537,7 @@ static int fmt_validate(char *template, char *user)
  * Failure conditions:
  * -EINVAL -  Invalid format specifiers in formatted message or illegal index.
  * -ENOMEM -  Unable to allocate memory.
-*/
+ */
 ssize_t spk_msg_set(enum msg_index_t index, char *text, size_t length)
 {
int rc = 0;
@@ -573,7 +573,7 @@ ssize_t spk_msg_set(enum msg_index_t index, char *text, 
size_t length)
 /*
  * Find a message group, given its name.  Return a pointer to the structure
  * if found, or NULL otherwise.
-*/
+ */
 struct msg_group_t *spk_find_msg_group(const char *group_name)
 {
struct msg_group_t *group = NULL;
diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
index bf539d38..c2f70ef5b9b3 100644
--- a/drivers/staging/speakup/main.c
+++ b/drivers/staging/speakup/main.c
@@ -16,7 +16,7 @@
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  *  GNU General Public License for more details.
-*/
+ */
 
 #include 
 #include 
diff --git 

Re: [PATCH 4.9 00/51] 4.9.8-stable review

[Guenter Roeck]

On 02/02/2017 10:37 AM, Greg Kroah-Hartman wrote:

This is the start of the stable review cycle for the 4.9.8 release.
There are 51 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Sat Feb  4 18:33:31 UTC 2017.
Anything received after that time might be too late.



Build results:
total: 149 pass: 149 fail: 0
Qemu test results:
total: 122 pass: 122 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter

Re: [PATCH 4.9 00/51] 4.9.8-stable review

[Guenter Roeck]

On 02/02/2017 10:37 AM, Greg Kroah-Hartman wrote:

This is the start of the stable review cycle for the 4.9.8 release.
There are 51 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Sat Feb  4 18:33:31 UTC 2017.
Anything received after that time might be too late.



Build results:
total: 149 pass: 149 fail: 0
Qemu test results:
total: 122 pass: 122 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter

Re: [PATCH 4.4 00/20] 4.4.47-stable review

[Guenter Roeck]

On 02/02/2017 10:33 AM, Greg Kroah-Hartman wrote:

This is the start of the stable review cycle for the 4.4.47 release.
There are 20 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Sat Feb  4 18:32:49 UTC 2017.
Anything received after that time might be too late.



Build results:
total: 149 pass: 149 fail: 0
Qemu test results:
total: 115 pass: 115 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter

Re: [PATCH 4.4 00/20] 4.4.47-stable review

[Guenter Roeck]

On 02/02/2017 10:33 AM, Greg Kroah-Hartman wrote:

This is the start of the stable review cycle for the 4.4.47 release.
There are 20 patches in this series, all will be posted as a response
to this one.  If anyone has any issues with these being applied, please
let me know.

Responses should be made by Sat Feb  4 18:32:49 UTC 2017.
Anything received after that time might be too late.



Build results:
total: 149 pass: 149 fail: 0
Qemu test results:
total: 115 pass: 115 fail: 0

Details are available at http://kerneltests.org/builders.

Guenter

Re: [PATCH v2] EDAC: mpc85xx: Add T2080 l2-cache support

[Michael Ellerman]

Chris Packham  writes:

> On 03/02/17 12:55, Michael Ellerman wrote:
>> Chris if you want to send a patch to add the compatible string to the
>> l2cache.txt I would merge that, but honestly it doesn't achieve much
>> other than possibly catching a typo in the compatible name.
>
> I think catching a typo might be worthwhile. It's 5 minutes work for me 
> to grep/sed through the code to find existing compatible strings and 
> update the document. Which might save someone else a lot of time 
> debugging only to find out they've transposed some digits in the dts.
>
> I'll whip something up and send it out shortly.

Thanks.

cheers

Re: [PATCH v2] EDAC: mpc85xx: Add T2080 l2-cache support

[Michael Ellerman]

Chris Packham  writes:

> On 03/02/17 12:55, Michael Ellerman wrote:
>> Chris if you want to send a patch to add the compatible string to the
>> l2cache.txt I would merge that, but honestly it doesn't achieve much
>> other than possibly catching a typo in the compatible name.
>
> I think catching a typo might be worthwhile. It's 5 minutes work for me 
> to grep/sed through the code to find existing compatible strings and 
> update the document. Which might save someone else a lot of time 
> debugging only to find out they've transposed some digits in the dts.
>
> I'll whip something up and send it out shortly.

Thanks.

cheers

Re: modversions: redefine kcrctab entries as 32-bit values

[Jessica Yu]

+++ Jessica Yu [02/02/17 22:54 -0500]:

+++ Ard Biesheuvel [24/01/17 16:16 +]:

This v4 is a followup to [0] 'modversions: redefine kcrctab entries as
relative CRC pointers', but since relative CRC pointers do not work in
modules, and are actually only needed by powerpc with CONFIG_RELOCATABLE=y,
I have made it a Kconfig selectable feature instead.

Patch #1 introduces the MODULE_REL_CRCS Kconfig symbol, and adds the kbuild
handling of it, i.e., modpost, genksyms and kallsyms.

Patch #2 switches all architectures to 32-bit CRC entries in kcrctab, where
all architectures except powerpc with CONFIG_RELOCATABLE=y use absolute ELF
symbol references as before.

v4: make relative CRCs kconfig selectable
  use absolute CRC symbols in modules regardless of kconfig selection
  split into two patches


This asymmetry threw me off a bit, especially the Kconfig naming (only
vmlinux crcs get the relative offsets, and only on powerpc atm, but all
modules keep the absolute syms, but it is called MODULE_REL_CRCS...),
if we keep this asymmetric crc treatment, it would be really nice to
note this discrepancy this somewhere, perhaps in the Kconfig, to keep
our heads from spinning :-)

I'm still catching up on the previous discussion threads, but can you
explain a bit more why you switched away from full blown relative crcs
from your last patchset [1]? I had lightly tested your v3 on ppc64le
previously, and relative offsets with modules seemed to worked very
well. I'm probably missing something very obvious.


Ah, I just saw your other comment about other arches not having support
for the rel32 offsets :-/

The asymmetry still bothers me though. Can't we have something that just
switches relative crcs on and off, that applies to *both* vmlinux and modules?
Then we can get rid of the crc_owner check in check_version() and just have
something like:

  if (IS_ENABLED(CONFIG_RELATIVE_CRCS))
  crcval = resolve_crc(crc);

Also we could get rid of the '&& !defined(MODULE)' checks scattered in
export.h. Then the arches that want relative crcs and that *do* have rel32
relocation support can turn relative crcs on, and powerpc can enable it, right?

Would that work or is there another reason this won't work with modules
(assuming that the arches that select this option support the relative
offsets)?

Jessica

Re: modversions: redefine kcrctab entries as 32-bit values

[Jessica Yu]

+++ Jessica Yu [02/02/17 22:54 -0500]:

+++ Ard Biesheuvel [24/01/17 16:16 +]:

This v4 is a followup to [0] 'modversions: redefine kcrctab entries as
relative CRC pointers', but since relative CRC pointers do not work in
modules, and are actually only needed by powerpc with CONFIG_RELOCATABLE=y,
I have made it a Kconfig selectable feature instead.

Patch #1 introduces the MODULE_REL_CRCS Kconfig symbol, and adds the kbuild
handling of it, i.e., modpost, genksyms and kallsyms.

Patch #2 switches all architectures to 32-bit CRC entries in kcrctab, where
all architectures except powerpc with CONFIG_RELOCATABLE=y use absolute ELF
symbol references as before.

v4: make relative CRCs kconfig selectable
  use absolute CRC symbols in modules regardless of kconfig selection
  split into two patches


This asymmetry threw me off a bit, especially the Kconfig naming (only
vmlinux crcs get the relative offsets, and only on powerpc atm, but all
modules keep the absolute syms, but it is called MODULE_REL_CRCS...),
if we keep this asymmetric crc treatment, it would be really nice to
note this discrepancy this somewhere, perhaps in the Kconfig, to keep
our heads from spinning :-)

I'm still catching up on the previous discussion threads, but can you
explain a bit more why you switched away from full blown relative crcs
from your last patchset [1]? I had lightly tested your v3 on ppc64le
previously, and relative offsets with modules seemed to worked very
well. I'm probably missing something very obvious.


Ah, I just saw your other comment about other arches not having support
for the rel32 offsets :-/

The asymmetry still bothers me though. Can't we have something that just
switches relative crcs on and off, that applies to *both* vmlinux and modules?
Then we can get rid of the crc_owner check in check_version() and just have
something like:

  if (IS_ENABLED(CONFIG_RELATIVE_CRCS))
  crcval = resolve_crc(crc);

Also we could get rid of the '&& !defined(MODULE)' checks scattered in
export.h. Then the arches that want relative crcs and that *do* have rel32
relocation support can turn relative crcs on, and powerpc can enable it, right?

Would that work or is there another reason this won't work with modules
(assuming that the arches that select this option support the relative
offsets)?

Jessica

Re: [PATCH 00/10] android: binder: support for domains and scatter-gather.

[John Stultz]

On Mon, Oct 24, 2016 at 6:20 AM, Martijn Coenen  wrote:
> Android userspace will start using binder IPC for communication with HAL
> modules. To clearly separate this IPC domain from the existing framework
> IPC domain, this patch series adds support for multiple "binder domains".
> This is implemented by each domain having its own binder context manager,
> and then having a separate device node per domain.
>
> The other change introduced by this series is scatter-gather; currently
> all objects passed through binder IPC must be serialized in userspace, with
> with the exception of binder objects and file descriptors. By adding scatter-
> gather support for memory buffers, we can avoid the serialization copy,
> thereby increasing performance for larger transaction sizes.
>
> The two patches from Arve are security patches that we've already applied
> in android-common. I included them in front of the series because my changes
> touch quite some of that code.

Hey Greg,
   Curious what the status of these patches are? Did you have any
outstanding objection, or were you just waiting for them to be resent?

AOSP is now making use of this feature, so mainline kernels have
trouble booting w/o it, so it would be good to get merged. :)

thanks
-john

Re: [PATCH 00/10] android: binder: support for domains and scatter-gather.

[John Stultz]

On Mon, Oct 24, 2016 at 6:20 AM, Martijn Coenen  wrote:
> Android userspace will start using binder IPC for communication with HAL
> modules. To clearly separate this IPC domain from the existing framework
> IPC domain, this patch series adds support for multiple "binder domains".
> This is implemented by each domain having its own binder context manager,
> and then having a separate device node per domain.
>
> The other change introduced by this series is scatter-gather; currently
> all objects passed through binder IPC must be serialized in userspace, with
> with the exception of binder objects and file descriptors. By adding scatter-
> gather support for memory buffers, we can avoid the serialization copy,
> thereby increasing performance for larger transaction sizes.
>
> The two patches from Arve are security patches that we've already applied
> in android-common. I included them in front of the series because my changes
> touch quite some of that code.

Hey Greg,
   Curious what the status of these patches are? Did you have any
outstanding objection, or were you just waiting for them to be resent?

AOSP is now making use of this feature, so mainline kernels have
trouble booting w/o it, so it would be good to get merged. :)

thanks
-john

linux-next: manual merge of the akpm tree with the block tree

[Stephen Rothwell]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in:

  fs/block_dev.c

between commit:

  b1d2dc5659b4 ("block: Make blk_get_backing_dev_info() safe without open bdev")

from the block tree and patch:

  "fs: add i_blocksize()"

from the akpm tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/block_dev.c
index 73031ec54a7b,921e868e84de..
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@@ -988,8 -971,7 +988,8 @@@ struct block_device *bdget(dev_t dev
bdev->bd_contains = NULL;
bdev->bd_super = NULL;
bdev->bd_inode = inode;
 +  bdev->bd_bdi = _backing_dev_info;
-   bdev->bd_block_size = (1 << inode->i_blkbits);
+   bdev->bd_block_size = i_blocksize(inode);
bdev->bd_part_count = 0;
bdev->bd_invalidated = 0;
inode->i_mode = S_IFBLK;

linux-next: manual merge of the akpm tree with the block tree

[Stephen Rothwell]

Hi Andrew,

Today's linux-next merge of the akpm tree got a conflict in:

  fs/block_dev.c

between commit:

  b1d2dc5659b4 ("block: Make blk_get_backing_dev_info() safe without open bdev")

from the block tree and patch:

  "fs: add i_blocksize()"

from the akpm tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc fs/block_dev.c
index 73031ec54a7b,921e868e84de..
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@@ -988,8 -971,7 +988,8 @@@ struct block_device *bdget(dev_t dev
bdev->bd_contains = NULL;
bdev->bd_super = NULL;
bdev->bd_inode = inode;
 +  bdev->bd_bdi = _backing_dev_info;
-   bdev->bd_block_size = (1 << inode->i_blkbits);
+   bdev->bd_block_size = i_blocksize(inode);
bdev->bd_part_count = 0;
bdev->bd_invalidated = 0;
inode->i_mode = S_IFBLK;

Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

[Inki Dae]

2017년 02월 02일 00:29에 Emil Velikov 이(가) 쓴 글:
> On 1 February 2017 at 14:52, Thierry Reding  wrote:
>> On Tue, Jan 31, 2017 at 02:54:53PM -0800, Eric Anholt wrote:
>>> Thierry Reding  writes:
>>>
 [ Unknown signature status ]
 On Tue, Jan 31, 2017 at 10:15:10AM -0800, Eric Anholt wrote:
> Thierry Reding  writes:
>
>> [ Unknown signature status ]
>> On Tue, Jan 31, 2017 at 09:38:53AM -0500, Sean Paul wrote:
>>> On Tue, Jan 31, 2017 at 09:54:49AM +0100, Thierry Reding wrote:
 On Tue, Jan 31, 2017 at 09:01:07AM +0900, Inki Dae wrote:
>
>
> 2017년 01월 24일 10:50에 Hoegeun Kwon 이(가) 쓴 글:
>> Dear Thierry,
>>
>> Could you please review this patch?
>
> Thierry, I think this patch has been reviewed enough but no comment
> from you. Seems you are busy. I will pick up this.

 Sorry, but that's not how it works. This patch has gone through 8
 revisions within 4 weeks, and I tend to ignore patches like that until
 the dust settles.

>>>
>>> Seems like the dust was pretty settled. It was posted on 1/11, pinged 
>>> on 1/24,
>>> and picked up on 1/31. I don't think it's unreasonable to take it 
>>> through
>>> another tree after that.
>>>
>>> I wonder if drm_panel would benefit from the -misc group maintainership 
>>> model
>>> as drm_bridge does. By spreading out the workload, the high-maintenance
>>> patches would hopefully find someone to shepherd them through.
>>
>> Except that nobody except me really cares. If we let people take patches
>> through separate trees or group-maintained trees they'll likely go in
>> without too much thought. DRM panel is somewhat different from core DRM
>> in this regard because its infrastructure is minimal and there's little
>> outside the panel-simple driver. So we're still at a stage where we need
>> to fine-tune what drivers should look like and how we can improve.
>
> I would love to care and participate in review, but with the structure
> of your tree you're the only one whose review counts, so I don't
> participate.

 Really? What exactly do you think is special about the structure of my
 tree? I require patches to be on dri-devel (I pick them up from the
 patchwork instance at freedesktop.org), the tree is publicly available
 and reviewed-by tags get picked up automatically by patchwork.

 The panel tree works exactly like any other maintainer tree. And my
 review is *not* the only one that counts. I appreciate every Reviewed-by
 tag I see on panel patches because it means that I don't have to look as
 closely as I have to otherwise.

 It is true that I am responsible for those patches, that's why I get to
 have the final word on whether or not a patch gets applied. And that's
 no different from any other maintainer tree either.
>>>
>>> If me reviewing a patch isn't part of unblocking that patch getting in,
>>> then I won't bother because all I could end up doing is punishing the
>>> developer of the patch.  Contributors have a hard enough time already.
>>
>> Maybe you should go and read my previous reply again more carefully.
>> Perhaps then you'll realize that reviews are in fact helping in getting
>> patches merged.
>>
>> Interestingly my inbox doesn't show you ever bothering to review panel
>> patches, so maybe you should be more careful about your assumptions.
>>
> Gents, it's understandable that emotions might be running high.
> 
> What's the point in pointing fingers at each other - there is enough
> to go in each direction.
> Let us all step back for a second and consider how we can make things better.
> 
> I think it'll be nice to have some/most of the common concerns that
> Thierry/others comes across documented - in-kernel, blog post, other.
> Such that one can reference to specific points as patch falls sub-par.
> We all want to have a balance of nicely written driver and quick
> merge.
> 
> Inki, I believe myself and others have invited you before on
> #dri-devel. This is another medium where you can poke devs and from my
> experience - it tends to be more efficient, most of the time.

It's true and totally agree. I can really understand Thierry but I think we 
need to think about maintainer's role for our community.
And also I think the big and small collisions between maintainers and 
contributors are just the process of getting better.

Thanks,
Inki Dae

> 
> Thanks
> Emil
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v8 2/3] drm/panel: Add support for S6E3HA2 panel driver on TM2 board

[Inki Dae]

2017년 02월 02일 00:29에 Emil Velikov 이(가) 쓴 글:
> On 1 February 2017 at 14:52, Thierry Reding  wrote:
>> On Tue, Jan 31, 2017 at 02:54:53PM -0800, Eric Anholt wrote:
>>> Thierry Reding  writes:
>>>
 [ Unknown signature status ]
 On Tue, Jan 31, 2017 at 10:15:10AM -0800, Eric Anholt wrote:
> Thierry Reding  writes:
>
>> [ Unknown signature status ]
>> On Tue, Jan 31, 2017 at 09:38:53AM -0500, Sean Paul wrote:
>>> On Tue, Jan 31, 2017 at 09:54:49AM +0100, Thierry Reding wrote:
 On Tue, Jan 31, 2017 at 09:01:07AM +0900, Inki Dae wrote:
>
>
> 2017년 01월 24일 10:50에 Hoegeun Kwon 이(가) 쓴 글:
>> Dear Thierry,
>>
>> Could you please review this patch?
>
> Thierry, I think this patch has been reviewed enough but no comment
> from you. Seems you are busy. I will pick up this.

 Sorry, but that's not how it works. This patch has gone through 8
 revisions within 4 weeks, and I tend to ignore patches like that until
 the dust settles.

>>>
>>> Seems like the dust was pretty settled. It was posted on 1/11, pinged 
>>> on 1/24,
>>> and picked up on 1/31. I don't think it's unreasonable to take it 
>>> through
>>> another tree after that.
>>>
>>> I wonder if drm_panel would benefit from the -misc group maintainership 
>>> model
>>> as drm_bridge does. By spreading out the workload, the high-maintenance
>>> patches would hopefully find someone to shepherd them through.
>>
>> Except that nobody except me really cares. If we let people take patches
>> through separate trees or group-maintained trees they'll likely go in
>> without too much thought. DRM panel is somewhat different from core DRM
>> in this regard because its infrastructure is minimal and there's little
>> outside the panel-simple driver. So we're still at a stage where we need
>> to fine-tune what drivers should look like and how we can improve.
>
> I would love to care and participate in review, but with the structure
> of your tree you're the only one whose review counts, so I don't
> participate.

 Really? What exactly do you think is special about the structure of my
 tree? I require patches to be on dri-devel (I pick them up from the
 patchwork instance at freedesktop.org), the tree is publicly available
 and reviewed-by tags get picked up automatically by patchwork.

 The panel tree works exactly like any other maintainer tree. And my
 review is *not* the only one that counts. I appreciate every Reviewed-by
 tag I see on panel patches because it means that I don't have to look as
 closely as I have to otherwise.

 It is true that I am responsible for those patches, that's why I get to
 have the final word on whether or not a patch gets applied. And that's
 no different from any other maintainer tree either.
>>>
>>> If me reviewing a patch isn't part of unblocking that patch getting in,
>>> then I won't bother because all I could end up doing is punishing the
>>> developer of the patch.  Contributors have a hard enough time already.
>>
>> Maybe you should go and read my previous reply again more carefully.
>> Perhaps then you'll realize that reviews are in fact helping in getting
>> patches merged.
>>
>> Interestingly my inbox doesn't show you ever bothering to review panel
>> patches, so maybe you should be more careful about your assumptions.
>>
> Gents, it's understandable that emotions might be running high.
> 
> What's the point in pointing fingers at each other - there is enough
> to go in each direction.
> Let us all step back for a second and consider how we can make things better.
> 
> I think it'll be nice to have some/most of the common concerns that
> Thierry/others comes across documented - in-kernel, blog post, other.
> Such that one can reference to specific points as patch falls sub-par.
> We all want to have a balance of nicely written driver and quick
> merge.
> 
> Inki, I believe myself and others have invited you before on
> #dri-devel. This is another medium where you can poke devs and from my
> experience - it tends to be more efficient, most of the time.

It's true and totally agree. I can really understand Thierry but I think we 
need to think about maintainer's role for our community.
And also I think the big and small collisions between maintainers and 
contributors are just the process of getting better.

Thanks,
Inki Dae

> 
> Thanks
> Emil
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [tip:sched/core] sched/core: Add debugging code to catch missing update_rq_clock() calls

[Sachin Sant]

> On 02-Feb-2017, at 9:25 PM, Peter Zijlstra <pet...@infradead.org> wrote:
> 
> On Tue, Jan 31, 2017 at 10:22:47AM -0700, Ross Zwisler wrote:
>> On Tue, Jan 31, 2017 at 4:48 AM, Mike Galbraith <efa...@gmx.de> wrote:
>>> On Tue, 2017-01-31 at 16:30 +0530, Sachin Sant wrote:
> 
> 
> Could some of you test this? It seems to cure things in my (very)
> limited testing.
> 

I ran few cycles of cpu hot(un)plug tests. In most cases it works except one
where I ran into rcu stall:

[  173.493453] INFO: rcu_sched detected stalls on CPUs/tasks:
[  173.493473]  8-...: (2 GPs behind) idle=006/140/0 softirq=0/0 
fqs=2996 
[  173.493476]  (detected by 0, t=6002 jiffies, g=885, c=884, q=6350)
[  173.493482] Task dump for CPU 8:
[  173.493484] cpuhp/8 R  running task0  3416  2 0x0884
[  173.493489] Call Trace:
[  173.493492] [c004f7b834a0] [c004f7b83560] 0xc004f7b83560 
(unreliable)
[  173.493498] [c004f7b83670] [c0008d28] 
alignment_common+0x128/0x130
[  173.493503] --- interrupt: 600 at _raw_spin_lock+0x2c/0xc0
[  173.493503] LR = try_to_wake_up+0x204/0x5c0
[  173.493507] [c004f7b83960] [c004f4d8084c] 0xc004f4d8084c 
(unreliable)
[  173.493511] [c004f7b83990] [c00fef54] try_to_wake_up+0x204/0x5c0
[  173.493515] [c004f7b83a10] [c00e2b88] create_worker+0x148/0x250
[  173.493519] [c004f7b83ab0] [c00e6e1c] 
alloc_unbound_pwq+0x3bc/0x4c0
[  173.493522] [c004f7b83b10] [c00e7084] 
wq_update_unbound_numa+0x164/0x270
[  173.493526] [c004f7b83bb0] [c00e8990] 
workqueue_online_cpu+0x250/0x3b0
[  173.493529] [c004f7b83c70] [c00c2758] 
cpuhp_invoke_callback+0x148/0x5b0
[  173.493533] [c004f7b83ce0] [c00c2df8] 
cpuhp_up_callbacks+0x48/0x140
[  173.493536] [c004f7b83d30] [c00c3e98] 
cpuhp_thread_fun+0x148/0x180
[  173.493540] [c004f7b83d60] [c00f3930] 
smpboot_thread_fn+0x290/0x2a0
[  173.493544] [c004f7b83dc0] [c00edb3c] kthread+0x14c/0x190
[  173.493547] [c004f7b83e30] [c000b4e8] 
ret_from_kernel_thread+0x5c/0x74
[  243.913715] INFO: task kworker/0:2:380 blocked for more than 120 seconds.
[  243.913732]   Not tainted 4.10.0-rc6-next-20170202 #6
[  243.913735] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  243.913738] kworker/0:2 D0   380  2 0x0800
[  243.913746] Workqueue: events vmstat_shepherd
[  243.913748] Call Trace:
[  243.913752] [c000ff07f820] [c011135c] 
enqueue_entity+0x81c/0x1200 (unreliable)
[  243.913757] [c000ff07f9f0] [c001a660] __switch_to+0x300/0x400
[  243.913762] [c000ff07fa50] [c08df4f4] __schedule+0x314/0xb10
[  243.913766] [c000ff07fb20] [c08dfd30] schedule+0x40/0xb0
[  243.913769] [c000ff07fb50] [c08e02b8] 
schedule_preempt_disabled+0x18/0x30
[  243.913773] [c000ff07fb70] [c08e1654] 
__mutex_lock.isra.6+0x1a4/0x660
[  243.913777] [c000ff07fc00] [c00c3828] get_online_cpus+0x48/0x90
[  243.913780] [c000ff07fc30] [c025fd78] vmstat_shepherd+0x38/0x150
[  243.913784] [c000ff07fc80] [c00e5794] 
process_one_work+0x1a4/0x4d0
[  243.913788] [c000ff07fd20] [c00e5b58] worker_thread+0x98/0x5a0
[  243.913791] [c000ff07fdc0] [c00edb3c] kthread+0x14c/0x190
[  243.913795] [c000ff07fe30] [c000b4e8] 
ret_from_kernel_thread+0x5c/0x74
[  243.913824] INFO: task drmgr:3413 blocked for more than 120 seconds.
[  243.913826]   Not tainted 4.10.0-rc6-next-20170202 #6
[  243.913829] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  243.913831] drmgr   D0  3413   3114 0x00040080
[  243.913834] Call Trace:
[  243.913836] [c00257ff3380] [c00257ff3440] 0xc00257ff3440 
(unreliable)
[  243.913840] [c00257ff3550] [c001a660] __switch_to+0x300/0x400
[  243.913844] [c00257ff35b0] [c08df4f4] __schedule+0x314/0xb10
[  243.913847] [c00257ff3680] [c08dfd30] schedule+0x40/0xb0
[  243.913851] [c00257ff36b0] [c08e4594] 
schedule_timeout+0x274/0x470
[  243.913855] [c00257ff37b0] [c08e0efc] wait_for_common+0x1ac/0x2c0
[  243.913858] [c00257ff3830] [c00c50e4] bringup_cpu+0x84/0xe0
[  243.913862] [c00257ff3860] [c00c2758] 
cpuhp_invoke_callback+0x148/0x5b0
[  243.913865] [c00257ff38d0] [c00c2df8] 
cpuhp_up_callbacks+0x48/0x140
[  243.913868] [c00257ff3920] [c00c5438] _cpu_up+0xe8/0x1c0
[  243.913872] [c00257ff3980] [c00c5630] do_cpu_up+0x120/0x150
[  243.913876] [c00257ff3a00] [c05c005c] cpu_subsys_online+0x5c/0xe0
[  243.913879] [c00257ff3a50] [c05b7d84] device_online+0xb4/0x120
[  243.913883] [c00257ff3a90] [c0093424] 
dlpar_online_cpu+0x144/0x1e0
[  243.913887] 

Re: [tip:sched/core] sched/core: Add debugging code to catch missing update_rq_clock() calls

[Sachin Sant]

> On 02-Feb-2017, at 9:25 PM, Peter Zijlstra  wrote:
> 
> On Tue, Jan 31, 2017 at 10:22:47AM -0700, Ross Zwisler wrote:
>> On Tue, Jan 31, 2017 at 4:48 AM, Mike Galbraith  wrote:
>>> On Tue, 2017-01-31 at 16:30 +0530, Sachin Sant wrote:
> 
> 
> Could some of you test this? It seems to cure things in my (very)
> limited testing.
> 

I ran few cycles of cpu hot(un)plug tests. In most cases it works except one
where I ran into rcu stall:

[  173.493453] INFO: rcu_sched detected stalls on CPUs/tasks:
[  173.493473]  8-...: (2 GPs behind) idle=006/140/0 softirq=0/0 
fqs=2996 
[  173.493476]  (detected by 0, t=6002 jiffies, g=885, c=884, q=6350)
[  173.493482] Task dump for CPU 8:
[  173.493484] cpuhp/8 R  running task0  3416  2 0x0884
[  173.493489] Call Trace:
[  173.493492] [c004f7b834a0] [c004f7b83560] 0xc004f7b83560 
(unreliable)
[  173.493498] [c004f7b83670] [c0008d28] 
alignment_common+0x128/0x130
[  173.493503] --- interrupt: 600 at _raw_spin_lock+0x2c/0xc0
[  173.493503] LR = try_to_wake_up+0x204/0x5c0
[  173.493507] [c004f7b83960] [c004f4d8084c] 0xc004f4d8084c 
(unreliable)
[  173.493511] [c004f7b83990] [c00fef54] try_to_wake_up+0x204/0x5c0
[  173.493515] [c004f7b83a10] [c00e2b88] create_worker+0x148/0x250
[  173.493519] [c004f7b83ab0] [c00e6e1c] 
alloc_unbound_pwq+0x3bc/0x4c0
[  173.493522] [c004f7b83b10] [c00e7084] 
wq_update_unbound_numa+0x164/0x270
[  173.493526] [c004f7b83bb0] [c00e8990] 
workqueue_online_cpu+0x250/0x3b0
[  173.493529] [c004f7b83c70] [c00c2758] 
cpuhp_invoke_callback+0x148/0x5b0
[  173.493533] [c004f7b83ce0] [c00c2df8] 
cpuhp_up_callbacks+0x48/0x140
[  173.493536] [c004f7b83d30] [c00c3e98] 
cpuhp_thread_fun+0x148/0x180
[  173.493540] [c004f7b83d60] [c00f3930] 
smpboot_thread_fn+0x290/0x2a0
[  173.493544] [c004f7b83dc0] [c00edb3c] kthread+0x14c/0x190
[  173.493547] [c004f7b83e30] [c000b4e8] 
ret_from_kernel_thread+0x5c/0x74
[  243.913715] INFO: task kworker/0:2:380 blocked for more than 120 seconds.
[  243.913732]   Not tainted 4.10.0-rc6-next-20170202 #6
[  243.913735] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  243.913738] kworker/0:2 D0   380  2 0x0800
[  243.913746] Workqueue: events vmstat_shepherd
[  243.913748] Call Trace:
[  243.913752] [c000ff07f820] [c011135c] 
enqueue_entity+0x81c/0x1200 (unreliable)
[  243.913757] [c000ff07f9f0] [c001a660] __switch_to+0x300/0x400
[  243.913762] [c000ff07fa50] [c08df4f4] __schedule+0x314/0xb10
[  243.913766] [c000ff07fb20] [c08dfd30] schedule+0x40/0xb0
[  243.913769] [c000ff07fb50] [c08e02b8] 
schedule_preempt_disabled+0x18/0x30
[  243.913773] [c000ff07fb70] [c08e1654] 
__mutex_lock.isra.6+0x1a4/0x660
[  243.913777] [c000ff07fc00] [c00c3828] get_online_cpus+0x48/0x90
[  243.913780] [c000ff07fc30] [c025fd78] vmstat_shepherd+0x38/0x150
[  243.913784] [c000ff07fc80] [c00e5794] 
process_one_work+0x1a4/0x4d0
[  243.913788] [c000ff07fd20] [c00e5b58] worker_thread+0x98/0x5a0
[  243.913791] [c000ff07fdc0] [c00edb3c] kthread+0x14c/0x190
[  243.913795] [c000ff07fe30] [c000b4e8] 
ret_from_kernel_thread+0x5c/0x74
[  243.913824] INFO: task drmgr:3413 blocked for more than 120 seconds.
[  243.913826]   Not tainted 4.10.0-rc6-next-20170202 #6
[  243.913829] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  243.913831] drmgr   D0  3413   3114 0x00040080
[  243.913834] Call Trace:
[  243.913836] [c00257ff3380] [c00257ff3440] 0xc00257ff3440 
(unreliable)
[  243.913840] [c00257ff3550] [c001a660] __switch_to+0x300/0x400
[  243.913844] [c00257ff35b0] [c08df4f4] __schedule+0x314/0xb10
[  243.913847] [c00257ff3680] [c08dfd30] schedule+0x40/0xb0
[  243.913851] [c00257ff36b0] [c08e4594] 
schedule_timeout+0x274/0x470
[  243.913855] [c00257ff37b0] [c08e0efc] wait_for_common+0x1ac/0x2c0
[  243.913858] [c00257ff3830] [c00c50e4] bringup_cpu+0x84/0xe0
[  243.913862] [c00257ff3860] [c00c2758] 
cpuhp_invoke_callback+0x148/0x5b0
[  243.913865] [c00257ff38d0] [c00c2df8] 
cpuhp_up_callbacks+0x48/0x140
[  243.913868] [c00257ff3920] [c00c5438] _cpu_up+0xe8/0x1c0
[  243.913872] [c00257ff3980] [c00c5630] do_cpu_up+0x120/0x150
[  243.913876] [c00257ff3a00] [c05c005c] cpu_subsys_online+0x5c/0xe0
[  243.913879] [c00257ff3a50] [c05b7d84] device_online+0xb4/0x120
[  243.913883] [c00257ff3a90] [c0093424] 
dlpar_online_cpu+0x144/0x1e0
[  243.913887] [c00257ff3b50] [c0093c08] dlpar_cpu_ad

Re: [PATCH 3/3] MIPS: BMIPS: enable CPUfreq

[Viresh Kumar]

On 01-02-17, 17:06, Markus Mayer wrote:
> From: Markus Mayer 
> 
> Enable all applicable CPUfreq options.
> 
> Signed-off-by: Markus Mayer 
> ---
>  arch/mips/configs/bmips_stb_defconfig | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/mips/configs/bmips_stb_defconfig 
> b/arch/mips/configs/bmips_stb_defconfig
> index 4eb5d6e..6fda604 100644
> --- a/arch/mips/configs/bmips_stb_defconfig
> +++ b/arch/mips/configs/bmips_stb_defconfig
> @@ -26,6 +26,16 @@ CONFIG_INET=y
>  # CONFIG_INET_XFRM_MODE_BEET is not set
>  # CONFIG_INET_LRO is not set
>  # CONFIG_INET_DIAG is not set
> +CONFIG_CPU_FREQ=y
> +CONFIG_CPU_FREQ_STAT=y
> +CONFIG_CPU_FREQ_STAT_DETAILS=y
> +CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE=y
> +CONFIG_CPU_FREQ_GOV_PERFORMANCE=y
> +CONFIG_CPU_FREQ_GOV_ONDEMAND=y
> +CONFIG_CPU_FREQ_GOV_POWERSAVE=y
> +CONFIG_CPU_FREQ_GOV_USERSPACE=y
> +CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y
> +CONFIG_BMIPS_CPUFREQ=y
>  CONFIG_CFG80211=y
>  CONFIG_NL80211_TESTMODE=y
>  CONFIG_MAC80211=y

Rebase your stuff over pm/linux-next and you will see some changes here. Also
schedutil is the new governor in town, you must give it a try.

-- 
viresh

Re: [PATCH 3/3] MIPS: BMIPS: enable CPUfreq

[Viresh Kumar]

On 01-02-17, 17:06, Markus Mayer wrote:
> From: Markus Mayer 
> 
> Enable all applicable CPUfreq options.
> 
> Signed-off-by: Markus Mayer 
> ---
>  arch/mips/configs/bmips_stb_defconfig | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/mips/configs/bmips_stb_defconfig 
> b/arch/mips/configs/bmips_stb_defconfig
> index 4eb5d6e..6fda604 100644
> --- a/arch/mips/configs/bmips_stb_defconfig
> +++ b/arch/mips/configs/bmips_stb_defconfig
> @@ -26,6 +26,16 @@ CONFIG_INET=y
>  # CONFIG_INET_XFRM_MODE_BEET is not set
>  # CONFIG_INET_LRO is not set
>  # CONFIG_INET_DIAG is not set
> +CONFIG_CPU_FREQ=y
> +CONFIG_CPU_FREQ_STAT=y
> +CONFIG_CPU_FREQ_STAT_DETAILS=y
> +CONFIG_CPU_FREQ_DEFAULT_GOV_PERFORMANCE=y
> +CONFIG_CPU_FREQ_GOV_PERFORMANCE=y
> +CONFIG_CPU_FREQ_GOV_ONDEMAND=y
> +CONFIG_CPU_FREQ_GOV_POWERSAVE=y
> +CONFIG_CPU_FREQ_GOV_USERSPACE=y
> +CONFIG_CPU_FREQ_GOV_CONSERVATIVE=y
> +CONFIG_BMIPS_CPUFREQ=y
>  CONFIG_CFG80211=y
>  CONFIG_NL80211_TESTMODE=y
>  CONFIG_MAC80211=y

Rebase your stuff over pm/linux-next and you will see some changes here. Also
schedutil is the new governor in town, you must give it a try.

-- 
viresh

Re: [PATCH 2/3] cpufreq: bmips-cpufreq: CPUfreq driver for Broadcom's BMIPS SoCs

[Viresh Kumar]

You must be a cpufreq driver expert by now. What's the count? Is this the 3rd
one you have written ? :)

On 01-02-17, 17:06, Markus Mayer wrote:
> diff --git a/drivers/cpufreq/bmips-cpufreq.c b/drivers/cpufreq/bmips-cpufreq.c
> +static struct cpufreq_frequency_table *
> +bmips_cpufreq_get_freq_table(const struct cpufreq_policy *policy)

Maybe call it bmips_cpufreq_create_freq_table() as that's what you are doing.
But its all up to you only.

> +{
> + struct cpufreq_frequency_table *table;
> + struct cpufreq_compat *cc;
> + unsigned long cpu_freq;
> + int i;
> +
> + cc = policy->driver_data;
> + cpu_freq = htp_freq_to_cpu_freq(cc->clk_mult);
> +
> + table = kzalloc((cc->max_freqs + 1) * sizeof(*table), GFP_KERNEL);

Maybe kmalloc as you are updating all the entries.

> + if (!table)
> + return ERR_PTR(-ENOMEM);
> +
> + for (i = 0; i < cc->max_freqs; i++) {
> + table[i].frequency = cpu_freq / (1 << i);
> + table[i].driver_data = i;
> + }
> + table[i].frequency = CPUFREQ_TABLE_END;
> +
> + return table;
> +}
> +
> +static unsigned int bmips_cpufreq_get(unsigned int cpu)
> +{
> + struct cpufreq_policy *policy;
> + struct cpufreq_compat *cc;
> + unsigned long freq, cpu_freq;
> + unsigned int div;
> + uint32_t mode;
> +
> + policy = cpufreq_cpu_get(cpu);

You need to do a corresponding cpufreq_cpu_put().

> + cc = policy->driver_data;
> +
> + switch (cc->bmips_type) {
> + case BMIPS5200:
> + case BMIPS5000:
> + mode = read_c0_brcm_mode();
> + div = ((mode >> BMIPS5_CLK_DIV_SHIFT) & BMIPS5_CLK_DIV_MASK);
> + break;
> + default:
> + div = 0;
> + }
> +
> + cpu_freq = htp_freq_to_cpu_freq(cc->clk_mult);
> + freq = cpu_freq / (1 << div);
> +
> + return freq;
> +}
> +
> +static int bmips_cpufreq_target_index(struct cpufreq_policy *policy,
> +   unsigned int index)
> +{
> + struct cpufreq_compat *cc;
> + unsigned int div;
> +
> + cc = policy->driver_data;
> + div = policy->freq_table[index].driver_data;
> +
> + switch (cc->bmips_type) {
> + case BMIPS5200:
> + case BMIPS5000:
> + change_c0_brcm_mode(BMIPS5_CLK_DIV_MASK << BMIPS5_CLK_DIV_SHIFT,
> + (1 << BMIPS5_CLK_DIV_SET_SHIFT) |
> + (div << BMIPS5_CLK_DIV_SHIFT));
> + break;
> + default:
> + return -ENOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int bmips_cpufreq_exit(struct cpufreq_policy *policy)
> +{
> + kfree(policy->freq_table);
> + policy->freq_table = NULL;

No need to set it to NULL.

> +
> + return 0;
> +}
> +
> +static int bmips_cpufreq_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_frequency_table *freq_table;
> + int ret;
> +
> + /* Store the compatibility data with the policy. */
> + policy->driver_data = cpufreq_get_driver_data();

Hmm, I wouldn't mind keeping a global variable for this. This driver will be
probed only once and so we can simplify the code a bit. Up to you.

> +
> + freq_table = bmips_cpufreq_get_freq_table(policy);
> + if (IS_ERR(freq_table)) {
> + ret = PTR_ERR(freq_table);
> + pr_err("%s: couldn't determine frequency table (%d).\n",
> + BMIPS_CPUFREQ_NAME, ret);
> + return ret;
> + }
> +
> + ret = cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
> + if (ret)
> + bmips_cpufreq_exit(policy);
> + else
> + pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
> +
> + return ret;
> +}
> +
> +static struct cpufreq_driver bmips_cpufreq_driver = {
> + .flags  = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target_index   = bmips_cpufreq_target_index,
> + .get= bmips_cpufreq_get,
> + .init   = bmips_cpufreq_init,
> + .exit   = bmips_cpufreq_exit,
> + .attr   = cpufreq_generic_attr,
> + .name   = BMIPS_CPUFREQ_PREFIX,
> +};
> +
> +static int __init bmips_cpufreq_probe(void)
> +{
> + struct cpufreq_compat *cc;
> + struct device_node *np;
> +
> + for (cc = bmips_cpufreq_compat; cc->compatible; cc++) {
> + np = of_find_compatible_node(NULL, "cpu", cc->compatible);
> + if (np) {
> + of_node_put(np);
> + bmips_cpufreq_driver.driver_data = cc;
> + break;
> + }
> + }
> +
> + /* We hit the guard element of the array. No compatible CPU found. */
> + if (!cc->compatible)
> + return -ENODEV;
> +
> + return cpufreq_register_driver(_cpufreq_driver);
> +}
> +device_initcall(bmips_cpufreq_probe);
> +
> +MODULE_AUTHOR("Markus Mayer ");
> 

Re: [PATCH 2/3] cpufreq: bmips-cpufreq: CPUfreq driver for Broadcom's BMIPS SoCs

[Viresh Kumar]

You must be a cpufreq driver expert by now. What's the count? Is this the 3rd
one you have written ? :)

On 01-02-17, 17:06, Markus Mayer wrote:
> diff --git a/drivers/cpufreq/bmips-cpufreq.c b/drivers/cpufreq/bmips-cpufreq.c
> +static struct cpufreq_frequency_table *
> +bmips_cpufreq_get_freq_table(const struct cpufreq_policy *policy)

Maybe call it bmips_cpufreq_create_freq_table() as that's what you are doing.
But its all up to you only.

> +{
> + struct cpufreq_frequency_table *table;
> + struct cpufreq_compat *cc;
> + unsigned long cpu_freq;
> + int i;
> +
> + cc = policy->driver_data;
> + cpu_freq = htp_freq_to_cpu_freq(cc->clk_mult);
> +
> + table = kzalloc((cc->max_freqs + 1) * sizeof(*table), GFP_KERNEL);

Maybe kmalloc as you are updating all the entries.

> + if (!table)
> + return ERR_PTR(-ENOMEM);
> +
> + for (i = 0; i < cc->max_freqs; i++) {
> + table[i].frequency = cpu_freq / (1 << i);
> + table[i].driver_data = i;
> + }
> + table[i].frequency = CPUFREQ_TABLE_END;
> +
> + return table;
> +}
> +
> +static unsigned int bmips_cpufreq_get(unsigned int cpu)
> +{
> + struct cpufreq_policy *policy;
> + struct cpufreq_compat *cc;
> + unsigned long freq, cpu_freq;
> + unsigned int div;
> + uint32_t mode;
> +
> + policy = cpufreq_cpu_get(cpu);

You need to do a corresponding cpufreq_cpu_put().

> + cc = policy->driver_data;
> +
> + switch (cc->bmips_type) {
> + case BMIPS5200:
> + case BMIPS5000:
> + mode = read_c0_brcm_mode();
> + div = ((mode >> BMIPS5_CLK_DIV_SHIFT) & BMIPS5_CLK_DIV_MASK);
> + break;
> + default:
> + div = 0;
> + }
> +
> + cpu_freq = htp_freq_to_cpu_freq(cc->clk_mult);
> + freq = cpu_freq / (1 << div);
> +
> + return freq;
> +}
> +
> +static int bmips_cpufreq_target_index(struct cpufreq_policy *policy,
> +   unsigned int index)
> +{
> + struct cpufreq_compat *cc;
> + unsigned int div;
> +
> + cc = policy->driver_data;
> + div = policy->freq_table[index].driver_data;
> +
> + switch (cc->bmips_type) {
> + case BMIPS5200:
> + case BMIPS5000:
> + change_c0_brcm_mode(BMIPS5_CLK_DIV_MASK << BMIPS5_CLK_DIV_SHIFT,
> + (1 << BMIPS5_CLK_DIV_SET_SHIFT) |
> + (div << BMIPS5_CLK_DIV_SHIFT));
> + break;
> + default:
> + return -ENOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> +static int bmips_cpufreq_exit(struct cpufreq_policy *policy)
> +{
> + kfree(policy->freq_table);
> + policy->freq_table = NULL;

No need to set it to NULL.

> +
> + return 0;
> +}
> +
> +static int bmips_cpufreq_init(struct cpufreq_policy *policy)
> +{
> + struct cpufreq_frequency_table *freq_table;
> + int ret;
> +
> + /* Store the compatibility data with the policy. */
> + policy->driver_data = cpufreq_get_driver_data();

Hmm, I wouldn't mind keeping a global variable for this. This driver will be
probed only once and so we can simplify the code a bit. Up to you.

> +
> + freq_table = bmips_cpufreq_get_freq_table(policy);
> + if (IS_ERR(freq_table)) {
> + ret = PTR_ERR(freq_table);
> + pr_err("%s: couldn't determine frequency table (%d).\n",
> + BMIPS_CPUFREQ_NAME, ret);
> + return ret;
> + }
> +
> + ret = cpufreq_generic_init(policy, freq_table, TRANSITION_LATENCY);
> + if (ret)
> + bmips_cpufreq_exit(policy);
> + else
> + pr_info("%s: registered\n", BMIPS_CPUFREQ_NAME);
> +
> + return ret;
> +}
> +
> +static struct cpufreq_driver bmips_cpufreq_driver = {
> + .flags  = CPUFREQ_NEED_INITIAL_FREQ_CHECK,
> + .verify = cpufreq_generic_frequency_table_verify,
> + .target_index   = bmips_cpufreq_target_index,
> + .get= bmips_cpufreq_get,
> + .init   = bmips_cpufreq_init,
> + .exit   = bmips_cpufreq_exit,
> + .attr   = cpufreq_generic_attr,
> + .name   = BMIPS_CPUFREQ_PREFIX,
> +};
> +
> +static int __init bmips_cpufreq_probe(void)
> +{
> + struct cpufreq_compat *cc;
> + struct device_node *np;
> +
> + for (cc = bmips_cpufreq_compat; cc->compatible; cc++) {
> + np = of_find_compatible_node(NULL, "cpu", cc->compatible);
> + if (np) {
> + of_node_put(np);
> + bmips_cpufreq_driver.driver_data = cc;
> + break;
> + }
> + }
> +
> + /* We hit the guard element of the array. No compatible CPU found. */
> + if (!cc->compatible)
> + return -ENODEV;
> +
> + return cpufreq_register_driver(_cpufreq_driver);
> +}
> +device_initcall(bmips_cpufreq_probe);
> +
> +MODULE_AUTHOR("Markus Mayer ");
> +MODULE_DESCRIPTION("CPUfreq driver 

Re: [patch 1/3] cpufreq: implement min/max/up/down functions

[Viresh Kumar]

On 02-02-17, 15:47, Marcelo Tosatti wrote:
> +++ kvm-pvfreq/drivers/cpufreq/cpufreq_userspace.c2017-02-02 
> 15:32:53.456262640 -0200
> @@ -118,6 +118,178 @@
>   mutex_unlock(_mutex);
>  }
>  
> +static int cpufreq_is_userspace_governor(int cpu)
> +{
> + int ret;
> +
> + mutex_lock(_mutex);
> + ret = per_cpu(cpu_is_managed, cpu);

The userspace governor is buggy in the sense that cpu_is_managed is only updated
for the policy->cpu and not any other CPU in that policy. But then it was never
used with anything other than policy->cpu, so it was fine.

But now that you are allowing any CPU number here, you need to do one of these:
- Either set cpu_is_managed for all the CPUs from a policy
- Or get the policy first and pass policy->cpu here.

> + mutex_unlock(_mutex);
> +
> + return ret;
> +}

All 4 routines defined below have too much in common and it would be very easy
to write a common routine cpufreq_userspace_freq_change(), which can be called
in all the four cases. You can pass a function pointer to that, which can give
min, max, up, or down frequencies. That will make it more robust and less error
prone.

> +int cpufreq_userspace_freq_up(int cpu)
> +{
> + unsigned int curfreq, nextminfreq;
> + unsigned int ret = 0;
> + struct cpufreq_frequency_table *pos, *table;
> + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> +
> + if (!policy)
> + return -EINVAL;
> +
> + if (!cpufreq_is_userspace_governor(cpu)) {
> + cpufreq_cpu_put(policy);
> + return -EINVAL;
> + }

Because the userspace_mutex is dropped after that routine returned, there is no
guarantee that 'cpu' is still managed by this governor. And so you need to make
sure that you drops the locks only at the end.

> +
> + cpufreq_cpu_put(policy);

This must be called only after you are done using the policy, to make sure that
the policy doesn't get freed while you are using it.

> + mutex_lock(_mutex);
> + table = policy->freq_table;
> + if (!table) {
> + mutex_unlock(_mutex);
> + return -ENODEV;
> + }
> + nextminfreq = cpufreq_quick_get_max(cpu);

Just use policy->max here, why waste time ?

> + curfreq = policy->cur;
> +
> + cpufreq_for_each_valid_entry(pos, table) {
> + if (pos->frequency > curfreq &&
> + pos->frequency < nextminfreq)
> + nextminfreq = pos->frequency;
> + }

The above part can be a routine of its own, whose pointer will be passed to
cpufreq_userspace_freq_change().

> +
> + if (nextminfreq != curfreq) {

You are missing similar checks in the last two routines, any special reason for
that ?

> + unsigned int *setspeed = policy->governor_data;
> +
> + *setspeed = nextminfreq;
> + ret = __cpufreq_driver_target(policy, nextminfreq,
> +   CPUFREQ_RELATION_L);
> + } else
> + ret = 1;

Why ret 1? What are the callers expected to do on seeing this value? Maybe
return 0 as the desired freq is set by the governor ?

And always use {} for even single line code if the 'if' block has them.

> + mutex_unlock(_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(cpufreq_userspace_freq_up);
> +++ kvm-pvfreq/include/linux/cpufreq.h2017-01-31 14:20:00.508613672 
> -0200
> @@ -890,4 +890,11 @@
>  int cpufreq_generic_init(struct cpufreq_policy *policy,
>   struct cpufreq_frequency_table *table,
>   unsigned int transition_latency);
> +#ifdef CONFIG_CPU_FREQ
> +int cpufreq_userspace_freq_down(int cpu);
> +int cpufreq_userspace_freq_up(int cpu);
> +int cpufreq_userspace_freq_max(int cpu);
> +int cpufreq_userspace_freq_min(int cpu);
> +#else

Don't want to put dummy routines here? Then why the blank #else part ?

> +#endif
>  #endif /* _LINUX_CPUFREQ_H */
> 

-- 
viresh

Re: [patch 1/3] cpufreq: implement min/max/up/down functions

[Viresh Kumar]

On 02-02-17, 15:47, Marcelo Tosatti wrote:
> +++ kvm-pvfreq/drivers/cpufreq/cpufreq_userspace.c2017-02-02 
> 15:32:53.456262640 -0200
> @@ -118,6 +118,178 @@
>   mutex_unlock(_mutex);
>  }
>  
> +static int cpufreq_is_userspace_governor(int cpu)
> +{
> + int ret;
> +
> + mutex_lock(_mutex);
> + ret = per_cpu(cpu_is_managed, cpu);

The userspace governor is buggy in the sense that cpu_is_managed is only updated
for the policy->cpu and not any other CPU in that policy. But then it was never
used with anything other than policy->cpu, so it was fine.

But now that you are allowing any CPU number here, you need to do one of these:
- Either set cpu_is_managed for all the CPUs from a policy
- Or get the policy first and pass policy->cpu here.

> + mutex_unlock(_mutex);
> +
> + return ret;
> +}

All 4 routines defined below have too much in common and it would be very easy
to write a common routine cpufreq_userspace_freq_change(), which can be called
in all the four cases. You can pass a function pointer to that, which can give
min, max, up, or down frequencies. That will make it more robust and less error
prone.

> +int cpufreq_userspace_freq_up(int cpu)
> +{
> + unsigned int curfreq, nextminfreq;
> + unsigned int ret = 0;
> + struct cpufreq_frequency_table *pos, *table;
> + struct cpufreq_policy *policy = cpufreq_cpu_get(cpu);
> +
> + if (!policy)
> + return -EINVAL;
> +
> + if (!cpufreq_is_userspace_governor(cpu)) {
> + cpufreq_cpu_put(policy);
> + return -EINVAL;
> + }

Because the userspace_mutex is dropped after that routine returned, there is no
guarantee that 'cpu' is still managed by this governor. And so you need to make
sure that you drops the locks only at the end.

> +
> + cpufreq_cpu_put(policy);

This must be called only after you are done using the policy, to make sure that
the policy doesn't get freed while you are using it.

> + mutex_lock(_mutex);
> + table = policy->freq_table;
> + if (!table) {
> + mutex_unlock(_mutex);
> + return -ENODEV;
> + }
> + nextminfreq = cpufreq_quick_get_max(cpu);

Just use policy->max here, why waste time ?

> + curfreq = policy->cur;
> +
> + cpufreq_for_each_valid_entry(pos, table) {
> + if (pos->frequency > curfreq &&
> + pos->frequency < nextminfreq)
> + nextminfreq = pos->frequency;
> + }

The above part can be a routine of its own, whose pointer will be passed to
cpufreq_userspace_freq_change().

> +
> + if (nextminfreq != curfreq) {

You are missing similar checks in the last two routines, any special reason for
that ?

> + unsigned int *setspeed = policy->governor_data;
> +
> + *setspeed = nextminfreq;
> + ret = __cpufreq_driver_target(policy, nextminfreq,
> +   CPUFREQ_RELATION_L);
> + } else
> + ret = 1;

Why ret 1? What are the callers expected to do on seeing this value? Maybe
return 0 as the desired freq is set by the governor ?

And always use {} for even single line code if the 'if' block has them.

> + mutex_unlock(_mutex);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(cpufreq_userspace_freq_up);
> +++ kvm-pvfreq/include/linux/cpufreq.h2017-01-31 14:20:00.508613672 
> -0200
> @@ -890,4 +890,11 @@
>  int cpufreq_generic_init(struct cpufreq_policy *policy,
>   struct cpufreq_frequency_table *table,
>   unsigned int transition_latency);
> +#ifdef CONFIG_CPU_FREQ
> +int cpufreq_userspace_freq_down(int cpu);
> +int cpufreq_userspace_freq_up(int cpu);
> +int cpufreq_userspace_freq_max(int cpu);
> +int cpufreq_userspace_freq_min(int cpu);
> +#else

Don't want to put dummy routines here? Then why the blank #else part ?

> +#endif
>  #endif /* _LINUX_CPUFREQ_H */
> 

-- 
viresh

Re: modversions: redefine kcrctab entries as 32-bit values

[Jessica Yu]

+++ Ard Biesheuvel [24/01/17 16:16 +]:

This v4 is a followup to [0] 'modversions: redefine kcrctab entries as
relative CRC pointers', but since relative CRC pointers do not work in
modules, and are actually only needed by powerpc with CONFIG_RELOCATABLE=y,
I have made it a Kconfig selectable feature instead.

Patch #1 introduces the MODULE_REL_CRCS Kconfig symbol, and adds the kbuild
handling of it, i.e., modpost, genksyms and kallsyms.

Patch #2 switches all architectures to 32-bit CRC entries in kcrctab, where
all architectures except powerpc with CONFIG_RELOCATABLE=y use absolute ELF
symbol references as before.

v4: make relative CRCs kconfig selectable
   use absolute CRC symbols in modules regardless of kconfig selection
   split into two patches


This asymmetry threw me off a bit, especially the Kconfig naming (only
vmlinux crcs get the relative offsets, and only on powerpc atm, but all
modules keep the absolute syms, but it is called MODULE_REL_CRCS...),
if we keep this asymmetric crc treatment, it would be really nice to
note this discrepancy this somewhere, perhaps in the Kconfig, to keep
our heads from spinning :-)

I'm still catching up on the previous discussion threads, but can you
explain a bit more why you switched away from full blown relative crcs
from your last patchset [1]? I had lightly tested your v3 on ppc64le
previously, and relative offsets with modules seemed to worked very
well. I'm probably missing something very obvious.

[1] http://marc.info/?l=linux-arch=148493613415294=2


v3: emit CRCs into .rodata rather than .rodata.modver, given that the latter
   will be emitted with read-write permissions, making the CRCs end up in a
   writable module segment.

   fold the modpost fix to ensure that the section address is only substracted
   from the symbol address when the ELF object in question is fully linked
   (i.e., ET_DYN or ET_EXEC, and not ET_REL)

v2: update modpost as well, so that genksyms no longer has to emit symbols
   for both the actual CRC value and the reference to where it is stored
   in the image

[0] http://marc.info/?l=linux-arch=148493613415294=2

Ard Biesheuvel (2):
 kbuild: modversions: add infrastructure for emitting relative CRCs
 modversions: treat symbol CRCs as 32 bit quantities

arch/powerpc/Kconfig  |  1 +
arch/powerpc/include/asm/module.h |  4 --
arch/powerpc/kernel/module_64.c   |  8 
include/asm-generic/export.h  | 11 ++---
include/linux/export.h| 14 ++
include/linux/module.h| 14 +++---
init/Kconfig  |  4 ++
kernel/module.c   | 45 ++--
scripts/Makefile.build|  2 +
scripts/genksyms/genksyms.c   | 19 ++---
scripts/kallsyms.c| 12 ++
scripts/mod/modpost.c | 10 +
12 files changed, 93 insertions(+), 51 deletions(-)

--
2.7.4

Re: modversions: redefine kcrctab entries as 32-bit values

[Jessica Yu]

+++ Ard Biesheuvel [24/01/17 16:16 +]:

This v4 is a followup to [0] 'modversions: redefine kcrctab entries as
relative CRC pointers', but since relative CRC pointers do not work in
modules, and are actually only needed by powerpc with CONFIG_RELOCATABLE=y,
I have made it a Kconfig selectable feature instead.

Patch #1 introduces the MODULE_REL_CRCS Kconfig symbol, and adds the kbuild
handling of it, i.e., modpost, genksyms and kallsyms.

Patch #2 switches all architectures to 32-bit CRC entries in kcrctab, where
all architectures except powerpc with CONFIG_RELOCATABLE=y use absolute ELF
symbol references as before.

v4: make relative CRCs kconfig selectable
   use absolute CRC symbols in modules regardless of kconfig selection
   split into two patches


This asymmetry threw me off a bit, especially the Kconfig naming (only
vmlinux crcs get the relative offsets, and only on powerpc atm, but all
modules keep the absolute syms, but it is called MODULE_REL_CRCS...),
if we keep this asymmetric crc treatment, it would be really nice to
note this discrepancy this somewhere, perhaps in the Kconfig, to keep
our heads from spinning :-)

I'm still catching up on the previous discussion threads, but can you
explain a bit more why you switched away from full blown relative crcs
from your last patchset [1]? I had lightly tested your v3 on ppc64le
previously, and relative offsets with modules seemed to worked very
well. I'm probably missing something very obvious.

[1] http://marc.info/?l=linux-arch=148493613415294=2


v3: emit CRCs into .rodata rather than .rodata.modver, given that the latter
   will be emitted with read-write permissions, making the CRCs end up in a
   writable module segment.

   fold the modpost fix to ensure that the section address is only substracted
   from the symbol address when the ELF object in question is fully linked
   (i.e., ET_DYN or ET_EXEC, and not ET_REL)

v2: update modpost as well, so that genksyms no longer has to emit symbols
   for both the actual CRC value and the reference to where it is stored
   in the image

[0] http://marc.info/?l=linux-arch=148493613415294=2

Ard Biesheuvel (2):
 kbuild: modversions: add infrastructure for emitting relative CRCs
 modversions: treat symbol CRCs as 32 bit quantities

arch/powerpc/Kconfig  |  1 +
arch/powerpc/include/asm/module.h |  4 --
arch/powerpc/kernel/module_64.c   |  8 
include/asm-generic/export.h  | 11 ++---
include/linux/export.h| 14 ++
include/linux/module.h| 14 +++---
init/Kconfig  |  4 ++
kernel/module.c   | 45 ++--
scripts/Makefile.build|  2 +
scripts/genksyms/genksyms.c   | 19 ++---
scripts/kallsyms.c| 12 ++
scripts/mod/modpost.c | 10 +
12 files changed, 93 insertions(+), 51 deletions(-)

--
2.7.4

[PATCH] Revert "vring: Force use of DMA API for ARM-based systems with legacy devices"

[Michael S. Tsirkin]

This reverts commit c7070619f3408d9a0dffbed9149e6f00479cf43b.

This has been shown to regress on some ARM systems:

by forcing on DMA API usage for ARM systems, we have inadvertently
kicked open a hornets' nest in terms of cache-coherency. Namely that
unless the virtio device is explicitly described as capable of coherent
DMA by firmware, the DMA APIs on ARM and other DT-based platforms will
assume it is non-coherent. This turns out to cause a big problem for the
likes of QEMU and kvmtool, which generate virtio-mmio devices in their
guest DTs but neglect to add the often-overlooked "dma-coherent"
property; as a result, we end up with the guest making non-cacheable
accesses to the vring, the host doing so cacheably, both talking past
each other and things going horribly wrong.

We are working on a safer work-around.

Fixes: c7070619f340 ("vring: Force use of DMA API for ARM-based systems with 
legacy devices")
Reported-by: Robin Murphy 
Cc: 
Signed-off-by: Will Deacon 
Signed-off-by: Michael S. Tsirkin 
Acked-by: Marc Zyngier 
---

I'll merge this for 4.10. Let's fix it properly for 4.11.

 drivers/virtio/virtio_ring.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 7e38ed7..409aeaa 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -159,13 +159,6 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
if (xen_domain())
return true;
 
-   /*
-* On ARM-based machines, the DMA ops will do the right thing,
-* so always use them with legacy devices.
-*/
-   if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64))
-   return !virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
-
return false;
 }
 
-- 
MST

[PATCH] Revert "vring: Force use of DMA API for ARM-based systems with legacy devices"

[Michael S. Tsirkin]

This reverts commit c7070619f3408d9a0dffbed9149e6f00479cf43b.

This has been shown to regress on some ARM systems:

by forcing on DMA API usage for ARM systems, we have inadvertently
kicked open a hornets' nest in terms of cache-coherency. Namely that
unless the virtio device is explicitly described as capable of coherent
DMA by firmware, the DMA APIs on ARM and other DT-based platforms will
assume it is non-coherent. This turns out to cause a big problem for the
likes of QEMU and kvmtool, which generate virtio-mmio devices in their
guest DTs but neglect to add the often-overlooked "dma-coherent"
property; as a result, we end up with the guest making non-cacheable
accesses to the vring, the host doing so cacheably, both talking past
each other and things going horribly wrong.

We are working on a safer work-around.

Fixes: c7070619f340 ("vring: Force use of DMA API for ARM-based systems with 
legacy devices")
Reported-by: Robin Murphy 
Cc: 
Signed-off-by: Will Deacon 
Signed-off-by: Michael S. Tsirkin 
Acked-by: Marc Zyngier 
---

I'll merge this for 4.10. Let's fix it properly for 4.11.

 drivers/virtio/virtio_ring.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 7e38ed7..409aeaa 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -159,13 +159,6 @@ static bool vring_use_dma_api(struct virtio_device *vdev)
if (xen_domain())
return true;
 
-   /*
-* On ARM-based machines, the DMA ops will do the right thing,
-* so always use them with legacy devices.
-*/
-   if (IS_ENABLED(CONFIG_ARM) || IS_ENABLED(CONFIG_ARM64))
-   return !virtio_has_feature(vdev, VIRTIO_F_VERSION_1);
-
return false;
 }
 
-- 
MST

Re: [PATCH] thermal: use cpumask_var_t for on-stack cpu masks

[Viresh Kumar]

On 02-02-17, 15:46, Arnd Bergmann wrote:
> Putting a bare cpumask structure on the stack produces a warning on
> large SMP configurations:
> 
> drivers/thermal/cpu_cooling.c: In function 'cpufreq_state2power':
> drivers/thermal/cpu_cooling.c:644:1: warning: the frame size of 1056 bytes is 
> larger than 1024 bytes [-Wframe-larger-than=]
> drivers/thermal/cpu_cooling.c: In function '__cpufreq_cooling_register':
> drivers/thermal/cpu_cooling.c:898:1: warning: the frame size of 1104 bytes is 
> larger than 1024 bytes [-Wframe-larger-than=]
> 
> The recommended workaround is to use cpumask_var_t, which behaves just like
> a normal cpu mask in most cases, but turns into a dynamic allocation
> when CONFIG_CPUMASK_OFFSTACK is set.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/thermal/cpu_cooling.c | 39 ++-
>  1 file changed, 26 insertions(+), 13 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCH] thermal: use cpumask_var_t for on-stack cpu masks

[Viresh Kumar]

On 02-02-17, 15:46, Arnd Bergmann wrote:
> Putting a bare cpumask structure on the stack produces a warning on
> large SMP configurations:
> 
> drivers/thermal/cpu_cooling.c: In function 'cpufreq_state2power':
> drivers/thermal/cpu_cooling.c:644:1: warning: the frame size of 1056 bytes is 
> larger than 1024 bytes [-Wframe-larger-than=]
> drivers/thermal/cpu_cooling.c: In function '__cpufreq_cooling_register':
> drivers/thermal/cpu_cooling.c:898:1: warning: the frame size of 1104 bytes is 
> larger than 1024 bytes [-Wframe-larger-than=]
> 
> The recommended workaround is to use cpumask_var_t, which behaves just like
> a normal cpu mask in most cases, but turns into a dynamic allocation
> when CONFIG_CPUMASK_OFFSTACK is set.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/thermal/cpu_cooling.c | 39 ++-
>  1 file changed, 26 insertions(+), 13 deletions(-)

Acked-by: Viresh Kumar 

-- 
viresh

Re: [PATCHv2] ARM: dts: Add EMAC AXI settings for Arria10

[Dinh Nguyen]

On Thu, Feb 2, 2017 at 4:05 PM,   wrote:
> From: Thor Thayer 
>
> Add the device tree entries needed to support the EMAC AXI
> bus settings on the Arria10 SoCFPGA chip.
>
> Signed-off-by: Thor Thayer 
> ---
> v2 Add the AXI configuration to the other DW EMACs in the chip.
> ---
>  arch/arm/boot/dts/socfpga_arria10.dtsi | 9 +
>  1 file changed, 9 insertions(+)
>

Applied!

Thanks,
Dinh

Re: [PATCHv2] ARM: dts: Add EMAC AXI settings for Arria10

[Dinh Nguyen]

On Thu, Feb 2, 2017 at 4:05 PM,   wrote:
> From: Thor Thayer 
>
> Add the device tree entries needed to support the EMAC AXI
> bus settings on the Arria10 SoCFPGA chip.
>
> Signed-off-by: Thor Thayer 
> ---
> v2 Add the AXI configuration to the other DW EMACs in the chip.
> ---
>  arch/arm/boot/dts/socfpga_arria10.dtsi | 9 +
>  1 file changed, 9 insertions(+)
>

Applied!

Thanks,
Dinh

[PATCH 1/4] ARM: dts: armada-xp-98dx3236: combine dfx server nodes

[Chris Packham]

Rather than having a separate node for the dfx server add a reg property
to the parent node. This give somes compatibility with the Marvell
supplied SDK.

Signed-off-by: Chris Packham 
---
 Documentation/devicetree/bindings/net/marvell,prestera.txt | 13 +
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi  |  8 ++--
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.txt 
b/Documentation/devicetree/bindings/net/marvell,prestera.txt
index 5fbab29718e8..c329608fa887 100644
--- a/Documentation/devicetree/bindings/net/marvell,prestera.txt
+++ b/Documentation/devicetree/bindings/net/marvell,prestera.txt
@@ -32,19 +32,16 @@ DFX Server bindings
 ---
 
 Required properties:
-- compatible: must be "marvell,dfx-server"
+- compatible: must be "marvell,dfx-server", "simple-bus"
+- ranges: describes the address mapping of a memory-mapped bus.
 - reg: address and length of the register set for the device.
 
 Example:
 
-dfx-registers {
-   compatible = "simple-bus";
+dfx-server {
+   compatible = "marvell,dfx-server", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 MBUS_ID(0x08, 0x00) 0 0x10>;
-
-   dfx: dfx@0 {
-   compatible = "marvell,dfx-server";
-   reg = <0 0x10>;
-   };
+   reg = ;
 };
diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi 
b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index f6a03dcee5ef..bd8261fdec81 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -195,11 +195,12 @@
};
};
 
-   dfxr: dfx-registers@ac00 {
+   dfx: dfx-server@ac00 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 MBUS_ID(0x08, 0x00) 0 0x10>;
+   reg = ;
 
dfx_coredivclk: corediv-clock@f8268 {
compatible = "marvell,mv98dx3236-corediv-clock";
@@ -208,11 +209,6 @@
clocks = <>;
clock-output-names = "nand";
};
-
-   dfx: dfx@0 {
-   compatible = "marvell,dfx-server";
-   reg = <0 0x10>;
-   };
};
 
switch: switch@a800 {
-- 
2.11.0.24.ge6920cf

[PATCH 1/4] ARM: dts: armada-xp-98dx3236: combine dfx server nodes

[Chris Packham]

Rather than having a separate node for the dfx server add a reg property
to the parent node. This give somes compatibility with the Marvell
supplied SDK.

Signed-off-by: Chris Packham 
---
 Documentation/devicetree/bindings/net/marvell,prestera.txt | 13 +
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi  |  8 ++--
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell,prestera.txt 
b/Documentation/devicetree/bindings/net/marvell,prestera.txt
index 5fbab29718e8..c329608fa887 100644
--- a/Documentation/devicetree/bindings/net/marvell,prestera.txt
+++ b/Documentation/devicetree/bindings/net/marvell,prestera.txt
@@ -32,19 +32,16 @@ DFX Server bindings
 ---
 
 Required properties:
-- compatible: must be "marvell,dfx-server"
+- compatible: must be "marvell,dfx-server", "simple-bus"
+- ranges: describes the address mapping of a memory-mapped bus.
 - reg: address and length of the register set for the device.
 
 Example:
 
-dfx-registers {
-   compatible = "simple-bus";
+dfx-server {
+   compatible = "marvell,dfx-server", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 MBUS_ID(0x08, 0x00) 0 0x10>;
-
-   dfx: dfx@0 {
-   compatible = "marvell,dfx-server";
-   reg = <0 0x10>;
-   };
+   reg = ;
 };
diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi 
b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index f6a03dcee5ef..bd8261fdec81 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -195,11 +195,12 @@
};
};
 
-   dfxr: dfx-registers@ac00 {
+   dfx: dfx-server@ac00 {
compatible = "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges = <0 MBUS_ID(0x08, 0x00) 0 0x10>;
+   reg = ;
 
dfx_coredivclk: corediv-clock@f8268 {
compatible = "marvell,mv98dx3236-corediv-clock";
@@ -208,11 +209,6 @@
clocks = <>;
clock-output-names = "nand";
};
-
-   dfx: dfx@0 {
-   compatible = "marvell,dfx-server";
-   reg = <0 0x10>;
-   };
};
 
switch: switch@a800 {
-- 
2.11.0.24.ge6920cf

[PATCH 4/4] clk: mvebu: Expand mv98dx3236-core-clock support

[Chris Packham]

The initial implementation in commit e120c17a70e5 ("clk: mvebu: support
for 98DX3236 SoC") hardcoded a fixed value for the main PLL frequency.
Port code from the Marvell supplied Linux kernel to support different
PLL frequencies and provide clock gating support.

Signed-off-by: Chris Packham 
---
 .../devicetree/bindings/clock/mvebu-core-clock.txt |   7 +
 .../bindings/clock/mvebu-gated-clock.txt   |  11 ++
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi  |  14 +-
 drivers/clk/mvebu/Makefile |   2 +-
 drivers/clk/mvebu/armada-xp.c  |  13 --
 drivers/clk/mvebu/mv98dx3236.c | 144 +
 6 files changed, 170 insertions(+), 21 deletions(-)
 create mode 100644 drivers/clk/mvebu/mv98dx3236.c

diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt 
b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index eb985a633d59..796c260c183d 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -31,6 +31,12 @@ The following is a list of provided IDs and clock names on 
Armada 39x:
  4 = dclk(SDRAM Interface Clock)
  5 = refclk  (Reference Clock)
 
+The following is a list of provided IDs and clock names on 98dx3236:
+ 0 = tclk(Internal Bus clock)
+ 1 = cpuclk  (CPU clock)
+ 2 = ddrclk   (DDR clock)
+ 3 = mpll(MPLL Clock)
+
 The following is a list of provided IDs and clock names on Kirkwood and Dove:
  0 = tclk   (Internal Bus clock)
  1 = cpuclk (CPU0 clock)
@@ -49,6 +55,7 @@ Required properties:
"marvell,armada-380-core-clock" - For Armada 380/385 SoC core clocks
"marvell,armada-390-core-clock" - For Armada 39x SoC core clocks
"marvell,armada-xp-core-clock" - For Armada XP SoC core clocks
+   "marvell,mv98dx3236-core-clock" - For 98dx3236 family SoC core clocks
"marvell,dove-core-clock" - for Dove SoC core clocks
"marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180)
"marvell,mv88f6180-core-clock" - for Kirkwood MV88f6180 SoC
diff --git a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt 
b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
index 5142efc8099d..de562da2ae77 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
@@ -119,6 +119,16 @@ ID Clock   Peripheral
 29 sata1lnk
 30 sata1   SATA Host 1
 
+The following is a list of provided IDs for 98dx3236:
+ID Clock   Peripheral
+---
+3  ge1 Gigabit Ethernet 1
+4  ge0 Gigabit Ethernet 0
+5  pex0PCIe Cntrl 0
+17 sdioSDHCI Host
+18 usb0USB Host 0
+22 xor0XOR DMA 0
+
 The following is a list of provided IDs for Dove:
 ID Clock   Peripheral
 ---
@@ -169,6 +179,7 @@ Required properties:
"marvell,armada-380-gating-clock" - for Armada 380/385 SoC clock gating
"marvell,armada-390-gating-clock" - for Armada 39x SoC clock gating
"marvell,armada-xp-gating-clock" - for Armada XP SoC clock gating
+   "marvell,mv98dx3236-gating-clock" - for 98dx3236 SoC clock gating
"marvell,dove-gating-clock" - for Dove SoC clock gating
"marvell,kirkwood-gating-clock" - for Kirkwood SoC clock gating
 - reg : shall be the register address of the Clock Gating Control register
diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi 
b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index e4baa97836e7..e80a5ee835e5 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -176,18 +176,12 @@
};
 
gateclk: clock-gating-control@18220 {
-   compatible = "marvell,armada-xp-gating-clock";
+   compatible = "marvell,mv98dx3236-gating-clock";
reg = <0x18220 0x4>;
clocks = < 0>;
#clock-cells = <1>;
};
 
-   coreclk: mvebu-sar@18230 {
-   compatible = "marvell,mv98dx3236-core-clock";
-   reg = <0x18230 0x08>;
-   #clock-cells = <1>;
-   };
-
cpuclk: clock-complex@18700 {
#clock-cells = <1>;
compatible = "marvell,mv98dx3236-cpu-clock";
@@ -264,6 +258,12 @@
ranges = <0 MBUS_ID(0x08, 0x00) 0 0x10>;
reg = ;
 
+   coreclk: mvebu-sar@f8204 {
+   compatible = "marvell,mv98dx3236-core-clock";
+   

[PATCH 4/4] clk: mvebu: Expand mv98dx3236-core-clock support

[Chris Packham]

The initial implementation in commit e120c17a70e5 ("clk: mvebu: support
for 98DX3236 SoC") hardcoded a fixed value for the main PLL frequency.
Port code from the Marvell supplied Linux kernel to support different
PLL frequencies and provide clock gating support.

Signed-off-by: Chris Packham 
---
 .../devicetree/bindings/clock/mvebu-core-clock.txt |   7 +
 .../bindings/clock/mvebu-gated-clock.txt   |  11 ++
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi  |  14 +-
 drivers/clk/mvebu/Makefile |   2 +-
 drivers/clk/mvebu/armada-xp.c  |  13 --
 drivers/clk/mvebu/mv98dx3236.c | 144 +
 6 files changed, 170 insertions(+), 21 deletions(-)
 create mode 100644 drivers/clk/mvebu/mv98dx3236.c

diff --git a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt 
b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
index eb985a633d59..796c260c183d 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-core-clock.txt
@@ -31,6 +31,12 @@ The following is a list of provided IDs and clock names on 
Armada 39x:
  4 = dclk(SDRAM Interface Clock)
  5 = refclk  (Reference Clock)
 
+The following is a list of provided IDs and clock names on 98dx3236:
+ 0 = tclk(Internal Bus clock)
+ 1 = cpuclk  (CPU clock)
+ 2 = ddrclk   (DDR clock)
+ 3 = mpll(MPLL Clock)
+
 The following is a list of provided IDs and clock names on Kirkwood and Dove:
  0 = tclk   (Internal Bus clock)
  1 = cpuclk (CPU0 clock)
@@ -49,6 +55,7 @@ Required properties:
"marvell,armada-380-core-clock" - For Armada 380/385 SoC core clocks
"marvell,armada-390-core-clock" - For Armada 39x SoC core clocks
"marvell,armada-xp-core-clock" - For Armada XP SoC core clocks
+   "marvell,mv98dx3236-core-clock" - For 98dx3236 family SoC core clocks
"marvell,dove-core-clock" - for Dove SoC core clocks
"marvell,kirkwood-core-clock" - for Kirkwood SoC (except mv88f6180)
"marvell,mv88f6180-core-clock" - for Kirkwood MV88f6180 SoC
diff --git a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt 
b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
index 5142efc8099d..de562da2ae77 100644
--- a/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
+++ b/Documentation/devicetree/bindings/clock/mvebu-gated-clock.txt
@@ -119,6 +119,16 @@ ID Clock   Peripheral
 29 sata1lnk
 30 sata1   SATA Host 1
 
+The following is a list of provided IDs for 98dx3236:
+ID Clock   Peripheral
+---
+3  ge1 Gigabit Ethernet 1
+4  ge0 Gigabit Ethernet 0
+5  pex0PCIe Cntrl 0
+17 sdioSDHCI Host
+18 usb0USB Host 0
+22 xor0XOR DMA 0
+
 The following is a list of provided IDs for Dove:
 ID Clock   Peripheral
 ---
@@ -169,6 +179,7 @@ Required properties:
"marvell,armada-380-gating-clock" - for Armada 380/385 SoC clock gating
"marvell,armada-390-gating-clock" - for Armada 39x SoC clock gating
"marvell,armada-xp-gating-clock" - for Armada XP SoC clock gating
+   "marvell,mv98dx3236-gating-clock" - for 98dx3236 SoC clock gating
"marvell,dove-gating-clock" - for Dove SoC clock gating
"marvell,kirkwood-gating-clock" - for Kirkwood SoC clock gating
 - reg : shall be the register address of the Clock Gating Control register
diff --git a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi 
b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
index e4baa97836e7..e80a5ee835e5 100644
--- a/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
+++ b/arch/arm/boot/dts/armada-xp-98dx3236.dtsi
@@ -176,18 +176,12 @@
};
 
gateclk: clock-gating-control@18220 {
-   compatible = "marvell,armada-xp-gating-clock";
+   compatible = "marvell,mv98dx3236-gating-clock";
reg = <0x18220 0x4>;
clocks = < 0>;
#clock-cells = <1>;
};
 
-   coreclk: mvebu-sar@18230 {
-   compatible = "marvell,mv98dx3236-core-clock";
-   reg = <0x18230 0x08>;
-   #clock-cells = <1>;
-   };
-
cpuclk: clock-complex@18700 {
#clock-cells = <1>;
compatible = "marvell,mv98dx3236-cpu-clock";
@@ -264,6 +258,12 @@
ranges = <0 MBUS_ID(0x08, 0x00) 0 0x10>;
reg = ;
 
+   coreclk: mvebu-sar@f8204 {
+   compatible = "marvell,mv98dx3236-core-clock";
+   reg = <0xf8204 0x4>;
+   #clock-cells =