Re: [PATCH v6 5/7] ASoC: fsl_asrc: Move common definition to fsl_asrc_common
Hi
On Tue, Apr 7, 2020 at 7:50 AM Nicolin Chen wrote:
>
> On Wed, Apr 01, 2020 at 04:45:38PM +0800, Shengjiu Wang wrote:
>
> > static int fsl_asrc_probe(struct platform_device *pdev)
> > {
> > struct device_node *np = pdev->dev.of_node;
> > struct fsl_asrc *asrc;
> > + struct fsl_asrc_priv *asrc_priv;
>
> Could we move it before "struct fsl_asrc *asrc;"?
>
> > diff --git a/sound/soc/fsl/fsl_asrc_common.h
> > b/sound/soc/fsl/fsl_asrc_common.h
> > new file mode 100644
> > index ..5c93ccdfc30a
> > --- /dev/null
> > +++ b/sound/soc/fsl/fsl_asrc_common.h
>
> > +#define PAIR_CTX_NUM 0x4
> > +#define PAIR_PRIVAT_SIZE 0x400
>
> "PRIVAT_" => "PRIVATE_"
>
> > +/**
> > + * fsl_asrc_pair: ASRC common data
>
> "fsl_asrc_pair" => "fsl_asrc"
>
> > + */
> > +struct fsl_asrc {
>
> > diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c
> > index b15946e03380..5cf0468ce6e3 100644
> > --- a/sound/soc/fsl/fsl_asrc_dma.c
> > +++ b/sound/soc/fsl/fsl_asrc_dma.c
>
> > @@ -311,11 +311,12 @@ static int fsl_asrc_dma_startup(struct
> > snd_soc_component *component,
> > return ret;
> > }
> >
> > - pair = kzalloc(sizeof(struct fsl_asrc_pair), GFP_KERNEL);
> > + pair = kzalloc(sizeof(struct fsl_asrc_pair) + PAIR_PRIVAT_SIZE,
> > GFP_KERNEL);
>
> If we only use the PAIR_PRIVATE_SIZE here, maybe we can put the
> define in this file too, rather than in the header file.
>
> And could fit 80 characters:
>
> + pair = kzalloc(sizeof(*pair) + PAIR_PRIVAT_SIZE, GFP_KERNEL);
I will use a function pointer
int (*get_pair_priv_size)(void)
to avoid definition of PAIR_PRIVATE_SIZE. which is not safe.
best regards
wang shengjiu
Re: Boot flakiness with QEMU 3.1.0 and Clang built kernels
Hi Nicholas,
On Sat, Apr 11, 2020 at 10:29:45AM +1000, Nicholas Piggin wrote:
> Nathan Chancellor's on April 11, 2020 6:59 am:
> > Hi all,
> >
> > Recently, our CI started running into several hangs when running the
> > spinlock torture tests during a boot with QEMU 3.1.0 on
> > powernv_defconfig and pseries_defconfig when compiled with Clang.
> >
> > I initially bisected Linux and came down to commit 3282a3da25bd
> > ("powerpc/64: Implement soft interrupt replay in C") [1], which seems to
> > make sense. However, I realized I could not reproduce this in my local
> > environment no matter how hard I tried, only in our Docker image. I then
> > realized my environment's QEMU version was 4.2.0; I compiled 3.1.0 and
> > was able to reproduce it then.
> >
> > I bisected QEMU down to two commits: powernv_defconfig was fixed by [2]
> > and pseries_defconfig was fixed by [3].
>
> Looks like it might have previously been testing power8, now power9?
> -cpu power8 might get it reproducing again.
Yes, that is what it looks like. I can reproduce the hang with both
pseries-3.1 and powernv8 on QEMU 4.2.0.
> > I ran 100 boots with our boot-qemu.sh script [4] and QEMU 3.1.0 failed
> > approximately 80% of the time but 4.2.0 and 5.0.0-rc1 only failed 1% of
> > the time [5]. GCC 9.3.0 built kernels failed approximately 3% of time
> > [6].
>
> Do they fail in the same way? Was the fail rate at 0% before upgrading
> kernels?
Yes, it just hangs after I see the print out that the torture tests are
running.
[2.277125] spin_lock-torture: Creating torture_shuffle task
[2.279058] spin_lock-torture: Creating torture_stutter task
[2.280285] spin_lock-torture: torture_shuffle task started
[2.281326] spin_lock-torture: Creating lock_torture_writer task
[2.282509] spin_lock-torture: torture_stutter task started
[2.283511] spin_lock-torture: Creating lock_torture_writer task
[2.285155] spin_lock-torture: lock_torture_writer task started
[2.286586] spin_lock-torture: Creating lock_torture_stats task
[2.287772] spin_lock-torture: lock_torture_writer task started
[2.290578] spin_lock-torture: lock_torture_stats task started
Yes, we never had any failures in our CI before that upgrade happened. I
will try to run a set of boot tests with a kernel built at the commit
right before 3282a3da25bd and at 3282a3da25bd to make triple sure I did
fall on the right commit.
> > Without access to real hardware, I cannot really say if there is a
> > problem here. We are going to upgrade to QEMU 4.2.0 to fix it. This is
> > more of an FYI so that there is some record of it outside of our issue
> > tracker and so people can be aware of it in case it comes up somewhere
> > else.
>
> Thanks for this I'll try to reproduce. You're not running SMP guest?
No, not as far as I am aware at least. You can see our QEMU line in our
CI and the boot-qemu.sh script I have listed below:
https://travis-ci.com/github/ClangBuiltLinux/continuous-integration/jobs/318260635
> Anything particular to run the lock torture test? This is just
> powernv_defconfig + CONFIG_LOCK_TORTURE_TEST=y ?
We do enable some other configs, you can see those here:
https://github.com/ClangBuiltLinux/continuous-integration/blob/c02d2f008a64d44e62518bc03beb1126db7619ce/configs/common.config
https://github.com/ClangBuiltLinux/continuous-integration/blob/c02d2f008a64d44e62518bc03beb1126db7619ce/configs/tt.config
The tt.config values are needed to reproduce but I did not verify that
ONLY tt.config was needed. Other than that, no, we are just building
either pseries_defconfig or powernv_defconfig with those configs and
letting it boot up with a simple initramfs, which prints the version
string then shuts the machine down.
Let me know if you need any more information, cheers!
Nathan
> Thanks,
> Nick
>
> >
> > [1]: https://git.kernel.org/linus/3282a3da25bd63fdb7240bc35dbdefa4b1947005
> > [2]:
> > https://git.qemu.org/?p=qemu.git;a=commit;h=f30c843ced5055fde71d28d10beb15af97fdfe39
> > [3]:
> > https://git.qemu.org/?p=qemu.git;a=commit;h=34a6b015a98733a4b32881777dafd70156c5a322.
> > [4]:
> > https://github.com/ClangBuiltLinux/boot-utils/blob/5f49a87e272fbe967a8d26cf405cec15b024702c/boot-qemu.sh
> > [5]:
> > https://user-images.githubusercontent.com/11478138/78957618-b1842080-7a9a-11ea-8856-279c3dcc6c19.png
> > [6]:
> > https://user-images.githubusercontent.com/11478138/78955535-62d38800-7a94-11ea-9e61-9e3d8c068ace.png
> >
> > Cheers,
> > Nathan
> >
Re: Boot flakiness with QEMU 3.1.0 and Clang built kernels
Nathan Chancellor's on April 11, 2020 6:59 am:
> Hi all,
>
> Recently, our CI started running into several hangs when running the
> spinlock torture tests during a boot with QEMU 3.1.0 on
> powernv_defconfig and pseries_defconfig when compiled with Clang.
>
> I initially bisected Linux and came down to commit 3282a3da25bd
> ("powerpc/64: Implement soft interrupt replay in C") [1], which seems to
> make sense. However, I realized I could not reproduce this in my local
> environment no matter how hard I tried, only in our Docker image. I then
> realized my environment's QEMU version was 4.2.0; I compiled 3.1.0 and
> was able to reproduce it then.
>
> I bisected QEMU down to two commits: powernv_defconfig was fixed by [2]
> and pseries_defconfig was fixed by [3].
Looks like it might have previously been testing power8, now power9?
-cpu power8 might get it reproducing again.
> I ran 100 boots with our boot-qemu.sh script [4] and QEMU 3.1.0 failed
> approximately 80% of the time but 4.2.0 and 5.0.0-rc1 only failed 1% of
> the time [5]. GCC 9.3.0 built kernels failed approximately 3% of time
> [6].
Do they fail in the same way? Was the fail rate at 0% before upgrading
kernels?
>
> Without access to real hardware, I cannot really say if there is a
> problem here. We are going to upgrade to QEMU 4.2.0 to fix it. This is
> more of an FYI so that there is some record of it outside of our issue
> tracker and so people can be aware of it in case it comes up somewhere
> else.
Thanks for this I'll try to reproduce. You're not running SMP guest?
Anything particular to run the lock torture test? This is just
powernv_defconfig + CONFIG_LOCK_TORTURE_TEST=y ?
Thanks,
Nick
>
> [1]: https://git.kernel.org/linus/3282a3da25bd63fdb7240bc35dbdefa4b1947005
> [2]:
> https://git.qemu.org/?p=qemu.git;a=commit;h=f30c843ced5055fde71d28d10beb15af97fdfe39
> [3]:
> https://git.qemu.org/?p=qemu.git;a=commit;h=34a6b015a98733a4b32881777dafd70156c5a322.
> [4]:
> https://github.com/ClangBuiltLinux/boot-utils/blob/5f49a87e272fbe967a8d26cf405cec15b024702c/boot-qemu.sh
> [5]:
> https://user-images.githubusercontent.com/11478138/78957618-b1842080-7a9a-11ea-8856-279c3dcc6c19.png
> [6]:
> https://user-images.githubusercontent.com/11478138/78955535-62d38800-7a94-11ea-9e61-9e3d8c068ace.png
>
> Cheers,
> Nathan
>
RE: [PATCH 01/28] x86/hyperv: use vmalloc_exec for the hypercall page
From: Christoph Hellwig Sent: Wednesday, April 8, 2020 4:59 AM > > Use the designated helper for allocating executable kernel memory, and > remove the now unused PAGE_KERNEL_RX define. > > Signed-off-by: Christoph Hellwig > --- > arch/x86/hyperv/hv_init.c| 2 +- > arch/x86/include/asm/pgtable_types.h | 2 -- > 2 files changed, 1 insertion(+), 3 deletions(-) > Reviewed-by: Michael Kelley
RE: [PATCH 20/28] mm: remove the pgprot argument to __vmalloc
From: Christoph Hellwig Sent: Wednesday, April 8, 2020 4:59 AM > > The pgprot argument to __vmalloc is always PROT_KERNEL now, so remove > it. > > Signed-off-by: Christoph Hellwig > --- > arch/x86/hyperv/hv_init.c | 3 +-- > arch/x86/include/asm/kvm_host.h| 3 +-- > arch/x86/kvm/svm.c | 3 +-- > drivers/block/drbd/drbd_bitmap.c | 4 +--- > drivers/gpu/drm/etnaviv/etnaviv_dump.c | 4 ++-- > drivers/lightnvm/pblk-init.c | 5 ++--- > drivers/md/dm-bufio.c | 4 ++-- > drivers/mtd/ubi/io.c | 4 ++-- > drivers/scsi/sd_zbc.c | 3 +-- > fs/gfs2/dir.c | 9 - > fs/gfs2/quota.c| 2 +- > fs/nfs/blocklayout/extent_tree.c | 2 +- > fs/ntfs/malloc.h | 2 +- > fs/ubifs/debug.c | 2 +- > fs/ubifs/lprops.c | 2 +- > fs/ubifs/lpt_commit.c | 4 ++-- > fs/ubifs/orphan.c | 2 +- > fs/xfs/kmem.c | 2 +- > include/linux/vmalloc.h| 2 +- > kernel/bpf/core.c | 6 +++--- > kernel/groups.c| 2 +- > kernel/module.c| 3 +-- > mm/nommu.c | 15 +++ > mm/page_alloc.c| 2 +- > mm/percpu.c| 2 +- > mm/vmalloc.c | 4 ++-- > net/bridge/netfilter/ebtables.c| 6 ++ > sound/core/memalloc.c | 2 +- > sound/core/pcm_memory.c| 2 +- > 29 files changed, 47 insertions(+), 59 deletions(-) > For Hyper-V changes, Reviewed-by: Michael Kelley
Re: [PATCH 10/28] mm: only allow page table mappings for built-in zsmalloc
Hi Sergey, On Fri, Apr 10, 2020 at 11:38:45AM +0900, Sergey Senozhatsky wrote: > On (20/04/09 10:08), Minchan Kim wrote: > > > > Even though I don't know how many usecase we have using zsmalloc as > > > > module(I heard only once by dumb reason), it could affect existing > > > > users. Thus, please include concrete explanation in the patch to > > > > justify when the complain occurs. > > > > > > The justification is 'we can unexport functions that have no sane reason > > > of being exported in the first place'. > > > > > > The Changelog pretty much says that. > > > > Okay, I hope there is no affected user since this patch. > > If there are someone, they need to provide sane reason why they want > > to have zsmalloc as module. > > I'm one of those who use zsmalloc as a module - mainly because I use zram > as a compressing general purpose block device, not as a swap device. > I create zram0, mkfs, mount, checkout and compile code, once done - > umount, rmmod. This reduces the number of writes to SSD. Some people use > tmpfs, but zram device(-s) can be much larger in size. That's a niche use > case and I'm not against the patch. It doesn't mean we couldn't use zsmalloc as module any longer. It means we couldn't use zsmalloc as module with pgtable mapping whcih was little bit faster on microbenchmark in some architecutre(However, I usually temped to remove it since it had several problems). However, we could still use zsmalloc as module as copy way instead of pgtable mapping. Thus, if someone really want to rollback the feature, they should provide reasonable reason why it doesn't work for them. "A little fast" wouldn't be enough to exports deep internal to the module. Thanks.
Boot flakiness with QEMU 3.1.0 and Clang built kernels
Hi all,
Recently, our CI started running into several hangs when running the
spinlock torture tests during a boot with QEMU 3.1.0 on
powernv_defconfig and pseries_defconfig when compiled with Clang.
I initially bisected Linux and came down to commit 3282a3da25bd
("powerpc/64: Implement soft interrupt replay in C") [1], which seems to
make sense. However, I realized I could not reproduce this in my local
environment no matter how hard I tried, only in our Docker image. I then
realized my environment's QEMU version was 4.2.0; I compiled 3.1.0 and
was able to reproduce it then.
I bisected QEMU down to two commits: powernv_defconfig was fixed by [2]
and pseries_defconfig was fixed by [3].
I ran 100 boots with our boot-qemu.sh script [4] and QEMU 3.1.0 failed
approximately 80% of the time but 4.2.0 and 5.0.0-rc1 only failed 1% of
the time [5]. GCC 9.3.0 built kernels failed approximately 3% of time
[6].
Without access to real hardware, I cannot really say if there is a
problem here. We are going to upgrade to QEMU 4.2.0 to fix it. This is
more of an FYI so that there is some record of it outside of our issue
tracker and so people can be aware of it in case it comes up somewhere
else.
[1]: https://git.kernel.org/linus/3282a3da25bd63fdb7240bc35dbdefa4b1947005
[2]:
https://git.qemu.org/?p=qemu.git;a=commit;h=f30c843ced5055fde71d28d10beb15af97fdfe39
[3]:
https://git.qemu.org/?p=qemu.git;a=commit;h=34a6b015a98733a4b32881777dafd70156c5a322.
[4]:
https://github.com/ClangBuiltLinux/boot-utils/blob/5f49a87e272fbe967a8d26cf405cec15b024702c/boot-qemu.sh
[5]:
https://user-images.githubusercontent.com/11478138/78957618-b1842080-7a9a-11ea-8856-279c3dcc6c19.png
[6]:
https://user-images.githubusercontent.com/11478138/78955535-62d38800-7a94-11ea-9e61-9e3d8c068ace.png
Cheers,
Nathan
Section mismatch in reference from the function .early_init_mmu() to the function .init.text:.radix__early_init_mmu() after PowerPC updates 5.7-1
Hello, The issue still exists after the new PowerPC updates 5.7-2. Please check the PowerPC updates 5.7-1. Thanks, Christian On 08 April 2020 at 6:32 pm, Christian Zigotzky wrote: Hello, Since the PowerPC updates 5.7-1 we have the following issue during the linking of vmlinux: MODPOST vmlinux.o WARNING: modpost: vmlinux.o(.text.unlikely+0x1a0): Section mismatch in reference from the function .early_init_mmu() to the function .init.text:.radix__early_init_mmu() The function .early_init_mmu() references the function __init .radix__early_init_mmu(). This is often because .early_init_mmu lacks a __init annotation or the annotation of .radix__early_init_mmu is wrong. WARNING: modpost: vmlinux.o(.text.unlikely+0x1ac): Section mismatch in reference from the function .early_init_mmu() to the function .init.text:.hash__early_init_mmu() The function .early_init_mmu() references the function __init .hash__early_init_mmu(). This is often because .early_init_mmu lacks a __init annotation or the annotation of .hash__early_init_mmu is wrong. --- But the kernel works without any problems after the linking. I reverted the following commits: 70fbdfef4ba63eeef83b2c94eac9a5a9f913e442 -- sysfs: remove redundant __compat_only_sysfs_link_entry_to_kobj fn d38c07afc356ddebaa3ed8ecb3f553340e05c969 -- Merge tag 'powerpc-5.7-1' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux And after that the linking of vmlinux works again. Please check the PowerPC updates 5.7-1. Thanks, Christian
Re: [PATCH v2 4/4] hugetlbfs: clean up command line processing
On Wed, Apr 01, 2020 at 11:38:19AM -0700, Mike Kravetz wrote: > With all hugetlb page processing done in a single file clean up code. > - Make code match desired semantics > - Update documentation with semantics > - Make all warnings and errors messages start with 'HugeTLB:'. > - Consistently name command line parsing routines. > - Check for hugepages_supported() before processing parameters. > - Add comments to code > - Describe some of the subtle interactions > - Describe semantics of command line arguments > > Signed-off-by: Mike Kravetz > --- > .../admin-guide/kernel-parameters.txt | 35 --- > Documentation/admin-guide/mm/hugetlbpage.rst | 44 + > mm/hugetlb.c | 96 +++ > 3 files changed, 142 insertions(+), 33 deletions(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > index 1bd5454b5e5f..de653cfe1726 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -832,12 +832,15 @@ > See also Documentation/networking/decnet.txt. > > default_hugepagesz= > - [same as hugepagesz=] The size of the default > - HugeTLB page size. This is the size represented by > - the legacy /proc/ hugepages APIs, used for SHM, and > - default size when mounting hugetlbfs filesystems. > - Defaults to the default architecture's huge page size > - if not specified. > + [HW] The size of the default HugeTLB page size. This Could I ask what's "HW"? Sorry this is not a comment at all but really a pure question I wanted to ask... :) > + is the size represented by the legacy /proc/ hugepages > + APIs. In addition, this is the default hugetlb size > + used for shmget(), mmap() and mounting hugetlbfs > + filesystems. If not specified, defaults to the > + architecture's default huge page size. Huge page > + sizes are architecture dependent. See also > + Documentation/admin-guide/mm/hugetlbpage.rst. > + Format: size[KMG] > > deferred_probe_timeout= > [KNL] Debugging option to set a timeout in seconds for > @@ -1480,13 +1483,19 @@ > If enabled, boot-time allocation of gigantic hugepages > is skipped. > > - hugepages= [HW,X86-32,IA-64] HugeTLB pages to allocate at boot. > - hugepagesz= [HW,IA-64,PPC,X86-64] The size of the HugeTLB pages. > - On x86-64 and powerpc, this option can be specified > - multiple times interleaved with hugepages= to reserve > - huge pages of different sizes. Valid pages sizes on > - x86-64 are 2M (when the CPU supports "pse") and 1G > - (when the CPU supports the "pdpe1gb" cpuinfo flag). > + hugepages= [HW] Number of HugeTLB pages to allocate at boot. > + If this follows hugepagesz (below), it specifies > + the number of pages of hugepagesz to be allocated. "... Otherwise it specifies the number of pages to allocate for the default huge page size." ? > + Format: How about add a new line here? > + hugepagesz= > + [HW] The size of the HugeTLB pages. This is used in > + conjunction with hugepages (above) to allocate huge > + pages of a specific size at boot. The pair > + hugepagesz=X hugepages=Y can be specified once for > + each supported huge page size. Huge page sizes are > + architecture dependent. See also > + Documentation/admin-guide/mm/hugetlbpage.rst. > + Format: size[KMG] > > hung_task_panic= > [KNL] Should the hung task detector generate panics. > diff --git a/Documentation/admin-guide/mm/hugetlbpage.rst > b/Documentation/admin-guide/mm/hugetlbpage.rst > index 1cc0bc78d10e..de340c586995 100644 > --- a/Documentation/admin-guide/mm/hugetlbpage.rst > +++ b/Documentation/admin-guide/mm/hugetlbpage.rst > @@ -100,6 +100,50 @@ with a huge page size selection parameter > "hugepagesz=". must > be specified in bytes with optional scale suffix [kKmMgG]. The default huge > page size may be selected with the "default_hugepagesz=" boot > parameter. > > +Hugetlb boot command line parameter semantics > +hugepagesz - Specify a huge page size. Used in conjunction with hugepages > + parameter to preallocate a number of huge pages of the specified > + size. Hence, hugepagesz and hugepages are typically specif
[PATCH] powerpc/ima: fix secure boot rules in ima arch policy
To prevent verifying the kernel module appended signature twice
(finit_module), once by the module_sig_check() and again by IMA, powerpc
IMA secure boot rules define an IMA architecture specific policy rule
only if CONFIG_MODULE_SIG_FORCE is not enabled. This, unfortunately, does
not take into account the ability of enabling "sig_enforce" on the boot
command line (module.sig_enforce=1).
This patch fixes secure boot policy rules to be based on CONFIG_MODULE_SIG
instead.
Fixes: 4238fad366a6 ("powerpc/ima: Add support to initialize ima policy rules")
Signed-off-by: Nayna Jain
---
arch/powerpc/kernel/ima_arch.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/ima_arch.c b/arch/powerpc/kernel/ima_arch.c
index e34116255ced..957abd592075 100644
--- a/arch/powerpc/kernel/ima_arch.c
+++ b/arch/powerpc/kernel/ima_arch.c
@@ -19,12 +19,12 @@ bool arch_ima_get_secureboot(void)
* to be stored as an xattr or as an appended signature.
*
* To avoid duplicate signature verification as much as possible, the IMA
- * policy rule for module appraisal is added only if CONFIG_MODULE_SIG_FORCE
+ * policy rule for module appraisal is added only if CONFIG_MODULE_SIG
* is not enabled.
*/
static const char *const secure_rules[] = {
"appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist
appraise_type=imasig|modsig",
-#ifndef CONFIG_MODULE_SIG_FORCE
+#ifndef CONFIG_MODULE_SIG
"appraise func=MODULE_CHECK appraise_flag=check_blacklist
appraise_type=imasig|modsig",
#endif
NULL
@@ -50,7 +50,7 @@ static const char *const secure_and_trusted_rules[] = {
"measure func=KEXEC_KERNEL_CHECK template=ima-modsig",
"measure func=MODULE_CHECK template=ima-modsig",
"appraise func=KEXEC_KERNEL_CHECK appraise_flag=check_blacklist
appraise_type=imasig|modsig",
-#ifndef CONFIG_MODULE_SIG_FORCE
+#ifndef CONFIG_MODULE_SIG
"appraise func=MODULE_CHECK appraise_flag=check_blacklist
appraise_type=imasig|modsig",
#endif
NULL
--
2.25.1
Re: [PATCH v2 3/4] hugetlbfs: remove hugetlb_add_hstate() warning for existing hstate
On Wed, Apr 01, 2020 at 11:38:18AM -0700, Mike Kravetz wrote:
[...]
> @@ -3255,7 +3254,6 @@ void __init hugetlb_add_hstate(unsigned int order)
> unsigned long i;
>
> if (size_to_hstate(PAGE_SIZE << order)) {
> - pr_warn("hugepagesz= specified twice, ignoring\n");
> return;
> }
Nitpick: I think the brackets need to be removed to follow linux
coding style. With that:
Reviewed-by: Peter Xu
--
Peter Xu
Re: [PATCH 1/1] powerpc/rtas: Implement reentrant rtas call
Leonardo Bras writes:
> Implement rtas_call_reentrant() for reentrant rtas-calls:
> "ibm,int-on", "ibm,int-off",ibm,get-xive" and "ibm,set-xive".
>
> On LoPAPR Version 1.1 (March 24, 2016), from 7.3.10.1 to 7.3.10.4,
> items 2 and 3 say:
>
> 2 - For the PowerPC External Interrupt option: The * call must be
> reentrant to the number of processors on the platform.
> 3 - For the PowerPC External Interrupt option: The * argument call
> buffer for each simultaneous call must be physically unique.
>
> So, these rtas-calls can be called in a lockless way, if using
> a different buffer for each call.
>From the language in the spec it's clear that these calls are intended
to be reentrant with respect to themselves, but it's less clear to me
that they are safe to call simultaneously with respect to each other or
arbitrary other RTAS methods.
> This can be useful to avoid deadlocks in crashing, where rtas-calls are
> needed, but some other thread crashed holding the rtas.lock.
Are these calls commonly used in the crash-handling path? Is this
addressing a real issue you've seen?
> +/*
> + * Used for reentrant rtas calls.
> + * According to LoPAR documentation, only "ibm,int-on", "ibm,int-off",
> + * "ibm,get-xive" and "ibm,set-xive" are currently reentrant.
> + * Reentrant calls need their own rtas_args buffer, so not using rtas.args.
> + */
Please use kernel-doc format in new code.
> +int rtas_call_reentrant(int token, int nargs, int nret, int *outputs, ...)
> +{
> + va_list list;
> + struct rtas_args rtas_args;
> +
> + if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
> + return -1;
> +
> + va_start(list, outputs);
> + va_rtas_call_unlocked(&rtas_args, token, nargs, nret, list);
> + va_end(list);
No, I don't think you can place the RTAS argument buffer on the stack:
7.2.7, Software Implementation Note:
| The OS must be aware that the effective address range for RTAS is 4
| GB when instantiated in 32-bit mode and the OS should not pass RTAS
| addresses or blocks of data which might fall outside of this range.
Re: [PATCH v2 2/4] hugetlbfs: move hugepagesz= parsing to arch independent code
On Wed, Apr 01, 2020 at 11:38:17AM -0700, Mike Kravetz wrote: > Now that architectures provide arch_hugetlb_valid_size(), parsing > of "hugepagesz=" can be done in architecture independent code. > Create a single routine to handle hugepagesz= parsing and remove > all arch specific routines. We can also remove the interface > hugetlb_bad_size() as this is no longer used outside arch independent > code. > > This also provides consistent behavior of hugetlbfs command line > options. The hugepagesz= option should only be specified once for > a specific size, but some architectures allow multiple instances. > This appears to be more of an oversight when code was added by some > architectures to set up ALL huge pages sizes. > > Signed-off-by: Mike Kravetz This could change the error messages for a wrong setup on archs, but I guess it's not a big deal, assuming even to capture error people will majorly still look for error lines in general.. Reviewed-by: Peter Xu -- Peter Xu
Re: Linux-next POWER9 NULL pointer NIP since 1st Apr.
> On Apr 9, 2020, at 10:14 AM, Steven Rostedt wrote: > > On Thu, 9 Apr 2020 06:06:35 -0400 > Qian Cai wrote: > I’ll go to bisect some more but it is going to take a while. $ git log --oneline 4c205c84e249..8e99cf91b99b 8e99cf91b99b tracing: Do not allocate buffer in trace_find_next_entry() in atomic 2ab2a0924b99 tracing: Add documentation on set_ftrace_notrace_pid and set_event_notrace_pid ebed9628f5c2 selftests/ftrace: Add test to test new set_event_notrace_pid file ed8839e072b8 selftests/ftrace: Add test to test new set_ftrace_notrace_pid file 276836260301 tracing: Create set_event_notrace_pid to not trace tasks >>> b3b1e6ededa4 ftrace: Create set_ftrace_notrace_pid to not trace tasks 717e3f5ebc82 ftrace: Make function trace pid filtering a bit more exact >>> >>> If it is affecting function tracing, it is probably one of the above two >>> commits. >> >> OK, it was narrowed down to one of those messed with mcount here, > > Thing is, nothing here touches mcount. Yes, you are right. I went back to test the commit just before the 5.7-trace merge request, I did reproduce there. The thing is that this bastard could take more 6-hour to happen, so my previous attempt did not wait long enough. Back to the square one... > >> >> 8e99cf91b99b tracing: Do not allocate buffer in trace_find_next_entry() in >> atomic > > Touches reading the trace buffer. > >> 2ab2a0924b99 tracing: Add documentation on set_ftrace_notrace_pid and >> set_event_notrace_pid > > Documentation. > >> 6a13a0d7b4d1 ftrace/kprobe: Show the maxactive number on kprobe_events > > kprobe output. > >> c9b7a4a72ff6 ring-buffer/tracing: Have iterator acknowledge dropped events > > Reading the buffer. > >> 06e0a548bad0 tracing: Do not disable tracing when reading the trace file > > Reading the buffer. > >> 1039221cc278 ring-buffer: Do not disable recording when there is an iterator > > Reading the buffer. > >> 07b8b10ec94f ring-buffer: Make resize disable per cpu buffer instead of >> total buffer > > Resizing the buffer. > >> 153368ce1bd0 ring-buffer: Optimize rb_iter_head_event() > > Reading the buffer. > >> ff84c50cfb4b ring-buffer: Do not die if rb_iter_peek() fails more than thrice > > Reading the buffer. > >> 785888c544e0 ring-buffer: Have rb_iter_head_event() handle concurrent writer > > Reading the buffer. > >> 28e3fc56a471 ring-buffer: Add page_stamp to iterator for synchronization > > Reading the buffer. > >> bc1a72afdc4a ring-buffer: Rename ring_buffer_read() to >> read_buffer_iter_advance() > > Reading the buffer. > >> ead6ecfddea5 ring-buffer: Have ring_buffer_empty() not depend on tracing >> stopped > > Reading the buffer. > >> ff895103a84a tracing: Save off entry when peeking at next entry > > Reading the buffer. > >> bf2cbe044da2 tracing: Use address-of operator on section symbols > > Affects trace_printk() > >> bbd9d05618a6 gpu/trace: add a gpu total memory usage tracepoint > > New tracepoint infrastructure (just new trace events for gpu) > >> 89b74cac7834 tools/bootconfig: Show line and column in parse error > > Extended command line boot config. > >> 306b69dce926 bootconfig: Support O= option > > Extended command line boot config > >> 5412e0b763e0 tracing: Remove unused TRACE_BUFFER bits > > Removed unused enums. > >> b396bfdebffc tracing: Have hwlat ts be first instance and record count of >> instances > > Affects only the hard ware latency detector (most likely not even > configured in the kernel). > > So I don't understand how any of the above commits can cause a problem. > > -- Steve
Re: [PATCH v2 1/4] hugetlbfs: add arch_hugetlb_valid_size
On Wed, Apr 01, 2020 at 11:38:16AM -0700, Mike Kravetz wrote:
> diff --git a/arch/arm64/include/asm/hugetlb.h
> b/arch/arm64/include/asm/hugetlb.h
> index 2eb6c234d594..81606223494f 100644
> --- a/arch/arm64/include/asm/hugetlb.h
> +++ b/arch/arm64/include/asm/hugetlb.h
> @@ -59,6 +59,8 @@ extern void huge_pte_clear(struct mm_struct *mm, unsigned
> long addr,
> extern void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr,
>pte_t *ptep, pte_t pte, unsigned long sz);
> #define set_huge_swap_pte_at set_huge_swap_pte_at
> +bool __init arch_hugetlb_valid_size(unsigned long size);
> +#define arch_hugetlb_valid_size arch_hugetlb_valid_size
Sorry for chimming in late.
Since we're working on removing arch-dependent codes after all.. I'm
thinking whether we can define arch_hugetlb_valid_size() once in the
common header (e.g. linux/hugetlb.h), then in mm/hugetlb.c:
bool __init __attribute((weak)) arch_hugetlb_valid_size(unsigned long size)
{
return size == HPAGE_SIZE;
}
We can simply redefine arch_hugetlb_valid_size() in arch specific C
files where we want to override the default. Would that be slightly
cleaner?
Thanks,
--
Peter Xu
Re: [PATCH v2 2/2] powerpc: Remove Xilinx PPC405/PPC440 support
On Mon, Mar 30, 2020 at 03:32:17PM +0200, Michal Simek wrote:
> The latest Xilinx design tools called ISE and EDK has been released in
> October 2013. New tool doesn't support any PPC405/PPC440 new designs.
> These platforms are no longer supported and tested.
>
> PowerPC 405/440 port is orphan from 2013 by
> commit cdeb89943bfc ("MAINTAINERS: Fix incorrect status tag") and
> commit 19624236cce1 ("MAINTAINERS: Update Grant's email address and
> maintainership")
> that's why it is time to remove the support fot these platforms.
>
> Signed-off-by: Michal Simek
> Acked-by: Arnd Bergmann
> ---
>
> Changes in v2:
> - Based on my chat with Arnd I removed arch/powerpc/xmon/ changes done in
> v1 to keep them the same as before. (kbuild reported some issues with it
> too)
>
> Documentation/devicetree/bindings/xilinx.txt | 143 --
Acked-by: Rob Herring
> Documentation/powerpc/bootwrapper.rst| 28 +-
> MAINTAINERS | 6 -
> arch/powerpc/Kconfig.debug | 2 +-
> arch/powerpc/boot/Makefile | 7 +-
> arch/powerpc/boot/dts/Makefile | 1 -
> arch/powerpc/boot/dts/virtex440-ml507.dts| 406
> arch/powerpc/boot/dts/virtex440-ml510.dts| 466 ---
> arch/powerpc/boot/ops.h | 1 -
> arch/powerpc/boot/serial.c | 5 -
> arch/powerpc/boot/uartlite.c | 79
> arch/powerpc/boot/virtex.c | 97
> arch/powerpc/boot/virtex405-head.S | 31 --
> arch/powerpc/boot/wrapper| 8 -
> arch/powerpc/configs/40x/virtex_defconfig| 75 ---
> arch/powerpc/configs/44x/virtex5_defconfig | 74 ---
> arch/powerpc/configs/ppc40x_defconfig| 8 -
> arch/powerpc/configs/ppc44x_defconfig| 8 -
> arch/powerpc/include/asm/xilinx_intc.h | 16 -
> arch/powerpc/include/asm/xilinx_pci.h| 21 -
> arch/powerpc/kernel/cputable.c | 39 --
> arch/powerpc/platforms/40x/Kconfig | 31 --
> arch/powerpc/platforms/40x/Makefile | 1 -
> arch/powerpc/platforms/40x/virtex.c | 54 ---
> arch/powerpc/platforms/44x/Kconfig | 37 --
> arch/powerpc/platforms/44x/Makefile | 2 -
> arch/powerpc/platforms/44x/virtex.c | 60 ---
> arch/powerpc/platforms/44x/virtex_ml510.c| 30 --
> arch/powerpc/platforms/Kconfig | 4 -
> arch/powerpc/sysdev/Makefile | 2 -
> arch/powerpc/sysdev/xilinx_intc.c| 88
> arch/powerpc/sysdev/xilinx_pci.c | 132 --
> drivers/char/Kconfig | 2 +-
> drivers/video/fbdev/Kconfig | 2 +-
> 34 files changed, 7 insertions(+), 1959 deletions(-)
Re: [PATCH 1/2] selftests: vm: Do not override definition of ARCH
On 2/10/20 7:49 PM, Michael Ellerman wrote:
Sandipan Das writes:
Independent builds of the vm selftests is currently broken
because commit 7549b3364201 overrides the value of ARCH with
the machine name from uname. This does not always match the
architecture names used for tasks like header installation.
In the interest not adding more delays to this patch, I fixed up
the commit log for the following error and applied it. I hope to
send this up for next rc.
ERROR: Please use git commit description style 'commit <12+ chars of
sha1> ("")' - ie: 'commit 7549b3364201 ("selftests: vm:
Build/Run 64bit tests only on 64bit arch")'
#93:
because commit 7549b3364201 overrides the value of ARCH with
The fixed up now looks as follows:
Independent builds of the vm selftests is currently broken because
commit 7549b3364201 ("selftests: vm: Build/Run 64bit tests only on
64bit arch") overrides the value of ARCH with the machine name from
uname. This does not always match the architecture names used for
tasks like header installation.
Please make sure to run checkpatch with --strict to catch these
kinds of errors in the future.
thanks,
-- Shuah
Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
On 10.04.2020 4:58, Tang Bin wrote: If the function "platform_get_irq()" failed, the negative value returned will not be detected here, including "-EPROBE_DEFER", which causes the application to fail to get the correct error message. Thus it must be fixed. platform_get_irq() prints an appropriate error message, the problem is that the current code calls request_irq() with error code instead of IRQ. Signed-off-by: Tang Bin Signed-off-by: Shengju Zhang [...] MBR, Sergei
Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
Hello!
On 10.04.2020 4:58, Tang Bin wrote:
If the function "platform_get_irq()" failed, the negative value
returned will not be detected here, including "-EPROBE_DEFER", which
causes the application to fail to get the correct error message.
Thus it must be fixed.
Signed-off-by: Tang Bin
Signed-off-by: Shengju Zhang
---
drivers/usb/gadget/udc/fsl_udc_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c
b/drivers/usb/gadget/udc/fsl_udc_core.c
index ec6eda426..60853ad10 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2441,8 +2441,8 @@ static int fsl_udc_probe(struct platform_device *pdev)
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
udc_controller->irq = platform_get_irq(pdev, 0);
- if (!udc_controller->irq) {
This check has always been wrong, doesn't account for real errors and
probe deferral...
- ret = -ENODEV;
+ if (udc_controller->irq <= 0) {
Note that platfrom_get_irq() no longer returns 0 on error.
+ ret = udc_controller->irq ? : -ENODEV;
goto err_iounmap;
}
MBR, Sergei
Re: usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
Hi Markus: On 2020/4/10 16:30, Markus Elfring wrote: Hardware experiments show that the negative return value is not just "-EPROBE_DEFER". How much will this specific error code influence our understanding of the discussed software situation? From my superficial knowledge, I think we should not think about it too complicated. The return value is just zero or negative, and for these negative return value, such as "-ENODEV"、"-ENXIO"、"-ENOENT"、“EPROBE_DEFER”，may be the same effect。But“-EPROBE_DEFER”has another importment function: Driver requested deferred probing，which is used in cases where the dependency resource is not ready during the driver initialization process. + ret = udc_controller->irq ? : -ENODEV; Will it be clearer to specify values for all cases in such a conditional operator (instead of leaving one case empty)? I don't know what you mean of "instead of leaving one case empty". I suggest to reconsider also the proposed specification “… ? : …”. What you mean is the way I'm written？ I have provided two ways of patching, both functional can be verified on hardware. Thanks for your patience. Tang Bin
Re: usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
> Hardware experiments show that the negative return value is not just > "-EPROBE_DEFER". How much will this specific error code influence our understanding of the discussed software situation? >>> + ret = udc_controller->irq ? : -ENODEV; >> Will it be clearer to specify values for all cases in such a conditional >> operator >> (instead of leaving one case empty)? > > I don't know what you mean of "instead of leaving one case empty". I suggest to reconsider also the proposed specification “… ? : …”. Regards, Markus
Re: [PATCH 19/28] gpu/drm: remove the powerpc hack in drm_legacy_sg_alloc
On Fri, Apr 10, 2020 at 12:57 AM Benjamin Herrenschmidt wrote: > > On Thu, 2020-04-09 at 11:41 +0200, Daniel Vetter wrote: > > Now if these boxes didn't ever have agp then I think we can get away > > with deleting this, since we've already deleted the legacy radeon > > driver. And that one used vmalloc for everything. The new kms one does > > use the dma-api if the gpu isn't connected through agp > > Definitely no AGP there. Ah in that case I think we can be sure that this code is dead. Acked-by: Daniel Vetter Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
Hi Markus
On 2020/4/10 15:33, Markus Elfring wrote:
If the function "platform_get_irq()" failed, the negative value
returned will not be detected here, including "-EPROBE_DEFER",
I suggest to adjust this change description.
Wording alternative:
The negative return value (which could eventually be “-EPROBE_DEFER”)
will not be detected here from a failed call of the function
“platform_get_irq”.
Hardware experiments show that the negative return value is not just
"-EPROBE_DEFER".
which causes the application to fail to get the correct error message.
Will another fine-tuning become relevant also for this wording?
Maybe that's not quite accurate.
Thus it must be fixed.
Wording alternative:
Thus adjust the error detection and corresponding exception handling.
Got it.
Signed-off-by: Tang Bin
Signed-off-by: Shengju Zhang
How do you think about to add the tags “Fixes”, “Link” and “Reported-by”?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=c0cc271173b2e1c2d8d0ceaef14e4dfa79eefc0d#n584
usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in
fsl_udc_probe()
https://lore.kernel.org/linux-usb/36341bb1-1e00-5eb1-d032-60dcc614d...@web.de/
https://lkml.org/lkml/2020/4/8/442
…
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2441,8 +2441,8 @@ static int fsl_udc_probe(struct platform_device *pdev)
udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
udc_controller->irq = platform_get_irq(pdev, 0);
- if (!udc_controller->irq) {
- ret = -ENODEV;
+ if (udc_controller->irq <= 0) {
Will such a failure predicate need any more clarification?
How does this check fit to the current software documentation?
Maybe my tags are not suitable.
+ ret = udc_controller->irq ? : -ENODEV;
Will it be clearer to specify values for all cases in such a conditional
operator
(instead of leaving one case empty)?
I don't know what you mean of "instead of leaving one case empty". But
by experiment, "ret = udc_controller->irq ? : -ENODEV" or "ret =
udc_controller->irq < 0 ? udc_controller->irq : -ENODEV" should be
suitable here.
Thank you for your guidance.
Tang Bin
Re: [PATCH] usb: gadget: fsl: Fix a wrong judgment in fsl_udc_probe()
> If the function "platform_get_irq()" failed, the negative value
> returned will not be detected here, including "-EPROBE_DEFER",
I suggest to adjust this change description.
Wording alternative:
The negative return value (which could eventually be “-EPROBE_DEFER”)
will not be detected here from a failed call of the function
“platform_get_irq”.
> which causes the application to fail to get the correct error message.
Will another fine-tuning become relevant also for this wording?
> Thus it must be fixed.
Wording alternative:
Thus adjust the error detection and corresponding exception handling.
> Signed-off-by: Tang Bin
> Signed-off-by: Shengju Zhang
How do you think about to add the tags “Fixes”, “Link” and “Reported-by”?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=c0cc271173b2e1c2d8d0ceaef14e4dfa79eefc0d#n584
usb: gadget: fsl_udc_core: Checking for a failed platform_get_irq() call in
fsl_udc_probe()
https://lore.kernel.org/linux-usb/36341bb1-1e00-5eb1-d032-60dcc614d...@web.de/
https://lkml.org/lkml/2020/4/8/442
…
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -2441,8 +2441,8 @@ static int fsl_udc_probe(struct platform_device *pdev)
> udc_controller->max_ep = (dccparams & DCCPARAMS_DEN_MASK) * 2;
>
> udc_controller->irq = platform_get_irq(pdev, 0);
> - if (!udc_controller->irq) {
> - ret = -ENODEV;
> + if (udc_controller->irq <= 0) {
Will such a failure predicate need any more clarification?
How does this check fit to the current software documentation?
> + ret = udc_controller->irq ? : -ENODEV;
Will it be clearer to specify values for all cases in such a conditional
operator
(instead of leaving one case empty)?
Regards,
Markus
