Re: [PATCH V11 00/17] riscv: Add Native/Paravirt qspinlock support
On Tue, Sep 12, 2023 at 09:33:57AM +0800, Guo Ren wrote: > On Mon, Sep 11, 2023 at 8:53 PM Conor Dooley > wrote: > > I added the new "riscv,isa-extensions" property in part to make > > communicating vendor extensions like this easier. Please try to use > > that. "qspinlock" is software configuration though, the vendor extension > > should focus on the guarantee of strong forward progress, since that is > > the non-standard aspect of your IP. > > The qspinlock contains three paths: > - Native qspinlock, this is your strong forward progress. > - virt_spin_lock, for KVM guest when paravirt qspinlock disabled. > > https://lore.kernel.org/linux-riscv/20230910082911.3378782-9-guo...@kernel.org/ > - paravirt qspinlock, for KVM guest > > So, we need a software configuration here, "riscv,isa-extensions" is > all about vendor extension. Ah right, yes it would only be able to be used to determine whether or not the platform is capable of supporting these spinlocks, not whether or not the kernel is a guest. I think I misinterpreted that snippet you posted, thinking you were trying to disable your new spinlock for KVM, sorry. On that note though, what about other sorts of guests? Will non-KVM guests not also want to use this virt spinlock? Thanks, Conor. signature.asc Description: PGP signature
Re: [REBASE PATCH v5 14/17] pinctrl: qcom: Use qcom_scm_io_update_field()
On Mon, Sep 11, 2023 at 12:56 PM Mukesh Ojha wrote: > Use qcom_scm_io_update_field() exported function in > pinctrl-msm driver. > > Signed-off-by: Mukesh Ojha As long as the qcom maintainers agree on the rest of the patches: Acked-by: Linus Walleij Yours, Linus Walleij
Re: [REBASE PATCH v5 15/17] firmware: scm: Modify only the download bits in TCSR register
On 9/11/2023 8:37 PM, Kathiravan Thirumoorthy wrote:
On 9/11/2023 4:23 PM, Mukesh Ojha wrote:
Crashdump collection is based on the DLOAD bit of TCSR register.
To retain other bits, we read the register and modify only the
DLOAD bit as the other bits have their own significance.
Co-developed-by: Poovendhan Selvaraj
Signed-off-by: Poovendhan Selvaraj
Signed-off-by: Mukesh Ojha
Tested-by: Kathiravan Thirumoorthy #
IPQ9574 and IPQ5332
---
drivers/firmware/qcom_scm.c | 16 ++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/qcom_scm.c b/drivers/firmware/qcom_scm.c
index 321133f0950d..5cacae63ee2a 100644
--- a/drivers/firmware/qcom_scm.c
+++ b/drivers/firmware/qcom_scm.c
@@ -5,6 +5,8 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -26,6 +28,14 @@
static bool download_mode =
IS_ENABLED(CONFIG_QCOM_SCM_DOWNLOAD_MODE_DEFAULT);
module_param(download_mode, bool, 0);
+#define SCM_HAS_CORE_CLK BIT(0)
+#define SCM_HAS_IFACE_CLK BIT(1)
+#define SCM_HAS_BUS_CLK BIT(2)
Is this intentional to add these macros back again?
This is a mistake, thanks for letting me know.
-Mukesh
+
+#define QCOM_DLOAD_MASK GENMASK(5, 4)
+#define QCOM_DLOAD_FULLDUMP 0x1
+#define QCOM_DLOAD_NODUMP 0x0
+
struct qcom_scm {
struct device *dev;
struct clk *core_clk;
@@ -440,6 +450,7 @@ static int __qcom_scm_set_dload_mode(struct device
*dev, bool enable)
static void qcom_scm_set_download_mode(bool enable)
{
+ u32 val = enable ? QCOM_DLOAD_FULLDUMP : QCOM_DLOAD_NODUMP;
bool avail;
int ret = 0;
@@ -449,8 +460,9 @@ static void qcom_scm_set_download_mode(bool enable)
if (avail) {
ret = __qcom_scm_set_dload_mode(__scm->dev, enable);
} else if (__scm->dload_mode_addr) {
- ret = qcom_scm_io_writel(__scm->dload_mode_addr,
- enable ? QCOM_SCM_BOOT_SET_DLOAD_MODE : 0);
+ ret = qcom_scm_io_update_field(__scm->dload_mode_addr,
+ QCOM_DLOAD_MASK,
+ FIELD_PREP(QCOM_DLOAD_MASK, val));
} else {
dev_err(__scm->dev,
"No available mechanism for setting download mode\n");
Re: [PATCH v5 06/17] soc: qcom: Add Qualcomm APSS minidump kernel driver
Thanks for your time in reviewing this.
On 9/11/2023 4:31 PM, Krzysztof Kozlowski wrote:
On 09/09/2023 22:16, Mukesh Ojha wrote:
Minidump is a best effort mechanism to collect useful and predefined
data for first level of debugging on end user devices running on
Qualcomm SoCs. It is built on the premise that System on Chip (SoC)
or subsystem part of SoC crashes, due to a range of hardware and
software bugs. Hence, the ability to collect accurate data is only
a best-effort. The data collected could be invalid or corrupted,
data collection itself could fail, and so on.
...
+static int qcom_apss_md_table_init(struct minidump *md,
+ struct minidump_subsystem *mdss_toc)
+{
+ struct minidump_ss_data *mdss_data;
+
+ mdss_data = devm_kzalloc(md->dev, sizeof(*mdss_data), GFP_KERNEL);
+ if (!mdss_data)
+ return -ENOMEM;
+
+ mdss_data->md_ss_toc = mdss_toc;
+ mdss_data->md_regions = devm_kcalloc(md->dev, MAX_NUM_ENTRIES,
+sizeof(struct minidump_region),
+GFP_KERNEL);
+ if (!mdss_data->md_regions)
+ return -ENOMEM;
+
+ mdss_toc = mdss_data->md_ss_toc;
+ mdss_toc->regions_baseptr =
cpu_to_le64(virt_to_phys(mdss_data->md_regions));
+ mdss_toc->enabled = cpu_to_le32(MINIDUMP_SS_ENABLED);
+ mdss_toc->status = cpu_to_le32(1);
+ mdss_toc->region_count = cpu_to_le32(0);
+
+ /* Tell bootloader not to encrypt the regions of this subsystem */
+ mdss_toc->encryption_status = cpu_to_le32(MINIDUMP_SS_ENCR_DONE);
+ mdss_toc->encryption_required = cpu_to_le32(MINIDUMP_SS_ENCR_NOTREQ);
+
+ md->apss_data = mdss_data;
+
+ return 0;
+}
+
+static int qcom_apss_minidump_probe(struct platform_device *pdev)
+{
+ struct minidump_global_toc *mdgtoc;
+ struct minidump *md;
+ size_t size;
+ int ret;
+
+ md = devm_kzalloc(&pdev->dev, sizeof(struct minidump), GFP_KERNEL);
sizeof(*)
Didn't you get such comments already?
Ok, will fix this, no i have not got such comments as of yet.
Any reason of using this way?
+ if (!md)
+ return -ENOMEM;
+
+ md->dev = &pdev->dev;
+ mdgtoc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, &size);
+ if (IS_ERR(mdgtoc)) {
+ ret = PTR_ERR(mdgtoc);
+ dev_err(md->dev, "Couldn't find minidump smem item: %d\n", ret);
+ return ret;
The syntax is:
return dev_err_probe
ACK.
+ }
+
+ if (size < sizeof(*mdgtoc) || !mdgtoc->status) {
+ dev_err(md->dev, "minidump table is not initialized: %d\n",
ret);
ret is uninitialized here. Please use automated tools for checking your
code:
coccinelle, smatch and sparse
Thanks.
+ return -EINVAL;
+ }
+
+ mutex_init(&md->md_lock);
+ ret = qcom_apss_md_table_init(md,
&mdgtoc->subsystems[MINIDUMP_APSS_DESC]);
+ if (ret) {
+ dev_err(md->dev, "apss minidump initialization failed: %d\n",
ret);
+ return ret;
+ }
+
+ /* First entry would be ELF header */
+ ret = qcom_md_add_elfheader(md);
+ if (ret) {
+ dev_err(md->dev, "Failed to add elf header: %d\n", ret);
+ memset(md->apss_data->md_ss_toc, 0, sizeof(struct
minidump_subsystem));
Why do you need it?
Earlier, i got comment about clearing the SS TOC(subsystem table of
content) which is shared with other SS and it will have stale values.
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, md);
+
+ return ret;
+}
+
+static int qcom_apss_minidump_remove(struct platform_device *pdev)
+{
+ struct minidump *md = platform_get_drvdata(pdev);
+ struct minidump_ss_data *mdss_data;
+
+ mdss_data = md->apss_data;
+ memset(mdss_data->md_ss_toc, cpu_to_le32(0), sizeof(struct
minidump_subsystem));
Why do you need it?
Same as above.
+ md = NULL;
That's useless assignment.
Ok.
+
+ return 0;
+}
+
+static struct platform_driver qcom_minidump_driver = {
+ .probe = qcom_apss_minidump_probe,
+ .remove = qcom_apss_minidump_remove,
+ .driver = {
+ .name = "qcom-minidump-smem",
+ },
+};
+
+module_platform_driver(qcom_minidump_driver);
+
+MODULE_DESCRIPTION("Qualcomm APSS minidump driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:qcom-minidump-smem");
Add a proper ID table instead of re-inventing it with module aliases.
Ok.
-Mukesh
Best regards,
Krzysztof
Re: [PATCH v5 09/17] pstore/ram: Use dynamic ramoops reserve resource
On 9/12/2023 6:09 AM, Pavan Kondeti wrote: On Mon, Sep 11, 2023 at 04:21:44PM +0530, Mukesh Ojha wrote: On 9/11/2023 11:03 AM, Pavan Kondeti wrote: On Sun, Sep 10, 2023 at 01:46:10AM +0530, Mukesh Ojha wrote: As dynamic ramoops command line parsing is now added, so lets add the support in ramoops driver to get the resource structure and add it during platform device registration. Signed-off-by: Mukesh Ojha --- fs/pstore/ram.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) Documentation/admin-guide/ramoops.rst might need an update as well. I have said in the cover-letter under changes in v5, it is open for comment and not yet documented it yet. Sure. To easy on the reviewers, the under cut portion of a specific patch could be used to add footer notes like TODO/Testing etc. In this case, I was lazy to read the loong cover letter posted in this series ;-) I have seen it, will comment related to particular patch under --- . Thanks for suggestion. -Mukesh Thanks, Pavan
Re: [PATCH v5 06/17] soc: qcom: Add Qualcomm APSS minidump kernel driver
On 12/09/2023 11:26, Mukesh Ojha wrote:
>>
>>> + return -EINVAL;
>>> + }
>>> +
>>> + mutex_init(&md->md_lock);
>>> + ret = qcom_apss_md_table_init(md,
>>> &mdgtoc->subsystems[MINIDUMP_APSS_DESC]);
>>> + if (ret) {
>>> + dev_err(md->dev, "apss minidump initialization failed: %d\n",
>>> ret);
>>> + return ret;
>>> + }
>>> +
>>> + /* First entry would be ELF header */
>>> + ret = qcom_md_add_elfheader(md);
>>> + if (ret) {
>>> + dev_err(md->dev, "Failed to add elf header: %d\n", ret);
>>> + memset(md->apss_data->md_ss_toc, 0, sizeof(struct
>>> minidump_subsystem));
>>
>> Why do you need it?
>
> Earlier, i got comment about clearing the SS TOC(subsystem table of
> content) which is shared with other SS and it will have stale values.
OK, but then the entire code is poorly readable. First, any cleanup of
qcom_apss_md_table_init() should be named similarly, e.g.
qcom_apss_md_table_clean() or qcom_apss_md_table_exit() or whatever
seems feasible.
Second, shouldn't writing to shared memory be the last step? Step which
cannot fail and there is no cleanup afterwards (like
platform_set_drvdata)? I don't enjoy looking at this interface...
Best regards,
Krzysztof
Re: [PATCH v5 06/17] soc: qcom: Add Qualcomm APSS minidump kernel driver
On 9/12/2023 2:56 PM, Mukesh Ojha wrote:
Thanks for your time in reviewing this.
On 9/11/2023 4:31 PM, Krzysztof Kozlowski wrote:
On 09/09/2023 22:16, Mukesh Ojha wrote:
Minidump is a best effort mechanism to collect useful and predefined
data for first level of debugging on end user devices running on
Qualcomm SoCs. It is built on the premise that System on Chip (SoC)
or subsystem part of SoC crashes, due to a range of hardware and
software bugs. Hence, the ability to collect accurate data is only
a best-effort. The data collected could be invalid or corrupted,
data collection itself could fail, and so on.
...
+static int qcom_apss_md_table_init(struct minidump *md,
+ struct minidump_subsystem *mdss_toc)
+{
+ struct minidump_ss_data *mdss_data;
+
+ mdss_data = devm_kzalloc(md->dev, sizeof(*mdss_data), GFP_KERNEL);
+ if (!mdss_data)
+ return -ENOMEM;
+
+ mdss_data->md_ss_toc = mdss_toc;
+ mdss_data->md_regions = devm_kcalloc(md->dev, MAX_NUM_ENTRIES,
+ sizeof(struct minidump_region),
+ GFP_KERNEL);
+ if (!mdss_data->md_regions)
+ return -ENOMEM;
+
+ mdss_toc = mdss_data->md_ss_toc;
+ mdss_toc->regions_baseptr =
cpu_to_le64(virt_to_phys(mdss_data->md_regions));
+ mdss_toc->enabled = cpu_to_le32(MINIDUMP_SS_ENABLED);
+ mdss_toc->status = cpu_to_le32(1);
+ mdss_toc->region_count = cpu_to_le32(0);
+
+ /* Tell bootloader not to encrypt the regions of this subsystem */
+ mdss_toc->encryption_status = cpu_to_le32(MINIDUMP_SS_ENCR_DONE);
+ mdss_toc->encryption_required =
cpu_to_le32(MINIDUMP_SS_ENCR_NOTREQ);
+
+ md->apss_data = mdss_data;
+
+ return 0;
+}
+
+static int qcom_apss_minidump_probe(struct platform_device *pdev)
+{
+ struct minidump_global_toc *mdgtoc;
+ struct minidump *md;
+ size_t size;
+ int ret;
+
+ md = devm_kzalloc(&pdev->dev, sizeof(struct minidump), GFP_KERNEL);
sizeof(*)
Didn't you get such comments already?
Ok, will fix this, no i have not got such comments as of yet.
Any reason of using this way?
Got the reason, thanks.
-Mukesh
+ if (!md)
+ return -ENOMEM;
+
+ md->dev = &pdev->dev;
+ mdgtoc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID,
&size);
+ if (IS_ERR(mdgtoc)) {
+ ret = PTR_ERR(mdgtoc);
+ dev_err(md->dev, "Couldn't find minidump smem item: %d\n",
ret);
+ return ret;
The syntax is:
return dev_err_probe
ACK.
+ }
+
+ if (size < sizeof(*mdgtoc) || !mdgtoc->status) {
+ dev_err(md->dev, "minidump table is not initialized: %d\n",
ret);
ret is uninitialized here. Please use automated tools for checking your
code:
coccinelle, smatch and sparse
Thanks.
+ return -EINVAL;
+ }
+
+ mutex_init(&md->md_lock);
+ ret = qcom_apss_md_table_init(md,
&mdgtoc->subsystems[MINIDUMP_APSS_DESC]);
+ if (ret) {
+ dev_err(md->dev, "apss minidump initialization failed:
%d\n", ret);
+ return ret;
+ }
+
+ /* First entry would be ELF header */
+ ret = qcom_md_add_elfheader(md);
+ if (ret) {
+ dev_err(md->dev, "Failed to add elf header: %d\n", ret);
+ memset(md->apss_data->md_ss_toc, 0, sizeof(struct
minidump_subsystem));
Why do you need it?
Earlier, i got comment about clearing the SS TOC(subsystem table of
content) which is shared with other SS and it will have stale values.
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, md);
+
+ return ret;
+}
+
+static int qcom_apss_minidump_remove(struct platform_device *pdev)
+{
+ struct minidump *md = platform_get_drvdata(pdev);
+ struct minidump_ss_data *mdss_data;
+
+ mdss_data = md->apss_data;
+ memset(mdss_data->md_ss_toc, cpu_to_le32(0), sizeof(struct
minidump_subsystem));
Why do you need it?
Same as above.
+ md = NULL;
That's useless assignment.
Ok.
+
+ return 0;
+}
+
+static struct platform_driver qcom_minidump_driver = {
+ .probe = qcom_apss_minidump_probe,
+ .remove = qcom_apss_minidump_remove,
+ .driver = {
+ .name = "qcom-minidump-smem",
+ },
+};
+
+module_platform_driver(qcom_minidump_driver);
+
+MODULE_DESCRIPTION("Qualcomm APSS minidump driver");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:qcom-minidump-smem");
Add a proper ID table instead of re-inventing it with module aliases.
Ok.
-Mukesh
Best regards,
Krzysztof
Re: [REBASE PATCH v5 08/17] arm64: mm: Add dynamic ramoops region support through command line
On Mon, Sep 11, 2023 at 04:23:50PM +0530, Mukesh Ojha wrote: > The reserved memory region for ramoops is assumed to be at a fixed > and known location when read from the devicetree. This may not be > required for something like Qualcomm's minidump which is interested > in knowing addresses of ramoops region but it does not put hard > requirement of address being fixed as most of it's SoC does not > support warm reset and does not use pstorefs at all instead it has > firmware way of collecting ramoops region if it gets to know the > address and register it with apss minidump table which is sitting > in shared memory region in DDR and firmware will have access to > these table during reset and collects it on crash of SoC. > > So, add the support of reserving ramoops region to be dynamically > allocated early during boot if it is request through command line > via 'dyn_ramoops_size=' and fill up reserved resource structure and > export the structure, so that it can be read by ramoops driver. > > Signed-off-by: Mukesh Ojha > --- > arch/arm64/mm/init.c | 94 > ++ Why does this need to be in the arch code? There's absolutely nothing arm64-specific here. Will
Re: [PATCH V11 00/17] riscv: Add Native/Paravirt qspinlock support
On Tue, Sep 12, 2023 at 4:08 PM Conor Dooley wrote: > > On Tue, Sep 12, 2023 at 09:33:57AM +0800, Guo Ren wrote: > > On Mon, Sep 11, 2023 at 8:53 PM Conor Dooley > > wrote: > > > > I added the new "riscv,isa-extensions" property in part to make > > > communicating vendor extensions like this easier. Please try to use > > > that. "qspinlock" is software configuration though, the vendor extension > > > should focus on the guarantee of strong forward progress, since that is > > > the non-standard aspect of your IP. > > > > The qspinlock contains three paths: > > - Native qspinlock, this is your strong forward progress. > > - virt_spin_lock, for KVM guest when paravirt qspinlock disabled. > > > > https://lore.kernel.org/linux-riscv/20230910082911.3378782-9-guo...@kernel.org/ > > - paravirt qspinlock, for KVM guest > > > > So, we need a software configuration here, "riscv,isa-extensions" is > > all about vendor extension. > > Ah right, yes it would only be able to be used to determine whether or > not the platform is capable of supporting these spinlocks, not whether or > not the kernel is a guest. I think I misinterpreted that snippet you posted, > thinking you were trying to disable your new spinlock for KVM, sorry. > On that note though, what about other sorts of guests? Will non-KVM > guests not also want to use this virt spinlock? I only put KVM guests here, and I can't answer other hypervisor that is another topic. > > Thanks, > Conor. -- Best Regards Guo Ren
[no subject]
Date: Tue, 12 Sep 2023 02:24:11 + Subject: [PATCH v2 0/2] mm/damon: add a tracepoint for damos apply target regions Changlog >From v1 (https://lore.kernel.org/damon/20230911045908.97649-1...@kernel.org/) - Get scheme/target indices only when the trace is enabled (Steven Rostedt) >From RFC (https://lore.kernel.org/damon/20230827004045.49516-1...@kernel.org/) - Fix the 4 byte hole (Steven Rostedt) - Add documentation Description --- DAMON provides damon_aggregated tracepoint to let users record full monitoring results. Sometimes, users need to record monitoring results of specific pattern. DAMOS tried regions directory of DAMON sysfs interface allows it, but the interface is mainly designed for snapshots and therefore would be inefficient for such recording. Implement yet another tracepoint for efficient support of the usecase. SeongJae Park (2): mm/damon/core: add a tracepoint for damos apply target regions Docs/admin-guide/mm/damon/usage: document damos_before_apply tracepoint Documentation/admin-guide/mm/damon/usage.rst | 37 +++ include/trace/events/damon.h | 39 mm/damon/core.c | 32 +++- 3 files changed, 100 insertions(+), 8 deletions(-) base-commit: b67dc18d1406be3598248d2cc78904a81176fa13 -- 2.25.1
Re: [PATCH V11 01/17] asm-generic: ticket-lock: Reuse arch_spinlock_t of qspinlock
On Tue, Sep 12, 2023 at 3:05 AM Leonardo Brás wrote:
>
> On Sun, 2023-09-10 at 04:28 -0400, guo...@kernel.org wrote:
> > From: Guo Ren
> >
> > The arch_spinlock_t of qspinlock has contained the atomic_t val, which
> > satisfies the ticket-lock requirement. Thus, unify the arch_spinlock_t
> > into qspinlock_types.h. This is the preparation for the next combo
> > spinlock.
> >
> > Signed-off-by: Guo Ren
> > Signed-off-by: Guo Ren
> > ---
> > include/asm-generic/spinlock.h | 14 +++---
> > include/asm-generic/spinlock_types.h | 12 ++--
> > 2 files changed, 9 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/asm-generic/spinlock.h b/include/asm-generic/spinlock.h
> > index 90803a826ba0..4773334ee638 100644
> > --- a/include/asm-generic/spinlock.h
> > +++ b/include/asm-generic/spinlock.h
> > @@ -32,7 +32,7 @@
> >
> > static __always_inline void arch_spin_lock(arch_spinlock_t *lock)
> > {
> > - u32 val = atomic_fetch_add(1<<16, lock);
> > + u32 val = atomic_fetch_add(1<<16, &lock->val);
> > u16 ticket = val >> 16;
> >
> > if (ticket == (u16)val)
> > @@ -46,31 +46,31 @@ static __always_inline void
> > arch_spin_lock(arch_spinlock_t *lock)
> >* have no outstanding writes due to the atomic_fetch_add() the extra
> >* orderings are free.
> >*/
> > - atomic_cond_read_acquire(lock, ticket == (u16)VAL);
> > + atomic_cond_read_acquire(&lock->val, ticket == (u16)VAL);
> > smp_mb();
> > }
> >
> > static __always_inline bool arch_spin_trylock(arch_spinlock_t *lock)
> > {
> > - u32 old = atomic_read(lock);
> > + u32 old = atomic_read(&lock->val);
> >
> > if ((old >> 16) != (old & 0x))
> > return false;
> >
> > - return atomic_try_cmpxchg(lock, &old, old + (1<<16)); /* SC, for RCsc
> > */
> > + return atomic_try_cmpxchg(&lock->val, &old, old + (1<<16)); /* SC,
> > for RCsc */
> > }
> >
> > static __always_inline void arch_spin_unlock(arch_spinlock_t *lock)
> > {
> > u16 *ptr = (u16 *)lock + IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
> > - u32 val = atomic_read(lock);
> > + u32 val = atomic_read(&lock->val);
> >
> > smp_store_release(ptr, (u16)val + 1);
> > }
> >
> > static __always_inline int arch_spin_value_unlocked(arch_spinlock_t lock)
> > {
> > - u32 val = lock.counter;
> > + u32 val = lock.val.counter;
> >
> > return ((val >> 16) == (val & 0x));
> > }
>
> This one seems to be different in torvalds/master, but I suppose it's because
> of
> the requirement patches I have not merged.
>
> > @@ -84,7 +84,7 @@ static __always_inline int
> > arch_spin_is_locked(arch_spinlock_t *lock)
> >
> > static __always_inline int arch_spin_is_contended(arch_spinlock_t *lock)
> > {
> > - u32 val = atomic_read(lock);
> > + u32 val = atomic_read(&lock->val);
> >
> > return (s16)((val >> 16) - (val & 0x)) > 1;
> > }
> > diff --git a/include/asm-generic/spinlock_types.h
> > b/include/asm-generic/spinlock_types.h
> > index 8962bb730945..f534aa5de394 100644
> > --- a/include/asm-generic/spinlock_types.h
> > +++ b/include/asm-generic/spinlock_types.h
> > @@ -3,15 +3,7 @@
> > #ifndef __ASM_GENERIC_SPINLOCK_TYPES_H
> > #define __ASM_GENERIC_SPINLOCK_TYPES_H
> >
> > -#include
> > -typedef atomic_t arch_spinlock_t;
> > -
> > -/*
> > - * qrwlock_types depends on arch_spinlock_t, so we must typedef that
> > before the
> > - * include.
> > - */
> > -#include
> > -
> > -#define __ARCH_SPIN_LOCK_UNLOCKEDATOMIC_INIT(0)
> > +#include
> > +#include
> >
> > #endif /* __ASM_GENERIC_SPINLOCK_TYPES_H */
>
> FWIW, LGTM:
>
> Reviewed-by: Leonardo Bras
>
>
> Just a suggestion: In this patch I could see a lot of usage changes to
> arch_spinlock_t, and only at the end I could see the actual change in the .h
> file.
include/asm-generic/spinlock.h | 14 +++---
include/asm-generic/spinlock_types.h | 12 ++--
All are .h files. So, how to use git.orderfile?
>
> In cases like this, it looks nicer to see the .h file first.
>
> I recently found out about this git diff.orderFile option, which helps to
> achieve exactly this.
>
> I use the following git.orderfile, adapted from qemu:
>
>
> #
> # order file for git, to produce patches which are easier to review
> # by diffing the important stuff like interface changes first.
> #
> # one-off usage:
> # git diff -O scripts/git.orderfile ...
> #
> # add to git config:
> # git config diff.orderFile scripts/git.orderfile
> #
>
> MAINTAINERS
>
> # Documentation
> Documentation/*
> *.rst
> *.rst.inc
>
> # build system
> Kbuild
> Makefile*
> *.mak
>
> # semantic patches
> *.cocci
>
> # headers
> *.h
> *.h.inc
>
> # code
> *.c
> *.c.inc
>
>
--
Best Regards
Guo Ren
Re: (no title)
Hello, On Tue, 12 Sep 2023 18:35:57 + SeongJae Park wrote: > Date: Tue, 12 Sep 2023 02:24:11 + > Subject: [PATCH v2 0/2] mm/damon: add a tracepoint for damos apply target > regions I added a blank line in the header of the original patch, and it resulted in this weird no-title mail, sorry. I will rebase this on latest mm-unstable and resend. Thanks, SJ > > Changlog > > > From v1 > (https://lore.kernel.org/damon/20230911045908.97649-1...@kernel.org/) > - Get scheme/target indices only when the trace is enabled (Steven Rostedt) > > From RFC > (https://lore.kernel.org/damon/20230827004045.49516-1...@kernel.org/) > - Fix the 4 byte hole (Steven Rostedt) > - Add documentation > > Description > --- > > DAMON provides damon_aggregated tracepoint to let users record full monitoring > results. Sometimes, users need to record monitoring results of specific > pattern. DAMOS tried regions directory of DAMON sysfs interface allows it, > but > the interface is mainly designed for snapshots and therefore would be > inefficient for such recording. Implement yet another tracepoint for > efficient > support of the usecase. > > > SeongJae Park (2): > mm/damon/core: add a tracepoint for damos apply target regions > Docs/admin-guide/mm/damon/usage: document damos_before_apply > tracepoint > > Documentation/admin-guide/mm/damon/usage.rst | 37 +++ > include/trace/events/damon.h | 39 > mm/damon/core.c | 32 +++- > 3 files changed, 100 insertions(+), 8 deletions(-) > > > base-commit: b67dc18d1406be3598248d2cc78904a81176fa13 > -- > 2.25.1 >
[PATCH RESEND v2 0/2] mm/damon: add a tracepoint for damos apply target regions
Changlog >From original v2 post (https://lore.kernel.org/damon/20230912183559.4733-1...@kernel.org/) - Fix header - Rebase on latest mm-unstable >From v1 (https://lore.kernel.org/damon/20230911045908.97649-1...@kernel.org/) - Get scheme/target indices only when the trace is enabled (Steven Rostedt) >From RFC (https://lore.kernel.org/damon/20230827004045.49516-1...@kernel.org/) - Fix the 4 byte hole (Steven Rostedt) - Add documentation Description --- DAMON provides damon_aggregated tracepoint to let users record full monitoring results. Sometimes, users need to record monitoring results of specific pattern. DAMOS tried regions directory of DAMON sysfs interface allows it, but the interface is mainly designed for snapshots and therefore would be inefficient for such recording. Implement yet another tracepoint for efficient support of the usecase. SeongJae Park (2): mm/damon/core: add a tracepoint for damos apply target regions Docs/admin-guide/mm/damon/usage: document damos_before_apply tracepoint Documentation/admin-guide/mm/damon/usage.rst | 37 +++ include/trace/events/damon.h | 39 mm/damon/core.c | 32 +++- 3 files changed, 100 insertions(+), 8 deletions(-) base-commit: 8abeac23845e94681a163299a52d802b82475761 -- 2.25.1
