[PATCH v2 2/5] video: smscufx: Less checks in ufx_usb_probe() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 26 Nov 2017 08:18:20 +0100

Up to four checks could be repeated by the ufx_usb_probe() function
during error handling even if the relevant properties can be determined
for the involved variables before by source code analysis.

* Return directly after a call of the function "kzalloc" failed
  at the beginning.

* Adjust jump targets so that extra checks can be omitted at the end.

* Delete initialisations for the variables "info" and "retval"
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring 
---

v2:
A call of the function "fb_dealloc_cmap" was preserved for the exception
handling at the end.

 drivers/video/fbdev/smscufx.c | 46 ++-
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 32de07d77184..a48a2dec3f5e 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -1619,8 +1619,8 @@ static int ufx_usb_probe(struct usb_interface *interface,
 {
struct usb_device *usbdev;
struct ufx_data *dev;
-   struct fb_info *info = NULL;
-   int retval = -ENOMEM;
+   struct fb_info *info;
+   int retval;
u32 id_rev, fpga_rev;
 
/* usb initialization */
@@ -1629,7 +1629,7 @@ static int ufx_usb_probe(struct usb_interface *interface,
 
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
-   goto error;
+   return -ENOMEM;
 
/* we need to wait for both usb and fbdev to spin down on disconnect */
kref_init(>kref); /* matching kref_put in usb .disconnect fn */
@@ -1649,9 +1649,8 @@ static int ufx_usb_probe(struct usb_interface *interface,
dev_dbg(dev->gdev, "fb_defio enable=%d\n", fb_defio);
 
if (!ufx_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
-   retval = -ENOMEM;
dev_err(dev->gdev, "ufx_alloc_urb_list failed\n");
-   goto error;
+   goto e_nomem;
}
 
/* We don't register a new USB class. Our client interface is fbdev */
@@ -1659,9 +1658,8 @@ static int ufx_usb_probe(struct usb_interface *interface,
/* allocates framebuffer driver structure, not framebuffer memory */
info = framebuffer_alloc(0, >dev);
if (!info) {
-   retval = -ENOMEM;
dev_err(dev->gdev, "framebuffer_alloc failed\n");
-   goto error;
+   goto e_nomem;
}
 
dev->info = info;
@@ -1672,7 +1670,7 @@ static int ufx_usb_probe(struct usb_interface *interface,
retval = fb_alloc_cmap(>cmap, 256, 0);
if (retval < 0) {
dev_err(dev->gdev, "fb_alloc_cmap failed %x\n", retval);
-   goto error;
+   goto destroy_modedb;
}
 
INIT_DELAYED_WORK(>free_framebuffer_work,
@@ -1733,26 +1731,20 @@ static int ufx_usb_probe(struct usb_interface 
*interface,
return 0;
 
 error:
-   if (dev) {
-   if (info) {
-   if (info->cmap.len != 0)
-   fb_dealloc_cmap(>cmap);
-   if (info->monspecs.modedb)
-   fb_destroy_modedb(info->monspecs.modedb);
-   vfree(info->screen_base);
-
-   fb_destroy_modelist(>modelist);
-
-   framebuffer_release(info);
-   }
-
-   kref_put(>kref, ufx_free); /* ref for framebuffer */
-   kref_put(>kref, ufx_free); /* last ref from kref_init */
-
-   /* dev has been deallocated. Do not dereference */
-   }
-
+   fb_dealloc_cmap(>cmap);
+destroy_modedb:
+   fb_destroy_modedb(info->monspecs.modedb);
+   vfree(info->screen_base);
+   fb_destroy_modelist(>modelist);
+   framebuffer_release(info);
+put_ref:
+   kref_put(>kref, ufx_free); /* ref for framebuffer */
+   kref_put(>kref, ufx_free); /* last ref from kref_init */
return retval;
+
+e_nomem:
+   retval = -ENOMEM;
+   goto put_ref;
 }
 
 static void ufx_usb_disconnect(struct usb_interface *interface)
-- 
2.15.0

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

[Stephen Rothwell]

Hi Liviu,

Commit

  7f2189b12359 ("drm: Fix checkpatch issue: "WARNING: braces {} are not 
necessary for single statement blocks."")

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

-- 
Cheers,
Stephen Rothwell

[PATCH v2 2/5] video: smscufx: Less checks in ufx_usb_probe() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Sun, 26 Nov 2017 08:18:20 +0100

Up to four checks could be repeated by the ufx_usb_probe() function
during error handling even if the relevant properties can be determined
for the involved variables before by source code analysis.

* Return directly after a call of the function "kzalloc" failed
  at the beginning.

* Adjust jump targets so that extra checks can be omitted at the end.

* Delete initialisations for the variables "info" and "retval"
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring 
---

v2:
A call of the function "fb_dealloc_cmap" was preserved for the exception
handling at the end.

 drivers/video/fbdev/smscufx.c | 46 ++-
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/video/fbdev/smscufx.c b/drivers/video/fbdev/smscufx.c
index 32de07d77184..a48a2dec3f5e 100644
--- a/drivers/video/fbdev/smscufx.c
+++ b/drivers/video/fbdev/smscufx.c
@@ -1619,8 +1619,8 @@ static int ufx_usb_probe(struct usb_interface *interface,
 {
struct usb_device *usbdev;
struct ufx_data *dev;
-   struct fb_info *info = NULL;
-   int retval = -ENOMEM;
+   struct fb_info *info;
+   int retval;
u32 id_rev, fpga_rev;
 
/* usb initialization */
@@ -1629,7 +1629,7 @@ static int ufx_usb_probe(struct usb_interface *interface,
 
dev = kzalloc(sizeof(*dev), GFP_KERNEL);
if (!dev)
-   goto error;
+   return -ENOMEM;
 
/* we need to wait for both usb and fbdev to spin down on disconnect */
kref_init(>kref); /* matching kref_put in usb .disconnect fn */
@@ -1649,9 +1649,8 @@ static int ufx_usb_probe(struct usb_interface *interface,
dev_dbg(dev->gdev, "fb_defio enable=%d\n", fb_defio);
 
if (!ufx_alloc_urb_list(dev, WRITES_IN_FLIGHT, MAX_TRANSFER)) {
-   retval = -ENOMEM;
dev_err(dev->gdev, "ufx_alloc_urb_list failed\n");
-   goto error;
+   goto e_nomem;
}
 
/* We don't register a new USB class. Our client interface is fbdev */
@@ -1659,9 +1658,8 @@ static int ufx_usb_probe(struct usb_interface *interface,
/* allocates framebuffer driver structure, not framebuffer memory */
info = framebuffer_alloc(0, >dev);
if (!info) {
-   retval = -ENOMEM;
dev_err(dev->gdev, "framebuffer_alloc failed\n");
-   goto error;
+   goto e_nomem;
}
 
dev->info = info;
@@ -1672,7 +1670,7 @@ static int ufx_usb_probe(struct usb_interface *interface,
retval = fb_alloc_cmap(>cmap, 256, 0);
if (retval < 0) {
dev_err(dev->gdev, "fb_alloc_cmap failed %x\n", retval);
-   goto error;
+   goto destroy_modedb;
}
 
INIT_DELAYED_WORK(>free_framebuffer_work,
@@ -1733,26 +1731,20 @@ static int ufx_usb_probe(struct usb_interface 
*interface,
return 0;
 
 error:
-   if (dev) {
-   if (info) {
-   if (info->cmap.len != 0)
-   fb_dealloc_cmap(>cmap);
-   if (info->monspecs.modedb)
-   fb_destroy_modedb(info->monspecs.modedb);
-   vfree(info->screen_base);
-
-   fb_destroy_modelist(>modelist);
-
-   framebuffer_release(info);
-   }
-
-   kref_put(>kref, ufx_free); /* ref for framebuffer */
-   kref_put(>kref, ufx_free); /* last ref from kref_init */
-
-   /* dev has been deallocated. Do not dereference */
-   }
-
+   fb_dealloc_cmap(>cmap);
+destroy_modedb:
+   fb_destroy_modedb(info->monspecs.modedb);
+   vfree(info->screen_base);
+   fb_destroy_modelist(>modelist);
+   framebuffer_release(info);
+put_ref:
+   kref_put(>kref, ufx_free); /* ref for framebuffer */
+   kref_put(>kref, ufx_free); /* last ref from kref_init */
return retval;
+
+e_nomem:
+   retval = -ENOMEM;
+   goto put_ref;
 }
 
 static void ufx_usb_disconnect(struct usb_interface *interface)
-- 
2.15.0

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

[Stephen Rothwell]

Hi Liviu,

Commit

  7f2189b12359 ("drm: Fix checkpatch issue: "WARNING: braces {} are not 
necessary for single statement blocks."")

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

-- 
Cheers,
Stephen Rothwell

Re: [PATCH v2 1/5] mm: memory_hotplug: Memory hotplug (add) support for arm64

[Arun KS]

On Fri, Nov 24, 2017 at 4:23 PM, Maciej Bielski
 wrote:
> On Fri, Nov 24, 2017 at 09:42:33AM +, Andrea Reale wrote:
>> Hi Arun,
>>
>>
>> On Fri 24 Nov 2017, 11:25, Arun KS wrote:
>> > On Thu, Nov 23, 2017 at 4:43 PM, Maciej Bielski
>> >  wrote:
>> >> [ ...]
>> > > Introduces memory hotplug functionality (hot-add) for arm64.
>> > > @@ -615,6 +616,44 @@ void __init paging_init(void)
>> > >   SWAPPER_DIR_SIZE - PAGE_SIZE);
>> > >  }
>> > >
>> > > +#ifdef CONFIG_MEMORY_HOTPLUG
>> > > +
>> > > +/*
>> > > + * hotplug_paging() is used by memory hotplug to build new page tables
>> > > + * for hot added memory.
>> > > + */
>> > > +
>> > > +struct mem_range {
>> > > +   phys_addr_t base;
>> > > +   phys_addr_t size;
>> > > +};
>> > > +
>> > > +static int __hotplug_paging(void *data)
>> > > +{
>> > > +   int flags = 0;
>> > > +   struct mem_range *section = data;
>> > > +
>> > > +   if (debug_pagealloc_enabled())
>> > > +   flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>> > > +
>> > > +   __create_pgd_mapping(swapper_pg_dir, section->base,
>> > > +   __phys_to_virt(section->base), section->size,
>> > > +   PAGE_KERNEL, pgd_pgtable_alloc, flags);
>> >
>> > Hello Andrea,
>> >
>> > __hotplug_paging runs on stop_machine context.
>> > cpu stop callbacks must not sleep.
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/stop_machine.c?h=v4.14#n479
>> >
>> > __create_pgd_mapping uses pgd_pgtable_alloc. which does
>> > __get_free_page(PGALLOC_GFP)
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/mm/mmu.c?h=v4.14#n342
>> >
>> > PGALLOC_GFP has GFP_KERNEL which inturn has __GFP_RECLAIM
>> >
>> > #define PGALLOC_GFP (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO)
>> > #define GFP_KERNEL  (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
>> >
>> > Now, prepare_alloc_pages() called by __alloc_pages_nodemask checks for
>> >
>> > might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
>> >
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/page_alloc.c?h=v4.14#n4150
>> >
>> > and then BUG()
>>
>> Well spotted, thanks for reporting the problem. One possible solution
>> would be to revert back to building the updated page tables on a copy
>> pgdir (as it was done in v1 of this patchset) and then replacing swapper
>> atomically with stop_machine.
>>
>> Actually, I am not sure if stop_machine is strictly needed,
>> if we modify the swapper pgdir live: for example, in x86_64
>> kernel_physical_mapping_init, atomicity is ensured by spin-locking on
>> init_mm.page_table_lock.
>> https://elixir.free-electrons.com/linux/v4.14/source/arch/x86/mm/init_64.c#L684
>> I'll spend some time investigating whoever else could be working
>> concurrently on the swapper pgdir.
>>
>> Any suggestion or pointer is very welcome.
>
> Hi Andrea, Arun,
>
> Alternative approach could be implementing pgd_pgtable_alloc_nosleep() and
> pointing this to hotplug_paging(). Subsequently, it could use different flags,
> eg:
>
> #define PGALLOC_GFP_NORECLAIM   (__GFP_IO | __GFP_FS | __GFP_NOTRACK | 
> __GFP_ZERO)

This solves the problem with __get_free_page.

But pgd_pgtable_alloc() ->  pgtable_page_ctor() -> ptlock_alloc() and
then kmem_cache_alloc(page_ptl_cachep, GFP_KERNEL)
Same BUG again.

Regards,
Arun

>
> Is this unefficient approach in any way?
> Do we like the fact that the memory-attaching thread can go to sleep?
>
> BR,
>
>>
>> Thanks,
>> Andrea
>>
>> > I was testing on 4.4 kernel, but cross checked with 4.14 as well.
>> >
>> > Regards,
>> > Arun
>> >
>> >
>> > > +
>> > > +   return 0;
>> > > +}
>> > > +
>> > > +inline void hotplug_paging(phys_addr_t start, phys_addr_t size)
>> > > +{
>> > > +   struct mem_range section = {
>> > > +   .base = start,
>> > > +   .size = size,
>> > > +   };
>> > > +
>> > > +   stop_machine(__hotplug_paging, , NULL);
>> > > +}
>> > > +#endif /* CONFIG_MEMORY_HOTPLUG */
>> > > +
>> > >  /*
>> > >   * Check whether a kernel address is valid (derived from arch/x86/).
>> > >   */
>> > > --
>> > > 2.7.4
>> > >
>> >
>>
>
> --
> Maciej Bielski

Re: [PATCH v2 1/5] mm: memory_hotplug: Memory hotplug (add) support for arm64

[Arun KS]

On Fri, Nov 24, 2017 at 4:23 PM, Maciej Bielski
 wrote:
> On Fri, Nov 24, 2017 at 09:42:33AM +, Andrea Reale wrote:
>> Hi Arun,
>>
>>
>> On Fri 24 Nov 2017, 11:25, Arun KS wrote:
>> > On Thu, Nov 23, 2017 at 4:43 PM, Maciej Bielski
>> >  wrote:
>> >> [ ...]
>> > > Introduces memory hotplug functionality (hot-add) for arm64.
>> > > @@ -615,6 +616,44 @@ void __init paging_init(void)
>> > >   SWAPPER_DIR_SIZE - PAGE_SIZE);
>> > >  }
>> > >
>> > > +#ifdef CONFIG_MEMORY_HOTPLUG
>> > > +
>> > > +/*
>> > > + * hotplug_paging() is used by memory hotplug to build new page tables
>> > > + * for hot added memory.
>> > > + */
>> > > +
>> > > +struct mem_range {
>> > > +   phys_addr_t base;
>> > > +   phys_addr_t size;
>> > > +};
>> > > +
>> > > +static int __hotplug_paging(void *data)
>> > > +{
>> > > +   int flags = 0;
>> > > +   struct mem_range *section = data;
>> > > +
>> > > +   if (debug_pagealloc_enabled())
>> > > +   flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
>> > > +
>> > > +   __create_pgd_mapping(swapper_pg_dir, section->base,
>> > > +   __phys_to_virt(section->base), section->size,
>> > > +   PAGE_KERNEL, pgd_pgtable_alloc, flags);
>> >
>> > Hello Andrea,
>> >
>> > __hotplug_paging runs on stop_machine context.
>> > cpu stop callbacks must not sleep.
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/stop_machine.c?h=v4.14#n479
>> >
>> > __create_pgd_mapping uses pgd_pgtable_alloc. which does
>> > __get_free_page(PGALLOC_GFP)
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/mm/mmu.c?h=v4.14#n342
>> >
>> > PGALLOC_GFP has GFP_KERNEL which inturn has __GFP_RECLAIM
>> >
>> > #define PGALLOC_GFP (GFP_KERNEL | __GFP_NOTRACK | __GFP_ZERO)
>> > #define GFP_KERNEL  (__GFP_RECLAIM | __GFP_IO | __GFP_FS)
>> >
>> > Now, prepare_alloc_pages() called by __alloc_pages_nodemask checks for
>> >
>> > might_sleep_if(gfp_mask & __GFP_DIRECT_RECLAIM);
>> >
>> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/page_alloc.c?h=v4.14#n4150
>> >
>> > and then BUG()
>>
>> Well spotted, thanks for reporting the problem. One possible solution
>> would be to revert back to building the updated page tables on a copy
>> pgdir (as it was done in v1 of this patchset) and then replacing swapper
>> atomically with stop_machine.
>>
>> Actually, I am not sure if stop_machine is strictly needed,
>> if we modify the swapper pgdir live: for example, in x86_64
>> kernel_physical_mapping_init, atomicity is ensured by spin-locking on
>> init_mm.page_table_lock.
>> https://elixir.free-electrons.com/linux/v4.14/source/arch/x86/mm/init_64.c#L684
>> I'll spend some time investigating whoever else could be working
>> concurrently on the swapper pgdir.
>>
>> Any suggestion or pointer is very welcome.
>
> Hi Andrea, Arun,
>
> Alternative approach could be implementing pgd_pgtable_alloc_nosleep() and
> pointing this to hotplug_paging(). Subsequently, it could use different flags,
> eg:
>
> #define PGALLOC_GFP_NORECLAIM   (__GFP_IO | __GFP_FS | __GFP_NOTRACK | 
> __GFP_ZERO)

This solves the problem with __get_free_page.

But pgd_pgtable_alloc() ->  pgtable_page_ctor() -> ptlock_alloc() and
then kmem_cache_alloc(page_ptl_cachep, GFP_KERNEL)
Same BUG again.

Regards,
Arun

>
> Is this unefficient approach in any way?
> Do we like the fact that the memory-attaching thread can go to sleep?
>
> BR,
>
>>
>> Thanks,
>> Andrea
>>
>> > I was testing on 4.4 kernel, but cross checked with 4.14 as well.
>> >
>> > Regards,
>> > Arun
>> >
>> >
>> > > +
>> > > +   return 0;
>> > > +}
>> > > +
>> > > +inline void hotplug_paging(phys_addr_t start, phys_addr_t size)
>> > > +{
>> > > +   struct mem_range section = {
>> > > +   .base = start,
>> > > +   .size = size,
>> > > +   };
>> > > +
>> > > +   stop_machine(__hotplug_paging, , NULL);
>> > > +}
>> > > +#endif /* CONFIG_MEMORY_HOTPLUG */
>> > > +
>> > >  /*
>> > >   * Check whether a kernel address is valid (derived from arch/x86/).
>> > >   */
>> > > --
>> > > 2.7.4
>> > >
>> >
>>
>
> --
> Maciej Bielski

Re: [PATCHv2 2/2] userns: control capabilities of some user namespaces

[Serge E. Hallyn]

Quoting Mahesh Bandewar (mah...@bandewar.net):
> From: Mahesh Bandewar 
> 
> With this new notion of "controlled" user-namespaces, the controlled
> user-namespaces are marked at the time of their creation while the
> capabilities of processes that belong to them are controlled using the
> global mask.
> 
> Init-user-ns is always uncontrolled and a process that has SYS_ADMIN
> that belongs to uncontrolled user-ns can create another (child) user-
> namespace that is uncontrolled. Any other process (that either does
> not have SYS_ADMIN or belongs to a controlled user-ns) can only
> create a user-ns that is controlled.
> 
> global-capability-whitelist (controlled_userns_caps_whitelist) is used
> at the capability check-time and keeps the semantics for the processes
> that belong to uncontrolled user-ns as it is. Processes that belong to
> controlled user-ns however are subjected to different checks-
> 
>(a) if the capability in question is controlled and process belongs
>to controlled user-ns, then it's always denied.
>(b) if the capability in question is NOT controlled then fall back
>to the traditional check.
> 
> Signed-off-by: Mahesh Bandewar 

Acked-by: Serge Hallyn 

Although a few comment addition requests below:

> ---
> v2:
>   Don't recalculate user-ns flags for every setns() call.
> v1:
>   Initial submission.
> 
>  include/linux/capability.h |  1 +
>  include/linux/user_namespace.h | 20 
>  kernel/capability.c|  5 +
>  kernel/user_namespace.c|  4 
>  security/commoncap.c   |  8 
>  5 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 7d79a4689625..a1fd9e460379 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -251,6 +251,7 @@ extern bool ptracer_capable(struct task_struct *tsk, 
> struct user_namespace *ns);
>  extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct 
> cpu_vfs_cap_data *cpu_caps);
>  int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
>void __user *buff, size_t *lenp, loff_t *ppos);

Here and at the definition below, please add a comment explaining
that a controlled cap is defined as not being in the sysctl.

> +bool is_capability_controlled(int cap);
>  
>  extern int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t 
> size);
>  
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 3fe714da7f5a..647f825c7b5f 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -23,6 +23,7 @@ struct uid_gid_map {/* 64 bytes -- 1 cache line */
>  };
>  
>  #define USERNS_SETGROUPS_ALLOWED 1UL
> +#define USERNS_CONTROLLED 2UL
>  
>  #define USERNS_INIT_FLAGS USERNS_SETGROUPS_ALLOWED
>  
> @@ -103,6 +104,16 @@ static inline void put_user_ns(struct user_namespace *ns)
>   __put_user_ns(ns);
>  }
>  

Please add a comment explaining that a controlled ns
is one created by a user which did not have CAP_SYS_ADMIN
(or descended from such an ns).

> +static inline bool is_user_ns_controlled(const struct user_namespace *ns)
> +{
> + return ns->flags & USERNS_CONTROLLED;
> +}
> +
> +static inline void mark_user_ns_controlled(struct user_namespace *ns)
> +{
> + ns->flags |= USERNS_CONTROLLED;
> +}
> +
>  struct seq_operations;
>  extern const struct seq_operations proc_uid_seq_operations;
>  extern const struct seq_operations proc_gid_seq_operations;
> @@ -161,6 +172,15 @@ static inline struct ns_common *ns_get_owner(struct 
> ns_common *ns)
>  {
>   return ERR_PTR(-EPERM);
>  }
> +
> +static inline bool is_user_ns_controlled(const struct user_namespace *ns)
> +{
> + return false;
> +}
> +
> +static inline void mark_user_ns_controlled(struct user_namespace *ns)
> +{
> +}
>  #endif
>  
>  #endif /* _LINUX_USER_H */
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 4a859b7d4902..bffe249922de 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -511,6 +511,11 @@ bool ptracer_capable(struct task_struct *tsk, struct 
> user_namespace *ns)
>  }
>  
>  /* Controlled-userns capabilities routines */
> +bool is_capability_controlled(int cap)
> +{
> + return !cap_raised(controlled_userns_caps_whitelist, cap);
> +}
> +
>  #ifdef CONFIG_SYSCTL
>  int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
>void __user *buff, size_t *lenp, loff_t *ppos)
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index c490f1e4313b..600c7dcb9ff7 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -139,6 +139,10 @@ int create_user_ns(struct cred *new)
>   goto fail_keyring;
>  
>   set_cred_user_ns(new, ns);
> + if (!ns_capable(parent_ns, CAP_SYS_ADMIN) ||
> + 

Re: [PATCHv2 2/2] userns: control capabilities of some user namespaces

[Serge E. Hallyn]

Quoting Mahesh Bandewar (mah...@bandewar.net):
> From: Mahesh Bandewar 
> 
> With this new notion of "controlled" user-namespaces, the controlled
> user-namespaces are marked at the time of their creation while the
> capabilities of processes that belong to them are controlled using the
> global mask.
> 
> Init-user-ns is always uncontrolled and a process that has SYS_ADMIN
> that belongs to uncontrolled user-ns can create another (child) user-
> namespace that is uncontrolled. Any other process (that either does
> not have SYS_ADMIN or belongs to a controlled user-ns) can only
> create a user-ns that is controlled.
> 
> global-capability-whitelist (controlled_userns_caps_whitelist) is used
> at the capability check-time and keeps the semantics for the processes
> that belong to uncontrolled user-ns as it is. Processes that belong to
> controlled user-ns however are subjected to different checks-
> 
>(a) if the capability in question is controlled and process belongs
>to controlled user-ns, then it's always denied.
>(b) if the capability in question is NOT controlled then fall back
>to the traditional check.
> 
> Signed-off-by: Mahesh Bandewar 

Acked-by: Serge Hallyn 

Although a few comment addition requests below:

> ---
> v2:
>   Don't recalculate user-ns flags for every setns() call.
> v1:
>   Initial submission.
> 
>  include/linux/capability.h |  1 +
>  include/linux/user_namespace.h | 20 
>  kernel/capability.c|  5 +
>  kernel/user_namespace.c|  4 
>  security/commoncap.c   |  8 
>  5 files changed, 38 insertions(+)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 7d79a4689625..a1fd9e460379 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -251,6 +251,7 @@ extern bool ptracer_capable(struct task_struct *tsk, 
> struct user_namespace *ns);
>  extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct 
> cpu_vfs_cap_data *cpu_caps);
>  int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
>void __user *buff, size_t *lenp, loff_t *ppos);

Here and at the definition below, please add a comment explaining
that a controlled cap is defined as not being in the sysctl.

> +bool is_capability_controlled(int cap);
>  
>  extern int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t 
> size);
>  
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 3fe714da7f5a..647f825c7b5f 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -23,6 +23,7 @@ struct uid_gid_map {/* 64 bytes -- 1 cache line */
>  };
>  
>  #define USERNS_SETGROUPS_ALLOWED 1UL
> +#define USERNS_CONTROLLED 2UL
>  
>  #define USERNS_INIT_FLAGS USERNS_SETGROUPS_ALLOWED
>  
> @@ -103,6 +104,16 @@ static inline void put_user_ns(struct user_namespace *ns)
>   __put_user_ns(ns);
>  }
>  

Please add a comment explaining that a controlled ns
is one created by a user which did not have CAP_SYS_ADMIN
(or descended from such an ns).

> +static inline bool is_user_ns_controlled(const struct user_namespace *ns)
> +{
> + return ns->flags & USERNS_CONTROLLED;
> +}
> +
> +static inline void mark_user_ns_controlled(struct user_namespace *ns)
> +{
> + ns->flags |= USERNS_CONTROLLED;
> +}
> +
>  struct seq_operations;
>  extern const struct seq_operations proc_uid_seq_operations;
>  extern const struct seq_operations proc_gid_seq_operations;
> @@ -161,6 +172,15 @@ static inline struct ns_common *ns_get_owner(struct 
> ns_common *ns)
>  {
>   return ERR_PTR(-EPERM);
>  }
> +
> +static inline bool is_user_ns_controlled(const struct user_namespace *ns)
> +{
> + return false;
> +}
> +
> +static inline void mark_user_ns_controlled(struct user_namespace *ns)
> +{
> +}
>  #endif
>  
>  #endif /* _LINUX_USER_H */
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 4a859b7d4902..bffe249922de 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -511,6 +511,11 @@ bool ptracer_capable(struct task_struct *tsk, struct 
> user_namespace *ns)
>  }
>  
>  /* Controlled-userns capabilities routines */
> +bool is_capability_controlled(int cap)
> +{
> + return !cap_raised(controlled_userns_caps_whitelist, cap);
> +}
> +
>  #ifdef CONFIG_SYSCTL
>  int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
>void __user *buff, size_t *lenp, loff_t *ppos)
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index c490f1e4313b..600c7dcb9ff7 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -139,6 +139,10 @@ int create_user_ns(struct cred *new)
>   goto fail_keyring;
>  
>   set_cred_user_ns(new, ns);
> + if (!ns_capable(parent_ns, CAP_SYS_ADMIN) ||
> + is_user_ns_controlled(parent_ns))
> + 

Re: [PATCHv2 1/2] capability: introduce sysctl for controlled user-ns capability whitelist

[Serge E. Hallyn]

Quoting Mahesh Bandewar (mah...@bandewar.net):
> From: Mahesh Bandewar 
> 
> Add a sysctl variable kernel.controlled_userns_caps_whitelist. This
> takes input as capability mask expressed as two comma separated hex
> u32 words. The mask, however, is stored in kernel as kernel_cap_t type.
> 
> Any capabilities that are not part of this mask will be controlled and
> will not be allowed to processes in controlled user-ns.
> 
> Signed-off-by: Mahesh Bandewar 

Acked-by: Serge Hallyn 

> ---
> v2:
>   Rebase
> v1:
>   Initial submission
> 
>  Documentation/sysctl/kernel.txt | 21 ++
>  include/linux/capability.h  |  3 +++
>  kernel/capability.c | 47 
> +
>  kernel/sysctl.c |  5 +
>  4 files changed, 76 insertions(+)
> 
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 694968c7523c..a1d39dbae847 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -25,6 +25,7 @@ show up in /proc/sys/kernel:
>  - bootloader_version  [ X86 only ]
>  - callhome[ S390 only ]
>  - cap_last_cap
> +- controlled_userns_caps_whitelist
>  - core_pattern
>  - core_pipe_limit
>  - core_uses_pid
> @@ -187,6 +188,26 @@ CAP_LAST_CAP from the kernel.
>  
>  ==
>  
> +controlled_userns_caps_whitelist
> +
> +Capability mask that is whitelisted for "controlled" user namespaces.
> +Any capability that is missing from this mask will not be allowed to
> +any process that is attached to a controlled-userns. e.g. if CAP_NET_RAW
> +is not part of this mask, then processes running inside any controlled
> +userns's will not be allowed to perform action that needs CAP_NET_RAW
> +capability. However, processes that are attached to a parent user-ns
> +hierarchy that is *not* controlled and has CAP_NET_RAW can continue
> +performing those actions. User-namespaces are marked "controlled" at
> +the time of their creation based on the capabilities of the creator.
> +A process that does not have CAP_SYS_ADMIN will create user-namespaces
> +that are controlled.
> +
> +The value is expressed as two comma separated hex words (u32). This
> +sysctl is avaialble in init-ns and users with CAP_SYS_ADMIN in init-ns
> +are allowed to make changes.
> +
> +==
> +
>  core_pattern:
>  
>  core_pattern is used to specify a core dumpfile pattern name.
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index f640dcbc880c..7d79a4689625 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -14,6 +14,7 @@
>  #define _LINUX_CAPABILITY_H
>  
>  #include 
> +#include 
>  
>  
>  #define _KERNEL_CAPABILITY_VERSION _LINUX_CAPABILITY_VERSION_3
> @@ -248,6 +249,8 @@ extern bool ptracer_capable(struct task_struct *tsk, 
> struct user_namespace *ns);
>  
>  /* audit system wants to get cap info from files as well */
>  extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct 
> cpu_vfs_cap_data *cpu_caps);
> +int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
> +  void __user *buff, size_t *lenp, loff_t *ppos);
>  
>  extern int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t 
> size);
>  
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 1e1c0236f55b..4a859b7d4902 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -29,6 +29,8 @@ EXPORT_SYMBOL(__cap_empty_set);
>  
>  int file_caps_enabled = 1;
>  
> +kernel_cap_t controlled_userns_caps_whitelist = CAP_FULL_SET;
> +
>  static int __init file_caps_disable(char *str)
>  {
>   file_caps_enabled = 0;
> @@ -507,3 +509,48 @@ bool ptracer_capable(struct task_struct *tsk, struct 
> user_namespace *ns)
>   rcu_read_unlock();
>   return (ret == 0);
>  }
> +
> +/* Controlled-userns capabilities routines */
> +#ifdef CONFIG_SYSCTL
> +int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
> +  void __user *buff, size_t *lenp, loff_t *ppos)
> +{
> + DECLARE_BITMAP(caps_bitmap, CAP_LAST_CAP);
> + struct ctl_table caps_table;
> + char tbuf[NAME_MAX];
> + int ret;
> +
> + ret = bitmap_from_u32array(caps_bitmap, CAP_LAST_CAP,
> +controlled_userns_caps_whitelist.cap,
> +_KERNEL_CAPABILITY_U32S);
> + if (ret != CAP_LAST_CAP)
> + return -1;
> +
> + scnprintf(tbuf, NAME_MAX, "%*pb", CAP_LAST_CAP, caps_bitmap);
> +
> + caps_table.data = tbuf;
> + caps_table.maxlen = NAME_MAX;
> + caps_table.mode = table->mode;
> + ret = proc_dostring(_table, write, buff, lenp, ppos);
> + if (ret)
> + return ret;
> + if (write) {
> + kernel_cap_t tmp;
> +
> +

Re: [PATCHv2 1/2] capability: introduce sysctl for controlled user-ns capability whitelist

[Serge E. Hallyn]

Quoting Mahesh Bandewar (mah...@bandewar.net):
> From: Mahesh Bandewar 
> 
> Add a sysctl variable kernel.controlled_userns_caps_whitelist. This
> takes input as capability mask expressed as two comma separated hex
> u32 words. The mask, however, is stored in kernel as kernel_cap_t type.
> 
> Any capabilities that are not part of this mask will be controlled and
> will not be allowed to processes in controlled user-ns.
> 
> Signed-off-by: Mahesh Bandewar 

Acked-by: Serge Hallyn 

> ---
> v2:
>   Rebase
> v1:
>   Initial submission
> 
>  Documentation/sysctl/kernel.txt | 21 ++
>  include/linux/capability.h  |  3 +++
>  kernel/capability.c | 47 
> +
>  kernel/sysctl.c |  5 +
>  4 files changed, 76 insertions(+)
> 
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 694968c7523c..a1d39dbae847 100644
> --- a/Documentation/sysctl/kernel.txt
> +++ b/Documentation/sysctl/kernel.txt
> @@ -25,6 +25,7 @@ show up in /proc/sys/kernel:
>  - bootloader_version  [ X86 only ]
>  - callhome[ S390 only ]
>  - cap_last_cap
> +- controlled_userns_caps_whitelist
>  - core_pattern
>  - core_pipe_limit
>  - core_uses_pid
> @@ -187,6 +188,26 @@ CAP_LAST_CAP from the kernel.
>  
>  ==
>  
> +controlled_userns_caps_whitelist
> +
> +Capability mask that is whitelisted for "controlled" user namespaces.
> +Any capability that is missing from this mask will not be allowed to
> +any process that is attached to a controlled-userns. e.g. if CAP_NET_RAW
> +is not part of this mask, then processes running inside any controlled
> +userns's will not be allowed to perform action that needs CAP_NET_RAW
> +capability. However, processes that are attached to a parent user-ns
> +hierarchy that is *not* controlled and has CAP_NET_RAW can continue
> +performing those actions. User-namespaces are marked "controlled" at
> +the time of their creation based on the capabilities of the creator.
> +A process that does not have CAP_SYS_ADMIN will create user-namespaces
> +that are controlled.
> +
> +The value is expressed as two comma separated hex words (u32). This
> +sysctl is avaialble in init-ns and users with CAP_SYS_ADMIN in init-ns
> +are allowed to make changes.
> +
> +==
> +
>  core_pattern:
>  
>  core_pattern is used to specify a core dumpfile pattern name.
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index f640dcbc880c..7d79a4689625 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -14,6 +14,7 @@
>  #define _LINUX_CAPABILITY_H
>  
>  #include 
> +#include 
>  
>  
>  #define _KERNEL_CAPABILITY_VERSION _LINUX_CAPABILITY_VERSION_3
> @@ -248,6 +249,8 @@ extern bool ptracer_capable(struct task_struct *tsk, 
> struct user_namespace *ns);
>  
>  /* audit system wants to get cap info from files as well */
>  extern int get_vfs_caps_from_disk(const struct dentry *dentry, struct 
> cpu_vfs_cap_data *cpu_caps);
> +int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
> +  void __user *buff, size_t *lenp, loff_t *ppos);
>  
>  extern int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t 
> size);
>  
> diff --git a/kernel/capability.c b/kernel/capability.c
> index 1e1c0236f55b..4a859b7d4902 100644
> --- a/kernel/capability.c
> +++ b/kernel/capability.c
> @@ -29,6 +29,8 @@ EXPORT_SYMBOL(__cap_empty_set);
>  
>  int file_caps_enabled = 1;
>  
> +kernel_cap_t controlled_userns_caps_whitelist = CAP_FULL_SET;
> +
>  static int __init file_caps_disable(char *str)
>  {
>   file_caps_enabled = 0;
> @@ -507,3 +509,48 @@ bool ptracer_capable(struct task_struct *tsk, struct 
> user_namespace *ns)
>   rcu_read_unlock();
>   return (ret == 0);
>  }
> +
> +/* Controlled-userns capabilities routines */
> +#ifdef CONFIG_SYSCTL
> +int proc_douserns_caps_whitelist(struct ctl_table *table, int write,
> +  void __user *buff, size_t *lenp, loff_t *ppos)
> +{
> + DECLARE_BITMAP(caps_bitmap, CAP_LAST_CAP);
> + struct ctl_table caps_table;
> + char tbuf[NAME_MAX];
> + int ret;
> +
> + ret = bitmap_from_u32array(caps_bitmap, CAP_LAST_CAP,
> +controlled_userns_caps_whitelist.cap,
> +_KERNEL_CAPABILITY_U32S);
> + if (ret != CAP_LAST_CAP)
> + return -1;
> +
> + scnprintf(tbuf, NAME_MAX, "%*pb", CAP_LAST_CAP, caps_bitmap);
> +
> + caps_table.data = tbuf;
> + caps_table.maxlen = NAME_MAX;
> + caps_table.mode = table->mode;
> + ret = proc_dostring(_table, write, buff, lenp, ppos);
> + if (ret)
> + return ret;
> + if (write) {
> + kernel_cap_t tmp;
> +
> + if (!capable(CAP_SYS_ADMIN))
> + 

Re: perf test LLVM & clang 6 failing

[Yonghong Song]

On 11/24/17 11:09 AM, Arnaldo Carvalho de Melo wrote:

Em Fri, Nov 24, 2017 at 04:16:52PM +0100, Daniel Borkmann escreveu:

[ +Yonghong ]

On 11/24/2017 03:47 PM, Arnaldo Carvalho de Melo wrote:

FYI, just noticed, recently updated clang to version 6, from its
upstream git repo.


Do you recall what was your LLVM version prior to this where it was
working fine? (Wild guess from below would be the BPF inline asm


IIRC it was 4.0


I tried the following example (the almost the same one as the failed
compilation one before except a few SEC(...) marking which should not
impact the result):

$ cat bpf_prog.c
#include 
#include 

int bpf_func__vfs_llseek(void *ctx)
{
return 0;
}
$ cat run.sh
KERNEL_INC_OPTIONS="-nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include 
-I/home/yhs/work/net-next/arch/x86/include 
-I/home/yhs/work/linux-bld/arch/x86/include 
-I/home/yhs/work/net-next/include -I/home/yhs/work/linux-bld/include 
-I/home/yhs/work/net-next/arch/x86/include/uapi 
-I/home/yhs/work/linux-bld/arch/x86/include/generated/uapi 
-I/home/yhs/work/net-next/include/uapi 
-I/home/yhs/work/linux-bld/include/generated/uapi -include 
/home/yhs/work/net-next/include/linux/kconfig.h"


clang -D__KERNEL__ -D__NR_CPUS__=4 -xc \
  $KERNEL_INC_OPTIONS \
  -Wno-unused-value -Wno-pointer-sign \
  -c bpf_prog.c -O2 -target bpf
$

You can actually get KERNEL_INC_OPTIONS by compiling kernel samples/bpf.

This program failed at both llvm 4.0 and latest trunk (6.0).

The same symptom:
In file included from bpf_prog.c:2:
In file included from 
/home/yhs/work/net-next/arch/x86/include/uapi/asm/ptrace.h:5:

In file included from /home/yhs/work/net-next/include/linux/compiler.h:237:
In file included from 
/home/yhs/work/net-next/arch/x86/include/asm/barrier.h:5:
In file included from 
/home/yhs/work/net-next/arch/x86/include/asm/alternative.h:10:
/home/yhs/work/net-next/arch/x86/include/asm/asm.h:145:50: error: 
unknown register name 'esp' in asm

register unsigned long current_stack_pointer asm(_ASM_SP);
 ^
/home/yhs/work/net-next/arch/x86/include/asm/asm.h:44:18: note: expanded 
from macro '_ASM_SP'

#define _ASM_SP __ASM_REG(sp)

I suggest that you use the same compilation process as
samples/bpf and tools/testing/selftests/bpf:

$ cat run1.sh
KERNEL_INC_OPTIONS="-nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include 
-I/home/yhs/work/net-next/arch/x86/include 
-I/home/yhs/work/linux-bld/arch/x86/include 
-I/home/yhs/work/net-next/include -I/home/yhs/work/linux-bld/include 
-I/home/yhs/work/net-next/arch/x86/include/uapi 
-I/home/yhs/work/linux-bld/arch/x86/include/generated/uapi 
-I/home/yhs/work/net-next/include/uapi 
-I/home/yhs/work/linux-bld/include/generated/uapi -include 
/home/yhs/work/net-next/include/linux/kconfig.h"


clang -D__KERNEL__ -D__NR_CPUS__=4 -xc \
  $KERNEL_INC_OPTIONS \
  -Wno-unused-value -Wno-pointer-sign \
  -c bpf_prog.c -O2 -emit-llvm -S
llc -filetype=obj -march=bpf bpf_prog.ll
$

This is especially true if you are doing tracing there ptrace.h may be 
included in your header files.


The "clang" compiler will be able to handle host inline asm properly and
filter out functions which uses host inline asm's. If any file-level
inline asms are still left (e.g., arm64), the llvm 6.0 should be able
to handle this but not llvm 4.0.




support that was added recently to LLVM (2865ab6996) vs asm() used
in headers included in the stdin header causing trouble due to arch
mixup?)


root@jouet ~]# perf test -v LLVM
37: LLVM search and compile   :
37.1: Basic BPF llvm compile  :
--- start ---
test child forked, pid 5255
Kernel build dir is set to /lib/modules/4.14.0+/build
set env: KBUILD_DIR=/lib/modules/4.14.0+/build
unset env: KBUILD_OPTS
include option is set to  -nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/7/include 
-I/home/acme/git/linux/arch/x86/include -I./arch/x86/include/generated  
-I/home/acme/git/linux/include -I./include 
-I/home/acme/git/linux/arch/x86/include/uapi 
-I./arch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi 
-I./include/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h
set env: NR_CPUS=4
set env: LINUX_VERSION_CODE=0x40e00
set env: CLANG_EXEC=/usr/local/bin/clang
set env: CLANG_OPTIONS=-xc
set env: KERNEL_INC_OPTIONS= -nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/7/include 
-I/home/acme/git/linux/arch/x86/include -I./arch/x86/include/generated  
-I/home/acme/git/linux/include -I./include 
-I/home/acme/git/linux/arch/x86/include/uapi 
-I./arch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi 
-I./include/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h
set env: WORKING_DIR=/lib/modules/4.14.0+/build
set env: CLANG_SOURCE=-
llvm compiling command template: echo '/*
  * bpf-script-example.c
  * Test basic LLVM building
  */
#ifndef 

Re: perf test LLVM & clang 6 failing

[Yonghong Song]

On 11/24/17 11:09 AM, Arnaldo Carvalho de Melo wrote:

Em Fri, Nov 24, 2017 at 04:16:52PM +0100, Daniel Borkmann escreveu:

[ +Yonghong ]

On 11/24/2017 03:47 PM, Arnaldo Carvalho de Melo wrote:

FYI, just noticed, recently updated clang to version 6, from its
upstream git repo.


Do you recall what was your LLVM version prior to this where it was
working fine? (Wild guess from below would be the BPF inline asm


IIRC it was 4.0


I tried the following example (the almost the same one as the failed
compilation one before except a few SEC(...) marking which should not
impact the result):

$ cat bpf_prog.c
#include 
#include 

int bpf_func__vfs_llseek(void *ctx)
{
return 0;
}
$ cat run.sh
KERNEL_INC_OPTIONS="-nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include 
-I/home/yhs/work/net-next/arch/x86/include 
-I/home/yhs/work/linux-bld/arch/x86/include 
-I/home/yhs/work/net-next/include -I/home/yhs/work/linux-bld/include 
-I/home/yhs/work/net-next/arch/x86/include/uapi 
-I/home/yhs/work/linux-bld/arch/x86/include/generated/uapi 
-I/home/yhs/work/net-next/include/uapi 
-I/home/yhs/work/linux-bld/include/generated/uapi -include 
/home/yhs/work/net-next/include/linux/kconfig.h"


clang -D__KERNEL__ -D__NR_CPUS__=4 -xc \
  $KERNEL_INC_OPTIONS \
  -Wno-unused-value -Wno-pointer-sign \
  -c bpf_prog.c -O2 -target bpf
$

You can actually get KERNEL_INC_OPTIONS by compiling kernel samples/bpf.

This program failed at both llvm 4.0 and latest trunk (6.0).

The same symptom:
In file included from bpf_prog.c:2:
In file included from 
/home/yhs/work/net-next/arch/x86/include/uapi/asm/ptrace.h:5:

In file included from /home/yhs/work/net-next/include/linux/compiler.h:237:
In file included from 
/home/yhs/work/net-next/arch/x86/include/asm/barrier.h:5:
In file included from 
/home/yhs/work/net-next/arch/x86/include/asm/alternative.h:10:
/home/yhs/work/net-next/arch/x86/include/asm/asm.h:145:50: error: 
unknown register name 'esp' in asm

register unsigned long current_stack_pointer asm(_ASM_SP);
 ^
/home/yhs/work/net-next/arch/x86/include/asm/asm.h:44:18: note: expanded 
from macro '_ASM_SP'

#define _ASM_SP __ASM_REG(sp)

I suggest that you use the same compilation process as
samples/bpf and tools/testing/selftests/bpf:

$ cat run1.sh
KERNEL_INC_OPTIONS="-nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include 
-I/home/yhs/work/net-next/arch/x86/include 
-I/home/yhs/work/linux-bld/arch/x86/include 
-I/home/yhs/work/net-next/include -I/home/yhs/work/linux-bld/include 
-I/home/yhs/work/net-next/arch/x86/include/uapi 
-I/home/yhs/work/linux-bld/arch/x86/include/generated/uapi 
-I/home/yhs/work/net-next/include/uapi 
-I/home/yhs/work/linux-bld/include/generated/uapi -include 
/home/yhs/work/net-next/include/linux/kconfig.h"


clang -D__KERNEL__ -D__NR_CPUS__=4 -xc \
  $KERNEL_INC_OPTIONS \
  -Wno-unused-value -Wno-pointer-sign \
  -c bpf_prog.c -O2 -emit-llvm -S
llc -filetype=obj -march=bpf bpf_prog.ll
$

This is especially true if you are doing tracing there ptrace.h may be 
included in your header files.


The "clang" compiler will be able to handle host inline asm properly and
filter out functions which uses host inline asm's. If any file-level
inline asms are still left (e.g., arm64), the llvm 6.0 should be able
to handle this but not llvm 4.0.




support that was added recently to LLVM (2865ab6996) vs asm() used
in headers included in the stdin header causing trouble due to arch
mixup?)


root@jouet ~]# perf test -v LLVM
37: LLVM search and compile   :
37.1: Basic BPF llvm compile  :
--- start ---
test child forked, pid 5255
Kernel build dir is set to /lib/modules/4.14.0+/build
set env: KBUILD_DIR=/lib/modules/4.14.0+/build
unset env: KBUILD_OPTS
include option is set to  -nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/7/include 
-I/home/acme/git/linux/arch/x86/include -I./arch/x86/include/generated  
-I/home/acme/git/linux/include -I./include 
-I/home/acme/git/linux/arch/x86/include/uapi 
-I./arch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi 
-I./include/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h
set env: NR_CPUS=4
set env: LINUX_VERSION_CODE=0x40e00
set env: CLANG_EXEC=/usr/local/bin/clang
set env: CLANG_OPTIONS=-xc
set env: KERNEL_INC_OPTIONS= -nostdinc -isystem 
/usr/lib/gcc/x86_64-redhat-linux/7/include 
-I/home/acme/git/linux/arch/x86/include -I./arch/x86/include/generated  
-I/home/acme/git/linux/include -I./include 
-I/home/acme/git/linux/arch/x86/include/uapi 
-I./arch/x86/include/generated/uapi -I/home/acme/git/linux/include/uapi 
-I./include/generated/uapi -include /home/acme/git/linux/include/linux/kconfig.h
set env: WORKING_DIR=/lib/modules/4.14.0+/build
set env: CLANG_SOURCE=-
llvm compiling command template: echo '/*
  * bpf-script-example.c
  * Test basic LLVM building
  */
#ifndef 

Re: [PATCH v2] checkpatch: Add a warning for log messages that don't end in a new line

[Joe Perches]

On Sun, 2017-11-26 at 06:51 +0100, Julia Lawall wrote:
> 
> On Sat, 25 Nov 2017, Logan Gunthorpe wrote:
> 
> > Check for lines with a log function using a relatively strict regular
> > expression catching only printk, dev_* and pr_* functions. Once
> > one is found, accumulate further lines for any functions that
> > are split over multiple lines.
> 
> I don't understand at all the second sentence.  Are you staying with the
> same call, or moving on to other calls?  Also, it would be the call that
> is split over multiple lines, not the function split over multiple lines.
> 
> I think this would have been much easier with Cocccinelle where the code
> is parsed and the control-flow graph is available to see whether there is
> a pr_cont afterwards.  But if it works, then it is surely good enough.

It doesn't really work.

Many of the messages aren't missing newlines.

I only looked a the first few dozen instances, but many of
them aren't really missing newlines, but are now missing a
KERN_CONT annotation.

This is because Linus changed how printk worked awhile
ago in

commit 4bcc595ccd80decb4245096e3d1258989c50ed41
Author: Linus Torvalds 
Date:   Sat Oct 8 20:32:40 2016 -0700

printk: reinstate KERN_CONT for printing continuation lines

Most of that commit message is BS, but the net effect is
that now printks must have a KERN_ marker or a
newline is inserted before the format.

Also, this patch logic will be very confused by patch
blocks and not files.

Re: [PATCH v2] checkpatch: Add a warning for log messages that don't end in a new line

[Joe Perches]

On Sun, 2017-11-26 at 06:51 +0100, Julia Lawall wrote:
> 
> On Sat, 25 Nov 2017, Logan Gunthorpe wrote:
> 
> > Check for lines with a log function using a relatively strict regular
> > expression catching only printk, dev_* and pr_* functions. Once
> > one is found, accumulate further lines for any functions that
> > are split over multiple lines.
> 
> I don't understand at all the second sentence.  Are you staying with the
> same call, or moving on to other calls?  Also, it would be the call that
> is split over multiple lines, not the function split over multiple lines.
> 
> I think this would have been much easier with Cocccinelle where the code
> is parsed and the control-flow graph is available to see whether there is
> a pr_cont afterwards.  But if it works, then it is surely good enough.

It doesn't really work.

Many of the messages aren't missing newlines.

I only looked a the first few dozen instances, but many of
them aren't really missing newlines, but are now missing a
KERN_CONT annotation.

This is because Linus changed how printk worked awhile
ago in

commit 4bcc595ccd80decb4245096e3d1258989c50ed41
Author: Linus Torvalds 
Date:   Sat Oct 8 20:32:40 2016 -0700

printk: reinstate KERN_CONT for printing continuation lines

Most of that commit message is BS, but the net effect is
that now printks must have a KERN_ marker or a
newline is inserted before the format.

Also, this patch logic will be very confused by patch
blocks and not files.

Re: [PATCH v2] checkpatch: Add a warning for log messages that don't end in a new line

[Julia Lawall]

On Sat, 25 Nov 2017, Logan Gunthorpe wrote:

> Check for lines with a log function using a relatively strict regular
> expression catching only printk, dev_* and pr_* functions. Once
> one is found, accumulate further lines for any functions that
> are split over multiple lines.

I don't understand at all the second sentence.  Are you staying with the
same call, or moving on to other calls?  Also, it would be the call that
is split over multiple lines, not the function split over multiple lines.

I think this would have been much easier with Cocccinelle where the code
is parsed and the control-flow graph is available to see whether there is
a pr_cont afterwards.  But if it works, then it is surely good enough.

julia

> Stop accumulating the log function line
> when we find a ';' or match the full format string.
>
> A get_log_fmt() function is used to find either the first or
> second argument (depending on the log function matched) as the
> format string. If the format argument ends in '\n"', then we conclude
> that the logging call is correct. If the line ends in '%s"', then we
> only print a CHK warning as the developer *may* have the newline in an
> argument string. Otherwise we print a warning that the line is
> likely missing a new line.
>
> Because we are trying to match multiple lines, we must also match
> lines that are part of the context of the patch. In order to ensure
> we don't print a message on log lines that are only part of context,
> we track that at least one line was added using the $log_func_changed
> flag.
>
> To handle continuations, during pre-processing we search for all
> pr_cont and KERN_CONT instances and create a hash set of the line
> numbers of all preceding log function calls. The hash set is then used
> to mask out any calls that are likely followed by continuations. This
> should handle many cases, however a small number of false positives are
> likely going to occur in cases where a patch's context does not cover
> the following KERN_CONT. This is acceptable, in my opinion, seeing
> KERN_CONT is already discouraged and will create it's own warnings that
> the developer has to reason about.
>
> I've created a small test suite to test this change which can be found
> on github[1]. I've also run this on the kernel source and found more
> than 6000 violations. I reviewed a random sampling of these for false
> positives and have not found any.
>
> Thanks to Joe for describing how to properly test a change such as this.
>
> [1] https://github.com/lsgunth/checkpatch-tests
>
> Signed-off-by: Logan Gunthorpe 
> Cc: Andy Whitcroft 
> Cc: Joe Perches 
> ---
>
> Here's my second attempt at this. As described in the commit log,
> I've made it a bit more sophisticated and fixed a number of issues with
> how the patch format is processed. The testing has also improved
> significantly having run it on the entire kernel source as well as
> a suite of test cases. The patch isn't entirely perfect (per the note
> about false positives) but I feel it's close enough that its benefits
> outweighs this. (As it only annoys developers who are working with a
> discouraged feature.)
>
> As mentioned in the commit log, I've found more than 6000 instances
> that look like this error. I've seen a few cases that appear to have
> intended to use KERN_CONT but didn't and are thus caught by this.
> If there are any heroic kernel janitors that are interested in tackling
> these issues, the list can be found here:
>
> https://github.com/lsgunth/checkpatch-tests/blob/master/results/v4.14
>
>  scripts/checkpatch.pl | 80 
> +++
>  1 file changed, 80 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8b80bac055e4..cdd767af6566 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -460,6 +460,34 @@ our $logFunctions = qr{(?x:
>   seq_vprintf|seq_printf|seq_puts
>  )};
>
> +
> +our $logNewLineFirstArg = qr{(?x:
> + printk(?:_ratelimited|_once|_deferred_once|_deferred|)|
> + 
> pr_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)
> +)};
> +
> +our $logNewLineSecondArg = qr{(?x:
> + 
> dev_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|WARN)(?:_ratelimited|_once|)
> +)};
> +
> +our $logNewLineFunctions = qr{(?x:
> +$logNewLineFirstArg |
> +$logNewLineSecondArg
> +)};
> +
> +sub get_log_fmt {
> + my ($line, $rawline) = @_;
> +
> + if ($line =~ m/\b$logNewLineFirstArg\(([^,)]+)[,)]/g) {
> + return substr($rawline, $-[1], $+[1] - $-[1]);
> + } elsif($line =~ m/\b$logNewLineSecondArg\([^,]+,([^,)]+)[,)]/g) {
> + return substr($rawline, $-[1], $+[1] - $-[1]);
> + } else {
> + return "";
> + }
> +}
> +
> +
>  our $signature_tags = qr{(?xi:
>   Signed-off-by:|
>   Acked-by:|
> @@ -2224,6 

Re: [PATCH v2] checkpatch: Add a warning for log messages that don't end in a new line

[Julia Lawall]

On Sat, 25 Nov 2017, Logan Gunthorpe wrote:

> Check for lines with a log function using a relatively strict regular
> expression catching only printk, dev_* and pr_* functions. Once
> one is found, accumulate further lines for any functions that
> are split over multiple lines.

I don't understand at all the second sentence.  Are you staying with the
same call, or moving on to other calls?  Also, it would be the call that
is split over multiple lines, not the function split over multiple lines.

I think this would have been much easier with Cocccinelle where the code
is parsed and the control-flow graph is available to see whether there is
a pr_cont afterwards.  But if it works, then it is surely good enough.

julia

> Stop accumulating the log function line
> when we find a ';' or match the full format string.
>
> A get_log_fmt() function is used to find either the first or
> second argument (depending on the log function matched) as the
> format string. If the format argument ends in '\n"', then we conclude
> that the logging call is correct. If the line ends in '%s"', then we
> only print a CHK warning as the developer *may* have the newline in an
> argument string. Otherwise we print a warning that the line is
> likely missing a new line.
>
> Because we are trying to match multiple lines, we must also match
> lines that are part of the context of the patch. In order to ensure
> we don't print a message on log lines that are only part of context,
> we track that at least one line was added using the $log_func_changed
> flag.
>
> To handle continuations, during pre-processing we search for all
> pr_cont and KERN_CONT instances and create a hash set of the line
> numbers of all preceding log function calls. The hash set is then used
> to mask out any calls that are likely followed by continuations. This
> should handle many cases, however a small number of false positives are
> likely going to occur in cases where a patch's context does not cover
> the following KERN_CONT. This is acceptable, in my opinion, seeing
> KERN_CONT is already discouraged and will create it's own warnings that
> the developer has to reason about.
>
> I've created a small test suite to test this change which can be found
> on github[1]. I've also run this on the kernel source and found more
> than 6000 violations. I reviewed a random sampling of these for false
> positives and have not found any.
>
> Thanks to Joe for describing how to properly test a change such as this.
>
> [1] https://github.com/lsgunth/checkpatch-tests
>
> Signed-off-by: Logan Gunthorpe 
> Cc: Andy Whitcroft 
> Cc: Joe Perches 
> ---
>
> Here's my second attempt at this. As described in the commit log,
> I've made it a bit more sophisticated and fixed a number of issues with
> how the patch format is processed. The testing has also improved
> significantly having run it on the entire kernel source as well as
> a suite of test cases. The patch isn't entirely perfect (per the note
> about false positives) but I feel it's close enough that its benefits
> outweighs this. (As it only annoys developers who are working with a
> discouraged feature.)
>
> As mentioned in the commit log, I've found more than 6000 instances
> that look like this error. I've seen a few cases that appear to have
> intended to use KERN_CONT but didn't and are thus caught by this.
> If there are any heroic kernel janitors that are interested in tackling
> these issues, the list can be found here:
>
> https://github.com/lsgunth/checkpatch-tests/blob/master/results/v4.14
>
>  scripts/checkpatch.pl | 80 
> +++
>  1 file changed, 80 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 8b80bac055e4..cdd767af6566 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -460,6 +460,34 @@ our $logFunctions = qr{(?x:
>   seq_vprintf|seq_printf|seq_puts
>  )};
>
> +
> +our $logNewLineFirstArg = qr{(?x:
> + printk(?:_ratelimited|_once|_deferred_once|_deferred|)|
> + 
> pr_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)
> +)};
> +
> +our $logNewLineSecondArg = qr{(?x:
> + 
> dev_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|WARN)(?:_ratelimited|_once|)
> +)};
> +
> +our $logNewLineFunctions = qr{(?x:
> +$logNewLineFirstArg |
> +$logNewLineSecondArg
> +)};
> +
> +sub get_log_fmt {
> + my ($line, $rawline) = @_;
> +
> + if ($line =~ m/\b$logNewLineFirstArg\(([^,)]+)[,)]/g) {
> + return substr($rawline, $-[1], $+[1] - $-[1]);
> + } elsif($line =~ m/\b$logNewLineSecondArg\([^,]+,([^,)]+)[,)]/g) {
> + return substr($rawline, $-[1], $+[1] - $-[1]);
> + } else {
> + return "";
> + }
> +}
> +
> +
>  our $signature_tags = qr{(?xi:
>   Signed-off-by:|
>   Acked-by:|
> @@ -2224,6 +2252,13 @@ sub process {
>
>   my 

Sparse warnings from sched.h

[Jakub Kicinski]

Hi!

Did these:

./include/linux/sched.h:476:62: error: dubious one-bit signed bitfield
./include/linux/sched.h:477:62: error: dubious one-bit signed bitfield
./include/linux/sched.h:478:62: error: dubious one-bit signed bitfield
./include/linux/sched.h:479:62: error: dubious one-bit signed bitfield

got fixed?  I saw there were patches posted, but nothing have reached
Linus's tree, yet.

Sparse warnings from sched.h

[Jakub Kicinski]

Hi!

Did these:

./include/linux/sched.h:476:62: error: dubious one-bit signed bitfield
./include/linux/sched.h:477:62: error: dubious one-bit signed bitfield
./include/linux/sched.h:478:62: error: dubious one-bit signed bitfield
./include/linux/sched.h:479:62: error: dubious one-bit signed bitfield

got fixed?  I saw there were patches posted, but nothing have reached
Linus's tree, yet.

[PATCH v2] checkpatch: Add a warning for log messages that don't end in a new line

[Logan Gunthorpe]

Check for lines with a log function using a relatively strict regular
expression catching only printk, dev_* and pr_* functions. Once
one is found, accumulate further lines for any functions that
are split over multiple lines. Stop accumulating the log function line
when we find a ';' or match the full format string.

A get_log_fmt() function is used to find either the first or
second argument (depending on the log function matched) as the
format string. If the format argument ends in '\n"', then we conclude
that the logging call is correct. If the line ends in '%s"', then we
only print a CHK warning as the developer *may* have the newline in an
argument string. Otherwise we print a warning that the line is
likely missing a new line.

Because we are trying to match multiple lines, we must also match
lines that are part of the context of the patch. In order to ensure
we don't print a message on log lines that are only part of context,
we track that at least one line was added using the $log_func_changed
flag.

To handle continuations, during pre-processing we search for all
pr_cont and KERN_CONT instances and create a hash set of the line
numbers of all preceding log function calls. The hash set is then used
to mask out any calls that are likely followed by continuations. This
should handle many cases, however a small number of false positives are
likely going to occur in cases where a patch's context does not cover
the following KERN_CONT. This is acceptable, in my opinion, seeing
KERN_CONT is already discouraged and will create it's own warnings that
the developer has to reason about.

I've created a small test suite to test this change which can be found
on github[1]. I've also run this on the kernel source and found more
than 6000 violations. I reviewed a random sampling of these for false
positives and have not found any.

Thanks to Joe for describing how to properly test a change such as this.

[1] https://github.com/lsgunth/checkpatch-tests

Signed-off-by: Logan Gunthorpe 
Cc: Andy Whitcroft 
Cc: Joe Perches 
---

Here's my second attempt at this. As described in the commit log,
I've made it a bit more sophisticated and fixed a number of issues with
how the patch format is processed. The testing has also improved
significantly having run it on the entire kernel source as well as
a suite of test cases. The patch isn't entirely perfect (per the note
about false positives) but I feel it's close enough that its benefits
outweighs this. (As it only annoys developers who are working with a
discouraged feature.)

As mentioned in the commit log, I've found more than 6000 instances
that look like this error. I've seen a few cases that appear to have
intended to use KERN_CONT but didn't and are thus caught by this.
If there are any heroic kernel janitors that are interested in tackling
these issues, the list can be found here:

https://github.com/lsgunth/checkpatch-tests/blob/master/results/v4.14

 scripts/checkpatch.pl | 80 +++
 1 file changed, 80 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8b80bac055e4..cdd767af6566 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -460,6 +460,34 @@ our $logFunctions = qr{(?x:
seq_vprintf|seq_printf|seq_puts
 )};

+
+our $logNewLineFirstArg = qr{(?x:
+   printk(?:_ratelimited|_once|_deferred_once|_deferred|)|
+   
pr_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)
+)};
+
+our $logNewLineSecondArg = qr{(?x:
+   
dev_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|WARN)(?:_ratelimited|_once|)
+)};
+
+our $logNewLineFunctions = qr{(?x:
+$logNewLineFirstArg |
+$logNewLineSecondArg
+)};
+
+sub get_log_fmt {
+   my ($line, $rawline) = @_;
+
+   if ($line =~ m/\b$logNewLineFirstArg\(([^,)]+)[,)]/g) {
+   return substr($rawline, $-[1], $+[1] - $-[1]);
+   } elsif($line =~ m/\b$logNewLineSecondArg\([^,]+,([^,)]+)[,)]/g) {
+   return substr($rawline, $-[1], $+[1] - $-[1]);
+   } else {
+   return "";
+   }
+}
+
+
 our $signature_tags = qr{(?xi:
Signed-off-by:|
Acked-by:|
@@ -2224,6 +2252,13 @@ sub process {

my $camelcase_file_seeded = 0;

+   my $in_log_function = 0;
+   my $log_func_changed = 0;
+   my $log_func_line = "";
+   my $log_func_rawline = "";
+   my $log_last_linenr = -1;
+   my %log_func_continued = ();
+
sanitise_line_reset();
my $line;
foreach my $rawline (@rawlines) {
@@ -2287,6 +2322,15 @@ sub process {
# simplify matching -- only bother with positive lines.
$line = sanitise_line($rawline);
}
+
+   if ($log_last_linenr > 0 && $line =~ /(KERN_CONT|pr_cont)/) {
+   

[PATCH v2] checkpatch: Add a warning for log messages that don't end in a new line

[Logan Gunthorpe]

Check for lines with a log function using a relatively strict regular
expression catching only printk, dev_* and pr_* functions. Once
one is found, accumulate further lines for any functions that
are split over multiple lines. Stop accumulating the log function line
when we find a ';' or match the full format string.

A get_log_fmt() function is used to find either the first or
second argument (depending on the log function matched) as the
format string. If the format argument ends in '\n"', then we conclude
that the logging call is correct. If the line ends in '%s"', then we
only print a CHK warning as the developer *may* have the newline in an
argument string. Otherwise we print a warning that the line is
likely missing a new line.

Because we are trying to match multiple lines, we must also match
lines that are part of the context of the patch. In order to ensure
we don't print a message on log lines that are only part of context,
we track that at least one line was added using the $log_func_changed
flag.

To handle continuations, during pre-processing we search for all
pr_cont and KERN_CONT instances and create a hash set of the line
numbers of all preceding log function calls. The hash set is then used
to mask out any calls that are likely followed by continuations. This
should handle many cases, however a small number of false positives are
likely going to occur in cases where a patch's context does not cover
the following KERN_CONT. This is acceptable, in my opinion, seeing
KERN_CONT is already discouraged and will create it's own warnings that
the developer has to reason about.

I've created a small test suite to test this change which can be found
on github[1]. I've also run this on the kernel source and found more
than 6000 violations. I reviewed a random sampling of these for false
positives and have not found any.

Thanks to Joe for describing how to properly test a change such as this.

[1] https://github.com/lsgunth/checkpatch-tests

Signed-off-by: Logan Gunthorpe 
Cc: Andy Whitcroft 
Cc: Joe Perches 
---

Here's my second attempt at this. As described in the commit log,
I've made it a bit more sophisticated and fixed a number of issues with
how the patch format is processed. The testing has also improved
significantly having run it on the entire kernel source as well as
a suite of test cases. The patch isn't entirely perfect (per the note
about false positives) but I feel it's close enough that its benefits
outweighs this. (As it only annoys developers who are working with a
discouraged feature.)

As mentioned in the commit log, I've found more than 6000 instances
that look like this error. I've seen a few cases that appear to have
intended to use KERN_CONT but didn't and are thus caught by this.
If there are any heroic kernel janitors that are interested in tackling
these issues, the list can be found here:

https://github.com/lsgunth/checkpatch-tests/blob/master/results/v4.14

 scripts/checkpatch.pl | 80 +++
 1 file changed, 80 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8b80bac055e4..cdd767af6566 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -460,6 +460,34 @@ our $logFunctions = qr{(?x:
seq_vprintf|seq_printf|seq_puts
 )};

+
+our $logNewLineFirstArg = qr{(?x:
+   printk(?:_ratelimited|_once|_deferred_once|_deferred|)|
+   
pr_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)
+)};
+
+our $logNewLineSecondArg = qr{(?x:
+   
dev_(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|WARN)(?:_ratelimited|_once|)
+)};
+
+our $logNewLineFunctions = qr{(?x:
+$logNewLineFirstArg |
+$logNewLineSecondArg
+)};
+
+sub get_log_fmt {
+   my ($line, $rawline) = @_;
+
+   if ($line =~ m/\b$logNewLineFirstArg\(([^,)]+)[,)]/g) {
+   return substr($rawline, $-[1], $+[1] - $-[1]);
+   } elsif($line =~ m/\b$logNewLineSecondArg\([^,]+,([^,)]+)[,)]/g) {
+   return substr($rawline, $-[1], $+[1] - $-[1]);
+   } else {
+   return "";
+   }
+}
+
+
 our $signature_tags = qr{(?xi:
Signed-off-by:|
Acked-by:|
@@ -2224,6 +2252,13 @@ sub process {

my $camelcase_file_seeded = 0;

+   my $in_log_function = 0;
+   my $log_func_changed = 0;
+   my $log_func_line = "";
+   my $log_func_rawline = "";
+   my $log_last_linenr = -1;
+   my %log_func_continued = ();
+
sanitise_line_reset();
my $line;
foreach my $rawline (@rawlines) {
@@ -2287,6 +2322,15 @@ sub process {
# simplify matching -- only bother with positive lines.
$line = sanitise_line($rawline);
}
+
+   if ($log_last_linenr > 0 && $line =~ /(KERN_CONT|pr_cont)/) {
+   $log_func_continued{$log_last_linenr} = ();
+   }
+
+

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Josh Poimboeuf]

On Sat, Nov 25, 2017 at 10:41:15PM -0600, Josh Poimboeuf wrote:
> On Sat, Nov 25, 2017 at 08:25:12PM -0800, Andy Lutomirski wrote:
> > On Sat, Nov 25, 2017 at 6:40 PM, Josh Poimboeuf  wrote:
> > > On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
> > >> Can you send me whatever config and exact commit hash generated this?
> > >> I can try to figure out why it failed.
> > >
> > > Sorry, I've been traveling.  I just got some time to take a look at
> > > this.  I think there are at least two unwinder issues here:
> > >
> > > - It doesn't deal gracefully with the case where the stack overflows and
> > >   the stack pointer itself isn't on a valid stack but the
> > >   to-be-dereferenced data *is*.
> > >
> > > - The oops dump code doesn't know how to print partial pt_regs, for the
> > >   case where if we get an interrupt/exception in *early* entry code
> > >   before the full pt_regs have been saved.
> > >
> > > (Andy, I'm not quite sure about your patch, and whether it's still
> > > needed after these patches.  I'll need to look at it later when I have
> > > more time.)
> > 
> > I haven't tested yet, but I think my patch is probably still needed.
> > The issue I fixed is that unwind_start() would bail out early if sp
> > was below the stack.  Also:
> 
> Makes sense, maybe both are needed.  Your patch deals with a bad SP at
> the beginning and mine deals with a bad SP in the middle.

I was able to recreate with the config Ingo posted earlier, along with
the following patch:

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index e12168936d3f..693a20d309e3 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -500,7 +500,7 @@
VMLINUX_SYMBOL(__entry_text_end) = .;

 #define IRQENTRY_TEXT  \
-   ALIGN_FUNCTION();   \
+   . = ALIGN(4096);\
VMLINUX_SYMBOL(__irqentry_text_start) = .;  \
*(.irqentry.text)   \
VMLINUX_SYMBOL(__irqentry_text_end) = .;


It looks a *lot* better with mine and your patches applied.  It probably
would have helped Ingo and Thomas figure the problem out a lot sooner:

[1.159016] PANIC: double fault, error_code: 0x0
[1.159583] CPU: 1 PID: 68 Comm: modprobe Not tainted 
4.14.0-01257-g761d390195b6-dirty #19
[1.159583] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.10.2-1.fc26 04/01/2014
[1.159583] task: 880136f6bb00 task.stack: c9984000
[1.159583] RIP: 0010:page_fault+0x11/0x60
[1.159583] RSP: :ff083fc8 EFLAGS: 00010046
[1.159583] RAX: 819d0a87 RBX: 0001 RCX: 819d0a87
[1.159583] RDX: 1000 RSI: 0010 RDI: ff084078
[1.159583] RBP: 0d68 R08: 7f6d6bb24278 R09: 0023
[1.159583] R10: 558e0feca600 R11: 0246 R12: 7f6d6bb203c0
[1.159583] R13: 7f6d6bb1f880 R14: 7793bebc R15: 0100
[1.159583] FS:  7f6d6c39c5c0() GS:88013fc4() 
knlGS:
[1.159583] CS:  0010 DS:  ES:  CR0: 80050033
[1.159583] CR2: ff083fb8 CR3: 000136f78002 CR4: 001606e0
[1.159583] Call Trace:
[1.159583]  
[1.159583]  __do_page_fault+0x4b0/0x4b0
[1.159583]  page_fault+0x2c/0x60
[1.159583] RIP: 0010:do_page_fault+0x0/0x100
[1.159583] RSP: :ff084120 EFLAGS: 00010012
[1.159583] RAX: 819d0a87 RBX: 0001 RCX: 819d0a87
[1.159583] RDX: 1000 RSI: 0010 RDI: ff084128
[1.159583] RBP: 0d68 R08: 7f6d6bb24278 R09: 0023
[1.159583] R10: 558e0feca600 R11: 0246 R12: 7f6d6bb203c0
[1.159583] R13: 7f6d6bb1f880 R14: 7793bebc R15: 0100
[1.159583]  ? native_iret+0x7/0x7
[1.159583]  page_fault+0x2c/0x60
[1.159583] RIP: 0010:apic_timer_interrupt+0x0/0xb0
[1.159583] RSP: :ff0841d8 EFLAGS: 00010046
[1.159583] RAX: 0374 RBX: 558e0feca2c0 RCX: 7f6d6b85aaf0
[1.159583] RDX: 1000 RSI: 558e0feca600 RDI: 
[1.159583] RBP: 0d68 R08: 7f6d6bb24278 R09: 0023
[1.159583] R10: 558e0feca600 R11: 0246 R12: 7f6d6bb203c0
[1.159583] R13: 7f6d6bb1f880 R14: 7793bebc R15: 0100
[1.159583] RIP: 0033:0x7f6d6b85aaf0
[1.159583] RSP: 002b:7793bd68 EFLAGS: 0246 ORIG_RAX: 
0003
[1.159583] RAX: 819d2000 RBX: 7f6d6b85aaf0 RCX: 0010
[1.159583] RDX: 00010046 RSI: ff0841d8 RDI: 
[1.159583] RBP: 

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Josh Poimboeuf]

On Sat, Nov 25, 2017 at 10:41:15PM -0600, Josh Poimboeuf wrote:
> On Sat, Nov 25, 2017 at 08:25:12PM -0800, Andy Lutomirski wrote:
> > On Sat, Nov 25, 2017 at 6:40 PM, Josh Poimboeuf  wrote:
> > > On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
> > >> Can you send me whatever config and exact commit hash generated this?
> > >> I can try to figure out why it failed.
> > >
> > > Sorry, I've been traveling.  I just got some time to take a look at
> > > this.  I think there are at least two unwinder issues here:
> > >
> > > - It doesn't deal gracefully with the case where the stack overflows and
> > >   the stack pointer itself isn't on a valid stack but the
> > >   to-be-dereferenced data *is*.
> > >
> > > - The oops dump code doesn't know how to print partial pt_regs, for the
> > >   case where if we get an interrupt/exception in *early* entry code
> > >   before the full pt_regs have been saved.
> > >
> > > (Andy, I'm not quite sure about your patch, and whether it's still
> > > needed after these patches.  I'll need to look at it later when I have
> > > more time.)
> > 
> > I haven't tested yet, but I think my patch is probably still needed.
> > The issue I fixed is that unwind_start() would bail out early if sp
> > was below the stack.  Also:
> 
> Makes sense, maybe both are needed.  Your patch deals with a bad SP at
> the beginning and mine deals with a bad SP in the middle.

I was able to recreate with the config Ingo posted earlier, along with
the following patch:

diff --git a/include/asm-generic/vmlinux.lds.h 
b/include/asm-generic/vmlinux.lds.h
index e12168936d3f..693a20d309e3 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -500,7 +500,7 @@
VMLINUX_SYMBOL(__entry_text_end) = .;

 #define IRQENTRY_TEXT  \
-   ALIGN_FUNCTION();   \
+   . = ALIGN(4096);\
VMLINUX_SYMBOL(__irqentry_text_start) = .;  \
*(.irqentry.text)   \
VMLINUX_SYMBOL(__irqentry_text_end) = .;


It looks a *lot* better with mine and your patches applied.  It probably
would have helped Ingo and Thomas figure the problem out a lot sooner:

[1.159016] PANIC: double fault, error_code: 0x0
[1.159583] CPU: 1 PID: 68 Comm: modprobe Not tainted 
4.14.0-01257-g761d390195b6-dirty #19
[1.159583] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.10.2-1.fc26 04/01/2014
[1.159583] task: 880136f6bb00 task.stack: c9984000
[1.159583] RIP: 0010:page_fault+0x11/0x60
[1.159583] RSP: :ff083fc8 EFLAGS: 00010046
[1.159583] RAX: 819d0a87 RBX: 0001 RCX: 819d0a87
[1.159583] RDX: 1000 RSI: 0010 RDI: ff084078
[1.159583] RBP: 0d68 R08: 7f6d6bb24278 R09: 0023
[1.159583] R10: 558e0feca600 R11: 0246 R12: 7f6d6bb203c0
[1.159583] R13: 7f6d6bb1f880 R14: 7793bebc R15: 0100
[1.159583] FS:  7f6d6c39c5c0() GS:88013fc4() 
knlGS:
[1.159583] CS:  0010 DS:  ES:  CR0: 80050033
[1.159583] CR2: ff083fb8 CR3: 000136f78002 CR4: 001606e0
[1.159583] Call Trace:
[1.159583]  
[1.159583]  __do_page_fault+0x4b0/0x4b0
[1.159583]  page_fault+0x2c/0x60
[1.159583] RIP: 0010:do_page_fault+0x0/0x100
[1.159583] RSP: :ff084120 EFLAGS: 00010012
[1.159583] RAX: 819d0a87 RBX: 0001 RCX: 819d0a87
[1.159583] RDX: 1000 RSI: 0010 RDI: ff084128
[1.159583] RBP: 0d68 R08: 7f6d6bb24278 R09: 0023
[1.159583] R10: 558e0feca600 R11: 0246 R12: 7f6d6bb203c0
[1.159583] R13: 7f6d6bb1f880 R14: 7793bebc R15: 0100
[1.159583]  ? native_iret+0x7/0x7
[1.159583]  page_fault+0x2c/0x60
[1.159583] RIP: 0010:apic_timer_interrupt+0x0/0xb0
[1.159583] RSP: :ff0841d8 EFLAGS: 00010046
[1.159583] RAX: 0374 RBX: 558e0feca2c0 RCX: 7f6d6b85aaf0
[1.159583] RDX: 1000 RSI: 558e0feca600 RDI: 
[1.159583] RBP: 0d68 R08: 7f6d6bb24278 R09: 0023
[1.159583] R10: 558e0feca600 R11: 0246 R12: 7f6d6bb203c0
[1.159583] R13: 7f6d6bb1f880 R14: 7793bebc R15: 0100
[1.159583] RIP: 0033:0x7f6d6b85aaf0
[1.159583] RSP: 002b:7793bd68 EFLAGS: 0246 ORIG_RAX: 
0003
[1.159583] RAX: 819d2000 RBX: 7f6d6b85aaf0 RCX: 0010
[1.159583] RDX: 00010046 RSI: ff0841d8 RDI: 
[1.159583] RBP: 0374 R08: 

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Josh Poimboeuf]

On Sat, Nov 25, 2017 at 08:25:12PM -0800, Andy Lutomirski wrote:
> On Sat, Nov 25, 2017 at 6:40 PM, Josh Poimboeuf  wrote:
> > On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
> >> Can you send me whatever config and exact commit hash generated this?
> >> I can try to figure out why it failed.
> >
> > Sorry, I've been traveling.  I just got some time to take a look at
> > this.  I think there are at least two unwinder issues here:
> >
> > - It doesn't deal gracefully with the case where the stack overflows and
> >   the stack pointer itself isn't on a valid stack but the
> >   to-be-dereferenced data *is*.
> >
> > - The oops dump code doesn't know how to print partial pt_regs, for the
> >   case where if we get an interrupt/exception in *early* entry code
> >   before the full pt_regs have been saved.
> >
> > (Andy, I'm not quite sure about your patch, and whether it's still
> > needed after these patches.  I'll need to look at it later when I have
> > more time.)
> 
> I haven't tested yet, but I think my patch is probably still needed.
> The issue I fixed is that unwind_start() would bail out early if sp
> was below the stack.  Also:

Makes sense, maybe both are needed.  Your patch deals with a bad SP at
the beginning and mine deals with a bad SP in the middle.

> > -static bool stack_access_ok(struct unwind_state *state, unsigned long addr,
> > +static bool stack_access_ok(struct unwind_state *state, unsigned long 
> > _addr,
> > size_t len)
> >  {
> > struct stack_info *info = >stack_info;
> > +   void *addr = (void *)_addr;
> >
> > -   /*
> > -* If the address isn't on the current stack, switch to the next 
> > one.
> > -*
> > -* We may have to traverse multiple stacks to deal with the 
> > possibility
> > -* that info->next_sp could point to an empty stack and the address
> > -* could be on a subsequent stack.
> > -*/
> > -   while (!on_stack(info, (void *)addr, len))
> > -   if (get_stack_info(info->next_sp, state->task, info,
> > -  >stack_mask))
> > -   return false;
> > +   if (!on_stack(info, addr, len) &&
> > +   (get_stack_info(addr, state->task, info, >stack_mask)))
> > +   return false;
> >
> > return true;
> >  }
> 
> This looks odd to me before and after.  Shouldn't this be side-effect
> free?  That is, shouldn't it create a copy of info and stack_mask and
> point that to get_stack_info() rather than allowing get_stack_info()
> to modify the unwind state?

I think the side effects are ok, but maybe stack_access_ok() should be
renamed to make it clearer that it has side effects.

-- 
Josh

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Josh Poimboeuf]

On Sat, Nov 25, 2017 at 08:25:12PM -0800, Andy Lutomirski wrote:
> On Sat, Nov 25, 2017 at 6:40 PM, Josh Poimboeuf  wrote:
> > On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
> >> Can you send me whatever config and exact commit hash generated this?
> >> I can try to figure out why it failed.
> >
> > Sorry, I've been traveling.  I just got some time to take a look at
> > this.  I think there are at least two unwinder issues here:
> >
> > - It doesn't deal gracefully with the case where the stack overflows and
> >   the stack pointer itself isn't on a valid stack but the
> >   to-be-dereferenced data *is*.
> >
> > - The oops dump code doesn't know how to print partial pt_regs, for the
> >   case where if we get an interrupt/exception in *early* entry code
> >   before the full pt_regs have been saved.
> >
> > (Andy, I'm not quite sure about your patch, and whether it's still
> > needed after these patches.  I'll need to look at it later when I have
> > more time.)
> 
> I haven't tested yet, but I think my patch is probably still needed.
> The issue I fixed is that unwind_start() would bail out early if sp
> was below the stack.  Also:

Makes sense, maybe both are needed.  Your patch deals with a bad SP at
the beginning and mine deals with a bad SP in the middle.

> > -static bool stack_access_ok(struct unwind_state *state, unsigned long addr,
> > +static bool stack_access_ok(struct unwind_state *state, unsigned long 
> > _addr,
> > size_t len)
> >  {
> > struct stack_info *info = >stack_info;
> > +   void *addr = (void *)_addr;
> >
> > -   /*
> > -* If the address isn't on the current stack, switch to the next 
> > one.
> > -*
> > -* We may have to traverse multiple stacks to deal with the 
> > possibility
> > -* that info->next_sp could point to an empty stack and the address
> > -* could be on a subsequent stack.
> > -*/
> > -   while (!on_stack(info, (void *)addr, len))
> > -   if (get_stack_info(info->next_sp, state->task, info,
> > -  >stack_mask))
> > -   return false;
> > +   if (!on_stack(info, addr, len) &&
> > +   (get_stack_info(addr, state->task, info, >stack_mask)))
> > +   return false;
> >
> > return true;
> >  }
> 
> This looks odd to me before and after.  Shouldn't this be side-effect
> free?  That is, shouldn't it create a copy of info and stack_mask and
> point that to get_stack_info() rather than allowing get_stack_info()
> to modify the unwind state?

I think the side effects are ok, but maybe stack_access_ok() should be
renamed to make it clearer that it has side effects.

-- 
Josh

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Andy Lutomirski]

On Sat, Nov 25, 2017 at 6:40 PM, Josh Poimboeuf  wrote:
> On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
>> Can you send me whatever config and exact commit hash generated this?
>> I can try to figure out why it failed.
>
> Sorry, I've been traveling.  I just got some time to take a look at
> this.  I think there are at least two unwinder issues here:
>
> - It doesn't deal gracefully with the case where the stack overflows and
>   the stack pointer itself isn't on a valid stack but the
>   to-be-dereferenced data *is*.
>
> - The oops dump code doesn't know how to print partial pt_regs, for the
>   case where if we get an interrupt/exception in *early* entry code
>   before the full pt_regs have been saved.
>
> (Andy, I'm not quite sure about your patch, and whether it's still
> needed after these patches.  I'll need to look at it later when I have
> more time.)

I haven't tested yet, but I think my patch is probably still needed.
The issue I fixed is that unwind_start() would bail out early if sp
was below the stack.  Also:

> -static bool stack_access_ok(struct unwind_state *state, unsigned long addr,
> +static bool stack_access_ok(struct unwind_state *state, unsigned long _addr,
> size_t len)
>  {
> struct stack_info *info = >stack_info;
> +   void *addr = (void *)_addr;
>
> -   /*
> -* If the address isn't on the current stack, switch to the next one.
> -*
> -* We may have to traverse multiple stacks to deal with the 
> possibility
> -* that info->next_sp could point to an empty stack and the address
> -* could be on a subsequent stack.
> -*/
> -   while (!on_stack(info, (void *)addr, len))
> -   if (get_stack_info(info->next_sp, state->task, info,
> -  >stack_mask))
> -   return false;
> +   if (!on_stack(info, addr, len) &&
> +   (get_stack_info(addr, state->task, info, >stack_mask)))
> +   return false;
>
> return true;
>  }

This looks odd to me before and after.  Shouldn't this be side-effect
free?  That is, shouldn't it create a copy of info and stack_mask and
point that to get_stack_info() rather than allowing get_stack_info()
to modify the unwind state?

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Andy Lutomirski]

On Sat, Nov 25, 2017 at 6:40 PM, Josh Poimboeuf  wrote:
> On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
>> Can you send me whatever config and exact commit hash generated this?
>> I can try to figure out why it failed.
>
> Sorry, I've been traveling.  I just got some time to take a look at
> this.  I think there are at least two unwinder issues here:
>
> - It doesn't deal gracefully with the case where the stack overflows and
>   the stack pointer itself isn't on a valid stack but the
>   to-be-dereferenced data *is*.
>
> - The oops dump code doesn't know how to print partial pt_regs, for the
>   case where if we get an interrupt/exception in *early* entry code
>   before the full pt_regs have been saved.
>
> (Andy, I'm not quite sure about your patch, and whether it's still
> needed after these patches.  I'll need to look at it later when I have
> more time.)

I haven't tested yet, but I think my patch is probably still needed.
The issue I fixed is that unwind_start() would bail out early if sp
was below the stack.  Also:

> -static bool stack_access_ok(struct unwind_state *state, unsigned long addr,
> +static bool stack_access_ok(struct unwind_state *state, unsigned long _addr,
> size_t len)
>  {
> struct stack_info *info = >stack_info;
> +   void *addr = (void *)_addr;
>
> -   /*
> -* If the address isn't on the current stack, switch to the next one.
> -*
> -* We may have to traverse multiple stacks to deal with the 
> possibility
> -* that info->next_sp could point to an empty stack and the address
> -* could be on a subsequent stack.
> -*/
> -   while (!on_stack(info, (void *)addr, len))
> -   if (get_stack_info(info->next_sp, state->task, info,
> -  >stack_mask))
> -   return false;
> +   if (!on_stack(info, addr, len) &&
> +   (get_stack_info(addr, state->task, info, >stack_mask)))
> +   return false;
>
> return true;
>  }

This looks odd to me before and after.  Shouldn't this be side-effect
free?  That is, shouldn't it create a copy of info and stack_mask and
point that to get_stack_info() rather than allowing get_stack_info()
to modify the unwind state?

[PATCH] fs: befs: btree: Fixed some coding standard issues

[Alex Frappier Lachapelle]

Fixed some coding standard issues.

Signed-off-by: Alex Frappier Lachapelle 
---
 fs/befs/btree.c | 97 ++---
 1 file changed, 51 insertions(+), 46 deletions(-)

diff --git a/fs/befs/btree.c b/fs/befs/btree.c
index 1b7e0f7128d6..76a401c48bae 100644
--- a/fs/befs/btree.c
+++ b/fs/befs/btree.c
@@ -6,7 +6,7 @@
  * Licensed under the GNU GPL. See the file COPYING for details.
  *
  * 2002-02-05: Sergey S. Kostyliov added binary search within
- * btree nodes.
+ * btree nodes.
  *
  * Many thanks to:
  *
@@ -88,13 +88,15 @@ struct befs_btree_node {
 static const befs_off_t BEFS_BT_INVAL = 0xULL;
 
 /* local functions */
-static int befs_btree_seekleaf(struct super_block *sb, const befs_data_stream 
*ds,
-  befs_btree_super * bt_super,
+static int befs_btree_seekleaf(struct super_block *sb,
+  const befs_data_stream *ds,
+  befs_btree_super *bt_super,
   struct befs_btree_node *this_node,
-  befs_off_t * node_off);
+  befs_off_t *node_off);
 
-static int befs_bt_read_super(struct super_block *sb, const befs_data_stream 
*ds,
- befs_btree_super * sup);
+static int befs_bt_read_super(struct super_block *sb,
+ const befs_data_stream *ds,
+ befs_btree_super *sup);
 
 static int befs_bt_read_node(struct super_block *sb, const befs_data_stream 
*ds,
 struct befs_btree_node *node,
@@ -110,11 +112,11 @@ static char *befs_bt_keydata(struct befs_btree_node 
*node);
 
 static int befs_find_key(struct super_block *sb,
 struct befs_btree_node *node,
-const char *findkey, befs_off_t * value);
+const char *findkey, befs_off_t *value);
 
 static char *befs_bt_get_key(struct super_block *sb,
 struct befs_btree_node *node,
-int index, u16 * keylen);
+int index, u16 *keylen);
 
 static int befs_compare_strings(const void *key1, int keylen1,
const void *key2, int keylen2);
@@ -132,7 +134,7 @@ static int befs_compare_strings(const void *key1, int 
keylen1,
  */
 static int
 befs_bt_read_super(struct super_block *sb, const befs_data_stream *ds,
-  befs_btree_super * sup)
+  befs_btree_super *sup)
 {
struct buffer_head *bh;
befs_disk_btree_super *od_sup;
@@ -163,7 +165,7 @@ befs_bt_read_super(struct super_block *sb, const 
befs_data_stream *ds,
befs_debug(sb, "<--- %s", __func__);
return BEFS_OK;
 
-  error:
+error:
befs_debug(sb, "<--- %s ERROR", __func__);
return BEFS_ERR;
 }
@@ -200,8 +202,8 @@ befs_bt_read_node(struct super_block *sb, const 
befs_data_stream *ds,
 
node->bh = befs_read_datastream(sb, ds, node_off, );
if (!node->bh) {
-   befs_error(sb, "%s failed to read "
-  "node at %llu", __func__, node_off);
+   befs_error(sb, "%s failed to read node at %llu",
+   __func__, node_off);
befs_debug(sb, "<--- %s ERROR", __func__);
 
return BEFS_ERR;
@@ -243,7 +245,7 @@ befs_bt_read_node(struct super_block *sb, const 
befs_data_stream *ds,
  */
 int
 befs_btree_find(struct super_block *sb, const befs_data_stream *ds,
-   const char *key, befs_off_t * value)
+   const char *key, befs_off_t *value)
 {
struct befs_btree_node *this_node;
befs_btree_super bt_super;
@@ -253,16 +255,16 @@ befs_btree_find(struct super_block *sb, const 
befs_data_stream *ds,
befs_debug(sb, "---> %s Key: %s", __func__, key);
 
if (befs_bt_read_super(sb, ds, _super) != BEFS_OK) {
-   befs_error(sb,
-  "befs_btree_find() failed to read index superblock");
+   befs_error(sb, "%s() failed to read index superblock",
+   __func__);
goto error;
}
 
this_node = kmalloc(sizeof(struct befs_btree_node),
GFP_NOFS);
if (!this_node) {
-   befs_error(sb, "befs_btree_find() failed to allocate %zu "
-  "bytes of memory", sizeof(struct befs_btree_node));
+   befs_error(sb, "%s() failed to allocate %zu bytes of memory",
+   __func__, sizeof(struct befs_btree_node));
goto error;
}
 
@@ -271,8 +273,8 @@ befs_btree_find(struct super_block *sb, const 
befs_data_stream *ds,
/* read in root node */
node_off = bt_super.root_node_ptr;
if (befs_bt_read_node(sb, ds, this_node, node_off) != 

[PATCH] fs: befs: btree: Fixed some coding standard issues

[Alex Frappier Lachapelle]

Fixed some coding standard issues.

Signed-off-by: Alex Frappier Lachapelle 
---
 fs/befs/btree.c | 97 ++---
 1 file changed, 51 insertions(+), 46 deletions(-)

diff --git a/fs/befs/btree.c b/fs/befs/btree.c
index 1b7e0f7128d6..76a401c48bae 100644
--- a/fs/befs/btree.c
+++ b/fs/befs/btree.c
@@ -6,7 +6,7 @@
  * Licensed under the GNU GPL. See the file COPYING for details.
  *
  * 2002-02-05: Sergey S. Kostyliov added binary search within
- * btree nodes.
+ * btree nodes.
  *
  * Many thanks to:
  *
@@ -88,13 +88,15 @@ struct befs_btree_node {
 static const befs_off_t BEFS_BT_INVAL = 0xULL;
 
 /* local functions */
-static int befs_btree_seekleaf(struct super_block *sb, const befs_data_stream 
*ds,
-  befs_btree_super * bt_super,
+static int befs_btree_seekleaf(struct super_block *sb,
+  const befs_data_stream *ds,
+  befs_btree_super *bt_super,
   struct befs_btree_node *this_node,
-  befs_off_t * node_off);
+  befs_off_t *node_off);
 
-static int befs_bt_read_super(struct super_block *sb, const befs_data_stream 
*ds,
- befs_btree_super * sup);
+static int befs_bt_read_super(struct super_block *sb,
+ const befs_data_stream *ds,
+ befs_btree_super *sup);
 
 static int befs_bt_read_node(struct super_block *sb, const befs_data_stream 
*ds,
 struct befs_btree_node *node,
@@ -110,11 +112,11 @@ static char *befs_bt_keydata(struct befs_btree_node 
*node);
 
 static int befs_find_key(struct super_block *sb,
 struct befs_btree_node *node,
-const char *findkey, befs_off_t * value);
+const char *findkey, befs_off_t *value);
 
 static char *befs_bt_get_key(struct super_block *sb,
 struct befs_btree_node *node,
-int index, u16 * keylen);
+int index, u16 *keylen);
 
 static int befs_compare_strings(const void *key1, int keylen1,
const void *key2, int keylen2);
@@ -132,7 +134,7 @@ static int befs_compare_strings(const void *key1, int 
keylen1,
  */
 static int
 befs_bt_read_super(struct super_block *sb, const befs_data_stream *ds,
-  befs_btree_super * sup)
+  befs_btree_super *sup)
 {
struct buffer_head *bh;
befs_disk_btree_super *od_sup;
@@ -163,7 +165,7 @@ befs_bt_read_super(struct super_block *sb, const 
befs_data_stream *ds,
befs_debug(sb, "<--- %s", __func__);
return BEFS_OK;
 
-  error:
+error:
befs_debug(sb, "<--- %s ERROR", __func__);
return BEFS_ERR;
 }
@@ -200,8 +202,8 @@ befs_bt_read_node(struct super_block *sb, const 
befs_data_stream *ds,
 
node->bh = befs_read_datastream(sb, ds, node_off, );
if (!node->bh) {
-   befs_error(sb, "%s failed to read "
-  "node at %llu", __func__, node_off);
+   befs_error(sb, "%s failed to read node at %llu",
+   __func__, node_off);
befs_debug(sb, "<--- %s ERROR", __func__);
 
return BEFS_ERR;
@@ -243,7 +245,7 @@ befs_bt_read_node(struct super_block *sb, const 
befs_data_stream *ds,
  */
 int
 befs_btree_find(struct super_block *sb, const befs_data_stream *ds,
-   const char *key, befs_off_t * value)
+   const char *key, befs_off_t *value)
 {
struct befs_btree_node *this_node;
befs_btree_super bt_super;
@@ -253,16 +255,16 @@ befs_btree_find(struct super_block *sb, const 
befs_data_stream *ds,
befs_debug(sb, "---> %s Key: %s", __func__, key);
 
if (befs_bt_read_super(sb, ds, _super) != BEFS_OK) {
-   befs_error(sb,
-  "befs_btree_find() failed to read index superblock");
+   befs_error(sb, "%s() failed to read index superblock",
+   __func__);
goto error;
}
 
this_node = kmalloc(sizeof(struct befs_btree_node),
GFP_NOFS);
if (!this_node) {
-   befs_error(sb, "befs_btree_find() failed to allocate %zu "
-  "bytes of memory", sizeof(struct befs_btree_node));
+   befs_error(sb, "%s() failed to allocate %zu bytes of memory",
+   __func__, sizeof(struct befs_btree_node));
goto error;
}
 
@@ -271,8 +273,8 @@ befs_btree_find(struct super_block *sb, const 
befs_data_stream *ds,
/* read in root node */
node_off = bt_super.root_node_ptr;
if (befs_bt_read_node(sb, ds, this_node, node_off) != BEFS_OK) {
-   

d7be102f29 ("cfg80211: initialize regulatory keys/database later"): kernel BUG at crypto/asymmetric_keys/public_key.c:80!

[Fengguang Wu]

FYI, we noticed the following commit (built with gcc-4.8):

commit: d7be102f2945a626f55e0501e52bb31ba3e77b81 ("cfg80211: initialize 
regulatory keys/database later")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master

in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu Nehalem -smp 2 -m 512M

caused below changes (please refer to attached dmesg/kmsg for entire 
log/backtrace):


+---+++
|   | 7cca2acdff | d7be102f29 |
+---+++
| boot_successes| 10 | 0  |
| boot_failures | 2  | 12 |
| BUG:kernel_hang_in_test_stage | 2  ||
| kernel_BUG_at_crypto/asymmetric_keys/public_key.c | 0  | 12 |
| invalid_opcode:#[##]  | 0  | 12 |
| RIP:public_key_verify_signature   | 0  | 12 |
| Kernel_panic-not_syncing:Fatal_exception  | 0  | 12 |
+---+++



[8.602885] kernel BUG at crypto/asymmetric_keys/public_key.c:80!
[8.604548] invalid opcode:  [#1]
[8.605140] Modules linked in:
[8.605603] CPU: 0 PID: 1 Comm: swapper Not tainted 4.14.0-12781-gd7be102 #1
[8.606870] task: 88001e08d500 task.stack: c9008000
[8.607873] RIP: 0010:public_key_verify_signature+0x267/0x280
[8.607873] RSP: :c900bbd8 EFLAGS: 00010246
[8.607873] RAX:  RBX: 88001b465180 RCX: 81839ff2
[8.607873] RDX: 0012 RSI: 88001b465258 RDI: 88001b465230
[8.607873] RBP: 88001b465258 R08: 0065 R09: 
[8.607873] R10: 0003 R11: fff8 R12: 
[8.607873] R13: 88001b465230 R14: ffec R15: 02a8
[8.607873] FS:  () GS:81a2f000() 
knlGS:
[8.607873] CS:  0010 DS:  ES:  CR0: 80050033
[8.607873] CR2:  CR3: 01a0e000 CR4: 06f0
[8.607873] Call Trace:
[8.607873]  ? cryptomgr_notify+0x2a4/0x4e0
[8.607873]  ? notifier_call_chain+0x44/0x70
[8.607873]  ? asymmetric_key_generate_id+0x28/0x70
[8.607873]  ? __kmalloc+0xa6/0x160
[8.607873]  ? crypto_alloc_tfm+0x52/0xe0
[8.607873]  x509_check_for_self_signed+0xbe/0xf0
[8.607873]  x509_cert_parse+0x130/0x190
[8.607873]  x509_key_preparse+0x23/0x1a0
[8.607873]  asymmetric_key_preparse+0x4a/0x80
[8.607873]  ? key_type_lookup+0x46/0x70
[8.607873]  key_create_or_update+0x122/0x430
[8.607873]  ? vprintk_emit+0x22d/0x2f0
[8.607873]  regulatory_init_db+0xfe/0x1c2
[8.607873]  ? cfg80211_init+0xd4/0xd4
[8.607873]  do_one_initcall+0x4c/0x1a0
[8.607873]  ? parse_args+0x1c0/0x2d0
[8.607873]  kernel_init_freeable+0x111/0x195
[8.607873]  ? set_debug_rodata+0x11/0x11
[8.607873]  ? rest_init+0xa0/0xa0
[8.607873]  kernel_init+0xa/0xf0
[8.607873]  ret_from_fork+0x24/0x30
[8.607873] Code: c1 48 8b 7d 20 4c 89 f6 e8 97 e0 35 00 85 c0 b8 7f ff ff 
ff 44 0f 45 e8 eb c1 b8 ea ff ff ff e9 7d fe ff ff e8 7b 69 e5 ff 0f 0b <0f> 0b 
0f 0b 0f 0b 41 bd f4 ff ff ff e9 57 fe ff ff 0f 1f 84 00 
[8.607873] RIP: public_key_verify_signature+0x267/0x280 RSP: 
c900bbd8
[8.641443] ---[ end trace 50904d4bfe4a1f13 ]---


To reproduce:

 git clone https://github.com/intel/lkp-tests.git
 cd lkp-tests
 bin/lkp qemu -k  job-script  # job-script is attached in this 
email

Thanks,
wfg
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86_64 4.14.0 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y

d7be102f29 ("cfg80211: initialize regulatory keys/database later"): kernel BUG at crypto/asymmetric_keys/public_key.c:80!

[Fengguang Wu]

FYI, we noticed the following commit (built with gcc-4.8):

commit: d7be102f2945a626f55e0501e52bb31ba3e77b81 ("cfg80211: initialize 
regulatory keys/database later")
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git master

in testcase: boot

on test machine: qemu-system-x86_64 -enable-kvm -cpu Nehalem -smp 2 -m 512M

caused below changes (please refer to attached dmesg/kmsg for entire 
log/backtrace):


+---+++
|   | 7cca2acdff | d7be102f29 |
+---+++
| boot_successes| 10 | 0  |
| boot_failures | 2  | 12 |
| BUG:kernel_hang_in_test_stage | 2  ||
| kernel_BUG_at_crypto/asymmetric_keys/public_key.c | 0  | 12 |
| invalid_opcode:#[##]  | 0  | 12 |
| RIP:public_key_verify_signature   | 0  | 12 |
| Kernel_panic-not_syncing:Fatal_exception  | 0  | 12 |
+---+++



[8.602885] kernel BUG at crypto/asymmetric_keys/public_key.c:80!
[8.604548] invalid opcode:  [#1]
[8.605140] Modules linked in:
[8.605603] CPU: 0 PID: 1 Comm: swapper Not tainted 4.14.0-12781-gd7be102 #1
[8.606870] task: 88001e08d500 task.stack: c9008000
[8.607873] RIP: 0010:public_key_verify_signature+0x267/0x280
[8.607873] RSP: :c900bbd8 EFLAGS: 00010246
[8.607873] RAX:  RBX: 88001b465180 RCX: 81839ff2
[8.607873] RDX: 0012 RSI: 88001b465258 RDI: 88001b465230
[8.607873] RBP: 88001b465258 R08: 0065 R09: 
[8.607873] R10: 0003 R11: fff8 R12: 
[8.607873] R13: 88001b465230 R14: ffec R15: 02a8
[8.607873] FS:  () GS:81a2f000() 
knlGS:
[8.607873] CS:  0010 DS:  ES:  CR0: 80050033
[8.607873] CR2:  CR3: 01a0e000 CR4: 06f0
[8.607873] Call Trace:
[8.607873]  ? cryptomgr_notify+0x2a4/0x4e0
[8.607873]  ? notifier_call_chain+0x44/0x70
[8.607873]  ? asymmetric_key_generate_id+0x28/0x70
[8.607873]  ? __kmalloc+0xa6/0x160
[8.607873]  ? crypto_alloc_tfm+0x52/0xe0
[8.607873]  x509_check_for_self_signed+0xbe/0xf0
[8.607873]  x509_cert_parse+0x130/0x190
[8.607873]  x509_key_preparse+0x23/0x1a0
[8.607873]  asymmetric_key_preparse+0x4a/0x80
[8.607873]  ? key_type_lookup+0x46/0x70
[8.607873]  key_create_or_update+0x122/0x430
[8.607873]  ? vprintk_emit+0x22d/0x2f0
[8.607873]  regulatory_init_db+0xfe/0x1c2
[8.607873]  ? cfg80211_init+0xd4/0xd4
[8.607873]  do_one_initcall+0x4c/0x1a0
[8.607873]  ? parse_args+0x1c0/0x2d0
[8.607873]  kernel_init_freeable+0x111/0x195
[8.607873]  ? set_debug_rodata+0x11/0x11
[8.607873]  ? rest_init+0xa0/0xa0
[8.607873]  kernel_init+0xa/0xf0
[8.607873]  ret_from_fork+0x24/0x30
[8.607873] Code: c1 48 8b 7d 20 4c 89 f6 e8 97 e0 35 00 85 c0 b8 7f ff ff 
ff 44 0f 45 e8 eb c1 b8 ea ff ff ff e9 7d fe ff ff e8 7b 69 e5 ff 0f 0b <0f> 0b 
0f 0b 0f 0b 41 bd f4 ff ff ff e9 57 fe ff ff 0f 1f 84 00 
[8.607873] RIP: public_key_verify_signature+0x267/0x280 RSP: 
c900bbd8
[8.641443] ---[ end trace 50904d4bfe4a1f13 ]---


To reproduce:

 git clone https://github.com/intel/lkp-tests.git
 cd lkp-tests
 bin/lkp qemu -k  job-script  # job-script is attached in this 
email

Thanks,
wfg
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86_64 4.14.0 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=28
CONFIG_ARCH_MMAP_RND_BITS_MAX=32
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y

[PATCH] auxdisplay: img-ascii-lcd: add MODULE_LICENSE("GPL")

[Daniel Axtens]

This matches the header at the top of the file and squashes:

WARNING: modpost: missing MODULE_LICENSE() in drivers/auxdisplay/img-ascii-lcd.o
see include/linux/module.h for more information

Signed-off-by: Daniel Axtens 
---
 drivers/auxdisplay/img-ascii-lcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/auxdisplay/img-ascii-lcd.c 
b/drivers/auxdisplay/img-ascii-lcd.c
index db040b378224..7315ffa79537 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -441,3 +441,5 @@ static struct platform_driver img_ascii_lcd_driver = {
.remove = img_ascii_lcd_remove,
 };
 module_platform_driver(img_ascii_lcd_driver);
+
+MODULE_LICENSE("GPL");
-- 
2.11.0

[PATCH] auxdisplay: img-ascii-lcd: add MODULE_LICENSE("GPL")

[Daniel Axtens]

This matches the header at the top of the file and squashes:

WARNING: modpost: missing MODULE_LICENSE() in drivers/auxdisplay/img-ascii-lcd.o
see include/linux/module.h for more information

Signed-off-by: Daniel Axtens 
---
 drivers/auxdisplay/img-ascii-lcd.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/auxdisplay/img-ascii-lcd.c 
b/drivers/auxdisplay/img-ascii-lcd.c
index db040b378224..7315ffa79537 100644
--- a/drivers/auxdisplay/img-ascii-lcd.c
+++ b/drivers/auxdisplay/img-ascii-lcd.c
@@ -441,3 +441,5 @@ static struct platform_driver img_ascii_lcd_driver = {
.remove = img_ascii_lcd_remove,
 };
 module_platform_driver(img_ascii_lcd_driver);
+
+MODULE_LICENSE("GPL");
-- 
2.11.0

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Josh Poimboeuf]

On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
> Can you send me whatever config and exact commit hash generated this?
> I can try to figure out why it failed.

Sorry, I've been traveling.  I just got some time to take a look at
this.  I think there are at least two unwinder issues here:

- It doesn't deal gracefully with the case where the stack overflows and
  the stack pointer itself isn't on a valid stack but the
  to-be-dereferenced data *is*.

- The oops dump code doesn't know how to print partial pt_regs, for the
  case where if we get an interrupt/exception in *early* entry code
  before the full pt_regs have been saved.

(Andy, I'm not quite sure about your patch, and whether it's still
needed after these patches.  I'll need to look at it later when I have
more time.)

I attempted to fix both of the issues with the below patch.  Thomas or
Ingo, can you test to see if this gets rid of the question marks?

I can split it up into proper patches next week.  I'm assuming this
isn't holding up the KAISER merge?


diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h
index f86a8caa561e..395c9631e000 100644
--- a/arch/x86/include/asm/kdebug.h
+++ b/arch/x86/include/asm/kdebug.h
@@ -26,6 +26,7 @@ extern void die(const char *, struct pt_regs *,long);
 extern int __must_check __die(const char *, struct pt_regs *, long);
 extern void show_stack_regs(struct pt_regs *regs);
 extern void __show_regs(struct pt_regs *regs, int all);
+extern void show_iret_regs(struct pt_regs *regs);
 extern unsigned long oops_begin(void);
 extern void oops_end(unsigned long, struct pt_regs *, int signr);
 
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index e9cc6fe1fc6f..5be2fb23825a 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -7,6 +7,9 @@
 #include 
 #include 
 
+#define IRET_FRAME_OFFSET (offsetof(struct pt_regs, ip))
+#define IRET_FRAME_SIZE   (sizeof(struct pt_regs) - IRET_FRAME_OFFSET)
+
 struct unwind_state {
struct stack_info stack_info;
unsigned long stack_mask;
@@ -52,6 +55,10 @@ void unwind_start(struct unwind_state *state, struct 
task_struct *task,
 }
 
 #if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
+/*
+ * WARNING: The entire pt_regs may not be safe to dereference.  In some cases,
+ * only the iret registers are accessible.  Use with caution!
+ */
 static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 {
if (unwind_done(state))
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index d58dd121c0af..f7f3b5b3bc05 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -68,6 +68,21 @@ static void printk_stack_address(unsigned long address, int 
reliable,
printk("%s %s%pB\n", log_lvl, reliable ? "" : "? ", (void *)address);
 }
 
+static void show_regs_safe(struct stack_info *info, struct pt_regs *regs)
+{
+   if (on_stack(info, regs, sizeof(*regs)))
+   __show_regs(regs, 0);
+   else if (on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
+ IRET_FRAME_SIZE)) {
+   /*
+* When an interrupt or exception occurs in entry code, the
+* full pt_regs might not have been saved yet.  In that case
+* just print the iret return frame.
+*/
+   show_iret_regs(regs);
+   }
+}
+
 void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, char *log_lvl)
 {
@@ -116,8 +131,8 @@ void show_trace_log_lvl(struct task_struct *task, struct 
pt_regs *regs,
if (stack_name)
printk("%s <%s>\n", log_lvl, stack_name);
 
-   if (regs && on_stack(_info, regs, sizeof(*regs)))
-   __show_regs(regs, 0);
+   if (regs)
+   show_regs_safe(_info, regs);
 
/*
 * Scan the stack, printing any text addresses we find.  At the
@@ -141,7 +156,7 @@ void show_trace_log_lvl(struct task_struct *task, struct 
pt_regs *regs,
 
/*
 * Don't print regs->ip again if it was already printed
-* by __show_regs() below.
+* by show_regs_safe() below.
 */
if (regs && stack == >ip)
goto next;
@@ -177,8 +192,8 @@ void show_trace_log_lvl(struct task_struct *task, struct 
pt_regs *regs,
 
/* if the frame has entry regs, print them */
regs = unwind_get_entry_regs();
-   if (regs && on_stack(_info, regs, sizeof(*regs)))
-   __show_regs(regs, 0);
+   if (regs)
+   show_regs_safe(_info, regs);
}
 
   

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Josh Poimboeuf]

On Sat, Nov 25, 2017 at 04:16:23PM -0800, Andy Lutomirski wrote:
> Can you send me whatever config and exact commit hash generated this?
> I can try to figure out why it failed.

Sorry, I've been traveling.  I just got some time to take a look at
this.  I think there are at least two unwinder issues here:

- It doesn't deal gracefully with the case where the stack overflows and
  the stack pointer itself isn't on a valid stack but the
  to-be-dereferenced data *is*.

- The oops dump code doesn't know how to print partial pt_regs, for the
  case where if we get an interrupt/exception in *early* entry code
  before the full pt_regs have been saved.

(Andy, I'm not quite sure about your patch, and whether it's still
needed after these patches.  I'll need to look at it later when I have
more time.)

I attempted to fix both of the issues with the below patch.  Thomas or
Ingo, can you test to see if this gets rid of the question marks?

I can split it up into proper patches next week.  I'm assuming this
isn't holding up the KAISER merge?


diff --git a/arch/x86/include/asm/kdebug.h b/arch/x86/include/asm/kdebug.h
index f86a8caa561e..395c9631e000 100644
--- a/arch/x86/include/asm/kdebug.h
+++ b/arch/x86/include/asm/kdebug.h
@@ -26,6 +26,7 @@ extern void die(const char *, struct pt_regs *,long);
 extern int __must_check __die(const char *, struct pt_regs *, long);
 extern void show_stack_regs(struct pt_regs *regs);
 extern void __show_regs(struct pt_regs *regs, int all);
+extern void show_iret_regs(struct pt_regs *regs);
 extern unsigned long oops_begin(void);
 extern void oops_end(unsigned long, struct pt_regs *, int signr);
 
diff --git a/arch/x86/include/asm/unwind.h b/arch/x86/include/asm/unwind.h
index e9cc6fe1fc6f..5be2fb23825a 100644
--- a/arch/x86/include/asm/unwind.h
+++ b/arch/x86/include/asm/unwind.h
@@ -7,6 +7,9 @@
 #include 
 #include 
 
+#define IRET_FRAME_OFFSET (offsetof(struct pt_regs, ip))
+#define IRET_FRAME_SIZE   (sizeof(struct pt_regs) - IRET_FRAME_OFFSET)
+
 struct unwind_state {
struct stack_info stack_info;
unsigned long stack_mask;
@@ -52,6 +55,10 @@ void unwind_start(struct unwind_state *state, struct 
task_struct *task,
 }
 
 #if defined(CONFIG_UNWINDER_ORC) || defined(CONFIG_UNWINDER_FRAME_POINTER)
+/*
+ * WARNING: The entire pt_regs may not be safe to dereference.  In some cases,
+ * only the iret registers are accessible.  Use with caution!
+ */
 static inline struct pt_regs *unwind_get_entry_regs(struct unwind_state *state)
 {
if (unwind_done(state))
diff --git a/arch/x86/kernel/dumpstack.c b/arch/x86/kernel/dumpstack.c
index d58dd121c0af..f7f3b5b3bc05 100644
--- a/arch/x86/kernel/dumpstack.c
+++ b/arch/x86/kernel/dumpstack.c
@@ -68,6 +68,21 @@ static void printk_stack_address(unsigned long address, int 
reliable,
printk("%s %s%pB\n", log_lvl, reliable ? "" : "? ", (void *)address);
 }
 
+static void show_regs_safe(struct stack_info *info, struct pt_regs *regs)
+{
+   if (on_stack(info, regs, sizeof(*regs)))
+   __show_regs(regs, 0);
+   else if (on_stack(info, (void *)regs + IRET_FRAME_OFFSET,
+ IRET_FRAME_SIZE)) {
+   /*
+* When an interrupt or exception occurs in entry code, the
+* full pt_regs might not have been saved yet.  In that case
+* just print the iret return frame.
+*/
+   show_iret_regs(regs);
+   }
+}
+
 void show_trace_log_lvl(struct task_struct *task, struct pt_regs *regs,
unsigned long *stack, char *log_lvl)
 {
@@ -116,8 +131,8 @@ void show_trace_log_lvl(struct task_struct *task, struct 
pt_regs *regs,
if (stack_name)
printk("%s <%s>\n", log_lvl, stack_name);
 
-   if (regs && on_stack(_info, regs, sizeof(*regs)))
-   __show_regs(regs, 0);
+   if (regs)
+   show_regs_safe(_info, regs);
 
/*
 * Scan the stack, printing any text addresses we find.  At the
@@ -141,7 +156,7 @@ void show_trace_log_lvl(struct task_struct *task, struct 
pt_regs *regs,
 
/*
 * Don't print regs->ip again if it was already printed
-* by __show_regs() below.
+* by show_regs_safe() below.
 */
if (regs && stack == >ip)
goto next;
@@ -177,8 +192,8 @@ void show_trace_log_lvl(struct task_struct *task, struct 
pt_regs *regs,
 
/* if the frame has entry regs, print them */
regs = unwind_get_entry_regs();
-   if (regs && on_stack(_info, regs, sizeof(*regs)))
-   __show_regs(regs, 0);
+   if (regs)
+   show_regs_safe(_info, regs);
}
 
   

[lib/rbtree,drm/mm] 653c9f9a4d: BUG:kernel_hang_in_boot_stage

[Fengguang Wu]

FYI, we noticed the following commit (built with gcc-5):

commit: 653c9f9a4dd8037ffc5afbb1040d15566aa8f533 ("lib/rbtree,drm/mm: Add 
rbtree_replace_node_cached()")
git://anongit.freedesktop.org/drm-intel topic/core-for-CI

in testcase: boot

on test machine: qemu-system-i386 -enable-kvm -smp 2 -m 320M

caused below changes (please refer to attached dmesg/kmsg for entire 
log/backtrace):

++++
|| 70c1cedd5c | 653c9f9a4d |
++++
| boot_successes | 0  | 0  |
| boot_failures  | 8  | 8  |
| genirq:Flags_mismatch_irq##(ttyS0)vs.#(sir_ir) | 7  | 4  |
| BUG:soft_lockup-CPU##stuck_for#s   | 1  ||
| EIP:insert_augmented   | 1  ||
| Kernel_panic-not_syncing:softlockup:hung_tasks | 1  ||
| BUG:kernel_hang_in_boot_stage  | 0  | 4  |
++++


 [   77.806701] Linux agpgart interface v0.103
 [   77.825237] Hangcheck: starting hangcheck timer 0.9.1 (tick is 180 
seconds, margin is 60 seconds).
 [   77.860502] drm_mm: Testing DRM range manger (struct drm_mm), with 
random_seed=0xbac20086 max_iterations=8192 max_prime=128
 [   77.898934] drm_mm: igt_sanitycheck - ok!
 [  328.297497] kworker/dying (33) used greatest stack depth: 7396 
bytes left
 BUG: kernel hang in boot stage

 Elapsed time: 440

To reproduce:

 git clone https://github.com/intel/lkp-tests.git
 cd lkp-tests
 bin/lkp qemu -k  job-script  # job-script is attached in this 
email

Thanks,
Fengguang
#
# Automatically generated file; DO NOT EDIT.
# Linux/i386 4.14.0 Kernel Configuration
#
# CONFIG_64BIT is not set
CONFIG_X86_32=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf32-i386"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_BITS_MAX=16
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=3
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_CONSTRUCTORS=y
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y

#
# General setup
#
CONFIG_BROKEN_ON_SMP=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
# CONFIG_KERNEL_GZIP is not set
CONFIG_KERNEL_BZIP2=y
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
# CONFIG_SYSVIPC is not set
# CONFIG_POSIX_MQUEUE is not set
# CONFIG_CROSS_MEMORY_ATTACH is not set
CONFIG_FHANDLE=y
# CONFIG_USELIB is not set
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_SIM=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_DEBUG=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
# CONFIG_GENERIC_IRQ_DEBUGFS is not set
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_HZ_PERIODIC=y
# CONFIG_NO_HZ_IDLE is not set
CONFIG_NO_HZ=y
# CONFIG_HIGH_RES_TIMERS is not set

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_IRQ_TIME_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
# CONFIG_TASKSTATS is not set

#
# RCU Subsystem
#
CONFIG_TINY_RCU=y
# 

[lib/rbtree,drm/mm] 653c9f9a4d: BUG:kernel_hang_in_boot_stage

[Fengguang Wu]

FYI, we noticed the following commit (built with gcc-5):

commit: 653c9f9a4dd8037ffc5afbb1040d15566aa8f533 ("lib/rbtree,drm/mm: Add 
rbtree_replace_node_cached()")
git://anongit.freedesktop.org/drm-intel topic/core-for-CI

in testcase: boot

on test machine: qemu-system-i386 -enable-kvm -smp 2 -m 320M

caused below changes (please refer to attached dmesg/kmsg for entire 
log/backtrace):

++++
|| 70c1cedd5c | 653c9f9a4d |
++++
| boot_successes | 0  | 0  |
| boot_failures  | 8  | 8  |
| genirq:Flags_mismatch_irq##(ttyS0)vs.#(sir_ir) | 7  | 4  |
| BUG:soft_lockup-CPU##stuck_for#s   | 1  ||
| EIP:insert_augmented   | 1  ||
| Kernel_panic-not_syncing:softlockup:hung_tasks | 1  ||
| BUG:kernel_hang_in_boot_stage  | 0  | 4  |
++++


 [   77.806701] Linux agpgart interface v0.103
 [   77.825237] Hangcheck: starting hangcheck timer 0.9.1 (tick is 180 
seconds, margin is 60 seconds).
 [   77.860502] drm_mm: Testing DRM range manger (struct drm_mm), with 
random_seed=0xbac20086 max_iterations=8192 max_prime=128
 [   77.898934] drm_mm: igt_sanitycheck - ok!
 [  328.297497] kworker/dying (33) used greatest stack depth: 7396 
bytes left
 BUG: kernel hang in boot stage

 Elapsed time: 440

To reproduce:

 git clone https://github.com/intel/lkp-tests.git
 cd lkp-tests
 bin/lkp qemu -k  job-script  # job-script is attached in this 
email

Thanks,
Fengguang
#
# Automatically generated file; DO NOT EDIT.
# Linux/i386 4.14.0 Kernel Configuration
#
# CONFIG_64BIT is not set
CONFIG_X86_32=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_OUTPUT_FORMAT="elf32-i386"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_MMU=y
CONFIG_ARCH_MMAP_RND_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_BITS_MAX=16
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8
CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=3
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_CONSTRUCTORS=y
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
CONFIG_THREAD_INFO_IN_TASK=y

#
# General setup
#
CONFIG_BROKEN_ON_SMP=y
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
# CONFIG_KERNEL_GZIP is not set
CONFIG_KERNEL_BZIP2=y
# CONFIG_KERNEL_LZMA is not set
# CONFIG_KERNEL_XZ is not set
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
# CONFIG_SYSVIPC is not set
# CONFIG_POSIX_MQUEUE is not set
# CONFIG_CROSS_MEMORY_ATTACH is not set
CONFIG_FHANDLE=y
# CONFIG_USELIB is not set
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_CHIP=y
CONFIG_IRQ_DOMAIN=y
CONFIG_IRQ_SIM=y
CONFIG_IRQ_DOMAIN_HIERARCHY=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_GENERIC_MSI_IRQ_DOMAIN=y
CONFIG_IRQ_DOMAIN_DEBUG=y
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
# CONFIG_GENERIC_IRQ_DEBUGFS is not set
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_HZ_PERIODIC=y
# CONFIG_NO_HZ_IDLE is not set
CONFIG_NO_HZ=y
# CONFIG_HIGH_RES_TIMERS is not set

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
CONFIG_IRQ_TIME_ACCOUNTING=y
CONFIG_BSD_PROCESS_ACCT=y
CONFIG_BSD_PROCESS_ACCT_V3=y
# CONFIG_TASKSTATS is not set

#
# RCU Subsystem
#
CONFIG_TINY_RCU=y
# 

Re: [PATCH v2] xfs: handle register_shrinker error

[Tetsuo Handa]

Dave Chinner wrote:
> IOWs, we don't actually need to touch this code, but if you really
> must, just remove the KM_NOFS tag.

OK. Then, please remove KM_NOFS. GFP_KERNEL is safer than GFP_NOFS
in the sense that it won't cause OOM lockup due to unable to invoke
the OOM killer.

Re: [PATCH v2] xfs: handle register_shrinker error

[Tetsuo Handa]

Dave Chinner wrote:
> IOWs, we don't actually need to touch this code, but if you really
> must, just remove the KM_NOFS tag.

OK. Then, please remove KM_NOFS. GFP_KERNEL is safer than GFP_NOFS
in the sense that it won't cause OOM lockup due to unable to invoke
the OOM killer.

Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE

[Alexei Starovoitov]

On 11/24/17 12:28 AM, Peter Zijlstra wrote:

On Thu, Nov 23, 2017 at 10:31:29PM -0800, Alexei Starovoitov wrote:

unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include 

struct S {
  unsigned long long a;
} s;

struct U {
  unsigned long long a;
} u;

int main()
{
printf("%d, %d\n", sizeof(unsigned long long),
   __alignof__(unsigned long long));
printf("%d, %d\n", sizeof(s), __alignof__(s));
printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4


*blink* how is that even correct? I understood the spec to say the
alignment of composite types should be the max alignment of any of its
member types (otherwise it cannot guarantee the alignment of its
members).


so we have to use __aligned_u64 in uapi.


Ideally yes, but effectively it most often doesn't matter.


Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.


I don't understand the reasoning why you cannot use them. Even if they
are not naturally aligned on x86_32, why would it matter?

x86_32 needs two loads in any case, but there is no concurrency, so
split loads is not a problem. Add to that that 'intptr_t' on ILP32
is in fact only a single u32 and thus the other u32 will always be 0.

So yes, alignment is screwy, but I really don't see who cares and why it
would matter in practise.


If we were poking into 'struct perf_event_attr __user *uptr'
directly like get|put_user(.., >config)
then 32-bit user space with 4-byte aligned u64s would cause
64-bit kernel to trap on archs like sparc.
But in this case you're right. We can use config[12] as-is, since these
u64 fields are passing the value one way only (into the kernel) and
we do full perf_copy_attr() first and all further accesses are from
copied structure and u64_to_user_ptr(event->attr.config) will be fine.

Do you mind we do
union {
 __u64 file_path;
 __u64 func_name;
 __u64 config;
};
and similar with config1 ?
Or prefer that we use 'config/config1' to store string+offset there?
I think config/config1 is cleaner than config1/config2

Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE

[Alexei Starovoitov]

On 11/24/17 12:28 AM, Peter Zijlstra wrote:

On Thu, Nov 23, 2017 at 10:31:29PM -0800, Alexei Starovoitov wrote:

unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include 

struct S {
  unsigned long long a;
} s;

struct U {
  unsigned long long a;
} u;

int main()
{
printf("%d, %d\n", sizeof(unsigned long long),
   __alignof__(unsigned long long));
printf("%d, %d\n", sizeof(s), __alignof__(s));
printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4


*blink* how is that even correct? I understood the spec to say the
alignment of composite types should be the max alignment of any of its
member types (otherwise it cannot guarantee the alignment of its
members).


so we have to use __aligned_u64 in uapi.


Ideally yes, but effectively it most often doesn't matter.


Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.


I don't understand the reasoning why you cannot use them. Even if they
are not naturally aligned on x86_32, why would it matter?

x86_32 needs two loads in any case, but there is no concurrency, so
split loads is not a problem. Add to that that 'intptr_t' on ILP32
is in fact only a single u32 and thus the other u32 will always be 0.

So yes, alignment is screwy, but I really don't see who cares and why it
would matter in practise.


If we were poking into 'struct perf_event_attr __user *uptr'
directly like get|put_user(.., >config)
then 32-bit user space with 4-byte aligned u64s would cause
64-bit kernel to trap on archs like sparc.
But in this case you're right. We can use config[12] as-is, since these
u64 fields are passing the value one way only (into the kernel) and
we do full perf_copy_attr() first and all further accesses are from
copied structure and u64_to_user_ptr(event->attr.config) will be fine.

Do you mind we do
union {
 __u64 file_path;
 __u64 func_name;
 __u64 config;
};
and similar with config1 ?
Or prefer that we use 'config/config1' to store string+offset there?
I think config/config1 is cleaner than config1/config2

[PATCH] Staging: comedi: das16: Fixed a const struct coding style issue

[Alex Frappier Lachapelle]

Fixed a coding style issue.

Signed-off-by: Alex Frappier Lachapelle 
---
 drivers/staging/comedi/drivers/das16.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index ddd4aeab6365..ed1e9e9b651d 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -967,7 +967,7 @@ static const struct comedi_lrange *das16_ai_range(struct 
comedi_device *dev,
 
/* get any user-defined input range */
if (pg_type == das16_pg_none && (min || max)) {
-   struct comedi_lrange *lrange;
+   const struct comedi_lrange *lrange;
struct comedi_krange *krange;
 
/* allocate single-range range table */
@@ -1001,7 +1001,7 @@ static const struct comedi_lrange *das16_ao_range(struct 
comedi_device *dev,
 
/* get any user-defined output range */
if (min || max) {
-   struct comedi_lrange *lrange;
+   const struct comedi_lrange *lrange;
struct comedi_krange *krange;
 
/* allocate single-range range table */
-- 
2.15.0

[PATCH] Staging: comedi: das16: Fixed a const struct coding style issue

[Alex Frappier Lachapelle]

Fixed a coding style issue.

Signed-off-by: Alex Frappier Lachapelle 
---
 drivers/staging/comedi/drivers/das16.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index ddd4aeab6365..ed1e9e9b651d 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -967,7 +967,7 @@ static const struct comedi_lrange *das16_ai_range(struct 
comedi_device *dev,
 
/* get any user-defined input range */
if (pg_type == das16_pg_none && (min || max)) {
-   struct comedi_lrange *lrange;
+   const struct comedi_lrange *lrange;
struct comedi_krange *krange;
 
/* allocate single-range range table */
@@ -1001,7 +1001,7 @@ static const struct comedi_lrange *das16_ao_range(struct 
comedi_device *dev,
 
/* get any user-defined output range */
if (min || max) {
-   struct comedi_lrange *lrange;
+   const struct comedi_lrange *lrange;
struct comedi_krange *krange;
 
/* allocate single-range range table */
-- 
2.15.0

[PATCH v2] Staging: sm750fb: Fix coding style issue in ddk750_sii164.c

[Jeremy Lacomis]

This is a patch to the ddk750_sii164.c file that fixes line length warnings
found by the checkpatch.pl script

Signed-off-by: Jeremy Lacomis 
---
Changes in v2:
- Change definition of i2cReadReg/i2cWriteReg
- Remove unnecessary casts

 drivers/staging/sm750fb/ddk750_sii164.c | 87 -
 1 file changed, 43 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
b/drivers/staging/sm750fb/ddk750_sii164.c
index c787a74c4f9c..66092c4645fc 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -12,11 +12,13 @@
 #define USE_HW_I2C
 
 #ifdef USE_HW_I2C
-#define i2cWriteReg sm750_hw_i2c_write_reg
-#define i2cReadReg  sm750_hw_i2c_read_reg
+#define i2c_write_reg(reg, config) sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, \
+ reg, config)
+#define i2c_read_reg(reg) sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, reg)
 #else
-#define i2cWriteReg sm750_sw_i2c_write_reg
-#define i2cReadReg  sm750_sw_i2c_read_reg
+#define i2c_write_reg(reg, config) sm750_sw_i2c_write_reg(SII164_I2C_ADDRESS, \
+ reg, config)
+#define i2c_read_reg(reg) sm750_sw_i2c_read_reg(SII164_I2C_ADDRESS, reg)
 #endif
 
 /* SII164 Vendor and Device ID */
@@ -39,8 +41,8 @@ unsigned short sii164GetVendorID(void)
 {
unsigned short vendorID;
 
-   vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_VENDOR_ID_HIGH) << 8) |
-   (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_VENDOR_ID_LOW);
+   vendorID = (i2c_read_reg(SII164_VENDOR_ID_HIGH) << 8) |
+   i2c_read_reg(SII164_VENDOR_ID_LOW);
 
return vendorID;
 }
@@ -56,15 +58,16 @@ unsigned short sii164GetDeviceID(void)
 {
unsigned short deviceID;
 
-   deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_DEVICE_ID_HIGH) << 8) |
-   (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_DEVICE_ID_LOW);
+   deviceID = (i2c_read_reg(SII164_DEVICE_ID_HIGH) << 8) |
+   i2c_read_reg(SII164_DEVICE_ID_LOW);
 
return deviceID;
 }
 
-
-
-/* DVI.C will handle all SiI164 chip stuffs and try it best to make code 
minimal and useful */
+/*
+ * DVI.C will handle all SiI164 chip stuffs and try it best to make code 
minimal
+ * and useful
+ */
 
 /*
  *  sii164InitChip
@@ -72,10 +75,10 @@ unsigned short sii164GetDeviceID(void)
  *
  *  Input:
  *  edgeSelect  - Edge Select:
- *  0 = Input data is falling edge latched 
(falling edge
- *  latched first in dual edge mode)
- *  1 = Input data is rising edge latched (rising 
edge
- *  latched first in dual edge mode)
+ *  0 = Input data is falling edge latched (falling
+ *  edge latched first in dual edge mode)
+ *  1 = Input data is rising edge latched (rising
+ *  edge latched first in dual edge mode)
  *  busSelect   - Input Bus Select:
  *  0 = Input data bus is 12-bits wide
  *  1 = Input data bus is 24-bits wide
@@ -135,7 +138,8 @@ long sii164InitChip(unsigned char edgeSelect,
 #endif
 
/* Check if SII164 Chip exists */
-   if ((sii164GetVendorID() == SII164_VENDOR_ID) && (sii164GetDeviceID() 
== SII164_DEVICE_ID)) {
+   if ((sii164GetVendorID() == SII164_VENDOR_ID) &&
+   (sii164GetDeviceID() == SII164_DEVICE_ID)) {
/*
 *  Initialize SII164 controller chip.
 */
@@ -170,7 +174,7 @@ long sii164InitChip(unsigned char edgeSelect,
else
config |= SII164_CONFIGURATION_VSYNC_AS_IS;
 
-   i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+   i2c_write_reg(SII164_CONFIGURATION, config);
 
/*
 * De-skew enabled with default 111b value.
@@ -208,7 +212,7 @@ long sii164InitChip(unsigned char edgeSelect,
config |= SII164_DESKEW_8_STEP;
break;
}
-   i2cWriteReg(SII164_I2C_ADDRESS, SII164_DESKEW, config);
+   i2c_write_reg(SII164_DESKEW, config);
 
/* Enable/Disable Continuous Sync. */
if (continuousSyncEnable == 0)
@@ -225,12 +229,12 @@ long sii164InitChip(unsigned char edgeSelect,
/* Set the PLL Filter value */
config |= ((pllFilterValue & 0x07) << 1);
 
-   i2cWriteReg(SII164_I2C_ADDRESS, SII164_PLL, config);
+   i2c_write_reg(SII164_PLL, config);
 
/* Recover from Power Down and enable output. */
-   config = 

[PATCH v2] Staging: sm750fb: Fix coding style issue in ddk750_sii164.c

[Jeremy Lacomis]

This is a patch to the ddk750_sii164.c file that fixes line length warnings
found by the checkpatch.pl script

Signed-off-by: Jeremy Lacomis 
---
Changes in v2:
- Change definition of i2cReadReg/i2cWriteReg
- Remove unnecessary casts

 drivers/staging/sm750fb/ddk750_sii164.c | 87 -
 1 file changed, 43 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
b/drivers/staging/sm750fb/ddk750_sii164.c
index c787a74c4f9c..66092c4645fc 100644
--- a/drivers/staging/sm750fb/ddk750_sii164.c
+++ b/drivers/staging/sm750fb/ddk750_sii164.c
@@ -12,11 +12,13 @@
 #define USE_HW_I2C
 
 #ifdef USE_HW_I2C
-#define i2cWriteReg sm750_hw_i2c_write_reg
-#define i2cReadReg  sm750_hw_i2c_read_reg
+#define i2c_write_reg(reg, config) sm750_hw_i2c_write_reg(SII164_I2C_ADDRESS, \
+ reg, config)
+#define i2c_read_reg(reg) sm750_hw_i2c_read_reg(SII164_I2C_ADDRESS, reg)
 #else
-#define i2cWriteReg sm750_sw_i2c_write_reg
-#define i2cReadReg  sm750_sw_i2c_read_reg
+#define i2c_write_reg(reg, config) sm750_sw_i2c_write_reg(SII164_I2C_ADDRESS, \
+ reg, config)
+#define i2c_read_reg(reg) sm750_sw_i2c_read_reg(SII164_I2C_ADDRESS, reg)
 #endif
 
 /* SII164 Vendor and Device ID */
@@ -39,8 +41,8 @@ unsigned short sii164GetVendorID(void)
 {
unsigned short vendorID;
 
-   vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_VENDOR_ID_HIGH) << 8) |
-   (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_VENDOR_ID_LOW);
+   vendorID = (i2c_read_reg(SII164_VENDOR_ID_HIGH) << 8) |
+   i2c_read_reg(SII164_VENDOR_ID_LOW);
 
return vendorID;
 }
@@ -56,15 +58,16 @@ unsigned short sii164GetDeviceID(void)
 {
unsigned short deviceID;
 
-   deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_DEVICE_ID_HIGH) << 8) |
-   (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
SII164_DEVICE_ID_LOW);
+   deviceID = (i2c_read_reg(SII164_DEVICE_ID_HIGH) << 8) |
+   i2c_read_reg(SII164_DEVICE_ID_LOW);
 
return deviceID;
 }
 
-
-
-/* DVI.C will handle all SiI164 chip stuffs and try it best to make code 
minimal and useful */
+/*
+ * DVI.C will handle all SiI164 chip stuffs and try it best to make code 
minimal
+ * and useful
+ */
 
 /*
  *  sii164InitChip
@@ -72,10 +75,10 @@ unsigned short sii164GetDeviceID(void)
  *
  *  Input:
  *  edgeSelect  - Edge Select:
- *  0 = Input data is falling edge latched 
(falling edge
- *  latched first in dual edge mode)
- *  1 = Input data is rising edge latched (rising 
edge
- *  latched first in dual edge mode)
+ *  0 = Input data is falling edge latched (falling
+ *  edge latched first in dual edge mode)
+ *  1 = Input data is rising edge latched (rising
+ *  edge latched first in dual edge mode)
  *  busSelect   - Input Bus Select:
  *  0 = Input data bus is 12-bits wide
  *  1 = Input data bus is 24-bits wide
@@ -135,7 +138,8 @@ long sii164InitChip(unsigned char edgeSelect,
 #endif
 
/* Check if SII164 Chip exists */
-   if ((sii164GetVendorID() == SII164_VENDOR_ID) && (sii164GetDeviceID() 
== SII164_DEVICE_ID)) {
+   if ((sii164GetVendorID() == SII164_VENDOR_ID) &&
+   (sii164GetDeviceID() == SII164_DEVICE_ID)) {
/*
 *  Initialize SII164 controller chip.
 */
@@ -170,7 +174,7 @@ long sii164InitChip(unsigned char edgeSelect,
else
config |= SII164_CONFIGURATION_VSYNC_AS_IS;
 
-   i2cWriteReg(SII164_I2C_ADDRESS, SII164_CONFIGURATION, config);
+   i2c_write_reg(SII164_CONFIGURATION, config);
 
/*
 * De-skew enabled with default 111b value.
@@ -208,7 +212,7 @@ long sii164InitChip(unsigned char edgeSelect,
config |= SII164_DESKEW_8_STEP;
break;
}
-   i2cWriteReg(SII164_I2C_ADDRESS, SII164_DESKEW, config);
+   i2c_write_reg(SII164_DESKEW, config);
 
/* Enable/Disable Continuous Sync. */
if (continuousSyncEnable == 0)
@@ -225,12 +229,12 @@ long sii164InitChip(unsigned char edgeSelect,
/* Set the PLL Filter value */
config |= ((pllFilterValue & 0x07) << 1);
 
-   i2cWriteReg(SII164_I2C_ADDRESS, SII164_PLL, config);
+   i2c_write_reg(SII164_PLL, config);
 
/* Recover from Power Down and enable output. */
-   config = 

Re: [PATCH] Input: elantech - add new icbody type 15

[Dmitry Torokhov]

On Sun, Nov 19, 2017 at 10:56:47PM +0800, Aaron Ma wrote:
> The touchpad of Lenovo Thinkpad L480 reports it's version as 15.
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Aaron Ma 

Applied, thank you.

> ---
>  drivers/input/mouse/elantech.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index b84cd978fce2..a4aaa748e987 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1613,7 +1613,7 @@ static int elantech_set_properties(struct elantech_data 
> *etd)
>   case 5:
>   etd->hw_version = 3;
>   break;
> - case 6 ... 14:
> + case 6 ... 15:
>   etd->hw_version = 4;
>   break;
>   default:
> -- 
> 2.13.6
> 

-- 
Dmitry

Re: [PATCH] Input: elantech - add new icbody type 15

[Dmitry Torokhov]

On Sun, Nov 19, 2017 at 10:56:47PM +0800, Aaron Ma wrote:
> The touchpad of Lenovo Thinkpad L480 reports it's version as 15.
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Aaron Ma 

Applied, thank you.

> ---
>  drivers/input/mouse/elantech.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index b84cd978fce2..a4aaa748e987 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1613,7 +1613,7 @@ static int elantech_set_properties(struct elantech_data 
> *etd)
>   case 5:
>   etd->hw_version = 3;
>   break;
> - case 6 ... 14:
> + case 6 ... 15:
>   etd->hw_version = 4;
>   break;
>   default:
> -- 
> 2.13.6
> 

-- 
Dmitry

Re: [PATCH 2/2] staging: rtl8188eu: Fix private WEXT IOCTL calls

[Ishraq Ibne Ashraf]

Hi,

What was broken was private/device specific IOCTL calls implemented by this 
driver. The standard IOCTL calls worked and the driver worked as it was in 
client mode.

But in AP mode with hostapd (https://w1.fi/hostapd/) the rtl871xdrv driver of 
hostapd (which is required for using devices that use this driver in AP mode) 
invokes private/device specific IOCTL calls and in the existing driver they 
could only be invoked through the ndo_do_ioctl interface of the driver. Support 
for which was removed in the mentioned commit by Johannes Berg. Hence the 
driver stopped working in AP mode with hostapd using rtl871xdrv driver.

The solution was to implement equivalent versions of the existing 
private/device specific IOCTLs which will be invoked by the iw_handler_def 
interface.


Cheers,
Ishraq


On 11/25/2017 05:40 AM, Dan Carpenter wrote:
> I added Johannes to the CC list again because this is sort of his
> fault...  To be honest, I'm a little bit puzzled.  I would have thought
> Johannes's change would have made some code unused and that we could
> delete it now.  I haven't looked at this.

Re: [PATCH 2/2] staging: rtl8188eu: Fix private WEXT IOCTL calls

[Ishraq Ibne Ashraf]

Hi,

What was broken was private/device specific IOCTL calls implemented by this 
driver. The standard IOCTL calls worked and the driver worked as it was in 
client mode.

But in AP mode with hostapd (https://w1.fi/hostapd/) the rtl871xdrv driver of 
hostapd (which is required for using devices that use this driver in AP mode) 
invokes private/device specific IOCTL calls and in the existing driver they 
could only be invoked through the ndo_do_ioctl interface of the driver. Support 
for which was removed in the mentioned commit by Johannes Berg. Hence the 
driver stopped working in AP mode with hostapd using rtl871xdrv driver.

The solution was to implement equivalent versions of the existing 
private/device specific IOCTLs which will be invoked by the iw_handler_def 
interface.


Cheers,
Ishraq


On 11/25/2017 05:40 AM, Dan Carpenter wrote:
> I added Johannes to the CC list again because this is sort of his
> fault...  To be honest, I'm a little bit puzzled.  I would have thought
> Johannes's change would have made some code unused and that we could
> delete it now.  I haven't looked at this.

Re: [PATCH] input: pegasus_notetaker: add license information

[Dmitry Torokhov]

Hi Martin,

On Sat, Nov 18, 2017 at 09:45:18AM +0100, Martin Kepplinger wrote:
> This adds an SPDX license identifier to this driver I wrote some time back.
> 
> Signed-off-by: Martin Kepplinger 
> ---
>  drivers/input/tablet/pegasus_notetaker.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/tablet/pegasus_notetaker.c 
> b/drivers/input/tablet/pegasus_notetaker.c
> index 47de5a81172f..cdf75c989469 100644
> --- a/drivers/input/tablet/pegasus_notetaker.c
> +++ b/drivers/input/tablet/pegasus_notetaker.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0

Should this be GPL-2.0+? The MODULE_LICENSE specifies that the module is
"GPL" which in kernel land means GPLv2+. Or we should change the module
license to strict "GPLv2"?

Also, why do we use C++ -style comments for this?

Greg, do you have any plans on dropping MODULE_LICENSE() altogether and
generating the appropriate string from SPDX markings in the source?
Doing this would prevent mismatches between license notices, SPDX tags
and MODULE_LCENSE() strings, which happen very often.

>  /*
>   * Pegasus Mobile Notetaker Pen input tablet driver
>   *

Thanks.

-- 
Dmitry

Re: [PATCH] input: pegasus_notetaker: add license information

[Dmitry Torokhov]

Hi Martin,

On Sat, Nov 18, 2017 at 09:45:18AM +0100, Martin Kepplinger wrote:
> This adds an SPDX license identifier to this driver I wrote some time back.
> 
> Signed-off-by: Martin Kepplinger 
> ---
>  drivers/input/tablet/pegasus_notetaker.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/tablet/pegasus_notetaker.c 
> b/drivers/input/tablet/pegasus_notetaker.c
> index 47de5a81172f..cdf75c989469 100644
> --- a/drivers/input/tablet/pegasus_notetaker.c
> +++ b/drivers/input/tablet/pegasus_notetaker.c
> @@ -1,3 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0

Should this be GPL-2.0+? The MODULE_LICENSE specifies that the module is
"GPL" which in kernel land means GPLv2+. Or we should change the module
license to strict "GPLv2"?

Also, why do we use C++ -style comments for this?

Greg, do you have any plans on dropping MODULE_LICENSE() altogether and
generating the appropriate string from SPDX markings in the source?
Doing this would prevent mismatches between license notices, SPDX tags
and MODULE_LCENSE() strings, which happen very often.

>  /*
>   * Pegasus Mobile Notetaker Pen input tablet driver
>   *

Thanks.

-- 
Dmitry

[PATCH] rtc: Remove unused RTC_DEVICE_NAME_SIZE

[Cole Robinson]

The last usage was removed in 5c82a6ae0 when rtc_device.name
was removed

Signed-off-by: Cole Robinson 
---
Just noticed this when looking at commits a few months back...

 include/linux/rtc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 41319a2e409b..fc6c90b57be0 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -87,7 +87,6 @@ struct rtc_class_ops {
int (*set_offset)(struct device *, long offset);
 };
 
-#define RTC_DEVICE_NAME_SIZE 20
 typedef struct rtc_task {
void (*func)(void *private_data);
void *private_data;
-- 
2.14.3

[PATCH] rtc: Remove unused RTC_DEVICE_NAME_SIZE

[Cole Robinson]

The last usage was removed in 5c82a6ae0 when rtc_device.name
was removed

Signed-off-by: Cole Robinson 
---
Just noticed this when looking at commits a few months back...

 include/linux/rtc.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 41319a2e409b..fc6c90b57be0 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -87,7 +87,6 @@ struct rtc_class_ops {
int (*set_offset)(struct device *, long offset);
 };
 
-#define RTC_DEVICE_NAME_SIZE 20
 typedef struct rtc_task {
void (*func)(void *private_data);
void *private_data;
-- 
2.14.3

Re: [PATCH] ASoC: fsl_ssi: Override bit clock rate based on slot number

[Nicolin Chen]

On Sat, Nov 25, 2017 at 11:29:48PM +0100, Lukasz Majewski wrote:

> Nicolin, do you know what happened with this patch? I couldn't find it
> in current linux/master.
> 
> Has it been applied to any asoc tree for being upstreamed?

A similar patch with an updated subject got applied:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20171124=b0a7043d5c2ccdd306959f295bf1a62be025cbf5

Re: [PATCH] ASoC: fsl_ssi: Override bit clock rate based on slot number

[Nicolin Chen]

On Sat, Nov 25, 2017 at 11:29:48PM +0100, Lukasz Majewski wrote:

> Nicolin, do you know what happened with this patch? I couldn't find it
> in current linux/master.
> 
> Has it been applied to any asoc tree for being upstreamed?

A similar patch with an updated subject got applied:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20171124=b0a7043d5c2ccdd306959f295bf1a62be025cbf5

Re: [PATCH 42/43] x86/mm/kaiser: Allow KAISER to be enabled/disabled at runtime

[Andy Lutomirski]

On Sat, Nov 25, 2017 at 2:48 PM, Thomas Gleixner  wrote:
> On Sat, 25 Nov 2017, Andy Lutomirski wrote:
>> > On Nov 25, 2017, at 1:05 PM, Thomas Gleixner  wrote:
>> > On Sat, 25 Nov 2017, Andy Lutomirski wrote:
>> >> Keep in mind that, for a static_branch, actually setting the thing needs
>> >> to be deferred, but that's straightforward.
>> >
>> > That's not an issue during boot. That would be an issue for a run time
>> > switch.
>>
>> What I mean is: if you modify a static_branch too early, it blows up 
>> terribly.
>
> I'm aware of that. We can't switch it in the early boot stage. But that
> does not matter as we can switch way before we reach user space.
>
> The early kaiser mappings are fine whether we use them later or not. At the
> point in boot where we actually make the decision, there is nothing more
> than the extra 4k shadow which got initialized.
>
> If we ever want to do runtime switching, then the full shadow mapping needs
> to be maintained even while kaiser is disabled, just the NX poisoning of
> the user space mappings is what makes the difference.

One unfortunate thing is that, if we boot with kaiser off and don't
intend to ever switch it on at runtime, we could avoid the 8k pgd
allocations.  But if we want to be able to enable kaiser, we need the
8k mappings.  My inclination is to not try for runtime control until
some distro asks for it.

In general, I think that trying to runtime switch without stop_machine
is a bit nuts, and getting it to be reliable even with stop_machine is
gross.  Not to mention that stop_machine is currently incompatible
with writing to static branches, although that's fixable.

--Andy

Re: [PATCH 42/43] x86/mm/kaiser: Allow KAISER to be enabled/disabled at runtime

[Andy Lutomirski]

On Sat, Nov 25, 2017 at 2:48 PM, Thomas Gleixner  wrote:
> On Sat, 25 Nov 2017, Andy Lutomirski wrote:
>> > On Nov 25, 2017, at 1:05 PM, Thomas Gleixner  wrote:
>> > On Sat, 25 Nov 2017, Andy Lutomirski wrote:
>> >> Keep in mind that, for a static_branch, actually setting the thing needs
>> >> to be deferred, but that's straightforward.
>> >
>> > That's not an issue during boot. That would be an issue for a run time
>> > switch.
>>
>> What I mean is: if you modify a static_branch too early, it blows up 
>> terribly.
>
> I'm aware of that. We can't switch it in the early boot stage. But that
> does not matter as we can switch way before we reach user space.
>
> The early kaiser mappings are fine whether we use them later or not. At the
> point in boot where we actually make the decision, there is nothing more
> than the extra 4k shadow which got initialized.
>
> If we ever want to do runtime switching, then the full shadow mapping needs
> to be maintained even while kaiser is disabled, just the NX poisoning of
> the user space mappings is what makes the difference.

One unfortunate thing is that, if we boot with kaiser off and don't
intend to ever switch it on at runtime, we could avoid the 8k pgd
allocations.  But if we want to be able to enable kaiser, we need the
8k mappings.  My inclination is to not try for runtime control until
some distro asks for it.

In general, I think that trying to runtime switch without stop_machine
is a bit nuts, and getting it to be reliable even with stop_machine is
gross.  Not to mention that stop_machine is currently incompatible
with writing to static branches, although that's fixable.

--Andy

Re: [PATCH] Support TrackStick of Thinkpad L570

[Dmitry Torokhov]

On Mon, Nov 20, 2017 at 04:55:30PM +0900, Masaki Ota wrote:
> From: Masaki Ota 
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.
> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota 
> ---
>  drivers/input/mouse/alps.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..cce52104ed5a 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,28 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + /* For support TrackStick of Thinkpad L570 device */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse, 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);

Should we execute this sequence only if is_dual is false above?

> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2585,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> -- 
> 2.14.1
> 

Thanks.

-- 
Dmitry

Re: [PATCH] Support TrackStick of Thinkpad L570

[Dmitry Torokhov]

On Mon, Nov 20, 2017 at 04:55:30PM +0900, Masaki Ota wrote:
> From: Masaki Ota 
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.
> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota 
> ---
>  drivers/input/mouse/alps.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..cce52104ed5a 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,28 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + /* For support TrackStick of Thinkpad L570 device */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse, 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);

Should we execute this sequence only if is_dual is false above?

> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2585,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> -- 
> 2.14.1
> 

Thanks.

-- 
Dmitry

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Andy Lutomirski]

Can you send me whatever config and exact commit hash generated this?
I can try to figure out why it failed.

On Sat, Nov 25, 2017 at 3:13 PM, Thomas Gleixner  wrote:
> On Sat, 25 Nov 2017, Andy Lutomirski wrote:
>
>> On Sat, Nov 25, 2017 at 9:28 AM, Andy Lutomirski  wrote:
>> > If we overflow the stack into a guard page and then try to unwind
>> > it with ORC, it should work perfectly: by construction, there can't
>> > be any meaningful data in the guard page because no writes to the
>> > guard page will have succeeded.
>> >
>> > ORC seems entirely capable of unwinding in this situation, except
>> > that it doesn't even try.  Adjust its initial stack check so that
>> > it's willing to try unwinding.
>> >
>> > I tested this by intentionally overflowing the task stack.  The
>> > result is an accurate call trace instead of a trace consisting
>> > purely of '?' entries.
>> >
>> > Signed-off-by: Andy Lutomirski 
>> > ---
>> >
>> > Hi all-
>> >
>> > Ingo, this would have fixed half the debugging problem you had, I think.
>> > To really nail it, we'd want some kind of magic to annotate the trace
>> > so that page_fault (and async_page_fault) entries show CR2 and error_code.
>> >
>> > Josh, any ideas of how to do that cleanly?  We could easily hard-code it
>> > in the OOPS unwinder, I guess.
>>
>> Actually, this does pretty well.  We don't get CR2, but, when I added
>> an intentional bug kind of along the lines of the one you debugged,
>> the intermediate page fault successfully dumps all the regs in the
>> stack trace, so we get the faulting instruction *and* the registers.
>> We also get ORIG_RAX, which tells us the error code.  We could be
>> fancy and decode that.
>
> It works in general, but for that case it's not much better than before
> vs. the '?' entries.
>
> Thanks,
>
> tglx
>
> [2.556065] PANIC: double fault, error_code: 0x0
> [2.557116] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
> 4.14.0-01256-g03dea81fe9f2-dirty #30
> [2.558930] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.10.2-1 04/01/2014
> [2.560133] task: 880428dd8000 task.stack: c900025fc000
> [2.560729] RIP: 0010:page_fault+0x11/0x60
> [2.561122] RSP: :ff083fc8 EFLAGS: 00010046
> [2.561607] RAX: 819d0ac7 RBX: 0001 RCX: 
> 819d0ac7
> [2.562357] RDX:  RSI: 0010 RDI: 
> ff084078
> [2.563027] RBP: 000b R08:  R09: 
> 0040
> [2.563726] R10: 0018 R11: 0246 R12: 
> 0003
> [2.564429] R13: 55719fd7d410 R14:  R15: 
> 03938700
> [2.565104] FS:  7f9edc0b78c0() GS:88042e44() 
> knlGS:
> [2.565844] CS:  0010 DS:  ES:  CR0: 80050033
> [2.566396] CR2: ff083fb8 CR3: 000428ec4005 CR4: 
> 001606e0
> [2.567097] DR0:  DR1:  DR2: 
> 
> [2.567761] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [2.568451] Call Trace:
> [2.568704]  
> [2.568950]  ? __do_page_fault+0x4b0/0x4b0
> [2.569348]  ? page_fault+0x2c/0x60
> [2.569680]  ? native_iret+0x7/0x7
> [2.570019]  ? __do_page_fault+0x4b0/0x4b0
> [2.570396]  ? page_fault+0x2c/0x60
> [2.570743]  ? call_function_interrupt+0xc0/0xc0
> [2.571199]  
> [2.571422] Code: ff e8 34 b7 6a ff e9 9f 02 00 00 0f 1f 44 00 00 66 2e 0f 
> 1f 84 00 00 00 00 00 0f 1f 00 48 83 c4 88 f6 84 24 88 00 00 00 03 75 20  
> 4a 01 00 00 48 89 e7 48 8b 74 24 78 48 c7 44 24 78 ff ff ff
> [2.573192] Kernel panic - not syncing: Machine halted.
> [2.573694] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
> 4.14.0-01256-g03dea81fe9f2-dirty #30
> [2.574528] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.10.2-1 04/01/2014
> [2.575330] Call Trace:
> [2.575570]  <#DF>
> [2.575760]  dump_stack+0x46/0x59
> [2.576120]  panic+0xde/0x223
> [2.576405]  df_debug+0x29/0x30
> [2.576687]  do_double_fault+0x9a/0x120
> [2.577057]  double_fault+0x22/0x30
> [2.577376] RIP: 0010:page_fault+0x11/0x60
> [2.55] RSP: :ff083fc8 EFLAGS: 00010046
> [2.578314] RAX: 819d0ac7 RBX: 0001 RCX: 
> 819d0ac7
> [2.578979] RDX:  RSI: 0010 RDI: 
> ff084078
> [2.579666] RBP: 000b R08:  R09: 
> 0040
> [2.580334] R10: 0018 R11: 0246 R12: 
> 0003
> [2.581008] R13: 55719fd7d410 R14:  R15: 
> 03938700
> [2.581684]  ? native_iret+0x7/0x7
> [2.582007] WARNING: can't dereference iret registers at ff084048 
> for ip page_fault+0x11/0x60
> [2.582008]  
> [2.583134]  
> [2.583367]  ? 

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Andy Lutomirski]

Can you send me whatever config and exact commit hash generated this?
I can try to figure out why it failed.

On Sat, Nov 25, 2017 at 3:13 PM, Thomas Gleixner  wrote:
> On Sat, 25 Nov 2017, Andy Lutomirski wrote:
>
>> On Sat, Nov 25, 2017 at 9:28 AM, Andy Lutomirski  wrote:
>> > If we overflow the stack into a guard page and then try to unwind
>> > it with ORC, it should work perfectly: by construction, there can't
>> > be any meaningful data in the guard page because no writes to the
>> > guard page will have succeeded.
>> >
>> > ORC seems entirely capable of unwinding in this situation, except
>> > that it doesn't even try.  Adjust its initial stack check so that
>> > it's willing to try unwinding.
>> >
>> > I tested this by intentionally overflowing the task stack.  The
>> > result is an accurate call trace instead of a trace consisting
>> > purely of '?' entries.
>> >
>> > Signed-off-by: Andy Lutomirski 
>> > ---
>> >
>> > Hi all-
>> >
>> > Ingo, this would have fixed half the debugging problem you had, I think.
>> > To really nail it, we'd want some kind of magic to annotate the trace
>> > so that page_fault (and async_page_fault) entries show CR2 and error_code.
>> >
>> > Josh, any ideas of how to do that cleanly?  We could easily hard-code it
>> > in the OOPS unwinder, I guess.
>>
>> Actually, this does pretty well.  We don't get CR2, but, when I added
>> an intentional bug kind of along the lines of the one you debugged,
>> the intermediate page fault successfully dumps all the regs in the
>> stack trace, so we get the faulting instruction *and* the registers.
>> We also get ORIG_RAX, which tells us the error code.  We could be
>> fancy and decode that.
>
> It works in general, but for that case it's not much better than before
> vs. the '?' entries.
>
> Thanks,
>
> tglx
>
> [2.556065] PANIC: double fault, error_code: 0x0
> [2.557116] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
> 4.14.0-01256-g03dea81fe9f2-dirty #30
> [2.558930] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.10.2-1 04/01/2014
> [2.560133] task: 880428dd8000 task.stack: c900025fc000
> [2.560729] RIP: 0010:page_fault+0x11/0x60
> [2.561122] RSP: :ff083fc8 EFLAGS: 00010046
> [2.561607] RAX: 819d0ac7 RBX: 0001 RCX: 
> 819d0ac7
> [2.562357] RDX:  RSI: 0010 RDI: 
> ff084078
> [2.563027] RBP: 000b R08:  R09: 
> 0040
> [2.563726] R10: 0018 R11: 0246 R12: 
> 0003
> [2.564429] R13: 55719fd7d410 R14:  R15: 
> 03938700
> [2.565104] FS:  7f9edc0b78c0() GS:88042e44() 
> knlGS:
> [2.565844] CS:  0010 DS:  ES:  CR0: 80050033
> [2.566396] CR2: ff083fb8 CR3: 000428ec4005 CR4: 
> 001606e0
> [2.567097] DR0:  DR1:  DR2: 
> 
> [2.567761] DR3:  DR6: fffe0ff0 DR7: 
> 0400
> [2.568451] Call Trace:
> [2.568704]  
> [2.568950]  ? __do_page_fault+0x4b0/0x4b0
> [2.569348]  ? page_fault+0x2c/0x60
> [2.569680]  ? native_iret+0x7/0x7
> [2.570019]  ? __do_page_fault+0x4b0/0x4b0
> [2.570396]  ? page_fault+0x2c/0x60
> [2.570743]  ? call_function_interrupt+0xc0/0xc0
> [2.571199]  
> [2.571422] Code: ff e8 34 b7 6a ff e9 9f 02 00 00 0f 1f 44 00 00 66 2e 0f 
> 1f 84 00 00 00 00 00 0f 1f 00 48 83 c4 88 f6 84 24 88 00 00 00 03 75 20  
> 4a 01 00 00 48 89 e7 48 8b 74 24 78 48 c7 44 24 78 ff ff ff
> [2.573192] Kernel panic - not syncing: Machine halted.
> [2.573694] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
> 4.14.0-01256-g03dea81fe9f2-dirty #30
> [2.574528] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
> 1.10.2-1 04/01/2014
> [2.575330] Call Trace:
> [2.575570]  <#DF>
> [2.575760]  dump_stack+0x46/0x59
> [2.576120]  panic+0xde/0x223
> [2.576405]  df_debug+0x29/0x30
> [2.576687]  do_double_fault+0x9a/0x120
> [2.577057]  double_fault+0x22/0x30
> [2.577376] RIP: 0010:page_fault+0x11/0x60
> [2.55] RSP: :ff083fc8 EFLAGS: 00010046
> [2.578314] RAX: 819d0ac7 RBX: 0001 RCX: 
> 819d0ac7
> [2.578979] RDX:  RSI: 0010 RDI: 
> ff084078
> [2.579666] RBP: 000b R08:  R09: 
> 0040
> [2.580334] R10: 0018 R11: 0246 R12: 
> 0003
> [2.581008] R13: 55719fd7d410 R14:  R15: 
> 03938700
> [2.581684]  ? native_iret+0x7/0x7
> [2.582007] WARNING: can't dereference iret registers at ff084048 
> for ip page_fault+0x11/0x60
> [2.582008]  
> [2.583134]  
> [2.583367]  ? __do_page_fault+0x4b0/0x4b0
> [2.583751]  ? page_fault+0x2c/0x60

Re: [PATCH] Support TrackStick of Thinkpad L570

[Dmitry Torokhov]

Hi,

On Mon, Nov 20, 2017 at 04:55:30PM +0900, Masaki Ota wrote:
> From: Masaki Ota 
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.

Is it custom protocol over SMBus/I2C or HID (via i2c-hid)? If it is
custom protocol, can we add the proper driver to the kernel for it so we
can get away from the PS/2 emulation in firmware?

Thanks!

> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota 
> ---
>  drivers/input/mouse/alps.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..cce52104ed5a 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,28 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + /* For support TrackStick of Thinkpad L570 device */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse, 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2585,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> -- 
> 2.14.1
> 

-- 
Dmitry

Re: [PATCH] Support TrackStick of Thinkpad L570

[Dmitry Torokhov]

Hi,

On Mon, Nov 20, 2017 at 04:55:30PM +0900, Masaki Ota wrote:
> From: Masaki Ota 
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.

Is it custom protocol over SMBus/I2C or HID (via i2c-hid)? If it is
custom protocol, can we add the proper driver to the kernel for it so we
can get away from the PS/2 emulation in firmware?

Thanks!

> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota 
> ---
>  drivers/input/mouse/alps.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..cce52104ed5a 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,28 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + /* For support TrackStick of Thinkpad L570 device */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse, 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2585,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> -- 
> 2.14.1
> 

-- 
Dmitry

Re: [PATCH v2] powerpc: fix boot on BOOK3S_32 with CONFIG_STRICT_KERNEL_RWX

[Balbir Singh]

On Thu, Nov 23, 2017 at 11:04 PM, Michael Ellerman  wrote:
> Christophe LEROY  writes:
>> Le 22/11/2017 à 12:48, Michael Ellerman a écrit :
>>> Christophe LEROY  writes:
 Le 22/11/2017 à 00:07, Balbir Singh a écrit :
> On Wed, Nov 22, 2017 at 1:28 AM, Christophe Leroy
>  wrote:
>> On powerpc32, patch_instruction() is called by apply_feature_fixups()
>> which is called from early_init()
> ...
>> diff --git a/arch/powerpc/lib/code-patching.c 
>> b/arch/powerpc/lib/code-patching.c
>> index c9de03e0c1f1..d469224c4ada 100644
>> --- a/arch/powerpc/lib/code-patching.c
>> +++ b/arch/powerpc/lib/code-patching.c
>> @@ -146,11 +147,8 @@ int patch_instruction(unsigned int *addr, unsigned 
>> int instr)
>>* During early early boot patch_instruction is called
>>* when text_poke_area is not ready, but we still need
>>* to allow patching. We just do the plain old patching
>> -* We use slab_is_available and per cpu read * via this_cpu_read
>> -* of text_poke_area. Per-CPU areas might not be up early
>> -* this can create problems with just using this_cpu_read()
>>*/
>> -   if (!slab_is_available() || !this_cpu_read(text_poke_area))
>> +   if (!this_cpu_read(*PTRRELOC(_poke_area)))
>>   return __patch_instruction(addr, instr);
>
> On ppc64, we call apply_feature_fixups() in early_setup() after we've
> relocated ourselves. Sorry for missing the ppc32 case. I would like to
> avoid PTRRELOC when unnecessary.

 What do you suggest then ?

 Some #ifdef PPC32 around that ?
>>>
>>> No I don't think that improves anything.
>>>
>>> I think the comment about per-cpu not being up is wrong, you'll just get
>>> the static version of text_poke_area, which should be NULL. So we don't
>>> need the slab_available() check anyway.
>>>
>>> So I'll take this as-is.
>>>
>>> Having said that I absolutely hate PTRRELOC, so if it starts spreading
>>> we will have to come up with something less bug prone.
>>
>> Would something like that be the solution ?
>
> I don't love that actual patch, there's a lot of churn just for one
> flag.
>
> But the idea is not so bad.
>
> In fact I don't think we ever need to use the text_poke_area when we
> call do_feature_fixups().
>
> Most of the calls are in early boot.
>
> The exception is for modules, but when we do the fixups *of the module*,
> the module text is not mapped read only yet.
>
> So I think we can just do something like below.
>
> cheers
>
>
> diff --git a/arch/powerpc/include/asm/code-patching.h 
> b/arch/powerpc/include/asm/code-patching.h
> index abef812de7f8..1090024e8519 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -31,6 +31,7 @@ unsigned int create_cond_branch(const unsigned int *addr,
> unsigned long target, int flags);
>  int patch_branch(unsigned int *addr, unsigned long target, int flags);
>  int patch_instruction(unsigned int *addr, unsigned int instr);
> +int raw_patch_instruction(unsigned int *addr, unsigned int instr);
>
>  int instr_is_relative_branch(unsigned int instr);
>  int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> diff --git a/arch/powerpc/lib/code-patching.c 
> b/arch/powerpc/lib/code-patching.c
> index d469224c4ada..d1eb24cbef58 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -23,7 +23,7 @@
>  #include 
>  #include 
>
> -static int __patch_instruction(unsigned int *addr, unsigned int instr)
> +int raw_patch_instruction(unsigned int *addr, unsigned int instr)
>  {
> int err;
>
> @@ -148,8 +148,8 @@ int patch_instruction(unsigned int *addr, unsigned int 
> instr)
>  * when text_poke_area is not ready, but we still need
>  * to allow patching. We just do the plain old patching
>  */
> -   if (!this_cpu_read(*PTRRELOC(_poke_area)))
> -   return __patch_instruction(addr, instr);
> +   if (!this_cpu_read(text_poke_area))
> +   return raw_patch_instruction(addr, instr);
>
> local_irq_save(flags);
>
> @@ -184,7 +184,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
> instr)
>
>  int patch_instruction(unsigned int *addr, unsigned int instr)
>  {
> -   return __patch_instruction(addr, instr);
> +   return raw_patch_instruction(addr, instr);
>  }
>
>  #endif /* CONFIG_STRICT_KERNEL_RWX */
> diff --git a/arch/powerpc/lib/feature-fixups.c 
> b/arch/powerpc/lib/feature-fixups.c
> index 41cf5ae273cf..0872d60ede10 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -62,7 +62,7 @@ static int patch_alt_instruction(unsigned int *src, 
> unsigned int *dest,
> }
> 

Re: [PATCH v2] powerpc: fix boot on BOOK3S_32 with CONFIG_STRICT_KERNEL_RWX

[Balbir Singh]

On Thu, Nov 23, 2017 at 11:04 PM, Michael Ellerman  wrote:
> Christophe LEROY  writes:
>> Le 22/11/2017 à 12:48, Michael Ellerman a écrit :
>>> Christophe LEROY  writes:
 Le 22/11/2017 à 00:07, Balbir Singh a écrit :
> On Wed, Nov 22, 2017 at 1:28 AM, Christophe Leroy
>  wrote:
>> On powerpc32, patch_instruction() is called by apply_feature_fixups()
>> which is called from early_init()
> ...
>> diff --git a/arch/powerpc/lib/code-patching.c 
>> b/arch/powerpc/lib/code-patching.c
>> index c9de03e0c1f1..d469224c4ada 100644
>> --- a/arch/powerpc/lib/code-patching.c
>> +++ b/arch/powerpc/lib/code-patching.c
>> @@ -146,11 +147,8 @@ int patch_instruction(unsigned int *addr, unsigned 
>> int instr)
>>* During early early boot patch_instruction is called
>>* when text_poke_area is not ready, but we still need
>>* to allow patching. We just do the plain old patching
>> -* We use slab_is_available and per cpu read * via this_cpu_read
>> -* of text_poke_area. Per-CPU areas might not be up early
>> -* this can create problems with just using this_cpu_read()
>>*/
>> -   if (!slab_is_available() || !this_cpu_read(text_poke_area))
>> +   if (!this_cpu_read(*PTRRELOC(_poke_area)))
>>   return __patch_instruction(addr, instr);
>
> On ppc64, we call apply_feature_fixups() in early_setup() after we've
> relocated ourselves. Sorry for missing the ppc32 case. I would like to
> avoid PTRRELOC when unnecessary.

 What do you suggest then ?

 Some #ifdef PPC32 around that ?
>>>
>>> No I don't think that improves anything.
>>>
>>> I think the comment about per-cpu not being up is wrong, you'll just get
>>> the static version of text_poke_area, which should be NULL. So we don't
>>> need the slab_available() check anyway.
>>>
>>> So I'll take this as-is.
>>>
>>> Having said that I absolutely hate PTRRELOC, so if it starts spreading
>>> we will have to come up with something less bug prone.
>>
>> Would something like that be the solution ?
>
> I don't love that actual patch, there's a lot of churn just for one
> flag.
>
> But the idea is not so bad.
>
> In fact I don't think we ever need to use the text_poke_area when we
> call do_feature_fixups().
>
> Most of the calls are in early boot.
>
> The exception is for modules, but when we do the fixups *of the module*,
> the module text is not mapped read only yet.
>
> So I think we can just do something like below.
>
> cheers
>
>
> diff --git a/arch/powerpc/include/asm/code-patching.h 
> b/arch/powerpc/include/asm/code-patching.h
> index abef812de7f8..1090024e8519 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -31,6 +31,7 @@ unsigned int create_cond_branch(const unsigned int *addr,
> unsigned long target, int flags);
>  int patch_branch(unsigned int *addr, unsigned long target, int flags);
>  int patch_instruction(unsigned int *addr, unsigned int instr);
> +int raw_patch_instruction(unsigned int *addr, unsigned int instr);
>
>  int instr_is_relative_branch(unsigned int instr);
>  int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
> diff --git a/arch/powerpc/lib/code-patching.c 
> b/arch/powerpc/lib/code-patching.c
> index d469224c4ada..d1eb24cbef58 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -23,7 +23,7 @@
>  #include 
>  #include 
>
> -static int __patch_instruction(unsigned int *addr, unsigned int instr)
> +int raw_patch_instruction(unsigned int *addr, unsigned int instr)
>  {
> int err;
>
> @@ -148,8 +148,8 @@ int patch_instruction(unsigned int *addr, unsigned int 
> instr)
>  * when text_poke_area is not ready, but we still need
>  * to allow patching. We just do the plain old patching
>  */
> -   if (!this_cpu_read(*PTRRELOC(_poke_area)))
> -   return __patch_instruction(addr, instr);
> +   if (!this_cpu_read(text_poke_area))
> +   return raw_patch_instruction(addr, instr);
>
> local_irq_save(flags);
>
> @@ -184,7 +184,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
> instr)
>
>  int patch_instruction(unsigned int *addr, unsigned int instr)
>  {
> -   return __patch_instruction(addr, instr);
> +   return raw_patch_instruction(addr, instr);
>  }
>
>  #endif /* CONFIG_STRICT_KERNEL_RWX */
> diff --git a/arch/powerpc/lib/feature-fixups.c 
> b/arch/powerpc/lib/feature-fixups.c
> index 41cf5ae273cf..0872d60ede10 100644
> --- a/arch/powerpc/lib/feature-fixups.c
> +++ b/arch/powerpc/lib/feature-fixups.c
> @@ -62,7 +62,7 @@ static int patch_alt_instruction(unsigned int *src, 
> unsigned int *dest,
> }
> }
>
> -   patch_instruction(dest, instr);
> +   raw_patch_instruction(dest, instr);
>
> 

Re: [patch V4 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses

[Philippe Ombredanne]

Pavel:

On Sat, Nov 25, 2017 at 7:51 PM, Pavel Machek  wrote:
> On Fri 2017-11-17 15:06:39, Mauro Carvalho Chehab wrote:
>> Hi Thomas,
>>
>> Em Fri, 17 Nov 2017 11:00:33 +0100 (CET)
>> Thomas Gleixner  escreveu:
>>
>> > Subject: Documentation: Add license-rules.rst to describe how to properly 
>> > identify file licenses
>> > From: Thomas Gleixner 
>> > Date: Fri, 10 Nov 2017 09:30:00 +0100
>> >
>> > Add a file to the Documentation directory to describe how file licenses
>> > should be described in all kernel files, using the SPDX identifier, as well
>> > as where all licenses should be in the kernel source tree for people to
>> > refer to (LICENSES/).
>> >
>> > Thanks to Kate, Greg and Jonathan for review and editing and Jonas for the
>> > suggestions concerning the meta tags in the licenses files.
>> >
>> > Signed-off-by: Thomas Gleixner 
>>
>> The document itself looks good, but I think it should also mention
>> what would be the expected values for the MODULE_LICENSE() macro and
>> how each license would be mapped into it.
>>
>> Right now, include/linux/module.h says:
>>
>> /*
>>  * The following license idents are currently accepted as indicating free
>>  * software modules
>>  *
>>  *"GPL"   [GNU Public License v2 or later]
>
> Hmm. AFAICT Greg translated GPL as GPL v1 or later. That seemed
> wrong... and now it seems even more wrong.

While this may come out as weird or wrong, this is neither wrong nor
"more wrong" when you dive in the details:

1. the meaning of a bare "GPL" in MODULE_LICENSE is well defined in
module.h as "GPL 2.0 or later" so there is no ambiguity there. It
would have been best to align this with SPDX, but this would break
instantly so many out of tree kernel modules and module loading tools
that expect these hard coded values and conventions that it is not
worth changing it IMHO.

2. the meaning of a bare "GPL" as a the only license notice is also
well defined in the GPL 2.0 text itself  in section 9 [1] and means
any version of the GPL that therefore can be made clear as GPL-1.0 or
later, i.e. GPL-1.+:
" If the Program does not specify a version number of this License,
you may choose any version ever published by the Free Software
Foundation. "

Therefore I do not think Greg did any translation and got anything
wrong but used exactly the convention in 2.

For instance when the only license notice in a file was a terse and
unclear: "Copyright (c) Jane Doe, GPL" or "Copyright (c) John Doe,
GPL'ed" then the resulting SPDX license id applied was  "GPL-1.0+"

I personally think this is unfortunate that we have warts like this:
it could have been the intent of author, or an oversight, or the
author may have meant 2.0 we can only guess! What is clear is that
in these cases and short of any other indication, "GPL-1.0+" is the
precise meaning that "GPL" or "GPL'ed" has in a notice outside of the
MODULE_LICENSE macro.

Note that no MODULE_LICENSE macro was harmed in the process though
having SPDX ids makes quite visible some discrepancies as you noticed
such as when:
- a MODULE_LICENSE is "GPL"  and the top level license is "GPL 2.0
only": here the MODULE_LICENSE would need to be fixed to "GPL v2"
- or MODULE_LICENSE is "GPL v2" and the top level license is "GPL 2.0
or later": here the MODULE_LICENSE would need to be fixed to "GPL"

These will need to be fixed over time and this is made easier with the
clarity brought by the SPDX id. My take there is that the best
approach is likely:

1. the top level license notice should take precedence over the
MODULE_LICENSE and MODULE_LICENSE should be updated accordingly
2. you might want an ack or a review from the original author in these
weird cases of mismatch

[1] https://www.gnu.org/licenses/old-licenses/gpl-2.0.html#section9
-- 
Cordially
Philippe Ombredanne

Re: [patch V4 01/11] Documentation: Add license-rules.rst to describe how to properly identify file licenses

[Philippe Ombredanne]

Pavel:

On Sat, Nov 25, 2017 at 7:51 PM, Pavel Machek  wrote:
> On Fri 2017-11-17 15:06:39, Mauro Carvalho Chehab wrote:
>> Hi Thomas,
>>
>> Em Fri, 17 Nov 2017 11:00:33 +0100 (CET)
>> Thomas Gleixner  escreveu:
>>
>> > Subject: Documentation: Add license-rules.rst to describe how to properly 
>> > identify file licenses
>> > From: Thomas Gleixner 
>> > Date: Fri, 10 Nov 2017 09:30:00 +0100
>> >
>> > Add a file to the Documentation directory to describe how file licenses
>> > should be described in all kernel files, using the SPDX identifier, as well
>> > as where all licenses should be in the kernel source tree for people to
>> > refer to (LICENSES/).
>> >
>> > Thanks to Kate, Greg and Jonathan for review and editing and Jonas for the
>> > suggestions concerning the meta tags in the licenses files.
>> >
>> > Signed-off-by: Thomas Gleixner 
>>
>> The document itself looks good, but I think it should also mention
>> what would be the expected values for the MODULE_LICENSE() macro and
>> how each license would be mapped into it.
>>
>> Right now, include/linux/module.h says:
>>
>> /*
>>  * The following license idents are currently accepted as indicating free
>>  * software modules
>>  *
>>  *"GPL"   [GNU Public License v2 or later]
>
> Hmm. AFAICT Greg translated GPL as GPL v1 or later. That seemed
> wrong... and now it seems even more wrong.

While this may come out as weird or wrong, this is neither wrong nor
"more wrong" when you dive in the details:

1. the meaning of a bare "GPL" in MODULE_LICENSE is well defined in
module.h as "GPL 2.0 or later" so there is no ambiguity there. It
would have been best to align this with SPDX, but this would break
instantly so many out of tree kernel modules and module loading tools
that expect these hard coded values and conventions that it is not
worth changing it IMHO.

2. the meaning of a bare "GPL" as a the only license notice is also
well defined in the GPL 2.0 text itself  in section 9 [1] and means
any version of the GPL that therefore can be made clear as GPL-1.0 or
later, i.e. GPL-1.+:
" If the Program does not specify a version number of this License,
you may choose any version ever published by the Free Software
Foundation. "

Therefore I do not think Greg did any translation and got anything
wrong but used exactly the convention in 2.

For instance when the only license notice in a file was a terse and
unclear: "Copyright (c) Jane Doe, GPL" or "Copyright (c) John Doe,
GPL'ed" then the resulting SPDX license id applied was  "GPL-1.0+"

I personally think this is unfortunate that we have warts like this:
it could have been the intent of author, or an oversight, or the
author may have meant 2.0 we can only guess! What is clear is that
in these cases and short of any other indication, "GPL-1.0+" is the
precise meaning that "GPL" or "GPL'ed" has in a notice outside of the
MODULE_LICENSE macro.

Note that no MODULE_LICENSE macro was harmed in the process though
having SPDX ids makes quite visible some discrepancies as you noticed
such as when:
- a MODULE_LICENSE is "GPL"  and the top level license is "GPL 2.0
only": here the MODULE_LICENSE would need to be fixed to "GPL v2"
- or MODULE_LICENSE is "GPL v2" and the top level license is "GPL 2.0
or later": here the MODULE_LICENSE would need to be fixed to "GPL"

These will need to be fixed over time and this is made easier with the
clarity brought by the SPDX id. My take there is that the best
approach is likely:

1. the top level license notice should take precedence over the
MODULE_LICENSE and MODULE_LICENSE should be updated accordingly
2. you might want an ack or a review from the original author in these
weird cases of mismatch

[1] https://www.gnu.org/licenses/old-licenses/gpl-2.0.html#section9
-- 
Cordially
Philippe Ombredanne

Re: [PATCH v2] xfs: handle register_shrinker error

[Dave Chinner]

On Fri, Nov 24, 2017 at 09:03:28PM +0900, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > Thanks. Updated patch below
> > ---
> > From 1009db61988c48c9a9e327a9d076945b29b02eee Mon Sep 17 00:00:00 2001
> > From: Michal Hocko 
> > Date: Thu, 23 Nov 2017 17:13:40 +0100
> > Subject: [PATCH] xfs: fortify xfs_alloc_buftarg error handling
> 
> Do we need below patch on top of Michal's patch?
> KM_NOFS was added by commit b17cb364dbbbf65a ("xfs: fix missing KM_NOFS
> tags to keep lockdep happy"). If not needed, some comment is expected.

Quite frankly, if the fix is "sprinkle magic undocumented
memalloc_nofs_save() calls around", then you need to think a little
more about the things you just read and the context we're operating
on here.

IOWs:

> ---
>  fs/xfs/xfs_buf.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 4c6e86d..b73fc76 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1806,6 +1806,7 @@ struct xfs_buf *
>   struct dax_device   *dax_dev)
>  {
>   xfs_buftarg_t   *btp;
> + unsigned int nofs_flag = memalloc_nofs_save();
>  
>   btp = kmem_zalloc(sizeof(*btp), KM_SLEEP | KM_NOFS);

xfs_alloc_buftarg() isn't called from transaction context, so this
KM_NOFS flag wasn't added to prevent reclaim deadlocks - it was
added to avoid stupid lockdep false positives (as was stated in the
commit you quoted).

IOWs, GFP_KERNEL allocations in this function used to trigger
lockdep false positives.

So - think for a minute rather than bashing on the keyboard. Why
aren't the other GFP_KERNEL allocations from this function causing
lockdep to trigger warnings?

Yeah - lockdep is a lot smarter these days and the false positive
trigger has clearly been fixed. i.e. there's no false positive
detection occurring here any more under GFP_KERNEL allocations,
so we don't need the KM_NOFS flag anymore.

IOWs, we don't actually need to touch this code, but if you really
must, just remove the KM_NOFS tag.

-Dave.


-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH v2] xfs: handle register_shrinker error

[Dave Chinner]

On Fri, Nov 24, 2017 at 09:03:28PM +0900, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > Thanks. Updated patch below
> > ---
> > From 1009db61988c48c9a9e327a9d076945b29b02eee Mon Sep 17 00:00:00 2001
> > From: Michal Hocko 
> > Date: Thu, 23 Nov 2017 17:13:40 +0100
> > Subject: [PATCH] xfs: fortify xfs_alloc_buftarg error handling
> 
> Do we need below patch on top of Michal's patch?
> KM_NOFS was added by commit b17cb364dbbbf65a ("xfs: fix missing KM_NOFS
> tags to keep lockdep happy"). If not needed, some comment is expected.

Quite frankly, if the fix is "sprinkle magic undocumented
memalloc_nofs_save() calls around", then you need to think a little
more about the things you just read and the context we're operating
on here.

IOWs:

> ---
>  fs/xfs/xfs_buf.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> index 4c6e86d..b73fc76 100644
> --- a/fs/xfs/xfs_buf.c
> +++ b/fs/xfs/xfs_buf.c
> @@ -1806,6 +1806,7 @@ struct xfs_buf *
>   struct dax_device   *dax_dev)
>  {
>   xfs_buftarg_t   *btp;
> + unsigned int nofs_flag = memalloc_nofs_save();
>  
>   btp = kmem_zalloc(sizeof(*btp), KM_SLEEP | KM_NOFS);

xfs_alloc_buftarg() isn't called from transaction context, so this
KM_NOFS flag wasn't added to prevent reclaim deadlocks - it was
added to avoid stupid lockdep false positives (as was stated in the
commit you quoted).

IOWs, GFP_KERNEL allocations in this function used to trigger
lockdep false positives.

So - think for a minute rather than bashing on the keyboard. Why
aren't the other GFP_KERNEL allocations from this function causing
lockdep to trigger warnings?

Yeah - lockdep is a lot smarter these days and the false positive
trigger has clearly been fixed. i.e. there's no false positive
detection occurring here any more under GFP_KERNEL allocations,
so we don't need the KM_NOFS flag anymore.

IOWs, we don't actually need to touch this code, but if you really
must, just remove the KM_NOFS tag.

-Dave.


-- 
Dave Chinner
da...@fromorbit.com

Re: [GIT PULL] afs: Fixes

[David Howells]

Linus Torvalds  wrote:

> However, even when you do that, the page can be writable in other
> mappings. At least fork(), for example, only clears the dirty bit,
> doesn't mark it write-protected.

I assumed the rmap walk done by page_mkclean() would take care of that but I'm
not really clear on what the code does.

> I just hope that the inconsistency isn't fatal to the afs client or
> server code. For example, if you retry writes forever when a checksum
> were to not match the data, that would be bad.

Shouldn't be a problem for the the in-Linux client.  Data is copied into
sk_bufs preparatory to doing further things to it like checksumming,
encryption or transmission (actually, in future, I would like to use the
encryption process to save on the copy, but this shouldn't bother that
either).

AFAIK, the servers are all userspace jobs that don't let anyone else touch
their storage so that they can maintain correctness on the data version number
of each vnode.

> so I just wanted to bring this up as a potential issue, not
> necessarily as a big problem.

Thanks.

David

Re: [GIT PULL] afs: Fixes

[David Howells]

Linus Torvalds  wrote:

> However, even when you do that, the page can be writable in other
> mappings. At least fork(), for example, only clears the dirty bit,
> doesn't mark it write-protected.

I assumed the rmap walk done by page_mkclean() would take care of that but I'm
not really clear on what the code does.

> I just hope that the inconsistency isn't fatal to the afs client or
> server code. For example, if you retry writes forever when a checksum
> were to not match the data, that would be bad.

Shouldn't be a problem for the the in-Linux client.  Data is copied into
sk_bufs preparatory to doing further things to it like checksumming,
encryption or transmission (actually, in future, I would like to use the
encryption process to save on the copy, but this shouldn't bother that
either).

AFAIK, the servers are all userspace jobs that don't let anyone else touch
their storage so that they can maintain correctness on the data version number
of each vnode.

> so I just wanted to bring this up as a potential issue, not
> necessarily as a big problem.

Thanks.

David

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Thomas Gleixner]

On Sat, 25 Nov 2017, Andy Lutomirski wrote:

> On Sat, Nov 25, 2017 at 9:28 AM, Andy Lutomirski  wrote:
> > If we overflow the stack into a guard page and then try to unwind
> > it with ORC, it should work perfectly: by construction, there can't
> > be any meaningful data in the guard page because no writes to the
> > guard page will have succeeded.
> >
> > ORC seems entirely capable of unwinding in this situation, except
> > that it doesn't even try.  Adjust its initial stack check so that
> > it's willing to try unwinding.
> >
> > I tested this by intentionally overflowing the task stack.  The
> > result is an accurate call trace instead of a trace consisting
> > purely of '?' entries.
> >
> > Signed-off-by: Andy Lutomirski 
> > ---
> >
> > Hi all-
> >
> > Ingo, this would have fixed half the debugging problem you had, I think.
> > To really nail it, we'd want some kind of magic to annotate the trace
> > so that page_fault (and async_page_fault) entries show CR2 and error_code.
> >
> > Josh, any ideas of how to do that cleanly?  We could easily hard-code it
> > in the OOPS unwinder, I guess.
> 
> Actually, this does pretty well.  We don't get CR2, but, when I added
> an intentional bug kind of along the lines of the one you debugged,
> the intermediate page fault successfully dumps all the regs in the
> stack trace, so we get the faulting instruction *and* the registers.
> We also get ORIG_RAX, which tells us the error code.  We could be
> fancy and decode that.

It works in general, but for that case it's not much better than before
vs. the '?' entries.

Thanks,

tglx

[2.556065] PANIC: double fault, error_code: 0x0
[2.557116] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
4.14.0-01256-g03dea81fe9f2-dirty #30
[2.558930] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.10.2-1 04/01/2014
[2.560133] task: 880428dd8000 task.stack: c900025fc000
[2.560729] RIP: 0010:page_fault+0x11/0x60
[2.561122] RSP: :ff083fc8 EFLAGS: 00010046
[2.561607] RAX: 819d0ac7 RBX: 0001 RCX: 819d0ac7
[2.562357] RDX:  RSI: 0010 RDI: ff084078
[2.563027] RBP: 000b R08:  R09: 0040
[2.563726] R10: 0018 R11: 0246 R12: 0003
[2.564429] R13: 55719fd7d410 R14:  R15: 03938700
[2.565104] FS:  7f9edc0b78c0() GS:88042e44() 
knlGS:
[2.565844] CS:  0010 DS:  ES:  CR0: 80050033
[2.566396] CR2: ff083fb8 CR3: 000428ec4005 CR4: 001606e0
[2.567097] DR0:  DR1:  DR2: 
[2.567761] DR3:  DR6: fffe0ff0 DR7: 0400
[2.568451] Call Trace:
[2.568704]  
[2.568950]  ? __do_page_fault+0x4b0/0x4b0
[2.569348]  ? page_fault+0x2c/0x60
[2.569680]  ? native_iret+0x7/0x7
[2.570019]  ? __do_page_fault+0x4b0/0x4b0
[2.570396]  ? page_fault+0x2c/0x60
[2.570743]  ? call_function_interrupt+0xc0/0xc0
[2.571199]  
[2.571422] Code: ff e8 34 b7 6a ff e9 9f 02 00 00 0f 1f 44 00 00 66 2e 0f 
1f 84 00 00 00 00 00 0f 1f 00 48 83 c4 88 f6 84 24 88 00 00 00 03 75 20  4a 
01 00 00 48 89 e7 48 8b 74 24 78 48 c7 44 24 78 ff ff ff 
[2.573192] Kernel panic - not syncing: Machine halted.
[2.573694] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
4.14.0-01256-g03dea81fe9f2-dirty #30
[2.574528] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.10.2-1 04/01/2014
[2.575330] Call Trace:
[2.575570]  <#DF>
[2.575760]  dump_stack+0x46/0x59
[2.576120]  panic+0xde/0x223
[2.576405]  df_debug+0x29/0x30
[2.576687]  do_double_fault+0x9a/0x120
[2.577057]  double_fault+0x22/0x30
[2.577376] RIP: 0010:page_fault+0x11/0x60
[2.55] RSP: :ff083fc8 EFLAGS: 00010046
[2.578314] RAX: 819d0ac7 RBX: 0001 RCX: 819d0ac7
[2.578979] RDX:  RSI: 0010 RDI: ff084078
[2.579666] RBP: 000b R08:  R09: 0040
[2.580334] R10: 0018 R11: 0246 R12: 0003
[2.581008] R13: 55719fd7d410 R14:  R15: 03938700
[2.581684]  ? native_iret+0x7/0x7
[2.582007] WARNING: can't dereference iret registers at ff084048 
for ip page_fault+0x11/0x60
[2.582008]  
[2.583134]  
[2.583367]  ? __do_page_fault+0x4b0/0x4b0
[2.583751]  ? page_fault+0x2c/0x60
[2.584127]  ? native_iret+0x7/0x7
[2.584466]  ? __do_page_fault+0x4b0/0x4b0
[2.584860]  ? page_fault+0x2c/0x60
[2.585195]  ? call_function_interrupt+0xc0/0xc0
[2.585621]  
[2.586966] Dumping ftrace buffer:
[2.587254](ftrace buffer empty)
[2.587534] Kernel Offset: disabled
[2.587814] ---[ 

Re: [PATCH] x86/orc: Don't bail on stack overflow

[Thomas Gleixner]

On Sat, 25 Nov 2017, Andy Lutomirski wrote:

> On Sat, Nov 25, 2017 at 9:28 AM, Andy Lutomirski  wrote:
> > If we overflow the stack into a guard page and then try to unwind
> > it with ORC, it should work perfectly: by construction, there can't
> > be any meaningful data in the guard page because no writes to the
> > guard page will have succeeded.
> >
> > ORC seems entirely capable of unwinding in this situation, except
> > that it doesn't even try.  Adjust its initial stack check so that
> > it's willing to try unwinding.
> >
> > I tested this by intentionally overflowing the task stack.  The
> > result is an accurate call trace instead of a trace consisting
> > purely of '?' entries.
> >
> > Signed-off-by: Andy Lutomirski 
> > ---
> >
> > Hi all-
> >
> > Ingo, this would have fixed half the debugging problem you had, I think.
> > To really nail it, we'd want some kind of magic to annotate the trace
> > so that page_fault (and async_page_fault) entries show CR2 and error_code.
> >
> > Josh, any ideas of how to do that cleanly?  We could easily hard-code it
> > in the OOPS unwinder, I guess.
> 
> Actually, this does pretty well.  We don't get CR2, but, when I added
> an intentional bug kind of along the lines of the one you debugged,
> the intermediate page fault successfully dumps all the regs in the
> stack trace, so we get the faulting instruction *and* the registers.
> We also get ORIG_RAX, which tells us the error code.  We could be
> fancy and decode that.

It works in general, but for that case it's not much better than before
vs. the '?' entries.

Thanks,

tglx

[2.556065] PANIC: double fault, error_code: 0x0
[2.557116] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
4.14.0-01256-g03dea81fe9f2-dirty #30
[2.558930] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.10.2-1 04/01/2014
[2.560133] task: 880428dd8000 task.stack: c900025fc000
[2.560729] RIP: 0010:page_fault+0x11/0x60
[2.561122] RSP: :ff083fc8 EFLAGS: 00010046
[2.561607] RAX: 819d0ac7 RBX: 0001 RCX: 819d0ac7
[2.562357] RDX:  RSI: 0010 RDI: ff084078
[2.563027] RBP: 000b R08:  R09: 0040
[2.563726] R10: 0018 R11: 0246 R12: 0003
[2.564429] R13: 55719fd7d410 R14:  R15: 03938700
[2.565104] FS:  7f9edc0b78c0() GS:88042e44() 
knlGS:
[2.565844] CS:  0010 DS:  ES:  CR0: 80050033
[2.566396] CR2: ff083fb8 CR3: 000428ec4005 CR4: 001606e0
[2.567097] DR0:  DR1:  DR2: 
[2.567761] DR3:  DR6: fffe0ff0 DR7: 0400
[2.568451] Call Trace:
[2.568704]  
[2.568950]  ? __do_page_fault+0x4b0/0x4b0
[2.569348]  ? page_fault+0x2c/0x60
[2.569680]  ? native_iret+0x7/0x7
[2.570019]  ? __do_page_fault+0x4b0/0x4b0
[2.570396]  ? page_fault+0x2c/0x60
[2.570743]  ? call_function_interrupt+0xc0/0xc0
[2.571199]  
[2.571422] Code: ff e8 34 b7 6a ff e9 9f 02 00 00 0f 1f 44 00 00 66 2e 0f 
1f 84 00 00 00 00 00 0f 1f 00 48 83 c4 88 f6 84 24 88 00 00 00 03 75 20  4a 
01 00 00 48 89 e7 48 8b 74 24 78 48 c7 44 24 78 ff ff ff 
[2.573192] Kernel panic - not syncing: Machine halted.
[2.573694] CPU: 1 PID: 273 Comm: systemd-udevd Not tainted 
4.14.0-01256-g03dea81fe9f2-dirty #30
[2.574528] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 
1.10.2-1 04/01/2014
[2.575330] Call Trace:
[2.575570]  <#DF>
[2.575760]  dump_stack+0x46/0x59
[2.576120]  panic+0xde/0x223
[2.576405]  df_debug+0x29/0x30
[2.576687]  do_double_fault+0x9a/0x120
[2.577057]  double_fault+0x22/0x30
[2.577376] RIP: 0010:page_fault+0x11/0x60
[2.55] RSP: :ff083fc8 EFLAGS: 00010046
[2.578314] RAX: 819d0ac7 RBX: 0001 RCX: 819d0ac7
[2.578979] RDX:  RSI: 0010 RDI: ff084078
[2.579666] RBP: 000b R08:  R09: 0040
[2.580334] R10: 0018 R11: 0246 R12: 0003
[2.581008] R13: 55719fd7d410 R14:  R15: 03938700
[2.581684]  ? native_iret+0x7/0x7
[2.582007] WARNING: can't dereference iret registers at ff084048 
for ip page_fault+0x11/0x60
[2.582008]  
[2.583134]  
[2.583367]  ? __do_page_fault+0x4b0/0x4b0
[2.583751]  ? page_fault+0x2c/0x60
[2.584127]  ? native_iret+0x7/0x7
[2.584466]  ? __do_page_fault+0x4b0/0x4b0
[2.584860]  ? page_fault+0x2c/0x60
[2.585195]  ? call_function_interrupt+0xc0/0xc0
[2.585621]  
[2.586966] Dumping ftrace buffer:
[2.587254](ftrace buffer empty)
[2.587534] Kernel Offset: disabled
[2.587814] ---[ end Kernel panic - not syncing: 

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Reindl Harald]

Am 25.11.2017 um 23:32 schrieb Dave Chinner:

On Fri, Nov 24, 2017 at 03:03:37PM -0700, Andreas Dilger wrote:

Any worse an idea than running a new kernel on an old system?
Newer e2fsck fixes a lot of bugs that are present in older
e2fsck as well...


I'm running with everything up to date (debian unstable) on these
VMs, they are just an old filesystem because some distros have had
reliable rolling updates for the entire life of these VMs. :P


but why not update the FS to ext4?

our whole infrastructure was installed with Fedora 9 on ext3 (currently 
running F26, yum/dnf dist-upgrades) but any FS including the rootfs was 
converted to ext4 in 2010

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Reindl Harald]

Am 25.11.2017 um 23:32 schrieb Dave Chinner:

On Fri, Nov 24, 2017 at 03:03:37PM -0700, Andreas Dilger wrote:

Any worse an idea than running a new kernel on an old system?
Newer e2fsck fixes a lot of bugs that are present in older
e2fsck as well...


I'm running with everything up to date (debian unstable) on these
VMs, they are just an old filesystem because some distros have had
reliable rolling updates for the entire life of these VMs. :P


but why not update the FS to ext4?

our whole infrastructure was installed with Fedora 9 on ext3 (currently 
running F26, yum/dnf dist-upgrades) but any FS including the rootfs was 
converted to ext4 in 2010

[PATCH] nvmem: core: let stride and word_size default to 1

[Heiner Kallweit]

If the caller doesn't set stride and/or word_size in struct nvmem_config 
then nvmem_register accepts this but we may face strange effects later
due to both values being 0. Therefore use 1 as default for both values.

Signed-off-by: Heiner Kallweit 
---
 drivers/nvmem/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ba0e3b453..129dad0ce 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -463,8 +463,8 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
nvmem->owner = config->owner;
if (!nvmem->owner && config->dev->driver)
nvmem->owner = config->dev->driver->owner;
-   nvmem->stride = config->stride;
-   nvmem->word_size = config->word_size;
+   nvmem->stride = config->stride ?: 1;
+   nvmem->word_size = config->word_size ?: 1;
nvmem->size = config->size;
nvmem->dev.type = _provider_type;
nvmem->dev.bus = _bus_type;
-- 
2.15.0

[PATCH] nvmem: core: let stride and word_size default to 1

[Heiner Kallweit]

If the caller doesn't set stride and/or word_size in struct nvmem_config 
then nvmem_register accepts this but we may face strange effects later
due to both values being 0. Therefore use 1 as default for both values.

Signed-off-by: Heiner Kallweit 
---
 drivers/nvmem/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index ba0e3b453..129dad0ce 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -463,8 +463,8 @@ struct nvmem_device *nvmem_register(const struct 
nvmem_config *config)
nvmem->owner = config->owner;
if (!nvmem->owner && config->dev->driver)
nvmem->owner = config->dev->driver->owner;
-   nvmem->stride = config->stride;
-   nvmem->word_size = config->word_size;
+   nvmem->stride = config->stride ?: 1;
+   nvmem->word_size = config->word_size ?: 1;
nvmem->size = config->size;
nvmem->dev.type = _provider_type;
nvmem->dev.bus = _bus_type;
-- 
2.15.0

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Dave Chinner]

On Sat, Nov 25, 2017 at 11:45:07PM +0100, Reindl Harald wrote:
> 
> Am 25.11.2017 um 23:32 schrieb Dave Chinner:
> >On Fri, Nov 24, 2017 at 03:03:37PM -0700, Andreas Dilger wrote:
> >>Any worse an idea than running a new kernel on an old system?
> >>Newer e2fsck fixes a lot of bugs that are present in older
> >>e2fsck as well...
> >
> >I'm running with everything up to date (debian unstable) on these
> >VMs, they are just an old filesystem because some distros have had
> >reliable rolling updates for the entire life of these VMs. :P
> 
> but why not update the FS to ext4?

Unlike ext3, ext4 is not a filesystem that takes kindly to being
abused by an environment that involves machines being crashed,
oopsed and forcibly rebooted without warning tens of times a day.
Every ext4 root filesytsem I've tried on these VMs has lasted less
than two weeks before being unrecoverably corrupted and needing to
be rebuilt from scratch.

Last time I tried a couple of years ago, the ext4 filesystems lasted
less than a day because corrupting itself in a way that it couldn't
mount but e2fsck didn't detect anything wrong and so it couldn't be
repaired. ext4 is just not robust enough for my use case.

And, FWIW, I don't use XFS for these root filesystems because the
reason I'm doing this to machines is that I'm trashing throwaway XFS
filesystems with broken XFS code on other devices on the VM...

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Dave Chinner]

On Sat, Nov 25, 2017 at 11:45:07PM +0100, Reindl Harald wrote:
> 
> Am 25.11.2017 um 23:32 schrieb Dave Chinner:
> >On Fri, Nov 24, 2017 at 03:03:37PM -0700, Andreas Dilger wrote:
> >>Any worse an idea than running a new kernel on an old system?
> >>Newer e2fsck fixes a lot of bugs that are present in older
> >>e2fsck as well...
> >
> >I'm running with everything up to date (debian unstable) on these
> >VMs, they are just an old filesystem because some distros have had
> >reliable rolling updates for the entire life of these VMs. :P
> 
> but why not update the FS to ext4?

Unlike ext3, ext4 is not a filesystem that takes kindly to being
abused by an environment that involves machines being crashed,
oopsed and forcibly rebooted without warning tens of times a day.
Every ext4 root filesytsem I've tried on these VMs has lasted less
than two weeks before being unrecoverably corrupted and needing to
be rebuilt from scratch.

Last time I tried a couple of years ago, the ext4 filesystems lasted
less than a day because corrupting itself in a way that it couldn't
mount but e2fsck didn't detect anything wrong and so it couldn't be
repaired. ext4 is just not robust enough for my use case.

And, FWIW, I don't use XFS for these root filesystems because the
reason I'm doing this to machines is that I'm trashing throwaway XFS
filesystems with broken XFS code on other devices on the VM...

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [GIT PULL] afs: Fixes

[Linus Torvalds]

On Sat, Nov 25, 2017 at 12:35 PM, David Howells  wrote:
>
> Doesn't clear_page_dirty_for_io() write-protect the PTE for the page to be
> written out, in which case ->page_mkwrite() will get called again before the
> page is redirtied?

No, it literally just sets the dirty bit (and does accounting).

But I think you may be right that we always write-protect he page when
we move the dirty bit from the page tables to the 'struct page'
(page_mkclean_one()).

However, even when you do that, the page can be writable in other
mappings. At least fork(), for example, only clears the dirty bit,
doesn't mark it write-protected.

So there is some rate-limiting of dirty pages, but I do not believe
that we've ever really *serialized* writes.

>>  (b) can cause some really nasty latency issues
>
> True, but I think the most common case is a file being opened, written start
> to finish and then closed.  Actually, the worst-handled thing I've seen is a
> shell script appending a bunch of things to a file because ->flush() syncs the
> file each time it is closed:-/
>
> What would you recommend instead?  I'm currently trying and keep track of what
> needs to be written so that I only write what's changed to the server, rather
> than writing only whole pages.

I don't think that what you are doing is necessarily wrong, I'm just
pointing out that you may still see mmap'ed pages being modified
concurrently with the actual IO, and that this will potentially mean
(for example) that things like checksums won't be reliably unless you
do the checksum as you copy the data to a network packet or something.

Of course, if that kind of inconsistency happens, a later write-back
will also happen, and eventually fix it. So the server may see
temporarily 'wrong' data, but it won't be final.

I just hope that the inconsistency isn't fatal to the afs client or
server code. For example, if you retry writes forever when a checksum
were to not match the data, that would be bad.

And yes, this can be

 (a) really hard  to trigger in practice

 (b) very hard to debug due to a combination of very specific timing
and behavior.

so I just wanted to bring this up as a potential issue, not
necessarily as a big problem.

 Linus

Re: [GIT PULL] afs: Fixes

[Linus Torvalds]

On Sat, Nov 25, 2017 at 12:35 PM, David Howells  wrote:
>
> Doesn't clear_page_dirty_for_io() write-protect the PTE for the page to be
> written out, in which case ->page_mkwrite() will get called again before the
> page is redirtied?

No, it literally just sets the dirty bit (and does accounting).

But I think you may be right that we always write-protect he page when
we move the dirty bit from the page tables to the 'struct page'
(page_mkclean_one()).

However, even when you do that, the page can be writable in other
mappings. At least fork(), for example, only clears the dirty bit,
doesn't mark it write-protected.

So there is some rate-limiting of dirty pages, but I do not believe
that we've ever really *serialized* writes.

>>  (b) can cause some really nasty latency issues
>
> True, but I think the most common case is a file being opened, written start
> to finish and then closed.  Actually, the worst-handled thing I've seen is a
> shell script appending a bunch of things to a file because ->flush() syncs the
> file each time it is closed:-/
>
> What would you recommend instead?  I'm currently trying and keep track of what
> needs to be written so that I only write what's changed to the server, rather
> than writing only whole pages.

I don't think that what you are doing is necessarily wrong, I'm just
pointing out that you may still see mmap'ed pages being modified
concurrently with the actual IO, and that this will potentially mean
(for example) that things like checksums won't be reliably unless you
do the checksum as you copy the data to a network packet or something.

Of course, if that kind of inconsistency happens, a later write-back
will also happen, and eventually fix it. So the server may see
temporarily 'wrong' data, but it won't be final.

I just hope that the inconsistency isn't fatal to the afs client or
server code. For example, if you retry writes forever when a checksum
were to not match the data, that would be bad.

And yes, this can be

 (a) really hard  to trigger in practice

 (b) very hard to debug due to a combination of very specific timing
and behavior.

so I just wanted to bring this up as a potential issue, not
necessarily as a big problem.

 Linus

[PATCH v2] staging: sm750b: Fix coding style issues in sm750_accel.c

[Jeremy Lacomis]

This is a patch to sm750_accel.c that fixes 80-character line length
warnings found by checkpatch.pl. It also fixes some grammatical errors
in comments and moves parameter-specific comments from inline to before
the function.

Signed-off-by: Jeremy Lacomis 
---
Changes in v2:
- Change function comments to the kernel-doc format

 drivers/staging/sm750fb/sm750_accel.c | 189 ++
 1 file changed, 103 insertions(+), 86 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_accel.c 
b/drivers/staging/sm750fb/sm750_accel.c
index 1035e91e7cd3..42cd920111bf 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+/* SPDX-License-Identifier: GPL-2.0 */
 #include 
 #include 
 #include 
@@ -68,11 +68,10 @@ void sm750_hw_de_init(struct lynx_accel *accel)
 }
 
 /*
- * set2dformat only be called from setmode functions
- * but if you need dual framebuffer driver,need call set2dformat
- * every time you use 2d function
+ * set2dformat can only be called from setmode functions, but if you need a 
dual
+ * framebuffer driver, set2dformat must be called every time a 2D function is
+ * used
  */
-
 void sm750_hw_set2dformat(struct lynx_accel *accel, int fmt)
 {
u32 reg;
@@ -94,7 +93,7 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
 
if (accel->de_wait() != 0) {
/*
-* int time wait and always busy,seems hardware
+* int time wait and always busy, seems hardware
 * got something error
 */
pr_debug("De engine always busy\n");
@@ -130,80 +129,88 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
return 0;
 }
 
-int sm750_hw_copyarea(
-struct lynx_accel *accel,
-unsigned int sBase,  /* Address of source: offset in frame buffer */
-unsigned int sPitch, /* Pitch value of source surface in BYTE */
-unsigned int sx,
-unsigned int sy, /* Starting coordinate of source surface */
-unsigned int dBase,  /* Address of destination: offset in frame buffer */
-unsigned int dPitch, /* Pitch value of destination surface in BYTE */
-unsigned int Bpp,/* Color depth of destination surface */
-unsigned int dx,
-unsigned int dy, /* Starting coordinate of destination surface */
-unsigned int width,
-unsigned int height, /* width and height of rectangle in pixel value */
-unsigned int rop2)   /* ROP value */
+/**
+ * sm750_hw_copyarea()
+ * @sBase: Address of the source offset in the frame buffer
+ * @sPitch: Pitch value of the source surface in BYTE
+ * @sx: Starting x-coordinate of the source surface
+ * @sy: Starting y-coordinate of the source surface
+ * @dBase: Address of the destination offset in the frame buffer
+ * @dPitch: Pitch value of the destination surface in BYTE
+ * @Bpp: Color depth of the destination surface
+ * @dx: Starting x-coordinate of the destination surface
+ * @dy: Starting y-coordinate of the destination surface
+ * @width: Width of the rectangle in pixels
+ * @height: Height of the rectangle in pixels
+ * @rop2: ROP value
+ */
+int sm750_hw_copyarea(struct lynx_accel *accel, unsigned int sBase,
+ unsigned int sPitch, unsigned int sx, unsigned int sy,
+ unsigned int dBase, unsigned int dPitch, unsigned int Bpp,
+ unsigned int dx, unsigned int dy, unsigned int width,
+ unsigned int height, unsigned int rop2)
 {
unsigned int nDirection, de_ctrl;
 
nDirection = LEFT_TO_RIGHT;
-   /* Direction of ROP2 operation: 1 = Left to Right, (-1) = Right to Left 
*/
+   /*
+* Direction of ROP2 operation:
+* 1  = Left to Right
+* -1 = Right to Left
+*/
de_ctrl = 0;
 
-   /* If source and destination are the same surface, need to check for 
overlay cases */
+   /*
+* If the source and destination are the same surface, need to check for
+* overlay cases
+*/
if (sBase == dBase && sPitch == dPitch) {
/* Determine direction of operation */
-   if (sy < dy) {
-   /* +--+
-* |S |
-* |   +--+
-* |   |  |   |
-* |   |  |   |
-* +---|--+   |
-* | D|
-* +--+
-*/
 
+   /* +--+
+* |S |
+* |   +--+
+* |   |  |   |
+* |   |  |   |
+* +---|--+   |
+* | D|
+* +--+
+*/
+   if (sy < dy) {
nDirection = BOTTOM_TO_TOP;
-   } else if (sy > dy) {
-   

[PATCH v2] staging: sm750b: Fix coding style issues in sm750_accel.c

[Jeremy Lacomis]

This is a patch to sm750_accel.c that fixes 80-character line length
warnings found by checkpatch.pl. It also fixes some grammatical errors
in comments and moves parameter-specific comments from inline to before
the function.

Signed-off-by: Jeremy Lacomis 
---
Changes in v2:
- Change function comments to the kernel-doc format

 drivers/staging/sm750fb/sm750_accel.c | 189 ++
 1 file changed, 103 insertions(+), 86 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750_accel.c 
b/drivers/staging/sm750fb/sm750_accel.c
index 1035e91e7cd3..42cd920111bf 100644
--- a/drivers/staging/sm750fb/sm750_accel.c
+++ b/drivers/staging/sm750fb/sm750_accel.c
@@ -1,4 +1,4 @@
-// SPDX-License-Identifier: GPL-2.0
+/* SPDX-License-Identifier: GPL-2.0 */
 #include 
 #include 
 #include 
@@ -68,11 +68,10 @@ void sm750_hw_de_init(struct lynx_accel *accel)
 }
 
 /*
- * set2dformat only be called from setmode functions
- * but if you need dual framebuffer driver,need call set2dformat
- * every time you use 2d function
+ * set2dformat can only be called from setmode functions, but if you need a 
dual
+ * framebuffer driver, set2dformat must be called every time a 2D function is
+ * used
  */
-
 void sm750_hw_set2dformat(struct lynx_accel *accel, int fmt)
 {
u32 reg;
@@ -94,7 +93,7 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
 
if (accel->de_wait() != 0) {
/*
-* int time wait and always busy,seems hardware
+* int time wait and always busy, seems hardware
 * got something error
 */
pr_debug("De engine always busy\n");
@@ -130,80 +129,88 @@ int sm750_hw_fillrect(struct lynx_accel *accel,
return 0;
 }
 
-int sm750_hw_copyarea(
-struct lynx_accel *accel,
-unsigned int sBase,  /* Address of source: offset in frame buffer */
-unsigned int sPitch, /* Pitch value of source surface in BYTE */
-unsigned int sx,
-unsigned int sy, /* Starting coordinate of source surface */
-unsigned int dBase,  /* Address of destination: offset in frame buffer */
-unsigned int dPitch, /* Pitch value of destination surface in BYTE */
-unsigned int Bpp,/* Color depth of destination surface */
-unsigned int dx,
-unsigned int dy, /* Starting coordinate of destination surface */
-unsigned int width,
-unsigned int height, /* width and height of rectangle in pixel value */
-unsigned int rop2)   /* ROP value */
+/**
+ * sm750_hw_copyarea()
+ * @sBase: Address of the source offset in the frame buffer
+ * @sPitch: Pitch value of the source surface in BYTE
+ * @sx: Starting x-coordinate of the source surface
+ * @sy: Starting y-coordinate of the source surface
+ * @dBase: Address of the destination offset in the frame buffer
+ * @dPitch: Pitch value of the destination surface in BYTE
+ * @Bpp: Color depth of the destination surface
+ * @dx: Starting x-coordinate of the destination surface
+ * @dy: Starting y-coordinate of the destination surface
+ * @width: Width of the rectangle in pixels
+ * @height: Height of the rectangle in pixels
+ * @rop2: ROP value
+ */
+int sm750_hw_copyarea(struct lynx_accel *accel, unsigned int sBase,
+ unsigned int sPitch, unsigned int sx, unsigned int sy,
+ unsigned int dBase, unsigned int dPitch, unsigned int Bpp,
+ unsigned int dx, unsigned int dy, unsigned int width,
+ unsigned int height, unsigned int rop2)
 {
unsigned int nDirection, de_ctrl;
 
nDirection = LEFT_TO_RIGHT;
-   /* Direction of ROP2 operation: 1 = Left to Right, (-1) = Right to Left 
*/
+   /*
+* Direction of ROP2 operation:
+* 1  = Left to Right
+* -1 = Right to Left
+*/
de_ctrl = 0;
 
-   /* If source and destination are the same surface, need to check for 
overlay cases */
+   /*
+* If the source and destination are the same surface, need to check for
+* overlay cases
+*/
if (sBase == dBase && sPitch == dPitch) {
/* Determine direction of operation */
-   if (sy < dy) {
-   /* +--+
-* |S |
-* |   +--+
-* |   |  |   |
-* |   |  |   |
-* +---|--+   |
-* | D|
-* +--+
-*/
 
+   /* +--+
+* |S |
+* |   +--+
+* |   |  |   |
+* |   |  |   |
+* +---|--+   |
+* | D|
+* +--+
+*/
+   if (sy < dy) {
nDirection = BOTTOM_TO_TOP;
-   } else if (sy > dy) {
-   /* +--+
- 

Re: [PATCH 42/43] x86/mm/kaiser: Allow KAISER to be enabled/disabled at runtime

[Thomas Gleixner]

On Sat, 25 Nov 2017, Andy Lutomirski wrote:
> > On Nov 25, 2017, at 1:05 PM, Thomas Gleixner  wrote:
> > On Sat, 25 Nov 2017, Andy Lutomirski wrote:
> >> Keep in mind that, for a static_branch, actually setting the thing needs
> >> to be deferred, but that's straightforward.
> > 
> > That's not an issue during boot. That would be an issue for a run time
> > switch.
> 
> What I mean is: if you modify a static_branch too early, it blows up terribly.

I'm aware of that. We can't switch it in the early boot stage. But that
does not matter as we can switch way before we reach user space.

The early kaiser mappings are fine whether we use them later or not. At the
point in boot where we actually make the decision, there is nothing more
than the extra 4k shadow which got initialized.

If we ever want to do runtime switching, then the full shadow mapping needs
to be maintained even while kaiser is disabled, just the NX poisoning of
the user space mappings is what makes the difference.

Thanks,

tglx

Re: [PATCH 42/43] x86/mm/kaiser: Allow KAISER to be enabled/disabled at runtime

[Thomas Gleixner]

On Sat, 25 Nov 2017, Andy Lutomirski wrote:
> > On Nov 25, 2017, at 1:05 PM, Thomas Gleixner  wrote:
> > On Sat, 25 Nov 2017, Andy Lutomirski wrote:
> >> Keep in mind that, for a static_branch, actually setting the thing needs
> >> to be deferred, but that's straightforward.
> > 
> > That's not an issue during boot. That would be an issue for a run time
> > switch.
> 
> What I mean is: if you modify a static_branch too early, it blows up terribly.

I'm aware of that. We can't switch it in the early boot stage. But that
does not matter as we can switch way before we reach user space.

The early kaiser mappings are fine whether we use them later or not. At the
point in boot where we actually make the decision, there is nothing more
than the extra 4k shadow which got initialized.

If we ever want to do runtime switching, then the full shadow mapping needs
to be maintained even while kaiser is disabled, just the NX poisoning of
the user space mappings is what makes the difference.

Thanks,

tglx

Re: [GIT PULL] afs: Fixes

[Dave Chinner]

On Sat, Nov 25, 2017 at 10:35:43PM +, David Howells wrote:
> Linus Torvalds  wrote:
> 
> > So I see in the commit message why afs needs to do this, but it's
> > worth pointing out that it's
> > 
> >  (a) impossible to avoid the "inconsistent data" case for writable mmap'ed
> >  pages
> 
> Doesn't clear_page_dirty_for_io() write-protect the PTE for the page to be
> written out, in which case ->page_mkwrite() will get called again before the
> page is redirtied?

Yes, but page_mkwrite will only block on writeback in progress is if
the backing device says it needs stable pages.  See
wait_for_stable_page().  e.g. stable pages are required if RAID is
in use, otherwise modification during IO can result in broken
on-disk parity/mirroring

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [GIT PULL] afs: Fixes

[Dave Chinner]

On Sat, Nov 25, 2017 at 10:35:43PM +, David Howells wrote:
> Linus Torvalds  wrote:
> 
> > So I see in the commit message why afs needs to do this, but it's
> > worth pointing out that it's
> > 
> >  (a) impossible to avoid the "inconsistent data" case for writable mmap'ed
> >  pages
> 
> Doesn't clear_page_dirty_for_io() write-protect the PTE for the page to be
> written out, in which case ->page_mkwrite() will get called again before the
> page is redirtied?

Yes, but page_mkwrite will only block on writeback in progress is if
the backing device says it needs stable pages.  See
wait_for_stable_page().  e.g. stable pages are required if RAID is
in use, otherwise modification during IO can result in broken
on-disk parity/mirroring

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH 0/5] Consolidate init_task handling

[David Howells]

I've updated my git branch with changed descriptions, but the content is the
same.

David

Re: [PATCH 0/5] Consolidate init_task handling

[David Howells]

I've updated my git branch with changed descriptions, but the content is the
same.

David

[PATCH] ALSA: ice1724: Fix resume issues with Prodigy 7.1 HiFi

[Yussuf Khalil]

There are two issues after resuming from suspend on the
Audiotrak Prodigy 7.1 HiFi:
 - the output volume is set to 100%
 - microphone input isn't working anymore

This patch fixes these issues by reinitializing both codecs of the device
and restoring the previous volumes during resume.

Signed-off-by: Yussuf Khalil 
---
 sound/pci/ice1712/prodigy_hifi.c | 131 ++-
 1 file changed, 102 insertions(+), 29 deletions(-)

diff --git a/sound/pci/ice1712/prodigy_hifi.c b/sound/pci/ice1712/prodigy_hifi.c
index 2697402b5195..8dabd4d0211d 100644
--- a/sound/pci/ice1712/prodigy_hifi.c
+++ b/sound/pci/ice1712/prodigy_hifi.c
@@ -965,13 +965,32 @@ static int prodigy_hd2_add_controls(struct snd_ice1712 
*ice)
return 0;
 }
 
+static void wm8766_init(struct snd_ice1712 *ice)
+{
+   static unsigned short wm8766_inits[] = {
+   WM8766_RESET,  0x,
+   WM8766_DAC_CTRL,0x0120,
+   WM8766_INT_CTRL,0x0022, /* I2S Normal Mode, 24 bit */
+   WM8766_DAC_CTRL2,   0x0001,
+   WM8766_DAC_CTRL3,   0x0080,
+   WM8766_LDA1,0x0100,
+   WM8766_LDA2,0x0100,
+   WM8766_LDA3,0x0100,
+   WM8766_RDA1,0x0100,
+   WM8766_RDA2,0x0100,
+   WM8766_RDA3,0x0100,
+   WM8766_MUTE1,  0x,
+   WM8766_MUTE2,  0x,
+   };
+   unsigned int i;
 
-/*
- * initialize the chip
- */
-static int prodigy_hifi_init(struct snd_ice1712 *ice)
+   for (i = 0; i < ARRAY_SIZE(wm8766_inits); i += 2)
+   wm8766_spi_write(ice, wm8766_inits[i], wm8766_inits[i + 1]);
+}
+
+static void wm8776_init(struct snd_ice1712 *ice)
 {
-   static unsigned short wm_inits[] = {
+   static unsigned short wm8776_inits[] = {
/* These come first to reduce init pop noise */
WM_ADC_MUX, 0x0003, /* ADC mute */
/* 0x00c0 replaced by 0x0003 */
@@ -982,7 +1001,76 @@ static int prodigy_hifi_init(struct snd_ice1712 *ice)
WM_POWERDOWN,   0x0008, /* All power-up except HP */
WM_RESET,   0x, /* reset */
};
-   static unsigned short wm_inits2[] = {
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(wm8776_inits); i += 2)
+   wm_put(ice, wm8776_inits[i], wm8776_inits[i + 1]);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int prodigy_hifi_resume(struct snd_ice1712 *ice)
+{
+   static unsigned short wm8776_reinit_registers[] = {
+   WM_MASTER_CTRL,
+   WM_DAC_INT,
+   WM_ADC_INT,
+   WM_OUT_MUX,
+   WM_HP_ATTEN_L,
+   WM_HP_ATTEN_R,
+   WM_PHASE_SWAP,
+   WM_DAC_CTRL2,
+   WM_ADC_ATTEN_L,
+   WM_ADC_ATTEN_R,
+   WM_ALC_CTRL1,
+   WM_ALC_CTRL2,
+   WM_ALC_CTRL3,
+   WM_NOISE_GATE,
+   WM_ADC_MUX,
+   /* no DAC attenuation here */
+   };
+   struct prodigy_hifi_spec *spec = ice->spec;
+   int i, ch;
+
+   mutex_lock(>gpio_mutex);
+
+   /* reinitialize WM8776 and re-apply old register values */
+   wm8776_init(ice);
+   schedule_timeout_uninterruptible(1);
+   for (i = 0; i < ARRAY_SIZE(wm8776_reinit_registers); i++)
+   wm_put(ice, wm8776_reinit_registers[i],
+  wm_get(ice, wm8776_reinit_registers[i]));
+
+   /* reinitialize WM8766 and re-apply volumes for all DACs */
+   wm8766_init(ice);
+   for (ch = 0; ch < 2; ch++) {
+   wm_set_vol(ice, WM_DAC_ATTEN_L + ch,
+  spec->vol[2 + ch], spec->master[ch]);
+
+   wm8766_set_vol(ice, WM8766_LDA1 + ch,
+  spec->vol[0 + ch], spec->master[ch]);
+
+   wm8766_set_vol(ice, WM8766_LDA2 + ch,
+  spec->vol[4 + ch], spec->master[ch]);
+
+   wm8766_set_vol(ice, WM8766_LDA3 + ch,
+  spec->vol[6 + ch], spec->master[ch]);
+   }
+
+   /* unmute WM8776 DAC */
+   wm_put(ice, WM_DAC_MUTE, 0x00);
+   wm_put(ice, WM_DAC_CTRL1, 0x90);
+
+   mutex_unlock(>gpio_mutex);
+   return 0;
+}
+#endif
+
+/*
+ * initialize the chip
+ */
+static int prodigy_hifi_init(struct snd_ice1712 *ice)
+{
+   static unsigned short wm8776_defaults[] = {
WM_MASTER_CTRL,  0x0022, /* 256fs, slave mode */
WM_DAC_INT, 0x0022, /* I2S, normal polarity, 24bit */
WM_ADC_INT, 0x0022, /* I2S, normal polarity, 24bit */
@@ -1010,22 +1098,6 @@ static int prodigy_hifi_init(struct snd_ice1712 *ice)
WM_DAC_MUTE,0x, /* DAC unmute */
WM_ADC_MUX, 0x0003, /* ADC unmute, both CD/Line On */
};
-   static unsigned short 

[PATCH] ALSA: ice1724: Fix resume issues with Prodigy 7.1 HiFi

[Yussuf Khalil]

There are two issues after resuming from suspend on the
Audiotrak Prodigy 7.1 HiFi:
 - the output volume is set to 100%
 - microphone input isn't working anymore

This patch fixes these issues by reinitializing both codecs of the device
and restoring the previous volumes during resume.

Signed-off-by: Yussuf Khalil 
---
 sound/pci/ice1712/prodigy_hifi.c | 131 ++-
 1 file changed, 102 insertions(+), 29 deletions(-)

diff --git a/sound/pci/ice1712/prodigy_hifi.c b/sound/pci/ice1712/prodigy_hifi.c
index 2697402b5195..8dabd4d0211d 100644
--- a/sound/pci/ice1712/prodigy_hifi.c
+++ b/sound/pci/ice1712/prodigy_hifi.c
@@ -965,13 +965,32 @@ static int prodigy_hd2_add_controls(struct snd_ice1712 
*ice)
return 0;
 }
 
+static void wm8766_init(struct snd_ice1712 *ice)
+{
+   static unsigned short wm8766_inits[] = {
+   WM8766_RESET,  0x,
+   WM8766_DAC_CTRL,0x0120,
+   WM8766_INT_CTRL,0x0022, /* I2S Normal Mode, 24 bit */
+   WM8766_DAC_CTRL2,   0x0001,
+   WM8766_DAC_CTRL3,   0x0080,
+   WM8766_LDA1,0x0100,
+   WM8766_LDA2,0x0100,
+   WM8766_LDA3,0x0100,
+   WM8766_RDA1,0x0100,
+   WM8766_RDA2,0x0100,
+   WM8766_RDA3,0x0100,
+   WM8766_MUTE1,  0x,
+   WM8766_MUTE2,  0x,
+   };
+   unsigned int i;
 
-/*
- * initialize the chip
- */
-static int prodigy_hifi_init(struct snd_ice1712 *ice)
+   for (i = 0; i < ARRAY_SIZE(wm8766_inits); i += 2)
+   wm8766_spi_write(ice, wm8766_inits[i], wm8766_inits[i + 1]);
+}
+
+static void wm8776_init(struct snd_ice1712 *ice)
 {
-   static unsigned short wm_inits[] = {
+   static unsigned short wm8776_inits[] = {
/* These come first to reduce init pop noise */
WM_ADC_MUX, 0x0003, /* ADC mute */
/* 0x00c0 replaced by 0x0003 */
@@ -982,7 +1001,76 @@ static int prodigy_hifi_init(struct snd_ice1712 *ice)
WM_POWERDOWN,   0x0008, /* All power-up except HP */
WM_RESET,   0x, /* reset */
};
-   static unsigned short wm_inits2[] = {
+   unsigned int i;
+
+   for (i = 0; i < ARRAY_SIZE(wm8776_inits); i += 2)
+   wm_put(ice, wm8776_inits[i], wm8776_inits[i + 1]);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int prodigy_hifi_resume(struct snd_ice1712 *ice)
+{
+   static unsigned short wm8776_reinit_registers[] = {
+   WM_MASTER_CTRL,
+   WM_DAC_INT,
+   WM_ADC_INT,
+   WM_OUT_MUX,
+   WM_HP_ATTEN_L,
+   WM_HP_ATTEN_R,
+   WM_PHASE_SWAP,
+   WM_DAC_CTRL2,
+   WM_ADC_ATTEN_L,
+   WM_ADC_ATTEN_R,
+   WM_ALC_CTRL1,
+   WM_ALC_CTRL2,
+   WM_ALC_CTRL3,
+   WM_NOISE_GATE,
+   WM_ADC_MUX,
+   /* no DAC attenuation here */
+   };
+   struct prodigy_hifi_spec *spec = ice->spec;
+   int i, ch;
+
+   mutex_lock(>gpio_mutex);
+
+   /* reinitialize WM8776 and re-apply old register values */
+   wm8776_init(ice);
+   schedule_timeout_uninterruptible(1);
+   for (i = 0; i < ARRAY_SIZE(wm8776_reinit_registers); i++)
+   wm_put(ice, wm8776_reinit_registers[i],
+  wm_get(ice, wm8776_reinit_registers[i]));
+
+   /* reinitialize WM8766 and re-apply volumes for all DACs */
+   wm8766_init(ice);
+   for (ch = 0; ch < 2; ch++) {
+   wm_set_vol(ice, WM_DAC_ATTEN_L + ch,
+  spec->vol[2 + ch], spec->master[ch]);
+
+   wm8766_set_vol(ice, WM8766_LDA1 + ch,
+  spec->vol[0 + ch], spec->master[ch]);
+
+   wm8766_set_vol(ice, WM8766_LDA2 + ch,
+  spec->vol[4 + ch], spec->master[ch]);
+
+   wm8766_set_vol(ice, WM8766_LDA3 + ch,
+  spec->vol[6 + ch], spec->master[ch]);
+   }
+
+   /* unmute WM8776 DAC */
+   wm_put(ice, WM_DAC_MUTE, 0x00);
+   wm_put(ice, WM_DAC_CTRL1, 0x90);
+
+   mutex_unlock(>gpio_mutex);
+   return 0;
+}
+#endif
+
+/*
+ * initialize the chip
+ */
+static int prodigy_hifi_init(struct snd_ice1712 *ice)
+{
+   static unsigned short wm8776_defaults[] = {
WM_MASTER_CTRL,  0x0022, /* 256fs, slave mode */
WM_DAC_INT, 0x0022, /* I2S, normal polarity, 24bit */
WM_ADC_INT, 0x0022, /* I2S, normal polarity, 24bit */
@@ -1010,22 +1098,6 @@ static int prodigy_hifi_init(struct snd_ice1712 *ice)
WM_DAC_MUTE,0x, /* DAC unmute */
WM_ADC_MUX, 0x0003, /* ADC unmute, both CD/Line On */
};
-   static unsigned short wm8766_inits[] = {
-  

Re: [PATCH 0/5] Consolidate init_task handling

[David Howells]

Linus Torvalds  wrote:

> Or at least it's a very unusual use of that word. Why doesn't the
> explanation just say what it does: "move", and say from where to where
> ("from macro to definition" or something)?
> 
> Or "remove macro XYZ, expanding it in place", or something? To me,
> "unroll" has a completely different meaning in computers.

I don't remember why I used the word "Unroll", but "Expand X in place" seems a
good substitute.

Do you want me to repost the patches?

David

Re: [PATCH 0/5] Consolidate init_task handling

[David Howells]

Linus Torvalds  wrote:

> Or at least it's a very unusual use of that word. Why doesn't the
> explanation just say what it does: "move", and say from where to where
> ("from macro to definition" or something)?
> 
> Or "remove macro XYZ, expanding it in place", or something? To me,
> "unroll" has a completely different meaning in computers.

I don't remember why I used the word "Unroll", but "Expand X in place" seems a
good substitute.

Do you want me to repost the patches?

David

Re: [PATCH] Staging: sm750fb: Fix coding style issue in ddk750_sii164.c

[Joe Perches]

On Sat, 2017-11-25 at 12:59 -0500, Jeremy Lacomis wrote:
> This is a patch to the ddk750_sii164.c file that fixes line length warnings
> found by the checkpatch.pl script
> 
> Signed-off-by: Jeremy Lacomis 
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 39 
> +++--
>  1 file changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
> b/drivers/staging/sm750fb/ddk750_sii164.c
> index c787a74c4f9c..d081ecbb3e3d 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -39,8 +39,10 @@ unsigned short sii164GetVendorID(void)
>  {
>   unsigned short vendorID;
>  
> - vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_VENDOR_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_VENDOR_ID_LOW);
> + vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_VENDOR_ID_HIGH) << 8) |
> + (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_VENDOR_ID_LOW);
>  
>   return vendorID;
>  }
> @@ -56,15 +58,20 @@ unsigned short sii164GetDeviceID(void)
>  {
>   unsigned short deviceID;
>  
> - deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_DEVICE_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_DEVICE_ID_LOW);
> + deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_DEVICE_ID_HIGH) << 8) |
> + (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_DEVICE_ID_LOW);
>  
>   return deviceID;
>  }

i2cReadReg is always used with SII154_I2C_ADDRESS.

Perhaps it'd be better to redefine i2cReadReg to something else.
i2cReadReg also returns an unsigned char so this cast isn't
particularly sensible.

Perhaps something like:

#define sii164_i2c_read_reg(reg)i2cReadReg(SII164_I2C_ADDRESS, reg)

device_id = sii164_i2c_read_reg(SII164_DEVICE_ID_HIGH) << 8 |
sii164_i2c_read_reg(SII164_DEVICE_ID_LOW);

Re: [PATCH] Staging: sm750fb: Fix coding style issue in ddk750_sii164.c

[Joe Perches]

On Sat, 2017-11-25 at 12:59 -0500, Jeremy Lacomis wrote:
> This is a patch to the ddk750_sii164.c file that fixes line length warnings
> found by the checkpatch.pl script
> 
> Signed-off-by: Jeremy Lacomis 
> ---
>  drivers/staging/sm750fb/ddk750_sii164.c | 39 
> +++--
>  1 file changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/ddk750_sii164.c 
> b/drivers/staging/sm750fb/ddk750_sii164.c
> index c787a74c4f9c..d081ecbb3e3d 100644
> --- a/drivers/staging/sm750fb/ddk750_sii164.c
> +++ b/drivers/staging/sm750fb/ddk750_sii164.c
> @@ -39,8 +39,10 @@ unsigned short sii164GetVendorID(void)
>  {
>   unsigned short vendorID;
>  
> - vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_VENDOR_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_VENDOR_ID_LOW);
> + vendorID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_VENDOR_ID_HIGH) << 8) |
> + (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_VENDOR_ID_LOW);
>  
>   return vendorID;
>  }
> @@ -56,15 +58,20 @@ unsigned short sii164GetDeviceID(void)
>  {
>   unsigned short deviceID;
>  
> - deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_DEVICE_ID_HIGH) << 8) |
> - (unsigned short) i2cReadReg(SII164_I2C_ADDRESS, 
> SII164_DEVICE_ID_LOW);
> + deviceID = ((unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_DEVICE_ID_HIGH) << 8) |
> + (unsigned short) i2cReadReg(SII164_I2C_ADDRESS,
> + SII164_DEVICE_ID_LOW);
>  
>   return deviceID;
>  }

i2cReadReg is always used with SII154_I2C_ADDRESS.

Perhaps it'd be better to redefine i2cReadReg to something else.
i2cReadReg also returns an unsigned char so this cast isn't
particularly sensible.

Perhaps something like:

#define sii164_i2c_read_reg(reg)i2cReadReg(SII164_I2C_ADDRESS, reg)

device_id = sii164_i2c_read_reg(SII164_DEVICE_ID_HIGH) << 8 |
sii164_i2c_read_reg(SII164_DEVICE_ID_LOW);

Re: [GIT PULL] afs: Fixes

[David Howells]

Linus Torvalds  wrote:

> So I see in the commit message why afs needs to do this, but it's
> worth pointing out that it's
> 
>  (a) impossible to avoid the "inconsistent data" case for writable mmap'ed
>  pages

Doesn't clear_page_dirty_for_io() write-protect the PTE for the page to be
written out, in which case ->page_mkwrite() will get called again before the
page is redirtied?

>  (b) can cause some really nasty latency issues

True, but I think the most common case is a file being opened, written start
to finish and then closed.  Actually, the worst-handled thing I've seen is a
shell script appending a bunch of things to a file because ->flush() syncs the
file each time it is closed:-/

What would you recommend instead?  I'm currently trying and keep track of what
needs to be written so that I only write what's changed to the server, rather
than writing only whole pages.

David

Re: [GIT PULL] afs: Fixes

[David Howells]

Linus Torvalds  wrote:

> So I see in the commit message why afs needs to do this, but it's
> worth pointing out that it's
> 
>  (a) impossible to avoid the "inconsistent data" case for writable mmap'ed
>  pages

Doesn't clear_page_dirty_for_io() write-protect the PTE for the page to be
written out, in which case ->page_mkwrite() will get called again before the
page is redirtied?

>  (b) can cause some really nasty latency issues

True, but I think the most common case is a file being opened, written start
to finish and then closed.  Actually, the worst-handled thing I've seen is a
shell script appending a bunch of things to a file because ->flush() syncs the
file each time it is closed:-/

What would you recommend instead?  I'm currently trying and keep track of what
needs to be written so that I only write what's changed to the server, rather
than writing only whole pages.

David

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Dave Chinner]

On Fri, Nov 24, 2017 at 03:03:37PM -0700, Andreas Dilger wrote:
> On Nov 24, 2017, at 9:51 AM, Andi Kleen  wrote:
> > 
> >> We checked old kernels, and old e2fsprogs, and didn't see any cases
> >> where fast (<= 60 chars) symlinks were created using external blocks.
> >> It seems that _something_ did create them, and it would be good to
> >> figure that out so we can determine if it is a widespread problem
> > 
> > I assume it was the original kernel.
> > 
> >> 
> >> I think e2fsck can fix this quite easily, and there really isn't
> >> an easy way to revert to the old method if the large xattr feature
> >> is enabled.  If you are willing to run a new kernel, you should also
> >> be willing to run a new e2fsck.
> > 
> > It's obviously not enabled on ext3.
> > 
> >> We could probably add a fallback to the old mechanism (and print
> >> a one-time warning to upgrade to a newer e2fsck) if an external fast
> >> symlink is found and the large xattr  feature is not enabled, which
> >> would give more time to fix this (hopefully rare in the wild) case.
> > 
> > If the old kernel created it, then likely all the
> > /lib{,64}/ld-linux.so.2 symlinks have that, which breaks all ELF
> > executables. I suspect in these old file systems it's not particularly rare.
> 
> Sure, but not many people are going to be running a 4.14 kernel with
> a 2007 system. 

I have multiple test VMs with root ext3 filesystems that date back
that far. Looks like the original install the root fs image was
derived from came from around 2006:

$ ls -lt /etc |tail -1
-rw-r--r--  1 root root   9 Aug  8  2006 host.conf
$ ls -lt /usr/bin |tail -2
-rwxr-xr-x 1 root   root 2038 Jun 18  2006 defoma-hints
-rwxr-xr-x 1 root   root 1761 Jun 18  2006 dh_installdefoma
$ uname -a
Linux test4 4.14.0-dgc #211 SMP PREEMPT Thu Nov 23 16:49:31 AEDT 2017 x86_64 
GNU/Linux
$

These VMs are in use 24x7, and have been since they were created way
back when. When something in ext3 breaks, I tend to notice it and
report it.

They don't have any whacky symlinks around, but the modern ext4 code
does try to eat these filesystems every so often. Extended operation
at ENOSPC will eventually corrupt the rootfs and crash the kernel,
and then I play the "e2fsck doesn't detect corruption, kernel does"
game to get them fixed up and working again

> > Requiring new e2fsck on old systems is a bad idea.
> 
> Any worse an idea than running a new kernel on an old system?
> Newer e2fsck fixes a lot of bugs that are present in older
> e2fsck as well...

I'm running with everything up to date (debian unstable) on these
VMs, they are just an old filesystem because some distros have had
reliable rolling updates for the entire life of these VMs. :P

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Dave Chinner]

On Fri, Nov 24, 2017 at 03:03:37PM -0700, Andreas Dilger wrote:
> On Nov 24, 2017, at 9:51 AM, Andi Kleen  wrote:
> > 
> >> We checked old kernels, and old e2fsprogs, and didn't see any cases
> >> where fast (<= 60 chars) symlinks were created using external blocks.
> >> It seems that _something_ did create them, and it would be good to
> >> figure that out so we can determine if it is a widespread problem
> > 
> > I assume it was the original kernel.
> > 
> >> 
> >> I think e2fsck can fix this quite easily, and there really isn't
> >> an easy way to revert to the old method if the large xattr feature
> >> is enabled.  If you are willing to run a new kernel, you should also
> >> be willing to run a new e2fsck.
> > 
> > It's obviously not enabled on ext3.
> > 
> >> We could probably add a fallback to the old mechanism (and print
> >> a one-time warning to upgrade to a newer e2fsck) if an external fast
> >> symlink is found and the large xattr  feature is not enabled, which
> >> would give more time to fix this (hopefully rare in the wild) case.
> > 
> > If the old kernel created it, then likely all the
> > /lib{,64}/ld-linux.so.2 symlinks have that, which breaks all ELF
> > executables. I suspect in these old file systems it's not particularly rare.
> 
> Sure, but not many people are going to be running a 4.14 kernel with
> a 2007 system. 

I have multiple test VMs with root ext3 filesystems that date back
that far. Looks like the original install the root fs image was
derived from came from around 2006:

$ ls -lt /etc |tail -1
-rw-r--r--  1 root root   9 Aug  8  2006 host.conf
$ ls -lt /usr/bin |tail -2
-rwxr-xr-x 1 root   root 2038 Jun 18  2006 defoma-hints
-rwxr-xr-x 1 root   root 1761 Jun 18  2006 dh_installdefoma
$ uname -a
Linux test4 4.14.0-dgc #211 SMP PREEMPT Thu Nov 23 16:49:31 AEDT 2017 x86_64 
GNU/Linux
$

These VMs are in use 24x7, and have been since they were created way
back when. When something in ext3 breaks, I tend to notice it and
report it.

They don't have any whacky symlinks around, but the modern ext4 code
does try to eat these filesystems every so often. Extended operation
at ENOSPC will eventually corrupt the rootfs and crash the kernel,
and then I play the "e2fsck doesn't detect corruption, kernel does"
game to get them fixed up and working again

> > Requiring new e2fsck on old systems is a bad idea.
> 
> Any worse an idea than running a new kernel on an old system?
> Newer e2fsck fixes a lot of bugs that are present in older
> e2fsck as well...

I'm running with everything up to date (debian unstable) on these
VMs, they are just an old filesystem because some distros have had
reliable rolling updates for the entire life of these VMs. :P

Cheers,

Dave.
-- 
Dave Chinner
da...@fromorbit.com

Re: [PATCH] ASoC: fsl_ssi: Override bit clock rate based on slot number

[Lukasz Majewski]

Hi Nicolin,

> On Wed, Sep 13, 2017 at 10:02:20AM +0200, Arnaud Mouiche wrote:
> 
> > >Could you please give me a few set of examples of how you set
> > >set_sysclk(), set_tdm_slot() with the current driver? The idea
> > >here is to figure out a way to calculate the bclk in hw_params
> > >without getting set_sysclk() involved any more.  
> 
> > Here is one, where a bclk = 4*16*fs is expected  
> 
> > In another setup, there are 8 x 16 bits slots, whatever the number
> > of active channels is.
> > In this case bclk = 128 * fs
> > The number of slots is completely arbitrary. Some slots can even be
> > reserved for communication between codecs that don't communicate
> > with linux.  
> 
> In summary, bclk = sample rate * slots * slot_width;
> 
> I will update my patch soon.
> 
> > >Unfortunately, it looks like a work around to me. I understand
> > >the idea of leaving set_sysclk() out there to override the bit
> > >clock is convenient, but it is not a standard ALSA design and
> > >may eventually introduce new problems like today.  
> > 
> > I agree. I'm not conservative at all concerning this question.
> > I don't see a way to remove set_sysclk without breaking current TDM
> > users anyway, at least for those who don't have their code
> > upstreamed.  
> 
> Which TDM case would be broken by this removal? The only impact
> that I can see is that the ASoC core returns an ENOTSUPP for a
> set_sysclk() call now, which is something that a dai-link driver
> should have taken care of anyway.
> 
> > All information provided through snd_soc_dai_set_tdm_slot( cpu_dai,
> > mask, mask, slots, width ) should be enough
> > In this case, for TDM users
> > 
> >bclk = slots * width * fs   (where slots is != channels)  
> 
> > will manage 99 % of the cases.
> > And the remaining 1% will concern people who need to hack the kernel
> > so widely they don't care about the set_sysclk removal.  
> 
> A patch from those people will be always welcome.
> 
> > - fsl-asoc-card.c : *something will break since
> > snd_soc_dai_set_sysclk returned code is checked*  
> 
> I've already submitted a patch to ignore all ENOTSUPP.

Nicolin, do you know what happened with this patch? I couldn't find it
in current linux/master.

Has it been applied to any asoc tree for being upstreamed?

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpkKx18KkRhZ.pgp
Description: OpenPGP digital signature

Re: [PATCH] ASoC: fsl_ssi: Override bit clock rate based on slot number

[Lukasz Majewski]

Hi Nicolin,

> On Wed, Sep 13, 2017 at 10:02:20AM +0200, Arnaud Mouiche wrote:
> 
> > >Could you please give me a few set of examples of how you set
> > >set_sysclk(), set_tdm_slot() with the current driver? The idea
> > >here is to figure out a way to calculate the bclk in hw_params
> > >without getting set_sysclk() involved any more.  
> 
> > Here is one, where a bclk = 4*16*fs is expected  
> 
> > In another setup, there are 8 x 16 bits slots, whatever the number
> > of active channels is.
> > In this case bclk = 128 * fs
> > The number of slots is completely arbitrary. Some slots can even be
> > reserved for communication between codecs that don't communicate
> > with linux.  
> 
> In summary, bclk = sample rate * slots * slot_width;
> 
> I will update my patch soon.
> 
> > >Unfortunately, it looks like a work around to me. I understand
> > >the idea of leaving set_sysclk() out there to override the bit
> > >clock is convenient, but it is not a standard ALSA design and
> > >may eventually introduce new problems like today.  
> > 
> > I agree. I'm not conservative at all concerning this question.
> > I don't see a way to remove set_sysclk without breaking current TDM
> > users anyway, at least for those who don't have their code
> > upstreamed.  
> 
> Which TDM case would be broken by this removal? The only impact
> that I can see is that the ASoC core returns an ENOTSUPP for a
> set_sysclk() call now, which is something that a dai-link driver
> should have taken care of anyway.
> 
> > All information provided through snd_soc_dai_set_tdm_slot( cpu_dai,
> > mask, mask, slots, width ) should be enough
> > In this case, for TDM users
> > 
> >bclk = slots * width * fs   (where slots is != channels)  
> 
> > will manage 99 % of the cases.
> > And the remaining 1% will concern people who need to hack the kernel
> > so widely they don't care about the set_sysclk removal.  
> 
> A patch from those people will be always welcome.
> 
> > - fsl-asoc-card.c : *something will break since
> > snd_soc_dai_set_sysclk returned code is checked*  
> 
> I've already submitted a patch to ignore all ENOTSUPP.

Nicolin, do you know what happened with this patch? I couldn't find it
in current linux/master.

Has it been applied to any asoc tree for being upstreamed?

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de


pgpkKx18KkRhZ.pgp
Description: OpenPGP digital signature