On 2024/3/25 22:21, Jason Yan wrote: > On 2024/3/25 22:03, Damien Le Moal wrote: >> On 3/25/24 22:17, Yihang Li wrote: >>> This series [1] reducing the kmalloc() minimum alignment on arm64 to 8 >>> (from 128). >> >> And ? What is the point you are trying to convey here ? >> >>> The hisi_sas has special requirements on the memory address alignment >>> (must be 16-byte-aligned) of the command request frame, so add a SMP >>> request allocation callback and fill it in for the hisi_sas driver. >> >> 128 is aligned to 16. So what is the problem you are trying to solve here ? >> Can you clarify ? I suspect this is all about memory allocation optimization ? > > After series [1] been merged, kmalloc is 8-byte-aligned, however hisi_sas hardware needs 16-byte-aligned. That's the problem. Yes, that's the problem. I will explain it in detail in the next version. > >> >>> >>> Link: https://lkml.kernel.org/r/20230612153201.554742-1-catalin.marinas@xxxxxxx [1] >>> Signed-off-by: Yihang Li <liyihang9@xxxxxxxxxx> >>> --- >>> drivers/scsi/hisi_sas/hisi_sas_main.c | 14 ++++++++++++ >>> drivers/scsi/libsas/sas_expander.c | 31 ++++++++++++++++++--------- >>> include/scsi/libsas.h | 3 +++ >>> 3 files changed, 38 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c >>> index 097dfe4b620d..40329558d435 100644 >>> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c >>> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c >>> @@ -2031,6 +2031,19 @@ static int hisi_sas_write_gpio(struct sas_ha_struct *sha, u8 reg_type, >>> reg_index, reg_count, write_data); >>> } >>> +static void *hisi_sas_alloc_smp_req(int size) >>> +{ >>> + u8 *p; >>> + >>> + /* The address must be 16-byte-aligned. */ >> >> ARCH_DMA_MINALIGN is not always 16, right ? >> >>> + size = ALIGN(size, ARCH_DMA_MINALIGN); >>> + p = kzalloc(size, GFP_KERNEL); >>> + if (p) >>> + p[0] = SMP_REQUEST; >>> + >>> + return p; >>> +} >>> + >>> static void hisi_sas_phy_disconnected(struct hisi_sas_phy *phy) >>> { >>> struct asd_sas_phy *sas_phy = &phy->sas_phy; >>> @@ -2130,6 +2143,7 @@ static struct sas_domain_function_template hisi_sas_transport_ops = { >>> .lldd_write_gpio = hisi_sas_write_gpio, >>> .lldd_tmf_aborted = hisi_sas_tmf_aborted, >>> .lldd_abort_timeout = hisi_sas_internal_abort_timeout, >>> + .lldd_alloc_smp_req = hisi_sas_alloc_smp_req, >> >> Why this complexity ? Why not simply modify alloc_smp_req() to have the required >> alignment ? This will avoid a costly indirect function call. > > Yeah, I think it's simpler to modify alloc_smp_req() directly too. Yihang, Can you please cook a new one? Sure, I will post a new version later. The reason why I did not directly modify alloc_smp_req() is that I think the requirement that the command frame must be 16-byte aligned is hisi_sas private and not a general requirement. To avoid the impact on other drivers, I only add aligned to 16B in hisi_sas. If this measure is considered pointless, I will directly modify alloc_smp_req(). Thanks, Yihang > > Thansk, > Jason > . >
