[2.6 patch] unexport complete_all

[2.6 patch] unexport complete_all

[Adrian Bunk]

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

Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- 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)
 {

Re: [2.6 patch] unexport complete_all

[Mike Waychison]

> I didn't find any possible modular usage in the kernel.
> 
> Signed-off-by: Adrian Bunk <bunk@stusta.de>
> 
> --- 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

Re: [2.6 patch] unexport complete_all

[Bartlomiej Zolnierkiewicz]

On Fri, 04 Mar 2005 03:09:39 -0500, Mike Waychison <mike@waychison.com> wrote:
> > I didn't find any possible modular usage in the kernel.
> >
> > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> >
> > --- 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 <Michael.Waychison@Sun.COM>

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

Re: [2.6 patch] unexport complete_all

[Adrian Bunk]

On Fri, Mar 04, 2005 at 11:40:08AM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Fri, 04 Mar 2005 03:09:39 -0500, Mike Waychison <mike@waychison.com> wrote:
> > > I didn't find any possible modular usage in the kernel.
> > >
> > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
> > >
> > > --- 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

Re: [2.6 patch] unexport complete_all

[mike]

> On Fri, Mar 04, 2005 at 11:40:08AM +0100, Bartlomiej Zolnierkiewicz wrote:
>> On Fri, 04 Mar 2005 03:09:39 -0500, Mike Waychison <mike@waychison.com>
>> wrote:
>> > > I didn't find any possible modular usage in the kernel.
>> > >
>> > > Signed-off-by: Adrian Bunk <bunk@stusta.de>
>> > >
>> > > --- 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

Re: [2.6 patch] unexport complete_all

[Adrian Bunk]

On Fri, Mar 04, 2005 at 12:08:33PM -0500, mike@waychison.com 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

Re: [2.6 patch] unexport complete_all

[Andrew Morton]

Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> 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...

Re: [2.6 patch] unexport complete_all

[Bartlomiej Zolnierkiewicz]

On Fri, 4 Mar 2005 03:15:04 -0800, Andrew Morton <akpm@osdl.org> wrote:
> Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> 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...

Re: [2.6 patch] unexport complete_all

[Bartlomiej Zolnierkiewicz]

On Fri, 4 Mar 2005 14:14:39 +0100, Bartlomiej Zolnierkiewicz
<bzolnier@gmail.com> wrote:
> On Fri, 4 Mar 2005 03:15:04 -0800, Andrew Morton <akpm@osdl.org> wrote:
> > Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> 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...

Re: [2.6 patch] unexport complete_all

[Adrian Bunk]

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

Re: [2.6 patch] unexport complete_all

[Alan Cox]

> 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. 

Re: [2.6 patch] unexport complete_all

[Adrian Bunk]

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 <bunk@stusta.de>
> > 
> > --- 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

Re: [2.6 patch] unexport complete_all

[mike]

> 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 <bunk@stusta.de>
>> >
>> > --- 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