Re: [PATCH v3 1/3] riscv: Set more data to cacheinfo
On Mon, Aug 31, 2020 at 3:00 PM Pekka Enberg wrote: > > Hi, > > On Mon, Aug 31, 2020 at 9:15 AM Zong Li wrote: > > If the sets is one, it means that the cache is fully associative, then > > we don't need to fill the ways number, just keep way number as zero, > > so here we want to find the fully associative case first and make the > > if expression fail at the beginning. We might also rewrite it as > > follow: > > [snip] > > Thanks for the explanation! The rewritten version is much easier to > read, at least to me. > Let me change it for readability in the next version, thanks. > - Pekka
Re: [PATCH v3 1/3] riscv: Set more data to cacheinfo
Hi, On Mon, Aug 31, 2020 at 9:15 AM Zong Li wrote: > If the sets is one, it means that the cache is fully associative, then > we don't need to fill the ways number, just keep way number as zero, > so here we want to find the fully associative case first and make the > if expression fail at the beginning. We might also rewrite it as > follow: [snip] Thanks for the explanation! The rewritten version is much easier to read, at least to me. - Pekka
Re: [PATCH v3 1/3] riscv: Set more data to cacheinfo
On Sun, Aug 30, 2020 at 3:54 PM Pekka Enberg wrote: > > Hi, > > On Fri, Aug 28, 2020 at 10:09 AM Zong Li wrote: > > > > Set cacheinfo.{size,sets,line_size} for each cache node, then we can > > get these information from userland through auxiliary vector. > > > > Signed-off-by: Zong Li > > --- > > arch/riscv/kernel/cacheinfo.c | 59 ++- > > 1 file changed, 44 insertions(+), 15 deletions(-) > > > > diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c > > index bd0f122965c3..8b85abfbd77a 100644 > > --- a/arch/riscv/kernel/cacheinfo.c > > +++ b/arch/riscv/kernel/cacheinfo.c > > @@ -25,12 +25,46 @@ cache_get_priv_group(struct cacheinfo *this_leaf) > > return NULL; > > } > > > > -static void ci_leaf_init(struct cacheinfo *this_leaf, > > -struct device_node *node, > > -enum cache_type type, unsigned int level) > > +static void ci_leaf_init(struct cacheinfo *this_leaf, enum cache_type type, > > +unsigned int level, unsigned int size, > > +unsigned int sets, unsigned int line_size) > > { > > this_leaf->level = level; > > this_leaf->type = type; > > + this_leaf->size = size; > > + this_leaf->number_of_sets = sets; > > + this_leaf->coherency_line_size = line_size; > > + > > + /* > > +* If the cache is fully associative, there is no need to > > +* check the other properties. > > +*/ > > + if (!(sets == 1) && (sets > 0 && size > 0 && line_size > 0)) > > Can you explain what this is attempting to do? AFAICT, the if > expression can be reduced to "sets > 1 && size > 0 && size > 0", but > what do you mean with the comment about fully associative caches? If the sets is one, it means that the cache is fully associative, then we don't need to fill the ways number, just keep way number as zero, so here we want to find the fully associative case first and make the if expression fail at the beginning. We might also rewrite it as follow: /* Fully associative */ if (sets == 1) return; /* n-ways associative, make sure all properties are big than zero, it is meaningless when one of them are zero. Full associative case is filtered by the condition above, so we don't need to consider sets == 1 again. */ if (sets > 0 && size > 0 && line_size >0) this_leaf->ways_of_associativity = (size / sets) / line_size; > > > + this_leaf->ways_of_associativity = (size / sets) / > > line_size; > > +}
Re: [PATCH v3 1/3] riscv: Set more data to cacheinfo
Hi, On Fri, Aug 28, 2020 at 10:09 AM Zong Li wrote: > > Set cacheinfo.{size,sets,line_size} for each cache node, then we can > get these information from userland through auxiliary vector. > > Signed-off-by: Zong Li > --- > arch/riscv/kernel/cacheinfo.c | 59 ++- > 1 file changed, 44 insertions(+), 15 deletions(-) > > diff --git a/arch/riscv/kernel/cacheinfo.c b/arch/riscv/kernel/cacheinfo.c > index bd0f122965c3..8b85abfbd77a 100644 > --- a/arch/riscv/kernel/cacheinfo.c > +++ b/arch/riscv/kernel/cacheinfo.c > @@ -25,12 +25,46 @@ cache_get_priv_group(struct cacheinfo *this_leaf) > return NULL; > } > > -static void ci_leaf_init(struct cacheinfo *this_leaf, > -struct device_node *node, > -enum cache_type type, unsigned int level) > +static void ci_leaf_init(struct cacheinfo *this_leaf, enum cache_type type, > +unsigned int level, unsigned int size, > +unsigned int sets, unsigned int line_size) > { > this_leaf->level = level; > this_leaf->type = type; > + this_leaf->size = size; > + this_leaf->number_of_sets = sets; > + this_leaf->coherency_line_size = line_size; > + > + /* > +* If the cache is fully associative, there is no need to > +* check the other properties. > +*/ > + if (!(sets == 1) && (sets > 0 && size > 0 && line_size > 0)) Can you explain what this is attempting to do? AFAICT, the if expression can be reduced to "sets > 1 && size > 0 && size > 0", but what do you mean with the comment about fully associative caches? > + this_leaf->ways_of_associativity = (size / sets) / line_size; > +}