Re: [PATCH v9 05/12] mm: HUGE_VMAP arch support cleanup
Excerpts from Ding Tianhong's message of January 4, 2021 10:33 pm:
> On 2020/12/5 14:57, Nicholas Piggin wrote:
>> This changes the awkward approach where architectures provide init
>> functions to determine which levels they can provide large mappings for,
>> to one where the arch is queried for each call.
>>
>> This removes code and indirection, and allows constant-folding of dead
>> code for unsupported levels.
>>
>> This also adds a prot argument to the arch query. This is unused
>> currently but could help with some architectures (e.g., some powerpc
>> processors can't map uncacheable memory with large pages).
>>
>> Cc: linuxppc-...@lists.ozlabs.org
>> Cc: Catalin Marinas
>> Cc: Will Deacon
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: Thomas Gleixner
>> Cc: Ingo Molnar
>> Cc: Borislav Petkov
>> Cc: x...@kernel.org
>> Cc: "H. Peter Anvin"
>> Acked-by: Catalin Marinas [arm64]
>> Signed-off-by: Nicholas Piggin
>> ---
>> arch/arm64/include/asm/vmalloc.h | 8 +++
>> arch/arm64/mm/mmu.c | 10 +--
>> arch/powerpc/include/asm/vmalloc.h | 8 +++
>> arch/powerpc/mm/book3s64/radix_pgtable.c | 8 +--
>> arch/x86/include/asm/vmalloc.h | 7 ++
>> arch/x86/mm/ioremap.c| 10 +--
>> include/linux/io.h | 9 ---
>> include/linux/vmalloc.h | 6 ++
>> init/main.c | 1 -
>> mm/ioremap.c | 88 +---
>> 10 files changed, 77 insertions(+), 78 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/vmalloc.h
>> b/arch/arm64/include/asm/vmalloc.h
>> index 2ca708ab9b20..597b40405319 100644
>> --- a/arch/arm64/include/asm/vmalloc.h
>> +++ b/arch/arm64/include/asm/vmalloc.h
>> @@ -1,4 +1,12 @@
>> #ifndef _ASM_ARM64_VMALLOC_H
>> #define _ASM_ARM64_VMALLOC_H
>>
>> +#include
>> +
>> +#ifdef CONFIG_HAVE_ARCH_HUGE_VMAP
>> +bool arch_vmap_p4d_supported(pgprot_t prot);
>> +bool arch_vmap_pud_supported(pgprot_t prot);
>> +bool arch_vmap_pmd_supported(pgprot_t prot);
>> +#endif
>> +
>> #endif /* _ASM_ARM64_VMALLOC_H */
>> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
>> index ca692a815731..1b60079c1cef 100644
>> --- a/arch/arm64/mm/mmu.c
>> +++ b/arch/arm64/mm/mmu.c
>> @@ -1315,12 +1315,12 @@ void *__init fixmap_remap_fdt(phys_addr_t dt_phys,
>> int *size, pgprot_t prot)
>> return dt_virt;
>> }
>>
>> -int __init arch_ioremap_p4d_supported(void)
>> +bool arch_vmap_p4d_supported(pgprot_t prot)
>> {
>> -return 0;
>> +return false;
>> }
>>
>
> I think you should put this function in the CONFIG_HAVE_ARCH_HUGE_VMAP,
> otherwise it may break the compile when disable the
> CONFIG_HAVE_ARCH_HUGE_VMAP, the same
> as the x86 and ppc.
Ah, good catch. arm64 is okay because it always selects
HAVE_ARCH_HUGE_VMAP, powerpc is okay because it places
them in a file that's only compiled for configs that select
huge vmap, but x86-32 without PAE build breaks. I'll fix that.
Thanks,
Nick
[PATCH 0/2] Remove the IIO counter ABI
The IIO counter driver has been superseded by the Counter subsystem as discussed in [1]. This patchset removes the IIO counter ABI code and documentation. [1] https://lore.kernel.org/lkml/20210119104105.1...@huawei.com/ William Breathitt Gray (2): counter: 104-quad-8: Remove IIO counter ABI iio: Remove the IIO counter ABI documentation Documentation/ABI/testing/sysfs-bus-iio | 40 -- .../testing/sysfs-bus-iio-counter-104-quad-8 | 133 MAINTAINERS | 1 - drivers/counter/104-quad-8.c | 652 ++ drivers/counter/Kconfig | 2 +- 5 files changed, 65 insertions(+), 763 deletions(-) delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 -- 2.30.0
[PATCH 2/2] iio: Remove the IIO counter ABI documentation
The IIO counter driver has been superseded by the Counter subsystem as discussed in [1]. This patch removes the IIO counter ABI documentation. [1] https://lore.kernel.org/lkml/20210119104105.1...@huawei.com/ Cc: Fabrice Gasnier Cc: Benjamin Gaignard Signed-off-by: William Breathitt Gray --- Documentation/ABI/testing/sysfs-bus-iio | 40 - 1 file changed, 40 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio index d957f5da5c04..c207484a86d0 100644 --- a/Documentation/ABI/testing/sysfs-bus-iio +++ b/Documentation/ABI/testing/sysfs-bus-iio @@ -1704,46 +1704,6 @@ Description: Raw (unscaled no offset etc.) electric conductivity reading that can be processed to siemens per meter. -What: /sys/bus/iio/devices/iio:deviceX/in_countY_raw -KernelVersion: 4.10 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - Raw counter device counts from channel Y. For quadrature - counters, multiplication by an available [Y]_scale results in - the counts of a single quadrature signal phase from channel Y. - -What: /sys/bus/iio/devices/iio:deviceX/in_indexY_raw -KernelVersion: 4.10 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - Raw counter device index value from channel Y. This attribute - provides an absolute positional reference (e.g. a pulse once per - revolution) which may be used to home positional systems as - required. - -What: /sys/bus/iio/devices/iio:deviceX/in_count_count_direction_available -KernelVersion: 4.12 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - A list of possible counting directions which are: - - - "up" : counter device is increasing. - - "down": counter device is decreasing. - -What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_direction -KernelVersion: 4.12 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - Raw counter device counters direction for channel Y. - What: /sys/bus/iio/devices/iio:deviceX/in_voltageY_label What: /sys/bus/iio/devices/iio:deviceX/out_voltageY_label KernelVersion: 5.8 -- 2.30.0
[PATCH 1/2] counter: 104-quad-8: Remove IIO counter ABI
The IIO counter driver has been superseded by the Counter subsystem as discussed in [1]. This patch removes the IIO counter ABI from the 104-QUAD-8 driver. [1] https://lore.kernel.org/lkml/20210119104105.1...@huawei.com/ Cc: Syed Nayyar Waris Signed-off-by: William Breathitt Gray --- .../testing/sysfs-bus-iio-counter-104-quad-8 | 133 MAINTAINERS | 1 - drivers/counter/104-quad-8.c | 652 ++ drivers/counter/Kconfig | 2 +- 4 files changed, 65 insertions(+), 723 deletions(-) delete mode 100644 Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 diff --git a/Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 b/Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 deleted file mode 100644 index bac3d0d48b7b.. --- a/Documentation/ABI/testing/sysfs-bus-iio-counter-104-quad-8 +++ /dev/null @@ -1,133 +0,0 @@ -What: /sys/bus/iio/devices/iio:deviceX/in_count_count_mode_available -What: /sys/bus/iio/devices/iio:deviceX/in_count_noise_error_available -What: /sys/bus/iio/devices/iio:deviceX/in_count_quadrature_mode_available -What: /sys/bus/iio/devices/iio:deviceX/in_index_index_polarity_available -What: /sys/bus/iio/devices/iio:deviceX/in_index_synchronous_mode_available -KernelVersion: 4.10 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - Discrete set of available values for the respective counter - configuration are listed in this file. - -What: /sys/bus/iio/devices/iio:deviceX/in_countY_count_mode -KernelVersion: 4.10 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - Count mode for channel Y. Four count modes are available: - normal, range limit, non-recycle, and modulo-n. The preset value - for channel Y is used by the count mode where required. - - Normal: - Counting is continuous in either direction. - - Range Limit: - An upper or lower limit is set, mimicking limit switches - in the mechanical counterpart. The upper limit is set to - the preset value, while the lower limit is set to 0. The - counter freezes at count = preset when counting up, and - at count = 0 when counting down. At either of these - limits, the counting is resumed only when the count - direction is reversed. - - Non-recycle: - Counter is disabled whenever a 24-bit count overflow or - underflow takes place. The counter is re-enabled when a - new count value is loaded to the counter via a preset - operation or write to raw. - - Modulo-N: - A count boundary is set between 0 and the preset value. - The counter is reset to 0 at count = preset when - counting up, while the counter is set to the preset - value at count = 0 when counting down; the counter does - not freeze at the bundary points, but counts - continuously throughout. - -What: /sys/bus/iio/devices/iio:deviceX/in_countY_noise_error -KernelVersion: 4.10 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - Read-only attribute that indicates whether excessive noise is - present at the channel Y count inputs in quadrature clock mode; - irrelevant in non-quadrature clock mode. - -What: /sys/bus/iio/devices/iio:deviceX/in_countY_preset -KernelVersion: 4.10 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - If the counter device supports preset registers, the preset - count for channel Y is provided by this attribute. - -What: /sys/bus/iio/devices/iio:deviceX/in_countY_quadrature_mode -KernelVersion: 4.10 -Contact: linux-...@vger.kernel.org -Description: - This interface is deprecated; please use the Counter subsystem. - - Configure channel Y counter for non-quadrature or quadrature - clock mode. Selecting non-quadrature clock mode will disable - synchronous load mode. In quadrature clock mode, the channel Y - scale attribute selects the encoder phase division (scale of 1 - selects full-cycle, scale of 0.5 selects half-cycle, scale of -
[PATCH] mtd: rawnand: Fix an error handling path in 'ebu_dma_start()'
If 'dmaengine_prep_slave_single()' fails, we must undo a previous
'dma_map_single()' call, as already done in all the other error handling
paths of this function.
Fixes: 0b1039f016e8 ("mtd: rawnand: Add NAND controller support on Intel LGM
SoC")
Signed-off-by: Christophe JAILLET
---
drivers/mtd/nand/raw/intel-nand-controller.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/intel-nand-controller.c
b/drivers/mtd/nand/raw/intel-nand-controller.c
index a304fda5d1fa..8b49fd56cf96 100644
--- a/drivers/mtd/nand/raw/intel-nand-controller.c
+++ b/drivers/mtd/nand/raw/intel-nand-controller.c
@@ -318,8 +318,10 @@ static int ebu_dma_start(struct ebu_nand_controller
*ebu_host, u32 dir,
}
tx = dmaengine_prep_slave_single(chan, buf_dma, len, dir, flags);
- if (!tx)
- return -ENXIO;
+ if (!tx) {
+ ret = -ENXIO;
+ goto err_unmap;
+ }
tx->callback = callback;
tx->callback_param = ebu_host;
--
2.27.0
Re: [PATCH v2 2/2] memblock: do not start bottom-up allocations with kernel_end
On Sat, Jan 23, 2021 at 06:09:11PM -0800, Andrew Morton wrote:
> On Fri, 22 Jan 2021 01:37:14 -0300 Thiago Jung Bauermann
> wrote:
>
> > Mike Rapoport writes:
> >
> > > > Signed-off-by: Roman Gushchin
> > >
> > > Reviewed-by: Mike Rapoport
> >
> > I've seen a couple of spurious triggers of the WARN_ONCE() removed by this
> > patch. This happens on some ppc64le bare metal (powernv) server machines
> > with
> > CONFIG_SWIOTLB=y and crashkernel=4G, as described in a candidate patch I
> > posted
> > to solve this issue in a different way:
> >
> > https://lore.kernel.org/linuxppc-dev/20201218062103.76102-1-bauer...@linux.ibm.com/
> >
> > Since this patch solves that problem, is it possible to include it in the
> > next
> > feasible v5.11-rcX, with the following tag?
>
> We could do this, if we're confident that this patch doesn't depend on
> [1/2] "mm: cma: allocate cma areas bottom-up"? I think it is...
A think it does not depend on cma bottom-up allocation, it's rather the other
way around: without this CMA bottom-up allocation could fail with KASLR
enabled.
Still, this patch may need updates to the way x86 does early reservations:
https://lore.kernel.org/lkml/20210115083255.12744-1-r...@kernel.org
> > Fixes: 8fabc623238e ("powerpc: Ensure that swiotlb buffer is allocated from
> > low memory")
>
> I added that.
>
>
--
Sincerely yours,
Mike.
[PATCH] dmaengine: ti: k3-udma: Fix a resource leak in an error handling path
In 'dma_pool_create()', we return -ENOMEM, but don't release the resources
already allocated, as in all the other error handling paths.
Go to 'err_res_free' instead of returning directly.
Fixes: 017794739702 ("dmaengine: ti: k3-udma: Initial support for K3 BCDMA")
Signed-off-by: Christophe JAILLET
---
This patch is not even compile tested.
I don't have the needed configuration.
---
drivers/dma/ti/k3-udma.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/ti/k3-udma.c b/drivers/dma/ti/k3-udma.c
index 8e3fd1119a77..96ad21869ba7 100644
--- a/drivers/dma/ti/k3-udma.c
+++ b/drivers/dma/ti/k3-udma.c
@@ -2447,7 +2447,8 @@ static int bcdma_alloc_chan_resources(struct dma_chan
*chan)
dev_err(ud->ddev.dev,
"Descriptor pool allocation failed\n");
uc->use_dma_pool = false;
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_res_free;
}
uc->use_dma_pool = true;
--
2.27.0
Re: [PATCH] counter: stm32-lptimer-cnt: remove iio counter abi
On Fri, Jan 22, 2021 at 04:03:23PM +0100, Fabrice Gasnier wrote: > Currently, the STM32 LP Timer counter driver registers into both IIO and > counter subsystems, which is redundant. > > Remove the IIO counter ABI and IIO registration from the STM32 LP Timer > counter driver since it's been superseded by the Counter subsystem > as discussed in [1]. > > Keep only the counter subsystem related part. > Move a part of the ABI documentation into a driver comment. > > This also removes a duplicate ABI warning > $ scripts/get_abi.pl validate > ... > /sys/bus/iio/devices/iio:deviceX/in_count0_preset is defined 2 times: > ./Documentation/ABI/testing/sysfs-bus-iio-timer-stm32:100 > ./Documentation/ABI/testing/sysfs-bus-iio-lptimer-stm32:0 > > [1] https://lkml.org/lkml/2021/1/19/347 > > Signed-off-by: Fabrice Gasnier Acked-by: William Breathitt Gray I'll also submit a patchset removing the IIO code from the 104-quad-8 driver and the rest of the IIO counter ABI documentation. William Breathitt Gray signature.asc Description: PGP signature
Re: [External] Re: [PATCH v13 03/12] mm: hugetlb: free the vmemmap pages associated with each HugeTLB page
On Sun, Jan 24, 2021 at 1:53 AM Oscar Salvador wrote: > > X-Gm-Spam: 0 > X-Gm-Phishy: 0 > > On Sun, Jan 17, 2021 at 11:10:44PM +0800, Muchun Song wrote: > > Every HugeTLB has more than one struct page structure. We __know__ that > > we only use the first 4(HUGETLB_CGROUP_MIN_ORDER) struct page structures > > to store metadata associated with each HugeTLB. > > > > There are a lot of struct page structures associated with each HugeTLB > > page. For tail pages, the value of compound_head is the same. So we can > > reuse first page of tail page structures. We map the virtual addresses > > of the remaining pages of tail page structures to the first tail page > > struct, and then free these page frames. Therefore, we need to reserve > > two pages as vmemmap areas. > > > > When we allocate a HugeTLB page from the buddy, we can free some vmemmap > > pages associated with each HugeTLB page. It is more appropriate to do it > > in the prep_new_huge_page(). > > > > The free_vmemmap_pages_per_hpage(), which indicates how many vmemmap > > pages associated with a HugeTLB page can be freed, returns zero for > > now, which means the feature is disabled. We will enable it once all > > the infrastructure is there. > > > > Signed-off-by: Muchun Song > > Overall looks good to me. > A few nits below, plus what Mike has already said. > > I was playing the other day (just for un) to see how hard would be to adapt > this to ppc64 but did not have the time :-) I have no idea about ppc64. But for aarch64, it is easy to adapt this to aarch64 (I have finished this part of the work). Is the size of the struct page 64 bytes for ppc64? If so, I think that it also easy. > > > --- /dev/null > > +++ b/mm/hugetlb_vmemmap.c > > @@ -0,0 +1,211 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Free some vmemmap pages of HugeTLB > > + * > > + * Copyright (c) 2020, Bytedance. All rights reserved. > > + * > > + * Author: Muchun Song > > + * > > + * The struct page structures (page structs) are used to describe a > > physical > > + * page frame. By default, there is a one-to-one mapping from a page frame > > to > > + * it's corresponding page struct. > > + * > > + * The HugeTLB pages consist of multiple base page size pages and is > > supported > "HugeTLB pages ..." Thanks. > > > + * When the system boot up, every HugeTLB page has more than one struct > > page > > + * structs whose size is (unit: pages): > which? I am not a native English. Thanks for pointing this out. > > + * > > + *struct_size = HugeTLB_Size / PAGE_SIZE * sizeof(struct page) / > > PAGE_SIZE > > + * > > + * Where HugeTLB_Size is the size of the HugeTLB page. We know that the > > size > > + * of the HugeTLB page is always n times PAGE_SIZE. So we can get the > > following > > + * relationship. > > + * > > + *HugeTLB_Size = n * PAGE_SIZE > > + * > > + * Then, > > + * > > + *struct_size = n * PAGE_SIZE / PAGE_SIZE * sizeof(struct page) / > > PAGE_SIZE > > + *= n * sizeof(struct page) / PAGE_SIZE > > + * > > + * We can use huge mapping at the pud/pmd level for the HugeTLB page. > > + * > > + * For the HugeTLB page of the pmd level mapping, then > > + * > > + *struct_size = n * sizeof(struct page) / PAGE_SIZE > > + *= PAGE_SIZE / sizeof(pte_t) * sizeof(struct page) / > > PAGE_SIZE > > + *= sizeof(struct page) / sizeof(pte_t) > > + *= 64 / 8 > > + *= 8 (pages) > > + * > > + * Where n is how many pte entries which one page can contains. So the > > value of > > + * n is (PAGE_SIZE / sizeof(pte_t)). > > + * > > + * This optimization only supports 64-bit system, so the value of > > sizeof(pte_t) > > + * is 8. And this optimization also applicable only when the size of > > struct page > > + * is a power of two. In most cases, the size of struct page is 64 (e.g. > > x86-64 > > + * and arm64). So if we use pmd level mapping for a HugeTLB page, the size > > of > > + * struct page structs of it is 8 pages whose size depends on the size of > > the > > + * base page. > > + * > > + * For the HugeTLB page of the pud level mapping, then > > + * > > + *struct_size = PAGE_SIZE / sizeof(pmd_t) * struct_size(pmd) > > + *= PAGE_SIZE / 8 * 8 (pages) > > + *= PAGE_SIZE (pages) > > I would try to condense above information and focus on what are the > key points you want people to get. > E.g: A 2MB HugeTLB page on x86_64 consists in 8 page frames while 1GB > HugeTLB page consists in 4096. > If you do not want to be that specific you can always write down the > formula, and maybe put the X86_64 example at the end. > But as I said, I would try to make it more brief. > > Maybe others disagree though. I want to make the formula more general. Because the PAGE_SIZE can be different on arm64. But you are right, I should make it brief and easy to understand. I will add some examples at the end of the formula. Thanks. > > > > + *
Re: [PATCH] x86/sgx: Allows ioctl PROVISION to execute before CREATE
On 1/21/21 6:37 AM, Jarkko Sakkinen wrote: On Thu, Jan 21, 2021 at 12:34:49AM +0200, Jarkko Sakkinen wrote: On Wed, Jan 20, 2021 at 11:57:18AM +0800, Tianjia Zhang wrote: Hi, On 1/20/21 4:05 AM, Sean Christopherson wrote: On Mon, Jan 18, 2021, Tianjia Zhang wrote: In function sgx_encl_create(), the logic of directly assigning value to attributes_mask determines that the call to SGX_IOC_ENCLAVE_PROVISION must be after the command of SGX_IOC_ENCLAVE_CREATE. If change this assignment statement to or operation, the PROVISION command can be executed earlier and more flexibly. Reported-by: Jia Zhang Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index f45957c05f69..0ca3fc238bc2 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -108,7 +108,7 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; + encl->attributes_mask |= SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; Alternatively, move the existing code to sgx_open()? Initializing the field when the encl object is allocated feels more correct. This seems like a good idea. Thanks for your suggestion. I have sent v2 patch, include the next two patches. Did you ask from Sean about suggested-by's? Now it looks like that doing these patches were originally proposed by Sean. Please do not add tags from people *unauthentically*. I do not see anything from Sean to any of the patches that would suggest adding those tags. You are basically just stamping that to all patches, which he has given a code review. Can you stop doing this? /Jarkko I am very sorry for the trouble caused to you, I have made improvements in the new patch, thanks for your suggestions. Best regards, Tianjia
Re: [PATCH v2] x86/sgx: Remove redundant if conditions in sgx_encl_create
On 1/21/21 1:47 AM, Sean Christopherson wrote: On Wed, Jan 20, 2021, Tianjia Zhang wrote: In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Reported-by: Jia Zhang Suggested-by: Sean Christopherson Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..efad2fb61c76 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,12 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + + if (WARN_ONCE(!va_page, + "the tail page of the VA page list had free slots\n")) IMO it's worth trimming down the message to keep this on a single line. The newline isn't necessary, and this code expects a completely empty list, e.g. it can be reworded to something like: if (WARN_ONCE(!va_page, "non-empty VA page list before ECREATE")) Thanks for your suggestion, I have resubmitted a set of patches. Best regards, Tianjia
[PATCH] driver core: Fix a reource leak in 'fw_devlink_create_devlink()'
In 'fw_devlink_create_devlink()', all exit paths, successful or not,
release the reference taken on 'sup_dev'.
All but this one.
Use the existing error handling case to call the missing 'put_device()'.
Fixes: f9aa460672c9c ("driver core: Refactor fw_devlink feature")
Signed-off-by: Christophe JAILLET
---
Speculative and compile tested only
---
drivers/base/core.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index e08d376631cc..8775717a6bf7 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1623,8 +1623,10 @@ static int fw_devlink_create_devlink(struct device *con,
* cycles. So cycle detection isn't necessary and shouldn't be
* done.
*/
- if (flags & DL_FLAG_SYNC_STATE_ONLY)
- return -EAGAIN;
+ if (flags & DL_FLAG_SYNC_STATE_ONLY) {
+ ret = -EAGAIN;
+ goto out;
+ }
/*
* If we can't find the supplier device from its fwnode, it might be
--
2.27.0
[PATCH v3 4/5] x86/sgx: Allows ioctl PROVISION to execute before CREATE
In the function sgx_create_enclave(), the direct assignment operation of attributes_mask determines that the ioctl PROVISION operation must be executed after the ioctl CREATE operation, which will limit the flexibility of sgx developers. This patch takes the assignment of `attributes_mask` from the function sgx_create_enclave() has been moved to the function sgx_open() to avoid this restriction. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c index f2eac41bb4ff..fba0d0bfe976 100644 --- a/arch/x86/kernel/cpu/sgx/driver.c +++ b/arch/x86/kernel/cpu/sgx/driver.c @@ -36,6 +36,7 @@ static int sgx_open(struct inode *inode, struct file *file) return ret; } + encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; file->private_data = encl; return 0; diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 90a5caf76939..1c6ecf9fbeff 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -109,7 +109,6 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->base = secs->base; encl->size = secs->size; encl->attributes = secs->attributes; - encl->attributes_mask = SGX_ATTR_DEBUG | SGX_ATTR_MODE64BIT | SGX_ATTR_KSS; /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, >flags); -- 2.19.1.3.ge56e4f7
[PATCH v3 0/5] Some optimizations related to sgx
This is an optimization of a set of sgx-related codes, each of which is independent of the patch. Because the second and third patches have conflicting dependencies, these patches are put together. --- v3 changes: * split free_cnt count and spin lock optimization into two patches v2 changes: * review suggested changes Tianjia Zhang (5): selftests/x86: Simplify the code to get vdso base address in sgx x86/sgx: Optimize the locking range in sgx_sanitize_section() x86/sgx: Optimize the free_cnt count in sgx_epc_section x86/sgx: Allows ioctl PROVISION to execute before CREATE x86/sgx: Remove redundant if conditions in sgx_encl_create arch/x86/kernel/cpu/sgx/driver.c | 1 + arch/x86/kernel/cpu/sgx/ioctl.c| 9 + arch/x86/kernel/cpu/sgx/main.c | 13 + tools/testing/selftests/sgx/main.c | 24 4 files changed, 15 insertions(+), 32 deletions(-) -- 2.19.1.3.ge56e4f7
[PATCH v3 2/5] x86/sgx: Optimize the locking range in sgx_sanitize_section()
The spin lock of sgx_epc_section only locks the page_list. The
EREMOVE operation and init_laundry_list is not necessary in the
protection range of the spin lock. This patch reduces the lock
range of the spin lock in the function sgx_sanitize_section()
and only protects the operation of the page_list.
Suggested-by: Sean Christopherson
Signed-off-by: Tianjia Zhang
---
arch/x86/kernel/cpu/sgx/main.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index c519fc5f6948..4465912174fd 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -41,20 +41,17 @@ static void sgx_sanitize_section(struct sgx_epc_section
*section)
if (kthread_should_stop())
return;
- /* needed for access to ->page_list: */
- spin_lock(>lock);
-
page = list_first_entry(>init_laundry_list,
struct sgx_epc_page, list);
ret = __eremove(sgx_get_epc_virt_addr(page));
- if (!ret)
+ if (!ret) {
+ spin_lock(>lock);
list_move(>list, >page_list);
- else
+ spin_unlock(>lock);
+ } else
list_move_tail(>list, );
- spin_unlock(>lock);
-
cond_resched();
}
--
2.19.1.3.ge56e4f7
[PATCH v3 3/5] x86/sgx: Optimize the free_cnt count in sgx_epc_section
`section->free_cnt` represents the free page in sgx_epc_section,
which is assigned once after initialization. In fact, just after the
initialization is completed, the pages are in the `init_laundry_list`
list and cannot be allocated. This needs to be recovered by EREMOVE
of function sgx_sanitize_section() before it can be used as a page
that can be allocated. The sgx_sanitize_section() will be called in
the kernel thread ksgxd.
This patch moves the initialization of `section->free_cnt` from the
initialization function `sgx_setup_epc_section()` to the function
`sgx_sanitize_section()`, and then accumulates the count after the
successful execution of EREMOVE. This seems to be more reasonable,
free_cnt will also truly reflect the allocatable free pages in EPC.
Sined-off-by: Tianjia Zhang
Reviewed-by: Sean Christopherson
---
arch/x86/kernel/cpu/sgx/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 4465912174fd..e455ec7b3449 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -48,6 +48,7 @@ static void sgx_sanitize_section(struct sgx_epc_section
*section)
if (!ret) {
spin_lock(>lock);
list_move(>list, >page_list);
+ section->free_cnt++;
spin_unlock(>lock);
} else
list_move_tail(>list, );
@@ -643,7 +644,6 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64
size,
list_add_tail(>pages[i].list,
>init_laundry_list);
}
- section->free_cnt = nr_pages;
return true;
}
--
2.19.1.3.ge56e4f7
[PATCH v3 5/5] x86/sgx: Remove redundant if conditions in sgx_encl_create
In this scenario, there is no case where va_page is NULL, and the error has been checked. The if condition statement here is redundant, so remove the condition detection. Signed-off-by: Tianjia Zhang --- arch/x86/kernel/cpu/sgx/ioctl.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 1c6ecf9fbeff..b0b829f1b761 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -66,9 +66,11 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) va_page = sgx_encl_grow(encl); if (IS_ERR(va_page)) return PTR_ERR(va_page); - else if (va_page) - list_add(_page->list, >va_pages); - /* else the tail page of the VA page list had free slots. */ + + if (WARN_ONCE(!va_page, "non-empty VA page list before ECREATE")) + return -EIO; + + list_add(_page->list, >va_pages); /* The extra page goes to SECS. */ encl_size = secs->size + PAGE_SIZE; -- 2.19.1.3.ge56e4f7
[PATCH v3 1/5] selftests/x86: Simplify the code to get vdso base address in sgx
This patch uses the library function `getauxval(AT_SYSINFO_EHDR)`
instead of the custom function `vdso_get_base_addr` to obtain the
base address of vDSO, which will simplify the code implementation.
Signed-off-by: Tianjia Zhang
---
tools/testing/selftests/sgx/main.c | 24
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/sgx/main.c
b/tools/testing/selftests/sgx/main.c
index 724cec700926..365d01dea67b 100644
--- a/tools/testing/selftests/sgx/main.c
+++ b/tools/testing/selftests/sgx/main.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include "defines.h"
#include "main.h"
#include "../kselftest.h"
@@ -28,24 +29,6 @@ struct vdso_symtab {
Elf64_Word *elf_hashtab;
};
-static void *vdso_get_base_addr(char *envp[])
-{
- Elf64_auxv_t *auxv;
- int i;
-
- for (i = 0; envp[i]; i++)
- ;
-
- auxv = (Elf64_auxv_t *)[i + 1];
-
- for (i = 0; auxv[i].a_type != AT_NULL; i++) {
- if (auxv[i].a_type == AT_SYSINFO_EHDR)
- return (void *)auxv[i].a_un.a_val;
- }
-
- return NULL;
-}
-
static Elf64_Dyn *vdso_get_dyntab(void *addr)
{
Elf64_Ehdr *ehdr = addr;
@@ -162,7 +145,7 @@ static int user_handler(long rdi, long rsi, long rdx, long
ursp, long r8, long r
return 0;
}
-int main(int argc, char *argv[], char *envp[])
+int main(int argc, char *argv[])
{
struct sgx_enclave_run run;
struct vdso_symtab symtab;
@@ -203,7 +186,8 @@ int main(int argc, char *argv[], char *envp[])
memset(, 0, sizeof(run));
run.tcs = encl.encl_base;
- addr = vdso_get_base_addr(envp);
+ /* Get vDSO base address */
+ addr = (void *)(uintptr_t)getauxval(AT_SYSINFO_EHDR);
if (!addr)
goto err;
--
2.19.1.3.ge56e4f7
Re: [PATCH v4 net-next 0/2] udp: allow forwarding of plain (non-fraglisted) UDP GRO packets
Hello: This series was applied to netdev/net-next.git (refs/heads/master): On Fri, 22 Jan 2021 18:19:36 + you wrote: > This series allows to form UDP GRO packets in cases without sockets > (for forwarding). To not change the current datapath, this is > performed only when the new corresponding netdev feature is enabled > via Ethtool (and fraglisted GRO is disabled). > Prior to this point, only fraglisted UDP GRO was available. Plain UDP > GRO shows better forwarding performance when a target NIC is capable > of GSO UDP offload. > > [...] Here is the summary with links: - [v4,net-next,1/2] net: introduce a netdev feature for UDP GRO forwarding https://git.kernel.org/netdev/net-next/c/6f1c0ea133a6 - [v4,net-next,2/2] udp: allow forwarding of plain (non-fraglisted) UDP GRO packets https://git.kernel.org/netdev/net-next/c/36707061d6ba You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
[tip:x86/cleanups] BUILD SUCCESS 8ece53ef7f428ee3f8eab936268b1a3fe2725e6b
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/cleanups branch HEAD: 8ece53ef7f428ee3f8eab936268b1a3fe2725e6b x86/vm86/32: Remove VM86_SCREEN_BITMAP support i386-tinyconfig vmlinux size: TOTAL TEXT init.text check_iommu_entries() 0 0 0 0 4af0e6e39b7e x86/mm: Remove duplicate definition of _PAGE_PAT_LARGE +1 0 0 0 11aa1415d8bd x86/entry: Remove now unused do_IRQ() declaration -76 -76-76-76 b86cb29287be x86: Remove definition of DEBUG +1 0 0 0 8ece53ef7f42 x86/vm86/32: Remove VM86_SCREEN_BITMAP support -74 -76-76-76 5c8fe583cce5..8ece53ef7f42 (ALL COMMITS) elapsed time: 3273m configs tested: 78 configs skipped: 71 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig powerpcamigaone_defconfig mipsmalta_qemu_32r6_defconfig mipsmaltaup_defconfig powerpc tqm8560_defconfig powerpc mpc8315_rdb_defconfig powerpc mpc885_ads_defconfig powerpc holly_defconfig powerpc storcenter_defconfig powerpc taishan_defconfig sh se7712_defconfig mipsbcm47xx_defconfig armspear3xx_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig i386 tinyconfig i386defconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig x86_64 randconfig-a002-20210121 x86_64 randconfig-a003-20210121 x86_64 randconfig-a001-20210121 x86_64 randconfig-a005-20210121 x86_64 randconfig-a006-20210121 x86_64 randconfig-a004-20210121 i386 randconfig-a001-20210121 i386 randconfig-a002-20210121 i386 randconfig-a004-20210121 i386 randconfig-a006-20210121 i386 randconfig-a005-20210121 i386 randconfig-a003-20210121 i386 randconfig-a013-20210121 i386 randconfig-a011-20210121 i386 randconfig-a012-20210121 i386 randconfig-a014-20210121 i386 randconfig-a015-20210121 i386 randconfig-a016-20210121 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64
[rcu:rcu/next] BUILD SUCCESS 68dda19311f01b3450d4a823e47ccbc2c3d06e3c
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git rcu/next branch HEAD: 68dda19311f01b3450d4a823e47ccbc2c3d06e3c kvfree_rcu: Make krc_this_cpu_unlock() use raw_spin_unlock_irqrestore() elapsed time: 3230m configs tested: 93 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig xtensa virt_defconfig armshmobile_defconfig sparc sparc32_defconfig microblaze mmu_defconfig sh sh7785lcr_32bit_defconfig powerpc mpc885_ads_defconfig powerpc holly_defconfig powerpc storcenter_defconfig powerpc taishan_defconfig powerpc mpc5200_defconfig m68k m5475evb_defconfig m68k m5208evb_defconfig arm stm32_defconfig powerpc mpc836x_rdk_defconfig sh sh7724_generic_defconfig sh ap325rxa_defconfig archsdk_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386 tinyconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig x86_64 randconfig-a002-20210121 x86_64 randconfig-a003-20210121 x86_64 randconfig-a001-20210121 x86_64 randconfig-a005-20210121 x86_64 randconfig-a006-20210121 x86_64 randconfig-a004-20210121 i386 randconfig-a001-20210121 i386 randconfig-a002-20210121 i386 randconfig-a004-20210121 i386 randconfig-a006-20210121 i386 randconfig-a005-20210121 i386 randconfig-a003-20210121 i386 randconfig-a001-20210122 i386 randconfig-a002-20210122 i386 randconfig-a004-20210122 i386 randconfig-a006-20210122 i386 randconfig-a003-20210122 i386 randconfig-a005-20210122 i386 randconfig-a013-20210121 i386 randconfig-a011-20210121 i386 randconfig-a012-20210121 i386 randconfig-a014-20210121 i386 randconfig-a015-20210121 i386 randconfig-a016-20210121 riscvnommu_k210_defconfig riscvnommu_virt_defconfig riscv defconfig riscv rv32_defconfig riscvallyesconfig riscv allnoconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kbuiltin x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[tip:x86/urgent] BUILD SUCCESS 6e1239c13953f3c2a76e70031f74ddca9ae57cd3
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/urgent branch HEAD: 6e1239c13953f3c2a76e70031f74ddca9ae57cd3 x86/cpu: Add another Alder Lake CPU to the Intel family elapsed time: 3272m configs tested: 78 configs skipped: 71 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig powerpcamigaone_defconfig mipsmalta_qemu_32r6_defconfig mipsmaltaup_defconfig powerpc tqm8560_defconfig powerpc mpc8315_rdb_defconfig powerpc mpc885_ads_defconfig powerpc holly_defconfig powerpc storcenter_defconfig powerpc taishan_defconfig sh se7712_defconfig mipsbcm47xx_defconfig armspear3xx_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig i386 tinyconfig i386defconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig x86_64 randconfig-a002-20210121 x86_64 randconfig-a003-20210121 x86_64 randconfig-a001-20210121 x86_64 randconfig-a005-20210121 x86_64 randconfig-a006-20210121 x86_64 randconfig-a004-20210121 i386 randconfig-a001-20210121 i386 randconfig-a002-20210121 i386 randconfig-a004-20210121 i386 randconfig-a006-20210121 i386 randconfig-a005-20210121 i386 randconfig-a003-20210121 i386 randconfig-a013-20210121 i386 randconfig-a011-20210121 i386 randconfig-a012-20210121 i386 randconfig-a014-20210121 i386 randconfig-a015-20210121 i386 randconfig-a016-20210121 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64 rhel x86_64 allyesconfig x86_64rhel-7.6-kselftests x86_64 defconfig x86_64 rhel-8.3 x86_64 rhel-8.3-kbuiltin x86_64 kexec --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
Re: [PATCH next v2 03/17] dt-bindings: phy: mediatek: dsi-phy: modify compatible dependence
Hi, Chunfeng: Chun-Kuang Hu 於 2021年1月24日 週日 下午12:47寫道： > > Hi, Chunfeng: > > Chunfeng Yun 於 2021年1月22日 週五 下午8:04寫道： > > > > The compatilbe "mediatek,mt7623-mipi-tx" is not supported in driver, > > and in fact uses "mediatek,mt2701-mipi-tx" instead on MT7623, so changes > > the compatible items to make dependence clear. > > Reviewed-by: Chun-Kuang Hu Sorry, please remove my reviewed-by tag. We should not define the binding document according to the implementation of driver. We define the binding document according to the hardware. So the description should be like "mt7623-mipi-tx is compatible to mt2701-mipi-tx". Regards, Chun-Kuang. > > > > > Cc: Chun-Kuang Hu > > Cc: Philipp Zabel > > Signed-off-by: Chunfeng Yun > > --- > > v2: separate two patches suggested by CK > > --- > > .../devicetree/bindings/phy/mediatek,dsi-phy.yaml | 13 - > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > > b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > > index 71d4acea1f66..6e4d795f9b02 100644 > > --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > > +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > > @@ -19,11 +19,14 @@ properties: > > pattern: "^dsi-phy@[0-9a-f]+$" > > > >compatible: > > -enum: > > - - mediatek,mt2701-mipi-tx > > - - mediatek,mt7623-mipi-tx > > - - mediatek,mt8173-mipi-tx > > - - mediatek,mt8183-mipi-tx > > +oneOf: > > + - items: > > + - enum: > > + - mediatek,mt7623-mipi-tx > > + - const: mediatek,mt2701-mipi-tx > > + - const: mediatek,mt2701-mipi-tx > > + - const: mediatek,mt8173-mipi-tx > > + - const: mediatek,mt8183-mipi-tx > > > >reg: > > maxItems: 1 > > -- > > 2.18.0
Re: [PATCH next v2 03/17] dt-bindings: phy: mediatek: dsi-phy: modify compatible dependence
Hi, Chunfeng: Chunfeng Yun 於 2021年1月22日 週五 下午8:04寫道： > > The compatilbe "mediatek,mt7623-mipi-tx" is not supported in driver, > and in fact uses "mediatek,mt2701-mipi-tx" instead on MT7623, so changes > the compatible items to make dependence clear. Reviewed-by: Chun-Kuang Hu > > Cc: Chun-Kuang Hu > Cc: Philipp Zabel > Signed-off-by: Chunfeng Yun > --- > v2: separate two patches suggested by CK > --- > .../devicetree/bindings/phy/mediatek,dsi-phy.yaml | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > index 71d4acea1f66..6e4d795f9b02 100644 > --- a/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > +++ b/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.yaml > @@ -19,11 +19,14 @@ properties: > pattern: "^dsi-phy@[0-9a-f]+$" > >compatible: > -enum: > - - mediatek,mt2701-mipi-tx > - - mediatek,mt7623-mipi-tx > - - mediatek,mt8173-mipi-tx > - - mediatek,mt8183-mipi-tx > +oneOf: > + - items: > + - enum: > + - mediatek,mt7623-mipi-tx > + - const: mediatek,mt2701-mipi-tx > + - const: mediatek,mt2701-mipi-tx > + - const: mediatek,mt8173-mipi-tx > + - const: mediatek,mt8183-mipi-tx > >reg: > maxItems: 1 > -- > 2.18.0
Re: [PATCH net v5] net: lapb: Add locking to the lapb module
On Fri, 22 Jan 2021 10:07:05 +0100 Martin Schiller wrote:
> On 2021-01-21 01:21, Xie He wrote:
> > In the lapb module, the timers may run concurrently with other code in
> > this module, and there is currently no locking to prevent the code from
> > racing on "struct lapb_cb". This patch adds locking to prevent racing.
> >
> > 1. Add "spinlock_t lock" to "struct lapb_cb"; Add "spin_lock_bh" and
> > "spin_unlock_bh" to APIs, timer functions and notifier functions.
> >
> > 2. Add "bool t1timer_stop, t2timer_stop" to "struct lapb_cb" to make us
> > able to ask running timers to abort; Modify "lapb_stop_t1timer" and
> > "lapb_stop_t2timer" to make them able to abort running timers;
> > Modify "lapb_t2timer_expiry" and "lapb_t1timer_expiry" to make them
> > abort after they are stopped by "lapb_stop_t1timer",
> > "lapb_stop_t2timer",
> > and "lapb_start_t1timer", "lapb_start_t2timer".
> >
> > 3. Let lapb_unregister wait for other API functions and running timers
> > to stop.
> >
> > 4. The lapb_device_event function calls lapb_disconnect_request. In
> > order to avoid trying to hold the lock twice, add a new function named
> > "__lapb_disconnect_request" which assumes the lock is held, and make
> > it called by lapb_disconnect_request and lapb_device_event.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Cc: Martin Schiller
> > Signed-off-by: Xie He
>
> I don't have the opportunity to test this at the moment, but code looks
> reasonable so far. Have you tested this at runtime?
Are you okay with this being merged or would you like to review
further/test?
Nothing jumps out to me either (other than a few nit picks).
> > Change from v4:
> > Make lapb_unregister wait for other refs to "lapb" to drop, to ensure
> > that other LAPB API calls have all finished.
> >
> > Change from v3:
> > In lapb_unregister make sure the self-restarting t1timer has really
> > been
> > stopped.
> >
> > Change from v2:
> > Create a new __lapb_disconnect_request function to reduce redundant
> > code.
> >
> > Change from v1:
> > Broke long lines to keep the line lengths within 80 characters.
> > @@ -178,11 +182,23 @@ int lapb_unregister(struct net_device *dev)
> > goto out;
> > lapb_put(lapb);
> >
> > + /* Wait for other refs to "lapb" to drop */
> > + while (refcount_read(>refcnt) > 2)
> > + ;
Tight loop like this is a little scary, perhaps add a small
usleep_range() here?
> > +
> > + spin_lock_bh(>lock);
> > +
> > lapb_stop_t1timer(lapb);
> > lapb_stop_t2timer(lapb);
> >
> > lapb_clear_queues(lapb);
> >
> > + spin_unlock_bh(>lock);
> > +
> > + /* Wait for running timers to stop */
> > + del_timer_sync(>t1timer);
> > + del_timer_sync(>t2timer);
> > +
> > __lapb_remove_cb(lapb);
> >
> > lapb_put(lapb);
> > -int lapb_disconnect_request(struct net_device *dev)
> > +static int __lapb_disconnect_request(struct lapb_cb *lapb)
> > {
> > - struct lapb_cb *lapb = lapb_devtostruct(dev);
> > - int rc = LAPB_BADTOKEN;
> > -
> > - if (!lapb)
> > - goto out;
> > -
> > switch (lapb->state) {
> > case LAPB_STATE_0:
> > - rc = LAPB_NOTCONNECTED;
> > - goto out_put;
> > + return LAPB_NOTCONNECTED;
> >
> > case LAPB_STATE_1:
> > lapb_dbg(1, "(%p) S1 TX DISC(1)\n", lapb->dev);
> > @@ -310,12 +328,10 @@ int lapb_disconnect_request(struct net_device
> > *dev)
> > lapb_send_control(lapb, LAPB_DISC, LAPB_POLLON, LAPB_COMMAND);
> > lapb->state = LAPB_STATE_0;
> > lapb_start_t1timer(lapb);
> > - rc = LAPB_NOTCONNECTED;
> > - goto out_put;
> > + return LAPB_NOTCONNECTED;
> >
> > case LAPB_STATE_2:
> > - rc = LAPB_OK;
> > - goto out_put;
> > + return LAPB_OK;
> > }
> >
> > lapb_clear_queues(lapb);
> > @@ -328,8 +344,22 @@ int lapb_disconnect_request(struct net_device
> > *dev)
> > lapb_dbg(1, "(%p) S3 DISC(1)\n", lapb->dev);
> > lapb_dbg(0, "(%p) S3 -> S2\n", lapb->dev);
> >
> > - rc = LAPB_OK;
> > -out_put:
> > + return LAPB_OK;
> > +}
Since this is a fix for net, I'd advise against converting the goto
into direct returns (as much as I generally like such conversion).
[GIT PULL] SMB3 Fixes
Please pull the following changes since commit 19c329f6808995b142b3966301f217c831e7cf31: Linux 5.11-rc4 (2021-01-17 16:37:05 -0800) are available in the Git repository at: git://git.samba.org/sfrench/cifs-2.6.git tags/5.11-rc4-smb3 for you to fetch changes up to 214a5ea081e77346e4963dd6d20c5539ff8b6ae6: cifs: do not fail __smb_send_rqst if non-fatal signals are pending (2021-01-23 01:28:20 -0600) an important signal handling patch for stable, and two small cleanup patches Test results: http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/2/builds/485 Jiapeng Zhong (2): fs/cifs: Assign boolean values to a bool variable fs/cifs: Simplify bool comparison. Ronnie Sahlberg (1): cifs: do not fail __smb_send_rqst if non-fatal signals are pending fs/cifs/connect.c | 4 ++-- fs/cifs/transport.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) -- Thanks, Steve
Re:[PATCH 1/1] [Add support Mediatek mt7921U]
From: Sean Wang
Hi Mark
>From: "mark-yw.chen"
>
Please use the appropirate version and prefix for the subject such as
[PATCH v4] Bluetooth: btusb: Add protocol support for MediaTek MT7921U USB
devices
>
>This patch adds the support of enableing MT7921U, it's USB-based Bluetooth
>function.
>
>There is mt7921 firmware download mechanism
>
>1. Read Chip id from MT7921.
>
>2. Download firmware by endpoint 2. (it’s medaitek specific header format for
>downloading firmware.)
>
>3. Enabling Bluetooth function.
Should attach the usb descriptor and endpoints information shown in
/sys/kernel/debug/usb/devices
about the Bluetooth controller.
Signed-off-by tag cannot be dropped and good to have the brief revsion history
put here.
>---
> drivers/bluetooth/btusb.c | 329 --
> 1 file changed, 315 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index
>03b83aa91277..fefbbf467aea 100644
>--- a/drivers/bluetooth/btusb.c
>+++ b/drivers/bluetooth/btusb.c
>@@ -3123,10 +3123,14 @@ static int btusb_shutdown_intel_new(struct hci_dev
>*hdev)
> return 0;
> }
>
>-#define FIRMWARE_MT7663 "mediatek/mt7663pr2h.bin"
>-#define FIRMWARE_MT7668 "mediatek/mt7668pr2h.bin"
Move to a separate patch and then state why you have to do it.
>-
> #define HCI_WMT_MAX_EVENT_SIZE64
>+/* It is for mt79xx download rom patch*/
>+#define FW_ROM_PATCH_HEADER_SIZE 32
>+#define FW_ROM_PATCH_GD_SIZE 64
>+#define FW_ROM_PATCH_SEC_MAP_SIZE 64
>+#define SEC_MAP_COMMON_SIZE 12
>+#define SEC_MAP_NEED_SEND_SIZE52
>+#define PATCH_STATUS 7
Drop the unused macro and have the specific vendor prefix for those macros.
>
> enum {
> BTMTK_WMT_PATCH_DWNLD = 0x1,
>@@ -3138,6 +3142,7 @@ enum {
> enum {
> BTMTK_WMT_INVALID,
> BTMTK_WMT_PATCH_UNDONE,
>+ BTMTK_WMT_PATCH_PROGRESS,
> BTMTK_WMT_PATCH_DONE,
> BTMTK_WMT_ON_UNDONE,
> BTMTK_WMT_ON_DONE,
>@@ -3153,7 +3158,7 @@ struct btmtk_wmt_hdr {
>
> struct btmtk_hci_wmt_cmd {
> struct btmtk_wmt_hdr hdr;
>- u8 data[256];
>+ u8 data[1000];
That seems to break mt7663 as wmt cmd is sent in a form of hci cmd.
> } __packed;
>
> struct btmtk_hci_wmt_evt {
>@@ -3182,6 +3187,82 @@ struct btmtk_hci_wmt_params {
> u32 *status;
> };
>
>+struct btmtk_patch_header {
>+ u8 datetime[16];
>+ u8 platform[4];
>+ u16 hwver;
>+ u16 swver;
__le16
>+ u32 magicnum;
__le32
>+} __packed;
>+
>+struct btmtk_global_desc {
>+ u32 patch_ver;
>+ u32 sub_sys;
>+ u32 feature_opt;
>+ u32 section_num;
__le32
>+} __packed;
>+
>+struct btmtk_section_map {
>+ u32 sectype;
>+ u32 secoffset;
>+ u32 secsize;
__le32
>+ union {
>+ u32 secspec[13];
>+ struct {
>+ u32 dladdr;
>+ u32 dlsize;
>+ u32 seckeyidx;
>+ u32 alignlen;
>+ u32 sectype;
>+ u32 dlmodecrctype;
>+ u32 crc;
>+ u32 reserved[6];
__le32
>+ } bin_info_spec;
>+ };
>+} __packed;
>+
>+struct sk_buff *hci_prepare_acl(struct hci_dev *hdev, u16 opcode, u32 plen,
>+ const void *param)
>+{
>+ int len = HCI_ACL_HDR_SIZE + plen;
>+ struct hci_acl_hdr *hdr;
>+ struct sk_buff *skb;
>+
>+ skb = bt_skb_alloc(len, GFP_ATOMIC);
>+ if (!skb)
>+ return NULL;
>+
>+ hdr = skb_put(skb, HCI_ACL_HDR_SIZE);
>+ hdr->handle = cpu_to_le16(opcode);
>+ hdr->dlen = cpu_to_le16(plen);
>+
>+ if (plen)
>+ skb_put_data(skb, param, plen);
>+
>+ hci_skb_pkt_type(skb) = HCI_ACLDATA_PKT;
>+ hci_skb_opcode(skb) = opcode;
>+
>+ return skb;
>+}
>+
>+int __hci_acl_send(struct hci_dev *hdev, u16 opcode, u32 plen,
>+ const void *param)
>+{
>+ struct sk_buff *skb;
>+
>+ skb = hci_prepare_acl(hdev, opcode, plen, param);
>+
>+ if (!skb) {
>+ bt_dev_err(hdev, "no memory for command (opcode 0x%4.4x)",
>+ opcode);
>+ return -ENOMEM;
>+ }
>+
>+ btusb_send_frame(hdev, skb);
>+
>+ return 0;
>+}
Move to a separate patch for the two new functions and then discuss with
Mercel further if it is worth moving up to the core layer.
>+
> static void btusb_mtk_wmt_recv(struct urb *urb) {
> struct hci_dev *hdev = urb->context;
>@@ -3252,7 +,7 @@ static void btusb_mtk_wmt_recv(struct urb *urb)
>* to generate the event. Otherwise, the WMT event cannot return from
>* the device successfully.
>*/
>- udelay(100);
>+ mdelay(10);
It should break. If i remember correctly that should be in amotic context.
>
> usb_anchor_urb(urb, >ctrl_anchor);
> err = usb_submit_urb(urb, GFP_ATOMIC); @@ -3320,6
Re: [PATCH v3] selftests: drivers: fpga: A test for interrupt support
Tom,
On Sun, Jan 17, 2021 at 08:18:15AM -0800, t...@redhat.com wrote:
> From: Tom Rix
>
> Check that the ioctl DFL_FPGA_PORT_ERR_GET_IRQ_NUM returns
> an expected result.
>
> Tested on vf device 0xbcc1
>
> Sample run with
> # make -C tools/testing/selftests TARGETS=drivers/fpga run_tests
> ...
> TAP version 13
> 1..1
> # selftests: drivers/fpga: intr
> # TAP version 13
> # 1..1
> # # Starting 1 tests from 1 test cases.
> # # RUN global.afu_intr ...
> # #OK global.afu_intr
> # ok 1 global.afu_intr
> # # PASSED: 1 / 1 tests passed.
> # # Totals: pass:1 fail:0 xfail:0 xpass:0 skip:0 error:0
> ok 1 selftests: drivers/fpga: intr
>
> Signed-off-by: Tom Rix
> ---
> v1: Convert to kselftest_harness.h framework
> v2: reverse xmas tree variables
> ---
> MAINTAINERS | 1 +
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/drivers/fpga/Makefile | 7
> tools/testing/selftests/drivers/fpga/config | 1 +
> tools/testing/selftests/drivers/fpga/intr.c | 36 +++
> 5 files changed, 46 insertions(+)
> create mode 100644 tools/testing/selftests/drivers/fpga/Makefile
> create mode 100644 tools/testing/selftests/drivers/fpga/config
> create mode 100644 tools/testing/selftests/drivers/fpga/intr.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index de610a06cb5c..7ed3ce58d95e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6973,6 +6973,7 @@ F: Documentation/driver-api/fpga/
> F: Documentation/fpga/
> F: drivers/fpga/
> F: include/linux/fpga/
> +F: tools/testing/selftests/drivers/fpga/
>
> FPGA SECURITY MANAGER DRIVERS
> M: Russ Weight
> diff --git a/tools/testing/selftests/Makefile
> b/tools/testing/selftests/Makefile
> index afbab4aeef3c..aad4763ec348 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -9,6 +9,7 @@ TARGETS += core
> TARGETS += cpufreq
> TARGETS += cpu-hotplug
> TARGETS += drivers/dma-buf
> +TARGETS += drivers/fpga
> TARGETS += efivarfs
> TARGETS += exec
> TARGETS += filesystems
> diff --git a/tools/testing/selftests/drivers/fpga/Makefile
> b/tools/testing/selftests/drivers/fpga/Makefile
> new file mode 100644
> index ..eba35c405d5b
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/fpga/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +CFLAGS += -I../../../../../usr/include/
> +CFLAGS += -I../../../../../include/uapi/
> +
> +TEST_GEN_PROGS := intr
> +
> +include ../../lib.mk
> diff --git a/tools/testing/selftests/drivers/fpga/config
> b/tools/testing/selftests/drivers/fpga/config
> new file mode 100644
> index ..e2111b81d8d7
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/fpga/config
> @@ -0,0 +1 @@
> +CONFIG_FPGA_DFL_AFU=m
> diff --git a/tools/testing/selftests/drivers/fpga/intr.c
> b/tools/testing/selftests/drivers/fpga/intr.c
> new file mode 100644
> index ..927dcc757f0b
> --- /dev/null
> +++ b/tools/testing/selftests/drivers/fpga/intr.c
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "../../kselftest_harness.h"
> +
> +TEST(afu_intr)
> +{
> + struct dfl_fpga_port_info port_info;
> + uint32_t irq_num = UINT32_MAX;
> + int devfd, status;
> +
> + devfd = open("/dev/dfl-port.0", O_RDONLY);
> + if (devfd < 0)
> + SKIP(0, "no fpga afu device 0");
> + /*
> + * From fpga-dl.h :
> + * Currently hardware supports up to 1 irq.
> + * Return: 0 on success, -errno on failure.
> + */
> + status = ioctl(devfd, DFL_FPGA_PORT_ERR_GET_IRQ_NUM, _num);
> + ASSERT_EQ(0, status) {
> + TH_LOG("ioctl() failed to get the number irqs");
> + }
> + ASSERT_LT(irq_num, 256) {
> + TH_LOG("unexpeced number of irqs");
> + }
> + close(devfd);
> +}
> +
> +TEST_HARNESS_MAIN
> --
> 2.27.0
>
Looks good to me, from FPGA perspective, needs Acked-by from Shua, though.
Also, this does not apply to linux-next, or for-5.12 or char-misc-next,
so I'm confused :)
Once that's sorted, feel free to add
Acked-by: Moritz Fischer
- Moritz
Re: [PATCH] mm/memfd: replace strcpy() by strscpy()
On Sun, Jan 24, 2021 at 10:23:37AM +0800, Stephen Zhang wrote: > - strcpy(name, MFD_NAME_PREFIX); > + strscpy(name, MFD_NAME_PREFIX, len + MFD_NAME_PREFIX_LEN); This is silly. Use memcpy() if you must remove strcpy.
Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
On Sat, 23 Jan 2021 16:56:43 -0800 Enke Chen wrote:
> On Sat, Jan 23, 2021 at 07:19:13PM -0500, Neal Cardwell wrote:
> > On Fri, Jan 22, 2021 at 9:45 PM Enke Chen wrote:
> > > On Fri, Jan 22, 2021 at 06:34:24PM -0800, Jakub Kicinski wrote:
> > > > On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:
> > > > > In terms of backporting, this patch should go together with:
> > > > >
> > > > > 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > > >
> > > > As in it:
> > > >
> > > > Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > > >
> > > > or does it further fix the same issue, so:
> > > >
> > > > Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on
> > > > USER_TIMEOUT")
> > > >
> > > > ?
> > >
> > > Let me clarify:
> > >
> > > 1) 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > >
> > >fixes the bug and makes it work.
> > >
> > > 2) The current patch makes the TCP_USER_TIMEOUT accurate for 0-window
> > > probes.
> > >It's independent.
> >
> > Patch (2) ("tcp: make TCP_USER_TIMEOUT accurate for zero window
> > probes") is indeed conceptually independent of (1) but its
> > implementation depends on the icsk_probes_tstamp field defined in (1),
> > so AFAICT (2) cannot be backported further back than (1).
> >
> > Patch (1) fixes a bug in 5.1:
> > Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on
> > USER_TIMEOUT")
> >
> > So probably (1) and (2) should be backported as a pair, and only back
> > as far as 5.1. (That covers 2 LTS kernels, 5.4 and 5.10, so hopefully
> > that is good enough.)
>
> What you described is more accurate, and is correct.
That makes it clear.
I added a Fixes tag, reworded the message slightly and applied, thanks!
YOU HAVE WON
LOTTO.NL, 2391 Beds 152 Koningin Julianaplein 21, Den Haag-Netherlands. (Lotto affiliate with Subscriber Agents). From: Susan Console (Lottery Coordinator) Website: www.lotto.nl Sir/Madam, CONGRATULATIONS!!! We are pleased to inform you of the result of the Lotto NL Winners International programs held on the 14th of January 2021. Your e-mail address attached to ticket #: 00903228100 with prize # 778009/UK drew €1,000,000.00 which was first in the 2nd class of the draws. you are to receive €1,000,000.00 (One Million Euros). Because of mix up in cash pay-outs, we ask that you keep your winning information confidential until your money (€1,000,000.00) has been fully remitted to you by our accredited pay-point bank. This measure must be adhere to avoid loss of your cash prize-winners of our cash prizes are advised to adhere to these instructions to forestall the abuse of this program by other participants. It's important to note that this draws were conducted formally, and winners are selected through an internet ballot system from 60,000 individual and companies e-mail addresses - the draws are conducted around the world through our internet based ballot system. The promotion is sponsored and promoted Lotto NL. We congratulate you once again. We hope you will use part of it in our next draws; the jackpot winning is €85million. Remember, all winning must be claimed not later than 20 days. After this date all unclaimed cash prize will be forfeited and included in the next sweepstake. Please, in order to avoid unnecessary delays and complications remember to quote personal and winning numbers in all correspondence with us. Congratulations once again from all members of Lotto NL. Thank you for being part of our promotional program. To file for the release of your winnings you are advice to contact our Foreign Transfer Manager: MR. WILSON WARREN JOHNSON Tel: +31-620-561-787 Fax: +31-84-438-5342 Email: johnsonwilson...@gmail.com
Re: [PATCH nf-next v4 1/5] net: sched: Micro-optimize egress handling
On Fri, 22 Jan 2021 09:47:01 +0100 Lukas Wunner wrote:
> sch_handle_egress() returns either the skb or NULL to signal to its
> caller __dev_queue_xmit() whether a packet should continue to be
> processed.
>
> The skb is always non-NULL, otherwise __dev_queue_xmit() would hit a
> NULL pointer deref right at its top.
>
> But the compiler doesn't know that. So if sch_handle_egress() signals
> success by returning the skb, the "if (!skb) goto out;" statement
> results in a gratuitous NULL pointer check in the Assembler output.
Which exact compiler are we talking about it? Did you report this?
As Eric pointed the compiler should be able to figure this out quite
easily.
> Avoid by telling the compiler that __dev_queue_xmit() is never passed a
> NULL skb. This also eliminates another gratuitous NULL pointer check in
> __dev_queue_xmit()
> qdisc_pkt_len_init()
> skb_header_pointer()
> __skb_header_pointer()
>
> The speedup is barely measurable:
> Before: 1877 1875 1878 1874 1882 1873 Mb/sec
> After: 1877 1877 1880 1883 1888 1886 Mb/sec
>
> However we're about to add a netfilter egress hook to __dev_queue_xmit()
> and without the micro-optimization, it will result in a performance
> degradation which is indeed measurable:
> With netfilter hook: 1853 1852 1850 1848 1849 1851 Mb/sec
> With netfilter hook + micro-optim: 1874 1877 1881 1875 1876 1876 Mb/sec
>
> The performance degradation is caused by a JNE instruction ("if (skb)")
> being flipped to a JE instruction ("if (!skb)") once the netfilter hook
> is added. The micro-optimization removes the test and jump instructions
> altogether.
>
> Measurements were performed on a Core i7-3615QM. Reproducer:
> ip link add dev foo type dummy
> ip link set dev foo up
> tc qdisc add dev foo clsact
> tc filter add dev foo egress bpf da bytecode '1,6 0 0 0,'
> modprobe pktgen
> echo "add_device foo" > /proc/net/pktgen/kpktgend_3
> samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh -i foo -n 4 -m
> "11:11:11:11:11:11" -d 1.1.1.1
>
> Signed-off-by: Lukas Wunner
> Cc: John Fastabend
> Cc: Daniel Borkmann
> Cc: Alexei Starovoitov
> Cc: Eric Dumazet
> Cc: Thomas Graf
> ---
> net/core/dev.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 7afbb642e203..4c16b9932823 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4072,6 +4072,7 @@ struct netdev_queue *netdev_core_pick_tx(struct
> net_device *dev,
> * the BH enable code must have IRQs enabled so that it will not
> deadlock.
> * --BLG
> */
> +__attribute__((nonnull(1)))
> static int __dev_queue_xmit(struct sk_buff *skb, struct net_device *sb_dev)
> {
> struct net_device *dev = skb->dev;
Re: [PATCH v1] trace: Fix race in trace_open and buffer resize call
On Sat, 23 Jan 2021 22:03:27 +0530 Gaurav Kohli wrote: > Sure I will do, I have never posted on backport branches. Let me check > and post it. > Basically you take your original patch that was in mainline (as the subject and commit message), and make it work as if you were doing the same exact fix for the stable release. Send it to me (and Cc everyone else), and I'll give it a test too. Thanks! -- Steve
Re: [PATCH v5] gpiolib: Bind gpio_device to a driver to enable fw_devlink=on by default
On Sat, Jan 23, 2021 at 2:52 PM Linus Walleij wrote:
>
> On Fri, Jan 22, 2021 at 8:36 PM Saravana Kannan wrote:
>
> > There are multiple instances of GPIO device tree nodes of the form:
> >
> > foo {
> > compatible = "acme,foo";
> > ...
> >
> > gpio0: gpio0@ {
> > compatible = "acme,bar";
> > ...
> > gpio-controller;
> > };
> >
> > gpio1: gpio1@ {
> > compatible = "acme,bar";
> > ...
> > gpio-controller;
> > };
> >
> > ...
> > }
> >
> > bazz {
> > my-gpios = < ...>;
> > }
> >
> > Case 1: The driver for "foo" populates struct device for these gpio*
> > nodes and then probes them using a driver that binds with "acme,bar".
> > This driver for "acme,bar" then registers the gpio* nodes with gpiolib.
> > This lines up with how DT nodes with the "compatible" property are
> > typically converted to struct devices and then registered with driver
> > core to probe them. This also allows the gpio* devices to hook into all
> > the driver core capabilities like runtime PM, probe deferral,
> > suspend/resume ordering, device links, etc.
> >
> > Case 2: The driver for "foo" doesn't populate struct devices for these
> > gpio* nodes before registering them with gpiolib. Instead it just loops
> > through its child nodes and directly registers the gpio* nodes with
> > gpiolib.
> >
> > Drivers that follow case 2 cause problems with fw_devlink=on. This is
> > because fw_devlink will prevent bazz from probing until there's a struct
> > device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO
> > supplier). Once the struct device is available, fw_devlink will create a
> > device link with gpio0 device as the supplier and bazz device as the
> > consumer. After this point, since the gpio0 device will never bind to a
> > driver, the device link will prevent bazz device from ever probing.
> >
> > Finding and refactoring all the instances of drivers that follow case 2
> > will cause a lot of code churn and it is not something that can be done
> > in one shot. In some instances it might not even be possible to refactor
> > them cleanly. Examples of such instances are [1] [2].
> >
> > This patch works around this problem and avoids all the code churn by
> > simply setting the fwnode of the gpio_device and creating a stub driver
> > to bind to the gpio_device. This allows all the consumers to continue
> > probing when the driver follows case 2.
> >
> > [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/
> > [2] -
> > https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba...@kernel.org/
> > Cc: Marc Zyngier
> > Cc: Jisheng Zhang
> > Cc: Kever Yang
> > Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> > Signed-off-by: Saravana Kannan
>
> This v5 version is a beauty!
Lol, thanks.
> Reviewed-by: Linus Walleij
Considering the "Fixes" is only in driver-core-next, should this go
through driver-core?
-Saravana
[PATCH] hwmon: (dell-smm) Add XPS 15 L502X to fan control blacklist
It has been reported[0] that the Dell XPS 15 L502X exhibits similar
freezing behavior to the other systems[1] on this blacklist. The issue
was exposed by a prior change of mine to automatically load
dell_smm_hwmon on a wider set of XPS models. To fix the regression, add
this model to the blacklist.
[0] https://bugzilla.kernel.org/show_bug.cgi?id=211081
[1] https://bugzilla.kernel.org/show_bug.cgi?id=195751
Fixes: b8a13e5e8f37 ("hwmon: (dell-smm) Use one DMI match for all XPS models")
Cc: sta...@vger.kernel.org
Reported-by: Bob Hepple
Tested-by: Bob Hepple
Signed-off-by: Thomas Hebb
---
drivers/hwmon/dell-smm-hwmon.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index ec448f5f2dc3..73b9db9e3aab 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -1159,6 +1159,13 @@ static struct dmi_system_id
i8k_blacklist_fan_support_dmi_table[] __initdata = {
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "XPS13 9333"),
},
},
+ {
+ .ident = "Dell XPS 15 L502X",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Dell System XPS
L502X"),
+ },
+ },
{ }
};
--
2.30.0
[PATCH] mm/memfd: replace strcpy() by strscpy()
The strcpy() function is being deprecated. Replace it by the safer
strscpy().
Signed-off-by: Stephen Zhang
---
mm/memfd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/memfd.c b/mm/memfd.c
index 2647c89..bb90862 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -278,7 +278,7 @@ long memfd_fcntl(struct file *file, unsigned int cmd,
unsigned long arg)
if (!name)
return -ENOMEM;
- strcpy(name, MFD_NAME_PREFIX);
+ strscpy(name, MFD_NAME_PREFIX, len + MFD_NAME_PREFIX_LEN);
if (copy_from_user([MFD_NAME_PREFIX_LEN], uname, len)) {
error = -EFAULT;
goto err_name;
--
1.8.3.1
回复: 回复: [PATCH] rcu: Release per-cpu krcp page cache when CPU going offline
发件人: Uladzislau Rezki 发送时间: 2021年1月22日 22:31 收件人: Zhang, Qiang 抄送: Uladzislau Rezki; Paul E. McKenney; r...@vger.kernel.org; linux-kernel@vger.kernel.org 主题: Re: 回复: [PATCH] rcu: Release per-cpu krcp page cache when CPU going offline On Fri, Jan 22, 2021 at 01:44:36AM +, Zhang, Qiang wrote: > > > > 发件人: Uladzislau Rezki > 发送时间: 2021年1月22日 4:26 > 收件人: Zhang, Qiang > 抄送: Paul E. McKenney; r...@vger.kernel.org; linux-kernel@vger.kernel.org; > ure...@gmail.com > 主题: Re: [PATCH] rcu: Release per-cpu krcp page cache when CPU going offline > >Hello, Qiang, > > > On Thu, Jan 21, 2021 at 02:49:49PM +0800, qiang.zh...@windriver.com wrote: > > > From: Zqiang > > > > > > If CPUs go offline, the corresponding krcp's page cache can > > > not be use util the CPU come back online, or maybe the CPU > > > will never go online again, this commit therefore free krcp's > > > page cache when CPUs go offline. > > > > > > Signed-off-by: Zqiang > > > >Do you consider it as an issue? We have 5 pages per CPU, that is 20480 bytes. > > > > Hello Rezki > > In a multi CPUs system, more than one CPUs may be offline, there are more > than 5 pages, and these offline CPUs may never go online again or in the > process of CPUs online, there are errors, which lead to the failure of > online, these scenarios will lead to the per-cpu krc page cache will never be > released. > >Thanks for your answer. I was thinking more about if you knew some >platforms >which suffer from such extra page usage when CPU goes offline. Any >issues >your platforms or devices run into because of that. > >So i understand that if CPU goes offline the 5 pages associated with it >are >unused until it goes online back. I agree with you, But I still want to talk about what I think My understanding is that when the CPU is offline, the pages is not accessible, beacuse we don't know when this CPU will go online again, so we best to return these page to the buddy system, when the CPU goes online again, we can allocate page from the buddy system to fill krcp's page cache. maybe you may think that this memory is small and don't need to. Thanks Qiang > >-- >Vlad Rezki
Re: [RFC PATCH] io_uring: add support for IORING_OP_GETDENTS64
Hi, On 2021-01-24 01:59:05 +, Al Viro wrote: > On Sat, Jan 23, 2021 at 03:50:55PM -0800, Andres Freund wrote: > > > As there's only a shared lock, seems like both would end up with the > > same ctx->pos and end up updating f_pos to the same offset (assuming the > > same count). > > > > Am I missing something? > > This: > f = fdget_pos(fd); > if (!f.file) > return -EBADF; > in the callers. Ah. Thanks for the explainer, userspace guy here ;). I hadn't realized that fdget_pos acquired a lock around the position... Regards, Andres
Re: [PATCH v2 2/2] memblock: do not start bottom-up allocations with kernel_end
On Fri, 22 Jan 2021 01:37:14 -0300 Thiago Jung Bauermann
wrote:
> Mike Rapoport writes:
>
> > > Signed-off-by: Roman Gushchin
> >
> > Reviewed-by: Mike Rapoport
>
> I've seen a couple of spurious triggers of the WARN_ONCE() removed by this
> patch. This happens on some ppc64le bare metal (powernv) server machines with
> CONFIG_SWIOTLB=y and crashkernel=4G, as described in a candidate patch I
> posted
> to solve this issue in a different way:
>
> https://lore.kernel.org/linuxppc-dev/20201218062103.76102-1-bauer...@linux.ibm.com/
>
> Since this patch solves that problem, is it possible to include it in the next
> feasible v5.11-rcX, with the following tag?
We could do this, if we're confident that this patch doesn't depend on
[1/2] "mm: cma: allocate cma areas bottom-up"? I think it is...
> Fixes: 8fabc623238e ("powerpc: Ensure that swiotlb buffer is allocated from
> low memory")
I added that.
[ANNOUNCE] 4.9.253-rt168
Hello RT-list! I'm pleased to announce the 4.9.253-rt168 stable release. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v4.9-rt Head SHA1: 39c70c2a2244ca70ab61d2442b3c4136ed40d8ee Or to build 4.9.253-rt168 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.9.tar.xz https://www.kernel.org/pub/linux/kernel/v4.x/patch-4.9.253.xz https://www.kernel.org/pub/linux/kernel/projects/rt/4.9/patch-4.9.253-rt168.patch.xz Enjoy! Luis
Re: [PATCH] mm: Fix potential pte_unmap_unlock pte error
On Fri, 22 Jan 2021 16:27:23 +0800 Miaohe Lin wrote:
> Hi Andrew:
> On 2021/1/14 10:51, Miaohe Lin wrote:
> > Hi:
> > On 2021/1/11 1:14, Andi Kleen wrote:
> >> On Sat, Jan 09, 2021 at 03:01:18AM -0500, Miaohe Lin wrote:
> >>> Since commit 42e4089c7890 ("x86/speculation/l1tf: Disallow non privileged
> >>> high MMIO PROT_NONE mappings"), when the first pfn modify is not allowed,
> >>> we would break the loop with pte unchanged. Then the wrong pte - 1 would
> >>> be passed to pte_unmap_unlock.
> >>
> >> Thanks.
> >>
> >> While the fix is correct, I'm not sure if it actually is a real bug. Is
> >> there
> >> any architecture that would do something else than unlocking the underlying
> >> page? If it's just the underlying page then it should be always the same
> >> page, so no bug.
> >>
> >
> > It's just a theoretical issue via code inspection.
>
> Should I send a new one without Cc statle or just drop this patch? Thanks.
Your patch makes the code much less scary looking. I added Andi's
observation to the changelog, removed the cc:stable and queued it up,
thanks.
Re: [RFC PATCH] io_uring: add support for IORING_OP_GETDENTS64
On Sat, Jan 23, 2021 at 03:50:55PM -0800, Andres Freund wrote: > As there's only a shared lock, seems like both would end up with the > same ctx->pos and end up updating f_pos to the same offset (assuming the > same count). > > Am I missing something? This: f = fdget_pos(fd); if (!f.file) return -EBADF; in the callers. Protection of struct file contents belongs to struct file, *not* struct inode. Specifically, file->f_pos_lock. *IF* struct file in question happens to be shared and the file is a regular or directory (sockets don't need any exclusion on read(2), etc.)
Re: [PATCH v3 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
On 1/23/21 1:00 PM, Jakub Kicinski wrote: > On Fri, 22 Jan 2021 22:16:41 -0700 David Ahern wrote: >> On 1/22/21 9:02 PM, Maciej Żenczykowski wrote: >>> Why can't we get rid of the special case for 0 and simply make 1024 the >>> default value? >> >> That would work too. > > Should we drop it then? Easier to bring it back than to change the > interpretation later. It doesn't seem to serve any clear purpose right > now. > > (Praveen if you post v4 please take a look at the checkpatch --strict > warnings and address the ones which make sense, e.g. drop the brackets > around comparisons, those are just noise, basic grasp of C operator > precedence can be assumed in readers of kernel code). > let's do a v4. Praveen: set the initial value to IP6_RT_PRIO_USER, do not allow 0, remove the checks on value and don't forget to update documentation. Oh and cc me on the next otherwise the review depends on me finding time to scan netdev.
Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
Hi, Neal:
What you described is more accurate, and is correct.
Thanks. -- Enke
On Sat, Jan 23, 2021 at 07:19:13PM -0500, Neal Cardwell wrote:
> On Fri, Jan 22, 2021 at 9:45 PM Enke Chen wrote:
> >
> > Hi, Jakub:
> >
> > On Fri, Jan 22, 2021 at 06:34:24PM -0800, Jakub Kicinski wrote:
> > > On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:
> > > > Hi, Jakub:
> > > >
> > > > In terms of backporting, this patch should go together with:
> > > >
> > > > 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > >
> > > As in it:
> > >
> > > Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> > >
> > > or does it further fix the same issue, so:
> > >
> > > Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on
> > > USER_TIMEOUT")
> > >
> > > ?
> >
> > Let me clarify:
> >
> > 1) 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> >
> >fixes the bug and makes it work.
> >
> > 2) The current patch makes the TCP_USER_TIMEOUT accurate for 0-window
> > probes.
> >It's independent.
>
> Patch (2) ("tcp: make TCP_USER_TIMEOUT accurate for zero window
> probes") is indeed conceptually independent of (1) but its
> implementation depends on the icsk_probes_tstamp field defined in (1),
> so AFAICT (2) cannot be backported further back than (1).
>
> Patch (1) fixes a bug in 5.1:
> Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on
> USER_TIMEOUT")
>
> So probably (1) and (2) should be backported as a pair, and only back
> as far as 5.1. (That covers 2 LTS kernels, 5.4 and 5.10, so hopefully
> that is good enough.)
>
> neal
[PATCH] USB: usblp: don't call usb_set_interface if there's a single alt
Some devices, such as the Winbond Electronics Corp. Virtual Com Port
(Vendor=0416, ProdId=5011), lockup when usb_set_interface() or
usb_clear_halt() are called. This device has only a single
altsetting, so it should not be necessary to call usb_set_interface().
Signed-off-by: Jeremy Figgins
---
An alternative fix was submitted previously as:
USB: usblp: add USBLP_QUIRK_NO_SETF_INTF flag
and this superscedes that patch. This is a different approach, so
I submitted it as a new patch, rather than adding a v2 to the first
one; I hope that was the correct thing to do in this case.
drivers/usb/class/usblp.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/class/usblp.c b/drivers/usb/class/usblp.c
index 134dc2005ce9..c9f6e9758288 100644
--- a/drivers/usb/class/usblp.c
+++ b/drivers/usb/class/usblp.c
@@ -1329,14 +1329,17 @@ static int usblp_set_protocol(struct usblp *usblp, int
protocol)
if (protocol < USBLP_FIRST_PROTOCOL || protocol > USBLP_LAST_PROTOCOL)
return -EINVAL;
- alts = usblp->protocol[protocol].alt_setting;
- if (alts < 0)
- return -EINVAL;
- r = usb_set_interface(usblp->dev, usblp->ifnum, alts);
- if (r < 0) {
- printk(KERN_ERR "usblp: can't set desired altsetting %d on
interface %d\n",
- alts, usblp->ifnum);
- return r;
+ /* Don't unnecessarily set the interface if there's a single alt. */
+ if (usblp->intf->num_altsetting > 1) {
+ alts = usblp->protocol[protocol].alt_setting;
+ if (alts < 0)
+ return -EINVAL;
+ r = usb_set_interface(usblp->dev, usblp->ifnum, alts);
+ if (r < 0) {
+ printk(KERN_ERR "usblp: can't set desired altsetting %d
on interface %d\n",
+ alts, usblp->ifnum);
+ return r;
+ }
}
usblp->bidir = (usblp->protocol[protocol].epread != NULL);
--
2.29.0
Re: [PATCH net] tcp: make TCP_USER_TIMEOUT accurate for zero window probes
On Fri, Jan 22, 2021 at 9:45 PM Enke Chen wrote:
>
> Hi, Jakub:
>
> On Fri, Jan 22, 2021 at 06:34:24PM -0800, Jakub Kicinski wrote:
> > On Fri, 22 Jan 2021 18:28:23 -0800 Enke Chen wrote:
> > > Hi, Jakub:
> > >
> > > In terms of backporting, this patch should go together with:
> > >
> > > 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> >
> > As in it:
> >
> > Fixes: 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
> >
> > or does it further fix the same issue, so:
> >
> > Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
> >
> > ?
>
> Let me clarify:
>
> 1) 9d9b1ee0b2d1 tcp: fix TCP_USER_TIMEOUT with zero window
>
>fixes the bug and makes it work.
>
> 2) The current patch makes the TCP_USER_TIMEOUT accurate for 0-window probes.
>It's independent.
Patch (2) ("tcp: make TCP_USER_TIMEOUT accurate for zero window
probes") is indeed conceptually independent of (1) but its
implementation depends on the icsk_probes_tstamp field defined in (1),
so AFAICT (2) cannot be backported further back than (1).
Patch (1) fixes a bug in 5.1:
Fixes: 9721e709fa68 ("tcp: simplify window probe aborting on USER_TIMEOUT")
So probably (1) and (2) should be backported as a pair, and only back
as far as 5.1. (That covers 2 LTS kernels, 5.4 and 5.10, so hopefully
that is good enough.)
neal
Re: [PATCH 4/4] perf tools: determine if LR is the return address
On Fri, Jan 22, 2021 at 04:18:54PM +, Alexandre Truong wrote:
> On arm64 and frame pointer mode (e.g: perf record --callgraph fp),
> use dwarf unwind info to check if the link register is the return
> address in order to inject it to the frame pointer stack.
>
> Write the following application:
>
> int a = 10;
>
> void f2(void)
> {
> for (int i = 0; i < 100; i++)
> a *= a;
> }
>
> void f1()
> {
> f2();
> }
>
> int main (void)
> {
> f1();
> return 0;
> }
>
> with the following compilation flags:
> gcc -g -fno-omit-frame-pointer -fno-inline -O1
>
> The compiler omits the frame pointer for f2 on arm. This is a problem
> with any leaf call, for example an application with many different
> calls to malloc() would always omit the calling frame, even if it
> can be determined.
>
> ./perf record --call-graph fp ./a.out
> ./perf report
>
> currently gives the following stack:
>
> 0xea52f361
> _start
> __libc_start_main
> main
> f2
reproduced on x86 as well
> +static bool get_leaf_frame_caller_enabled(struct perf_sample *sample)
> +{
> + return callchain_param.record_mode != CALLCHAIN_FP ||
> !sample->user_regs.regs
> + || sample->user_regs.mask != PERF_REGS_MASK;
> +}
> +
> +static int add_entry(struct unwind_entry *entry, void *arg)
> +{
> + struct entries *entries = arg;
> +
> + entries->stack[entries->i++] = entry->ip;
> + return 0;
> +}
> +
> +u64 get_leaf_frame_caller_aarch64(struct perf_sample *sample, struct thread
> *thread)
> +{
> + u64 leaf_frame;
> + struct entries entries = {{0, 0}, 0};
> +
> + if (get_leaf_frame_caller_enabled(sample))
the name suggest you'd want to continue if it's true
> + return 0;
> +
> + unwind__get_entries(add_entry, , thread, sample, 2);
I'm scratching my head how this unwinds anything, you enabled just
registers, not the stack right? so the unwind code would do just
IP -> LR + 1 shift?
thanks,
jirka
> + leaf_frame = callchain_param.order == ORDER_CALLER ?
> + entries.stack[0] : entries.stack[1];
> +
> + if (leaf_frame + 1 == sample->user_regs.regs[PERF_REG_ARM64_LR])
> + return sample->user_regs.regs[PERF_REG_ARM64_LR];
> + return 0;
> +}
SNIP
Re: [RFC PATCH] io_uring: add support for IORING_OP_GETDENTS64
Hi, On 2021-01-23 15:50:55 -0800, Andres Freund wrote: > It's also not clear to me that right now you'd necessarily get correct > results if multiple IORING_OP_GETDENTS64 for the same fd get processed > in different workers. Looking at iterate_dir(), it looks to me that the > locking around the file position would end up being insufficient on > filesystems that implement iterate_shared? > [...] > As there's only a shared lock, seems like both would end up with the > same ctx->pos and end up updating f_pos to the same offset (assuming the > same count). > > Am I missing something? A minimal and brute force approach to this would be to use io_op_def.hash_reg_file, but brrr, that doesn't seem like a great way forward. Greetings, Andres Freund
Re: [PATCH v1 0/3] Support building gpio-tegra driver as loadable module
24.01.2021 01:50, Linus Walleij пишет: > On Fri, Jan 22, 2021 at 7:59 PM Dmitry Osipenko wrote: > >> This small series adds modularization support to the gpio-tegra driver, >> i.e. driver now could be built as a loadable kernel module. >> >> Dmitry Osipenko (3): >> gpio: tegra: Use debugfs_create_devm_seqfile() >> gpio: tegra: Clean up whitespaces in tegra_gpio_driver >> gpio: tegra: Support building driver as a loadable module > > As these three patches clearly make the kernel look better after > than before: > Reviewed-by: Linus Walleij > > However when we are doing this I would strongly encourage you > to also make a patch implementing remove() so you can insmod > rmmod the module at runtime. The remove() is optional for drivers, it doesn't prevent the rmmod.
Re: [RFC PATCH] io_uring: add support for IORING_OP_GETDENTS64
Hi,
On 2021-01-23 13:41:52 +0200, Lennert Buytenhek wrote:
> IORING_OP_GETDENTS64 behaves like getdents64(2) and takes the same
> arguments.
I've wished for this before, this would be awesome.
> One open question is whether IORING_OP_GETDENTS64 should be more like
> pread(2) and allow passing in a starting offset to read from the
> directory from. (This would require some more surgery in fs/readdir.c.)
That would imo be preferrable from my end - using the fd's position
means that the fd cannot be shared between threads etc.
It's also not clear to me that right now you'd necessarily get correct
results if multiple IORING_OP_GETDENTS64 for the same fd get processed
in different workers. Looking at iterate_dir(), it looks to me that the
locking around the file position would end up being insufficient on
filesystems that implement iterate_shared?
int iterate_dir(struct file *file, struct dir_context *ctx)
{
struct inode *inode = file_inode(file);
bool shared = false;
int res = -ENOTDIR;
if (file->f_op->iterate_shared)
shared = true;
else if (!file->f_op->iterate)
goto out;
res = security_file_permission(file, MAY_READ);
if (res)
goto out;
if (shared)
res = down_read_killable(>i_rwsem);
else
res = down_write_killable(>i_rwsem);
if (res)
goto out;
res = -ENOENT;
if (!IS_DEADDIR(inode)) {
ctx->pos = file->f_pos;
if (shared)
res = file->f_op->iterate_shared(file, ctx);
else
res = file->f_op->iterate(file, ctx);
file->f_pos = ctx->pos;
fsnotify_access(file);
file_accessed(file);
}
if (shared)
inode_unlock_shared(inode);
else
inode_unlock(inode);
out:
return res;
}
As there's only a shared lock, seems like both would end up with the
same ctx->pos and end up updating f_pos to the same offset (assuming the
same count).
Am I missing something?
Greetings,
Andres Freund
[PATCH 2/2] hrtimer: Use pr_warn_deferred_once() in hrtimer_interrupt()
printk is not allowed in this context and causes a BUG: Invalid wait context.
Signed-off-by: Qais Yousef
---
kernel/time/hrtimer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 743c852e10f2..2d9b7cf1d5e2 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -1700,7 +1700,8 @@ void hrtimer_interrupt(struct clock_event_device *dev)
else
expires_next = ktime_add(now, delta);
tick_program_event(expires_next, 1);
- pr_warn_once("hrtimer: interrupt took %llu ns\n", ktime_to_ns(delta));
+ pr_warn_deferred_once("hrtimer: interrupt took %llu ns\n",
+ ktime_to_ns(delta));
}
/* called with interrupts disabled */
--
2.25.1
Re: Infinite recursion in device_reorder_to_tail() due to circular device links
On Tue, 12 Jan 2021, Greg Kroah-Hartman wrote:
> On Tue, Jan 12, 2021 at 03:32:04PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Jan 11, 2021 at 7:46 PM Stephan Gerhold wrote:
> > >
> > > Hi,
> > >
> > > since 5.11-rc1 I get kernel crashes with infinite recursion in
> > > device_reorder_to_tail() in some situations... It's a bit complicated to
> > > explain so I want to apologize in advance for the long mail. :)
> > >
> > > Kernel panic - not syncing: kernel stack overflow
> > > CPU: 1 PID: 33 Comm: kworker/1:1 Not tainted 5.11.0-rc3 #1
> > > Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
> > > Call trace:
> > >...
> > >device_reorder_to_tail+0x4c/0xf0
> > >device_reorder_to_tail+0x98/0xf0
> > >device_reorder_to_tail+0x60/0xf0
> > >device_reorder_to_tail+0x60/0xf0
> > >device_reorder_to_tail+0x60/0xf0
> > >...
> > >
> > > The crash happens only in 5.11 with commit 5b6164d3465f ("driver core:
> > > Reorder devices on successful probe"). It stops happening when I revert
> > > this commit.
> >
> > Thanks for the report!
> >
> > Greg, please revert commit 5b6164d3465f, it clearly is not an
> > improvement, at least at this point.
>
> Now reverted, thanks.
>
> greg k-h
I think that there has been a misunderstanding here: although
5b6164d3465f ("driver core: Reorder devices on successful probe")
has been reverted from linux-next (thank you), it has not yet been
reverted from 5.11-rc, and still causing problems there (in my case,
not the infinite recursion Stephan reported in this thread, but the
ThinkPad rmi4 suspend failure that I reported in another thread).
Thanks,
Hugh
[PATCH 0/2] Fix BUG: Invalid wait context in hrtimer_interrupt()
I hit a pr_warn() inside hrtimer_interrupt() which lead to a BUG: Invalid wait context splat. The problem wasn't reproducible but I think the cause is obvious, printk can't be called from interrupt context. AFAICU printk_deferred() is safe from NMI, so I assumed it is safe to be called from hrtimer_interrupt() too. Adding a pr_warn_once() inside hrtimer_interrupt() in a location where it is always hit produces the BUG splat. Replacing it with pr_warn_deferred_once() generates the printk warning without any splat. I added a new pr_*_deferred_once() variants to avoid open coding; but the name ended not much shorter and I'm not sure if the wrappers are a win overall. Since I've already done it, I'm sticking to it in this post. But will be happy to drop it and just open code the printk_deferred_once(KERN_WARN, ...) in hrtimer_interrupt() instead. Thanks Qais Yousef (2): printk: Add new pr_*_deferred_once() variants hrtimer: Use pr_warn_deferred_once() in hrtimer_interrupt() include/linux/printk.h | 24 kernel/time/hrtimer.c | 3 ++- 2 files changed, 26 insertions(+), 1 deletion(-) -- 2.25.1
[PATCH 1/2] printk: Add new pr_*_deferred_once() variants
To allow users in code where printk is not allowed. Signed-off-by: Qais Yousef --- include/linux/printk.h | 24 1 file changed, 24 insertions(+) diff --git a/include/linux/printk.h b/include/linux/printk.h index fe7eb2351610..92c0978c7b44 100644 --- a/include/linux/printk.h +++ b/include/linux/printk.h @@ -480,21 +480,45 @@ extern int kptr_restrict; printk_once(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) /* no pr_cont_once, don't do that... */ +#define pr_emerg_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__) +#define pr_alert_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__) +#define pr_crit_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__) +#define pr_err_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__) +#define pr_warn_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__) +#define pr_notice_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__) +#define pr_info_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) +/* no pr_cont_deferred_once, don't do that... */ + #if defined(DEBUG) #define pr_devel_once(fmt, ...)\ printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) +#define pr_devel_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) #else #define pr_devel_once(fmt, ...)\ no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) +#define pr_devel_deferred_once(fmt, ...) \ + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) #endif /* If you are writing a driver, please use dev_dbg instead */ #if defined(DEBUG) #define pr_debug_once(fmt, ...)\ printk_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) +#define pr_debug_deferred_once(fmt, ...) \ + printk_deferred_once(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) #else #define pr_debug_once(fmt, ...)\ no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) +#define pr_debug_deferred_once(fmt, ...) \ + no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) #endif /* -- 2.25.1
Re: [PATCH 1/3] drivers: firmware: Add Pdi load API support
On Mon, Jan 18, 2021 at 08:13:16AM +0530, Nava kishore Manne wrote:
> This patch adds load pdi api support to enable pdi/partial loading from
> linux. Programmable Device Image (PDI) is combination of headers, images
> and bitstream files to be loaded. Partial PDI is partial set of image/
> images to be loaded.
>
> Signed-off-by: Nava kishore Manne
> ---
> drivers/firmware/xilinx/zynqmp.c | 17 +
> include/linux/firmware/xlnx-zynqmp.h | 9 +
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/firmware/xilinx/zynqmp.c
> b/drivers/firmware/xilinx/zynqmp.c
> index 7eb9958662dd..a466225b9f9e 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -897,6 +897,23 @@ int zynqmp_pm_set_requirement(const u32 node, const u32
> capabilities,
> }
> EXPORT_SYMBOL_GPL(zynqmp_pm_set_requirement);
>
> +/**
> + * zynqmp_pm_load_pdi - Load and process pdi
Nit: Is it pdi or PDI. Pick one :)
> + * @src: Source device where PDI is located
> + * @address: Pdi src address
> + *
> + * This function provides support to load pdi from linux
> + *
> + * Return: Returns status, either success or error+reason
> + */
> +int zynqmp_pm_load_pdi(const u32 src, const u64 address)
> +{
> + return zynqmp_pm_invoke_fn(PM_LOAD_PDI, src,
> +lower_32_bits(address),
> +upper_32_bits(address), 0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(zynqmp_pm_load_pdi);
> +
> /**
> * zynqmp_pm_aes - Access AES hardware to encrypt/decrypt the data using
> * AES-GCM core.
> diff --git a/include/linux/firmware/xlnx-zynqmp.h
> b/include/linux/firmware/xlnx-zynqmp.h
> index 2a0da841c942..87114ee645b1 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -52,6 +52,9 @@
> #define ZYNQMP_PM_CAPABILITY_WAKEUP 0x4U
> #define ZYNQMP_PM_CAPABILITY_UNUSABLE 0x8U
>
> +/* Loader commands */
> +#define PM_LOAD_PDI 0x701
> +
> /*
> * Firmware FPGA Manager flags
> * XILINX_ZYNQMP_PM_FPGA_FULL: FPGA full reconfiguration
> @@ -354,6 +357,7 @@ int zynqmp_pm_write_pggs(u32 index, u32 value);
> int zynqmp_pm_read_pggs(u32 index, u32 *value);
> int zynqmp_pm_system_shutdown(const u32 type, const u32 subtype);
> int zynqmp_pm_set_boot_health_status(u32 value);
> +int zynqmp_pm_load_pdi(const u32 src, const u64 address);
> #else
> static inline struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
> {
> @@ -538,6 +542,11 @@ static inline int zynqmp_pm_set_boot_health_status(u32
> value)
> {
> return -ENODEV;
> }
> +
> +static inline int zynqmp_pm_load_pdi(const u32 src, const u64 address)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif /* __FIRMWARE_ZYNQMP_H__ */
> --
> 2.18.0
>
Re: [PATCH 3/3] fpga: versal-fpga: Add versal fpga manager driver
Hi Nava,
On Fri, Jan 22, 2021 at 10:34:15AM +, Nava kishore Manne wrote:
> Hi Moritz,
>
> Thanks for the review.
> Please find my response inline.
>
> > -Original Message-
> > From: Moritz Fischer
> > Sent: Tuesday, January 19, 2021 6:03 AM
> > To: Nava kishore Manne
> > Cc: m...@kernel.org; t...@redhat.com; robh...@kernel.org; Michal Simek
> > ; linux-f...@vger.kernel.org;
> > devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> > ker...@vger.kernel.org; git ; chinnikishore...@gmail.com;
> > Appana Durga Kedareswara Rao
> > Subject: Re: [PATCH 3/3] fpga: versal-fpga: Add versal fpga manager driver
> >
> > Hi Nava,
> >
> > On Mon, Jan 18, 2021 at 08:13:18AM +0530, Nava kishore Manne wrote:
> > > This patch adds driver for versal fpga manager.
> > Nit: Add support for Xilinx Versal FPGA manager
>
> Will fix in v2.
>
> > >
> > > PDI source type can be DDR, OCM, QSPI flash etc..
> > No idea what PDI is :)
>
> Programmable device image (PDI).
> This file is generated by Xilinx Vivado tool and it contains configuration
> data objects.
>
> > > But driver allocates memory always from DDR, Since driver supports
> > > only DDR source type.
> > >
> > > Signed-off-by: Appana Durga Kedareswara rao
> > >
> > > Signed-off-by: Nava kishore Manne
> > > ---
> > > drivers/fpga/Kconfig | 8 ++
> > > drivers/fpga/Makefile | 1 +
> > > drivers/fpga/versal-fpga.c | 149
> > > +
> > > 3 files changed, 158 insertions(+)
> > > create mode 100644 drivers/fpga/versal-fpga.c
> > >
> > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index
> > > 5645226ca3ce..9f779c3a6739 100644
> > > --- a/drivers/fpga/Kconfig
> > > +++ b/drivers/fpga/Kconfig
> > > @@ -216,4 +216,12 @@ config FPGA_MGR_ZYNQMP_FPGA
> > > to configure the programmable logic(PL) through PS
> > > on ZynqMP SoC.
> > >
> > > +config FPGA_MGR_VERSAL_FPGA
> > > +tristate "Xilinx Versal FPGA"
> > > +depends on ARCH_ZYNQMP || COMPILE_TEST
> > > +help
> > > + Select this option to enable FPGA manager driver support for
> > > + Xilinx Versal SOC. This driver uses the versal soc firmware
> > > + interface to load programmable logic(PL) images
> > > + on versal soc.
> > > endif # FPGA
> > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile index
> > > d8e21dfc6778..40c9adb6a644 100644
> > > --- a/drivers/fpga/Makefile
> > > +++ b/drivers/fpga/Makefile
> > > @@ -18,6 +18,7 @@ obj-$(CONFIG_FPGA_MGR_TS73XX) +=
> > ts73xx-fpga.o
> > > obj-$(CONFIG_FPGA_MGR_XILINX_SPI)+= xilinx-spi.o
> > > obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o
> > > obj-$(CONFIG_FPGA_MGR_ZYNQMP_FPGA) += zynqmp-fpga.o
> > > +obj-$(CONFIG_FPGA_MGR_VERSAL_FPGA) += versal-fpga.o
> > > obj-$(CONFIG_ALTERA_PR_IP_CORE) += altera-pr-ip-core.o
> > > obj-$(CONFIG_ALTERA_PR_IP_CORE_PLAT)+= altera-pr-ip-core-plat.o
> > >
> > > diff --git a/drivers/fpga/versal-fpga.c b/drivers/fpga/versal-fpga.c
> > > new file mode 100644 index ..2a42aa78b182
> > > --- /dev/null
> > > +++ b/drivers/fpga/versal-fpga.c
> > > @@ -0,0 +1,149 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2021 Xilinx, Inc.
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +/* Constant Definitions */
> > > +#define PDI_SOURCE_TYPE 0xF
> > > +
> > > +/**
> > > + * struct versal_fpga_priv - Private data structure
> > > + * @dev: Device data structure
> > > + * @flags: flags which is used to identify the PL Image type
> > > + */
> > > +struct versal_fpga_priv {
> > > + struct device *dev;
> > > + u32 flags;
> > This seems unused ... please introduce them when/if you start using them.
>
> Will fix in v2.
>
> > > +};
> > > +
> > > +static int versal_fpga_ops_write_init(struct fpga_manager *mgr,
> > > + struct fpga_image_info *info,
> > > + const char *buf, size_t size) {
> > > + struct versal_fpga_priv *priv;
> > > +
> > > + priv = mgr->priv;
> > > + priv->flags = info->flags;
> > ? What uses this ? It seems this function could just be 'return 0' right
> > now.
>
> Will fix in v2.
>
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int versal_fpga_ops_write(struct fpga_manager *mgr,
> > > + const char *buf, size_t size)
> > > +{
> > > + struct versal_fpga_priv *priv;
> > > + dma_addr_t dma_addr = 0;
> > > + char *kbuf;
> > > + int ret;
> > > +
> > > + priv = mgr->priv;
> > > +
> > > + kbuf = dma_alloc_coherent(priv->dev, size, _addr,
> > GFP_KERNEL);
> > > + if (!kbuf)
> > > + return -ENOMEM;
> > > +
> > > + memcpy(kbuf, buf, size);
> > > +
> > > + wmb(); /* ensure all writes are done before initiate FW call */
> > > +
> > > + ret =
Re: [RFC PATCH] io_uring: add support for IORING_OP_GETDENTS64
On 1/23/21 4:27 PM, Matthew Wilcox wrote: > On Sat, Jan 23, 2021 at 01:41:52PM +0200, Lennert Buytenhek wrote: >> IORING_OP_GETDENTS64 behaves like getdents64(2) and takes the same > > Could we drop the '64'? We don't, for example, have IOURING_OP_FADVISE64 > even though that's the name of the syscall. Agreed, only case we do mimic the names are for things like IORING_OP_OPENAT2 where it does carry meaning. For this one, it should just be IORING_OP_GETDENTS. -- Jens Axboe
Re: [RFC PATCH] io_uring: add support for IORING_OP_GETDENTS64
On Sat, Jan 23, 2021 at 01:41:52PM +0200, Lennert Buytenhek wrote: > IORING_OP_GETDENTS64 behaves like getdents64(2) and takes the same Could we drop the '64'? We don't, for example, have IOURING_OP_FADVISE64 even though that's the name of the syscall.
Re: [PATCH v6 2/2] iio: accel: Add support for the Bosch-Sensortec BMI088
On Sat, Jan 23, 2021 at 4:35 PM Jonathan Cameron wrote: > [Me] > > Next, I think it is better to let suspend/resume, i.e. system PM > > reuse runtime PM since you're implementing that. This is why > > we invented PM runtime force resume and force suspend. > > Here the driver is turning more off for full suspend than in the > runtime path. If that results in significant extra delay then > it's not appropriate to have that in the runtime suspend path. I see the point. The resume path calls bmi088_accel_enable() which incurs a 5ms delay. The runtime resume path incurs a 1 ms delay. The runtime autosuspend kicks in after 2 ms. > Maybe the simplification of not doing the deeper power saving > mode is worth the extra power cost or extra delay, but > I'm not yet convinced. I would personally set the autosuspend to ~20ms and just use one path and take a hit of 5 ms whenever we go down between measures if it is a system that is for human interaction, but for control systems this more complex set-up may be better for response latencies. The current approach may be better tuned to perfection and we are all perfectionists :D I'm just worrying a little about bugs and maintainability. Yours, Linus Walleij
Re: [PATCH v2] perf stat: Append to default list if use -e +event
On Thu, Jan 21, 2021 at 10:02:25AM -0300, Arnaldo Carvalho de Melo wrote: > Em Thu, Jan 21, 2021 at 02:08:52PM +0800, Jin, Yao escreveu: > > Hi Arnaldo, > > > > On 1/21/2021 8:03 AM, Arnaldo Carvalho de Melo wrote: > > > Em Wed, Jan 20, 2021 at 10:25:53PM +0100, Jiri Olsa escreveu: > > > > On Mon, Jan 18, 2021 at 12:54:37PM +0800, Jin, Yao wrote: > > > > > root@kbl-ppc:# ./perf stat -e +power/energy-pkg/ -a -- sleep 1 > > > > > > > > Performance counter stats for 'system wide': > > > > > > > >2.02 Joules +power/energy-pkg/ > > > > > > > > 1.000859434 seconds time elapsed > > > > > > > > The '+' prefix is printed. So I finally decide not to remove the '+' > > > > > prefix > > > > > in order to keep original behavior. > > > > hm, originaly there's no purpose for the '+', right? > > > > it seems it's more like bug then anything else > > > > you added function to the '+' to add default events to specified event, > > > > which I think is good idea, but I don't think we should display the > > > > extra '+' in output > > > > > > The value would be to stress that that is an event added to the ones > > > without the + prefix, i.e. the default ones. > > > > > > But by having the command line copied over and the added events at the > > > first lines we should have that abundantly clear. > > > > > > Also we won't print removed events (using -), is that available already? > > > > > > > Sorry, the '-' support is not available in this patch. Can I do the patch > > for '+' first and then do a follow up patch for '-' at next step? > > Yeah, it can be done afterwards, to be symmetric. > > > > Nope: > > > > > > [root@quaco ~]# perf stat -e -cycles sleep > > > event syntax error: '-cycles' > > > \___ parser error > > > Run 'perf list' for a list of valid events > > > > > > Usage: perf stat [] [] > > > > > > -e, --eventevent selector. use 'perf list' to list > > > available events > > > So if we just want to append the default list, we only need to set > > detailed_run=1, then ideally perf-stat will print the default list. > > > But for now, there are no task-clock, context-switches, cpu-migrations, > > page-faults, instructions, branches and branch-misses displayed. > > > root@kbl-ppc:~# ./perf stat -e cycles -d -a -- sleep 1 > > > > Performance counter stats for 'system wide': > > > >124,178,207 cycles > > (80.02%) > > 6,444,490 L1-dcache-loads > > (80.01%) > > 1,043,169 L1-dcache-load-misses # 16.19% of all > > L1-dcache accesses (80.02%) > >564,474 LLC-loads > > (80.02%) > > 49,262 LLC-load-misses #8.73% of all > > LL-cache accesses (79.92%) > > > >1.001614947 seconds time elapsed > > > > Do we still need the '+' prefix to add the specified event on top of default > > list? It looks current syntax should already support that feature, but just > > need to fix some issues. > > I think we can do away with that '+' when showing the added events and > its counts. I was thinking of people parsing the stat output (this is probably also in CSV output, right?) having the extra '+' prrefix could cause issues jirka
Re: [PATCH] perf auxtrace: Automatically group aux-output events
On Thu, Jan 21, 2021 at 04:04:18PM +0200, Adrian Hunter wrote:
> aux-output events need to have an AUX area event as the group leader.
> However, grouping events does not allow the AUX area event to be given
> an address filter because the --filter option must come after the event,
> which conflicts with the grouping syntax.
>
> To allow filtering in that case, automatically create a group since that
> is the requirement anyway.
>
> Example: (requires Intel Tremont)
>
> perf record -c 500 -e 'intel_pt//u' --filter 'filter main @ /bin/ls' -e
> 'cycles/aux-output/pp' ls
great, nice to see this working
jirka
>
> Signed-off-by: Adrian Hunter
> ---
> tools/perf/builtin-record.c | 2 ++
> tools/perf/util/auxtrace.c | 15 +++
> tools/perf/util/auxtrace.h | 6 ++
> 3 files changed, 23 insertions(+)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 8a0127d4fb52..687f1449a845 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -731,6 +731,8 @@ static int record__auxtrace_init(struct record *rec)
> if (err)
> return err;
>
> + auxtrace_regroup_aux_output(rec->evlist);
> +
> return auxtrace_parse_filters(rec->evlist);
> }
>
> diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
> index a60878498139..953f4afacd3b 100644
> --- a/tools/perf/util/auxtrace.c
> +++ b/tools/perf/util/auxtrace.c
> @@ -788,6 +788,21 @@ int auxtrace_parse_sample_options(struct auxtrace_record
> *itr,
> return auxtrace_validate_aux_sample_size(evlist, opts);
> }
>
> +void auxtrace_regroup_aux_output(struct evlist *evlist)
> +{
> + struct evsel *evsel, *aux_evsel = NULL;
> + struct evsel_config_term *term;
> +
> + evlist__for_each_entry(evlist, evsel) {
> + if (evsel__is_aux_event(evsel))
> + aux_evsel = evsel;
> + term = evsel__get_config_term(evsel, AUX_OUTPUT);
> + /* If possible, group with the AUX event */
> + if (term && aux_evsel)
> + evlist__regroup(evlist, aux_evsel, evsel);
> + }
> +}
> +
> struct auxtrace_record *__weak
> auxtrace_record__init(struct evlist *evlist __maybe_unused, int *err)
> {
> diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h
> index 7e5c9e1552bd..a4fbb33b7245 100644
> --- a/tools/perf/util/auxtrace.h
> +++ b/tools/perf/util/auxtrace.h
> @@ -559,6 +559,7 @@ int auxtrace_parse_snapshot_options(struct
> auxtrace_record *itr,
> int auxtrace_parse_sample_options(struct auxtrace_record *itr,
> struct evlist *evlist,
> struct record_opts *opts, const char *str);
> +void auxtrace_regroup_aux_output(struct evlist *evlist);
> int auxtrace_record__options(struct auxtrace_record *itr,
>struct evlist *evlist,
>struct record_opts *opts);
> @@ -740,6 +741,11 @@ int auxtrace_parse_sample_options(struct auxtrace_record
> *itr __maybe_unused,
> return -EINVAL;
> }
>
> +static inline
> +void auxtrace_regroup_aux_output(struct evlist *evlist __maybe_unused)
> +{
> +}
> +
> static inline
> int auxtrace__process_event(struct perf_session *session __maybe_unused,
> union perf_event *event __maybe_unused,
> --
> 2.17.1
>
[PATCH v3] PM / clk: make PM clock layer compatible with clocks that must sleep
The clock API splits its interface into sleepable ant atomic contexts:
- clk_prepare/clk_unprepare for stuff that might sleep
- clk_enable_clk_disable for anything that may be done in atomic context
The code handling runtime PM for clocks only calls clk_disable() on
suspend requests, and clk_enable on resume requests. This means that
runtime PM with clock providers that only have the prepare/unprepare
methods implemented is basically useless.
Many clock implementations can't accommodate atomic contexts. This is
often the case when communication with the clock happens through another
subsystem like I2C or SCMI.
Let's make the clock PM code useful with such clocks by safely invoking
clk_prepare/clk_unprepare upon resume/suspend requests. Of course, when
such clocks are registered with the PM layer then pm_runtime_irq_safe()
can't be used, and neither pm_runtime_suspend() nor pm_runtime_resume()
may be invoked in atomic context.
For clocks that do implement the enable and disable methods then
everything just works as before.
A note on sparse:
According to https://lwn.net/Articles/109066/ there are things
that sparse can't cope with. In particular, pm_clk_op_lock() and
pm_clk_op_unlock() may or may not lock/unlock psd->lock depending on
some runtime condition. To work around that we tell sparse the lock
is always untaken for the purpose of static analisys.
Thanks to Naresh Kamboju for reporting issues with the initial patch.
Signed-off-by: Nicolas Pitre
Tested-by: Naresh Kamboju
---
Changes from v2:
- added workarounds to cope with sparse limitations (see above).
Changes from v1:
- made dummy clk_is_enabled_when_prepared() dependent on
CONFIG_HAVE_CLK_PREPARE instead of CONFIG_HAVE_CLK.
diff --git a/drivers/base/power/clock_ops.c b/drivers/base/power/clock_ops.c
index ced6863a16..e6956ce301 100644
--- a/drivers/base/power/clock_ops.c
+++ b/drivers/base/power/clock_ops.c
@@ -23,6 +23,7 @@
enum pce_status {
PCE_STATUS_NONE = 0,
PCE_STATUS_ACQUIRED,
+ PCE_STATUS_PREPARED,
PCE_STATUS_ENABLED,
PCE_STATUS_ERROR,
};
@@ -32,8 +33,108 @@ struct pm_clock_entry {
char *con_id;
struct clk *clk;
enum pce_status status;
+ bool enabled_when_prepared;
};
+/**
+ * pm_clk_list_lock - ensure exclusive access for modifying the PM clock
+ * entry list.
+ * @psd: pm_subsys_data instance corresponding to the PM clock entry list
+ * and clk_op_might_sleep count to be modified.
+ *
+ * Get exclusive access before modifying the PM clock entry list and the
+ * clock_op_might_sleep count to guard against concurrent modifications.
+ * This also protects against a concurrent clock_op_might_sleep and PM clock
+ * entry list usage in pm_clk_suspend()/pm_clk_resume() that may or may not
+ * happen in atomic context, hence both the mutex and the spinlock must be
+ * taken here.
+ */
+static inline void pm_clk_list_lock(struct pm_subsys_data *psd)
+{
+ mutex_lock(>clock_mutex);
+ spin_lock_irq(>lock);
+}
+
+/**
+ * pm_clk_list_unlock - counterpart to pm_clk_list_lock().
+ * @psd: the same pm_subsys_data instance previously passed to
+ * pm_clk_list_lock().
+ */
+static inline void pm_clk_list_unlock(struct pm_subsys_data *psd)
+{
+ spin_unlock_irq(>lock);
+ mutex_unlock(>clock_mutex);
+}
+
+/**
+ * pm_clk_op_lock - ensure exclusive access for performing clock operations.
+ * @psd: pm_subsys_data instance corresponding to the PM clock entry list
+ * and clk_op_might_sleep count being used.
+ * @flags: stored irq flags.
+ * @fn: string for the caller function's name.
+ *
+ * This is used by pm_clk_suspend() and pm_clk_resume() to guard
+ * against concurrent modifications to the clock entry list and the
+ * clock_op_might_sleep count. If clock_op_might_sleep is != 0 then
+ * only the mutex can be locked and those functions can only be used in
+ * non atomic context. If clock_op_might_sleep == 0 then these functions
+ * may be used in any context and only the spinlock can be locked.
+ * Returns -EINVAL if called in atomic context when clock ops might sleep.
+ */
+static int pm_clk_op_lock(struct pm_subsys_data *psd, unsigned long *flags,
+ const char *fn)
+{
+ bool atomic_context = in_atomic() || irqs_disabled();
+
+try_again:
+ spin_lock_irqsave(>lock, *flags);
+ if (!psd->clock_op_might_sleep) {
+ /* the __release is there to work around sparse limitations */
+ __release(>lock);
+ return 0;
+ }
+
+ /* bail out if in atomic context */
+ if (atomic_context) {
+ pr_err("%s: atomic context with clock_ops_might_sleep = %d",
+ fn, psd->clock_op_might_sleep);
+ spin_unlock_irqrestore(>lock, *flags);
+ might_sleep();
+ return -EPERM;
+ }
+
+ /* we must switch to the mutex */
+ spin_unlock_irqrestore(>lock,
Re: [PATCH] bus: qcom: Put child node before return
On Sat, Jan 23, 2021 at 12:19 AM Bjorn Andersson
wrote:
> On Fri 22 Jan 16:47 CST 2021, Linus Walleij wrote:
>
> > On Thu, Jan 21, 2021 at 12:49 PM Pan Bian wrote:
> >
> > > Put child node before return to fix potential reference count leak.
> > > Generally, the reference count of child is incremented and decremented
> > > automatically in the macro for_each_available_child_of_node() and should
> > > be decremented manually if the loop is broken in loop body.
> > >
> > > Fixes: 335a12754808 ("bus: qcom: add EBI2 driver")
> > > Signed-off-by: Pan Bian
> >
> > Reviewed-by: Linus Walleij
> >
> > Please submit this patch for inclusion through the ARM SoC tree.
> >
>
> Any objections to me picking it in the Qualcomm tree? Or that's what you
> indirectly meant?
Kind-of-sort-of :D
Apply it to the Qualcomm tree!
Yours,
Linus Walleij
Re: [PATCH v4 2/3] Input: atmel_mxt_ts - support wakeup methods
On Fri, Jan 22, 2021 at 9:07 PM Dmitry Osipenko wrote: > According to datasheets, chips like mXT1386 have a WAKE line, it is used > to wake the chip up from deep sleep mode before communicating with it via > the I2C-compatible interface. > > If the WAKE line is connected to a GPIO line, the line must be asserted > 25 ms before the host attempts to communicate with the controller. If the > WAKE line is connected to the SCL pin, the controller will send a NACK on > the first attempt to address it, the host must then retry 25 ms later. > > Implement the wake-up methods in the driver. Touchscreen now works > properly on devices like Acer A500 tablet, fixing problems like this: > > atmel_mxt_ts 0-004c: __mxt_read_reg: i2c transfer failed (-121) > atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121) > atmel_mxt_ts 0-004c: Trying alternate bootloader address > atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121) > atmel_mxt_ts: probe of 0-004c failed with error -121 > > Signed-off-by: Jiada Wang > Signed-off-by: Dmitry Osipenko Reviewed-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v4 00/10] Function Granular KASLR
On 2020-08-28, Josh Poimboeuf wrote:
On Fri, Aug 28, 2020 at 12:21:13PM +0200, Miroslav Benes wrote:
> Hi there! I was trying to find a super easy way to address this, so I
> thought the best thing would be if there were a compiler or linker
> switch to just eliminate any duplicate symbols at compile time for
> vmlinux. I filed this question on the binutils bugzilla looking to see
> if there were existing flags that might do this, but H.J. Lu went ahead
> and created a new one "-z unique", that seems to do what we would need
> it to do.
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=26391
>
> When I use this option, it renames any duplicate symbols with an
> extension - for example duplicatefunc.1 or duplicatefunc.2. You could
> either match on the full unique name of the specific binary you are
> trying to patch, or you match the base name and use the extension to
> determine original position. Do you think this solution would work?
Yes, I think so (thanks, Joe, for testing!).
It looks cleaner to me than the options above, but it may just be a matter
of taste. Anyway, I'd go with full name matching, because -z unique-symbol
would allow us to remove sympos altogether, which is appealing.
> If
> so, I can modify livepatch to refuse to patch on duplicated symbols if
> CONFIG_FG_KASLR and when this option is merged into the tool chain I
> can add it to KBUILD_LDFLAGS when CONFIG_FG_KASLR and livepatching
> should work in all cases.
Ok.
Josh, Petr, would this work for you too?
Sounds good to me. Kristen, thanks for finding a solution!
(I am not subscribed. I came here via
https://sourceware.org/bugzilla/show_bug.cgi?id=26391 (ld -z unique-symbol))
This works great after randomization because it always receives the
current address at runtime rather than relying on any kind of
buildtime address. The issue with with the live-patching code's
algorithm for resolving duplicate symbol names. If they request a
symbol by name from the kernel and there are 3 symbols with the same
name, they use the symbol's position in the built binary image to
select the correct symbol.
If a.o, b.o and c.o define local symbol 'foo'.
By position, do you mean that
* the live-patching code uses something like (findall("foo")[0], findall("foo")[1],
findall("foo")[2]) ?
* shuffling a.o/b.o/c.o will make the returned triple different
Local symbols are not required to be unique. Instead of patching the toolchain,
have you thought about making the live-patching code smarter?
(Depend on the duplicates, such a linker option can increase the link
time/binary size considerably
AND I don't know in what other cases such an option will be useful)
For the following example,
https://sourceware.org/bugzilla/show_bug.cgi?id=26822
# RUN: split-file %s %t
# RUN: gcc -c %t/a.s -o %t/a.o
# RUN: gcc -c %t/b.s -o %t/b.o
# RUN: gcc -c %t/c.s -o %t/c.o
# RUN: ld-new %t/a.o %t/b.o %t/c.o -z unique-symbol -o %t.exe
#--- a.s
a: a.1: a.2: nop
#--- b.s
a: nop
#--- c.s
a: nop
readelf -Ws output:
Symbol table '.symtab' contains 13 entries:
Num:Value Size TypeBind Vis Ndx Name
0: 0 NOTYPE LOCAL DEFAULT UND
1: 0 FILELOCAL DEFAULT ABS a.o
2: 00401000 0 NOTYPE LOCAL DEFAULT1 a
3: 00401000 0 NOTYPE LOCAL DEFAULT1 a.1
4: 00401000 0 NOTYPE LOCAL DEFAULT1 a.2
5: 0 FILELOCAL DEFAULT ABS b.o
6: 00401001 0 NOTYPE LOCAL DEFAULT1 a.1
7: 0 FILELOCAL DEFAULT ABS c.o
8: 00401002 0 NOTYPE LOCAL DEFAULT1 a.2
9: 0 NOTYPE GLOBAL DEFAULT UND _start
10: 00402000 0 NOTYPE GLOBAL DEFAULT1 __bss_start
11: 00402000 0 NOTYPE GLOBAL DEFAULT1 _edata
12: 00402000 0 NOTYPE GLOBAL DEFAULT1 _end
Note that you have STT_FILE SHN_ABS symbols.
If the compiler does not produce them, they will be synthesized by GNU ld.
https://sourceware.org/bugzilla/show_bug.cgi?id=26822
ld.bfd copies non-STT_SECTION local symbols from input object files. If an
object file does not have STT_FILE symbols (no .file directive) but has
non-STT_SECTION local symbols, ld.bfd synthesizes a STT_FILE symbol
The filenames are usually base names, so "a.o" and "a.o" in two directories will
be indistinguishable. The live-patching code can possibly work around this by
not changing the relative order of the two "a.o".
Re: [PATCH v7] perf stat: Fix wrong skipping for per-die aggregation
On Thu, Jan 21, 2021 at 12:21:36PM +0800, Jin, Yao wrote:
sNIP
> mask = hashmap__new(pkg_id_hash, pkg_id_equal, NULL);
> d = cpu_map__get_die(cpus, cpu, NULL).die;
> key = (size_t)d << KEY_SHIFT | s; /* s is socket id */
> if (hashmap__find(mask, (void *)key, NULL))
> *skip = true;
> else
> ret = hashmap__add(mask, (void *)key, (void *)1);
>
> If we use 'unsigned long' to replace 'size_t', it reports the build error for
> 32 bits:
>
> stat.c:320:23: warning: passing argument 1 of ‘hashmap__new’ from
> incompatible pointer type [-Wincompatible-pointer-types]
>mask = hashmap__new(pkg_id_hash, pkg_id_equal, NULL);
>^~~
> In file included from stat.c:16:
> hashmap.h:75:17: note: expected ‘hashmap_hash_fn’ {aka ‘unsigned int
> (*)(const void *, void *)’} but argument is of type ‘long unsigned int
> (*)(const void *, void *)’
>
> If we use "unsigned int", it's not good for 64 bits. So I still use 'size_t'
> in this patch.
>
> Any comments for this idea (using conditional compilation)?
isn't it simpler to allocate the key then? like below
(just compile tested)
jirka
---
diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c
index 5aba8fa92386..195fda142c98 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -276,19 +276,31 @@ void evlist__save_aggr_prev_raw_counts(struct evlist
*evlist)
static void zero_per_pkg(struct evsel *counter)
{
- if (counter->per_pkg_mask)
+ struct hashmap_entry *cur;
+ size_t bkt;
+
+ if (counter->per_pkg_mask) {
+ hashmap__for_each_entry(counter->per_pkg_mask, cur, bkt)
+ free((char *)cur->key);
+
hashmap__clear(counter->per_pkg_mask);
+ }
}
-static size_t pkg_id_hash(const void *key, void *ctx __maybe_unused)
+static size_t pkg_id_hash(const void *__key, void *ctx __maybe_unused)
{
- return (size_t)key & 0x;
+ uint64_t *key = (uint64_t*) __key;
+
+ return *key & 0x;
}
-static bool pkg_id_equal(const void *key1, const void *key2,
+static bool pkg_id_equal(const void *__key1, const void *__key2,
void *ctx __maybe_unused)
{
- return (size_t)key1 == (size_t)key2;
+ uint64_t *key1 = (uint64_t*) __key1;
+ uint64_t *key2 = (uint64_t*) __key2;
+
+ return *key1 == *key2;
}
static int check_per_pkg(struct evsel *counter,
@@ -297,7 +309,7 @@ static int check_per_pkg(struct evsel *counter,
struct hashmap *mask = counter->per_pkg_mask;
struct perf_cpu_map *cpus = evsel__cpus(counter);
int s, d, ret = 0;
- size_t key;
+ uint64_t *key;
*skip = false;
@@ -338,7 +350,11 @@ static int check_per_pkg(struct evsel *counter,
if (d < 0)
return -1;
- key = (size_t)d << 16 | s;
+ key = malloc(sizeof(*key));
+ if (!key)
+ return -ENOMEM;
+
+ *key = (size_t)d << 32 | s;
if (hashmap__find(mask, (void *)key, NULL))
*skip = true;
else
Re: [PATCH v1] gpio: tegra: Improve formatting of the code
On Fri, Jan 22, 2021 at 9:00 PM Dmitry Osipenko wrote: > Don't cross 80 chars of line length in order to keep formatting of the > code consistent. > > Signed-off-by: Dmitry Osipenko Acked-by: Linus Walleij Yours, Linus Walleij
Re: [PATCH v5] gpiolib: Bind gpio_device to a driver to enable fw_devlink=on by default
On Fri, Jan 22, 2021 at 8:36 PM Saravana Kannan wrote:
> There are multiple instances of GPIO device tree nodes of the form:
>
> foo {
> compatible = "acme,foo";
> ...
>
> gpio0: gpio0@ {
> compatible = "acme,bar";
> ...
> gpio-controller;
> };
>
> gpio1: gpio1@ {
> compatible = "acme,bar";
> ...
> gpio-controller;
> };
>
> ...
> }
>
> bazz {
> my-gpios = < ...>;
> }
>
> Case 1: The driver for "foo" populates struct device for these gpio*
> nodes and then probes them using a driver that binds with "acme,bar".
> This driver for "acme,bar" then registers the gpio* nodes with gpiolib.
> This lines up with how DT nodes with the "compatible" property are
> typically converted to struct devices and then registered with driver
> core to probe them. This also allows the gpio* devices to hook into all
> the driver core capabilities like runtime PM, probe deferral,
> suspend/resume ordering, device links, etc.
>
> Case 2: The driver for "foo" doesn't populate struct devices for these
> gpio* nodes before registering them with gpiolib. Instead it just loops
> through its child nodes and directly registers the gpio* nodes with
> gpiolib.
>
> Drivers that follow case 2 cause problems with fw_devlink=on. This is
> because fw_devlink will prevent bazz from probing until there's a struct
> device that has gpio0 as its fwnode (because bazz lists gpio0 as a GPIO
> supplier). Once the struct device is available, fw_devlink will create a
> device link with gpio0 device as the supplier and bazz device as the
> consumer. After this point, since the gpio0 device will never bind to a
> driver, the device link will prevent bazz device from ever probing.
>
> Finding and refactoring all the instances of drivers that follow case 2
> will cause a lot of code churn and it is not something that can be done
> in one shot. In some instances it might not even be possible to refactor
> them cleanly. Examples of such instances are [1] [2].
>
> This patch works around this problem and avoids all the code churn by
> simply setting the fwnode of the gpio_device and creating a stub driver
> to bind to the gpio_device. This allows all the consumers to continue
> probing when the driver follows case 2.
>
> [1] - https://lore.kernel.org/lkml/20201014191235.7f71fcb4@xhacker.debian/
> [2] -
> https://lore.kernel.org/lkml/e28e1f38d87c12a3c714a6573beba...@kernel.org/
> Cc: Marc Zyngier
> Cc: Jisheng Zhang
> Cc: Kever Yang
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> Signed-off-by: Saravana Kannan
This v5 version is a beauty!
Reviewed-by: Linus Walleij
Yours,
Linus Walleij
Re: [PATCH v1 0/3] Support building gpio-tegra driver as loadable module
On Fri, Jan 22, 2021 at 7:59 PM Dmitry Osipenko wrote: > This small series adds modularization support to the gpio-tegra driver, > i.e. driver now could be built as a loadable kernel module. > > Dmitry Osipenko (3): > gpio: tegra: Use debugfs_create_devm_seqfile() > gpio: tegra: Clean up whitespaces in tegra_gpio_driver > gpio: tegra: Support building driver as a loadable module As these three patches clearly make the kernel look better after than before: Reviewed-by: Linus Walleij However when we are doing this I would strongly encourage you to also make a patch implementing remove() so you can insmod rmmod the module at runtime. Yours, Linus Walleij
Re: [PATCH v5 4/9] libperf: Add evsel mmap support
On Wed, Jan 13, 2021 at 08:06:00PM -0600, Rob Herring wrote:
> In order to support usersapce access, an event must be mmapped. While
> there's already mmap support for evlist, the usecase is a bit different
> than the self monitoring with userspace access. So let's add a new
> perf_evsel__mmap() function to mmap an evsel. This allows implementing
> userspace access as a fastpath for perf_evsel__read().
>
> The mmapped address is returned by perf_evsel__mmap_base() which
> primarily for users/tests to check if userspace access is enabled.
>
> Signed-off-by: Rob Herring
> ---
> v5:
> - Create an mmap for every underlying event opened. Due to this, we
>need a different way to get the mmap ptr, so perf_evsel__mmap_base()
>is introduced.
> v4:
> - Change perf_evsel__mmap size to pages instead of bytes
> v3:
> - New patch split out from user access patch
> ---
> tools/lib/perf/Documentation/libperf.txt | 2 +
> tools/lib/perf/evsel.c | 47 +++-
> tools/lib/perf/include/internal/evsel.h | 2 +
> tools/lib/perf/include/perf/evsel.h | 2 +
> tools/lib/perf/libperf.map | 2 +
> 5 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/tools/lib/perf/Documentation/libperf.txt
> b/tools/lib/perf/Documentation/libperf.txt
> index 0c74c30ed23a..a2c73df191ca 100644
> --- a/tools/lib/perf/Documentation/libperf.txt
> +++ b/tools/lib/perf/Documentation/libperf.txt
> @@ -136,6 +136,8 @@ SYNOPSIS
> struct perf_thread_map *threads);
>void perf_evsel__close(struct perf_evsel *evsel);
>void perf_evsel__close_cpu(struct perf_evsel *evsel, int cpu);
> + int perf_evsel__mmap(struct perf_evsel *evsel, int pages);
> + void *perf_evsel__mmap_base(struct perf_evsel *evsel, int cpu, int thread);
>int perf_evsel__read(struct perf_evsel *evsel, int cpu, int thread,
> struct perf_counts_values *count);
>int perf_evsel__enable(struct perf_evsel *evsel);
> diff --git a/tools/lib/perf/evsel.c b/tools/lib/perf/evsel.c
> index 4dc06289f4c7..0b5bdf4badae 100644
> --- a/tools/lib/perf/evsel.c
> +++ b/tools/lib/perf/evsel.c
> @@ -11,10 +11,12 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> +#include
>
> void perf_evsel__init(struct perf_evsel *evsel, struct perf_event_attr *attr)
> {
> @@ -37,11 +39,17 @@ void perf_evsel__delete(struct perf_evsel *evsel)
> free(evsel);
> }
>
> -#define FD(e, x, y) (*(int *) xyarray__entry(e->fd, x, y))
> +struct evsel_fd {
> + int fd;
> + struct perf_mmap mmap;
> +};
nice shortcut ;-) but 'struct perf_mmap' is too big for that
I think it's better to add new 'evsel::mmap' xyarray to hold it,
add perf_evsel__alloc_mmap to allocate it and call it from
perf_evsel__mmap the same way as we callperf_evsel__alloc_fd
from perf_evsel__open
thanks,
jirka
Re: bisected regression in v5.11-rc1 snd-usb-audio
Takashi Iwai wrote:
> On Sat, 23 Jan 2021 12:08:02 +0100,
> Jamie Heilman wrote:
> >
> > > If the problem is still seen with the very latest Linus tree and the
> > > previous patch, please enable the dyndbg, e.g. pass dydbg=+p option to
> > > snd-usb-audio module, i.e. reload like
> > >modprobe snd-usb-audio dydbg=+p
> > > or boot with
> > >snd_usb_audio.dyndbg=+p
> > > boot option, retest, and give the kernel messages.
> >
> > OK, updated to 5.11.0-rc4-00202-gfe75a21824e7, repatched & rebuilt with:
> > CONFIG_SND_DEBUG=y
> > CONFIG_SND_CTL_VALIDATION=y
> > CONFIG_DYNAMIC_DEBUG=y
> > CONFIG_DYNAMIC_DEBUG_CORE=y
> >
> > and added dyndbg=+p to my snd-usb-audio options, dmesg after an
> > attempted aplay execution is attached.
> (snip)
> > [ 108.941521] WARNING: CPU: 0 PID: 2776 at sound/core/pcm_lib.c:1148
> > snd_pcm_hw_rule_add+0xd3/0x160 [snd_pcm]
>
> OK, this must be the culprit of the weird behavior.
> I totally overlooked that the max dependencies are three because of
> the terminator.
>
> Could you try the patch below in addition?
Yep, this works now, thanks! FWIW, here's what the debug output looks
like when its working, and I start jack now:
usb 1-1.1.2: 1:1: found sync_ep=0x81, iface=1, alt=1, implicit_fb=0
usb 1-1.1.2: 1:1: add audio endpoint 0x1
usb 1-1.1.2: Creating new data endpoint #1
usb 1-1.1.2: Creating new sync endpoint #81
usb 1-1.1.2: 1:1 Set sample rate 192000, clock 40
usb 1-1.1.2: [10] FU [M2Tech Audio 2.0 Output Playback Switch] ch = 2, val =
0/1/1
usb 1-1.1.2: [10] FU [M2Tech Audio 2.0 Output Playback Switch] ch = 1, val =
0/1/1
usb 1-1.1.2: RANGE setting not yet supported
usb 1-1.1.2: [10] FU [M2Tech Audio 2.0 Output Playback Volume] ch = 2, val =
-32512/0/256
usb 1-1.1.2: RANGE setting not yet supported
usb 1-1.1.2: [10] FU [M2Tech Audio 2.0 Output Playback Volume] ch = 1, val =
-32512/0/256
usb 1-1.1.2: Open EP 0x1, iface=1:1, idx=0
usb 1-1.1.2: channels=2, rate=96000, format=S32_LE, period_bytes=8192,
periods=3, implicit_fb=0
usb 1-1.1.2: Open EP 0x81, iface=1:1, idx=1
usb 1-1.1.2: channels=2, rate=96000, format=S32_LE, period_bytes=8192,
periods=3, implicit_fb=0
usb 1-1.1.2: Setting usb interface 1:0 for EP 0x1
usb 1-1.1.2: 1:1 Set sample rate 96000, clock 40
usb 1-1.1.2: Setting params for data EP 0x1, pipe 0x8600
usb 1-1.1.2: Set up 12 URBS, ret=0
usb 1-1.1.2: Setting usb interface 1:1 for EP 0x1
usb 1-1.1.2: Setting params for sync EP 0x81, pipe 0x8680
usb 1-1.1.2: Set up 4 URBS, ret=0
usb 1-1.1.2: Starting data EP 0x1 (running 0)
usb 1-1.1.2: 12 URBs submitted for EP 0x1
usb 1-1.1.2: Starting sync EP 0x81 (running 0)
usb 1-1.1.2: 4 URBs submitted for EP 0x81
usb 1-1.1.2: 1:1 Start Playback PCM
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -229,7 +229,7 @@ typedef int (*snd_pcm_hw_rule_func_t)(struct
> snd_pcm_hw_params *params,
> struct snd_pcm_hw_rule {
> unsigned int cond;
> int var;
> - int deps[4];
> + int deps[5];
>
> snd_pcm_hw_rule_func_t func;
> void *private;
--
Jamie Heilman http://audible.transient.net/~jamie/
[PATCH] vt: keyboard, use new API for keyboard_tasklet
This converts the keyboard_tasklet to use the new API in
commit 12cc923f1ccc ("tasklet: Introduce new initialization API")
The new API changes the argument passed to the callback function, but
fortunately the argument isn't used so it is straight forward to use
DECLARE_TASKLET_DISABLED() rather than DECLARE_TASKLET_DISABLED_OLD().
Signed-off-by: Emil Renner Berthing
---
drivers/tty/vt/keyboard.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/vt/keyboard.c b/drivers/tty/vt/keyboard.c
index 52922d21a49f..6c85f024bec4 100644
--- a/drivers/tty/vt/keyboard.c
+++ b/drivers/tty/vt/keyboard.c
@@ -1233,7 +1233,7 @@ void vt_kbd_con_stop(int console)
* handle the scenario when keyboard handler is not registered yet
* but we already getting updates from the VT to update led state.
*/
-static void kbd_bh(unsigned long dummy)
+static void kbd_bh(struct tasklet_struct *unused)
{
unsigned int leds;
unsigned long flags;
@@ -1249,7 +1249,7 @@ static void kbd_bh(unsigned long dummy)
}
}
-DECLARE_TASKLET_DISABLED_OLD(keyboard_tasklet, kbd_bh);
+DECLARE_TASKLET_DISABLED(keyboard_tasklet, kbd_bh);
#if defined(CONFIG_X86) || defined(CONFIG_IA64) || defined(CONFIG_ALPHA) ||\
defined(CONFIG_MIPS) || defined(CONFIG_PPC) || defined(CONFIG_SPARC) ||\
--
2.30.0
Re: [PATCH] printk: fix buffer overflow potential for print_text()
Sven Schnelle writes: > John Ogness writes: > >> On 2021-01-22, Sven Schnelle wrote: > I was able to reproduce it in a virtual machine where i have a few more > ways to debug. What i got was: > > 01: -> 001B8814" MVI 92001000 >> 0163F1CD CC 2 > > That's a watchpoint telling me that the code at 0x1b8814 wants to store > one byte to 0x163f1cd, which is the second byte of console_drivers. > > gdb tells me about 0x1b8814: > > (gdb) list *(0x1b8814) > 0x1b8814 is in record_print_text > (/home/svens/ibmgit/linux/kernel/printk/printk.c:1402). > 1397 * If a buffer was provided, it will be terminated. Space for > the > 1398 * string terminator is guaranteed to be available. The > terminator is > 1399 * not counted in the return value. > 1400 */ > 1401 if (buf_size > 0) > 1402 text[len] = 0; I don't think i have really understood how all the printk magic works, but using r->text_buf[len] seems to be the correct place to put the zero byte in that case? > 1403 > 1404 return len; > 1405 } > 1406 >
Re: [PATCH] doc: networking: ip-sysctl: Document conf/all/disable_ipv6 and conf/default/disable_ipv6
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Thu, 21 Jan 2021 16:02:44 +0100 you wrote: > This patch adds documentation for sysctl conf/all/disable_ipv6 and > conf/default/disable_ipv6 settings which is currently missing. > > Signed-off-by: Pali Rohár > --- > Documentation/networking/ip-sysctl.rst | 12 > 1 file changed, 12 insertions(+) Here is the summary with links: - doc: networking: ip-sysctl: Document conf/all/disable_ipv6 and conf/default/disable_ipv6 https://git.kernel.org/netdev/net/c/fc024c5c07aa You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH] NFC: fix resource leak when target index is invalid
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Thu, 21 Jan 2021 07:27:48 -0800 you wrote:
> Goto to the label put_dev instead of the label error to fix potential
> resource leak on path that the target index is invalid.
>
> Fixes: c4fbb6515a4d ("NFC: The core part should generate the target index")
> Signed-off-by: Pan Bian
> ---
> net/nfc/rawsock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Here is the summary with links:
- NFC: fix resource leak when target index is invalid
https://git.kernel.org/netdev/net/c/3a30537cee23
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
RE: [watchdog] watchdog: mei_wdt: request stop on unregister
> > Tomas, > > On Thu, Jan 07, 2021 at 04:12:15PM -0800, Guenter Roeck wrote: > > Hi, > > > > On Thu, Jan 07, 2021 at 09:57:30PM +0200, Tomas Winkler wrote: > > > From: Alexander Usyskin > > > > > > Send the stop command to the firmware on watchdog unregister to > > > eleminate false event on suspend. > > > > > > > Normally the watchdog driver would not be expected to unregister > > during suspend, only when the driver is manually unloaded. > > To support suspend/resume, other watchdog drivers implement > > suspend/resume functions to stop the watchdog on suspend and to > > restart it on resume. Unloading a watchdog driver on suspend would > > also have odd implications for userspace watchdog daemons. > > > > On top of that, it should not actually be possible to unregister a > > watchdog while it is in use (because it is open in that case and > > should be marked as busy). watchdog_stop_on_unregister() only serves > > as backup in case someone actually manages to unload the driver while > > the watchdog is running. The function was implemented to avoid calls > > to stop the watchdog in the remove function because I can not > > mathematically prove that there are no situations where the watchdog > > is unloaded while running. > > However, I have not actually been able to do that. > > > > Are you sure this patch is doing what you expect it to do ? > > > > I have not heard anything back. I tried to understand how this patch would > resolve a problem during suspend/resume, but I didn't find anything. Sorry, I've already prepared better commit message, just had to move the attention to other issues. > > Can you maybe add a log message showing the false event that is prevented > with this patch, and some context explaining how the patch fixes the > problem ? The MEI watchdog device lives on mei client bus and currently this bus has a special behavior, on suspend it destroys all the devices that are present on the bus. This is due to fact that the context in the MEI firmware is also lost on suspend and the resume is always a fresh start. Thanks Tomas > > Thanks, > Guenter > > > Thanks, > > Guenter > > > > > Cc: > > > Signed-off-by: Alexander Usyskin > > > Signed-off-by: Tomas Winkler > > > --- > > > drivers/watchdog/mei_wdt.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/watchdog/mei_wdt.c b/drivers/watchdog/mei_wdt.c > > > index 5391bf3e6b11..c5967d8b4256 100644 > > > --- a/drivers/watchdog/mei_wdt.c > > > +++ b/drivers/watchdog/mei_wdt.c > > > @@ -382,6 +382,7 @@ static int mei_wdt_register(struct mei_wdt *wdt) > > > > > > watchdog_set_drvdata(>wdd, wdt); > > > watchdog_stop_on_reboot(>wdd); > > > + watchdog_stop_on_unregister(>wdd); > > > > > > ret = watchdog_register_device(>wdd); > > > if (ret) > > > -- > > > 2.26.2 > > >
Re: [PATCH] NFC: fix possible resource leak
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Thu, 21 Jan 2021 07:37:45 -0800 you wrote:
> Put the device to avoid resource leak on path that the polling flag is
> invalid.
>
> Fixes: a831b9132065 ("NFC: Do not return EBUSY when stopping a poll that's
> already stopped")
> Signed-off-by: Pan Bian
> ---
> net/nfc/netlink.c | 1 +
> 1 file changed, 1 insertion(+)
Here is the summary with links:
- NFC: fix possible resource leak
https://git.kernel.org/netdev/net/c/d8f923c3ab96
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH net 2/4] net: mvpp2: Remove unneeded Kconfig dependency.
On Sat, 23 Jan 2021 13:14:00 -0800 Richard Cochran wrote: > On Sat, Jan 23, 2021 at 12:12:27PM -0800, Jakub Kicinski wrote: > > I see. The only thing I'm worried about then is the churn in patch 3. > > This would land in Linus's tree shortly before rc6, kinda late to be > > taking chances in the name of minor optimizations :S > > ;^) > > Yeah, by all means, avoid ARM churn... I remember Bad Things there... > > Maybe you could take #1 and #2 for net-next? Done, thanks! > I should probably submit 3-4 throught the SoC tree anyhow.
Re: [PATCH] net: dsa: bcm_sf2: put device node before return
Hello:
This patch was applied to netdev/net.git (refs/heads/master):
On Thu, 21 Jan 2021 04:33:43 -0800 you wrote:
> Put the device node dn before return error code on failure path.
>
> Fixes: 461cd1b03e32 ("net: dsa: bcm_sf2: Register our slave MDIO bus")
> Signed-off-by: Pan Bian
> ---
> drivers/net/dsa/bcm_sf2.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
Here is the summary with links:
- net: dsa: bcm_sf2: put device node before return
https://git.kernel.org/netdev/net/c/cf3c46631e16
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH] chtls: Fix potential resource leak
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Thu, 21 Jan 2021 06:57:38 -0800 you wrote: > The dst entry should be released if no neighbour is found. Goto label > free_dst to fix the issue. Besides, the check of ndev against NULL is > redundant. > > Signed-off-by: Pan Bian > --- > .../net/ethernet/chelsio/inline_crypto/chtls/chtls_cm.c| 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) Here is the summary with links: - chtls: Fix potential resource leak https://git.kernel.org/netdev/net/c/b6011966ac6f You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH net-next v2 0/5] net: ipa: NAPI poll updates
Hello: This series was applied to netdev/net-next.git (refs/heads/master): On Thu, 21 Jan 2021 05:48:16 -0600 you wrote: > Version 1 of this series inadvertently dropped the "static" that > limits the scope of gsi_channel_update(). Version 2 fixes this > (in patch 3). > > While reviewing the IPA NAPI polling code in detail I found two > problems. This series fixes those, and implements a few other > improvements to this part of the code. > > [...] Here is the summary with links: - [net-next,v2,1/5] net: ipa: count actual work done in gsi_channel_poll() https://git.kernel.org/netdev/net-next/c/c80c4a1ea47f - [net-next,v2,2/5] net: ipa: heed napi_complete() return value https://git.kernel.org/netdev/net-next/c/148604e7eafb - [net-next,v2,3/5] net: ipa: have gsi_channel_update() return a value https://git.kernel.org/netdev/net-next/c/223f5b34b409 - [net-next,v2,4/5] net: ipa: repurpose gsi_irq_ieob_disable() https://git.kernel.org/netdev/net-next/c/5725593e6f18 - [net-next,v2,5/5] net: ipa: disable IEOB interrupts before clearing https://git.kernel.org/netdev/net-next/c/7bd9785f683a You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH] printk: fix buffer overflow potential for print_text()
John Ogness writes: > On 2021-01-22, Sven Schnelle wrote: >> >> So somehow the pointer for console_drivers changes. >> >> I can't provide the normal kernel crash output as printk is no longer >> working, > > I don't understand what you mean here. The crash tool can dump the > printk buffer. > > I would be curious to see what the messages look like. It would also be > helpful to know the last message you saw on the console. > The last message is: [ 1845.640466] User process fault: interruption code 0007 ilc:2 in libm.so[7d78c000+a3000] [ 1845.640474] CPU: 9 PID: 859915 Comm: ld.so.1 Not tainted 5.11.0-20210122.rc4.git0.9f29bd8b2e71.300.fc33.s390x+git #1 [ 1845.640476] Hardware name: IBM 8561 T01 703 (LPAR) [ 1845.640478] User PSW : 070530008000 7d7983b2 [ 1845.640480]R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:1 AS:0 CC:3 PM:0 RI:0 EA:1 [ 1845.640483] User GPRS: 7d798360 0010 03ff0004 [ 1845.640485]7f854dd0 7d7e9788 03ff00401b30 7f854ff8 [ 1845.640487]03ff 03ff7d5b4da0 03ff 03ff0010 [ 1845.640489]03ff004022c6 03ff004024b0 80401796 7f854de8 That's also what's written to the console. Please note the the output above is expected. It's only reporting that a user space program performed an invalid FPU operation. The kernel crash is neither written to the console nor the dmesg buffer. >> kmsg_dump_rewind+290 is: >> >> if (!(con->flags & CON_ENABLED)) >> continue; >> >> in kernel/printk/printk.c:1845 >> >> I haven't dived into whether our show_code() is doing something wrong >> which was covered in the past or whether that's because of the patch >> above but wanted to make you aware of that in case you have an idea. >> Otherwise i have to dig into the code. > > Unless we are dealing with very long printk messages that normally get > truncated (800+ characters) this patch simply adds a string > terminator. I do not see how that could possibly cause this kind of > damage. > > When this triggers, there is nothing happening with consoles registering > or deregistering, right? That's correct. No registering/unregistering taking place. > The string terminator (kernel/printk/printk.c:1402) is only added for > paranoia. If you comment that out, this patch will have no effect (for > "normal" length lines). I would be curious if that somehow makes a > difference for you. I was able to reproduce it in a virtual machine where i have a few more ways to debug. What i got was: 01: -> 001B8814" MVI 92001000 >> 0163F1CD CC 2 That's a watchpoint telling me that the code at 0x1b8814 wants to store one byte to 0x163f1cd, which is the second byte of console_drivers. gdb tells me about 0x1b8814: (gdb) list *(0x1b8814) 0x1b8814 is in record_print_text (/home/svens/ibmgit/linux/kernel/printk/printk.c:1402). 1397 * If a buffer was provided, it will be terminated. Space for the 1398 * string terminator is guaranteed to be available. The terminator is 1399 * not counted in the return value. 1400 */ 1401if (buf_size > 0) 1402text[len] = 0; 1403 1404return len; 1405} 1406 In s390 assembly, this is the store: 0x001b8810 <+0x130>: la %r1,0(%r7,%r9) 0x001b8814 <+0x134>: mvi 0(%r1),0 The cpu registers at the time of write: 01: GRG 0 = 0020 0163F1CD 01: GRG 2 = 0042 03E000623A98 01: GRG 4 = 000E 0087BB70 01: GRG 6 = 0400 *0224* 01: GRG 8 = 000F *0163EFA9* 01: GRG 10 = 0033 01: GRG 12 = 809AE000 03E000623A98 01: GRG 14 = 001B884C 03E0006239E8 So r9 is 0163EFA9 - seems to be the start of the current message: 0163EFA9: *[ 24.069514]7d7af3c2: b30d0002..debr.%f0,%f2.debr.%f0,%f2 while r7 is the offset, and that one is way to big: 0224 If you add that to 0163EFA9, you'll see that we're outside of the buffer and overwriting space after, which is console_drivers. The message it's trying to print is rather long, so the 0x224 could be the size of the whole line: This is not from the same crash, but this is how the message looks like it's trying to print: [ 23.960773] User Code: 7d7af3ba: 78005000le %f0,0(%r5) [ 23.960773]7d7af3be: 78205004le %f2,4(%r5) [ 23.960773] #7d7af3c2: b30d0002debr%f0,%f2 [ 23.960773] >7d7af3c6: a728lhi %r2,0 [ 23.960773]7d7af3ca: 07febcr 15,%r14 [ 23.960773]7d7af3cc: 0707bcr 0,%r7 [
Re: [PATCH net 2/4] net: mvpp2: Remove unneeded Kconfig dependency.
On Sat, Jan 23, 2021 at 12:12:27PM -0800, Jakub Kicinski wrote: > I see. The only thing I'm worried about then is the churn in patch 3. > This would land in Linus's tree shortly before rc6, kinda late to be > taking chances in the name of minor optimizations :S ;^) Yeah, by all means, avoid ARM churn... I remember Bad Things there... Maybe you could take #1 and #2 for net-next? I should probably submit 3-4 throught the SoC tree anyhow. Thanks, Richard
Re: [PATCH v3] checkpatch: add warning for avoiding .L prefix symbols in assembly files
On Sun, 2021-01-24 at 00:34 +0530, Aditya Srivastava wrote:
> objtool requires that all code must be contained in an ELF symbol.
> Symbol names that have a '.L' prefix do not emit symbol table entries, as
> they have special meaning for the assembler.
>
> '.L' prefixed symbols can be used within a code region, but should be
> avoided for denoting a range of code via 'SYM_*_START/END' annotations.
>
> Add a new check to emit a warning on finding the usage of '.L' symbols
> for '.S' files, if it denotes range of code via SYM_*_START/END
> annotation pair.
>
> Suggested-by: Mark Brown
> Link: https://lore.kernel.org/lkml/20210112210154.gi4...@sirena.org.uk
> Signed-off-by: Aditya Srivastava
Acked-by: Joe Perches
> ---
> * Applies perfectly on next-20210122
>
> Changes in v3:
> - Modify regex for SYM_*_START/END pair
> - remove check for arch/x86/entry/* and arch/x86/lib/*
> - change 'Link:' in commit message to lkml
> - Modify commit description accordingly
>
> Changes in v2:
> - Reduce the check to only SYM_*_START/END lines
> - Reduce the check for only .S files in arch/x86/entry/* and arch/x86/lib/*
> as suggested by Josh and Nick
> - Modify commit message
>
> scripts/checkpatch.pl | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 7030c4d6d126..4a03326c87b6 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -3590,6 +3590,13 @@ sub process {
> }
> }
>
>
> +# check for .L prefix local symbols in .S files
> + if ($realfile =~ /\.S$/ &&
> + $line =~
> /^\+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) {
> + WARN("AVOID_L_PREFIX",
> + "Avoid using '.L' prefixed local symbol names for
> denoting a range of code via 'SYM_*_START/END' annotations; see
> Documentation/asm-annotations.rst\n" . $herecurr);
> + }
> +
> # check we are in a valid source file C or perl if not then ignore this hunk
> next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/);
>
>
[PATCH] configfs: make directories inherit uid/gid from creator
From: Maciej Żenczykowski
Currently a non-root process can create directories, but cannot
create stuff in the directories it creates.
Example (before this patch):
phone:/ $ id
uid=1000(system) gid=1000(system) groups=1000(system),... context=u:r:su:s0
phone:/ $ cd /config/usb_gadget/g1/configs/
phone:/config/usb_gadget/g1/configs $ ls -lZ
drwxr-xr-x 3 system system u:object_r:configfs:s0 0 2020-12-28 06:03 b.1
phone:/config/usb_gadget/g1/configs $ mkdir b.2
phone:/config/usb_gadget/g1/configs $ ls -lZ
drwxr-xr-x 3 system system u:object_r:configfs:s0 0 2020-12-28 06:03 b.1
drwxr-xr-x 3 root root u:object_r:configfs:s0 0 2020-12-28 06:51 b.2
phone:/config/usb_gadget/g1/configs $ chown system:system b.2
chown: 'b.2' to 'system:system': Operation not permitted
phone:/config/usb_gadget/g1/configs $ ls -lZ b.1
-rw-r--r-- 1 system system u:object_r:configfs:s0 4096 2020-12-28 05:23
MaxPower
-rw-r--r-- 1 system system u:object_r:configfs:s0 4096 2020-12-28 05:23
bmAttributes
lrwxrwxrwx 1 root root u:object_r:configfs:s0 0 2020-12-28 05:23
function0 -> ../../../../usb_gadget/g1/functions/ffs.adb
drwxr-xr-x 2 system system u:object_r:configfs:s0 0 2020-12-28 05:23
strings
phone:/config/usb_gadget/g1/configs $ ln -s
../../../../usb_gadget/g1/functions/ffs.adb b.2/function0
ln: cannot create symbolic link from
'../../../../usb_gadget/g1/functions/ffs.adb' to 'b.2/function0': Permission
denied
Cc: Greg Kroah-Hartman
Cc: Joel Becker
Cc: Christoph Hellwig
Signed-off-by: Maciej Żenczykowski
Change-Id: Ia907b2def940197b44aa87b337d37c5dde9c5b91
---
fs/configfs/dir.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index b839dd1b459f..04f18402ef7c 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1410,6 +1410,21 @@ static int configfs_mkdir(struct inode *dir, struct
dentry *dentry, umode_t mode
else
ret = configfs_attach_item(parent_item, item, dentry, frag);
+ /* inherit uid/gid from process creating the directory */
+ if (!uid_eq(current_fsuid(), GLOBAL_ROOT_UID) ||
+ !gid_eq(current_fsgid(), GLOBAL_ROOT_GID)) {
+ struct inode * inode = d_inode(dentry);
+ struct iattr ia = {
+ .ia_uid = current_fsuid(),
+ .ia_gid = current_fsgid(),
+ .ia_valid = ATTR_UID | ATTR_GID,
+ };
+ inode->i_uid = ia.ia_uid;
+ inode->i_gid = ia.ia_gid;
+ /* (note: the above manual assignments skip the permission
checks) */
+ (void)configfs_setattr(dentry, );
+ }
+
spin_lock(_dirent_lock);
sd->s_type &= ~CONFIGFS_USET_IN_MKDIR;
if (!ret)
--
2.30.0.280.ga3ce27912f-goog
Re: [PATCH bpf-next v5 4/4] selftests/bpf: Add a selftest for the tracing bpf_get_socket_cookie
On 1/22/21 7:34 AM, Florent Revest wrote:
On Wed, Jan 20, 2021 at 8:06 PM Florent Revest wrote:
On Wed, Jan 20, 2021 at 8:04 PM Alexei Starovoitov
wrote:
On Wed, Jan 20, 2021 at 9:08 AM KP Singh wrote:
On Tue, Jan 19, 2021 at 5:00 PM Florent Revest wrote:
This builds up on the existing socket cookie test which checks whether
the bpf_get_socket_cookie helpers provide the same value in
cgroup/connect6 and sockops programs for a socket created by the
userspace part of the test.
Adding a tracing program to the existing objects requires a different
attachment strategy and different headers.
Signed-off-by: Florent Revest
Acked-by: KP Singh
(one minor note, doesn't really need fixing as a part of this though)
---
.../selftests/bpf/prog_tests/socket_cookie.c | 24 +++
.../selftests/bpf/progs/socket_cookie_prog.c | 41 ---
2 files changed, 52 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/socket_cookie.c
b/tools/testing/selftests/bpf/prog_tests/socket_cookie.c
index 53d0c44e7907..e5c5e2ea1deb 100644
--- a/tools/testing/selftests/bpf/prog_tests/socket_cookie.c
+++ b/tools/testing/selftests/bpf/prog_tests/socket_cookie.c
@@ -15,8 +15,8 @@ struct socket_cookie {
void test_socket_cookie(void)
{
+ struct bpf_link *set_link, *update_sockops_link, *update_tracing_link;
socklen_t addr_len = sizeof(struct sockaddr_in6);
- struct bpf_link *set_link, *update_link;
int server_fd, client_fd, cgroup_fd;
struct socket_cookie_prog *skel;
__u32 cookie_expected_value;
@@ -39,15 +39,21 @@ void test_socket_cookie(void)
PTR_ERR(set_link)))
goto close_cgroup_fd;
- update_link = bpf_program__attach_cgroup(skel->progs.update_cookie,
-cgroup_fd);
- if (CHECK(IS_ERR(update_link), "update-link-cg-attach", "err %ld\n",
- PTR_ERR(update_link)))
+ update_sockops_link = bpf_program__attach_cgroup(
+ skel->progs.update_cookie_sockops, cgroup_fd);
+ if (CHECK(IS_ERR(update_sockops_link), "update-sockops-link-cg-attach",
+ "err %ld\n", PTR_ERR(update_sockops_link)))
goto free_set_link;
+ update_tracing_link = bpf_program__attach(
+ skel->progs.update_cookie_tracing);
+ if (CHECK(IS_ERR(update_tracing_link), "update-tracing-link-attach",
+ "err %ld\n", PTR_ERR(update_tracing_link)))
+ goto free_update_sockops_link;
+
server_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0);
if (CHECK(server_fd < 0, "start_server", "errno %d\n", errno))
- goto free_update_link;
+ goto free_update_tracing_link;
client_fd = connect_to_fd(server_fd, 0);
if (CHECK(client_fd < 0, "connect_to_fd", "errno %d\n", errno))
@@ -71,8 +77,10 @@ void test_socket_cookie(void)
close(client_fd);
close_server_fd:
close(server_fd);
-free_update_link:
- bpf_link__destroy(update_link);
+free_update_tracing_link:
+ bpf_link__destroy(update_tracing_link);
I don't think this need to block submission unless there are other
issues but the
bpf_link__destroy can just be called in a single cleanup label because
it handles null or
erroneous inputs:
int bpf_link__destroy(struct bpf_link *link)
{
int err = 0;
if (IS_ERR_OR_NULL(link))
return 0;
[...]
+1 to KP's point.
Also Florent, how did you test it?
This test fails in CI and in my manual run:
./test_progs -t cook
libbpf: load bpf program failed: Permission denied
libbpf: -- BEGIN DUMP LOG ---
libbpf:
; int update_cookie_sockops(struct bpf_sock_ops *ctx)
0: (bf) r6 = r1
; if (ctx->family != AF_INET6)
1: (61) r1 = *(u32 *)(r6 +20)
; if (ctx->family != AF_INET6)
2: (56) if w1 != 0xa goto pc+21
R1_w=inv10 R6_w=ctx(id=0,off=0,imm=0) R10=fp0
; if (ctx->op != BPF_SOCK_OPS_TCP_CONNECT_CB)
3: (61) r1 = *(u32 *)(r6 +0)
; if (ctx->op != BPF_SOCK_OPS_TCP_CONNECT_CB)
4: (56) if w1 != 0x3 goto pc+19
R1_w=inv3 R6_w=ctx(id=0,off=0,imm=0) R10=fp0
; if (!ctx->sk)
5: (79) r1 = *(u64 *)(r6 +184)
; if (!ctx->sk)
6: (15) if r1 == 0x0 goto pc+17
R1_w=sock(id=0,ref_obj_id=0,off=0,imm=0) R6_w=ctx(id=0,off=0,imm=0) R10=fp0
; p = bpf_sk_storage_get(_cookies, ctx->sk, 0, 0);
7: (79) r2 = *(u64 *)(r6 +184)
; p = bpf_sk_storage_get(_cookies, ctx->sk, 0, 0);
8: (18) r1 = 0x888106e41400
10: (b7) r3 = 0
11: (b7) r4 = 0
12: (85) call bpf_sk_storage_get#107
R2 type=sock_or_null expected=sock_common, sock, tcp_sock, xdp_sock, ptr_
processed 12 insns (limit 100) max_states_per_insn 0 total_states
0 peak_states 0 mark_read 0
libbpf: -- END LOG --
libbpf: failed to load program 'update_cookie_sockops'
libbpf: failed to load object 'socket_cookie_prog'
libbpf: failed to load BPF skeleton 'socket_cookie_prog': -4007
test_socket_cookie:FAIL:socket_cookie_prog__open_and_load
Re: [PATCH net v2 0/2] fix and move definitions of MRP data structures
On Thu, 21 Jan 2021 21:40:35 +0100 Rasmus Villemoes wrote: > v2: update commit log of the patch to include comments on 32 bit > alignment; include second patch moving the structs out of uapi. Applied, thanks!
Re: [PATCH v2 1/1] vhost scsi: alloc vhost_scsi with kvzalloc() to avoid delay
According to my "git send-email" history, I have CCed jasow...@redhat.com. Not
sure why Jason is not on the list.
CCed Jason. Thank you very much!
Dongli Zhang
On 1/23/21 12:08 AM, Dongli Zhang wrote:
> The size of 'struct vhost_scsi' is order-10 (~2.3MB). It may take long time
> delay by kzalloc() to compact memory pages by retrying multiple times when
> there is a lack of high-order pages. As a result, there is latency to
> create a VM (with vhost-scsi) or to hotadd vhost-scsi-based storage.
>
> The prior commit 595cb754983d ("vhost/scsi: use vmalloc for order-10
> allocation") prefers to fallback only when really needed, while this patch
> allocates with kvzalloc() with __GFP_NORETRY implicitly set to avoid
> retrying memory pages compact for multiple times.
>
> The __GFP_NORETRY is implicitly set if the size to allocate is more than
> PAGE_SZIE and when __GFP_RETRY_MAYFAIL is not explicitly set.
>
> Cc: Aruna Ramakrishna
> Cc: Joe Jin
> Signed-off-by: Dongli Zhang
> ---
> Changed since v1:
> - To combine kzalloc() and vzalloc() as kvzalloc()
> (suggested by Jason Wang)
>
> drivers/vhost/scsi.c | 9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4ce9f00ae10e..5de21ad4bd05 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1814,12 +1814,9 @@ static int vhost_scsi_open(struct inode *inode, struct
> file *f)
> struct vhost_virtqueue **vqs;
> int r = -ENOMEM, i;
>
> - vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN |
> __GFP_RETRY_MAYFAIL);
> - if (!vs) {
> - vs = vzalloc(sizeof(*vs));
> - if (!vs)
> - goto err_vs;
> - }
> + vs = kvzalloc(sizeof(*vs), GFP_KERNEL);
> + if (!vs)
> + goto err_vs;
>
> vqs = kmalloc_array(VHOST_SCSI_MAX_VQ, sizeof(*vqs), GFP_KERNEL);
> if (!vqs)
>
Re: [PATCH net-next] net: core: devlink: add new trap action HARD_DROP
On Sat, 23 Jan 2021 18:03:48 +0200 Ido Schimmel wrote: > [DEVLINK_ATTR_STATS_RX_DROPPED] nit: maybe discarded? dropped sounds like may have been due to an overflow or something
[PATCH 1/3] smp: Process pending softirqs in flush_smp_call_function_from_idle()
send_call_function_single_ipi() may wake an idle CPU without sending an
IPI. The woken up CPU will process the SMP-functions in
flush_smp_call_function_from_idle(). Any raised softirq from within the
SMP-function call will not be processed.
Should the CPU have no tasks assigned, then it will go back to idle with
pending softirqs and the NOHZ will rightfully complain.
Process pending softirqs on return from flush_smp_call_function_queue().
Fixes: b2a02fc43a1f4 ("smp: Optimize send_call_function_single_ipi()")
Reported-by: Jens Axboe
Signed-off-by: Sebastian Andrzej Siewior
---
kernel/smp.c | 4
1 file changed, 4 insertions(+)
diff --git a/kernel/smp.c b/kernel/smp.c
index 1b6070bf97bb0..aeb0adfa06063 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -14,6 +14,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -449,6 +450,9 @@ void flush_smp_call_function_from_idle(void)
local_irq_save(flags);
flush_smp_call_function_queue(true);
+ if (local_softirq_pending())
+ do_softirq();
+
local_irq_restore(flags);
}
--
2.30.0
Re: [PATCH net 2/4] net: mvpp2: Remove unneeded Kconfig dependency.
On Sat, 23 Jan 2021 05:26:26 -0800 Richard Cochran wrote:
> On Fri, Jan 22, 2021 at 06:14:44PM -0800, Jakub Kicinski wrote:
>
> > (I would put it in net-next tho, given the above this at most a space
> > optimization.)
>
> It isn't just about space but also time. The reason why I targeted
> net and not net-next was that NETWORK_PHY_TIMESTAMPING activates a
> function call to skb_clone_tx_timestamp() for every transmitted frame.
>
> static inline void skb_tx_timestamp(struct sk_buff *skb)
> {
> skb_clone_tx_timestamp(skb);
> if (skb_shinfo(skb)->tx_flags & SKBTX_SW_TSTAMP)
> skb_tstamp_tx(skb, NULL);
> }
>
> In the abscence of a PHY time stamping device, the check for its
> presence inside of skb_clone_tx_timestamp() will of course fail, but
> this still incurs the cost of the call on every transmitted skb.
>
> Similarly netif_receive_skb() futilely calls skb_defer_rx_timestamp()
> on every received skb.
>
> I would argue that most users don't want this option activated by
> accident.
I see. The only thing I'm worried about then is the churn in patch 3.
This would land in Linus's tree shortly before rc6, kinda late to be
taking chances in the name of minor optimizations :S
> (And yes, we could avoid the functions call by moving the check
> outside of the global functions and inline to the call sites. I'll be
> sure to have that in the shiny new improved scheme under discussion.)
[PATCH 3/3] blk-mq: Use llist_head for blk_cpu_done
With llist_head it is possible to avoid the locking (the irq-off region)
when items are added. This makes it possible to add items on a remote
CPU without additional locking.
llist_add() returns true if the list was previously empty. This can be
used to invoke the SMP function call / raise sofirq only if the first
item was added (otherwise it is already pending).
This simplifies the code a little and reduces the IRQ-off regions.
blk_mq_raise_softirq() needs a preempt-disable section to ensure the
request is enqueued on the same CPU as the softirq is raised.
Some callers (USB-storage) invoke this path in preemptible context.
Signed-off-by: Sebastian Andrzej Siewior
---
block/blk-mq.c | 97 ++
include/linux/blkdev.h | 2 +-
2 files changed, 42 insertions(+), 57 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 90348ae518461..463de2981df8a 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -41,7 +41,7 @@
#include "blk-mq-sched.h"
#include "blk-rq-qos.h"
-static DEFINE_PER_CPU(struct list_head, blk_cpu_done);
+static DEFINE_PER_CPU(struct llist_head, blk_cpu_done);
static void blk_mq_poll_stats_start(struct request_queue *q);
static void blk_mq_poll_stats_fn(struct blk_stat_callback *cb);
@@ -567,68 +567,29 @@ void blk_mq_end_request(struct request *rq, blk_status_t
error)
}
EXPORT_SYMBOL(blk_mq_end_request);
-/*
- * Softirq action handler - move entries to local list and loop over them
- * while passing them to the queue registered handler.
- */
-static __latent_entropy void blk_done_softirq(struct softirq_action *h)
+static void blk_complete_reqs(struct llist_head *list)
{
- struct list_head *cpu_list, local_list;
+ struct llist_node *entry = llist_reverse_order(llist_del_all(list));
+ struct request *rq, *next;
- local_irq_disable();
- cpu_list = this_cpu_ptr(_cpu_done);
- list_replace_init(cpu_list, _list);
- local_irq_enable();
-
- while (!list_empty(_list)) {
- struct request *rq;
-
- rq = list_entry(local_list.next, struct request, ipi_list);
- list_del_init(>ipi_list);
+ llist_for_each_entry_safe(rq, next, entry, ipi_list)
rq->q->mq_ops->complete(rq);
- }
}
-static void blk_mq_trigger_softirq(struct request *rq)
+static __latent_entropy void blk_done_softirq(struct softirq_action *h)
{
- struct list_head *list;
- unsigned long flags;
-
- local_irq_save(flags);
- list = this_cpu_ptr(_cpu_done);
- list_add_tail(>ipi_list, list);
-
- /*
-* If the list only contains our just added request, signal a raise of
-* the softirq. If there are already entries there, someone already
-* raised the irq but it hasn't run yet.
-*/
- if (list->next == >ipi_list)
- raise_softirq_irqoff(BLOCK_SOFTIRQ);
- local_irq_restore(flags);
+ blk_complete_reqs(this_cpu_ptr(_cpu_done));
}
static int blk_softirq_cpu_dead(unsigned int cpu)
{
- /*
-* If a CPU goes away, splice its entries to the current CPU
-* and trigger a run of the softirq
-*/
- local_irq_disable();
- list_splice_init(_cpu(blk_cpu_done, cpu),
-this_cpu_ptr(_cpu_done));
- raise_softirq_irqoff(BLOCK_SOFTIRQ);
- local_irq_enable();
-
+ blk_complete_reqs(_cpu(blk_cpu_done, cpu));
return 0;
}
-
static void __blk_mq_complete_request_remote(void *data)
{
- struct request *rq = data;
-
- blk_mq_trigger_softirq(rq);
+ __raise_softirq_irqoff(BLOCK_SOFTIRQ);
}
static inline bool blk_mq_complete_need_ipi(struct request *rq)
@@ -657,6 +618,30 @@ static inline bool blk_mq_complete_need_ipi(struct request
*rq)
return cpu_online(rq->mq_ctx->cpu);
}
+static void blk_mq_complete_send_ipi(struct request *rq)
+{
+ struct llist_head *list;
+ unsigned int cpu;
+
+ cpu = rq->mq_ctx->cpu;
+ list = _cpu(blk_cpu_done, cpu);
+ if (llist_add(>ipi_list, list)) {
+ INIT_CSD(>csd, __blk_mq_complete_request_remote, rq);
+ smp_call_function_single_async(cpu, >csd);
+ }
+}
+
+static void blk_mq_raise_softirq(struct request *rq)
+{
+ struct llist_head *list;
+
+ preempt_disable();
+ list = this_cpu_ptr(_cpu_done);
+ if (llist_add(>ipi_list, list))
+ raise_softirq(BLOCK_SOFTIRQ);
+ preempt_enable();
+}
+
bool blk_mq_complete_request_remote(struct request *rq)
{
WRITE_ONCE(rq->state, MQ_RQ_COMPLETE);
@@ -669,15 +654,15 @@ bool blk_mq_complete_request_remote(struct request *rq)
return false;
if (blk_mq_complete_need_ipi(rq)) {
- INIT_CSD(>csd, __blk_mq_complete_request_remote, rq);
- smp_call_function_single_async(rq->mq_ctx->cpu, >csd);
- } else {
- if (rq->q->nr_hw_queues > 1)
[PATCH v3 0/3] blk-mq: Don't complete in IRQ, use llist_head
Patch 2+3 were applied and then dropped by Jens due to a NOHZ+softirq related warning [0]. Turns out a successful wakeup via set_nr_if_polling() will not process any softirqs and the CPU may go back to idle. This is addressed by patch #1. smpcfd_dying_cpu() will also invoke SMP-functions calls via flush_smp_call_function_queue() but the block layer shouldn't queue anything because the CPU isn't online anymore. The two caller of flush_smp_call_function_from_idle() look fine with opening interrupts from within do_softirq(). [0] https://lkml.kernel.org/r/1ee4b31b-350e-a9f5-4349-cfb34b898...@kernel.dk Sebastian
[PATCH 2/3] blk-mq: Always complete remote completions requests in softirq
Controllers with multiple queues have their IRQ-handelers pinned to a
CPU. The core shouldn't need to complete the request on a remote CPU.
Remove this case and always raise the softirq to complete the request.
Signed-off-by: Sebastian Andrzej Siewior
---
block/blk-mq.c | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index f285a9123a8b0..90348ae518461 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -628,19 +628,7 @@ static void __blk_mq_complete_request_remote(void *data)
{
struct request *rq = data;
- /*
-* For most of single queue controllers, there is only one irq vector
-* for handling I/O completion, and the only irq's affinity is set
-* to all possible CPUs. On most of ARCHs, this affinity means the irq
-* is handled on one specific CPU.
-*
-* So complete I/O requests in softirq context in case of single queue
-* devices to avoid degrading I/O performance due to irqsoff latency.
-*/
- if (rq->q->nr_hw_queues == 1)
- blk_mq_trigger_softirq(rq);
- else
- rq->q->mq_ops->complete(rq);
+ blk_mq_trigger_softirq(rq);
}
static inline bool blk_mq_complete_need_ipi(struct request *rq)
--
2.30.0
Re: [GIT PULL] KUnit update for Linux 5.11-rc5
The pull request you sent on Fri, 22 Jan 2021 17:43:04 -0700: > git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest > tags/linux-kselftest-kunit-fixes-5.11-rc5 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/929b979611f5d2a264a2c1b9fe84baa975828522 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] SCSI fixes for 5.11-rc4
The pull request you sent on Fri, 22 Jan 2021 18:30:49 -0800: > git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git scsi-fixes has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/15cfb0f06db41542ba16907a964874ea9cfe99b2 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [GIT PULL] mtd: Fixes for v5.11-rc5
The pull request you sent on Sat, 23 Jan 2021 19:59:40 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/e1ae4b0be15891faf46d390e9f3dc9bd71a8cae1 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PULL REQUEST] i2c for 5.11
The pull request you sent on Sat, 23 Jan 2021 13:43:29 +0100: > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/077e81d51d1fce056b361c27b6cddd41e1469b9d Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH] usb: typec: tcpm: Create legacy PDOs for PD2 connection
On 1/15/21 8:33 AM, Kyle Tso wrote:
> If the port partner is PD2, the PDOs of the local port should follow the
> format defined in PD2 Spec. Dynamically modify the pre-defined PD3 PDOs
> and transform them into PD2 format before sending them to the PD2 port
> partner.
>
> Signed-off-by: Kyle Tso
I don't like this too much, but I don't have a better idea. Thus,
Reviewed-by: Guenter Roeck
Guenter
> ---
> drivers/usb/typec/tcpm/tcpm.c | 62 +--
> include/linux/usb/pd.h| 1 +
> 2 files changed, 53 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 22a85b396f69..1220ab1ed47d 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -911,13 +911,47 @@ static int tcpm_set_pwr_role(struct tcpm_port *port,
> enum typec_role role)
> return 0;
> }
>
> +/*
> + * Transform the PDO to be compliant to PD rev2.0.
> + * Return 0 if the PDO type is not defined in PD rev2.0.
> + * Otherwise, return the converted PDO.
> + */
> +static u32 tcpm_forge_legacy_pdo(struct tcpm_port *port, u32 pdo, enum
> typec_role role)
> +{
> + switch (pdo_type(pdo)) {
> + case PDO_TYPE_FIXED:
> + if (role == TYPEC_SINK)
> + return pdo & ~PDO_FIXED_FRS_CURR_MASK;
> + else
> + return pdo & ~PDO_FIXED_UNCHUNK_EXT;
> + case PDO_TYPE_VAR:
> + case PDO_TYPE_BATT:
> + return pdo;
> + case PDO_TYPE_APDO:
> + default:
> + return 0;
> + }
> +}
> +
> static int tcpm_pd_send_source_caps(struct tcpm_port *port)
> {
> struct pd_message msg;
> - int i;
> + u32 pdo;
> + unsigned int i, nr_pdo = 0;
>
> memset(, 0, sizeof(msg));
> - if (!port->nr_src_pdo) {
> +
> + for (i = 0; i < port->nr_src_pdo; i++) {
> + if (port->negotiated_rev >= PD_REV30) {
> + msg.payload[nr_pdo++] = cpu_to_le32(port->src_pdo[i]);
> + } else {
> + pdo = tcpm_forge_legacy_pdo(port, port->src_pdo[i],
> TYPEC_SOURCE);
> + if (pdo)
> + msg.payload[nr_pdo++] = cpu_to_le32(pdo);
> + }
> + }
> +
> + if (!nr_pdo) {
> /* No source capabilities defined, sink only */
> msg.header = PD_HEADER_LE(PD_CTRL_REJECT,
> port->pwr_role,
> @@ -930,10 +964,8 @@ static int tcpm_pd_send_source_caps(struct tcpm_port
> *port)
> port->data_role,
> port->negotiated_rev,
> port->message_id,
> - port->nr_src_pdo);
> + nr_pdo);
> }
> - for (i = 0; i < port->nr_src_pdo; i++)
> - msg.payload[i] = cpu_to_le32(port->src_pdo[i]);
>
> return tcpm_pd_transmit(port, TCPC_TX_SOP, );
> }
> @@ -941,10 +973,22 @@ static int tcpm_pd_send_source_caps(struct tcpm_port
> *port)
> static int tcpm_pd_send_sink_caps(struct tcpm_port *port)
> {
> struct pd_message msg;
> - int i;
> + u32 pdo;
> + unsigned int i, nr_pdo = 0;
>
> memset(, 0, sizeof(msg));
> - if (!port->nr_snk_pdo) {
> +
> + for (i = 0; i < port->nr_snk_pdo; i++) {
> + if (port->negotiated_rev >= PD_REV30) {
> + msg.payload[nr_pdo++] = cpu_to_le32(port->snk_pdo[i]);
> + } else {
> + pdo = tcpm_forge_legacy_pdo(port, port->snk_pdo[i],
> TYPEC_SINK);
> + if (pdo)
> + msg.payload[nr_pdo++] = cpu_to_le32(pdo);
> + }
> + }
> +
> + if (!nr_pdo) {
> /* No sink capabilities defined, source only */
> msg.header = PD_HEADER_LE(PD_CTRL_REJECT,
> port->pwr_role,
> @@ -957,10 +1001,8 @@ static int tcpm_pd_send_sink_caps(struct tcpm_port
> *port)
> port->data_role,
> port->negotiated_rev,
> port->message_id,
> - port->nr_snk_pdo);
> + nr_pdo);
> }
> - for (i = 0; i < port->nr_snk_pdo; i++)
> - msg.payload[i] = cpu_to_le32(port->snk_pdo[i]);
>
> return tcpm_pd_transmit(port, TCPC_TX_SOP, );
> }
> diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
> index bb9a782e1411..88f64bce5dea 100644
> --- a/include/linux/usb/pd.h
> +++ b/include/linux/usb/pd.h
> @@ -225,6 +225,7 @@ enum pd_pdo_type {
> #define PDO_FIXED_EXTPOWER BIT(27) /* Externally powered */
> #define PDO_FIXED_USB_COMM BIT(26) /* USB communications capable */
> #define PDO_FIXED_DATA_SWAP BIT(25) /* Data
Re: [PATCH v3 net-next 1/1] Allow user to set metric on default route learned via Router Advertisement.
On Fri, 22 Jan 2021 22:16:41 -0700 David Ahern wrote: > On 1/22/21 9:02 PM, Maciej Żenczykowski wrote: > > Why can't we get rid of the special case for 0 and simply make 1024 the > > default value? > > That would work too. Should we drop it then? Easier to bring it back than to change the interpretation later. It doesn't seem to serve any clear purpose right now. (Praveen if you post v4 please take a look at the checkpatch --strict warnings and address the ones which make sense, e.g. drop the brackets around comparisons, those are just noise, basic grasp of C operator precedence can be assumed in readers of kernel code).
