2013-03-01 08:21:47

by Erik Gilling

[permalink] [raw]
Subject: Re: [PATCH 10/30] staging: sync: Export sync API symbols

On Thu, Feb 28, 2013 at 7:59 PM, John Stultz <[email protected]> wrote:
>>> +EXPORT_SYMBOL(sync_timeline_create);
>>
>> As these are now global, should they be a bit more "specific"? "sync_"
>> seems pretty broad.
>
>
> Given its the sync driver, its most obvious choice, but I agree its likely
> to collide with filesystem related or other sync_ named functions that don't
> have a subsystem prefix.
>
> Any suggestions?
>
> The only good alternative I can think of is that in some private
> conversations with DanielV, he referred to Android using "sync-points".
>
> Erik: Would syncpoint_ be an ok prefix? Or do you have other ideas?

syncpoint would be semantically weird when you end up with struct
syncpoint_pt. I'm open to suggestions as long as it works with
XXXX_pt, XXXX_timeline, and XXXX_fence. I'll ask around the office
and see if someone has a good idea.

>> Also, EXPORT_SYMBOL_GPL() perhaps?
>>
>> And who is using these exports?
>
>
> From some quick git grepping...

As John pointed out, the exynos and msm display and code uses them. I
know nvidia is working on adding suport to their tegra tree. My knee
jerk reaction is to make the export as permissible as possible. That
being said, all of the ARM SoC vendors I've worked with have GPL
kernel drivers even if their user space is closed. I'll reach out to
them and ask for their opinions. Are there any issues with keeping
them EXPORT_SYMBOL?

Cheers,
Erik


2013-03-01 13:55:22

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 10/30] staging: sync: Export sync API symbols

On Fri, Mar 01, 2013 at 12:21:24AM -0800, Erik Gilling wrote:
> On Thu, Feb 28, 2013 at 7:59 PM, John Stultz <[email protected]> wrote:
> >> Also, EXPORT_SYMBOL_GPL() perhaps?
> >>
> >> And who is using these exports?
> >
> >
> > From some quick git grepping...
>
> As John pointed out, the exynos and msm display and code uses them. I
> know nvidia is working on adding suport to their tegra tree. My knee
> jerk reaction is to make the export as permissible as possible. That
> being said, all of the ARM SoC vendors I've worked with have GPL
> kernel drivers even if their user space is closed. I'll reach out to
> them and ask for their opinions. Are there any issues with keeping
> them EXPORT_SYMBOL?

There's no "issues", it's just that it is preferred by some people and
companies to add new symbols to the kernel in this manner. It's really
up to you / Google as you are the ones contributing the code.

thanks,

greg k-h

2013-03-01 16:30:27

by Erik Gilling

[permalink] [raw]
Subject: Re: [PATCH 10/30] staging: sync: Export sync API symbols

On Fri, Mar 1, 2013 at 5:55 AM, Greg KH <[email protected]> wrote:
> On Fri, Mar 01, 2013 at 12:21:24AM -0800, Erik Gilling wrote:
>> As John pointed out, the exynos and msm display and code uses them. I
>> know nvidia is working on adding suport to their tegra tree. My knee
>> jerk reaction is to make the export as permissible as possible. That
>> being said, all of the ARM SoC vendors I've worked with have GPL
>> kernel drivers even if their user space is closed. I'll reach out to
>> them and ask for their opinions. Are there any issues with keeping
>> them EXPORT_SYMBOL?
>
> There's no "issues", it's just that it is preferred by some people and
> companies to add new symbols to the kernel in this manner. It's really
> up to you / Google as you are the ones contributing the code.

Ok. Lets keep it EXPORT_SYMBOL then.

Cheers,
Erik

2013-03-01 22:48:19

by Erik Gilling

[permalink] [raw]
Subject: Re: [PATCH 10/30] staging: sync: Export sync API symbols

On Fri, Mar 1, 2013 at 12:21 AM, Erik Gilling <[email protected]> wrote:
> On Thu, Feb 28, 2013 at 7:59 PM, John Stultz <[email protected]> wrote:
>> Given its the sync driver, its most obvious choice, but I agree its likely
>> to collide with filesystem related or other sync_ named functions that don't
>> have a subsystem prefix.
>>
>> Any suggestions?
>>
>> The only good alternative I can think of is that in some private
>> conversations with DanielV, he referred to Android using "sync-points".
>>
>> Erik: Would syncpoint_ be an ok prefix? Or do you have other ideas?
>
> syncpoint would be semantically weird when you end up with struct
> syncpoint_pt. I'm open to suggestions as long as it works with
> XXXX_pt, XXXX_timeline, and XXXX_fence. I'll ask around the office
> and see if someone has a good idea.

Colin Cross pointed out that this is limited to sync_fence_*,
sync_pt_*, and sync_timeline_* and not sync_* so it's much less likely
to have naming collisions.

Cheers,
Erik