Re: [PATCH] staging: fsl-mc: fix coding style warning

[Greg KH]

On Fri, Feb 24, 2017 at 08:24:42PM +0200, Lucian Zala wrote:
> Hi Greg,
> 
> I am a little bit confused regarding your reply, it seems
> that the patch applied afterall, or?
> 
> http://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/drivers/staging/fsl-mc?h=staging-testing=3bee20f39e4df8d65e8d34ab9b3ab92e5acc
> 
> Have I messed something in it?

No, my mistake, I must have already applied it, sorry, dealing with
hundreds of patches causes confusion at times :)

sorry for the noise,

greg k-h

Re: [Outreachy kernel] Re: [PATCH v2 3/3] staging: rtl8192u: Prefer using the BIT macro

[Greg KH]

On Fri, Feb 24, 2017 at 12:10:21PM -0800, SIMRAN SINGHAL wrote:
> 
> 
> On Friday, February 24, 2017 at 10:46:21 PM UTC+5:30, gregkh wrote:
> 
> On Fri, Feb 17, 2017 at 11:20:59PM +0530, simran singhal wrote:
> > Replaces left shift operation (1 << d) by BIT(x) macro.
> >
> > Fix the checkpatch.pl issue:
> > CHECK: Prefer using the BIT macro replacing
> >
> > Signed-off-by: simran singhal 
> > ---
> >  
> >  V2:
> >    -modified patch message.
> >
> >  drivers/staging/rtl8192u/ieee80211/ieee80211.h | 140
> -
> >  1 file changed, 70 insertions(+), 70 deletions(-)
> 
> The last chunk of this patch did not apply, please fix up and resend.
> 
> thanks,
> 
> greg k-h
> 
> 
>   Can you please elaborate what's the problem with the last chunk of this 
> patch
> and 
>   to be more precise what do you mean by "last chunk of this patch". I am not
> getting 
>   what's the problem with this patch.

I don't know what the problem was, just that it did not apply properly.
Take your patch and try to apply it now to my tree and see what
happens...

thanks,

greg k-h

Re: [PATCH] staging: fsl-mc: fix coding style warning

[Greg KH]

On Fri, Feb 24, 2017 at 08:24:42PM +0200, Lucian Zala wrote:
> Hi Greg,
> 
> I am a little bit confused regarding your reply, it seems
> that the patch applied afterall, or?
> 
> http://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/drivers/staging/fsl-mc?h=staging-testing=3bee20f39e4df8d65e8d34ab9b3ab92e5acc
> 
> Have I messed something in it?

No, my mistake, I must have already applied it, sorry, dealing with
hundreds of patches causes confusion at times :)

sorry for the noise,

greg k-h

Re: [Outreachy kernel] Re: [PATCH v2 3/3] staging: rtl8192u: Prefer using the BIT macro

[Greg KH]

On Fri, Feb 24, 2017 at 12:10:21PM -0800, SIMRAN SINGHAL wrote:
> 
> 
> On Friday, February 24, 2017 at 10:46:21 PM UTC+5:30, gregkh wrote:
> 
> On Fri, Feb 17, 2017 at 11:20:59PM +0530, simran singhal wrote:
> > Replaces left shift operation (1 << d) by BIT(x) macro.
> >
> > Fix the checkpatch.pl issue:
> > CHECK: Prefer using the BIT macro replacing
> >
> > Signed-off-by: simran singhal 
> > ---
> >  
> >  V2:
> >    -modified patch message.
> >
> >  drivers/staging/rtl8192u/ieee80211/ieee80211.h | 140
> -
> >  1 file changed, 70 insertions(+), 70 deletions(-)
> 
> The last chunk of this patch did not apply, please fix up and resend.
> 
> thanks,
> 
> greg k-h
> 
> 
>   Can you please elaborate what's the problem with the last chunk of this 
> patch
> and 
>   to be more precise what do you mean by "last chunk of this patch". I am not
> getting 
>   what's the problem with this patch.

I don't know what the problem was, just that it did not apply properly.
Take your patch and try to apply it now to my tree and see what
happens...

thanks,

greg k-h

Re: [PATCH v2 6/6] staging: rtl8192e: Removed unnecessary parentheses

[Greg KH]

On Sat, Feb 25, 2017 at 04:18:27AM +0530, SIMRAN SINGHAL wrote:
> On Fri, Feb 24, 2017 at 10:47 PM, Greg KH  wrote:
> > On Mon, Feb 20, 2017 at 10:41:47PM +0530, simran singhal wrote:
> >> Extra parentheses were causing checkpatch issues
> >> and were removed.
> >>
> >> Signed-off-by: simran singhal 
> >> ---
> >>
> >>  v2:
> >>-Removed parentheses around argument of cast
> >>-Removed cast
> >
> > I can't keep track of random patches in the middle of a series that is
> > updated.
> >
> > Please fix up and resend the whole series, as a series, that are linked
> > together in an email client, not as independent emails (i.e. use
> > git-send-email properly).
> 
>  I am using mutt for sending patches. In mutt I have to send each patch
>  of a particular patch series one by one. So, they come as independent
>  emails.
> 
>  And, they all get mix because of the reason, if I started sending new patch
>  series and at the same time sending new versions of patches of previous
>  patch series, so because of which their order changes.

Please do not do that.  Would you like to receive patches out of order,
not threaded, and all mixed up in the middle of hundreds of other
patches?  How would you be able to figure it out?  Please consider that
someone has to read what you send out :)

>  So, please suggest me some better way, considering I am using mutt.

You can use mutt for that, but you need to know what you are doing and
how to thread emails with it.  It's not simple, so please just use git
send-email for now until you learn mutt better.

good luck!

greg k-h

Re: [PATCH v2 6/6] staging: rtl8192e: Removed unnecessary parentheses

[Greg KH]

On Sat, Feb 25, 2017 at 04:18:27AM +0530, SIMRAN SINGHAL wrote:
> On Fri, Feb 24, 2017 at 10:47 PM, Greg KH  wrote:
> > On Mon, Feb 20, 2017 at 10:41:47PM +0530, simran singhal wrote:
> >> Extra parentheses were causing checkpatch issues
> >> and were removed.
> >>
> >> Signed-off-by: simran singhal 
> >> ---
> >>
> >>  v2:
> >>-Removed parentheses around argument of cast
> >>-Removed cast
> >
> > I can't keep track of random patches in the middle of a series that is
> > updated.
> >
> > Please fix up and resend the whole series, as a series, that are linked
> > together in an email client, not as independent emails (i.e. use
> > git-send-email properly).
> 
>  I am using mutt for sending patches. In mutt I have to send each patch
>  of a particular patch series one by one. So, they come as independent
>  emails.
> 
>  And, they all get mix because of the reason, if I started sending new patch
>  series and at the same time sending new versions of patches of previous
>  patch series, so because of which their order changes.

Please do not do that.  Would you like to receive patches out of order,
not threaded, and all mixed up in the middle of hundreds of other
patches?  How would you be able to figure it out?  Please consider that
someone has to read what you send out :)

>  So, please suggest me some better way, considering I am using mutt.

You can use mutt for that, but you need to know what you are doing and
how to thread emails with it.  It's not simple, so please just use git
send-email for now until you learn mutt better.

good luck!

greg k-h

[PATCH v2] f2fs: change the codes of checking CP_CRC_RECOVERY_FLAG to macro

[Yunlong Song]

No need to check the "if" condition each time, just change it to macro codes.

Signed-off-by: Yunlong Song 
---
 fs/f2fs/node.h| 10 +-
 fs/f2fs/segment.c |  5 +++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
index 3fc9c4b..caebb23 100644
--- a/fs/f2fs/node.h
+++ b/fs/f2fs/node.h
@@ -303,11 +303,11 @@ static inline void fill_node_footer_blkaddr(struct page 
*page, block_t blkaddr)
size_t crc_offset = le32_to_cpu(ckpt->checksum_offset);
__u64 cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
 
-   if (__is_set_ckpt_flags(ckpt, CP_CRC_RECOVERY_FLAG)) {
-   __u64 crc = le32_to_cpu(*((__le32 *)
-   ((unsigned char *)ckpt + crc_offset)));
-   cp_ver |= (crc << 32);
-   }
+#ifdef CP_CRC_RECOVERY_FLAG
+   __u64 crc = le32_to_cpu(*((__le32 *)
+   ((unsigned char *)ckpt + crc_offset)));
+   cp_ver |= (crc << 32);
+#endif
rn->footer.cp_ver = cpu_to_le64(cp_ver);
rn->footer.next_blkaddr = cpu_to_le32(blkaddr);
 }
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9eb6d89..6c2e1ee 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1573,9 +1573,10 @@ static void allocate_segment_by_default(struct 
f2fs_sb_info *sbi,
 {
if (force)
new_curseg(sbi, type, true);
-   else if (!is_set_ckpt_flags(sbi, CP_CRC_RECOVERY_FLAG) &&
-   type == CURSEG_WARM_NODE)
+#ifndef CP_CRC_RECOVERY_FLAG
+   else if (type == CURSEG_WARM_NODE)
new_curseg(sbi, type, false);
+#endif
else if (need_SSR(sbi) && get_ssr_segment(sbi, type))
change_curseg(sbi, type, true);
else
-- 
1.8.5.2

[PATCH v2] f2fs: change the codes of checking CP_CRC_RECOVERY_FLAG to macro

[Yunlong Song]

No need to check the "if" condition each time, just change it to macro codes.

Signed-off-by: Yunlong Song 
---
 fs/f2fs/node.h| 10 +-
 fs/f2fs/segment.c |  5 +++--
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h
index 3fc9c4b..caebb23 100644
--- a/fs/f2fs/node.h
+++ b/fs/f2fs/node.h
@@ -303,11 +303,11 @@ static inline void fill_node_footer_blkaddr(struct page 
*page, block_t blkaddr)
size_t crc_offset = le32_to_cpu(ckpt->checksum_offset);
__u64 cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
 
-   if (__is_set_ckpt_flags(ckpt, CP_CRC_RECOVERY_FLAG)) {
-   __u64 crc = le32_to_cpu(*((__le32 *)
-   ((unsigned char *)ckpt + crc_offset)));
-   cp_ver |= (crc << 32);
-   }
+#ifdef CP_CRC_RECOVERY_FLAG
+   __u64 crc = le32_to_cpu(*((__le32 *)
+   ((unsigned char *)ckpt + crc_offset)));
+   cp_ver |= (crc << 32);
+#endif
rn->footer.cp_ver = cpu_to_le64(cp_ver);
rn->footer.next_blkaddr = cpu_to_le32(blkaddr);
 }
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 9eb6d89..6c2e1ee 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1573,9 +1573,10 @@ static void allocate_segment_by_default(struct 
f2fs_sb_info *sbi,
 {
if (force)
new_curseg(sbi, type, true);
-   else if (!is_set_ckpt_flags(sbi, CP_CRC_RECOVERY_FLAG) &&
-   type == CURSEG_WARM_NODE)
+#ifndef CP_CRC_RECOVERY_FLAG
+   else if (type == CURSEG_WARM_NODE)
new_curseg(sbi, type, false);
+#endif
else if (need_SSR(sbi) && get_ssr_segment(sbi, type))
change_curseg(sbi, type, true);
else
-- 
1.8.5.2

Re: [PATCH 5/7] watchdog: s3c2410: Constify local structures

[Krzysztof Kozlowski]

On Fri, Feb 24, 2017 at 02:05:56PM -0800, Guenter Roeck wrote:
> On 02/24/2017 01:07 PM, Krzysztof Kozlowski wrote:
> > Structures watchdog_device, watchdog_ops and s3c2410_wdt_variant are not
> > modified so they can be made const to increase code safeness.
> > 
> > Signed-off-by: Krzysztof Kozlowski 
> 
> Conflicts with "watchdog: constify watchdog_ops structures" in my tree.
> I rebased my tree to v4.10 (which I should not really have done ;-), but
> it still doesn't apply due to "fatal: sha1 information is lacking or useless"
> and "error: could not build fake ancestor". The subsequent patches also don't
> apply. You may have to wait until after -rc1, rebase your patches, and
> resubmit.

Sure, I'll resend after merge window. These aren't important fixes so
there is no problem in waiting for merge window to end.

Thanks for reviews.

Best regards,
Krzysztof

Re: [PATCH 5/7] watchdog: s3c2410: Constify local structures

[Krzysztof Kozlowski]

On Fri, Feb 24, 2017 at 02:05:56PM -0800, Guenter Roeck wrote:
> On 02/24/2017 01:07 PM, Krzysztof Kozlowski wrote:
> > Structures watchdog_device, watchdog_ops and s3c2410_wdt_variant are not
> > modified so they can be made const to increase code safeness.
> > 
> > Signed-off-by: Krzysztof Kozlowski 
> 
> Conflicts with "watchdog: constify watchdog_ops structures" in my tree.
> I rebased my tree to v4.10 (which I should not really have done ;-), but
> it still doesn't apply due to "fatal: sha1 information is lacking or useless"
> and "error: could not build fake ancestor". The subsequent patches also don't
> apply. You may have to wait until after -rc1, rebase your patches, and
> resubmit.

Sure, I'll resend after merge window. These aren't important fixes so
there is no problem in waiting for merge window to end.

Thanks for reviews.

Best regards,
Krzysztof

Re: [ANNOUNCE] Git v2.12.0

[Willy Tarreau]

Hi Junio,

On Fri, Feb 24, 2017 at 11:28:58AM -0800, Junio C Hamano wrote:
>  * Use of an empty string that is used for 'everything matches' is
>still warned and Git asks users to use a more explicit '.' for that
>instead.  The hope is that existing users will not mind this
>change, and eventually the warning can be turned into a hard error,
>upgrading the deprecation into removal of this (mis)feature.  That
>is not scheduled to happen in the upcoming release (yet).

FWIW '.' is not equivalent to '' when it comes to grep or such commands,
you should suggest '^' or '$' instead, otherwise you'll miss empty lines
and users will continue to use '' for that purpose. BTW I do use grep ''
a lot, but only on file systems, not within git (eg: to display contents
of /sys).

Cheers,
Willy

Re: [ANNOUNCE] Git v2.12.0

[Willy Tarreau]

Hi Junio,

On Fri, Feb 24, 2017 at 11:28:58AM -0800, Junio C Hamano wrote:
>  * Use of an empty string that is used for 'everything matches' is
>still warned and Git asks users to use a more explicit '.' for that
>instead.  The hope is that existing users will not mind this
>change, and eventually the warning can be turned into a hard error,
>upgrading the deprecation into removal of this (mis)feature.  That
>is not scheduled to happen in the upcoming release (yet).

FWIW '.' is not equivalent to '' when it comes to grep or such commands,
you should suggest '^' or '$' instead, otherwise you'll miss empty lines
and users will continue to use '' for that purpose. BTW I do use grep ''
a lot, but only on file systems, not within git (eg: to display contents
of /sys).

Cheers,
Willy

Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support

[Xiongfeng Wang]

Hi Tyler,


On 2017/2/22 5:22, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.
> 
> Signed-off-by: Tyler Baicar 
> ---
>  arch/arm/include/asm/kvm_arm.h   |  1 +
>  arch/arm/include/asm/system_misc.h   |  5 +
>  arch/arm/kvm/mmu.c   | 18 --
>  arch/arm64/include/asm/kvm_arm.h |  1 +
>  arch/arm64/include/asm/system_misc.h |  2 ++
>  arch/arm64/mm/fault.c| 18 ++
>  6 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..33a77509 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,7 @@
>  #define FSC_FAULT(0x04)
>  #define FSC_ACCESS   (0x08)
>  #define FSC_PERM (0x0c)
> +#define FSC_EXTABT   (0x10)
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK   (~0xf)
> diff --git a/arch/arm/include/asm/system_misc.h 
> b/arch/arm/include/asm/system_misc.h
> index a3d61ad..ea45d94 100644
> --- a/arch/arm/include/asm/system_misc.h
> +++ b/arch/arm/include/asm/system_misc.h
> @@ -24,4 +24,9 @@
>  
>  #endif /* !__ASSEMBLY__ */
>  
> +static inline int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> + return -1;
> +}
> +
>  #endif /* __ASM_ARM_SYSTEM_MISC_H */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..04f1dd50 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "trace.h"
>  
> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, 
> struct kvm_run *run)
>  
>   /* Check the stage-2 fault is trans. fault or write fault */
>   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> - if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> - fault_status != FSC_ACCESS) {
> +
> + /* The host kernel will handle the synchronous external abort. There
> +  * is no need to pass the error into the guest.
> +  */

Can we inject an sea into the guest, so that the guest can kill the
application which causes the error if the guest won't be terminated
later. I'm not sure whether ghes_handle_memory_failure() called in
ghes_do_proc() will kill the qemu process. I think it only kill user
processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.

> + if (fault_status == FSC_EXTABT) {
> + if(handle_guest_sea((unsigned long)fault_ipa,
> + kvm_vcpu_get_hsr(vcpu))) {
> + kvm_err("Failed to handle guest SEA, FSC: EC=%#x 
> xFSC=%#lx ESR_EL2=%#lx\n",
> + kvm_vcpu_trap_get_class(vcpu),
> + (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> + (unsigned long)kvm_vcpu_get_hsr(vcpu));
> + return -EFAULT;
> + }
> + } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +fault_status != FSC_ACCESS) {
>   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>   kvm_vcpu_trap_get_class(vcpu),
>   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> diff --git a/arch/arm64/include/asm/kvm_arm.h 
> b/arch/arm64/include/asm/kvm_arm.h
> index 2a2752b..2b11d59 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -201,6 +201,7 @@
>  #define FSC_FAULTESR_ELx_FSC_FAULT
>  #define FSC_ACCESS   ESR_ELx_FSC_ACCESS
>  #define FSC_PERM ESR_ELx_FSC_PERM
> +#define FSC_EXTABT   ESR_ELx_FSC_EXTABT
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK   (~UL(0xf))
> diff --git a/arch/arm64/include/asm/system_misc.h 
> b/arch/arm64/include/asm/system_misc.h
> index bc81243..5b2cecd1 100644
> --- a/arch/arm64/include/asm/system_misc.h
> +++ b/arch/arm64/include/asm/system_misc.h
> @@ -58,4 +58,6 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, 
> unsigned int,
>  
>  #endif   /* __ASSEMBLY__ */
>  
> +int handle_guest_sea(unsigned long addr, unsigned int esr);
> +
>  #endif   /* __ASM_SYSTEM_MISC_H */
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..403277b 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>  }
>  
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> + /*
> +  * synchronize_rcu() will wait for nmi_exit(), so no need to
> +  * rcu_read_lock().
> +  */
> + if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
> + nmi_enter();
> + 

Re: [PATCH V11 10/10] arm/arm64: KVM: add guest SEA support

[Xiongfeng Wang]

Hi Tyler,


On 2017/2/22 5:22, Tyler Baicar wrote:
> Currently external aborts are unsupported by the guest abort
> handling. Add handling for SEAs so that the host kernel reports
> SEAs which occur in the guest kernel.
> 
> Signed-off-by: Tyler Baicar 
> ---
>  arch/arm/include/asm/kvm_arm.h   |  1 +
>  arch/arm/include/asm/system_misc.h   |  5 +
>  arch/arm/kvm/mmu.c   | 18 --
>  arch/arm64/include/asm/kvm_arm.h |  1 +
>  arch/arm64/include/asm/system_misc.h |  2 ++
>  arch/arm64/mm/fault.c| 18 ++
>  6 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/kvm_arm.h b/arch/arm/include/asm/kvm_arm.h
> index e22089f..33a77509 100644
> --- a/arch/arm/include/asm/kvm_arm.h
> +++ b/arch/arm/include/asm/kvm_arm.h
> @@ -187,6 +187,7 @@
>  #define FSC_FAULT(0x04)
>  #define FSC_ACCESS   (0x08)
>  #define FSC_PERM (0x0c)
> +#define FSC_EXTABT   (0x10)
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK   (~0xf)
> diff --git a/arch/arm/include/asm/system_misc.h 
> b/arch/arm/include/asm/system_misc.h
> index a3d61ad..ea45d94 100644
> --- a/arch/arm/include/asm/system_misc.h
> +++ b/arch/arm/include/asm/system_misc.h
> @@ -24,4 +24,9 @@
>  
>  #endif /* !__ASSEMBLY__ */
>  
> +static inline int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> + return -1;
> +}
> +
>  #endif /* __ASM_ARM_SYSTEM_MISC_H */
> diff --git a/arch/arm/kvm/mmu.c b/arch/arm/kvm/mmu.c
> index a5265ed..04f1dd50 100644
> --- a/arch/arm/kvm/mmu.c
> +++ b/arch/arm/kvm/mmu.c
> @@ -29,6 +29,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "trace.h"
>  
> @@ -1444,8 +1445,21 @@ int kvm_handle_guest_abort(struct kvm_vcpu *vcpu, 
> struct kvm_run *run)
>  
>   /* Check the stage-2 fault is trans. fault or write fault */
>   fault_status = kvm_vcpu_trap_get_fault_type(vcpu);
> - if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> - fault_status != FSC_ACCESS) {
> +
> + /* The host kernel will handle the synchronous external abort. There
> +  * is no need to pass the error into the guest.
> +  */

Can we inject an sea into the guest, so that the guest can kill the
application which causes the error if the guest won't be terminated
later. I'm not sure whether ghes_handle_memory_failure() called in
ghes_do_proc() will kill the qemu process. I think it only kill user
processes marked with PF_MCE_PROCESS & PF_MCE_EARLY.

> + if (fault_status == FSC_EXTABT) {
> + if(handle_guest_sea((unsigned long)fault_ipa,
> + kvm_vcpu_get_hsr(vcpu))) {
> + kvm_err("Failed to handle guest SEA, FSC: EC=%#x 
> xFSC=%#lx ESR_EL2=%#lx\n",
> + kvm_vcpu_trap_get_class(vcpu),
> + (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> + (unsigned long)kvm_vcpu_get_hsr(vcpu));
> + return -EFAULT;
> + }
> + } else if (fault_status != FSC_FAULT && fault_status != FSC_PERM &&
> +fault_status != FSC_ACCESS) {
>   kvm_err("Unsupported FSC: EC=%#x xFSC=%#lx ESR_EL2=%#lx\n",
>   kvm_vcpu_trap_get_class(vcpu),
>   (unsigned long)kvm_vcpu_trap_get_fault(vcpu),
> diff --git a/arch/arm64/include/asm/kvm_arm.h 
> b/arch/arm64/include/asm/kvm_arm.h
> index 2a2752b..2b11d59 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -201,6 +201,7 @@
>  #define FSC_FAULTESR_ELx_FSC_FAULT
>  #define FSC_ACCESS   ESR_ELx_FSC_ACCESS
>  #define FSC_PERM ESR_ELx_FSC_PERM
> +#define FSC_EXTABT   ESR_ELx_FSC_EXTABT
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK   (~UL(0xf))
> diff --git a/arch/arm64/include/asm/system_misc.h 
> b/arch/arm64/include/asm/system_misc.h
> index bc81243..5b2cecd1 100644
> --- a/arch/arm64/include/asm/system_misc.h
> +++ b/arch/arm64/include/asm/system_misc.h
> @@ -58,4 +58,6 @@ void hook_debug_fault_code(int nr, int (*fn)(unsigned long, 
> unsigned int,
>  
>  #endif   /* __ASSEMBLY__ */
>  
> +int handle_guest_sea(unsigned long addr, unsigned int esr);
> +
>  #endif   /* __ASM_SYSTEM_MISC_H */
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index b2d57fc..403277b 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -602,6 +602,24 @@ static const char *fault_name(unsigned int esr)
>  }
>  
>  /*
> + * Handle Synchronous External Aborts that occur in a guest kernel.
> + */
> +int handle_guest_sea(unsigned long addr, unsigned int esr)
> +{
> + /*
> +  * synchronize_rcu() will wait for nmi_exit(), so no need to
> +  * rcu_read_lock().
> +  */
> + if(IS_ENABLED(HAVE_ACPI_APEI_SEA)) {
> + nmi_enter();
> + ghes_notify_sea();
> + 

[PATCH RFC] arm64/mm: handle memmap kernel option

[Yisheng Xie]

When use device tree mode, user can reserve memory by changes the dts,
however, when boot with ACPI, user cannot reserve memory except by
changing the ACPI table in BIOS, which is not so convenient.

To make user reserve memory for some specific use more convenient,
this patch implement the following memmap variants:
 - memmap=nn[KMG]$ss[KMG]: mark specified memory as reserved;
 - memmap=nn[KMG]@ss[KMG]: force usage of a specific region of memory;

Reported-by: Bob Dong 
Signed-off-by: Yisheng Xie 
---
 arch/arm64/mm/init.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index e19e065..cf90c1d 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -188,6 +188,52 @@ static int __init early_mem(char *p)
 }
 early_param("mem", early_mem);
 
+static void __init parse_memmap_one(char *p)
+{
+   char *oldp;
+   unsigned long start_at, mem_size;
+
+   if (!p)
+   return;
+
+   oldp = p;
+   mem_size = memparse(p, );
+   if (p == oldp)
+   return;
+
+   switch (*p) {
+   case '@':
+   start_at = memparse(p + 1, );
+   memblock_add(start_at, mem_size);
+   break;
+
+   case '$':
+   start_at = memparse(p + 1, );
+   memblock_reserve(start_at, mem_size);
+   break;
+
+   default:
+   pr_warn("Unrecognized memmap syntax: %s\n", p);
+   break;
+   }
+}
+
+static int __init parse_memmap_opt(char *str)
+{
+   while (str) {
+   char *k = strchr(str, ',');
+
+   if (k)
+   *k++ = 0;
+
+   parse_memmap_one(str);
+   str = k;
+   }
+
+   return 0;
+}
+early_param("memmap", parse_memmap_opt);
+
 void __init arm64_memblock_init(void)
 {
const s64 linear_region_size = -(s64)PAGE_OFFSET;
-- 
1.7.12.4

[PATCH RFC] arm64/mm: handle memmap kernel option

[Yisheng Xie]

When use device tree mode, user can reserve memory by changes the dts,
however, when boot with ACPI, user cannot reserve memory except by
changing the ACPI table in BIOS, which is not so convenient.

To make user reserve memory for some specific use more convenient,
this patch implement the following memmap variants:
 - memmap=nn[KMG]$ss[KMG]: mark specified memory as reserved;
 - memmap=nn[KMG]@ss[KMG]: force usage of a specific region of memory;

Reported-by: Bob Dong 
Signed-off-by: Yisheng Xie 
---
 arch/arm64/mm/init.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index e19e065..cf90c1d 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -188,6 +188,52 @@ static int __init early_mem(char *p)
 }
 early_param("mem", early_mem);
 
+static void __init parse_memmap_one(char *p)
+{
+   char *oldp;
+   unsigned long start_at, mem_size;
+
+   if (!p)
+   return;
+
+   oldp = p;
+   mem_size = memparse(p, );
+   if (p == oldp)
+   return;
+
+   switch (*p) {
+   case '@':
+   start_at = memparse(p + 1, );
+   memblock_add(start_at, mem_size);
+   break;
+
+   case '$':
+   start_at = memparse(p + 1, );
+   memblock_reserve(start_at, mem_size);
+   break;
+
+   default:
+   pr_warn("Unrecognized memmap syntax: %s\n", p);
+   break;
+   }
+}
+
+static int __init parse_memmap_opt(char *str)
+{
+   while (str) {
+   char *k = strchr(str, ',');
+
+   if (k)
+   *k++ = 0;
+
+   parse_memmap_one(str);
+   str = k;
+   }
+
+   return 0;
+}
+early_param("memmap", parse_memmap_opt);
+
 void __init arm64_memblock_init(void)
 {
const s64 linear_region_size = -(s64)PAGE_OFFSET;
-- 
1.7.12.4

Re: [Outreachy kernel] [PATCH v4 1/3] staging: wilc1000: Rename struct tstrRSSI to rssi_history_buffer

[Julia Lawall]

On Fri, 24 Feb 2017, Tahia Khan wrote:

> Rename struct tstrRSSI to rssi_history_buffer for clarity
> and to remove camel casing.
>
> Signed-off-by: Tahia Khan 

Acked-by: Julia Lawall 


> ---
>  drivers/staging/wilc1000/coreconfigurator.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/coreconfigurator.h 
> b/drivers/staging/wilc1000/coreconfigurator.h
> index cff1698..1c77529 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.h
> +++ b/drivers/staging/wilc1000/coreconfigurator.h
> @@ -70,7 +70,7 @@ enum connect_status {
>   CONNECT_STS_FORCE_16_BIT = 0x
>  };
>
> -struct tstrRSSI {
> +struct rssi_history_buffer {
>   u8 u8Full;
>   u8 u8Index;
>   s8 as8RSSI[NUM_RSSI];
> @@ -93,7 +93,7 @@ struct network_info {
>   u8 *ies;
>   u16 ies_len;
>   void *join_params;
> - struct tstrRSSI str_rssi;
> + struct rssi_history_buffer str_rssi;
>   u64 tsf_hi;
>  };
>
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/da73d1aef379243ba94a5c1a4e1d6663160451fd.1487947616.git.tahia.khan%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH v4 1/3] staging: wilc1000: Rename struct tstrRSSI to rssi_history_buffer

[Julia Lawall]

On Fri, 24 Feb 2017, Tahia Khan wrote:

> Rename struct tstrRSSI to rssi_history_buffer for clarity
> and to remove camel casing.
>
> Signed-off-by: Tahia Khan 

Acked-by: Julia Lawall 


> ---
>  drivers/staging/wilc1000/coreconfigurator.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/coreconfigurator.h 
> b/drivers/staging/wilc1000/coreconfigurator.h
> index cff1698..1c77529 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.h
> +++ b/drivers/staging/wilc1000/coreconfigurator.h
> @@ -70,7 +70,7 @@ enum connect_status {
>   CONNECT_STS_FORCE_16_BIT = 0x
>  };
>
> -struct tstrRSSI {
> +struct rssi_history_buffer {
>   u8 u8Full;
>   u8 u8Index;
>   s8 as8RSSI[NUM_RSSI];
> @@ -93,7 +93,7 @@ struct network_info {
>   u8 *ies;
>   u16 ies_len;
>   void *join_params;
> - struct tstrRSSI str_rssi;
> + struct rssi_history_buffer str_rssi;
>   u64 tsf_hi;
>  };
>
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/da73d1aef379243ba94a5c1a4e1d6663160451fd.1487947616.git.tahia.khan%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH] staging: xgifb: function prototype argument should have an identifier name

[Julia Lawall]

On Sat, 25 Feb 2017, Arushi Singhal wrote:

>
>
> On Fri, Feb 24, 2017 at 10:18 PM, Julia Lawall  wrote:
>
>
>   On Fri, 24 Feb 2017, Arushi Singhal wrote:
>
>   >
>   >
>   > On Fri, Feb 24, 2017 at 10:01 PM, Greg Kroah-Hartman
>   >  wrote:
>   >       On Wed, Feb 22, 2017 at 04:49:19PM +0530, Arushi Singhal
>   wrote:
>   >       > function prototype arguments like 'struct
>   vb_device_info
>   >       *','unsigned
>   >       > long' etc. should have an identifier name.
>   >       >
>   >       > Signed-off-by: Arushi Singhal
>   >       
>   >       > ---
>   >       > Changes in v3:
>   >       >   - By mistake one irrelevant line was added which is
>   removed
>   >       in this
>   >       >     patch.
>   >       >   - write the changes done in previous version in
>   correct
>   >       format.
>   >       >   - make the commit subject more relevant and
>   accurate.
>   >
>   >       This patch doesn't apply to my tree at all :(
>   >
>   > Why this is so?
>
>   Maybe someone else made a change on the same lines as yours. 
>   Check that
>   your tree is up to date.
>
> Is all the changes being done by someone before?

I don't know.  Just update your tree and try to apply your patch again.
Or look at the code that your patch should apply to.  It should be easy to
see what is wrong.

julia

>
>   julia
>
>   > thanks
>   > Arushi
>   >       sorry,
>   >
>   >       greg k-h
>   >
>   >
> > --
> > You received this message because you are subscribed to the Google
> Groups
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> send an
> > email to outreachy-kernel+unsubscr...@googlegroups.com.
> > To post to this group, send email to
> outreachy-ker...@googlegroups.com.
> > To view this discussion on the 
> > webvisithttps://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF_sihk6_Uhg
> LzUH6
> > FVSFMu02Vadp1tjatQft-UJ14SJeA%40mail.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
> >
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web 
> visithttps://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF91bd_er%2BidvUB
> PZCTYFHaxF6Mgf1oQ6vTz9gmk6ggC6Q%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

Re: [Outreachy kernel] Re: [PATCH] staging: xgifb: function prototype argument should have an identifier name

[Julia Lawall]

On Sat, 25 Feb 2017, Arushi Singhal wrote:

>
>
> On Fri, Feb 24, 2017 at 10:18 PM, Julia Lawall  wrote:
>
>
>   On Fri, 24 Feb 2017, Arushi Singhal wrote:
>
>   >
>   >
>   > On Fri, Feb 24, 2017 at 10:01 PM, Greg Kroah-Hartman
>   >  wrote:
>   >       On Wed, Feb 22, 2017 at 04:49:19PM +0530, Arushi Singhal
>   wrote:
>   >       > function prototype arguments like 'struct
>   vb_device_info
>   >       *','unsigned
>   >       > long' etc. should have an identifier name.
>   >       >
>   >       > Signed-off-by: Arushi Singhal
>   >       
>   >       > ---
>   >       > Changes in v3:
>   >       >   - By mistake one irrelevant line was added which is
>   removed
>   >       in this
>   >       >     patch.
>   >       >   - write the changes done in previous version in
>   correct
>   >       format.
>   >       >   - make the commit subject more relevant and
>   accurate.
>   >
>   >       This patch doesn't apply to my tree at all :(
>   >
>   > Why this is so?
>
>   Maybe someone else made a change on the same lines as yours. 
>   Check that
>   your tree is up to date.
>
> Is all the changes being done by someone before?

I don't know.  Just update your tree and try to apply your patch again.
Or look at the code that your patch should apply to.  It should be easy to
see what is wrong.

julia

>
>   julia
>
>   > thanks
>   > Arushi
>   >       sorry,
>   >
>   >       greg k-h
>   >
>   >
> > --
> > You received this message because you are subscribed to the Google
> Groups
> > "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> send an
> > email to outreachy-kernel+unsubscr...@googlegroups.com.
> > To post to this group, send email to
> outreachy-ker...@googlegroups.com.
> > To view this discussion on the 
> > webvisithttps://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF_sihk6_Uhg
> LzUH6
> > FVSFMu02Vadp1tjatQft-UJ14SJeA%40mail.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.
> >
> >
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web 
> visithttps://groups.google.com/d/msgid/outreachy-kernel/CA%2BXqjF91bd_er%2BidvUB
> PZCTYFHaxF6Mgf1oQ6vTz9gmk6ggC6Q%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

Re: [Outreachy kernel] [PATCH] staging: ks7010: codes in the comments are removed

[Julia Lawall]

Remember to use the imperative and to make your subject lines as short as
possible.  Here the subject line could be "remove code in comments".

julia

On Sat, 25 Feb 2017, Arushi Singhal wrote:

> Commenting Code Is a Bad Idea.
> Comments are their to explain the code and how the code achieves its
> goal and as codes in the comments  does not explain what the code is
> doing so there is no use of commenting them.
> So in thos patch codes in the comments are removed.
>
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
> b/drivers/staging/ks7010/ks7010_sdio.c
> index 2c263f98bdbb..54aa2e2f516f 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -495,10 +495,6 @@ static void ks7010_rw_function(struct work_struct *work)
>   /* wiat after WAKEUP */
>   while (time_after(priv->last_wakeup + ((30 * HZ) / 1000), jiffies)) {
>   DPRINTK(4, "wait after WAKEUP\n");
> -/*
> - *   
> queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,>ks_wlan_hw.rw_wq,
> - *   (priv->last_wakeup + ((30*HZ)/1000) - jiffies));
> - */
>   dev_info(>ks_wlan_hw.sdio_card->func->dev,
>"wake: %lu %lu\n",
>priv->last_wakeup + (30 * HZ) / 1000,
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170224231154.GA32083%40arushi-HP-Pavilion-Notebook.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] [PATCH] staging: ks7010: codes in the comments are removed

[Julia Lawall]

Remember to use the imperative and to make your subject lines as short as
possible.  Here the subject line could be "remove code in comments".

julia

On Sat, 25 Feb 2017, Arushi Singhal wrote:

> Commenting Code Is a Bad Idea.
> Comments are their to explain the code and how the code achieves its
> goal and as codes in the comments  does not explain what the code is
> doing so there is no use of commenting them.
> So in thos patch codes in the comments are removed.
>
> Signed-off-by: Arushi Singhal 
> ---
>  drivers/staging/ks7010/ks7010_sdio.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
> b/drivers/staging/ks7010/ks7010_sdio.c
> index 2c263f98bdbb..54aa2e2f516f 100644
> --- a/drivers/staging/ks7010/ks7010_sdio.c
> +++ b/drivers/staging/ks7010/ks7010_sdio.c
> @@ -495,10 +495,6 @@ static void ks7010_rw_function(struct work_struct *work)
>   /* wiat after WAKEUP */
>   while (time_after(priv->last_wakeup + ((30 * HZ) / 1000), jiffies)) {
>   DPRINTK(4, "wait after WAKEUP\n");
> -/*
> - *   
> queue_delayed_work(priv->ks_wlan_hw.ks7010sdio_wq,>ks_wlan_hw.rw_wq,
> - *   (priv->last_wakeup + ((30*HZ)/1000) - jiffies));
> - */
>   dev_info(>ks_wlan_hw.sdio_card->func->dev,
>"wake: %lu %lu\n",
>priv->last_wakeup + (30 * HZ) / 1000,
> --
> 2.11.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20170224231154.GA32083%40arushi-HP-Pavilion-Notebook.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [PATCH V2] f2fs: introduce free nid bitmap

[Chao Yu]

Hi Jaegeuk,

I added below diff code into this patch in order to fix incorrectly nid status
updating to reduce large latency of generic/251 in fstest, could you help to
review code below?

Latency:beforeafter
generic/251 1483s ... 184s

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 52db02396878..d25dcadf901a 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1774,23 +1774,24 @@ static int add_free_nid(struct f2fs_sb_info *sbi, nid_t
nid, bool build)

/* 0 nid should not be used */
if (unlikely(nid == 0))
-   return 0;
+   return 1;

if (build) {
/* do not add allocated nids */
ne = __lookup_nat_cache(nm_i, nid);
if (ne && (!get_nat_flag(ne, IS_CHECKPOINTED) ||
nat_get_blkaddr(ne) != NULL_ADDR))
-   return 0;
+   return 1;
}

i = f2fs_kmem_cache_alloc(free_nid_slab, GFP_NOFS);
i->nid = nid;
i->state = NID_NEW;

-   if (radix_tree_preload(GFP_NOFS)) {
+   err = radix_tree_preload(GFP_NOFS);
+   if (err) {
kmem_cache_free(free_nid_slab, i);
-   return 0;
+   return err;
}

spin_lock(_i->nid_list_lock);
@@ -1799,9 +1800,9 @@ static int add_free_nid(struct f2fs_sb_info *sbi, nid_t
nid, bool build)
radix_tree_preload_end();
if (err) {
kmem_cache_free(free_nid_slab, i);
-   return 0;
+   return err;
}
-   return 1;
+   return 0;
 }

 static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
@@ -1844,7 +1845,7 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
struct f2fs_nat_block *nat_blk = page_address(nat_page);
block_t blk_addr;
unsigned int nat_ofs = NAT_BLOCK_OFFSET(start_nid);
-   int i;
+   int i, ret;

set_bit_le(nat_ofs, nm_i->nat_block_bitmap);

@@ -1857,9 +1858,12 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,

blk_addr = le32_to_cpu(nat_blk->entries[i].block_addr);
f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
-   if (blk_addr == NULL_ADDR)
-   add_free_nid(sbi, start_nid, true);
-   update_free_nid_bitmap(sbi, start_nid, blk_addr == NULL_ADDR);
+   if (blk_addr == NULL_ADDR) {
+   ret = add_free_nid(sbi, start_nid, true);
+   update_free_nid_bitmap(sbi, start_nid, ret <= 0);
+   } else {
+   update_free_nid_bitmap(sbi, start_nid, false);
+   }
}
 }


On 2017/2/23 10:53, Chao Yu wrote:
> In scenario of intensively node allocation, free nids will be ran out
> soon, then it needs to stop to load free nids by traversing NAT blocks,
> in worse case, if NAT blocks does not be cached in memory, it generates
> IOs which slows down our foreground operations.
> 
> In order to speed up node allocation, in this patch we introduce a new
> free_nid_bitmap array, so there is an bitmap table for each NAT block,
> Once the NAT block is loaded, related bitmap cache will be switched on,
> and bitmap will be set during traversing nat entries in NAT block, later
> we can query and update nid usage status in memory completely.
> 
> With such implementation, I expect performance of node allocation can be
> improved in the long-term after filesystem image is mounted.
> 
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/debug.c |   2 +
>  fs/f2fs/f2fs.h  |   2 +
>  fs/f2fs/node.c  | 109 
> ++--
>  include/linux/f2fs_fs.h |   1 +
>  4 files changed, 111 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 015ad2b73a92..a77df377e2e8 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -194,6 +194,8 @@ static void update_mem_info(struct f2fs_sb_info *sbi)
>   si->base_mem += sizeof(struct f2fs_nm_info);
>   si->base_mem += __bitmap_size(sbi, NAT_BITMAP);
>   si->base_mem += (NM_I(sbi)->nat_bits_blocks << F2FS_BLKSIZE_BITS);
> + si->base_mem += NM_I(sbi)->nat_blocks * NAT_ENTRY_BITMAP_SIZE;
> + si->base_mem += NM_I(sbi)->nat_blocks / 8;
>  
>  get_cache:
>   si->cache_mem = 0;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 1c9f0cc8f027..6a48e649bfef 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -556,6 +556,8 @@ struct f2fs_nm_info {
>   unsigned int nid_cnt[MAX_NID_LIST]; /* the number of free node id */
>   spinlock_t nid_list_lock;   /* protect nid lists ops */
>   struct mutex build_lock;/* lock for build free nids */
> + unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
> + unsigned char *nat_block_bitmap;
>  
>   /* for checkpoint */
>   char *nat_bitmap;   /* NAT bitmap pointer */
> diff --git a/fs/f2fs/node.c 

Re: [PATCH V2] f2fs: introduce free nid bitmap

[Chao Yu]

Hi Jaegeuk,

I added below diff code into this patch in order to fix incorrectly nid status
updating to reduce large latency of generic/251 in fstest, could you help to
review code below?

Latency:beforeafter
generic/251 1483s ... 184s

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 52db02396878..d25dcadf901a 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1774,23 +1774,24 @@ static int add_free_nid(struct f2fs_sb_info *sbi, nid_t
nid, bool build)

/* 0 nid should not be used */
if (unlikely(nid == 0))
-   return 0;
+   return 1;

if (build) {
/* do not add allocated nids */
ne = __lookup_nat_cache(nm_i, nid);
if (ne && (!get_nat_flag(ne, IS_CHECKPOINTED) ||
nat_get_blkaddr(ne) != NULL_ADDR))
-   return 0;
+   return 1;
}

i = f2fs_kmem_cache_alloc(free_nid_slab, GFP_NOFS);
i->nid = nid;
i->state = NID_NEW;

-   if (radix_tree_preload(GFP_NOFS)) {
+   err = radix_tree_preload(GFP_NOFS);
+   if (err) {
kmem_cache_free(free_nid_slab, i);
-   return 0;
+   return err;
}

spin_lock(_i->nid_list_lock);
@@ -1799,9 +1800,9 @@ static int add_free_nid(struct f2fs_sb_info *sbi, nid_t
nid, bool build)
radix_tree_preload_end();
if (err) {
kmem_cache_free(free_nid_slab, i);
-   return 0;
+   return err;
}
-   return 1;
+   return 0;
 }

 static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
@@ -1844,7 +1845,7 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
struct f2fs_nat_block *nat_blk = page_address(nat_page);
block_t blk_addr;
unsigned int nat_ofs = NAT_BLOCK_OFFSET(start_nid);
-   int i;
+   int i, ret;

set_bit_le(nat_ofs, nm_i->nat_block_bitmap);

@@ -1857,9 +1858,12 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,

blk_addr = le32_to_cpu(nat_blk->entries[i].block_addr);
f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
-   if (blk_addr == NULL_ADDR)
-   add_free_nid(sbi, start_nid, true);
-   update_free_nid_bitmap(sbi, start_nid, blk_addr == NULL_ADDR);
+   if (blk_addr == NULL_ADDR) {
+   ret = add_free_nid(sbi, start_nid, true);
+   update_free_nid_bitmap(sbi, start_nid, ret <= 0);
+   } else {
+   update_free_nid_bitmap(sbi, start_nid, false);
+   }
}
 }


On 2017/2/23 10:53, Chao Yu wrote:
> In scenario of intensively node allocation, free nids will be ran out
> soon, then it needs to stop to load free nids by traversing NAT blocks,
> in worse case, if NAT blocks does not be cached in memory, it generates
> IOs which slows down our foreground operations.
> 
> In order to speed up node allocation, in this patch we introduce a new
> free_nid_bitmap array, so there is an bitmap table for each NAT block,
> Once the NAT block is loaded, related bitmap cache will be switched on,
> and bitmap will be set during traversing nat entries in NAT block, later
> we can query and update nid usage status in memory completely.
> 
> With such implementation, I expect performance of node allocation can be
> improved in the long-term after filesystem image is mounted.
> 
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/debug.c |   2 +
>  fs/f2fs/f2fs.h  |   2 +
>  fs/f2fs/node.c  | 109 
> ++--
>  include/linux/f2fs_fs.h |   1 +
>  4 files changed, 111 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 015ad2b73a92..a77df377e2e8 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -194,6 +194,8 @@ static void update_mem_info(struct f2fs_sb_info *sbi)
>   si->base_mem += sizeof(struct f2fs_nm_info);
>   si->base_mem += __bitmap_size(sbi, NAT_BITMAP);
>   si->base_mem += (NM_I(sbi)->nat_bits_blocks << F2FS_BLKSIZE_BITS);
> + si->base_mem += NM_I(sbi)->nat_blocks * NAT_ENTRY_BITMAP_SIZE;
> + si->base_mem += NM_I(sbi)->nat_blocks / 8;
>  
>  get_cache:
>   si->cache_mem = 0;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 1c9f0cc8f027..6a48e649bfef 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -556,6 +556,8 @@ struct f2fs_nm_info {
>   unsigned int nid_cnt[MAX_NID_LIST]; /* the number of free node id */
>   spinlock_t nid_list_lock;   /* protect nid lists ops */
>   struct mutex build_lock;/* lock for build free nids */
> + unsigned char (*free_nid_bitmap)[NAT_ENTRY_BITMAP_SIZE];
> + unsigned char *nat_block_bitmap;
>  
>   /* for checkpoint */
>   char *nat_bitmap;   /* NAT bitmap pointer */
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index 

Re: [Outreachy kernel] [PATCH v2] staging: xgifb: correct the multiple line dereference

[Julia Lawall]

On Sat, 25 Feb 2017, Arushi Singhal wrote:

> Error was reported by checkpatch.pl as
> WARNING: Avoid multiple line dereference...
> And If there is boolean operator then it is fixed by Splitting line at
> boolean operator.This is a coding style error.

There is strange spacing and capitalization in the above two lines.  Only
the first word in a sentence should be capitalized.  There should be at
least one space after a period.

> The changes are made such that the other errors does not generate
> beacause of this change like line exceeding 80 characters length.

This doesn't say anything about what you did.

> Signed-off-by: Arushi Singhal 
> ---
>  changes in v2
>  - changes done such that no other errors can generate.
> ---

You don't need --- here.  Only the one just below the Signed off by.

julia

>  drivers/staging/xgifb/XGI_main_26.c | 21 +
>  drivers/staging/xgifb/vb_setmode.c  | 14 +++---
>  2 files changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/xgifb/XGI_main_26.c 
> b/drivers/staging/xgifb/XGI_main_26.c
> index 6930f7eb741b..651987398d42 100644
> --- a/drivers/staging/xgifb/XGI_main_26.c
> +++ b/drivers/staging/xgifb/XGI_main_26.c
> @@ -878,30 +878,27 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
> *xgifb_info)
>   }
>
>   if ((filter >= 0) && (filter <= 7)) {
> + const u8 *f = 
> XGI_TV_filter[filter_tb].filter[filter];
> +
>   pr_debug("FilterTable[%d]-%d: %*ph\n",
>filter_tb, filter,
> -  4, XGI_TV_filter[filter_tb].
> -filter[filter]);
> +  4, f);
>   xgifb_reg_set(
>   XGIPART2,
>   0x35,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][0]));
> - xgifb_reg_set(
> + (f[0]));
> + xgifb_reg_set(
>   XGIPART2,
>   0x36,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][1]));
> - xgifb_reg_set(
> + (f[1]));
> + xgifb_reg_set(
>   XGIPART2,
>   0x37,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][2]));
> + (f[2]));
>   xgifb_reg_set(
>   XGIPART2,
>   0x38,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][3]));
> + (f[3]));
>   }
>   }
>   }
> diff --git a/drivers/staging/xgifb/vb_setmode.c 
> b/drivers/staging/xgifb/vb_setmode.c
> index 7c7c8c8f1df3..1220d0cea87d 100644
> --- a/drivers/staging/xgifb/vb_setmode.c
> +++ b/drivers/staging/xgifb/vb_setmode.c
> @@ -221,8 +221,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
> ModeIdIndex,
>
>   for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
>  tempbx; (*i)--) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> + infoflag =
> + XGI330_RefIndex[RefreshRateTableIndex + (*i)].Ext_InfoFlag;
>   if (infoflag & tempax)
>   return 1;
>
> @@ -231,8 +231,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
> ModeIdIndex,
>   }
>
>   for ((*i) = 0;; (*i)++) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> + infoflag =
> + XGI330_RefIndex[RefreshRateTableIndex + (*i)].Ext_InfoFlag;
>   if (XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID
>   != tempbx) {
>   return 0;
> @@ -5092,8 +5092,8 @@ unsigned short XGI_GetRatePtrCRT2(struct 
> xgi_hw_device_info *pXGIHWDE,
>
>   i = 0;
>   do {
> - if (XGI330_RefIndex[RefreshRateTableIndex + i].
> - ModeID != ModeNo)
> + if (XGI330_RefIndex[RefreshRateTableIndex + i].ModeID
> + != ModeNo)
>   break;
>   temp = XGI330_RefIndex[RefreshRateTableIndex + i].Ext_InfoFlag;
>

Re: [Outreachy kernel] [PATCH v2] staging: xgifb: correct the multiple line dereference

[Julia Lawall]

On Sat, 25 Feb 2017, Arushi Singhal wrote:

> Error was reported by checkpatch.pl as
> WARNING: Avoid multiple line dereference...
> And If there is boolean operator then it is fixed by Splitting line at
> boolean operator.This is a coding style error.

There is strange spacing and capitalization in the above two lines.  Only
the first word in a sentence should be capitalized.  There should be at
least one space after a period.

> The changes are made such that the other errors does not generate
> beacause of this change like line exceeding 80 characters length.

This doesn't say anything about what you did.

> Signed-off-by: Arushi Singhal 
> ---
>  changes in v2
>  - changes done such that no other errors can generate.
> ---

You don't need --- here.  Only the one just below the Signed off by.

julia

>  drivers/staging/xgifb/XGI_main_26.c | 21 +
>  drivers/staging/xgifb/vb_setmode.c  | 14 +++---
>  2 files changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/staging/xgifb/XGI_main_26.c 
> b/drivers/staging/xgifb/XGI_main_26.c
> index 6930f7eb741b..651987398d42 100644
> --- a/drivers/staging/xgifb/XGI_main_26.c
> +++ b/drivers/staging/xgifb/XGI_main_26.c
> @@ -878,30 +878,27 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
> *xgifb_info)
>   }
>
>   if ((filter >= 0) && (filter <= 7)) {
> + const u8 *f = 
> XGI_TV_filter[filter_tb].filter[filter];
> +
>   pr_debug("FilterTable[%d]-%d: %*ph\n",
>filter_tb, filter,
> -  4, XGI_TV_filter[filter_tb].
> -filter[filter]);
> +  4, f);
>   xgifb_reg_set(
>   XGIPART2,
>   0x35,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][0]));
> - xgifb_reg_set(
> + (f[0]));
> + xgifb_reg_set(
>   XGIPART2,
>   0x36,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][1]));
> - xgifb_reg_set(
> + (f[1]));
> + xgifb_reg_set(
>   XGIPART2,
>   0x37,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][2]));
> + (f[2]));
>   xgifb_reg_set(
>   XGIPART2,
>   0x38,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][3]));
> + (f[3]));
>   }
>   }
>   }
> diff --git a/drivers/staging/xgifb/vb_setmode.c 
> b/drivers/staging/xgifb/vb_setmode.c
> index 7c7c8c8f1df3..1220d0cea87d 100644
> --- a/drivers/staging/xgifb/vb_setmode.c
> +++ b/drivers/staging/xgifb/vb_setmode.c
> @@ -221,8 +221,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
> ModeIdIndex,
>
>   for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
>  tempbx; (*i)--) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> + infoflag =
> + XGI330_RefIndex[RefreshRateTableIndex + (*i)].Ext_InfoFlag;
>   if (infoflag & tempax)
>   return 1;
>
> @@ -231,8 +231,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short 
> ModeIdIndex,
>   }
>
>   for ((*i) = 0;; (*i)++) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> + infoflag =
> + XGI330_RefIndex[RefreshRateTableIndex + (*i)].Ext_InfoFlag;
>   if (XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID
>   != tempbx) {
>   return 0;
> @@ -5092,8 +5092,8 @@ unsigned short XGI_GetRatePtrCRT2(struct 
> xgi_hw_device_info *pXGIHWDE,
>
>   i = 0;
>   do {
> - if (XGI330_RefIndex[RefreshRateTableIndex + i].
> - ModeID != ModeNo)
> + if (XGI330_RefIndex[RefreshRateTableIndex + i].ModeID
> + != ModeNo)
>   break;
>   temp = XGI330_RefIndex[RefreshRateTableIndex + i].Ext_InfoFlag;
>   temp &= ModeTypeMask;

[PATCH 2/6] ARM: dts: socfpga: Add unit name to memory nodes

[Florian Vaussard]

Memory nodes in Arria5, Cyclone5 and Arria10 do not have a unit name.
This will trigger several warnings like this one (when compiled with
W=1):

Node /memory has a reg or ranges property, but no unit name

Add the corresponding unit name to each node.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga_arria10_socdk.dtsi   | 2 +-
 arch/arm/boot/dts/socfpga_arria5_socdk.dts | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts  | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi| 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_socdk.dts   | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_sockit.dts  | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_socrates.dts| 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_sodia.dts   | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts | 2 +-
 arch/arm/boot/dts/socfpga_vt.dts   | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi 
b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
index c57e6ce..4d0a729 100644
--- a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
@@ -30,7 +30,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_arria5_socdk.dts 
b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
index 8672edf..aac4fee 100644
--- a/arch/arm/boot/dts/socfpga_arria5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
@@ -26,7 +26,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
index 5ecd2ef..7b49395 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
@@ -25,7 +25,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi 
b/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi
index 6ad3b1eb..3c03da6 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi
+++ b/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi
@@ -21,7 +21,7 @@
model = "Aries/DENX MCV";
compatible = "altr,socfpga-cyclone5", "altr,socfpga";
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1 GiB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
index 7ea32c8..155829f 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
@@ -26,7 +26,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
index a0c90b3b..a4a555c 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
@@ -26,7 +26,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
index c3d52f2..8e3b017 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
@@ -25,7 +25,7 @@
bootargs = "console=ttyS0,115200";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
index d69ad29..8860dd2 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
@@ -28,7 +28,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>;
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
index 363ee62..8931980 100644
--- 

[PATCH 6/6] ARM: dts: socfpga: Do not include skeleton.dtsi

[Florian Vaussard]

The skeleton.dtsi file is now deprecated as noted in commit 9c0da3cc61f1
("ARM: dts: explicitly mark skeleton.dtsi as deprecated"). The SoCFPGA
device trees already contain the nodes that are defined in skeleton.dtsi
(#address-cells, #size-cells, chosen, aliases, memory).

Including skeleton.dtsi is useless and will produce the following
warning when compiled with W=1:

Node /memory has a reg or ranges property, but no unit name

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga.dtsi | 1 -
 arch/arm/boot/dts/socfpga_arria10.dtsi | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 25a68af..774c239 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -15,7 +15,6 @@
  * along with this program.  If not, see .
  */
 
-#include "skeleton.dtsi"
 #include 
 
 / {
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi 
b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 4e1f653..bead79e 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -14,7 +14,6 @@
  * this program.  If not, see .
  */
 
-#include "skeleton.dtsi"
 #include 
 #include 
 
-- 
2.7.4

[PATCH 1/6] ARM: dts: socfpga: Add unit name to clock nodes

[Florian Vaussard]

Most clock nodes in Arria5, Cyclone5 and Arria10 have a reg property but
does not have a unit name. This will trigger several warnings like this
one (when compiled with W=1):

Node /soc/clkmgr@ffd04000/clocks/periph_pll has a reg or ranges
property, but no unit name

Add the corresponding unit name to each node.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga.dtsi | 38 ++--
 arch/arm/boot/dts/socfpga_arria10.dtsi | 46 +-
 2 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 2c43c4d..4dda6d8 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -145,7 +145,7 @@
compatible = "fixed-clock";
};
 
-   main_pll: main_pll {
+   main_pll: main_pll@40 {
#address-cells = <1>;
#size-cells = <0>;
#clock-cells = <0>;
@@ -153,7 +153,7 @@
clocks = <>;
reg = <0x40>;
 
-   mpuclk: mpuclk {
+   mpuclk: mpuclk@48 {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
@@ -161,7 +161,7 @@
reg = <0x48>;
};
 
-   mainclk: mainclk {
+   mainclk: mainclk@4c {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
@@ -169,7 +169,7 @@
reg = <0x4C>;
};
 
-   dbg_base_clk: dbg_base_clk {
+   dbg_base_clk: dbg_base_clk@50 {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>, 
<>;
@@ -177,21 +177,21 @@
reg = <0x50>;
};
 
-   main_qspi_clk: main_qspi_clk {
+   main_qspi_clk: main_qspi_clk@54 
{
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
reg = <0x54>;
};
 
-   main_nand_sdmmc_clk: 
main_nand_sdmmc_clk {
+   main_nand_sdmmc_clk: 
main_nand_sdmmc_clk@58 {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
reg = <0x58>;
};
 
-   cfg_h2f_usr0_clk: 
cfg_h2f_usr0_clk {
+   cfg_h2f_usr0_clk: 
cfg_h2f_usr0_clk@5c {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
@@ -199,7 +199,7 @@
};
};
 
-   periph_pll: periph_pll {
+   periph_pll: periph_pll@80 {
#address-cells = <1>;
#size-cells = <0>;
#clock-cells = <0>;
@@ -207,42 +207,42 @@
   

[PATCH 2/6] ARM: dts: socfpga: Add unit name to memory nodes

[Florian Vaussard]

Memory nodes in Arria5, Cyclone5 and Arria10 do not have a unit name.
This will trigger several warnings like this one (when compiled with
W=1):

Node /memory has a reg or ranges property, but no unit name

Add the corresponding unit name to each node.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga_arria10_socdk.dtsi   | 2 +-
 arch/arm/boot/dts/socfpga_arria5_socdk.dts | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts  | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi| 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_socdk.dts   | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_sockit.dts  | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_socrates.dts| 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_sodia.dts   | 2 +-
 arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts | 2 +-
 arch/arm/boot/dts/socfpga_vt.dts   | 2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi 
b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
index c57e6ce..4d0a729 100644
--- a/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10_socdk.dtsi
@@ -30,7 +30,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_arria5_socdk.dts 
b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
index 8672edf..aac4fee 100644
--- a/arch/arm/boot/dts/socfpga_arria5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_arria5_socdk.dts
@@ -26,7 +26,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
index 5ecd2ef..7b49395 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts
@@ -25,7 +25,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi 
b/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi
index 6ad3b1eb..3c03da6 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi
+++ b/arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi
@@ -21,7 +21,7 @@
model = "Aries/DENX MCV";
compatible = "altr,socfpga-cyclone5", "altr,socfpga";
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1 GiB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
index 7ea32c8..155829f 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socdk.dts
@@ -26,7 +26,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
index a0c90b3b..a4a555c 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sockit.dts
@@ -26,7 +26,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
index c3d52f2..8e3b017 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
@@ -25,7 +25,7 @@
bootargs = "console=ttyS0,115200";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>; /* 1GB */
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
index d69ad29..8860dd2 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_sodia.dts
@@ -28,7 +28,7 @@
stdout-path = "serial0:115200n8";
};
 
-   memory {
+   memory@0 {
name = "memory";
device_type = "memory";
reg = <0x0 0x4000>;
diff --git a/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts
index 363ee62..8931980 100644
--- 

[PATCH 6/6] ARM: dts: socfpga: Do not include skeleton.dtsi

[Florian Vaussard]

The skeleton.dtsi file is now deprecated as noted in commit 9c0da3cc61f1
("ARM: dts: explicitly mark skeleton.dtsi as deprecated"). The SoCFPGA
device trees already contain the nodes that are defined in skeleton.dtsi
(#address-cells, #size-cells, chosen, aliases, memory).

Including skeleton.dtsi is useless and will produce the following
warning when compiled with W=1:

Node /memory has a reg or ranges property, but no unit name

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga.dtsi | 1 -
 arch/arm/boot/dts/socfpga_arria10.dtsi | 1 -
 2 files changed, 2 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 25a68af..774c239 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -15,7 +15,6 @@
  * along with this program.  If not, see .
  */
 
-#include "skeleton.dtsi"
 #include 
 
 / {
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi 
b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 4e1f653..bead79e 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -14,7 +14,6 @@
  * this program.  If not, see .
  */
 
-#include "skeleton.dtsi"
 #include 
 #include 
 
-- 
2.7.4

[PATCH 1/6] ARM: dts: socfpga: Add unit name to clock nodes

[Florian Vaussard]

Most clock nodes in Arria5, Cyclone5 and Arria10 have a reg property but
does not have a unit name. This will trigger several warnings like this
one (when compiled with W=1):

Node /soc/clkmgr@ffd04000/clocks/periph_pll has a reg or ranges
property, but no unit name

Add the corresponding unit name to each node.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga.dtsi | 38 ++--
 arch/arm/boot/dts/socfpga_arria10.dtsi | 46 +-
 2 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 2c43c4d..4dda6d8 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -145,7 +145,7 @@
compatible = "fixed-clock";
};
 
-   main_pll: main_pll {
+   main_pll: main_pll@40 {
#address-cells = <1>;
#size-cells = <0>;
#clock-cells = <0>;
@@ -153,7 +153,7 @@
clocks = <>;
reg = <0x40>;
 
-   mpuclk: mpuclk {
+   mpuclk: mpuclk@48 {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
@@ -161,7 +161,7 @@
reg = <0x48>;
};
 
-   mainclk: mainclk {
+   mainclk: mainclk@4c {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
@@ -169,7 +169,7 @@
reg = <0x4C>;
};
 
-   dbg_base_clk: dbg_base_clk {
+   dbg_base_clk: dbg_base_clk@50 {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>, 
<>;
@@ -177,21 +177,21 @@
reg = <0x50>;
};
 
-   main_qspi_clk: main_qspi_clk {
+   main_qspi_clk: main_qspi_clk@54 
{
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
reg = <0x54>;
};
 
-   main_nand_sdmmc_clk: 
main_nand_sdmmc_clk {
+   main_nand_sdmmc_clk: 
main_nand_sdmmc_clk@58 {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
reg = <0x58>;
};
 
-   cfg_h2f_usr0_clk: 
cfg_h2f_usr0_clk {
+   cfg_h2f_usr0_clk: 
cfg_h2f_usr0_clk@5c {
#clock-cells = <0>;
compatible = 
"altr,socfpga-perip-clk";
clocks = <_pll>;
@@ -199,7 +199,7 @@
};
};
 
-   periph_pll: periph_pll {
+   periph_pll: periph_pll@80 {
#address-cells = <1>;
#size-cells = <0>;
#clock-cells = <0>;
@@ -207,42 +207,42 @@
clocks = <>, 

[PATCH 3/6] ARM: dts: socfpga: Remove unneeded unit names

[Florian Vaussard]

Node eccmgr has a unit name, but do not have a reg property as only the
child nodes do have this property. Likewise the usbphy node do not have
a reg property. This will trigger the following warnings when compiled
with W=1:

Node /soc/eccmgr@ffd08140 has a unit name, but no reg property
Node /soc/usbphy@0 has a unit name, but no reg property

Remove the superfluous unit names.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga.dtsi | 4 ++--
 arch/arm/boot/dts/socfpga_arria10.dtsi | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 4dda6d8..25a68af 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -678,7 +678,7 @@
status = "disabled";
};
 
-   eccmgr: eccmgr@ffd08140 {
+   eccmgr: eccmgr {
compatible = "altr,socfpga-ecc-manager";
#address-cells = <1>;
#size-cells = <1>;
@@ -879,7 +879,7 @@
dma-names = "tx", "rx";
};
 
-   usbphy0: usbphy@0 {
+   usbphy0: usbphy {
#phy-cells = <0>;
compatible = "usb-nop-xceiv";
status = "okay";
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi 
b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 0332d51..4e1f653 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -649,7 +649,7 @@
reg = <0xffe0 0x4>;
};
 
-   eccmgr: eccmgr@ffd06000 {
+   eccmgr: eccmgr {
compatible = "altr,socfpga-a10-ecc-manager";
altr,sysmgr-syscon = <>;
#address-cells = <1>;
@@ -806,7 +806,7 @@
status = "disabled";
};
 
-   usbphy0: usbphy@0 {
+   usbphy0: usbphy {
#phy-cells = <0>;
compatible = "usb-nop-xceiv";
status = "okay";
-- 
2.7.4

[PATCH 5/6] ARM: dts: socfpga: Remove unit name for LEDs in EBV SOCrates

[Florian Vaussard]

GPIO LEDs in the Cyclone5 EBV SOCrates board have a unit name but no reg
property. Indeed, GPIO LEDs do not need such a property. They do not
need a unit name neither. This will trigger the following warnings when
compiled with W=1:

Node /gpio-leds/led@0 has a unit name, but no reg property
Node /gpio-leds/led@1 has a unit name, but no reg property
Node /gpio-leds/led@2 has a unit name, but no reg property

The solution is to remove the unit name. In order to have unique node
names, a rename is necessary. This should be harmless as all the LEDs
have a 'label' property, hence their name do not derive from the node
name and will stay the same after this patch.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga_cyclone5_socrates.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
index 8e3b017..53bf99e 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
@@ -60,18 +60,18 @@
  {
compatible = "gpio-leds";
 
-   led@0 {
+   led0 {
label = "led:green:heartbeat";
gpios = < 28 1>;
linux,default-trigger = "heartbeat";
};
 
-   led@1 {
+   led1 {
label = "led:green:D7";
gpios = < 19 1>;
};
 
-   led@2 {
+   led2 {
label = "led:green:D8";
gpios = < 25 1>;
};
-- 
2.7.4

[PATCH 3/6] ARM: dts: socfpga: Remove unneeded unit names

[Florian Vaussard]

Node eccmgr has a unit name, but do not have a reg property as only the
child nodes do have this property. Likewise the usbphy node do not have
a reg property. This will trigger the following warnings when compiled
with W=1:

Node /soc/eccmgr@ffd08140 has a unit name, but no reg property
Node /soc/usbphy@0 has a unit name, but no reg property

Remove the superfluous unit names.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga.dtsi | 4 ++--
 arch/arm/boot/dts/socfpga_arria10.dtsi | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga.dtsi b/arch/arm/boot/dts/socfpga.dtsi
index 4dda6d8..25a68af 100644
--- a/arch/arm/boot/dts/socfpga.dtsi
+++ b/arch/arm/boot/dts/socfpga.dtsi
@@ -678,7 +678,7 @@
status = "disabled";
};
 
-   eccmgr: eccmgr@ffd08140 {
+   eccmgr: eccmgr {
compatible = "altr,socfpga-ecc-manager";
#address-cells = <1>;
#size-cells = <1>;
@@ -879,7 +879,7 @@
dma-names = "tx", "rx";
};
 
-   usbphy0: usbphy@0 {
+   usbphy0: usbphy {
#phy-cells = <0>;
compatible = "usb-nop-xceiv";
status = "okay";
diff --git a/arch/arm/boot/dts/socfpga_arria10.dtsi 
b/arch/arm/boot/dts/socfpga_arria10.dtsi
index 0332d51..4e1f653 100644
--- a/arch/arm/boot/dts/socfpga_arria10.dtsi
+++ b/arch/arm/boot/dts/socfpga_arria10.dtsi
@@ -649,7 +649,7 @@
reg = <0xffe0 0x4>;
};
 
-   eccmgr: eccmgr@ffd06000 {
+   eccmgr: eccmgr {
compatible = "altr,socfpga-a10-ecc-manager";
altr,sysmgr-syscon = <>;
#address-cells = <1>;
@@ -806,7 +806,7 @@
status = "disabled";
};
 
-   usbphy0: usbphy@0 {
+   usbphy0: usbphy {
#phy-cells = <0>;
compatible = "usb-nop-xceiv";
status = "okay";
-- 
2.7.4

[PATCH 5/6] ARM: dts: socfpga: Remove unit name for LEDs in EBV SOCrates

[Florian Vaussard]

GPIO LEDs in the Cyclone5 EBV SOCrates board have a unit name but no reg
property. Indeed, GPIO LEDs do not need such a property. They do not
need a unit name neither. This will trigger the following warnings when
compiled with W=1:

Node /gpio-leds/led@0 has a unit name, but no reg property
Node /gpio-leds/led@1 has a unit name, but no reg property
Node /gpio-leds/led@2 has a unit name, but no reg property

The solution is to remove the unit name. In order to have unique node
names, a rename is necessary. This should be harmless as all the LEDs
have a 'label' property, hence their name do not derive from the node
name and will stay the same after this patch.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga_cyclone5_socrates.dts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
index 8e3b017..53bf99e 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_socrates.dts
@@ -60,18 +60,18 @@
  {
compatible = "gpio-leds";
 
-   led@0 {
+   led0 {
label = "led:green:heartbeat";
gpios = < 28 1>;
linux,default-trigger = "heartbeat";
};
 
-   led@1 {
+   led1 {
label = "led:green:D7";
gpios = < 19 1>;
};
 
-   led@2 {
+   led2 {
label = "led:green:D8";
gpios = < 25 1>;
};
-- 
2.7.4

[PATCH 0/6] ARM: dts: socfpga: Fix dtc warnings

[Florian Vaussard]

We get a bunch of warnings when compiling the SoCFPGA device trees with W=1.
This warnings happens because some nodes have a unit name but no 'reg' property,
or are missing a unit name while having a 'reg' property. This series enables to
compile all SoCFPGA dts without warnings.

Patches 1 to 5 fix these warnings by adding the unit name or removing the
'reg' property when approriate.

Patch 6 removes the inclusion of the deprecated skeleton.dtsi file, as SoCFPGA
device trees already define the mandatory properties and nodes. This is needed
to remove the latest warning.

This series was boot tested on a Cyclone5 SoC DK, thus covering some of the
changes dones by patches 1 -> 3 + 6.

Regards,
Florian

Florian Vaussard (6):
  ARM: dts: socfpga: Add unit name to clock nodes
  ARM: dts: socfpga: Add unit name to memory nodes
  ARM: dts: socfpga: Remove unneeded unit names
  ARM: dts: socfpga: Remove unneeded reg from stmpe_touchscreen
  ARM: dts: socfpga: Remove unit name for LEDs in EBV SOCrates
  ARM: dts: socfpga: Do not include skeleton.dtsi

 arch/arm/boot/dts/socfpga.dtsi | 43 +-
 arch/arm/boot/dts/socfpga_arria10.dtsi | 51 +++---
 arch/arm/boot/dts/socfpga_arria10_socdk.dtsi   |  2 +-
 arch/arm/boot/dts/socfpga_arria5_socdk.dts |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts  |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi|  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts  |  1 -
 arch/arm/boot/dts/socfpga_cyclone5_socdk.dts   |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_sockit.dts  |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_socrates.dts|  8 ++--
 arch/arm/boot/dts/socfpga_cyclone5_sodia.dts   |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts |  2 +-
 arch/arm/boot/dts/socfpga_vt.dts   |  2 +-
 13 files changed, 59 insertions(+), 62 deletions(-)

-- 
2.7.4

[PATCH 0/6] ARM: dts: socfpga: Fix dtc warnings

[Florian Vaussard]

We get a bunch of warnings when compiling the SoCFPGA device trees with W=1.
This warnings happens because some nodes have a unit name but no 'reg' property,
or are missing a unit name while having a 'reg' property. This series enables to
compile all SoCFPGA dts without warnings.

Patches 1 to 5 fix these warnings by adding the unit name or removing the
'reg' property when approriate.

Patch 6 removes the inclusion of the deprecated skeleton.dtsi file, as SoCFPGA
device trees already define the mandatory properties and nodes. This is needed
to remove the latest warning.

This series was boot tested on a Cyclone5 SoC DK, thus covering some of the
changes dones by patches 1 -> 3 + 6.

Regards,
Florian

Florian Vaussard (6):
  ARM: dts: socfpga: Add unit name to clock nodes
  ARM: dts: socfpga: Add unit name to memory nodes
  ARM: dts: socfpga: Remove unneeded unit names
  ARM: dts: socfpga: Remove unneeded reg from stmpe_touchscreen
  ARM: dts: socfpga: Remove unit name for LEDs in EBV SOCrates
  ARM: dts: socfpga: Do not include skeleton.dtsi

 arch/arm/boot/dts/socfpga.dtsi | 43 +-
 arch/arm/boot/dts/socfpga_arria10.dtsi | 51 +++---
 arch/arm/boot/dts/socfpga_arria10_socdk.dtsi   |  2 +-
 arch/arm/boot/dts/socfpga_arria5_socdk.dts |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_de0_sockit.dts  |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_mcv.dtsi|  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts  |  1 -
 arch/arm/boot/dts/socfpga_cyclone5_socdk.dts   |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_sockit.dts  |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_socrates.dts|  8 ++--
 arch/arm/boot/dts/socfpga_cyclone5_sodia.dts   |  2 +-
 arch/arm/boot/dts/socfpga_cyclone5_vining_fpga.dts |  2 +-
 arch/arm/boot/dts/socfpga_vt.dts   |  2 +-
 13 files changed, 59 insertions(+), 62 deletions(-)

-- 
2.7.4

[PATCH 4/6] ARM: dts: socfpga: Remove unneeded reg from stmpe_touchscreen

[Florian Vaussard]

The stmpe_touchscreen node in Cyclone5 MCV EVK has a reg property, but
this is not used by the driver. Moreover the binding documentation do
not define this property. Having a reg property without a unit name will
trigger the following warning when compiled with W=1:

Node /soc/i2c@ffc04000/stmpe811@41/stmpe_touchscreen has a reg or ranges
property, but no unit name

Remove the superfluous reg property.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
index e5a98e5..21e3972 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
@@ -71,7 +71,6 @@
 
stmpe_touchscreen {
compatible = "st,stmpe-ts";
-   reg = <0>;
ts,sample-time = <4>;
ts,mod-12b = <1>;
ts,ref-sel = <0>;
-- 
2.7.4

[PATCH 4/6] ARM: dts: socfpga: Remove unneeded reg from stmpe_touchscreen

[Florian Vaussard]

The stmpe_touchscreen node in Cyclone5 MCV EVK has a reg property, but
this is not used by the driver. Moreover the binding documentation do
not define this property. Having a reg property without a unit name will
trigger the following warning when compiled with W=1:

Node /soc/i2c@ffc04000/stmpe811@41/stmpe_touchscreen has a reg or ranges
property, but no unit name

Remove the superfluous reg property.

Signed-off-by: Florian Vaussard 
---
 arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts 
b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
index e5a98e5..21e3972 100644
--- a/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
+++ b/arch/arm/boot/dts/socfpga_cyclone5_mcvevk.dts
@@ -71,7 +71,6 @@
 
stmpe_touchscreen {
compatible = "st,stmpe-ts";
-   reg = <0>;
ts,sample-time = <4>;
ts,mod-12b = <1>;
ts,ref-sel = <0>;
-- 
2.7.4

Re: v4.10: kernel stack frame pointer .. has bad value (null)

[Josh Poimboeuf]

On Thu, Feb 23, 2017 at 09:10:39PM +0100, Pavel Machek wrote:
> Hi!
> 
> 
> > > > > Somehow, startup_32_smp() is on the stack twice.  The stack unwind led
> > > > > to the startup_32_smp() frame at 0xf50cdf9c rather than the one at
> > > > > 0xf50cdfa8 (which is where it should normally be).  So the question is
> > > > > how startup_32_smp() got executed the second time, with the wrong 
> > > > > stack
> > > > > offset.
> > > > 
> > > > Not much idea... but this is stack dump, right? Just because some
> > > > value is on the stack does not mean it is a return address, no?
> > > 
> > > Right, but the one at 0xf50cdfa8 is where the startup_32_smp() is
> > > *supposed* to be.  If the unwinder had unwinded to that one, it wouldn't
> > > have complained.  So it looks to me like the CPU somehow booted twice:
> > > the first time at the right stack address, and the second time it
> > > somehow ended up with a different stack address.
> > > 
> > > > And  startup_32_smp is kind of "interesting" function. Take a
> > > > look...
> > > 
> > > Yes, it's used in bringing up the CPU.
> > 
> > Can you share your .config?  
> 
> Here you go...

What version of gcc are you using?

Can you post a disassembly of the first 10 instructions of
start_secondary()?

-- 
Josh

Re: v4.10: kernel stack frame pointer .. has bad value (null)

[Josh Poimboeuf]

On Thu, Feb 23, 2017 at 09:10:39PM +0100, Pavel Machek wrote:
> Hi!
> 
> 
> > > > > Somehow, startup_32_smp() is on the stack twice.  The stack unwind led
> > > > > to the startup_32_smp() frame at 0xf50cdf9c rather than the one at
> > > > > 0xf50cdfa8 (which is where it should normally be).  So the question is
> > > > > how startup_32_smp() got executed the second time, with the wrong 
> > > > > stack
> > > > > offset.
> > > > 
> > > > Not much idea... but this is stack dump, right? Just because some
> > > > value is on the stack does not mean it is a return address, no?
> > > 
> > > Right, but the one at 0xf50cdfa8 is where the startup_32_smp() is
> > > *supposed* to be.  If the unwinder had unwinded to that one, it wouldn't
> > > have complained.  So it looks to me like the CPU somehow booted twice:
> > > the first time at the right stack address, and the second time it
> > > somehow ended up with a different stack address.
> > > 
> > > > And  startup_32_smp is kind of "interesting" function. Take a
> > > > look...
> > > 
> > > Yes, it's used in bringing up the CPU.
> > 
> > Can you share your .config?  
> 
> Here you go...

What version of gcc are you using?

Can you post a disassembly of the first 10 instructions of
start_secondary()?

-- 
Josh

Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

[Namhyung Kim]

Hi Arnaldo,

On Fri, Feb 24, 2017 at 06:08:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> > The cpu_map__snprint_mask() is to generate string representation of
> > cpumask bitmap.  For cpu 0 to 11, it'll return "fff".
> > 
> > Cc: Steven Rostedt 
> > Cc: Frederic Weisbecker 
> > Signed-off-by: Namhyung Kim 
> > ---
> >  tools/perf/util/cpumap.c | 46 
> > ++
> >  tools/perf/util/cpumap.h |  1 +
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> > index 8c7504939113..08540ab2a891 100644
> > --- a/tools/perf/util/cpumap.c
> > +++ b/tools/perf/util/cpumap.c
> > @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char 
> > *buf, size_t size)
> > pr_debug("cpumask list: %s\n", buf);
> > return ret;
> >  }
> > +
> > +static char hex_char(char val)
> 
> Why do you use 'char' above and...

My bad.  The argument should be unsigned.  The semantics is that it
converts a bitmap value into a character (string).

> 
> > +{
> > +   if (0 <= val && val <= 9)
> > +   return val + '0';
> > +   if (10 <= val && val < 16)
> > +   return val - 10 + 'a';
> > +   return '?';
> > +}
> > +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
> 
> > +   for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
> > +   unsigned char bits = bitmap[cpu / 8];
> 
> 'unsigned char' here?
> 
> Some compilers don't like it, for instance:
> 
>   19 fedora:24-x-ARC-uClibc: FAIL
> 
>   CC   /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:2: error: comparison is always true due to limited range of 
> data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
>   ^
> cc1: all warnings being treated as errors
> 
> And:
> 
>   10 debian:experimental-x-arm64: FAIL
> 
>   CC   /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:8: error: comparison is always true due to limited range of 
> data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
> ^~
> 
> Are you ok with the patch below?

I'd rather change hex_char() instead.  How about this?

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 6ab8699f0233..061018b42393 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -674,11 +674,11 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, 
size_t size)
return ret;
 }
 
-static char hex_char(char val)
+static char hex_char(unsigned char val)
 {
-   if (0 <= val && val <= 9)
+   if (val < 10)
return val + '0';
-   if (10 <= val && val < 16)
+   if (val < 16)
return val - 10 + 'a';
return '?';
 }

Re: [PATCH 2/4] perf tools: Introduce cpu_map__snprint_mask()

[Namhyung Kim]

Hi Arnaldo,

On Fri, Feb 24, 2017 at 06:08:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Feb 24, 2017 at 10:12:49AM +0900, Namhyung Kim escreveu:
> > The cpu_map__snprint_mask() is to generate string representation of
> > cpumask bitmap.  For cpu 0 to 11, it'll return "fff".
> > 
> > Cc: Steven Rostedt 
> > Cc: Frederic Weisbecker 
> > Signed-off-by: Namhyung Kim 
> > ---
> >  tools/perf/util/cpumap.c | 46 
> > ++
> >  tools/perf/util/cpumap.h |  1 +
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> > index 8c7504939113..08540ab2a891 100644
> > --- a/tools/perf/util/cpumap.c
> > +++ b/tools/perf/util/cpumap.c
> > @@ -673,3 +673,49 @@ size_t cpu_map__snprint(struct cpu_map *map, char 
> > *buf, size_t size)
> > pr_debug("cpumask list: %s\n", buf);
> > return ret;
> >  }
> > +
> > +static char hex_char(char val)
> 
> Why do you use 'char' above and...

My bad.  The argument should be unsigned.  The semantics is that it
converts a bitmap value into a character (string).

> 
> > +{
> > +   if (0 <= val && val <= 9)
> > +   return val + '0';
> > +   if (10 <= val && val < 16)
> > +   return val - 10 + 'a';
> > +   return '?';
> > +}
> > +size_t cpu_map__snprint_mask(struct cpu_map *map, char *buf, size_t size)
> 
> > +   for (cpu = last_cpu / 4 * 4; cpu >= 0; cpu -= 4) {
> > +   unsigned char bits = bitmap[cpu / 8];
> 
> 'unsigned char' here?
> 
> Some compilers don't like it, for instance:
> 
>   19 fedora:24-x-ARC-uClibc: FAIL
> 
>   CC   /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:2: error: comparison is always true due to limited range of 
> data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
>   ^
> cc1: all warnings being treated as errors
> 
> And:
> 
>   10 debian:experimental-x-arm64: FAIL
> 
>   CC   /tmp/build/perf/util/cpumap.o
> util/cpumap.c: In function 'hex_char':
> util/cpumap.c:679:8: error: comparison is always true due to limited range of 
> data type [-Werror=type-limits]
>   if (0 <= val && val <= 9)
> ^~
> 
> Are you ok with the patch below?

I'd rather change hex_char() instead.  How about this?

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 6ab8699f0233..061018b42393 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -674,11 +674,11 @@ size_t cpu_map__snprint(struct cpu_map *map, char *buf, 
size_t size)
return ret;
 }
 
-static char hex_char(char val)
+static char hex_char(unsigned char val)
 {
-   if (0 <= val && val <= 9)
+   if (val < 10)
return val + '0';
-   if (10 <= val && val < 16)
+   if (val < 16)
return val - 10 + 'a';
return '?';
 }

[PATCH] objtool: prevent gcc from merging annotate_unreachable()

[Josh Poimboeuf]

0-day bot reported some new objtool warnings which were caused by the
new annotate_unreachable() macro:

  fs/afs/flock.o: warning: objtool: afs_do_unlk()+0x0: duplicate frame pointer 
save
  fs/afs/flock.o: warning: objtool: afs_do_unlk()+0x0: frame pointer state 
mismatch
  fs/btrfs/delayed-inode.o: warning: objtool: 
btrfs_delete_delayed_dir_index()+0x0: duplicate frame pointer save
  fs/btrfs/delayed-inode.o: warning: objtool: 
btrfs_delete_delayed_dir_index()+0x0: frame pointer state mismatch
  fs/dlm/lock.o: warning: objtool: _grant_lock()+0x0: duplicate frame pointer 
save
  fs/dlm/lock.o: warning: objtool: _grant_lock()+0x0: frame pointer state 
mismatch
  fs/ocfs2/alloc.o: warning: objtool: ocfs2_mv_path()+0x0: duplicate frame 
pointer save
  fs/ocfs2/alloc.o: warning: objtool: ocfs2_mv_path()+0x0: frame pointer state 
mismatch

It turns out that, for older versions of gcc, if a function has multiple
BUG() incantations, gcc will sometimes merge the corresponding
annotate_unreachable() inline asm statements into a single block.  That
has the undesirable effect of removing one of the entries in the
__unreachable section, confusing objtool greatly.

A workaround for this issue is to ensure that each instance of the
inline asm statement uses a different label, so that gcc sees the
statements are unique and leaves them alone.  The inline asm ‘%=’ token
could be used for that, but unfortunately older versions of gcc don't
support it.  So I implemented a poor man's version of it with the
__LINE__ macro.

Fixes: d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
Reported-by: kbuild test robot 
Cc: Peter Zijlstra 
Signed-off-by: Josh Poimboeuf 
---
 include/linux/compiler-gcc.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 8ea159f..de47134 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -197,10 +197,10 @@
 
 #ifdef CONFIG_STACK_VALIDATION
 #define annotate_unreachable() ({  \
-   asm("1:\t\n"\
+   asm("%c0:\t\n"  \
".pushsection __unreachable, \"a\"\t\n" \
-   ".long 1b\t\n"  \
-   ".popsection\t\n"); \
+   ".long %c0b\t\n"\
+   ".popsection\t\n" : : "i" (__LINE__));  \
 })
 #else
 #define annotate_unreachable()
-- 
2.7.4

[PATCH] objtool: prevent gcc from merging annotate_unreachable()

[Josh Poimboeuf]

0-day bot reported some new objtool warnings which were caused by the
new annotate_unreachable() macro:

  fs/afs/flock.o: warning: objtool: afs_do_unlk()+0x0: duplicate frame pointer 
save
  fs/afs/flock.o: warning: objtool: afs_do_unlk()+0x0: frame pointer state 
mismatch
  fs/btrfs/delayed-inode.o: warning: objtool: 
btrfs_delete_delayed_dir_index()+0x0: duplicate frame pointer save
  fs/btrfs/delayed-inode.o: warning: objtool: 
btrfs_delete_delayed_dir_index()+0x0: frame pointer state mismatch
  fs/dlm/lock.o: warning: objtool: _grant_lock()+0x0: duplicate frame pointer 
save
  fs/dlm/lock.o: warning: objtool: _grant_lock()+0x0: frame pointer state 
mismatch
  fs/ocfs2/alloc.o: warning: objtool: ocfs2_mv_path()+0x0: duplicate frame 
pointer save
  fs/ocfs2/alloc.o: warning: objtool: ocfs2_mv_path()+0x0: frame pointer state 
mismatch

It turns out that, for older versions of gcc, if a function has multiple
BUG() incantations, gcc will sometimes merge the corresponding
annotate_unreachable() inline asm statements into a single block.  That
has the undesirable effect of removing one of the entries in the
__unreachable section, confusing objtool greatly.

A workaround for this issue is to ensure that each instance of the
inline asm statement uses a different label, so that gcc sees the
statements are unique and leaves them alone.  The inline asm ‘%=’ token
could be used for that, but unfortunately older versions of gcc don't
support it.  So I implemented a poor man's version of it with the
__LINE__ macro.

Fixes: d1091c7fa3d5 ("objtool: Improve detection of BUG() and other dead ends")
Reported-by: kbuild test robot 
Cc: Peter Zijlstra 
Signed-off-by: Josh Poimboeuf 
---
 include/linux/compiler-gcc.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 8ea159f..de47134 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -197,10 +197,10 @@
 
 #ifdef CONFIG_STACK_VALIDATION
 #define annotate_unreachable() ({  \
-   asm("1:\t\n"\
+   asm("%c0:\t\n"  \
".pushsection __unreachable, \"a\"\t\n" \
-   ".long 1b\t\n"  \
-   ".popsection\t\n"); \
+   ".long %c0b\t\n"\
+   ".popsection\t\n" : : "i" (__LINE__));  \
 })
 #else
 #define annotate_unreachable()
-- 
2.7.4

Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting

[John Stultz]

On Fri, Feb 24, 2017 at 7:39 PM, Nick Kralevich  wrote:
> Can you try adding the androidboot.selinux=permissive line to the kernel
> command line, to boot in permissive mode? I suspect the policy just needs to
> be adjusted.

Yep. It does seem to boot fine in permissive mode, just not in enforcing.

Any clues as to what might need to be tweaked policy-wise?

I know selinux is sort of special, as its all about restricting
functionality, but this still "feels" a little bit like a regression
though, as userspace that worked before suddenly stopped working. I
don't want to throw a wrench in things and am ok if we can sort out
the policy changes, but longer term, it makes it hard to advocate for
devices to update their kernel if new kernels aren't going to just
work.

thanks
-john

Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting

[John Stultz]

On Fri, Feb 24, 2017 at 7:39 PM, Nick Kralevich  wrote:
> Can you try adding the androidboot.selinux=permissive line to the kernel
> command line, to boot in permissive mode? I suspect the policy just needs to
> be adjusted.

Yep. It does seem to boot fine in permissive mode, just not in enforcing.

Any clues as to what might need to be tweaked policy-wise?

I know selinux is sort of special, as its all about restricting
functionality, but this still "feels" a little bit like a regression
though, as userspace that worked before suddenly stopped working. I
don't want to throw a wrench in things and am ok if we can sort out
the policy changes, but longer term, it makes it hard to advocate for
devices to update their kernel if new kernels aren't going to just
work.

thanks
-john

Re: [PATCH 4.10 00/21] 4.10.1-stable review

[Guenter Roeck]

On 02/24/2017 12:39 AM, Greg Kroah-Hartman wrote:

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

Responses should be made by Sun Feb 26 08:38:42 UTC 2017.
Anything received after that time might be too late.



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

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

Guenter

Re: [PATCH 4.10 00/21] 4.10.1-stable review

[Guenter Roeck]

On 02/24/2017 12:39 AM, Greg Kroah-Hartman wrote:

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

Responses should be made by Sun Feb 26 08:38:42 UTC 2017.
Anything received after that time might be too late.



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

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

Guenter

Re: [PATCH] f2fs: add F2FS_DIRTY_DATA to has_not_enough_free_secs and need_SSR

[Yunlong Song]

Continue to get mailing wrong message and can not receive some of emails from 
linux-f2fs-de...@lists.sourceforge.net, very strange, so send again.

The benefit is much, let me give an example to make the point more clear, the 
reserved_sections for a 64G image is
about 500M, and if the current free_sections is 600M, and the IO pattern is 
like this:

Before this patch:
 time 1: node & dent * imeta 20M to write, 100M data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M + 500M

time 2: node & dent * imeta 20M*2  to write, 100M*2 data to write
   free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M*2 + 500M

time 3: node & dent * imeta 20M*3  to write, 100M*3 data to write
   free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M*3 + 500M

time 4: node & dent * imeta 20M*4  to write, 100M*4 data to write
   free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M*4 + 500M

then here comes a sync, and wait for all the node & dent * imeta and data to 
flush to the flash device
what will happen after this sync?
the free_sections will decrease to 600M-20M*4(node & dent * imeta)-100M*4(data) 
= 120M
next time in f2fs_balance_fs:
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
  120M  <= 0 + 500M

then f2fs_gc will gc_more times and times again until free_sections increases 
from 120M to 500M..
It will cost a lot of time!

After this patch:
time 1: node & dent * imeta 20M to write, 100M data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M < 20M + 100M  + 500M
this time f2fs_gc will only gc_more for the gap 20M + 100M  + 500M - 600M = 20M

time 2: node & dent * imeta 20M*2 to write, 100M*2 data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M + 20M (gc_more from time 
1) < 20M*2 + 100M*2  + 500M
this time f2fs_gc will only gc_more for the gap 20M*2 + 100M *2 + 500M - (600M 
+ 20M) = 120M

time 3: node & dent * imeta 20M*3 to write, 100M*3 data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M + 20M (gc_more from time 
1) + 120M (gc_more from time 2) < 20M*3 + 100M*3  + 500M
this time f2fs_gc will only gc_more for the gap 20M*3 + 100M *3 + 500M - (600M 
+ 20M + 120M) = 120M

time 4: node & dent * imeta 20M*4 to write, 100M*4 data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M + 20M (gc_more from time 
1) + 120M (gc_more from time 2) + 120M (gc_more from time 3) < 20M*4 + 100M*4  
+ 500M
this time f2fs_gc will only gc_more for the gap 20M*4 + 100M *4 + 500M - (600M 
+ 20M + 120M + 120M) = 120M

then here comes a sync, and wait for all the node & dent * imeta and data to 
flush to the flash device
what will happen after this sync?
the free_sections will decrease to 600M + 20M (gc_more from time 1) + 120M 
(gc_more from time 2) + 120M (gc_more from time 3) +120M (gc_more from time 4) 
- 20M*4(node & dent * imeta)-100M*4(data) = 500M
next time in f2fs_balance_fs:
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
  500M  <= 0 + 500M
this time f2fs_gc will only gc for 1 free segment, comparing with the current 
design without the patch, which has to gc_more from 120M to 500M

You see, after the patch, gc_more are separated to 4 times but the old design 
make gc_more to one time, which will cost much performance.

Besides, after the patch, we can make sure the reserved_sections are remained 
unused and free all the time, which can avoid the segment using up case!

So now we can use the patch I sent last time, which changes the mkfs.f2fs to 
reduce the reserved_segments a lot and use SSR, I remember you pointed out an
issue that if there are not enough SSR segments then there is a problem, now I 
can solve your issue with this patch, since we can make sure the 
reserved_sections
are free all the time, then we are always able to make gc to have more free 
segments (or just take up the reserved_segments for temporary use when SSR 
segments
are not enough for performance).

Finally, the performance can be improved and the reserved_segments can be 
reduced a lot.

However, if this path is not allowed finally, I still suggest to reduce the 
reserved_segments to a smaller number, this is to avoid the amount 

Re: [PATCH] f2fs: add F2FS_DIRTY_DATA to has_not_enough_free_secs and need_SSR

[Yunlong Song]

Continue to get mailing wrong message and can not receive some of emails from 
linux-f2fs-de...@lists.sourceforge.net, very strange, so send again.

The benefit is much, let me give an example to make the point more clear, the 
reserved_sections for a 64G image is
about 500M, and if the current free_sections is 600M, and the IO pattern is 
like this:

Before this patch:
 time 1: node & dent * imeta 20M to write, 100M data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M + 500M

time 2: node & dent * imeta 20M*2  to write, 100M*2 data to write
   free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M*2 + 500M

time 3: node & dent * imeta 20M*3  to write, 100M*3 data to write
   free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M*3 + 500M

time 4: node & dent * imeta 20M*4  to write, 100M*4 data to write
   free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
then has_not_enough_free_secs returns false, since 600M > 20M*4 + 500M

then here comes a sync, and wait for all the node & dent * imeta and data to 
flush to the flash device
what will happen after this sync?
the free_sections will decrease to 600M-20M*4(node & dent * imeta)-100M*4(data) 
= 120M
next time in f2fs_balance_fs:
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
  120M  <= 0 + 500M

then f2fs_gc will gc_more times and times again until free_sections increases 
from 120M to 500M..
It will cost a lot of time!

After this patch:
time 1: node & dent * imeta 20M to write, 100M data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M < 20M + 100M  + 500M
this time f2fs_gc will only gc_more for the gap 20M + 100M  + 500M - 600M = 20M

time 2: node & dent * imeta 20M*2 to write, 100M*2 data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M + 20M (gc_more from time 
1) < 20M*2 + 100M*2  + 500M
this time f2fs_gc will only gc_more for the gap 20M*2 + 100M *2 + 500M - (600M 
+ 20M) = 120M

time 3: node & dent * imeta 20M*3 to write, 100M*3 data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M + 20M (gc_more from time 
1) + 120M (gc_more from time 2) < 20M*3 + 100M*3  + 500M
this time f2fs_gc will only gc_more for the gap 20M*3 + 100M *3 + 500M - (600M 
+ 20M + 120M) = 120M

time 4: node & dent * imeta 20M*4 to write, 100M*4 data to write
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + data + 
reserved_sections(sbi);
then has_not_enough_free_secs returns true, since 600M + 20M (gc_more from time 
1) + 120M (gc_more from time 2) + 120M (gc_more from time 3) < 20M*4 + 100M*4  
+ 500M
this time f2fs_gc will only gc_more for the gap 20M*4 + 100M *4 + 500M - (600M 
+ 20M + 120M + 120M) = 120M

then here comes a sync, and wait for all the node & dent * imeta and data to 
flush to the flash device
what will happen after this sync?
the free_sections will decrease to 600M + 20M (gc_more from time 1) + 120M 
(gc_more from time 2) + 120M (gc_more from time 3) +120M (gc_more from time 4) 
- 20M*4(node & dent * imeta)-100M*4(data) = 500M
next time in f2fs_balance_fs:
  free_sections(sbi) <= node_secs + 2 * dent_secs + imeta_secs + 
reserved_sections(sbi);
  500M  <= 0 + 500M
this time f2fs_gc will only gc for 1 free segment, comparing with the current 
design without the patch, which has to gc_more from 120M to 500M

You see, after the patch, gc_more are separated to 4 times but the old design 
make gc_more to one time, which will cost much performance.

Besides, after the patch, we can make sure the reserved_sections are remained 
unused and free all the time, which can avoid the segment using up case!

So now we can use the patch I sent last time, which changes the mkfs.f2fs to 
reduce the reserved_segments a lot and use SSR, I remember you pointed out an
issue that if there are not enough SSR segments then there is a problem, now I 
can solve your issue with this patch, since we can make sure the 
reserved_sections
are free all the time, then we are always able to make gc to have more free 
segments (or just take up the reserved_segments for temporary use when SSR 
segments
are not enough for performance).

Finally, the performance can be improved and the reserved_segments can be 
reduced a lot.

However, if this path is not allowed finally, I still suggest to reduce the 
reserved_segments to a smaller number, this is to avoid the amount 

Re: [PATCH] staging: bcm2835: Fix a memory leak in error handling path

[Dan Carpenter]

On Fri, Feb 24, 2017 at 10:38:38PM +0100, Stefan Wahren wrote:
> 
> > Dan Carpenter  hat am 24. Februar 2017 um 20:57 
> > geschrieben:
> > 
> > 
> > On Fri, Feb 24, 2017 at 01:37:30PM +0100, Stefan Wahren wrote:
> > > Hi Christophe,
> > > 
> > > Am 19.02.2017 um 11:34 schrieb Christophe JAILLET:
> > > >If 'kzalloc()' fails, we should release resources allocated so far, just 
> > > >as
> > > >done in all other cases in this function.
> > > >
> > > >Signed-off-by: Christophe JAILLET 
> > > >---
> > > >Not sure that the error handling path is correct.
> > > >Is 'gdev[0]' freed? Should it be?
> > > 
> > 
> > Yes, but I already sent a patch to fix this and your leak as well and
> > Greg merged it.
> 
> My leak? I'm confused.

The one you're fixing I mean.

Re: [PATCH] staging: bcm2835: Fix a memory leak in error handling path

[Dan Carpenter]

On Fri, Feb 24, 2017 at 10:38:38PM +0100, Stefan Wahren wrote:
> 
> > Dan Carpenter  hat am 24. Februar 2017 um 20:57 
> > geschrieben:
> > 
> > 
> > On Fri, Feb 24, 2017 at 01:37:30PM +0100, Stefan Wahren wrote:
> > > Hi Christophe,
> > > 
> > > Am 19.02.2017 um 11:34 schrieb Christophe JAILLET:
> > > >If 'kzalloc()' fails, we should release resources allocated so far, just 
> > > >as
> > > >done in all other cases in this function.
> > > >
> > > >Signed-off-by: Christophe JAILLET 
> > > >---
> > > >Not sure that the error handling path is correct.
> > > >Is 'gdev[0]' freed? Should it be?
> > > 
> > 
> > Yes, but I already sent a patch to fix this and your leak as well and
> > Greg merged it.
> 
> My leak? I'm confused.

The one you're fixing I mean.

Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs

[Oleg Drokin]

On Feb 24, 2017, at 11:59 AM, Greg Kroah-Hartman wrote:

> On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote:
>> From: Alex Zhuravlev 
>> 
>> on small filesystems plain log can grow dramatically. especially
>> given large record sizes produced by DNE and extended chunksize.
>> I saw >50% of space consumed by a single llog file which was still
>> in use. this leads to test failures (sanityn, etc).
>> the patch introduces additional limit on plain llog size, which
>> is calculated as /64 (128MB at most) at llog creation
>> time.
>> 
>> Signed-off-by: Alex Zhuravlev 
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
>> Reviewed-on: https://review.whamcloud.com/18028
>> Reviewed-by: Andreas Dilger 
>> Reviewed-by: wangdi 
>> Reviewed-by: Mike Pershin 
>> Reviewed-by: Oleg Drokin 
>> Signed-off-by: James Simmons 
>> ---
>> drivers/staging/lustre/lustre/obdclass/llog.c | 16 
>> 1 file changed, 16 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/llog.c 
>> b/drivers/staging/lustre/lustre/obdclass/llog.c
>> index 83c5b62..320ff6b 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/llog.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/llog.c
>> @@ -319,10 +319,26 @@ static int llog_process_thread(void *arg)
>>   * the case and re-read the current chunk
>>   * otherwise.
>>   */
>> +int records;
>> +
>>  if (index > loghandle->lgh_last_idx) {
>>  rc = 0;
>>  goto out;
>>  }
>> +/* <2 records means no more records
>> + * if the last record we processed was
>> + * the final one, then the underlying
>> + * object might have been destroyed yet.
>> + * we better don't access that..
>> + */
>> +mutex_lock(>lgh_hdr_mutex);
>> +records = loghandle->lgh_hdr->llh_count;
>> +mutex_unlock(>lgh_hdr_mutex);
>> +if (records <= 1) {
>> +rc = 0;
>> +goto out;
>> +}
> 
> 
> So you now use the lock, in only one place, when reading a single value?
> That makes no sense, it's obviously wrong, or not needed.
> 
> Please fix up these two patches…

Ah, this is in fact server-side fix, so all the other users were in the
parts not really present in the client.
James, we don't really need this patch in the client, I guess.

Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs

[Oleg Drokin]

On Feb 24, 2017, at 11:59 AM, Greg Kroah-Hartman wrote:

> On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote:
>> From: Alex Zhuravlev 
>> 
>> on small filesystems plain log can grow dramatically. especially
>> given large record sizes produced by DNE and extended chunksize.
>> I saw >50% of space consumed by a single llog file which was still
>> in use. this leads to test failures (sanityn, etc).
>> the patch introduces additional limit on plain llog size, which
>> is calculated as /64 (128MB at most) at llog creation
>> time.
>> 
>> Signed-off-by: Alex Zhuravlev 
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
>> Reviewed-on: https://review.whamcloud.com/18028
>> Reviewed-by: Andreas Dilger 
>> Reviewed-by: wangdi 
>> Reviewed-by: Mike Pershin 
>> Reviewed-by: Oleg Drokin 
>> Signed-off-by: James Simmons 
>> ---
>> drivers/staging/lustre/lustre/obdclass/llog.c | 16 
>> 1 file changed, 16 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/llog.c 
>> b/drivers/staging/lustre/lustre/obdclass/llog.c
>> index 83c5b62..320ff6b 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/llog.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/llog.c
>> @@ -319,10 +319,26 @@ static int llog_process_thread(void *arg)
>>   * the case and re-read the current chunk
>>   * otherwise.
>>   */
>> +int records;
>> +
>>  if (index > loghandle->lgh_last_idx) {
>>  rc = 0;
>>  goto out;
>>  }
>> +/* <2 records means no more records
>> + * if the last record we processed was
>> + * the final one, then the underlying
>> + * object might have been destroyed yet.
>> + * we better don't access that..
>> + */
>> +mutex_lock(>lgh_hdr_mutex);
>> +records = loghandle->lgh_hdr->llh_count;
>> +mutex_unlock(>lgh_hdr_mutex);
>> +if (records <= 1) {
>> +rc = 0;
>> +goto out;
>> +}
> 
> 
> So you now use the lock, in only one place, when reading a single value?
> That makes no sense, it's obviously wrong, or not needed.
> 
> Please fix up these two patches…

Ah, this is in fact server-side fix, so all the other users were in the
parts not really present in the client.
James, we don't really need this patch in the client, I guess.

Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs

[Oleg Drokin]

On Feb 24, 2017, at 11:59 AM, Greg Kroah-Hartman wrote:

> On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote:
>> From: Alex Zhuravlev 
>> 
>> on small filesystems plain log can grow dramatically. especially
>> given large record sizes produced by DNE and extended chunksize.
>> I saw >50% of space consumed by a single llog file which was still
>> in use. this leads to test failures (sanityn, etc).
>> the patch introduces additional limit on plain llog size, which
>> is calculated as /64 (128MB at most) at llog creation
>> time.
>> 
>> Signed-off-by: Alex Zhuravlev 
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
>> Reviewed-on: https://review.whamcloud.com/18028
>> Reviewed-by: Andreas Dilger 
>> Reviewed-by: wangdi 
>> Reviewed-by: Mike Pershin 
>> Reviewed-by: Oleg Drokin 
>> Signed-off-by: James Simmons 
>> ---
>> drivers/staging/lustre/lustre/obdclass/llog.c | 16 
>> 1 file changed, 16 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/llog.c 
>> b/drivers/staging/lustre/lustre/obdclass/llog.c
>> index 83c5b62..320ff6b 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/llog.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/llog.c
>> @@ -319,10 +319,26 @@ static int llog_process_thread(void *arg)
>>   * the case and re-read the current chunk
>>   * otherwise.
>>   */
>> +int records;
>> +
>>  if (index > loghandle->lgh_last_idx) {
>>  rc = 0;
>>  goto out;
>>  }
>> +/* <2 records means no more records
>> + * if the last record we processed was
>> + * the final one, then the underlying
>> + * object might have been destroyed yet.
>> + * we better don't access that..
>> + */
>> +mutex_lock(>lgh_hdr_mutex);
>> +records = loghandle->lgh_hdr->llh_count;
>> +mutex_unlock(>lgh_hdr_mutex);
>> +if (records <= 1) {
>> +rc = 0;
>> +goto out;
>> +}
> 
> 
> So you now use the lock, in only one place, when reading a single value?
> That makes no sense, it's obviously wrong, or not needed.
> 
> Please fix up these two patches…

Ah, this is in fact server-side fix, so all the other users were in the
parts not really present in the client.
James, we don't really need this patch in the client, I guess.

Re: [PATCH 13/14] staging: lustre: llog: limit file size of plain logs

[Oleg Drokin]

On Feb 24, 2017, at 11:59 AM, Greg Kroah-Hartman wrote:

> On Sat, Feb 18, 2017 at 04:47:14PM -0500, James Simmons wrote:
>> From: Alex Zhuravlev 
>> 
>> on small filesystems plain log can grow dramatically. especially
>> given large record sizes produced by DNE and extended chunksize.
>> I saw >50% of space consumed by a single llog file which was still
>> in use. this leads to test failures (sanityn, etc).
>> the patch introduces additional limit on plain llog size, which
>> is calculated as /64 (128MB at most) at llog creation
>> time.
>> 
>> Signed-off-by: Alex Zhuravlev 
>> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6838
>> Reviewed-on: https://review.whamcloud.com/18028
>> Reviewed-by: Andreas Dilger 
>> Reviewed-by: wangdi 
>> Reviewed-by: Mike Pershin 
>> Reviewed-by: Oleg Drokin 
>> Signed-off-by: James Simmons 
>> ---
>> drivers/staging/lustre/lustre/obdclass/llog.c | 16 
>> 1 file changed, 16 insertions(+)
>> 
>> diff --git a/drivers/staging/lustre/lustre/obdclass/llog.c 
>> b/drivers/staging/lustre/lustre/obdclass/llog.c
>> index 83c5b62..320ff6b 100644
>> --- a/drivers/staging/lustre/lustre/obdclass/llog.c
>> +++ b/drivers/staging/lustre/lustre/obdclass/llog.c
>> @@ -319,10 +319,26 @@ static int llog_process_thread(void *arg)
>>   * the case and re-read the current chunk
>>   * otherwise.
>>   */
>> +int records;
>> +
>>  if (index > loghandle->lgh_last_idx) {
>>  rc = 0;
>>  goto out;
>>  }
>> +/* <2 records means no more records
>> + * if the last record we processed was
>> + * the final one, then the underlying
>> + * object might have been destroyed yet.
>> + * we better don't access that..
>> + */
>> +mutex_lock(>lgh_hdr_mutex);
>> +records = loghandle->lgh_hdr->llh_count;
>> +mutex_unlock(>lgh_hdr_mutex);
>> +if (records <= 1) {
>> +rc = 0;
>> +goto out;
>> +}
> 
> 
> So you now use the lock, in only one place, when reading a single value?
> That makes no sense, it's obviously wrong, or not needed.
> 
> Please fix up these two patches…

Ah, this is in fact server-side fix, so all the other users were in the
parts not really present in the client.
James, we don't really need this patch in the client, I guess.

Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Chanwoo Choi]

2017-02-25 12:46 GMT+09:00 Chanwoo Choi :
> Hi,
>
> 2017-02-24 21:02 GMT+09:00 Roger Quadros :
>> +Chanwoo
>>
>> Hi Vivek,
>>
>> On 23/02/17 10:34, Vivek Gautam wrote:
>>>
>>>
>>> On 02/16/2017 06:36 PM, Roger Quadros wrote:
 dra7 OTG core limits the host controller to USB2.0 (high-speed) mode
 when we're operating in dual-role.

 We work around that by bypassing the OTG core and reading the
 extcon framework directly for ID/VBUS events.

 Signed-off-by: Roger Quadros 
 ---
   Documentation/devicetree/bindings/usb/dwc3.txt |   2 +
   drivers/usb/dwc3/core.c| 169 
 -
   drivers/usb/dwc3/core.h|   5 +
   3 files changed, 170 insertions(+), 6 deletions(-)

 diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
 b/Documentation/devicetree/bindings/usb/dwc3.txt
 index e3e6983..9955c0d 100644
 --- a/Documentation/devicetree/bindings/usb/dwc3.txt
 +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
 @@ -53,6 +53,8 @@ Optional properties:
- snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of 
 GFLADJ
   register for post-silicon frame length adjustment when the
   fladj_30mhz_sdbnd signal is invalid or incorrect.
 + - extcon: phandle to the USB connector extcon device. If present, extcon
 +device will be used to get USB cable events instead of OTG controller.
  -  tx-fifo-resize: determines if the FIFO *has* to be 
 reallocated.
   diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index 619fa7c..b02d911 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
>>>
>>> [snip]
>>>
 @@ -1587,6 +1727,14 @@ static int dwc3_probe(struct platform_device *pdev)
 dwc3_get_properties(dwc);
   +if (dev->of_node) {
 +if (of_property_read_bool(dev->of_node, "extcon"))
 +dwc->edev = extcon_get_edev_by_phandle(dev, 0);
>>>
>>> Don't we want separate edev's for vbus and id ?
>>> One can have separate pins connected to them and in that case
>>> we can't get the events out of one pin only.
>>
>> Is such kind of hardware really there? Ideally one extcon device
>> represents one connector. So VBUS and ID events of a single USB
>> port should come on one extcon device.
>> If it doesn't then you need to fix your platforms extcon driver
>> so that it does.
>> Chanwoo can correct me if I'm wrong on this understanding.
>>
>> Currently, if VBUS and ID come on GPIOs the extcon-usb-gpio driver
>> takes care of both.
>
> Basically, one extcon device driver can support the multiple
> external connectors if the detection h/w or port is same.
>
> One extcon device with extcon-usb-gpio.c can support the detection
> of both ID and VBUS.
>
> But, if there is any issue of h/w schematic, we need to consider
> how to use the extcon device.

Additionally, the extcon-usb-gpio.c use the two gpio pins
to detect both ID and VBUS pins. We can check it on documentation[1]
of extcon-usb-gpio driver.

[1] 
https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/tree/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Chanwoo Choi]

2017-02-25 12:46 GMT+09:00 Chanwoo Choi :
> Hi,
>
> 2017-02-24 21:02 GMT+09:00 Roger Quadros :
>> +Chanwoo
>>
>> Hi Vivek,
>>
>> On 23/02/17 10:34, Vivek Gautam wrote:
>>>
>>>
>>> On 02/16/2017 06:36 PM, Roger Quadros wrote:
 dra7 OTG core limits the host controller to USB2.0 (high-speed) mode
 when we're operating in dual-role.

 We work around that by bypassing the OTG core and reading the
 extcon framework directly for ID/VBUS events.

 Signed-off-by: Roger Quadros 
 ---
   Documentation/devicetree/bindings/usb/dwc3.txt |   2 +
   drivers/usb/dwc3/core.c| 169 
 -
   drivers/usb/dwc3/core.h|   5 +
   3 files changed, 170 insertions(+), 6 deletions(-)

 diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
 b/Documentation/devicetree/bindings/usb/dwc3.txt
 index e3e6983..9955c0d 100644
 --- a/Documentation/devicetree/bindings/usb/dwc3.txt
 +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
 @@ -53,6 +53,8 @@ Optional properties:
- snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of 
 GFLADJ
   register for post-silicon frame length adjustment when the
   fladj_30mhz_sdbnd signal is invalid or incorrect.
 + - extcon: phandle to the USB connector extcon device. If present, extcon
 +device will be used to get USB cable events instead of OTG controller.
  -  tx-fifo-resize: determines if the FIFO *has* to be 
 reallocated.
   diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
 index 619fa7c..b02d911 100644
 --- a/drivers/usb/dwc3/core.c
 +++ b/drivers/usb/dwc3/core.c
>>>
>>> [snip]
>>>
 @@ -1587,6 +1727,14 @@ static int dwc3_probe(struct platform_device *pdev)
 dwc3_get_properties(dwc);
   +if (dev->of_node) {
 +if (of_property_read_bool(dev->of_node, "extcon"))
 +dwc->edev = extcon_get_edev_by_phandle(dev, 0);
>>>
>>> Don't we want separate edev's for vbus and id ?
>>> One can have separate pins connected to them and in that case
>>> we can't get the events out of one pin only.
>>
>> Is such kind of hardware really there? Ideally one extcon device
>> represents one connector. So VBUS and ID events of a single USB
>> port should come on one extcon device.
>> If it doesn't then you need to fix your platforms extcon driver
>> so that it does.
>> Chanwoo can correct me if I'm wrong on this understanding.
>>
>> Currently, if VBUS and ID come on GPIOs the extcon-usb-gpio driver
>> takes care of both.
>
> Basically, one extcon device driver can support the multiple
> external connectors if the detection h/w or port is same.
>
> One extcon device with extcon-usb-gpio.c can support the detection
> of both ID and VBUS.
>
> But, if there is any issue of h/w schematic, we need to consider
> how to use the extcon device.

Additionally, the extcon-usb-gpio.c use the two gpio pins
to detect both ID and VBUS pins. We can check it on documentation[1]
of extcon-usb-gpio driver.

[1] 
https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/tree/Documentation/devicetree/bindings/extcon/extcon-usb-gpio.txt


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Chanwoo Choi]

Hi,

2017-02-24 21:02 GMT+09:00 Roger Quadros :
> +Chanwoo
>
> Hi Vivek,
>
> On 23/02/17 10:34, Vivek Gautam wrote:
>>
>>
>> On 02/16/2017 06:36 PM, Roger Quadros wrote:
>>> dra7 OTG core limits the host controller to USB2.0 (high-speed) mode
>>> when we're operating in dual-role.
>>>
>>> We work around that by bypassing the OTG core and reading the
>>> extcon framework directly for ID/VBUS events.
>>>
>>> Signed-off-by: Roger Quadros 
>>> ---
>>>   Documentation/devicetree/bindings/usb/dwc3.txt |   2 +
>>>   drivers/usb/dwc3/core.c| 169 
>>> -
>>>   drivers/usb/dwc3/core.h|   5 +
>>>   3 files changed, 170 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
>>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> index e3e6983..9955c0d 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> @@ -53,6 +53,8 @@ Optional properties:
>>>- snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of 
>>> GFLADJ
>>>   register for post-silicon frame length adjustment when the
>>>   fladj_30mhz_sdbnd signal is invalid or incorrect.
>>> + - extcon: phandle to the USB connector extcon device. If present, extcon
>>> +device will be used to get USB cable events instead of OTG controller.
>>>  -  tx-fifo-resize: determines if the FIFO *has* to be 
>>> reallocated.
>>>   diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 619fa7c..b02d911 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>
>> [snip]
>>
>>> @@ -1587,6 +1727,14 @@ static int dwc3_probe(struct platform_device *pdev)
>>> dwc3_get_properties(dwc);
>>>   +if (dev->of_node) {
>>> +if (of_property_read_bool(dev->of_node, "extcon"))
>>> +dwc->edev = extcon_get_edev_by_phandle(dev, 0);
>>
>> Don't we want separate edev's for vbus and id ?
>> One can have separate pins connected to them and in that case
>> we can't get the events out of one pin only.
>
> Is such kind of hardware really there? Ideally one extcon device
> represents one connector. So VBUS and ID events of a single USB
> port should come on one extcon device.
> If it doesn't then you need to fix your platforms extcon driver
> so that it does.
> Chanwoo can correct me if I'm wrong on this understanding.
>
> Currently, if VBUS and ID come on GPIOs the extcon-usb-gpio driver
> takes care of both.

Basically, one extcon device driver can support the multiple
external connectors if the detection h/w or port is same.

One extcon device with extcon-usb-gpio.c can support the detection
of both ID and VBUS.

But, if there is any issue of h/w schematic, we need to consider
how to use the extcon device.

>
>>
>>> +
>>> +if (IS_ERR(dwc->edev))
>>> +return PTR_ERR(dwc->edev);
>>
>>  Took me a while to get to this. :)
>>
>> if (IS_ERR(dwc->edev)) {
>>ret = PTR_ERR(dwc->edev);
>>goto err0;
>> }
>>
>> We want to reset the res->start back to its original offset.
>>
>> Testing this series currently. Will get back with my results.
>>
>>
> Thanks :)
>
> --
> cheers,
> -roger

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Chanwoo Choi]

Hi,

2017-02-24 21:02 GMT+09:00 Roger Quadros :
> +Chanwoo
>
> Hi Vivek,
>
> On 23/02/17 10:34, Vivek Gautam wrote:
>>
>>
>> On 02/16/2017 06:36 PM, Roger Quadros wrote:
>>> dra7 OTG core limits the host controller to USB2.0 (high-speed) mode
>>> when we're operating in dual-role.
>>>
>>> We work around that by bypassing the OTG core and reading the
>>> extcon framework directly for ID/VBUS events.
>>>
>>> Signed-off-by: Roger Quadros 
>>> ---
>>>   Documentation/devicetree/bindings/usb/dwc3.txt |   2 +
>>>   drivers/usb/dwc3/core.c| 169 
>>> -
>>>   drivers/usb/dwc3/core.h|   5 +
>>>   3 files changed, 170 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
>>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> index e3e6983..9955c0d 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> @@ -53,6 +53,8 @@ Optional properties:
>>>- snps,quirk-frame-length-adjustment: Value for GFLADJ_30MHZ field of 
>>> GFLADJ
>>>   register for post-silicon frame length adjustment when the
>>>   fladj_30mhz_sdbnd signal is invalid or incorrect.
>>> + - extcon: phandle to the USB connector extcon device. If present, extcon
>>> +device will be used to get USB cable events instead of OTG controller.
>>>  -  tx-fifo-resize: determines if the FIFO *has* to be 
>>> reallocated.
>>>   diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 619fa7c..b02d911 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>
>> [snip]
>>
>>> @@ -1587,6 +1727,14 @@ static int dwc3_probe(struct platform_device *pdev)
>>> dwc3_get_properties(dwc);
>>>   +if (dev->of_node) {
>>> +if (of_property_read_bool(dev->of_node, "extcon"))
>>> +dwc->edev = extcon_get_edev_by_phandle(dev, 0);
>>
>> Don't we want separate edev's for vbus and id ?
>> One can have separate pins connected to them and in that case
>> we can't get the events out of one pin only.
>
> Is such kind of hardware really there? Ideally one extcon device
> represents one connector. So VBUS and ID events of a single USB
> port should come on one extcon device.
> If it doesn't then you need to fix your platforms extcon driver
> so that it does.
> Chanwoo can correct me if I'm wrong on this understanding.
>
> Currently, if VBUS and ID come on GPIOs the extcon-usb-gpio driver
> takes care of both.

Basically, one extcon device driver can support the multiple
external connectors if the detection h/w or port is same.

One extcon device with extcon-usb-gpio.c can support the detection
of both ID and VBUS.

But, if there is any issue of h/w schematic, we need to consider
how to use the extcon device.

>
>>
>>> +
>>> +if (IS_ERR(dwc->edev))
>>> +return PTR_ERR(dwc->edev);
>>
>>  Took me a while to get to this. :)
>>
>> if (IS_ERR(dwc->edev)) {
>>ret = PTR_ERR(dwc->edev);
>>goto err0;
>> }
>>
>> We want to reset the res->start back to its original offset.
>>
>> Testing this series currently. Will get back with my results.
>>
>>
> Thanks :)
>
> --
> cheers,
> -roger

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting

[Nick Kralevich]

Can you try adding the androidboot.selinux=permissive line to the kernel
command line, to boot in permissive mode? I suspect the policy just needs
to be adjusted.

-- Nick

On Fri, Feb 24, 2017 at 6:01 PM, John Stultz  wrote:
> On Thu, Feb 23, 2017 at 4:01 PM, Paul Moore  wrote:
>> On Thu, Feb 23, 2017 at 1:43 PM, John Stultz  wrote:
>>> Hey folks,
>>>I've not been able to figure out why yet, but I wanted to raise the
>>> issue that last night I found I couldn't boot Android on my Hikey
>>> board with Linus' HEAD kernel. It seems to cause logd to crash
>>> repeatedly so I'm not able to get debug info from logcat.
>>>
>>> I do see the following over and over on the console:
>>>
>>> [   12.505838] init: computing context for service 'logd'
>>> [   12.506355] init: starting service 'logd'...
>>> [   12.507683] init: property_set("ro.boottime.logd", "12500792498")
>>> failed: property already set
>>> [   12.508701] init: Created socket '/dev/socket/logd', mode 666, user
>>> 1036, group 1036
>>> [   12.509294] init: Created socket '/dev/socket/logdr', mode 666,
>>> user 1036, group 1036
>>> [   12.509891] init: Created socket '/dev/socket/logdw', mode 222,
>>> user 1036, group 1036
>>> [   12.510132] init: Opened file '/proc/kmsg', flags 0
>>> [   12.510187] init: Opened file '/dev/kmsg', flags 1
>>> [   12.510353] init: couldn't write 1941 to
>>> /dev/cpuset/system-background/tasks: No such file or directory
>>> [   12.533046] init: Service 'logd' (pid 1941) exited with status 255
>>>
>>>
>>> I did some bisection and narrowed it down to 1ea0ce4069 ("selinux:
>>> allow changing labels for cgroupfs"), which was merged in yesterday.
>>> I've not yet been able to figure out the root cause, but reverting
>>> that patch makes things work again.
>>>
>>> So I wanted to raise the issue here so folks were aware.
>>>
>>> If there is anything folks want me to test or try, please let me know.
>>
>> Unfortunately I don't have an Android test system to play with, have
>> any of the SEAndroid folks on the To/CC line seen a similar problem?
>
> So from my very limited knowledge here, adding the patch in question
> seems to make the cgroup mount get the SBLABEL_MNT flag?
> Which I'm guessing this is causing additional selinux restrictions on
> processes accessing cgroup mounts, which causes some of the early
> initialization processes to fail?
>
> Should this change mean the selinux policy needs to be updated?
>
> thanks
> -john



-- 
Nick Kralevich | Android Security | n...@google.com | 650.214.4037

Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting

[Nick Kralevich]

Can you try adding the androidboot.selinux=permissive line to the kernel
command line, to boot in permissive mode? I suspect the policy just needs
to be adjusted.

-- Nick

On Fri, Feb 24, 2017 at 6:01 PM, John Stultz  wrote:
> On Thu, Feb 23, 2017 at 4:01 PM, Paul Moore  wrote:
>> On Thu, Feb 23, 2017 at 1:43 PM, John Stultz  wrote:
>>> Hey folks,
>>>I've not been able to figure out why yet, but I wanted to raise the
>>> issue that last night I found I couldn't boot Android on my Hikey
>>> board with Linus' HEAD kernel. It seems to cause logd to crash
>>> repeatedly so I'm not able to get debug info from logcat.
>>>
>>> I do see the following over and over on the console:
>>>
>>> [   12.505838] init: computing context for service 'logd'
>>> [   12.506355] init: starting service 'logd'...
>>> [   12.507683] init: property_set("ro.boottime.logd", "12500792498")
>>> failed: property already set
>>> [   12.508701] init: Created socket '/dev/socket/logd', mode 666, user
>>> 1036, group 1036
>>> [   12.509294] init: Created socket '/dev/socket/logdr', mode 666,
>>> user 1036, group 1036
>>> [   12.509891] init: Created socket '/dev/socket/logdw', mode 222,
>>> user 1036, group 1036
>>> [   12.510132] init: Opened file '/proc/kmsg', flags 0
>>> [   12.510187] init: Opened file '/dev/kmsg', flags 1
>>> [   12.510353] init: couldn't write 1941 to
>>> /dev/cpuset/system-background/tasks: No such file or directory
>>> [   12.533046] init: Service 'logd' (pid 1941) exited with status 255
>>>
>>>
>>> I did some bisection and narrowed it down to 1ea0ce4069 ("selinux:
>>> allow changing labels for cgroupfs"), which was merged in yesterday.
>>> I've not yet been able to figure out the root cause, but reverting
>>> that patch makes things work again.
>>>
>>> So I wanted to raise the issue here so folks were aware.
>>>
>>> If there is anything folks want me to test or try, please let me know.
>>
>> Unfortunately I don't have an Android test system to play with, have
>> any of the SEAndroid folks on the To/CC line seen a similar problem?
>
> So from my very limited knowledge here, adding the patch in question
> seems to make the cgroup mount get the SBLABEL_MNT flag?
> Which I'm guessing this is causing additional selinux restrictions on
> processes accessing cgroup mounts, which causes some of the early
> initialization processes to fail?
>
> Should this change mean the selinux policy needs to be updated?
>
> thanks
> -john



-- 
Nick Kralevich | Android Security | n...@google.com | 650.214.4037

Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Chanwoo Choi]

Hi Roger,

[snip]

>  /* dwc->lock must be held */
>  static void dwc3_otg_core_exit(struct dwc3 *dwc)
>  {
> +   if (dwc->edev)
> +   return;
> +
> /* disable all otg irqs */
> dwc3_otg_disable_events(dwc, DWC3_OTG_ALL_EVENTS);
> /* clear all events */
> @@ -1286,6 +1364,57 @@ static int dwc3_drd_init(struct dwc3 *dwc)
>
> INIT_WORK(>otg_work, dwc3_drd_work);
>
> +   /* If extcon device is present we don't rely on OTG core for ID event 
> */
> +   if (dwc->edev) {
> +   int id, vbus;
> +
> +   dwc->edev_nb.notifier_call = dwc3_drd_notifier;
> +   ret = extcon_register_notifier(dwc->edev, EXTCON_USB,
> +  >edev_nb);

I recommend that you better to use the devm_extcon_register_notifier()

> +   if (ret < 0) {
> +   dev_err(dwc->dev, "Couldn't register USB cable 
> notifier\n");
> +   return -ENODEV;
> +   }
> +
> +   ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
> +  >edev_nb);

Ditto.

[snip]

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: [PATCH v2 4/4] usb: dwc3: Workaround for super-speed host on dra7 in dual-role mode

[Chanwoo Choi]

Hi Roger,

[snip]

>  /* dwc->lock must be held */
>  static void dwc3_otg_core_exit(struct dwc3 *dwc)
>  {
> +   if (dwc->edev)
> +   return;
> +
> /* disable all otg irqs */
> dwc3_otg_disable_events(dwc, DWC3_OTG_ALL_EVENTS);
> /* clear all events */
> @@ -1286,6 +1364,57 @@ static int dwc3_drd_init(struct dwc3 *dwc)
>
> INIT_WORK(>otg_work, dwc3_drd_work);
>
> +   /* If extcon device is present we don't rely on OTG core for ID event 
> */
> +   if (dwc->edev) {
> +   int id, vbus;
> +
> +   dwc->edev_nb.notifier_call = dwc3_drd_notifier;
> +   ret = extcon_register_notifier(dwc->edev, EXTCON_USB,
> +  >edev_nb);

I recommend that you better to use the devm_extcon_register_notifier()

> +   if (ret < 0) {
> +   dev_err(dwc->dev, "Couldn't register USB cable 
> notifier\n");
> +   return -ENODEV;
> +   }
> +
> +   ret = extcon_register_notifier(dwc->edev, EXTCON_USB_HOST,
> +  >edev_nb);

Ditto.

[snip]

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Frederic Weisbecker]

On Thu, Feb 23, 2017 at 07:40:13PM +0100, Pavel Machek wrote:
> On Thu 2017-02-23 17:28:26, Frederic Weisbecker wrote:
> > On Tue, Feb 14, 2017 at 08:27:43PM +0100, Pavel Machek wrote:
> > > On Tue 2017-02-14 18:59:56, Pavel Machek wrote:
> > > > Hi!
> > > > 
> > > > > > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no 
> > > > > > > > longer
> > > > > > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, 
> > > > > > > > I
> > > > > > > > could not get it to work. I believe v4.9 and some v4.10-rc's 
> > > > > > > > worked,
> > > > > > > > but I'll have to double check.
> > > > > > > 
> > > > > > > But all the kernel versions worked when the keyboard was plugged 
> > > > > > > into
> > > > > > > its original USB port?
> > > > > > 
> > > > > > Aha. So it looks difference is probably in "where is keyboard 
> > > > > > plugged
> > > > > > in" but in "reboot" vs. "cold boot". I did not do a cold boot in 
> > > > > > quite
> > > > > > a while :-(.
> > > > > > 
> > > > > > Booting to grub, then hitting ctrl-alt-del is enough to make it 
> > > > > > work. Ouch.
> > > > > > 
> > > > > > It happens with current Linus' tree.
> > > > > 
> > > > > v4.10-rc6-feb3 : broken
> > > > > v4.9 : ok
> > > > > (v4.6 : ok)
> > > > 
> > > > Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   
> > > > 
> > > > With debug patch below, I get
> > > > 
> > > > ...1d.7: PCI fixup... pass 2
> > > > ...1d.7: PCI fixup... pass 3
> > > > ...1d.7: PCI fixup... pass 3 done
> > > > 
> > > > ...followed by hang. So yes, it looks USB related.
> > > > 
> > > > (Sometimes it hangs with some kind backtrace involving secondary CPU
> > > > startup, unfortunately useful info is off screen at that point).
> > > 
> > > Forgot to say, 1d.7 is EHCI controller.
> > > 
> > > 00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI
> > > Controller (rev 01)
> > 
> > Ok, I should have access soon to a EeePc 1015CX (which seem to have this 
> > controller).
> > I hope I'll be able to reproduce the issue there. If not, I'm sorry but 
> > I'll have to
> > burden you again :-)
> 
> Go through more mails.

I've read the whole thread several times, I couldn't get much more clues.

> It is only reproducible after cold boot. .. so I doubt it will be easy to 
> reproduce on another machine.

I have no idea. That's just my only hope for now.

> 
> Now... I do have serial port, and I even might have serial cable
> somewhere, but Giving how sensitive it is, it is probably going to
> go away with console on ttyS...

We'll see how it goes. I'll be off next week and then I should get the eeepc.
I'll get back to it there.

What gets me surprised is that the tick doesn't even fire yet on pci quirks 
time,
at least not on my machine where the clocksource is setup afterward. That said 
if
some of the pci quirks are async works, it might explain some later relation 
with the tick.

Thanks.

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Frederic Weisbecker]

On Thu, Feb 23, 2017 at 07:40:13PM +0100, Pavel Machek wrote:
> On Thu 2017-02-23 17:28:26, Frederic Weisbecker wrote:
> > On Tue, Feb 14, 2017 at 08:27:43PM +0100, Pavel Machek wrote:
> > > On Tue 2017-02-14 18:59:56, Pavel Machek wrote:
> > > > Hi!
> > > > 
> > > > > > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no 
> > > > > > > > longer
> > > > > > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, 
> > > > > > > > I
> > > > > > > > could not get it to work. I believe v4.9 and some v4.10-rc's 
> > > > > > > > worked,
> > > > > > > > but I'll have to double check.
> > > > > > > 
> > > > > > > But all the kernel versions worked when the keyboard was plugged 
> > > > > > > into
> > > > > > > its original USB port?
> > > > > > 
> > > > > > Aha. So it looks difference is probably in "where is keyboard 
> > > > > > plugged
> > > > > > in" but in "reboot" vs. "cold boot". I did not do a cold boot in 
> > > > > > quite
> > > > > > a while :-(.
> > > > > > 
> > > > > > Booting to grub, then hitting ctrl-alt-del is enough to make it 
> > > > > > work. Ouch.
> > > > > > 
> > > > > > It happens with current Linus' tree.
> > > > > 
> > > > > v4.10-rc6-feb3 : broken
> > > > > v4.9 : ok
> > > > > (v4.6 : ok)
> > > > 
> > > > Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   
> > > > 
> > > > With debug patch below, I get
> > > > 
> > > > ...1d.7: PCI fixup... pass 2
> > > > ...1d.7: PCI fixup... pass 3
> > > > ...1d.7: PCI fixup... pass 3 done
> > > > 
> > > > ...followed by hang. So yes, it looks USB related.
> > > > 
> > > > (Sometimes it hangs with some kind backtrace involving secondary CPU
> > > > startup, unfortunately useful info is off screen at that point).
> > > 
> > > Forgot to say, 1d.7 is EHCI controller.
> > > 
> > > 00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI
> > > Controller (rev 01)
> > 
> > Ok, I should have access soon to a EeePc 1015CX (which seem to have this 
> > controller).
> > I hope I'll be able to reproduce the issue there. If not, I'm sorry but 
> > I'll have to
> > burden you again :-)
> 
> Go through more mails.

I've read the whole thread several times, I couldn't get much more clues.

> It is only reproducible after cold boot. .. so I doubt it will be easy to 
> reproduce on another machine.

I have no idea. That's just my only hope for now.

> 
> Now... I do have serial port, and I even might have serial cable
> somewhere, but Giving how sensitive it is, it is probably going to
> go away with console on ttyS...

We'll see how it goes. I'll be off next week and then I should get the eeepc.
I'll get back to it there.

What gets me surprised is that the tick doesn't even fire yet on pci quirks 
time,
at least not on my machine where the clocksource is setup afterward. That said 
if
some of the pci quirks are async works, it might explain some later relation 
with the tick.

Thanks.

Re: [f2fs-dev] [PATCH 2/3] f2fs: add bitmaps for empty or full NAT blocks

[Chao Yu]

On 2017/2/24 6:54, Jaegeuk Kim wrote:
> On 02/23, Chao Yu wrote:
>> On 2017/2/14 10:06, Jaegeuk Kim wrote:
>>> This patches adds bitmaps to represent empty or full NAT blocks containing
>>> free nid entries.
>>>
>>> If we can find valid crc|cp_ver in the last block of checkpoint pack, we'll
>>> use these bitmaps when building free nids. In order to avoid checkpointing
>>> burden, up-to-date bitmaps will be flushed only during umount time. So,
>>> normally we can get this gain, but when power-cut happens, we rely on 
>>> fsck.f2fs
>>> which recovers this bitmap again.
>>>
>>> After this patch, we build free nids from nid #0 at mount time to make more
>>> full NAT blocks, but in runtime, we check empty NAT blocks to load free nids
>>> without loading any NAT pages from disk.
>>>
>>> Signed-off-by: Jaegeuk Kim 
>>> ---
>>>  fs/f2fs/checkpoint.c|  29 +++-
>>>  fs/f2fs/debug.c |   1 +
>>>  fs/f2fs/f2fs.h  |  23 +-
>>>  fs/f2fs/node.c  | 188 
>>> +++-
>>>  fs/f2fs/segment.c   |   2 +-
>>>  include/linux/f2fs_fs.h |   1 +
>>>  6 files changed, 224 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index 042f8d9afe44..783c5c3f16a4 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -1024,6 +1024,10 @@ static void update_ckpt_flags(struct f2fs_sb_info 
>>> *sbi, struct cp_control *cpc)
>>>  
>>> spin_lock(>cp_lock);
>>>  
>>> +   if (ckpt->cp_pack_total_block_count >
>>> +   sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks)
>>> +   disable_nat_bits(sbi, false);
>>
>> I think we need to drop nat full/empty bitmap only if there is no enough 
>> space
>> in CP area while doing umount, otherwise we can keep this in memory.
> 
> Yup.
> 
>>
>>> +
>>> if (cpc->reason == CP_UMOUNT)
>>> __set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
>>> else
>>> @@ -1136,6 +1140,29 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, 
>>> struct cp_control *cpc)
>>>  
>>> start_blk = __start_cp_next_addr(sbi);
>>>  
>>> +   /* write nat bits */
> 
> ...
> 
>>> +static int scan_nat_bits(struct f2fs_sb_info *sbi)
>>> +{
>>> +   struct f2fs_nm_info *nm_i = NM_I(sbi);
>>> +   struct page *page;
>>> +   unsigned int i = 0;
>>> +   nid_t target = FREE_NID_PAGES * NAT_ENTRY_PER_BLOCK;
>>> +   nid_t nid;
>>> +
>>> +   if (!is_set_ckpt_flags(sbi, CP_NAT_BITS_FLAG))
>>> +   return -EAGAIN;
>>> +
>>> +   down_read(_i->nat_tree_lock);
>>> +check_empty:
>>> +   i = find_next_bit_le(nm_i->empty_nat_bits, nm_i->nat_blocks, i);
>>> +   if (i >= nm_i->nat_blocks) {
>>> +   i = 0;
>>> +   goto check_partial;
>>> +   }
>>> +
>>> +   for (nid = i * NAT_ENTRY_PER_BLOCK; nid < (i + 1) * NAT_ENTRY_PER_BLOCK;
>>> +   nid++) {
>>> +   if (unlikely(nid >= nm_i->max_nid))
>>> +   break;
>>> +   add_free_nid(sbi, nid, true);
>>> +   }
>>> +
>>> +   if (nm_i->nid_cnt[FREE_NID_LIST] >= target)
>>> +   goto out;
>>> +   i++;
>>> +   goto check_empty;
>>> +
>>> +check_partial:
>>> +   i = find_next_zero_bit_le(nm_i->full_nat_bits, nm_i->nat_blocks, i);
>>> +   if (i >= nm_i->nat_blocks) {
>>> +   disable_nat_bits(sbi, true);
>>
>> Can this happen in real world? Should be a bug in somewhere?
> 
> It happens, since current design handles full_nat_bits optionally in order
> to avoid scanning a whole NAT page to set it back as 1 from 0.

All 0 value in full_nat_bits means the NAT block has at least one free nid,
right? so if we traverse all such NAT blocks, and still haven't collect enough
*target* number of free nids, we don't have to disable nat bit cache, we can
just break out here.

Thanks,

> 
>>
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   nid = i * NAT_ENTRY_PER_BLOCK;
>>> +   page = get_current_nat_page(sbi, nid);
>>> +   scan_nat_page(sbi, page, nid);
>>> +   f2fs_put_page(page, 1);
>>> +
>>> +   if (nm_i->nid_cnt[FREE_NID_LIST] < target) {
>>> +   i++;
>>> +   goto check_partial;
>>> +   }
>>> +out:
>>> +   up_read(_i->nat_tree_lock);
>>> +   return 0;
>>> +}
>>> +
>>> +static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool 
>>> mount)
>>>  {
>>> struct f2fs_nm_info *nm_i = NM_I(sbi);
>>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>> @@ -1854,6 +1911,20 @@ static void __build_free_nids(struct f2fs_sb_info 
>>> *sbi, bool sync)
>>> if (!sync && !available_free_memory(sbi, FREE_NIDS))
>>> return;
>>>  
>>> +   /* try to find free nids with nat_bits */
>>> +   if (!mount && !scan_nat_bits(sbi) && nm_i->nid_cnt[FREE_NID_LIST])
>>> +   return;
>>> +
>>> +   /* find next valid candidate */
>>
>> This is just for mount case?
> 
> Yup, it reuses free nids in dirty NAT blocks, so that we can make them as full
> NAT pages.
> 
> 

Re: [f2fs-dev] [PATCH 2/3] f2fs: add bitmaps for empty or full NAT blocks

[Chao Yu]

On 2017/2/24 6:54, Jaegeuk Kim wrote:
> On 02/23, Chao Yu wrote:
>> On 2017/2/14 10:06, Jaegeuk Kim wrote:
>>> This patches adds bitmaps to represent empty or full NAT blocks containing
>>> free nid entries.
>>>
>>> If we can find valid crc|cp_ver in the last block of checkpoint pack, we'll
>>> use these bitmaps when building free nids. In order to avoid checkpointing
>>> burden, up-to-date bitmaps will be flushed only during umount time. So,
>>> normally we can get this gain, but when power-cut happens, we rely on 
>>> fsck.f2fs
>>> which recovers this bitmap again.
>>>
>>> After this patch, we build free nids from nid #0 at mount time to make more
>>> full NAT blocks, but in runtime, we check empty NAT blocks to load free nids
>>> without loading any NAT pages from disk.
>>>
>>> Signed-off-by: Jaegeuk Kim 
>>> ---
>>>  fs/f2fs/checkpoint.c|  29 +++-
>>>  fs/f2fs/debug.c |   1 +
>>>  fs/f2fs/f2fs.h  |  23 +-
>>>  fs/f2fs/node.c  | 188 
>>> +++-
>>>  fs/f2fs/segment.c   |   2 +-
>>>  include/linux/f2fs_fs.h |   1 +
>>>  6 files changed, 224 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
>>> index 042f8d9afe44..783c5c3f16a4 100644
>>> --- a/fs/f2fs/checkpoint.c
>>> +++ b/fs/f2fs/checkpoint.c
>>> @@ -1024,6 +1024,10 @@ static void update_ckpt_flags(struct f2fs_sb_info 
>>> *sbi, struct cp_control *cpc)
>>>  
>>> spin_lock(>cp_lock);
>>>  
>>> +   if (ckpt->cp_pack_total_block_count >
>>> +   sbi->blocks_per_seg - NM_I(sbi)->nat_bits_blocks)
>>> +   disable_nat_bits(sbi, false);
>>
>> I think we need to drop nat full/empty bitmap only if there is no enough 
>> space
>> in CP area while doing umount, otherwise we can keep this in memory.
> 
> Yup.
> 
>>
>>> +
>>> if (cpc->reason == CP_UMOUNT)
>>> __set_ckpt_flags(ckpt, CP_UMOUNT_FLAG);
>>> else
>>> @@ -1136,6 +1140,29 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, 
>>> struct cp_control *cpc)
>>>  
>>> start_blk = __start_cp_next_addr(sbi);
>>>  
>>> +   /* write nat bits */
> 
> ...
> 
>>> +static int scan_nat_bits(struct f2fs_sb_info *sbi)
>>> +{
>>> +   struct f2fs_nm_info *nm_i = NM_I(sbi);
>>> +   struct page *page;
>>> +   unsigned int i = 0;
>>> +   nid_t target = FREE_NID_PAGES * NAT_ENTRY_PER_BLOCK;
>>> +   nid_t nid;
>>> +
>>> +   if (!is_set_ckpt_flags(sbi, CP_NAT_BITS_FLAG))
>>> +   return -EAGAIN;
>>> +
>>> +   down_read(_i->nat_tree_lock);
>>> +check_empty:
>>> +   i = find_next_bit_le(nm_i->empty_nat_bits, nm_i->nat_blocks, i);
>>> +   if (i >= nm_i->nat_blocks) {
>>> +   i = 0;
>>> +   goto check_partial;
>>> +   }
>>> +
>>> +   for (nid = i * NAT_ENTRY_PER_BLOCK; nid < (i + 1) * NAT_ENTRY_PER_BLOCK;
>>> +   nid++) {
>>> +   if (unlikely(nid >= nm_i->max_nid))
>>> +   break;
>>> +   add_free_nid(sbi, nid, true);
>>> +   }
>>> +
>>> +   if (nm_i->nid_cnt[FREE_NID_LIST] >= target)
>>> +   goto out;
>>> +   i++;
>>> +   goto check_empty;
>>> +
>>> +check_partial:
>>> +   i = find_next_zero_bit_le(nm_i->full_nat_bits, nm_i->nat_blocks, i);
>>> +   if (i >= nm_i->nat_blocks) {
>>> +   disable_nat_bits(sbi, true);
>>
>> Can this happen in real world? Should be a bug in somewhere?
> 
> It happens, since current design handles full_nat_bits optionally in order
> to avoid scanning a whole NAT page to set it back as 1 from 0.

All 0 value in full_nat_bits means the NAT block has at least one free nid,
right? so if we traverse all such NAT blocks, and still haven't collect enough
*target* number of free nids, we don't have to disable nat bit cache, we can
just break out here.

Thanks,

> 
>>
>>> +   return -EINVAL;
>>> +   }
>>> +
>>> +   nid = i * NAT_ENTRY_PER_BLOCK;
>>> +   page = get_current_nat_page(sbi, nid);
>>> +   scan_nat_page(sbi, page, nid);
>>> +   f2fs_put_page(page, 1);
>>> +
>>> +   if (nm_i->nid_cnt[FREE_NID_LIST] < target) {
>>> +   i++;
>>> +   goto check_partial;
>>> +   }
>>> +out:
>>> +   up_read(_i->nat_tree_lock);
>>> +   return 0;
>>> +}
>>> +
>>> +static void __build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool 
>>> mount)
>>>  {
>>> struct f2fs_nm_info *nm_i = NM_I(sbi);
>>> struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
>>> @@ -1854,6 +1911,20 @@ static void __build_free_nids(struct f2fs_sb_info 
>>> *sbi, bool sync)
>>> if (!sync && !available_free_memory(sbi, FREE_NIDS))
>>> return;
>>>  
>>> +   /* try to find free nids with nat_bits */
>>> +   if (!mount && !scan_nat_bits(sbi) && nm_i->nid_cnt[FREE_NID_LIST])
>>> +   return;
>>> +
>>> +   /* find next valid candidate */
>>
>> This is just for mount case?
> 
> Yup, it reuses free nids in dirty NAT blocks, so that we can make them as full
> NAT pages.
> 
> Thanks,
> 
>>
>>> +   

Re: [PATCH v1] regulator: Add driver for voltage controlled regulators

[Matthias Kaehlcke]

Rob, thanks for your comments!

El Tue, Feb 21, 2017 at 06:22:14PM -0600 Rob Herring ha dit:

> On Fri, Feb 10, 2017 at 12:43:48PM -0800, Matthias Kaehlcke wrote:
> > The output voltage of a voltage controlled regulator can be controlled
> > through the voltage of another regulator. The current version of this
> > driver assumes that the output voltage is a linear function of the control
> > voltage.
> >
> > ...
> > 
> >  .../devicetree/bindings/regulator/vctrl.txt|  56 +++
> 
> Please split bindings to separate patch.

The driver is not functional without the bindings, is a separate patch
preferred nevertheless? I saw other recentish regulator drivers
(ltc3676, pv88080) added the driver and the bindings together.

> > +Optional properties:
> > +
> > ...
> > +- min-slew-down-rate   : Describes how slowly the regulator voltage 
> > will decay
> > + down in the worst case (lightest expected load).
> > + Specified in uV / us (like main regulator ramp rate).
> > + This value is required when ovp-threshold-percent is
> > + specified.
> 
> Don't we have a standard prop for this or that's just for ramp?

regulator-ramp-delay is related, but not exactly the same. The
ramp-delay is applied at the end of an up- or downward transition,
while this prop only specifies the downward rate and is applied in
between partial transitions towards the final voltage.

We possibly could use ramp-delay and add a set_voltage_time() op to
vctrl to prevent the core code from adding the "normal" ramp-delay at
the end of the transition. However it could be confusing that vctrl
handles the ramp-delay differently than other drivers, especially we
don't want a delay in the upward transition for vctrl. But maybe
nobody would care about the different behavior, as long as the
regulator does its job ...

> Perhaps this should be common?

I agree this could be a property other regulators might have. I think
it could be common as long as it is a descriptive property which
regulator drivers can use or not, without adding functionality to the
core code (except for DT parsing).

Mark, what do you think?

> > +
> > +Example:
> > +
> > +   vctrl_reg {
> 
> Don't use '_' in node names.

Will fix in next revision.

> > +   compatible = "vctrl-regulator";
> > +   regulator-name = "vctrl_reg";
> > +
> > +   ctrl-supply = <_supply>;
> > +
> 
> > +   regulator-min-microvolt = <80>;
> > +   regulator-max-microvolt = <150>;
> > +
> > +   output-voltage-range = <80 150>;
> 
> Why do you need both?

We don't necessarily need both. The constraints are not available yet
when the driver parses and validates the DT node, but we can read
regulator-min/max-microvolt manually in the driver instead of
duplicating the values.

> > +   ctrl-voltage-range = <20 50>;
> > +
> > +   slew-rate = <225>;
> 
> Not documented.

Should be min-slew-down-rate. Will fix in next revision.

Thanks

Matthias

Re: [PATCH v1] regulator: Add driver for voltage controlled regulators

[Matthias Kaehlcke]

Rob, thanks for your comments!

El Tue, Feb 21, 2017 at 06:22:14PM -0600 Rob Herring ha dit:

> On Fri, Feb 10, 2017 at 12:43:48PM -0800, Matthias Kaehlcke wrote:
> > The output voltage of a voltage controlled regulator can be controlled
> > through the voltage of another regulator. The current version of this
> > driver assumes that the output voltage is a linear function of the control
> > voltage.
> >
> > ...
> > 
> >  .../devicetree/bindings/regulator/vctrl.txt|  56 +++
> 
> Please split bindings to separate patch.

The driver is not functional without the bindings, is a separate patch
preferred nevertheless? I saw other recentish regulator drivers
(ltc3676, pv88080) added the driver and the bindings together.

> > +Optional properties:
> > +
> > ...
> > +- min-slew-down-rate   : Describes how slowly the regulator voltage 
> > will decay
> > + down in the worst case (lightest expected load).
> > + Specified in uV / us (like main regulator ramp rate).
> > + This value is required when ovp-threshold-percent is
> > + specified.
> 
> Don't we have a standard prop for this or that's just for ramp?

regulator-ramp-delay is related, but not exactly the same. The
ramp-delay is applied at the end of an up- or downward transition,
while this prop only specifies the downward rate and is applied in
between partial transitions towards the final voltage.

We possibly could use ramp-delay and add a set_voltage_time() op to
vctrl to prevent the core code from adding the "normal" ramp-delay at
the end of the transition. However it could be confusing that vctrl
handles the ramp-delay differently than other drivers, especially we
don't want a delay in the upward transition for vctrl. But maybe
nobody would care about the different behavior, as long as the
regulator does its job ...

> Perhaps this should be common?

I agree this could be a property other regulators might have. I think
it could be common as long as it is a descriptive property which
regulator drivers can use or not, without adding functionality to the
core code (except for DT parsing).

Mark, what do you think?

> > +
> > +Example:
> > +
> > +   vctrl_reg {
> 
> Don't use '_' in node names.

Will fix in next revision.

> > +   compatible = "vctrl-regulator";
> > +   regulator-name = "vctrl_reg";
> > +
> > +   ctrl-supply = <_supply>;
> > +
> 
> > +   regulator-min-microvolt = <80>;
> > +   regulator-max-microvolt = <150>;
> > +
> > +   output-voltage-range = <80 150>;
> 
> Why do you need both?

We don't necessarily need both. The constraints are not available yet
when the driver parses and validates the DT node, but we can read
regulator-min/max-microvolt manually in the driver instead of
duplicating the values.

> > +   ctrl-voltage-range = <20 50>;
> > +
> > +   slew-rate = <225>;
> 
> Not documented.

Should be min-slew-down-rate. Will fix in next revision.

Thanks

Matthias

Re: [PATCH v4 1/5] vfs: Add file timestamp range support

[Darrick J. Wong]

On Fri, Feb 24, 2017 at 05:40:59PM -0800, Deepa Dinamani wrote:
> Add fields to the superblock to track the min and max
> timestamps supported by filesystems.
> 
> Initially, when a superblock is allocated, initialize
> it to the max and min values the fields can hold.
> Individual filesystems override these to match their
> actual limits.
> 
> Pseudo filesystems are assumed to always support the
> min and max allowable values for the fields.
> 
> Note that the time ranges are saved in type time64_t
> rather than time_t.
> This is required because if we save ranges in time_t
> then we would not be able to save timestamp ranges
> for files that support timestamps beyond y2038.
> 
> Signed-off-by: Deepa Dinamani 
> ---
>  fs/super.c | 2 ++
>  include/linux/fs.h | 3 +++
>  include/linux/time64.h | 2 ++
>  3 files changed, 7 insertions(+)
> 
> diff --git a/fs/super.c b/fs/super.c
> index b8b6a08..f9c2241 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -247,6 +247,8 @@ static struct super_block *alloc_super(struct 
> file_system_type *type, int flags,
>   s->s_maxbytes = MAX_NON_LFS;
>   s->s_op = _op;
>   s->s_time_gran = 10;
> + s->s_time_min = TIME64_MIN;
> + s->s_time_max = TIME64_MAX;

Just out of curiosity, does this enable 64-bit timestamps for everything
by default?  I see that ext4 later sets its own values in fill_super;
what about things like XFS that really only have 32-bit seconds fields?

--D

>   s->cleancache_poolid = CLEANCACHE_NO_POOL;
>  
>   s->s_shrink.seeks = DEFAULT_SEEKS;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index de8ed0b..ef55dfb 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1337,6 +1337,9 @@ struct super_block {
>   /* Granularity of c/m/atime in ns.
>  Cannot be worse than a second */
>   u32s_time_gran;
> + /* Time limits for c/m/atime in seconds. */
> + time64_t   s_time_min;
> + time64_t   s_time_max;
>  
>   /*
>* The next field is for VFS *only*. No filesystems have any business
> diff --git a/include/linux/time64.h b/include/linux/time64.h
> index 980c71b..25433b18 100644
> --- a/include/linux/time64.h
> +++ b/include/linux/time64.h
> @@ -38,6 +38,8 @@ struct itimerspec64 {
>  
>  /* Located here for timespec[64]_valid_strict */
>  #define TIME64_MAX   ((s64)~((u64)1 << 63))
> +#define TIME64_MIN   (-TIME64_MAX - 1)
> +
>  #define KTIME_MAX((s64)~((u64)1 << 63))
>  #define KTIME_SEC_MAX(KTIME_MAX / NSEC_PER_SEC)
>  
> -- 
> 2.7.4
> 

Re: [PATCH v4 1/5] vfs: Add file timestamp range support

[Darrick J. Wong]

On Fri, Feb 24, 2017 at 05:40:59PM -0800, Deepa Dinamani wrote:
> Add fields to the superblock to track the min and max
> timestamps supported by filesystems.
> 
> Initially, when a superblock is allocated, initialize
> it to the max and min values the fields can hold.
> Individual filesystems override these to match their
> actual limits.
> 
> Pseudo filesystems are assumed to always support the
> min and max allowable values for the fields.
> 
> Note that the time ranges are saved in type time64_t
> rather than time_t.
> This is required because if we save ranges in time_t
> then we would not be able to save timestamp ranges
> for files that support timestamps beyond y2038.
> 
> Signed-off-by: Deepa Dinamani 
> ---
>  fs/super.c | 2 ++
>  include/linux/fs.h | 3 +++
>  include/linux/time64.h | 2 ++
>  3 files changed, 7 insertions(+)
> 
> diff --git a/fs/super.c b/fs/super.c
> index b8b6a08..f9c2241 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -247,6 +247,8 @@ static struct super_block *alloc_super(struct 
> file_system_type *type, int flags,
>   s->s_maxbytes = MAX_NON_LFS;
>   s->s_op = _op;
>   s->s_time_gran = 10;
> + s->s_time_min = TIME64_MIN;
> + s->s_time_max = TIME64_MAX;

Just out of curiosity, does this enable 64-bit timestamps for everything
by default?  I see that ext4 later sets its own values in fill_super;
what about things like XFS that really only have 32-bit seconds fields?

--D

>   s->cleancache_poolid = CLEANCACHE_NO_POOL;
>  
>   s->s_shrink.seeks = DEFAULT_SEEKS;
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index de8ed0b..ef55dfb 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1337,6 +1337,9 @@ struct super_block {
>   /* Granularity of c/m/atime in ns.
>  Cannot be worse than a second */
>   u32s_time_gran;
> + /* Time limits for c/m/atime in seconds. */
> + time64_t   s_time_min;
> + time64_t   s_time_max;
>  
>   /*
>* The next field is for VFS *only*. No filesystems have any business
> diff --git a/include/linux/time64.h b/include/linux/time64.h
> index 980c71b..25433b18 100644
> --- a/include/linux/time64.h
> +++ b/include/linux/time64.h
> @@ -38,6 +38,8 @@ struct itimerspec64 {
>  
>  /* Located here for timespec[64]_valid_strict */
>  #define TIME64_MAX   ((s64)~((u64)1 << 63))
> +#define TIME64_MIN   (-TIME64_MAX - 1)
> +
>  #define KTIME_MAX((s64)~((u64)1 << 63))
>  #define KTIME_SEC_MAX(KTIME_MAX / NSEC_PER_SEC)
>  
> -- 
> 2.7.4
> 

[PATCH] f2fs: show simple call stack in fault injection message

[Chao Yu]

Previously kernel message can show that in which function we do the
injection, but unfortunately, most of the caller are the same, for
tracking more information of injection path, it needs to show upper
caller's name. This patch supports that ability.

Signed-off-by: Chao Yu 
---
 fs/f2fs/checkpoint.c |  1 +
 fs/f2fs/data.c   |  4 +++-
 fs/f2fs/dir.c|  4 +++-
 fs/f2fs/f2fs.h   | 20 +---
 fs/f2fs/gc.c |  4 +++-
 fs/f2fs/inode.c  |  4 +++-
 fs/f2fs/node.c   |  4 +++-
 fs/f2fs/segment.c|  4 +++-
 8 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index cd7132121573..04d7c244c0f0 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -494,6 +494,7 @@ int acquire_orphan_inode(struct f2fs_sb_info *sbi)
 #ifdef CONFIG_F2FS_FAULT_INJECTION
if (time_to_inject(sbi, FAULT_ORPHAN)) {
spin_unlock(>ino_lock);
+   f2fs_show_injection_info(FAULT_ORPHAN);
return -ENOSPC;
}
 #endif
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9e51c5e40ce1..b0a2e3faabb2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -55,8 +55,10 @@ static void f2fs_read_end_io(struct bio *bio)
int i;
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(F2FS_P_SB(bio->bi_io_vec->bv_page), FAULT_IO))
+   if (time_to_inject(F2FS_P_SB(bio->bi_io_vec->bv_page), FAULT_IO)) {
+   f2fs_show_injection_info(FAULT_IO);
bio->bi_error = -EIO;
+   }
 #endif
 
if (f2fs_bio_encrypted(bio)) {
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 54aa30ee028f..295a223ae11e 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -549,8 +549,10 @@ int f2fs_add_regular_entry(struct inode *dir, const struct 
qstr *new_name,
 
 start:
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(F2FS_I_SB(dir), FAULT_DIR_DEPTH))
+   if (time_to_inject(F2FS_I_SB(dir), FAULT_DIR_DEPTH)) {
+   f2fs_show_injection_info(FAULT_DIR_DEPTH);
return -ENOSPC;
+   }
 #endif
if (unlikely(current_depth == MAX_DIR_HASH_DEPTH))
return -ENOSPC;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8b6d49642a15..6a31b36e59bc 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -954,6 +954,10 @@ struct f2fs_sb_info {
 };
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
+#define f2fs_show_injection_info(type) \
+   printk("%sF2FS-fs : inject %s in %s of %pF\n",  \
+   KERN_INFO, fault_name[type],\
+   __func__, __builtin_return_address(0))
 static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
 {
struct f2fs_fault_info *ffi = >fault_info;
@@ -967,10 +971,6 @@ static inline bool time_to_inject(struct f2fs_sb_info 
*sbi, int type)
atomic_inc(>inject_ops);
if (atomic_read(>inject_ops) >= ffi->inject_rate) {
atomic_set(>inject_ops, 0);
-   printk("%sF2FS-fs : inject %s in %pF\n",
-   KERN_INFO,
-   fault_name[type],
-   __builtin_return_address(0));
return true;
}
return false;
@@ -1279,8 +1279,10 @@ static inline bool inc_valid_block_count(struct 
f2fs_sb_info *sbi,
blkcnt_t diff;
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(sbi, FAULT_BLOCK))
+   if (time_to_inject(sbi, FAULT_BLOCK)) {
+   f2fs_show_injection_info(FAULT_BLOCK);
return false;
+   }
 #endif
/*
 * let's increase this in prior to actual block count change in order
@@ -1520,8 +1522,10 @@ static inline struct page *f2fs_grab_cache_page(struct 
address_space *mapping,
if (page)
return page;
 
-   if (time_to_inject(F2FS_M_SB(mapping), FAULT_PAGE_ALLOC))
+   if (time_to_inject(F2FS_M_SB(mapping), FAULT_PAGE_ALLOC)) {
+   f2fs_show_injection_info(FAULT_PAGE_ALLOC);
return NULL;
+   }
 #endif
if (!for_write)
return grab_cache_page(mapping, index);
@@ -1997,8 +2001,10 @@ static inline void *f2fs_kmalloc(struct f2fs_sb_info 
*sbi,
size_t size, gfp_t flags)
 {
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(sbi, FAULT_KMALLOC))
+   if (time_to_inject(sbi, FAULT_KMALLOC)) {
+   f2fs_show_injection_info(FAULT_KMALLOC);
return NULL;
+   }
 #endif
return kmalloc(size, flags);
 }
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 6c996e39b59a..8be5144da8e6 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -48,8 +48,10 @@ static int gc_thread_func(void *data)
}
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(sbi, FAULT_CHECKPOINT))
+   if (time_to_inject(sbi, FAULT_CHECKPOINT)) {
+   

[PATCH] f2fs: show simple call stack in fault injection message

[Chao Yu]

Previously kernel message can show that in which function we do the
injection, but unfortunately, most of the caller are the same, for
tracking more information of injection path, it needs to show upper
caller's name. This patch supports that ability.

Signed-off-by: Chao Yu 
---
 fs/f2fs/checkpoint.c |  1 +
 fs/f2fs/data.c   |  4 +++-
 fs/f2fs/dir.c|  4 +++-
 fs/f2fs/f2fs.h   | 20 +---
 fs/f2fs/gc.c |  4 +++-
 fs/f2fs/inode.c  |  4 +++-
 fs/f2fs/node.c   |  4 +++-
 fs/f2fs/segment.c|  4 +++-
 8 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index cd7132121573..04d7c244c0f0 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -494,6 +494,7 @@ int acquire_orphan_inode(struct f2fs_sb_info *sbi)
 #ifdef CONFIG_F2FS_FAULT_INJECTION
if (time_to_inject(sbi, FAULT_ORPHAN)) {
spin_unlock(>ino_lock);
+   f2fs_show_injection_info(FAULT_ORPHAN);
return -ENOSPC;
}
 #endif
diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 9e51c5e40ce1..b0a2e3faabb2 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -55,8 +55,10 @@ static void f2fs_read_end_io(struct bio *bio)
int i;
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(F2FS_P_SB(bio->bi_io_vec->bv_page), FAULT_IO))
+   if (time_to_inject(F2FS_P_SB(bio->bi_io_vec->bv_page), FAULT_IO)) {
+   f2fs_show_injection_info(FAULT_IO);
bio->bi_error = -EIO;
+   }
 #endif
 
if (f2fs_bio_encrypted(bio)) {
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 54aa30ee028f..295a223ae11e 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -549,8 +549,10 @@ int f2fs_add_regular_entry(struct inode *dir, const struct 
qstr *new_name,
 
 start:
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(F2FS_I_SB(dir), FAULT_DIR_DEPTH))
+   if (time_to_inject(F2FS_I_SB(dir), FAULT_DIR_DEPTH)) {
+   f2fs_show_injection_info(FAULT_DIR_DEPTH);
return -ENOSPC;
+   }
 #endif
if (unlikely(current_depth == MAX_DIR_HASH_DEPTH))
return -ENOSPC;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 8b6d49642a15..6a31b36e59bc 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -954,6 +954,10 @@ struct f2fs_sb_info {
 };
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
+#define f2fs_show_injection_info(type) \
+   printk("%sF2FS-fs : inject %s in %s of %pF\n",  \
+   KERN_INFO, fault_name[type],\
+   __func__, __builtin_return_address(0))
 static inline bool time_to_inject(struct f2fs_sb_info *sbi, int type)
 {
struct f2fs_fault_info *ffi = >fault_info;
@@ -967,10 +971,6 @@ static inline bool time_to_inject(struct f2fs_sb_info 
*sbi, int type)
atomic_inc(>inject_ops);
if (atomic_read(>inject_ops) >= ffi->inject_rate) {
atomic_set(>inject_ops, 0);
-   printk("%sF2FS-fs : inject %s in %pF\n",
-   KERN_INFO,
-   fault_name[type],
-   __builtin_return_address(0));
return true;
}
return false;
@@ -1279,8 +1279,10 @@ static inline bool inc_valid_block_count(struct 
f2fs_sb_info *sbi,
blkcnt_t diff;
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(sbi, FAULT_BLOCK))
+   if (time_to_inject(sbi, FAULT_BLOCK)) {
+   f2fs_show_injection_info(FAULT_BLOCK);
return false;
+   }
 #endif
/*
 * let's increase this in prior to actual block count change in order
@@ -1520,8 +1522,10 @@ static inline struct page *f2fs_grab_cache_page(struct 
address_space *mapping,
if (page)
return page;
 
-   if (time_to_inject(F2FS_M_SB(mapping), FAULT_PAGE_ALLOC))
+   if (time_to_inject(F2FS_M_SB(mapping), FAULT_PAGE_ALLOC)) {
+   f2fs_show_injection_info(FAULT_PAGE_ALLOC);
return NULL;
+   }
 #endif
if (!for_write)
return grab_cache_page(mapping, index);
@@ -1997,8 +2001,10 @@ static inline void *f2fs_kmalloc(struct f2fs_sb_info 
*sbi,
size_t size, gfp_t flags)
 {
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(sbi, FAULT_KMALLOC))
+   if (time_to_inject(sbi, FAULT_KMALLOC)) {
+   f2fs_show_injection_info(FAULT_KMALLOC);
return NULL;
+   }
 #endif
return kmalloc(size, flags);
 }
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 6c996e39b59a..8be5144da8e6 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -48,8 +48,10 @@ static int gc_thread_func(void *data)
}
 
 #ifdef CONFIG_F2FS_FAULT_INJECTION
-   if (time_to_inject(sbi, FAULT_CHECKPOINT))
+   if (time_to_inject(sbi, FAULT_CHECKPOINT)) {
+   

Re: [PATCH v7 2/2] mtd: nand: Add support for Arasan NAND Flash Controller

[punnaiah choudary kalluri]

Hi Boris,

  Thanks for the review

On Sun, Feb 19, 2017 at 3:56 PM, Boris Brezillon
 wrote:
> Hi Punnaiah,
>
> Sorry for the late reply.
>
> On Mon, 9 Jan 2017 08:28:54 +0530
> Punnaiah Choudary Kalluri  wrote:
>
>> Added the basic driver for Arasan NAND Flash Controller used in
>> Zynq UltraScale+ MPSoC. It supports only Hw ECC and upto 24bit
>> correction.
>>
>> Signed-off-by: Punnaiah Choudary Kalluri 
>> ---
>> Changes in v7:
>> - Implemented Marek suggestions and comments
>> - Corrected the acronyms those should be in caps
>> - Modified kconfig/Make file to keep arasan entry in sorted order
>> - Added is_vmlloc_addr check
>> - Used ioread/write32_rep variants to avoid compilation error for intel
>>   platforms
>> - separated PIO and DMA mode read/write functions
>> - Minor cleanup
>> Chnages in v6:
>> - Addressed most of the Brian and Boris comments
>> - Separated the nandchip from the nand controller
>> - Removed the ecc lookup table from driver
>> - Now use framework nand waitfunction and readoob
>> - Fixed the compiler warning
>> - Adapted the new frameowrk changes related to ecc and ooblayout
>> - Disabled the clocks after the nand_reelase
>> - Now using only one completion object
>> - Boris suggessions like adapting cmd_ctrl and rework on read/write byte
>>   are not implemented and i will patch them later
>> - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr will
>>   implement later once the basic driver is mainlined.
>> Changes in v5:
>> - Renamed the driver filei as arasan_nand.c
>> - Fixed all comments relaqted coding style
>> - Fixed comments related to propagating the errors
>> - Modified the anfc_write_page_hwecc as per the write_page
>>   prototype
>> Changes in v4:
>> - Added support for onfi timing mode configuration
>> - Added clock suort
>> - Added support for multiple chipselects
>> Changes in v3:
>> - Removed unused variables
>> - Avoided busy loop and used jifies based implementation
>> - Fixed compiler warnings "right shift count >= width of type"
>> - Removed unneeded codei and improved error reporting
>> - Added onfi version check to ensure reading the valid address cycles
>> Changes in v2:
>> - Added missing of.h to avoid kbuild system report erro
>> ---
>>  drivers/mtd/nand/Kconfig   |   8 +
>>  drivers/mtd/nand/Makefile  |   1 +
>>  drivers/mtd/nand/arasan_nand.c | 932 
>> +
>>  3 files changed, 941 insertions(+)
>>  create mode 100644 drivers/mtd/nand/arasan_nand.c
>
> checkpatch.pl --strict reports a few coding style problems. Can you fix
> them?
>

Ok.

> [...]
>
>> +#define PROG_PGRDBIT(0)
>> +#define PROG_ERASE   BIT(2)
>> +#define PROG_STATUS  BIT(3)
>> +#define PROG_PGPROG  BIT(4)
>> +#define PROG_RDIDBIT(6)
>> +#define PROG_RDPARAM BIT(7)
>> +#define PROG_RST BIT(8)
>> +#define PROG_GET_FEATURE BIT(9)
>> +#define PROG_SET_FEATURE BIT(10)
>
> I know I'm being insistent on this, but I don't understand what these
> different prog modes are meant for. You still have to set the NAND
> command and address cycles, so it probably has to do with timing
> sequences, but that's not clearly described in the doc you pointed.
>

As per the spec, deepening on the operation to be perform,
the corresponding program bit need to be set. After this step, the controller
will initiate the cmd and data phase sequence.But even i am not sure why
we need to program though the required parameters are already configured
probably the controller internal state machine might need this information.


> [...]
>
>> +static void anfc_rw_buf_dma(struct mtd_info *mtd, uint8_t *buf, int len,
>> + int operation, u32 prog)
>> +{
>> + dma_addr_t paddr;
>> + struct nand_chip *chip = mtd_to_nand(mtd);
>> + struct anfc *nfc = to_anfc(chip->controller);
>> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
>> + u32 eccintr = 0, dir;
>> + u32 pktsize = len, pktcount = 1;
>> +
>> + if ((nfc->curr_cmd == NAND_CMD_READ0) ||
>> + ((nfc->curr_cmd == NAND_CMD_SEQIN) && !nfc->iswriteoob)) {
>> + pktsize = achip->pktsize;
>> + pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
>> + }
>
> I really don't like what's done here (the fact that you test
> ->curr_cmd in something that is supposed to be command agnostic). Maybe
> you should just avoid using ->write_buf() when programming a page, and
> have a custom function doing that.
>

Since we are overriding the write_buf/read_buf function because of the reason it
requires custom implementation. Also if we see nand transactions it
always follow the
COMMAND and DATA phase sequence. so, i didn't see any thing harm if
you keep track
of the command while reading the data in 

Re: [PATCH v7 2/2] mtd: nand: Add support for Arasan NAND Flash Controller

[punnaiah choudary kalluri]

Hi Boris,

  Thanks for the review

On Sun, Feb 19, 2017 at 3:56 PM, Boris Brezillon
 wrote:
> Hi Punnaiah,
>
> Sorry for the late reply.
>
> On Mon, 9 Jan 2017 08:28:54 +0530
> Punnaiah Choudary Kalluri  wrote:
>
>> Added the basic driver for Arasan NAND Flash Controller used in
>> Zynq UltraScale+ MPSoC. It supports only Hw ECC and upto 24bit
>> correction.
>>
>> Signed-off-by: Punnaiah Choudary Kalluri 
>> ---
>> Changes in v7:
>> - Implemented Marek suggestions and comments
>> - Corrected the acronyms those should be in caps
>> - Modified kconfig/Make file to keep arasan entry in sorted order
>> - Added is_vmlloc_addr check
>> - Used ioread/write32_rep variants to avoid compilation error for intel
>>   platforms
>> - separated PIO and DMA mode read/write functions
>> - Minor cleanup
>> Chnages in v6:
>> - Addressed most of the Brian and Boris comments
>> - Separated the nandchip from the nand controller
>> - Removed the ecc lookup table from driver
>> - Now use framework nand waitfunction and readoob
>> - Fixed the compiler warning
>> - Adapted the new frameowrk changes related to ecc and ooblayout
>> - Disabled the clocks after the nand_reelase
>> - Now using only one completion object
>> - Boris suggessions like adapting cmd_ctrl and rework on read/write byte
>>   are not implemented and i will patch them later
>> - Also check_erased_ecc_chunk for erase and check for is_vmalloc_addr will
>>   implement later once the basic driver is mainlined.
>> Changes in v5:
>> - Renamed the driver filei as arasan_nand.c
>> - Fixed all comments relaqted coding style
>> - Fixed comments related to propagating the errors
>> - Modified the anfc_write_page_hwecc as per the write_page
>>   prototype
>> Changes in v4:
>> - Added support for onfi timing mode configuration
>> - Added clock suort
>> - Added support for multiple chipselects
>> Changes in v3:
>> - Removed unused variables
>> - Avoided busy loop and used jifies based implementation
>> - Fixed compiler warnings "right shift count >= width of type"
>> - Removed unneeded codei and improved error reporting
>> - Added onfi version check to ensure reading the valid address cycles
>> Changes in v2:
>> - Added missing of.h to avoid kbuild system report erro
>> ---
>>  drivers/mtd/nand/Kconfig   |   8 +
>>  drivers/mtd/nand/Makefile  |   1 +
>>  drivers/mtd/nand/arasan_nand.c | 932 
>> +
>>  3 files changed, 941 insertions(+)
>>  create mode 100644 drivers/mtd/nand/arasan_nand.c
>
> checkpatch.pl --strict reports a few coding style problems. Can you fix
> them?
>

Ok.

> [...]
>
>> +#define PROG_PGRDBIT(0)
>> +#define PROG_ERASE   BIT(2)
>> +#define PROG_STATUS  BIT(3)
>> +#define PROG_PGPROG  BIT(4)
>> +#define PROG_RDIDBIT(6)
>> +#define PROG_RDPARAM BIT(7)
>> +#define PROG_RST BIT(8)
>> +#define PROG_GET_FEATURE BIT(9)
>> +#define PROG_SET_FEATURE BIT(10)
>
> I know I'm being insistent on this, but I don't understand what these
> different prog modes are meant for. You still have to set the NAND
> command and address cycles, so it probably has to do with timing
> sequences, but that's not clearly described in the doc you pointed.
>

As per the spec, deepening on the operation to be perform,
the corresponding program bit need to be set. After this step, the controller
will initiate the cmd and data phase sequence.But even i am not sure why
we need to program though the required parameters are already configured
probably the controller internal state machine might need this information.


> [...]
>
>> +static void anfc_rw_buf_dma(struct mtd_info *mtd, uint8_t *buf, int len,
>> + int operation, u32 prog)
>> +{
>> + dma_addr_t paddr;
>> + struct nand_chip *chip = mtd_to_nand(mtd);
>> + struct anfc *nfc = to_anfc(chip->controller);
>> + struct anfc_nand_chip *achip = to_anfc_nand(chip);
>> + u32 eccintr = 0, dir;
>> + u32 pktsize = len, pktcount = 1;
>> +
>> + if ((nfc->curr_cmd == NAND_CMD_READ0) ||
>> + ((nfc->curr_cmd == NAND_CMD_SEQIN) && !nfc->iswriteoob)) {
>> + pktsize = achip->pktsize;
>> + pktcount = DIV_ROUND_UP(mtd->writesize, pktsize);
>> + }
>
> I really don't like what's done here (the fact that you test
> ->curr_cmd in something that is supposed to be command agnostic). Maybe
> you should just avoid using ->write_buf() when programming a page, and
> have a custom function doing that.
>

Since we are overriding the write_buf/read_buf function because of the reason it
requires custom implementation. Also if we see nand transactions it
always follow the
COMMAND and DATA phase sequence. so, i didn't see any thing harm if
you keep track
of the command while reading the data in DATA phase. Please suggest.

> Let's try to keep ->read/write_buf() as generic as possible.
>
>> 

Re: [PATCH v2] staging: xgifb: correct the multiple line dereference

[Joe Perches]

On Sat, 2017-02-25 at 05:11 +0530, Arushi Singhal wrote:
> Error was reported by checkpatch.pl as
> WARNING: Avoid multiple line dereference...
> And If there is boolean operator then it is fixed by Splitting line at
> boolean operator.This is a coding style error.
> The changes are made such that the other errors does not generate
> beacause of this change like line exceeding 80 characters length.
> 
> Signed-off-by: Arushi Singhal 
> ---
>  changes in v2
>  - changes done such that no other errors can generate.
> ---
>  drivers/staging/xgifb/XGI_main_26.c | 21 +
>  drivers/staging/xgifb/vb_setmode.c  | 14 +++---
>  2 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/xgifb/XGI_main_26.c 
> b/drivers/staging/xgifb/XGI_main_26.c
> index 6930f7eb741b..651987398d42 100644
> --- a/drivers/staging/xgifb/XGI_main_26.c
> +++ b/drivers/staging/xgifb/XGI_main_26.c
> @@ -878,30 +878,27 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
> *xgifb_info)
>   }
>  
>   if ((filter >= 0) && (filter <= 7)) {
> + const u8 *f = 
> XGI_TV_filter[filter_tb].filter[filter];
> +
>   pr_debug("FilterTable[%d]-%d: %*ph\n",
>filter_tb, filter,
> -  4, XGI_TV_filter[filter_tb].
> -filter[filter]);
> +  4, f);

Do please rewrap these appropriately.  i.e.:

pr_debug(""FilterTable[%d]-%d: %*ph\n",
     filter_tb, filter, 4, f);

>   xgifb_reg_set(
>   XGIPART2,
>   0x35,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][0]));
> - xgifb_reg_set(
> + (f[0]));


and:
xgifb_reg_set(XGIPART2, 0x35, f[0]);

> + xgifb_reg_set(
>   XGIPART2,
>   0x36,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][1]));
> - xgifb_reg_set(
> + (f[1]));

xgifb_reg_set(XGIPART2, 0x36, f[1]);

etc...

Re: [PATCH v2] staging: xgifb: correct the multiple line dereference

[Joe Perches]

On Sat, 2017-02-25 at 05:11 +0530, Arushi Singhal wrote:
> Error was reported by checkpatch.pl as
> WARNING: Avoid multiple line dereference...
> And If there is boolean operator then it is fixed by Splitting line at
> boolean operator.This is a coding style error.
> The changes are made such that the other errors does not generate
> beacause of this change like line exceeding 80 characters length.
> 
> Signed-off-by: Arushi Singhal 
> ---
>  changes in v2
>  - changes done such that no other errors can generate.
> ---
>  drivers/staging/xgifb/XGI_main_26.c | 21 +
>  drivers/staging/xgifb/vb_setmode.c  | 14 +++---
>  2 files changed, 16 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/xgifb/XGI_main_26.c 
> b/drivers/staging/xgifb/XGI_main_26.c
> index 6930f7eb741b..651987398d42 100644
> --- a/drivers/staging/xgifb/XGI_main_26.c
> +++ b/drivers/staging/xgifb/XGI_main_26.c
> @@ -878,30 +878,27 @@ static void XGIfb_post_setmode(struct xgifb_video_info 
> *xgifb_info)
>   }
>  
>   if ((filter >= 0) && (filter <= 7)) {
> + const u8 *f = 
> XGI_TV_filter[filter_tb].filter[filter];
> +
>   pr_debug("FilterTable[%d]-%d: %*ph\n",
>filter_tb, filter,
> -  4, XGI_TV_filter[filter_tb].
> -filter[filter]);
> +  4, f);

Do please rewrap these appropriately.  i.e.:

pr_debug(""FilterTable[%d]-%d: %*ph\n",
     filter_tb, filter, 4, f);

>   xgifb_reg_set(
>   XGIPART2,
>   0x35,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][0]));
> - xgifb_reg_set(
> + (f[0]));


and:
xgifb_reg_set(XGIPART2, 0x35, f[0]);

> + xgifb_reg_set(
>   XGIPART2,
>   0x36,
> - (XGI_TV_filter[filter_tb].
> - filter[filter][1]));
> - xgifb_reg_set(
> + (f[1]));

xgifb_reg_set(XGIPART2, 0x36, f[1]);

etc...

Re: [PATCH V2 1/3] blk-mq: allocate blk_mq_tags and requests in correct node

[Jens Axboe]

On 02/01/2017 10:53 AM, Shaohua Li wrote:
> blk_mq_tags/requests of specific hardware queue are mostly used in
> specific cpus, which might not be in the same numa node as disk. For
> example, a nvme card is in node 0. half hardware queue will be used by
> node 0, the other node 1.

Applied 1-3 for this series, thanks Shaohua!

-- 
Jens Axboe

Re: [PATCH V2 1/3] blk-mq: allocate blk_mq_tags and requests in correct node

[Jens Axboe]

On 02/01/2017 10:53 AM, Shaohua Li wrote:
> blk_mq_tags/requests of specific hardware queue are mostly used in
> specific cpus, which might not be in the same numa node as disk. For
> example, a nvme card is in node 0. half hardware queue will be used by
> node 0, the other node 1.

Applied 1-3 for this series, thanks Shaohua!

-- 
Jens Axboe

Re: [PATCH v2 6/6] staging: rtl8192e: Removed unnecessary parentheses

[Joe Perches]

On Sat, 2017-02-25 at 04:18 +0530, SIMRAN SINGHAL wrote:
> On Fri, Feb 24, 2017 at 10:47 PM, Greg KH  wrote:
> > On Mon, Feb 20, 2017 at 10:41:47PM +0530, simran singhal wrote:
> > > Extra parentheses were causing checkpatch issues
> > > and were removed.
> > > 
> > > Signed-off-by: simran singhal 
> > > ---
> > > 
> > >  v2:
> > >-Removed parentheses around argument of cast
> > >-Removed cast
> > 
> > I can't keep track of random patches in the middle of a series that is
> > updated.
> > 
> > Please fix up and resend the whole series, as a series, that are linked
> > together in an email client, not as independent emails (i.e. use
> > git-send-email properly).
> 
>  I am using mutt for sending patches. In mutt I have to send each patch
>  of a particular patch series one by one. So, they come as independent
>  emails.
> 
>  And, they all get mix because of the reason, if I started sending new patch
>  series and at the same time sending new versions of patches of previous
>  patch series, so because of which their order changes.
> 
>  So, please suggest me some better way, considering I am using mutt.

git send-email

Re: [PATCH v2 6/6] staging: rtl8192e: Removed unnecessary parentheses

[Joe Perches]

On Sat, 2017-02-25 at 04:18 +0530, SIMRAN SINGHAL wrote:
> On Fri, Feb 24, 2017 at 10:47 PM, Greg KH  wrote:
> > On Mon, Feb 20, 2017 at 10:41:47PM +0530, simran singhal wrote:
> > > Extra parentheses were causing checkpatch issues
> > > and were removed.
> > > 
> > > Signed-off-by: simran singhal 
> > > ---
> > > 
> > >  v2:
> > >-Removed parentheses around argument of cast
> > >-Removed cast
> > 
> > I can't keep track of random patches in the middle of a series that is
> > updated.
> > 
> > Please fix up and resend the whole series, as a series, that are linked
> > together in an email client, not as independent emails (i.e. use
> > git-send-email properly).
> 
>  I am using mutt for sending patches. In mutt I have to send each patch
>  of a particular patch series one by one. So, they come as independent
>  emails.
> 
>  And, they all get mix because of the reason, if I started sending new patch
>  series and at the same time sending new versions of patches of previous
>  patch series, so because of which their order changes.
> 
>  So, please suggest me some better way, considering I am using mutt.

git send-email

Re: [PATCH v2 2/5] powerpc: kretprobes: override default function entry offset

[Ananth N Mavinakayanahalli]

On Wed, Feb 22, 2017 at 07:23:38PM +0530, Naveen N. Rao wrote:
> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
> 

Looks good.

> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

Re: [PATCH v2 2/5] powerpc: kretprobes: override default function entry offset

[Ananth N Mavinakayanahalli]

On Wed, Feb 22, 2017 at 07:23:38PM +0530, Naveen N. Rao wrote:
> With ABIv2, we offset 8 bytes into a function to get at the local entry
> point.
> 

Looks good.

> Signed-off-by: Naveen N. Rao 

Acked-by: Ananth N Mavinakayanahalli 

RE: [PATCH] net: intel: e1000: use new api ethtool_{get|set}_link_ksettings

[Brown, Aaron F]

> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Philippe Reynes
> Sent: Saturday, January 21, 2017 7:06 AM
> To: Kirsher, Jeffrey T ; da...@davemloft.net
> Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Philippe Reynes 
> Subject: [PATCH] net: intel: e1000: use new api
> ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c |   93 +++-
> --
>  1 files changed, 46 insertions(+), 47 deletions(-)

Tested-by: Aaron Brown 

RE: [PATCH] net: intel: e1000: use new api ethtool_{get|set}_link_ksettings

[Brown, Aaron F]

> From: netdev-ow...@vger.kernel.org [mailto:netdev-
> ow...@vger.kernel.org] On Behalf Of Philippe Reynes
> Sent: Saturday, January 21, 2017 7:06 AM
> To: Kirsher, Jeffrey T ; da...@davemloft.net
> Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; linux-
> ker...@vger.kernel.org; Philippe Reynes 
> Subject: [PATCH] net: intel: e1000: use new api
> ethtool_{get|set}_link_ksettings
> 
> The ethtool api {get|set}_settings is deprecated.
> We move this driver to new api {get|set}_link_ksettings.
> 
> As I don't have the hardware, I'd be very pleased if
> someone may test this patch.
> 
> Signed-off-by: Philippe Reynes 
> ---
>  drivers/net/ethernet/intel/e1000/e1000_ethtool.c |   93 +++-
> --
>  1 files changed, 46 insertions(+), 47 deletions(-)

Tested-by: Aaron Brown 

Re: [PATCH 3/3] f2fs: provide more chance for node and data to get ssr segment

[Yunlong Song]

The get_ssr_segment happens when need_SSR is true, that is to say we must make 
sure we can
achieve a right available SSR segment at least, so why not select the type with 
larger probability?
And I think warm and hot are more similar with each other, maybe the update 
frequency is difference,
but if time goes by, warm segments will finally update their blocks like hot 
segments, but cold segments
are something like which should and will never update, so I prefer to put hot 
and warm to one side,
and put cold to the other side.

On 2017/2/25 4:05, Jaegeuk Kim wrote:
> On 02/25, Yunlong Song wrote:
>> Hi，Jaegeuk and Chao,
>> Sorry，I misunderstood CURSEG_HOT_DATA is greater than CURSEG_WARM_DATA 
>> and use "i--" in the codes, so just forget that.
>> But there is still a issue, when type is CURSEG_WARM_DATA, chao's patch 
>> will select cold SSR segment first, but my patch will select hot SSR segment 
>> first, since I think the probability of having hot SSR segment is bigger 
>> than having cold SSR segment due to the temperature itself.
> One thing that I'm seeing is, theoretically hot segments will get dirty more
> frequently. So if we select dirty cold segments for warm data, we can gain
> fully valid cold segments more, which can mitigate log thrashing problem.
>
> Thanks,
>
>>
>> On 02/25/2017 01:58, Jaegeuk Kim wrote:
>> On 02/24, Yunlong Song wrote:
>>> Hi, Chao,
>>>
>>> Not looks good to me, since there is some case your code does not include:
>>> if type is CURSEG_HOT_DATA, and if get_victim also returns 0 for both 
>>> CURSEG_HOT_DATA and
>>> CURSEG_WARM_DATA, then i will be -1 and pass to get_victim in your code.  
>>> So I still suggest
>>> my original patch attached below.
>> Why does i become -1?
>>
>>> On 2017/2/24 18:47, Chao Yu wrote:
 On 2017/2/24 17:19, Yunlong Song wrote:
> Hi Jaegeuk and Chao,
>
> How about the question I pointed out in last mail:
> Why not take "neighboring temperature" for ssr? For example, if type 
> == CURSEG_COLD_DATA,
> the new patch selects CURSEG_HOT_DATA first, why not select 
> CURSEG_WARM_DATA first?
> The patch I sent ensure this "neighboring temperature" for ssr. This 
> is to reduce the influence of
> mixing different levels of hot/code node types.
 Agreed, I sent one patch for changing the policy of SSR, how do you think 
 of it?

 Thanks,

> On 2017/2/24 17:05, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> Reviewed-by: Chao Yu 
>>
>> For your attached two patches.
>>
>> Thanks,
>>
>> On 2017/2/23 9:17, Jaegeuk Kim wrote:
>>> Hi Yunlong,
>>>
>>> I've been testing the similar patches as I attached.
>>>
>>> Thanks,
>>>
>>> On 02/22, Yunlong Song wrote:
 Signed-off-by: Yunlong Song 
 ---
  fs/f2fs/segment.c | 9 +
  1 file changed, 9 insertions(+)

 diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
 index 9d13743..5fe71b9 100644
 --- a/fs/f2fs/segment.c
 +++ b/fs/f2fs/segment.c
 @@ -1540,12 +1540,17 @@ static int get_ssr_segment(struct f2fs_sb_info 
 *sbi, int type)
  {
  struct curseg_info *curseg = CURSEG_I(sbi, type);
  const struct victim_selection *v_ops = DIRTY_I(sbi)->v_ops;
 +int old_type = type;
  
  if (IS_NODESEG(type)) {
  for (; type >= CURSEG_HOT_NODE; type--)
  if (v_ops->get_victim(sbi, &(curseg)->next_segno,
  BG_GC, type, SSR))
  return 1;
 +for (type = old_type + 1; type <= CURSEG_COLD_NODE; type++)
 +if (v_ops->get_victim(sbi, &(curseg)->next_segno,
 +BG_GC, type, SSR))
 +return 1;
  return 0;
  }
  
 @@ -1554,6 +1559,10 @@ static int get_ssr_segment(struct f2fs_sb_info 
 *sbi, int type)
  if (v_ops->get_victim(sbi, &(curseg)->next_segno,
  BG_GC, type, SSR))
  return 1;
 +for (type = old_type + 1; type <= CURSEG_COLD_DATA; type++)
 +if (v_ops->get_victim(sbi, &(curseg)->next_segno,
 +BG_GC, type, SSR))
 +return 1;
  return 0;
  }
  
 --
 1.8.5.2
>> .
>>
 .

>>>
>>> --
>>> Thanks,
>>> Yunlong Song
>>>
> .
>


-- 
Thanks,
Yunlong Song

Re: [PATCH 3/3] f2fs: provide more chance for node and data to get ssr segment

[Yunlong Song]

The get_ssr_segment happens when need_SSR is true, that is to say we must make 
sure we can
achieve a right available SSR segment at least, so why not select the type with 
larger probability?
And I think warm and hot are more similar with each other, maybe the update 
frequency is difference,
but if time goes by, warm segments will finally update their blocks like hot 
segments, but cold segments
are something like which should and will never update, so I prefer to put hot 
and warm to one side,
and put cold to the other side.

On 2017/2/25 4:05, Jaegeuk Kim wrote:
> On 02/25, Yunlong Song wrote:
>> Hi，Jaegeuk and Chao,
>> Sorry，I misunderstood CURSEG_HOT_DATA is greater than CURSEG_WARM_DATA 
>> and use "i--" in the codes, so just forget that.
>> But there is still a issue, when type is CURSEG_WARM_DATA, chao's patch 
>> will select cold SSR segment first, but my patch will select hot SSR segment 
>> first, since I think the probability of having hot SSR segment is bigger 
>> than having cold SSR segment due to the temperature itself.
> One thing that I'm seeing is, theoretically hot segments will get dirty more
> frequently. So if we select dirty cold segments for warm data, we can gain
> fully valid cold segments more, which can mitigate log thrashing problem.
>
> Thanks,
>
>>
>> On 02/25/2017 01:58, Jaegeuk Kim wrote:
>> On 02/24, Yunlong Song wrote:
>>> Hi, Chao,
>>>
>>> Not looks good to me, since there is some case your code does not include:
>>> if type is CURSEG_HOT_DATA, and if get_victim also returns 0 for both 
>>> CURSEG_HOT_DATA and
>>> CURSEG_WARM_DATA, then i will be -1 and pass to get_victim in your code.  
>>> So I still suggest
>>> my original patch attached below.
>> Why does i become -1?
>>
>>> On 2017/2/24 18:47, Chao Yu wrote:
 On 2017/2/24 17:19, Yunlong Song wrote:
> Hi Jaegeuk and Chao,
>
> How about the question I pointed out in last mail:
> Why not take "neighboring temperature" for ssr? For example, if type 
> == CURSEG_COLD_DATA,
> the new patch selects CURSEG_HOT_DATA first, why not select 
> CURSEG_WARM_DATA first?
> The patch I sent ensure this "neighboring temperature" for ssr. This 
> is to reduce the influence of
> mixing different levels of hot/code node types.
 Agreed, I sent one patch for changing the policy of SSR, how do you think 
 of it?

 Thanks,

> On 2017/2/24 17:05, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> Reviewed-by: Chao Yu 
>>
>> For your attached two patches.
>>
>> Thanks,
>>
>> On 2017/2/23 9:17, Jaegeuk Kim wrote:
>>> Hi Yunlong,
>>>
>>> I've been testing the similar patches as I attached.
>>>
>>> Thanks,
>>>
>>> On 02/22, Yunlong Song wrote:
 Signed-off-by: Yunlong Song 
 ---
  fs/f2fs/segment.c | 9 +
  1 file changed, 9 insertions(+)

 diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
 index 9d13743..5fe71b9 100644
 --- a/fs/f2fs/segment.c
 +++ b/fs/f2fs/segment.c
 @@ -1540,12 +1540,17 @@ static int get_ssr_segment(struct f2fs_sb_info 
 *sbi, int type)
  {
  struct curseg_info *curseg = CURSEG_I(sbi, type);
  const struct victim_selection *v_ops = DIRTY_I(sbi)->v_ops;
 +int old_type = type;
  
  if (IS_NODESEG(type)) {
  for (; type >= CURSEG_HOT_NODE; type--)
  if (v_ops->get_victim(sbi, &(curseg)->next_segno,
  BG_GC, type, SSR))
  return 1;
 +for (type = old_type + 1; type <= CURSEG_COLD_NODE; type++)
 +if (v_ops->get_victim(sbi, &(curseg)->next_segno,
 +BG_GC, type, SSR))
 +return 1;
  return 0;
  }
  
 @@ -1554,6 +1559,10 @@ static int get_ssr_segment(struct f2fs_sb_info 
 *sbi, int type)
  if (v_ops->get_victim(sbi, &(curseg)->next_segno,
  BG_GC, type, SSR))
  return 1;
 +for (type = old_type + 1; type <= CURSEG_COLD_DATA; type++)
 +if (v_ops->get_victim(sbi, &(curseg)->next_segno,
 +BG_GC, type, SSR))
 +return 1;
  return 0;
  }
  
 --
 1.8.5.2
>> .
>>
 .

>>>
>>> --
>>> Thanks,
>>> Yunlong Song
>>>
> .
>


-- 
Thanks,
Yunlong Song

Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting

[John Stultz]

On Thu, Feb 23, 2017 at 4:01 PM, Paul Moore  wrote:
> On Thu, Feb 23, 2017 at 1:43 PM, John Stultz  wrote:
>> Hey folks,
>>I've not been able to figure out why yet, but I wanted to raise the
>> issue that last night I found I couldn't boot Android on my Hikey
>> board with Linus' HEAD kernel. It seems to cause logd to crash
>> repeatedly so I'm not able to get debug info from logcat.
>>
>> I do see the following over and over on the console:
>>
>> [   12.505838] init: computing context for service 'logd'
>> [   12.506355] init: starting service 'logd'...
>> [   12.507683] init: property_set("ro.boottime.logd", "12500792498")
>> failed: property already set
>> [   12.508701] init: Created socket '/dev/socket/logd', mode 666, user
>> 1036, group 1036
>> [   12.509294] init: Created socket '/dev/socket/logdr', mode 666,
>> user 1036, group 1036
>> [   12.509891] init: Created socket '/dev/socket/logdw', mode 222,
>> user 1036, group 1036
>> [   12.510132] init: Opened file '/proc/kmsg', flags 0
>> [   12.510187] init: Opened file '/dev/kmsg', flags 1
>> [   12.510353] init: couldn't write 1941 to
>> /dev/cpuset/system-background/tasks: No such file or directory
>> [   12.533046] init: Service 'logd' (pid 1941) exited with status 255
>>
>>
>> I did some bisection and narrowed it down to 1ea0ce4069 ("selinux:
>> allow changing labels for cgroupfs"), which was merged in yesterday.
>> I've not yet been able to figure out the root cause, but reverting
>> that patch makes things work again.
>>
>> So I wanted to raise the issue here so folks were aware.
>>
>> If there is anything folks want me to test or try, please let me know.
>
> Unfortunately I don't have an Android test system to play with, have
> any of the SEAndroid folks on the To/CC line seen a similar problem?

So from my very limited knowledge here, adding the patch in question
seems to make the cgroup mount get the SBLABEL_MNT flag?
Which I'm guessing this is causing additional selinux restrictions on
processes accessing cgroup mounts, which causes some of the early
initialization processes to fail?

Should this change mean the selinux policy needs to be updated?

thanks
-john

Re: [Regression?] 1ea0ce4069 ("selinux: allow changing labels for cgroupfs") stops Android from booting

[John Stultz]

On Thu, Feb 23, 2017 at 4:01 PM, Paul Moore  wrote:
> On Thu, Feb 23, 2017 at 1:43 PM, John Stultz  wrote:
>> Hey folks,
>>I've not been able to figure out why yet, but I wanted to raise the
>> issue that last night I found I couldn't boot Android on my Hikey
>> board with Linus' HEAD kernel. It seems to cause logd to crash
>> repeatedly so I'm not able to get debug info from logcat.
>>
>> I do see the following over and over on the console:
>>
>> [   12.505838] init: computing context for service 'logd'
>> [   12.506355] init: starting service 'logd'...
>> [   12.507683] init: property_set("ro.boottime.logd", "12500792498")
>> failed: property already set
>> [   12.508701] init: Created socket '/dev/socket/logd', mode 666, user
>> 1036, group 1036
>> [   12.509294] init: Created socket '/dev/socket/logdr', mode 666,
>> user 1036, group 1036
>> [   12.509891] init: Created socket '/dev/socket/logdw', mode 222,
>> user 1036, group 1036
>> [   12.510132] init: Opened file '/proc/kmsg', flags 0
>> [   12.510187] init: Opened file '/dev/kmsg', flags 1
>> [   12.510353] init: couldn't write 1941 to
>> /dev/cpuset/system-background/tasks: No such file or directory
>> [   12.533046] init: Service 'logd' (pid 1941) exited with status 255
>>
>>
>> I did some bisection and narrowed it down to 1ea0ce4069 ("selinux:
>> allow changing labels for cgroupfs"), which was merged in yesterday.
>> I've not yet been able to figure out the root cause, but reverting
>> that patch makes things work again.
>>
>> So I wanted to raise the issue here so folks were aware.
>>
>> If there is anything folks want me to test or try, please let me know.
>
> Unfortunately I don't have an Android test system to play with, have
> any of the SEAndroid folks on the To/CC line seen a similar problem?

So from my very limited knowledge here, adding the patch in question
seems to make the cgroup mount get the SBLABEL_MNT flag?
Which I'm guessing this is causing additional selinux restrictions on
processes accessing cgroup mounts, which causes some of the early
initialization processes to fail?

Should this change mean the selinux policy needs to be updated?

thanks
-john

Re: [Merge branch 'core-debugobjects-for-linus' of git] 575260e3f8: WARNING: CPU: 0 PID: 1 at kernel/time/hrtimer.c:1090 hrtimer_init

[Fengguang Wu]

Hi Linus,

On Fri, Feb 24, 2017 at 02:37:04PM -0800, Linus Torvalds wrote:

This looks like two -tip trees together show some issue - the timer
updates from Thomas triggering a debugobjects check from Ingo, thus
fingering my merge as the culprit.

Added Thomas/Ingo to the cc, leaving everything quoted for their
edification and enjoyment.


The call trace looks similar to the below bug, so CC mac80211
maintainer and Marc.

The discussed fix patch is for drivers/net/wireless/mac80211_hwsim.c
and is not mainlined yet due to in a different tree.

Regards,
Fengguang

- Forwarded message from Marc Zyngier  -

Date: Fri, 17 Feb 2017 10:09:06 +
From: Marc Zyngier 
To: Thomas Gleixner , kernel test robot 

CC: Greg Kroah-Hartman , Tomasz Nowicki 
, Christoffer Dall
, Sasha Levin , 
"linux-wirel...@vger.kernel.org"
, Johannes Berg 

Subject: Re: [linux-stable] 4fc2942b6e kernel BUG at kernel/time/hrtimer.c:109!
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 
Icedove/45.6.0

On 17/02/17 09:28, Thomas Gleixner wrote:

On Fri, 17 Feb 2017, kernel test robot wrote:


Hi Marc,

We find this oops in linux-4.4.y. The gcc-6 compiled mainline kernel is fine.


The last bit is worrying, as mainline has the exact same bug. Has it
been tested the same way?



commit 4fc2942b6e2de2efc8a9d3784d4b0d3543149613
 hrtimer: Catch illegal clockids


And that commit is doing what the subject line says. Catch illegal usage.


[   38.101342] Call Trace:
[   38.101342] Call Trace:
[   38.102045]  [] tasklet_hrtimer_init+0x16/0x52
[   38.102045]  [] tasklet_hrtimer_init+0x16/0x52
[   38.103698]  [] mac80211_hwsim_new_radio+0x766/0x84d


The real bug is in this code:

drivers/net/wireless/mac80211_hwsim.c

mac80211_hwsim_new_radio()

tasklet_hrtimer_init(>beacon_timer,
 mac80211_hwsim_beacon,
 CLOCK_MONOTONIC_RAW, HRTIMER_MODE_ABS);

CLOCK_MONOTONIC_RAW is not a supported clockid for hrtimers. Sigh.

Fix below.

Thanks,

tglx

8<--

diff --git a/drivers/net/wireless/mac80211_hwsim.c 
b/drivers/net/wireless/mac80211_hwsim.c
index 0cd95120bc78..da363ec91a1c 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2535,9 +2535,8 @@ static int mac80211_hwsim_new_radio(struct genl_info 
*info,
data->debugfs,
data, _simulate_radar);

-   tasklet_hrtimer_init(>beacon_timer,
-mac80211_hwsim_beacon,
-CLOCK_MONOTONIC_RAW, HRTIMER_MODE_ABS);
+   tasklet_hrtimer_init(>beacon_timer, mac80211_hwsim_beacon,
+CLOCK_MONOTONIC, HRTIMER_MODE_ABS);

spin_lock_bh(_radio_lock);
list_add_tail(>list, _radios);



Acked-by: Marc Zyngier 

M.
--
Jazz is not dead. It just smells funny...

- End forwarded message -

Re: [Merge branch 'core-debugobjects-for-linus' of git] 575260e3f8: WARNING: CPU: 0 PID: 1 at kernel/time/hrtimer.c:1090 hrtimer_init

[Fengguang Wu]

Hi Linus,

On Fri, Feb 24, 2017 at 02:37:04PM -0800, Linus Torvalds wrote:

This looks like two -tip trees together show some issue - the timer
updates from Thomas triggering a debugobjects check from Ingo, thus
fingering my merge as the culprit.

Added Thomas/Ingo to the cc, leaving everything quoted for their
edification and enjoyment.


The call trace looks similar to the below bug, so CC mac80211
maintainer and Marc.

The discussed fix patch is for drivers/net/wireless/mac80211_hwsim.c
and is not mainlined yet due to in a different tree.

Regards,
Fengguang

- Forwarded message from Marc Zyngier  -

Date: Fri, 17 Feb 2017 10:09:06 +
From: Marc Zyngier 
To: Thomas Gleixner , kernel test robot 

CC: Greg Kroah-Hartman , Tomasz Nowicki 
, Christoffer Dall
, Sasha Levin , 
"linux-wirel...@vger.kernel.org"
, Johannes Berg 

Subject: Re: [linux-stable] 4fc2942b6e kernel BUG at kernel/time/hrtimer.c:109!
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 
Icedove/45.6.0

On 17/02/17 09:28, Thomas Gleixner wrote:

On Fri, 17 Feb 2017, kernel test robot wrote:


Hi Marc,

We find this oops in linux-4.4.y. The gcc-6 compiled mainline kernel is fine.


The last bit is worrying, as mainline has the exact same bug. Has it
been tested the same way?



commit 4fc2942b6e2de2efc8a9d3784d4b0d3543149613
 hrtimer: Catch illegal clockids


And that commit is doing what the subject line says. Catch illegal usage.


[   38.101342] Call Trace:
[   38.101342] Call Trace:
[   38.102045]  [] tasklet_hrtimer_init+0x16/0x52
[   38.102045]  [] tasklet_hrtimer_init+0x16/0x52
[   38.103698]  [] mac80211_hwsim_new_radio+0x766/0x84d


The real bug is in this code:

drivers/net/wireless/mac80211_hwsim.c

mac80211_hwsim_new_radio()

tasklet_hrtimer_init(>beacon_timer,
 mac80211_hwsim_beacon,
 CLOCK_MONOTONIC_RAW, HRTIMER_MODE_ABS);

CLOCK_MONOTONIC_RAW is not a supported clockid for hrtimers. Sigh.

Fix below.

Thanks,

tglx

8<--

diff --git a/drivers/net/wireless/mac80211_hwsim.c 
b/drivers/net/wireless/mac80211_hwsim.c
index 0cd95120bc78..da363ec91a1c 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -2535,9 +2535,8 @@ static int mac80211_hwsim_new_radio(struct genl_info 
*info,
data->debugfs,
data, _simulate_radar);

-   tasklet_hrtimer_init(>beacon_timer,
-mac80211_hwsim_beacon,
-CLOCK_MONOTONIC_RAW, HRTIMER_MODE_ABS);
+   tasklet_hrtimer_init(>beacon_timer, mac80211_hwsim_beacon,
+CLOCK_MONOTONIC, HRTIMER_MODE_ABS);

spin_lock_bh(_radio_lock);
list_add_tail(>list, _radios);



Acked-by: Marc Zyngier 

M.
--
Jazz is not dead. It just smells funny...

- End forwarded message -

Re: [RFC][PATCH v2] drm: kirin: Add a mutex to avoid fb initialization race

[John Stultz]

On Fri, Feb 24, 2017 at 5:45 PM, liuxinliang
 wrote:
>
>
> On 2017/2/25 9:39, liuxinliang wrote:
>>
>> Hi John,
>>
>> The patch seems good to me, except one minus comment.
>> Maybe change fb_lock to fbdev_lock would be better.
>>
>> Thanks,
>> -xinliang
>>
>> On 2017/2/25 9:25, John Stultz wrote:
>>>
>>> In some cases I've been seeing a race where two framebuffers
>>> would be initialized, as kirin_fbdev_output_poll_changed()
>>> might get called quickly in succession, resulting in the
>>> initialization happening twice. This could cause the system
>>> to boot up with a blank screen.
>>>
>>> This patch adds a simple mutex to serialize it and seems to
>>> avoid the race.
>>>
>>> Suggestions or feedback would be greatly appreciated!
>>>
>>> Cc: Xinliang Liu 
>>> Cc: Rongrong Zou 
>>> Cc: Xinwei Kong 
>>> Cc: Chen Feng 
>>> Cc: David Airlie 
>>> Cc: Daniel Vetter 
>>> Cc: Sean Paul 
>>> Cc: dri-de...@lists.freedesktop.org
>>> Signed-off-by: John Stultz 
>>> ---
>>>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 
>>>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 1 +
>>>   2 files changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> index 7ec93ae..b83556a 100644
>>> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> @@ -55,6 +55,7 @@ static void kirin_fbdev_output_poll_changed(struct
>>> drm_device *dev)
>>>   {
>>>   struct kirin_drm_private *priv = dev->dev_private;
>>>   +mutex_lock(>fb_lock);
>>>   if (priv->fbdev) {
>>>   drm_fbdev_cma_hotplug_event(priv->fbdev);
>>>   } else {
>>> @@ -63,6 +64,7 @@ static void kirin_fbdev_output_poll_changed(struct
>>> drm_device *dev)
>>>   if (IS_ERR(priv->fbdev))
>>>   priv->fbdev = NULL;
>>>   }
>>> +mutex_unlock(>fb_lock);
>>>   }
>>>   #endif
>>>   @@ -95,6 +97,8 @@ static int kirin_drm_kms_init(struct drm_device *dev)
>>>   if (!priv)
>>>   return -ENOMEM;
>>>   +mutex_init(>fb_lock);
>
> And put this line in CONFIG_DRM_FBDEV_EMULATION


Ok. Done! Thanks again for the review and feedback!
-john

Re: [RFC][PATCH v2] drm: kirin: Add a mutex to avoid fb initialization race

[John Stultz]

On Fri, Feb 24, 2017 at 5:45 PM, liuxinliang
 wrote:
>
>
> On 2017/2/25 9:39, liuxinliang wrote:
>>
>> Hi John,
>>
>> The patch seems good to me, except one minus comment.
>> Maybe change fb_lock to fbdev_lock would be better.
>>
>> Thanks,
>> -xinliang
>>
>> On 2017/2/25 9:25, John Stultz wrote:
>>>
>>> In some cases I've been seeing a race where two framebuffers
>>> would be initialized, as kirin_fbdev_output_poll_changed()
>>> might get called quickly in succession, resulting in the
>>> initialization happening twice. This could cause the system
>>> to boot up with a blank screen.
>>>
>>> This patch adds a simple mutex to serialize it and seems to
>>> avoid the race.
>>>
>>> Suggestions or feedback would be greatly appreciated!
>>>
>>> Cc: Xinliang Liu 
>>> Cc: Rongrong Zou 
>>> Cc: Xinwei Kong 
>>> Cc: Chen Feng 
>>> Cc: David Airlie 
>>> Cc: Daniel Vetter 
>>> Cc: Sean Paul 
>>> Cc: dri-de...@lists.freedesktop.org
>>> Signed-off-by: John Stultz 
>>> ---
>>>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 
>>>   drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 1 +
>>>   2 files changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> index 7ec93ae..b83556a 100644
>>> --- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> +++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
>>> @@ -55,6 +55,7 @@ static void kirin_fbdev_output_poll_changed(struct
>>> drm_device *dev)
>>>   {
>>>   struct kirin_drm_private *priv = dev->dev_private;
>>>   +mutex_lock(>fb_lock);
>>>   if (priv->fbdev) {
>>>   drm_fbdev_cma_hotplug_event(priv->fbdev);
>>>   } else {
>>> @@ -63,6 +64,7 @@ static void kirin_fbdev_output_poll_changed(struct
>>> drm_device *dev)
>>>   if (IS_ERR(priv->fbdev))
>>>   priv->fbdev = NULL;
>>>   }
>>> +mutex_unlock(>fb_lock);
>>>   }
>>>   #endif
>>>   @@ -95,6 +97,8 @@ static int kirin_drm_kms_init(struct drm_device *dev)
>>>   if (!priv)
>>>   return -ENOMEM;
>>>   +mutex_init(>fb_lock);
>
> And put this line in CONFIG_DRM_FBDEV_EMULATION


Ok. Done! Thanks again for the review and feedback!
-john

Re: [PATCH v2] arm64: print a fault message when attempting to write RO memory

[Stephen Boyd]

Quoting James Morse (2017-02-20 03:10:10)
> Hi Stephen,
> 
> On 17/02/17 15:53, Stephen Boyd wrote:
> > Quoting James Morse (2017-02-17 03:00:39)
> >> On 17/02/17 01:19, Stephen Boyd wrote:
> >>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> >>> index 156169c6981b..8bd4e7f11c70 100644
> >>> --- a/arch/arm64/mm/fault.c
> >>> +++ b/arch/arm64/mm/fault.c
> >>> @@ -177,9 +193,19 @@ static void __do_kernel_fault(struct mm_struct *mm, 
> >>> unsigned long addr,
> >>>* No handler, we'll have to terminate things with extreme 
> >>> prejudice.
> >>>*/
> >>>   bust_spinlocks(1);
> >>> - pr_alert("Unable to handle kernel %s at virtual address %08lx\n",
> >>> -  (addr < PAGE_SIZE) ? "NULL pointer dereference" :
> >>> -  "paging request", addr);
> >>> +
> >>> + if (is_permission_fault(esr, regs)) {
> >>
> >> is_permission_fault() was previously guarded with a 'addr >> this
> >> is because it assumes software-PAN is relevant.
> >>
> >> The corner case is when the kernel accesses TTBR1-mapped memory while
> >> software-PAN happens to have swivelled TTBR0. Translation faults will be 
> >> matched
> >> by is_permission_fault(), but permission faults won't.
> > 
> > If I understand correctly, and I most definitely don't because there are
> > quite a few combinations, you're saying that __do_kernel_fault() could
> > be called if the kernel attempts to access some userspace address with
> > software PAN? That won't be caught in do_page_fault() with the previous
> > is_permission_fault() check?
> 
> You're right the user-address side of things will get caught in 
> do_page_fault().
> I was trying to badly-explain 'is_permission_fault(esr)' isn't as general
> purpose as its name and prototype suggest, it only gives the results that the
> PAN checks expect when called with a user address.

Ok. I'd rather not change the function in this patch because I'm only
moving the code around to use it higher up in the file. But if you
prefer I can combine the code movement with the addition of a new 'addr'
argument to this function and rework things based on that.

Re: [PATCH v2] arm64: print a fault message when attempting to write RO memory

[Stephen Boyd]

Quoting James Morse (2017-02-20 03:10:10)
> Hi Stephen,
> 
> On 17/02/17 15:53, Stephen Boyd wrote:
> > Quoting James Morse (2017-02-17 03:00:39)
> >> On 17/02/17 01:19, Stephen Boyd wrote:
> >>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> >>> index 156169c6981b..8bd4e7f11c70 100644
> >>> --- a/arch/arm64/mm/fault.c
> >>> +++ b/arch/arm64/mm/fault.c
> >>> @@ -177,9 +193,19 @@ static void __do_kernel_fault(struct mm_struct *mm, 
> >>> unsigned long addr,
> >>>* No handler, we'll have to terminate things with extreme 
> >>> prejudice.
> >>>*/
> >>>   bust_spinlocks(1);
> >>> - pr_alert("Unable to handle kernel %s at virtual address %08lx\n",
> >>> -  (addr < PAGE_SIZE) ? "NULL pointer dereference" :
> >>> -  "paging request", addr);
> >>> +
> >>> + if (is_permission_fault(esr, regs)) {
> >>
> >> is_permission_fault() was previously guarded with a 'addr >> this
> >> is because it assumes software-PAN is relevant.
> >>
> >> The corner case is when the kernel accesses TTBR1-mapped memory while
> >> software-PAN happens to have swivelled TTBR0. Translation faults will be 
> >> matched
> >> by is_permission_fault(), but permission faults won't.
> > 
> > If I understand correctly, and I most definitely don't because there are
> > quite a few combinations, you're saying that __do_kernel_fault() could
> > be called if the kernel attempts to access some userspace address with
> > software PAN? That won't be caught in do_page_fault() with the previous
> > is_permission_fault() check?
> 
> You're right the user-address side of things will get caught in 
> do_page_fault().
> I was trying to badly-explain 'is_permission_fault(esr)' isn't as general
> purpose as its name and prototype suggest, it only gives the results that the
> PAN checks expect when called with a user address.

Ok. I'd rather not change the function in this patch because I'm only
moving the code around to use it higher up in the file. But if you
prefer I can combine the code movement with the addition of a new 'addr'
argument to this function and rework things based on that.

Re: [RFC][PATCH v2] drm: kirin: Add a mutex to avoid fb initialization race

[liuxinliang]

On 2017/2/25 9:39, liuxinliang wrote:

Hi John,

The patch seems good to me, except one minus comment.
Maybe change fb_lock to fbdev_lock would be better.

Thanks,
-xinliang

On 2017/2/25 9:25, John Stultz wrote:

In some cases I've been seeing a race where two framebuffers
would be initialized, as kirin_fbdev_output_poll_changed()
might get called quickly in succession, resulting in the
initialization happening twice. This could cause the system
to boot up with a blank screen.

This patch adds a simple mutex to serialize it and seems to
avoid the race.

Suggestions or feedback would be greatly appreciated!

Cc: Xinliang Liu 
Cc: Rongrong Zou 
Cc: Xinwei Kong 
Cc: Chen Feng 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sean Paul 
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: John Stultz 
---
  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 
  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 1 +
  2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c 
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c

index 7ec93ae..b83556a 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -55,6 +55,7 @@ static void kirin_fbdev_output_poll_changed(struct 
drm_device *dev)

  {
  struct kirin_drm_private *priv = dev->dev_private;
  +mutex_lock(>fb_lock);
  if (priv->fbdev) {
  drm_fbdev_cma_hotplug_event(priv->fbdev);
  } else {
@@ -63,6 +64,7 @@ static void kirin_fbdev_output_poll_changed(struct 
drm_device *dev)

  if (IS_ERR(priv->fbdev))
  priv->fbdev = NULL;
  }
+mutex_unlock(>fb_lock);
  }
  #endif
  @@ -95,6 +97,8 @@ static int kirin_drm_kms_init(struct drm_device 
*dev)

  if (!priv)
  return -ENOMEM;
  +mutex_init(>fb_lock);

And put this line in CONFIG_DRM_FBDEV_EMULATION

+
  dev->dev_private = priv;
  dev_set_drvdata(dev->dev, dev);
  diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h 
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h

index 7f60c649..9b6d2b1 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
@@ -23,6 +23,7 @@ struct kirin_drm_private {
  #ifdef CONFIG_DRM_FBDEV_EMULATION
  struct drm_fbdev_cma *fbdev;
  #endif
+struct mutex fb_lock;

And here.

-xinliang


  };
extern const struct kirin_dc_ops ade_dc_ops;

Re: [RFC][PATCH v2] drm: kirin: Add a mutex to avoid fb initialization race

[liuxinliang]

On 2017/2/25 9:39, liuxinliang wrote:

Hi John,

The patch seems good to me, except one minus comment.
Maybe change fb_lock to fbdev_lock would be better.

Thanks,
-xinliang

On 2017/2/25 9:25, John Stultz wrote:

In some cases I've been seeing a race where two framebuffers
would be initialized, as kirin_fbdev_output_poll_changed()
might get called quickly in succession, resulting in the
initialization happening twice. This could cause the system
to boot up with a blank screen.

This patch adds a simple mutex to serialize it and seems to
avoid the race.

Suggestions or feedback would be greatly appreciated!

Cc: Xinliang Liu 
Cc: Rongrong Zou 
Cc: Xinwei Kong 
Cc: Chen Feng 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: Sean Paul 
Cc: dri-de...@lists.freedesktop.org
Signed-off-by: John Stultz 
---
  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 4 
  drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h | 1 +
  2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c 
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c

index 7ec93ae..b83556a 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c
@@ -55,6 +55,7 @@ static void kirin_fbdev_output_poll_changed(struct 
drm_device *dev)

  {
  struct kirin_drm_private *priv = dev->dev_private;
  +mutex_lock(>fb_lock);
  if (priv->fbdev) {
  drm_fbdev_cma_hotplug_event(priv->fbdev);
  } else {
@@ -63,6 +64,7 @@ static void kirin_fbdev_output_poll_changed(struct 
drm_device *dev)

  if (IS_ERR(priv->fbdev))
  priv->fbdev = NULL;
  }
+mutex_unlock(>fb_lock);
  }
  #endif
  @@ -95,6 +97,8 @@ static int kirin_drm_kms_init(struct drm_device 
*dev)

  if (!priv)
  return -ENOMEM;
  +mutex_init(>fb_lock);

And put this line in CONFIG_DRM_FBDEV_EMULATION

+
  dev->dev_private = priv;
  dev_set_drvdata(dev->dev, dev);
  diff --git a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h 
b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h

index 7f60c649..9b6d2b1 100644
--- a/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
+++ b/drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.h
@@ -23,6 +23,7 @@ struct kirin_drm_private {
  #ifdef CONFIG_DRM_FBDEV_EMULATION
  struct drm_fbdev_cma *fbdev;
  #endif
+struct mutex fb_lock;

And here.

-xinliang


  };
extern const struct kirin_dc_ops ade_dc_ops;