Messages in this thread | | | Date | Tue, 9 Aug 2016 12:46:10 +0300 | From | Jarkko Sakkinen <> | Subject | Re: [PATCH] tpm: fix cacheline alignment for DMA-able buffers |
| |
On Fri, Jul 29, 2016 at 10:30:22AM -0700, Dmitry Torokhov wrote: > On Fri, Jul 29, 2016 at 10:27 AM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > On Thu, Jul 28, 2016 at 07:59:13PM -0700, Andrey Pronin wrote: > > Annotate buffers used in spi transactions as ____cacheline_aligned > > to use in DMA transfers. > > > > Signed-off-by: Andrey Pronin <apronin@chromium.org> > > drivers/char/tpm/st33zp24/spi.c | 4 ++-- > > drivers/char/tpm/tpm_tis_spi.c | 4 ++-- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/char/tpm/st33zp24/spi.c > b/drivers/char/tpm/st33zp24/spi.c > > index 9f5a011..0e9aad9 100644 > > +++ b/drivers/char/tpm/st33zp24/spi.c > > @@ -70,8 +70,8 @@ > > struct st33zp24_spi_phy { > >    struct spi_device *spi_device; > > > > -   u8 tx_buf[ST33ZP24_SPI_BUFFER_SIZE]; > > -   u8 rx_buf[ST33ZP24_SPI_BUFFER_SIZE]; > > +   u8 tx_buf[ST33ZP24_SPI_BUFFER_SIZE] ____cacheline_aligned; > > +   u8 rx_buf[ST33ZP24_SPI_BUFFER_SIZE] ____cacheline_aligned; > > > >    int io_lpcpd; > >    int latency; > > Hurm, this still looks wrong to me. Aligning the start of buffers is > not enough, the DMA'able space must also end on a cache line as well. > > So, the buffers must also always be placed at the end of the struct. > > IMHO It would be cleaner and safer to always kmalloc the DMA buffer > alone than to try and optimize like this. > > In this case moving them to the end of the structure and commenting why > they have to be at the end might be less invasive change. More > performance-efficient and resilient in low memory situations too.
kmallocs would be done in the driver initialization:
* you rarely are in low memory situation * performance gain/loss is insignificant
I really don't see your point.
> Thanks, > Dmitry
/Jarkko
| |