Re: [PATCH] counter: fix privdata alignment

[William Breathitt Gray <william.gray@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 2109 bytes --]

On Fri, Feb 09, 2024 at 10:07:19AM +0100, Nuno Sá wrote:
> On Fri, 2024-02-09 at 09:30 +0100, Uwe Kleine-König wrote:
> > Hi Nuno,
> > 
> > On Fri, Feb 09, 2024 at 08:42:02AM +0100, Nuno Sá wrote:
> > > On Thu, 2024-02-08 at 13:34 -0500, William Breathitt Gray wrote:
> > > > On Mon, Feb 05, 2024 at 04:58:14PM +0100, Nuno Sa via B4 Relay wrote:
> > > > > diff --git a/drivers/counter/counter-core.c b/drivers/counter/counter-
> > > > > core.c
> > > > > index 09c77afb33ca..073bf6b67a57 100644
> > > > > --- a/drivers/counter/counter-core.c
> > > > > +++ b/drivers/counter/counter-core.c
> > > > > @@ -34,7 +34,7 @@ struct counter_device_allochelper {
> > > > >  	 * This is cache line aligned to ensure private data behaves
> > > > > like if it
> > > > >  	 * were kmalloced separately.
> > > > >  	 */
> > > > > -	unsigned long privdata[] ____cacheline_aligned;
> > > > > +	unsigned long privdata[] __aligned(ARCH_DMA_MINALIGN);
> > > > >  };
> > > > >  
> > > > >  static void counter_device_release(struct device *dev)
> > > > > 
> > > > 
> > > > This change sounds reasonable, but should the comment block above
> > > > privdata be updated to reflect the change?
> > > 
> > > Yeah, maybe. I can spin a new version with that... To be sure, you mean (in
> > > the
> > > comment) private -> privdata, right?
> > 
> > I guess he means: "This is cache line aligned to ensure private data
> > behaves like if it were kmalloced separately." After your change it's
> > not cache line aligned any more. IMHO keeping "private" is fine.
> > 
> > 
> 
> Oh yeah...
> 
> Yeah, it will depend on the platform. In some, it will still be cache aligned
> but in others (as x86 which is DMA coeherent I think), it won't be and we can
> actually safe some memory.
> 
> - Nuno Sá

Yes, I was referring to the possibility that it won't be cache aligned
anymore in some platforms as you mentioned, so a different comment would
be better there now. You can keep the "private" wording if you like, or
use "privdata" if you think it's clearer.

Thanks,

William Breathitt Gray

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]