Re: [Outreachy kernel] [PATCH] staging: iio: adc: Replace mlock with driver private lock

[Alison Schofield <amsfield22@gmail.com>]

On Wed, Mar 15, 2017 at 11:45:11PM +0530, Gargi Sharma wrote:
> On Wed, Mar 15, 2017 at 4:14 AM, Jonathan Cameron
> <jic23@jic23.retrosnub.co.uk> wrote:
> >
> >
> > On 14 March 2017 17:36:16 GMT+00:00, Gargi Sharma <gs051095@gmail.com> wrote:
> >>On Tue, Mar 14, 2017 at 2:24 AM, Lars-Peter Clausen <lars@metafoo.de>
> >>wrote:
> >>> On 03/13/2017 09:51 PM, Jonathan Cameron wrote:
> >>> [...]
> >>>>>> Gargi,
> >>>>>>
> >>>>>> Please insert the new lock above the __cacheline_aligned struct
> >>>>>> member.
> >>>>>
> >>>>> I will do that, but is there any reason why the lock should be
> >>above
> >>>>> ____cacheline_aligned struct member?
> >>>> Yes.  It's in fact very important that nothing comes after that.
> >>>>
> >>>> Will leave the why as an exercise for the reader.   I'll give the
> >>>> hit of spi drivers that do DMA...
> >>>
> >>>
> >>> One hint though, the answer is somewhere in Documentation/DMA-API.txt
> >>
> > >From what I have understood is that cacheline is used for keeping most
> >>frequently accessed values in adjacent cachelines.
> >
> > Why? What does cache line actually mean?
> >
> >> We want the
> >>cacheline_aligned at the end in the struct definition so as to avoid
> >>any holes after the cacheline boundary.
> >
> > Nope.  Try
> > http://www.pebblebay.com/a-guide-to-using-direct-memory-access-in-embedded-systems-part-two/
> >
> > (Hint, search for 'shares')
> 
> Okay, I got two things from the article:
> 1. Hardware that uses DMA reads faster if the data is aligned at
> 16/32/64B boundaries.
> 2. You want to align buffer being accessed for DMa transfers to
> prevent cache incoherence.
> 
> What I still don't understand is why nothing should come after
> __cacheline_aligned struct member?
> 
> Thanks,
> Gargi

Hi Gargi,

Short answer to why nothing can come after, is because it could
indeed share the cacheline.  ___cacheline_aligned only guarantees
alignment at the beginning of the cacheline.

I've been following and reading the posted links.  Here's another
link, kind of ancient, but does a nice explanation of the process
whereby that extra field, sharing your buffers cacheline, could
lead to corruption of that field or the buffer data.
https://lwn.net/Articles/2265/

I still haven't bottomed out on this though. I get the need
to do this to be safe, but I'd like to understand how it fits
in with systems that claim cache coherency.  Is this a case
those schemes cannot handle, or, are there MP systems that
are just not cache incoherent?

alisons
> >
> >>>
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> >>the body of a message to majordomo@vger.kernel.org
> >>More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Sent from my Android device with K-9 Mail. Please excuse my brevity.