2006-03-21 19:50:41

by Kristen Carlson Accardi

[permalink] [raw]
Subject: [patch] add private data to notifier_block

While most current uses of notifier_block use a global struct, I would
like to be able to use it on a per device basis for drivers which have
multiple device instances. I would also like to be able to have a
private data struct associated with the notifier block so that per
device data can be easily accessed. This patch will modify the
notifier_block struct to add a void *, and will require no modifications
to any other users of the notifier_block.

Signed-off-by: Kristen Carlson Accardi <[email protected]>

---
include/linux/notifier.h | 1 +
1 files changed, 1 insertion(+)

--- 2.6-git-kca.orig/include/linux/notifier.h
+++ 2.6-git-kca/include/linux/notifier.h
@@ -15,6 +15,7 @@ struct notifier_block
{
int (*notifier_call)(struct notifier_block *self, unsigned long, void *);
struct notifier_block *next;
+ void *data;
int priority;
};


2006-03-22 18:00:44

by Zach Brown

[permalink] [raw]
Subject: Re: [patch] add private data to notifier_block

Kristen Accardi wrote:

> device data can be easily accessed. This patch will modify the
> notifier_block struct to add a void *, and will require no modifications
> to any other users of the notifier_block.

> {
> int (*notifier_call)(struct notifier_block *self, unsigned long, void *);
> struct notifier_block *next;
> + void *data;
> int priority;
> };

Well, it might introduce warnings in users who weren't using named
initializers -- their bare priority initialization might now shift to
trying to initialize a pointer.

Though, that's probably a good thing as it gives an opportunity to
convert them. We also don't want to create a padded structure by
putting the void * after the int.

- z

2006-03-22 19:04:05

by Alan Stern

[permalink] [raw]
Subject: Re: [patch] add private data to notifier_block

On Tue, 21 Mar 2006, Kristen Accardi wrote:

> While most current uses of notifier_block use a global struct, I would
> like to be able to use it on a per device basis for drivers which have
> multiple device instances. I would also like to be able to have a
> private data struct associated with the notifier block so that per
> device data can be easily accessed. This patch will modify the
> notifier_block struct to add a void *, and will require no modifications
> to any other users of the notifier_block.
>
> Signed-off-by: Kristen Carlson Accardi <[email protected]>
>
> ---
> include/linux/notifier.h | 1 +
> 1 files changed, 1 insertion(+)
>
> --- 2.6-git-kca.orig/include/linux/notifier.h
> +++ 2.6-git-kca/include/linux/notifier.h
> @@ -15,6 +15,7 @@ struct notifier_block
> {
> int (*notifier_call)(struct notifier_block *self, unsigned long, void *);
> struct notifier_block *next;
> + void *data;
> int priority;
> };

I still think this isn't really needed. The same effect can be
accomplished by embedding a notifier_block struct within a larger
structure that also contains the data pointer.

On the other hand this isn't a terribly big change, so I don't actually
object to it.

Alan Stern

2006-03-22 20:53:12

by Kristen Carlson Accardi

[permalink] [raw]
Subject: Re: [patch] add private data to notifier_block

On Wed, 2006-03-22 at 14:04 -0500, Alan Stern wrote:
> On Tue, 21 Mar 2006, Kristen Accardi wrote:
>
> > While most current uses of notifier_block use a global struct, I would
> > like to be able to use it on a per device basis for drivers which have
> > multiple device instances. I would also like to be able to have a
> > private data struct associated with the notifier block so that per
> > device data can be easily accessed. This patch will modify the
> > notifier_block struct to add a void *, and will require no modifications
> > to any other users of the notifier_block.
> >
> > Signed-off-by: Kristen Carlson Accardi <[email protected]>
> >
> > ---
> > include/linux/notifier.h | 1 +
> > 1 files changed, 1 insertion(+)
> >
> > --- 2.6-git-kca.orig/include/linux/notifier.h
> > +++ 2.6-git-kca/include/linux/notifier.h
> > @@ -15,6 +15,7 @@ struct notifier_block
> > {
> > int (*notifier_call)(struct notifier_block *self, unsigned long, void *);
> > struct notifier_block *next;
> > + void *data;
> > int priority;
> > };
>
> I still think this isn't really needed. The same effect can be
> accomplished by embedding a notifier_block struct within a larger
> structure that also contains the data pointer.
>

I thought of this, but felt it would make for less easy to read code.
But, that's just my personal style.

> On the other hand this isn't a terribly big change, so I don't actually
> object to it.
>

Thanks.

> Alan Stern

2006-03-23 10:06:20

by Jes Sorensen

[permalink] [raw]
Subject: Re: [patch] add private data to notifier_block

>>>>> "Kristen" == Kristen Accardi <[email protected]> writes:

Kristen> On Wed, 2006-03-22 at 14:04 -0500, Alan Stern wrote:
>> I still think this isn't really needed. The same effect can be
>> accomplished by embedding a notifier_block struct within a larger
>> structure that also contains the data pointer.

Kristen> I thought of this, but felt it would make for less easy to
Kristen> read code. But, that's just my personal style.

I'd have to vote for Alan's side here. Thats why we have the
containerof() stuff. It also means you can embed more than just a
pointer with the notifier block and it will generate more efficient
code when doing so.

Cheers,
Jes

2006-03-23 17:54:51

by Kristen Carlson Accardi

[permalink] [raw]
Subject: Re: [patch] add private data to notifier_block

On Thu, 2006-03-23 at 05:06 -0500, Jes Sorensen wrote:
> >>>>> "Kristen" == Kristen Accardi <[email protected]> writes:
>
> Kristen> On Wed, 2006-03-22 at 14:04 -0500, Alan Stern wrote:
> >> I still think this isn't really needed. The same effect can be
> >> accomplished by embedding a notifier_block struct within a larger
> >> structure that also contains the data pointer.
>
> Kristen> I thought of this, but felt it would make for less easy to
> Kristen> read code. But, that's just my personal style.
>
> I'd have to vote for Alan's side here. Thats why we have the
> containerof() stuff. It also means you can embed more than just a
> pointer with the notifier block and it will generate more efficient
> code when doing so.
>
> Cheers,
> Jes

Ok, I bow to peer pressure and will do this your way. :) You can ignore
this patch.

Thanks,
Kristen