On Tue, Aug 09, 2016 at 12:46:10PM +0300, Jarkko Sakkinen wrote:
> On Fri, Jul 29, 2016 at 10:30:22AM -0700, Dmitry Torokhov wrote:
> > On Fri, Jul 29, 2016 at 10:27 AM, Jason Gunthorpe
> > <jguntho...@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 <apro...@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.
I'm fine having them at the end of the structure mainly for simplicity
reasons but those arguments just didn't hold at all.
/Jarkko
