| Message ID | 20190821145837.3686-4-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] riscv: refactor the IPI code | expand |
On Wed, 2019-08-21 at 23:58 +0900, Christoph Hellwig wrote: > Don't go through send_ipi_mask, but just set the op bit an then pass > a > simple generate hartid mask directly to sbi_send_ipi. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/riscv/kernel/smp.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c > index 2e21669aa068..a3715d621f60 100644 > --- a/arch/riscv/kernel/smp.c > +++ b/arch/riscv/kernel/smp.c > @@ -94,7 +94,13 @@ static void send_ipi_mask(const struct cpumask > *mask, enum ipi_message_type op) > > static void send_ipi_single(int cpu, enum ipi_message_type op) > { > - send_ipi_mask(cpumask_of(cpu), op); > + int hartid = cpuid_to_hartid_map(cpu); > + > + smp_mb__before_atomic(); > + set_bit(op, &ipi_data[cpu].bits); > + smp_mb__after_atomic(); > + > + sbi_send_ipi(cpumask_bits(cpumask_of(hartid))); The only cost save I see is you don't have to cpumask_clear anymore. Is there any other cost save ? If not is it worth duplicating the code ? May be I am being too pedantic here.. :) :) > } > > static inline void clear_ipi(void)
On Sat, Aug 24, 2019 at 12:26:26AM +0000, Atish Patra wrote: > > static void send_ipi_single(int cpu, enum ipi_message_type op) > > { > > - send_ipi_mask(cpumask_of(cpu), op); > > + int hartid = cpuid_to_hartid_map(cpu); > > + > > + smp_mb__before_atomic(); > > + set_bit(op, &ipi_data[cpu].bits); > > + smp_mb__after_atomic(); > > + > > + sbi_send_ipi(cpumask_bits(cpumask_of(hartid))); > > The only cost save I see is you don't have to cpumask_clear anymore. > Is there any other cost save ? If not is it worth duplicating the code > ? > > May be I am being too pedantic here.. :) :) It avoids the additional potentially huge cpumask, and generally makes the code a lot more obvious. This might not really be needed, but helps with sharing the code nicely with the native IPI path.
On Mon, 2019-08-26 at 13:29 +0200, hch@lst.de wrote: > On Sat, Aug 24, 2019 at 12:26:26AM +0000, Atish Patra wrote: > > > static void send_ipi_single(int cpu, enum ipi_message_type op) > > > { > > > - send_ipi_mask(cpumask_of(cpu), op); > > > + int hartid = cpuid_to_hartid_map(cpu); > > > + > > > + smp_mb__before_atomic(); > > > + set_bit(op, &ipi_data[cpu].bits); > > > + smp_mb__after_atomic(); > > > + > > > + sbi_send_ipi(cpumask_bits(cpumask_of(hartid))); > > > > The only cost save I see is you don't have to cpumask_clear > > anymore. > > Is there any other cost save ? If not is it worth duplicating the > > code > > ? > > > > May be I am being too pedantic here.. :) :) > > It avoids the additional potentially huge cpumask, and generally > makes > the code a lot more obvious. This might not really be needed, but > helps with sharing the code nicely with the native IPI path. ok. Reviewed-by: Atish Patra <atish.patra@wdc.com>
On Wed, 21 Aug 2019, Christoph Hellwig wrote: > Don't go through send_ipi_mask, but just set the op bit an then pass a > simple generate hartid mask directly to sbi_send_ipi. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Thanks, I fixed some minor issues in the patch description and queued the following for v5.4-rc1. - Paul From: Christoph Hellwig <hch@lst.de> Date: Wed, 21 Aug 2019 23:58:34 +0900 Subject: [PATCH] riscv: optimize send_ipi_single Don't go through send_ipi_mask, but just set the op bit and then pass a simple generated hartid mask directly to sbi_send_ipi. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Atish Patra <atish.patra@wdc.com> [paul.walmsley@sifive.com: minor patch description fixes] Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com> --- arch/riscv/kernel/smp.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c index 2e21669aa068..a3715d621f60 100644 --- a/arch/riscv/kernel/smp.c +++ b/arch/riscv/kernel/smp.c @@ -94,7 +94,13 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op) static void send_ipi_single(int cpu, enum ipi_message_type op) { - send_ipi_mask(cpumask_of(cpu), op); + int hartid = cpuid_to_hartid_map(cpu); + + smp_mb__before_atomic(); + set_bit(op, &ipi_data[cpu].bits); + smp_mb__after_atomic(); + + sbi_send_ipi(cpumask_bits(cpumask_of(hartid))); } static inline void clear_ipi(void)
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c index 2e21669aa068..a3715d621f60 100644 --- a/arch/riscv/kernel/smp.c +++ b/arch/riscv/kernel/smp.c @@ -94,7 +94,13 @@ static void send_ipi_mask(const struct cpumask *mask, enum ipi_message_type op) static void send_ipi_single(int cpu, enum ipi_message_type op) { - send_ipi_mask(cpumask_of(cpu), op); + int hartid = cpuid_to_hartid_map(cpu); + + smp_mb__before_atomic(); + set_bit(op, &ipi_data[cpu].bits); + smp_mb__after_atomic(); + + sbi_send_ipi(cpumask_bits(cpumask_of(hartid))); } static inline void clear_ipi(void)
Don't go through send_ipi_mask, but just set the op bit an then pass a simple generate hartid mask directly to sbi_send_ipi. Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/riscv/kernel/smp.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)