2005-03-04 00:54:41

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] unexport complete_all

I didn't find any possible modular usage in the kernel.

Signed-off-by: Adrian Bunk <[email protected]>

--- linux-2.6.11-rc5-mm1-full/kernel/sched.c.old 2005-03-04 01:04:28.000000000 +0100
+++ linux-2.6.11-rc5-mm1-full/kernel/sched.c 2005-03-04 01:04:34.000000000 +0100
@@ -3053,7 +3053,6 @@
0, 0, NULL);
spin_unlock_irqrestore(&x->wait.lock, flags);
}
-EXPORT_SYMBOL(complete_all);

void fastcall __sched wait_for_completion(struct completion *x)
{


2005-03-04 08:15:31

by Mike Waychison

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

> I didn't find any possible modular usage in the kernel.
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> --- linux-2.6.11-rc5-mm1-full/kernel/sched.c.old 2005-03-04 01:04:28.000000000 +0100
> +++ linux-2.6.11-rc5-mm1-full/kernel/sched.c 2005-03-04 01:04:34.000000000 +0100
> @@ -3053,7 +3053,6 @@
> 0, 0, NULL);
> spin_unlock_irqrestore(&x->wait.lock, flags);
> }
> -EXPORT_SYMBOL(complete_all);
>
> void fastcall __sched wait_for_completion(struct completion *x)
> {
> -

This is a valid piece of API that is exported for future use.

Please stop blindly posting patches for unexports that make the APIs
half available.

--
Mike Waychison

Subject: Re: [2.6 patch] unexport complete_all

On Fri, 04 Mar 2005 03:09:39 -0500, Mike Waychison <[email protected]> wrote:
> > I didn't find any possible modular usage in the kernel.
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
> >
> > --- linux-2.6.11-rc5-mm1-full/kernel/sched.c.old 2005-03-04 01:04:28.000000000 +0100
> > +++ linux-2.6.11-rc5-mm1-full/kernel/sched.c 2005-03-04 01:04:34.000000000 +0100
> > @@ -3053,7 +3053,6 @@
> > 0, 0, NULL);
> > spin_unlock_irqrestore(&x->wait.lock, flags);
> > }
> > -EXPORT_SYMBOL(complete_all);
> >
> > void fastcall __sched wait_for_completion(struct completion *x)
> > {
> > -
>
> This is a valid piece of API that is exported for future use.

Let me guess: autofsng?

It was you who added it (through akpm):


[PATCH] export complete_all()

From: Mike Waychison <[email protected]>

Export complete_all for module use.


Andrew, what is the policy for adding exports for out of tree GPL code?

IMHO although it is convenient for maintainers of such code it is
inconvenient for us (ie. when making changes to code) and gives
people false assumptions about stability of *in-kernel* APIs.

Bartlomiej

2005-03-04 11:15:57

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

On Fri, Mar 04, 2005 at 03:09:39AM -0500, Mike Waychison wrote:
> > I didn't find any possible modular usage in the kernel.
> >
> > Signed-off-by: Adrian Bunk <[email protected]>
> >
> > --- linux-2.6.11-rc5-mm1-full/kernel/sched.c.old 2005-03-04 01:04:28.000000000 +0100
> > +++ linux-2.6.11-rc5-mm1-full/kernel/sched.c 2005-03-04 01:04:34.000000000 +0100
> > @@ -3053,7 +3053,6 @@
> > 0, 0, NULL);
> > spin_unlock_irqrestore(&x->wait.lock, flags);
> > }
> > -EXPORT_SYMBOL(complete_all);
> >
> > void fastcall __sched wait_for_completion(struct completion *x)
> > {
> > -
>
> This is a valid piece of API that is exported for future use.
>...

You exported this function nearly one year ago with the only comment
"Export complete_all for module use.".

Why did you add the export last year instead of simply adding it when it
will be required?

> Mike Waychison

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-03-04 11:15:56

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

On Fri, Mar 04, 2005 at 11:40:08AM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Fri, 04 Mar 2005 03:09:39 -0500, Mike Waychison <[email protected]> wrote:
> > > I didn't find any possible modular usage in the kernel.
> > >
> > > Signed-off-by: Adrian Bunk <[email protected]>
> > >
> > > --- linux-2.6.11-rc5-mm1-full/kernel/sched.c.old 2005-03-04 01:04:28.000000000 +0100
> > > +++ linux-2.6.11-rc5-mm1-full/kernel/sched.c 2005-03-04 01:04:34.000000000 +0100
> > > @@ -3053,7 +3053,6 @@
> > > 0, 0, NULL);
> > > spin_unlock_irqrestore(&x->wait.lock, flags);
> > > }
> > > -EXPORT_SYMBOL(complete_all);
> > >
> > > void fastcall __sched wait_for_completion(struct completion *x)
> > > {
> > > -
> >
> > This is a valid piece of API that is exported for future use.
>
> Let me guess: autofsng?
>...

I grepped through autofsng-0.4 but didn't find any usage.

> Bartlomiej

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-03-04 12:20:01

by Andrew Morton

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

Bartlomiej Zolnierkiewicz <[email protected]> wrote:
>
> Andrew, what is the policy for adding exports for out of tree GPL code?
>

There isn't one. Such things cause way too much email.

What complete_all() does is to permit more than one task to wait on a
completion and for all those tasks to be woken by a single complete().
Without it you'd need to record how many tasks are sleeping there and do
complete() that many times.

So it's a sensible part of the completion API from a regularity-of-the-API
POV. We use it in the coredump code and I don't think we'd be likely to want
to rip it out.

In fact, I'd say that complete() should have always done it this way...

2005-03-04 13:07:16

by Alan

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

> Andrew, what is the policy for adding exports for out of tree GPL code?
>
> IMHO although it is convenient for maintainers of such code it is
> inconvenient for us (ie. when making changes to code) and gives
> people false assumptions about stability of *in-kernel* APIs.

Why isn't it _GPL if it is for a specific module need ?

Even more so for those cases where a person adds a non _GPL export for
another persons code.

2005-03-04 13:24:40

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

On Fri, Mar 04, 2005 at 03:15:04AM -0800, Andrew Morton wrote:
>...
> So it's a sensible part of the completion API from a regularity-of-the-API
> POV. We use it in the coredump code and I don't think we'd be likely to want
> to rip it out.
>...

Modular coredump code?

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

Subject: Re: [2.6 patch] unexport complete_all

On Fri, 4 Mar 2005 03:15:04 -0800, Andrew Morton <[email protected]> wrote:
> Bartlomiej Zolnierkiewicz <[email protected]> wrote:
> >
> > Andrew, what is the policy for adding exports for out of tree GPL code?
> >
>
> There isn't one. Such things cause way too much email.

Lack of policy causes the same thing (ie. this thread).

> What complete_all() does is to permit more than one task to wait on a
> completion and for all those tasks to be woken by a single complete().
> Without it you'd need to record how many tasks are sleeping there and do
> complete() that many times.
>
> So it's a sensible part of the completion API from a regularity-of-the-API

This function was already part of in-kernel API, just wasn't exported
for modules because there were no in-kernel users.

> POV. We use it in the coredump code and I don't think we'd be likely to want
> to rip it out.

OK, I understand that the unwritten policy is the following:
symbols for out-of-tree code used by OSDL are fine. 8)

> In fact, I'd say that complete() should have always done it this way...

2005-03-04 17:05:45

by Mike Waychison

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

> On Fri, Mar 04, 2005 at 03:09:39AM -0500, Mike Waychison wrote:
>> > I didn't find any possible modular usage in the kernel.
>> >
>> > Signed-off-by: Adrian Bunk <[email protected]>
>> >
>> > --- linux-2.6.11-rc5-mm1-full/kernel/sched.c.old 2005-03-04
>> 01:04:28.000000000 +0100
>> > +++ linux-2.6.11-rc5-mm1-full/kernel/sched.c 2005-03-04
>> 01:04:34.000000000 +0100
>> > @@ -3053,7 +3053,6 @@
>> > 0, 0, NULL);
>> > spin_unlock_irqrestore(&x->wait.lock, flags);
>> > }
>> > -EXPORT_SYMBOL(complete_all);
>> >
>> > void fastcall __sched wait_for_completion(struct completion *x)
>> > {
>> > -
>>
>> This is a valid piece of API that is exported for future use.
>>...
>
> You exported this function nearly one year ago with the only comment
> "Export complete_all for module use.".
>
> Why did you add the export last year instead of simply adding it when it
> will be required?
>

Because it makes no sense to export only part of an API-set. A good
interface should be as consistent as possible; selectively choosing what
is available for modular use fails this requirement.

The original patch was sent as I believed the lack of an exported
complete_all was purely an oversight.

Mike Waychison

2005-03-04 17:12:18

by Mike Waychison

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

> On Fri, Mar 04, 2005 at 11:40:08AM +0100, Bartlomiej Zolnierkiewicz wrote:
>> On Fri, 04 Mar 2005 03:09:39 -0500, Mike Waychison <[email protected]>
>> wrote:
>> > > I didn't find any possible modular usage in the kernel.
>> > >
>> > > Signed-off-by: Adrian Bunk <[email protected]>
>> > >
>> > > --- linux-2.6.11-rc5-mm1-full/kernel/sched.c.old 2005-03-04
>> 01:04:28.000000000 +0100
>> > > +++ linux-2.6.11-rc5-mm1-full/kernel/sched.c 2005-03-04
>> 01:04:34.000000000 +0100
>> > > @@ -3053,7 +3053,6 @@
>> > > 0, 0, NULL);
>> > > spin_unlock_irqrestore(&x->wait.lock, flags);
>> > > }
>> > > -EXPORT_SYMBOL(complete_all);
>> > >
>> > > void fastcall __sched wait_for_completion(struct completion *x)
>> > > {
>> > > -
>> >
>> > This is a valid piece of API that is exported for future use.
>>
>> Let me guess: autofsng?
>>...
>
> I grepped through autofsng-0.4 but didn't find any usage.

Did you just blindly grep the userspace tarball?

There is no kernel code in there. It's all in linux-2.6.*-autofsng-*.patch.

Mike Waychison

2005-03-05 06:17:18

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] unexport complete_all

On Fri, Mar 04, 2005 at 12:08:33PM -0500, [email protected] wrote:
>
> Did you just blindly grep the userspace tarball?
>
> There is no kernel code in there. It's all in linux-2.6.*-autofsng-*.patch.

Sorry, my bad.

I couldn't connect to your FTP server this morning (I don't know why)
and I found the wrong file with Google.

This makes my patch void.

> Mike Waychison

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

Subject: Re: [2.6 patch] unexport complete_all

On Fri, 4 Mar 2005 14:14:39 +0100, Bartlomiej Zolnierkiewicz
<[email protected]> wrote:
> On Fri, 4 Mar 2005 03:15:04 -0800, Andrew Morton <[email protected]> wrote:
> > Bartlomiej Zolnierkiewicz <[email protected]> wrote:
> > >
> > > Andrew, what is the policy for adding exports for out of tree GPL code?
> > >
> >
> > There isn't one. Such things cause way too much email.
>
> Lack of policy causes the same thing (ie. this thread).
>
> > What complete_all() does is to permit more than one task to wait on a
> > completion and for all those tasks to be woken by a single complete().
> > Without it you'd need to record how many tasks are sleeping there and do
> > complete() that many times.
> >
> > So it's a sensible part of the completion API from a regularity-of-the-API
>
> This function was already part of in-kernel API, just wasn't exported
> for modules because there were no in-kernel users.
>
> > POV. We use it in the coredump code and I don't think we'd be likely to want
> > to rip it out.

It was my misunderstanding w.r.t. 'We' here...

> OK, I understand that the unwritten policy is the following:
> symbols for out-of-tree code used by OSDL are fine. 8)

/me takes this bad joke back and says sorry to Andrew

> > In fact, I'd say that complete() should have always done it this way...