| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <PH7PR11MB5862283EF5540BBF7BF0C853F7EE2@PH7PR11MB5862.namprd11.prod.outlook.com>
Date: Wed, 29 Jan 2025 09:30:10 +0000
From: "Mohan, Subramanian" <subramanian.mohan@...el.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"akpm@...ux-foundation.org" <akpm@...ux-foundation.org>, "greg@...ah.com"
<greg@...ah.com>, "corbet@....net" <corbet@....net>, "Hall, Christopher S"
<christopher.s.hall@...el.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
"Dong, Eddie" <eddie.dong@...el.com>, "N, Pandith" <pandith.n@...el.com>, "T
R, Thejesh Reddy" <thejesh.reddy.t.r@...el.com>, "Zage, David"
<david.zage@...el.com>, "Chinnadurai, Srinivasan"
<srinivasan.chinnadurai@...el.com>, "rdunlap@...radead.org"
<rdunlap@...radead.org>, "bagasdotme@...il.com" <bagasdotme@...il.com>,
"giometti@...eenne.com" <giometti@...eenne.com>
Subject: RE: [PATCH v2 1/1] pps: retrieve generator specific data from
framework
HI Andy,
> -----Original Message-----
> From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Sent: Wednesday, January 29, 2025 12:24 PM
> To: Mohan, Subramanian <subramanian.mohan@...el.com>
> Cc: linux-doc@...r.kernel.org; linux-kernel@...r.kernel.org; akpm@...ux-
> foundation.org; greg@...ah.com; corbet@....net; Hall, Christopher S
> <christopher.s.hall@...el.com>; tglx@...utronix.de; Dong, Eddie
> <eddie.dong@...el.com>; N, Pandith <pandith.n@...el.com>; T R, Thejesh
> Reddy <thejesh.reddy.t.r@...el.com>; Zage, David <david.zage@...el.com>;
> Chinnadurai, Srinivasan <srinivasan.chinnadurai@...el.com>;
> rdunlap@...radead.org; bagasdotme@...il.com; giometti@...eenne.com
> Subject: Re: [PATCH v2 1/1] pps: retrieve generator specific data from
> framework
>
> On Tue, Jan 28, 2025 at 07:47:43PM +0530, subramanian.mohan@...el.com
> wrote:
> > From: Subramanian Mohan <subramanian.mohan@...el.com>
> >
> > While adapting pps generator driver(tio generator as an example)to the
> > new generator framework, As part of driver registration the
> > pps_gen_device pointer is returned from framework. Due to which there
> > is difficulty in getting generator driver data back in enable
> > function. we won't be able to use container_of macro as it results in
> > static assert. we might end up
>
> container_of()
>
> Can you be more specific, what exactly happens?
Let me provide some more info:
/* PPS Gen Device main struct*/
struct pps_gen_device {
struct pps_gen_source_info info; /* PSS generator info */
bool enabled; /* PSS generator status */
......
/* More members */
}
/* TIO struct */
struct pps_tio {
struct pps_gen_source_info pps_tio_source_info;
struct pps_gen_device *pps_gen;
struct hrtimer timer;
struct device *dev;
spinlock_t lock;
........
/* More members */
};
Once the framework registration is done in probe function through pps_gen_register_source we get back
pps_gen pointer and stored in our struct pps_tio.
As part of enable function:
We are trying to get back pps_tio (parent structure) with the help of pps_gen as this is pointer container_of does not work here.
Hence proposed add a new generic private pointer inside the pps_gen_source_info structure which is exposed to driver.
static int pps_tio_enable(struct pps_gen_device *pps_gen, bool enable)
{
/* Trying to get back pps_tio struct is not possible as pps_gen is a pointer in structure */
container_of(pps_gen, struct pps_tio, pps_gen);
}
Compilation Error:
In file included from ./include/linux/bitfield.h:10,
from drivers/pps/generators/pps_gen_tio.c:8:
drivers/pps/generators/pps_gen_tio.c: In function 'pps_tio_gen_enable':
./include/linux/build_bug.h:78:41: error: static assertion failed: "pointer type mismatch in container_of()"
78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
| ^~~~~~~~~~~~~~
./include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert'
77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr)
| ^~~~~~~~~~~~~~~
./include/linux/container_of.h:20:2: note: in expansion of macro 'static_assert'
20 | static_assert(__same_type(*(ptr), ((type *)0)->member) || \
| ^~~~~~~~~~~~~
drivers/pps/generators/pps_gen_tio.c:172:24: note: in expansion of macro 'container_of'
172 | struct pps_tio *tio = container_of(pps_gen,struct pps_tio, pps_gen);
>
> > in using static pointer. To avoid the same and get back the generator
> > driver data back we are proposing generic approach to add drv_prv_data
> > pointer inside the struct pps_gen_source_info.
>
> > Example TIO structure wrapped with pps_gen_device and usage.
> >
> > struct pps_tio {
> > /* Framework Related * /
> > struct pps_gen_source_info pps_tio_source_info
> > struct pps_gen_device *pps_gen;
>
> > };
>
> > static int pps_tio_enable(struct pps_gen_device *pps_gen, bool enable)
> > {
> >
> > /* Getting TIO data back */
> > /* Note: drv_prv_data will be initialized in our init routine */
> > struct pps_tio *tio = pps_gen->info.drv_prv_data;
>
> So, what's wrong with the container_of() against pps_gen->info?
> We have tons of code in the kernel that does it.
We are trying to get our TIO/struct pps_tio structure back using pps_gen pointer..
>
> > /* Access tio members here to set some of the parameters */
> >
> > return 0;
> > }
>
> Okay, looking into the implementation I see what's the issue (but it doesn't
> mean that commit message is good here, you still need to explain better why
> you can't get the correct address).
Ok, Something like this:
"Adding new generic private pointer to the struct pps_gen_source_info which can help in saving the parent driver data.
In the existing adaption using container_of with a pointer to fetch the parent driver data/address is not possible"
"Example as shown above can be given as reference"
>
> The problem is that info is got fully copied. Perhaps we should rely on the fact
> that it always be provided? I dunno any data structure that defines callbacks
> and other "info" material should be considered as temporary storage (on
> stack), it makes not much sense.
We are trying to get our TIO/struct pps_tio structure back using pps_gen pointer..
>
> ...
>
> > V1 -> V2:
> > * Updated reviewers.
>
> Please, use...
>
> > Signed-off-by: Subramanian Mohan <subramanian.mohan@...el.com>
> > ---
>
> ...this place for the comments and changelogs.
Ok Will take care in next version.
>
> ...
>
> > + void *drv_prv_data;
>
> If really needed we should probably use a better name, like "private".
Ok, will change the member name to private.
Thanks,
Subbu
>
> --
> With Best Regards,
> Andy Shevchenko
>
Powered by blists - more mailing lists