On Wed, Feb 10, 2021 at 08:22:21PM +0530, Mukesh Ojha wrote: > There could be a sceanario where we define some region > in normal memory and use them store to logs which is later > retrieved by bootloader during warm reset. > > In this scenario, we wanted to treat this memory as normal > cacheable memory instead of default behaviour which > is an overhead. Making it cacheable could improve > performance.
Cool; yeah. I like this idea. > > This commit gives control to change mem_type from Device > tree, and also documents the value for normal memory. What's the safest default setting? > > Signed-off-by: Huang Yiwei <hyi...@codeaurora.org> > Signed-off-by: Mukesh Ojha <mo...@codeaurora.org> > --- > Documentation/admin-guide/ramoops.rst | 4 +++- > fs/pstore/ram.c | 1 + > fs/pstore/ram_core.c | 10 ++++++++-- > 3 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/Documentation/admin-guide/ramoops.rst > b/Documentation/admin-guide/ramoops.rst > index b0a1ae7..8f107d8 100644 > --- a/Documentation/admin-guide/ramoops.rst > +++ b/Documentation/admin-guide/ramoops.rst > @@ -3,7 +3,7 @@ Ramoops oops/panic logger > > Sergiu Iordache <ser...@chromium.org> > > -Updated: 17 November 2011 > +Updated: 10 Feb 2021 > > Introduction > ------------ > @@ -30,6 +30,8 @@ mapping to pgprot_writecombine. Setting ``mem_type=1`` > attempts to use > depends on atomic operations. At least on ARM, pgprot_noncached causes the > memory to be mapped strongly ordered, and atomic operations on strongly > ordered > memory are implementation defined, and won't work on many ARMs such as omaps. > +Setting ``mem_type=2`` attempts to treat the memory region as normal memory, > +which enables full cache on it. This can improve the performance. > > The memory area is divided into ``record_size`` chunks (also rounded down to > power of two) and each kmesg dump writes a ``record_size`` chunk of > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index ca6d8a8..b262c57 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -666,6 +666,7 @@ static int ramoops_parse_dt(struct platform_device *pdev, > field = value; \ > } > > + parse_u32("mem-type", pdata->record_size, pdata->mem_type); This was handled by "unbuffered" above. Can you find a way to resolve potential conflicts between these? > parse_u32("record-size", pdata->record_size, 0); > parse_u32("console-size", pdata->console_size, 0); > parse_u32("ftrace-size", pdata->ftrace_size, 0); > diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c > index aa8e0b6..83cd612 100644 > --- a/fs/pstore/ram_core.c > +++ b/fs/pstore/ram_core.c > @@ -396,6 +396,10 @@ void persistent_ram_zap(struct persistent_ram_zone *prz) > persistent_ram_update_header_ecc(prz); > } > > +#define MEM_TYPE_WCOMBINE 0 > +#define MEM_TYPE_NONCACHED 1 > +#define MEM_TYPE_NORMAL 2 It might be nice for this to have a human-readable name too, but let's start with numeric. :) Please update the mem_type MODULE_PARM_DESC to detail the new values too. > + > static void *persistent_ram_vmap(phys_addr_t start, size_t size, > unsigned int memtype) > { > @@ -409,9 +413,11 @@ static void *persistent_ram_vmap(phys_addr_t start, > size_t size, > page_start = start - offset_in_page(start); > page_count = DIV_ROUND_UP(size + offset_in_page(start), PAGE_SIZE); > > - if (memtype) > + if (memtype == MEM_TYPE_NORMAL) > + prot = PAGE_KERNEL; > + else if (memtype == MEM_TYPE_NONCACHED) > prot = pgprot_noncached(PAGE_KERNEL); > - else > + else if (memtype == MEM_TYPE_WCOMBINE) > prot = pgprot_writecombine(PAGE_KERNEL); Let's make this a switch statement. > > pages = kmalloc_array(page_count, sizeof(struct page *), GFP_KERNEL); > -- > Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, > Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative > Project > -- Kees Cook