| Message ID | 158859897252.19836.5614675872684760741.stgit@buzz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] block/part_stat: remove rcu_read_lock() from part_stat_lock() | expand |
> +#define __part_stat_add(part, field, addnd) \ > + (part_stat_get(part, field) += (addnd)) Just open coding part_stat_get for the UP side would seems a little easier to read. Otherwise this looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
On 04/05/2020 17.03, Christoph Hellwig wrote: >> +#define __part_stat_add(part, field, addnd) \ >> + (part_stat_get(part, field) += (addnd)) > > Just open coding part_stat_get for the UP side would seems a little > easier to read. If rewrite filed definition as #ifdef CONFIG_SMP struct disk_stats __percpu *dkstats; #else struct disk_stats __percpu dkstats[1]; #endif Then all per-cpu macro should work as is for UP case too. Surprisingly arrow operator (struct->filed) works for arrays too =) Inlining per-cpu structures should be good for tiny UP systems. Especially if this could be done by few macro only in three places: definition, allocating and freeing. But sparse still complains. ./include/linux/part_stat.h:45:24: warning: incorrect type in initializer (different address spaces) ./include/linux/part_stat.h:45:24: expected void const [noderef] <asn:3> *__vpp_verify ./include/linux/part_stat.h:45:24: got struct disk_stats [noderef] * Looks like it cannot assign different address-space to the filed. > > Otherwise this looks good: > > Reviewed-by: Christoph Hellwig <hch@lst.de> >
On Mon, May 04, 2020 at 06:02:28PM +0300, Konstantin Khlebnikov wrote: > Then all per-cpu macro should work as is for UP case too. > Surprisingly arrow operator (struct->filed) works for arrays too =) > > > Inlining per-cpu structures should be good for tiny UP systems. > Especially if this could be done by few macro only in three places: > definition, allocating and freeing. Or we could just use the percpu ops always, which is what most users do. I never really go the UP microoptmization here.
diff --git a/include/linux/part_stat.h b/include/linux/part_stat.h index 755a01f0fd61..a0ddeff3798e 100644 --- a/include/linux/part_stat.h +++ b/include/linux/part_stat.h @@ -36,6 +36,9 @@ res; \ }) +#define __part_stat_add(part, field, addnd) \ + __this_cpu_add((part)->dkstats->field, addnd) + static inline void part_stat_set_all(struct hd_struct *part, int value) { int i; @@ -64,6 +67,9 @@ static inline void free_part_stats(struct hd_struct *part) #define part_stat_get_cpu(part, field, cpu) part_stat_get(part, field) #define part_stat_read(part, field) part_stat_get(part, field) +#define __part_stat_add(part, field, addnd) \ + (part_stat_get(part, field) += (addnd)) + static inline void part_stat_set_all(struct hd_struct *part, int value) { memset(&part->dkstats, value, sizeof(struct disk_stats)); @@ -85,9 +91,6 @@ static inline void free_part_stats(struct hd_struct *part) part_stat_read(part, field[STAT_WRITE]) + \ part_stat_read(part, field[STAT_DISCARD])) -#define __part_stat_add(part, field, addnd) \ - (part_stat_get(part, field) += (addnd)) - #define part_stat_add(part, field, addnd) do { \ __part_stat_add((part), field, addnd); \ if ((part)->partno) \
Most architectures have fast path to access percpu for current cpu. Required preempt_disable() is provided by part_stat_lock(). Signed-off-by: Konstantin Khlebnikov <khlebnikov@yandex-team.ru> --- include/linux/part_stat.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)