Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-06 Thread Wang YanQing
On Thu, Jun 06, 2013 at 01:47:46PM +0200, Pavel Machek wrote:
> On Thu 2013-06-06 09:23:13, Wang YanQing wrote:
> > On Tue, Jun 04, 2013 at 10:13:18PM +0200, Pavel Machek wrote:
> > > On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
> > > > Impact:
> > > > 1:convert all remain take_over_console to do_take_over_console
> > > 
> > > This is step backwards.
> > 
> > What is step backwards? do_take_over_console appear MUCH MUCH later in 
> > kernel
> > than take_over_console, do_take_over_console is the new API, I can't 
> > understand
> > what is step backwards.
> 
> "do_*" is internal api, "*" is external api. You sprinkle internal api
> all over the place.
> 

internal vs external? No, they only have one difference, callee vs caller hold
the console lock.

> > > > --- a/arch/alpha/kernel/console.c
> > > > +++ b/arch/alpha/kernel/console.c
> > > > @@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))
> > > >  
> > > > /* Set the VGA hose and init the new console. */
> > > > pci_vga_hose = hose;
> > > > -   take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> > > > +   console_lock();
> > > > +   do_take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> > > > +   console_unlock();
> > > >  }
> > > 
> > > Original was better.
> > 
> > Except reduce some console_lock/unlock scatter scattered in kernel, I 
> > can't see the "BETTER", and it is not a BIG problem for the benefit to
> > unify the API.
> 
> You replaced "calling exported function" with "calling
> internal-sounding function and adding locking too".

do_take_over_console is also exported by:

EXPORT_SYMBOL_GPL(do_take_over_console)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-06 Thread Pavel Machek
On Thu 2013-06-06 09:23:13, Wang YanQing wrote:
> On Tue, Jun 04, 2013 at 10:13:18PM +0200, Pavel Machek wrote:
> > On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
> > > Impact:
> > > 1:convert all remain take_over_console to do_take_over_console
> > 
> > This is step backwards.
> 
> What is step backwards? do_take_over_console appear MUCH MUCH later in kernel
> than take_over_console, do_take_over_console is the new API, I can't 
> understand
> what is step backwards.

"do_*" is internal api, "*" is external api. You sprinkle internal api
all over the place.

> > > --- a/arch/alpha/kernel/console.c
> > > +++ b/arch/alpha/kernel/console.c
> > > @@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))
> > >  
> > >   /* Set the VGA hose and init the new console. */
> > >   pci_vga_hose = hose;
> > > - take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> > > + console_lock();
> > > + do_take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> > > + console_unlock();
> > >  }
> > 
> > Original was better.
> 
> Except reduce some console_lock/unlock scatter scattered in kernel, I 
> can't see the "BETTER", and it is not a BIG problem for the benefit to
> unify the API.

You replaced "calling exported function" with "calling
internal-sounding function and adding locking too".


Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-06 Thread Pavel Machek
On Thu 2013-06-06 09:23:13, Wang YanQing wrote:
 On Tue, Jun 04, 2013 at 10:13:18PM +0200, Pavel Machek wrote:
  On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
   Impact:
   1:convert all remain take_over_console to do_take_over_console
  
  This is step backwards.
 
 What is step backwards? do_take_over_console appear MUCH MUCH later in kernel
 than take_over_console, do_take_over_console is the new API, I can't 
 understand
 what is step backwards.

do_* is internal api, * is external api. You sprinkle internal api
all over the place.

   --- a/arch/alpha/kernel/console.c
   +++ b/arch/alpha/kernel/console.c
   @@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))

 /* Set the VGA hose and init the new console. */
 pci_vga_hose = hose;
   - take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
   + console_lock();
   + do_take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
   + console_unlock();
}
  
  Original was better.
 
 Except reduce some console_lock/unlock scatter scattered in kernel, I 
 can't see the BETTER, and it is not a BIG problem for the benefit to
 unify the API.

You replaced calling exported function with calling
internal-sounding function and adding locking too.


Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-06 Thread Wang YanQing
On Thu, Jun 06, 2013 at 01:47:46PM +0200, Pavel Machek wrote:
 On Thu 2013-06-06 09:23:13, Wang YanQing wrote:
  On Tue, Jun 04, 2013 at 10:13:18PM +0200, Pavel Machek wrote:
   On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
Impact:
1:convert all remain take_over_console to do_take_over_console
   
   This is step backwards.
  
  What is step backwards? do_take_over_console appear MUCH MUCH later in 
  kernel
  than take_over_console, do_take_over_console is the new API, I can't 
  understand
  what is step backwards.
 
 do_* is internal api, * is external api. You sprinkle internal api
 all over the place.
 

internal vs external? No, they only have one difference, callee vs caller hold
the console lock.

--- a/arch/alpha/kernel/console.c
+++ b/arch/alpha/kernel/console.c
@@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))
 
/* Set the VGA hose and init the new console. */
pci_vga_hose = hose;
-   take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
+   console_lock();
+   do_take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
+   console_unlock();
 }
   
   Original was better.
  
  Except reduce some console_lock/unlock scatter scattered in kernel, I 
  can't see the BETTER, and it is not a BIG problem for the benefit to
  unify the API.
 
 You replaced calling exported function with calling
 internal-sounding function and adding locking too.

do_take_over_console is also exported by:

EXPORT_SYMBOL_GPL(do_take_over_console)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-05 Thread Wang YanQing
On Tue, Jun 04, 2013 at 10:13:18PM +0200, Pavel Machek wrote:
> On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
> > Impact:
> > 1:convert all remain take_over_console to do_take_over_console
> 
> This is step backwards.

What is step backwards? do_take_over_console appear MUCH MUCH later in kernel
than take_over_console, do_take_over_console is the new API, I can't understand
what is step backwards.

> > --- a/arch/alpha/kernel/console.c
> > +++ b/arch/alpha/kernel/console.c
> > @@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))
> >  
> > /* Set the VGA hose and init the new console. */
> > pci_vga_hose = hose;
> > -   take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> > +   console_lock();
> > +   do_take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> > +   console_unlock();
> >  }
> 
> Original was better.

Except reduce some console_lock/unlock scatter scattered in kernel, I 
can't see the "BETTER", and it is not a BIG problem for the benefit to
unify the API.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-05 Thread Wang YanQing
On Tue, Jun 04, 2013 at 10:13:18PM +0200, Pavel Machek wrote:
 On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
  Impact:
  1:convert all remain take_over_console to do_take_over_console
 
 This is step backwards.

What is step backwards? do_take_over_console appear MUCH MUCH later in kernel
than take_over_console, do_take_over_console is the new API, I can't understand
what is step backwards.

  --- a/arch/alpha/kernel/console.c
  +++ b/arch/alpha/kernel/console.c
  @@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))
   
  /* Set the VGA hose and init the new console. */
  pci_vga_hose = hose;
  -   take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
  +   console_lock();
  +   do_take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
  +   console_unlock();
   }
 
 Original was better.

Except reduce some console_lock/unlock scatter scattered in kernel, I 
can't see the BETTER, and it is not a BIG problem for the benefit to
unify the API.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-04 Thread Pavel Machek
On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
> Impact:
> 1:convert all remain take_over_console to do_take_over_console

This is step backwards.

> --- a/arch/alpha/kernel/console.c
> +++ b/arch/alpha/kernel/console.c
> @@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))
>  
>   /* Set the VGA hose and init the new console. */
>   pci_vga_hose = hose;
> - take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> + console_lock();
> + do_take_over_console(_con, 0, MAX_NR_CONSOLES-1, 1);
> + console_unlock();
>  }

Original was better.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-06-04 Thread Pavel Machek
On Tue 2013-05-21 13:15:12, Wang YanQing wrote:
 Impact:
 1:convert all remain take_over_console to do_take_over_console

This is step backwards.

 --- a/arch/alpha/kernel/console.c
 +++ b/arch/alpha/kernel/console.c
 @@ -61,7 +61,9 @@ locate_and_init_vga(void *(*sel_func)(void *, void *))
  
   /* Set the VGA hose and init the new console. */
   pci_vga_hose = hose;
 - take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
 + console_lock();
 + do_take_over_console(vga_con, 0, MAX_NR_CONSOLES-1, 1);
 + console_unlock();
  }

Original was better.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Peter Hurley

On 05/21/2013 12:29 PM, Wang YanQing wrote:

On Wed, May 22, 2013 at 12:18:49AM +0800, Wang YanQing wrote:

Except now you're spreading the brokenness that is console_lock()
over many more source files than the single-use case of
do_take_over_console().



The actual interface is take_over_console(); the _workaround_ is
exposing do_take_over_console() for fbcon to wrap.


This _workaround_ willn't work, take_over_console will hold console_lock 
internal,
but do_take_over_console need caller hold console_lock, then we can't rewrite
do_take_over_console as a wrap base on take_over_console.


The workaround I'm referring to is commit 50e244cc793 which
exposed do_take_over_console() as an interface to band aid the lock
problems.

I'm ok with take_over_console() being a lock wrapper around
do_take_over_console(), if you are trying to preserve your
other changes.

Regards,
Peter Hurley



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Wang YanQing
On Wed, May 22, 2013 at 12:18:49AM +0800, Wang YanQing wrote:
> > Except now you're spreading the brokenness that is console_lock()
> > over many more source files than the single-use case of
> > do_take_over_console().
> 
> > The actual interface is take_over_console(); the _workaround_ is
> > exposing do_take_over_console() for fbcon to wrap.
> 
> This _workaround_ willn't work, take_over_console will hold console_lock 
> internal,
> but do_take_over_console need caller hold console_lock, then we can't rewrite 
> do_take_over_console as a wrap base on take_over_console. 
> 
> But the reverse is ok. So if we have to do it, then the actual interface 
> is do_take_over_console, and the "_workaround_" is exposing take_over_console 
> as a wrap base on do_take_over_console.

But if we do this, then we have two version functions do the same thing except
caller/callee hold lock, I can't see much sense to have them at the same time.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Wang YanQing
On Tue, May 21, 2013 at 11:48:58AM -0400, Peter Hurley wrote:
> On 05/21/2013 10:42 AM, Wang YanQing wrote:
> > On Tue, May 21, 2013 at 09:10:33AM -0400, Peter Hurley wrote:
> >> I would rather revert dc9641895abb which purported to delete
> >> _unneeded_ functions than have this. Obviously the functions
> >> were needed.
> >>
> >
> > Hi Peter, this series patches' goal is to reduce codes'
> > redundance and function duplication. But if we keep take_over_console,
> > then we have to rewrite it as a trivial wrapper over do_take_over_console,
> > or we have to keep bind_con_driver and register_con_driver, and this
> > will bring use codes' redundance.
> >
> > And if we rewrite take_over_console as a wrapper over
> > do_take_over_console, it is so trivial, delete it and let kernel
> > use the unified version of APIs will simplify the APIs.
> 
> Except now you're spreading the brokenness that is console_lock()
> over many more source files than the single-use case of
> do_take_over_console().

> The actual interface is take_over_console(); the _workaround_ is
> exposing do_take_over_console() for fbcon to wrap.

This _workaround_ willn't work, take_over_console will hold console_lock 
internal,
but do_take_over_console need caller hold console_lock, then we can't rewrite 
do_take_over_console as a wrap base on take_over_console. 

But the reverse is ok. So if we have to do it, then the actual interface 
is do_take_over_console, and the "_workaround_" is exposing take_over_console 
as a wrap base on do_take_over_console.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Peter Hurley

On 05/21/2013 10:42 AM, Wang YanQing wrote:

On Tue, May 21, 2013 at 09:10:33AM -0400, Peter Hurley wrote:

I would rather revert dc9641895abb which purported to delete
_unneeded_ functions than have this. Obviously the functions
were needed.



Hi Peter, this series patches' goal is to reduce codes'
redundance and function duplication. But if we keep take_over_console,
then we have to rewrite it as a trivial wrapper over do_take_over_console,
or we have to keep bind_con_driver and register_con_driver, and this
will bring use codes' redundance.

And if we rewrite take_over_console as a wrapper over
do_take_over_console, it is so trivial, delete it and let kernel
use the unified version of APIs will simplify the APIs.


Except now you're spreading the brokenness that is console_lock()
over many more source files than the single-use case of
do_take_over_console().

The actual interface is take_over_console(); the _workaround_ is
exposing do_take_over_console() for fbcon to wrap.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Wang YanQing
On Tue, May 21, 2013 at 09:10:33AM -0400, Peter Hurley wrote:
> I would rather revert dc9641895abb which purported to delete
> _unneeded_ functions than have this. Obviously the functions
> were needed.
> 

Hi Peter, this series patches' goal is to reduce codes'
redundance and function duplication. But if we keep take_over_console, 
then we have to rewrite it as a trivial wrapper over do_take_over_console,
or we have to keep bind_con_driver and register_con_driver, and this
will bring use codes' redundance.

And if we rewrite take_over_console as a wrapper over
do_take_over_console, it is so trivial, delete it and let kernel
use the unified version of APIs will simplify the APIs.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Peter Hurley

On 05/21/2013 01:15 AM, Wang YanQing wrote:

Impact:
1:convert all remain take_over_console to do_take_over_console
2:update take_over_console to do_take_over_console in comment

Commit dc9641895abb ("vt: delete unneeded functions
register_con_driver|take_over_console") delete take_over_console,
but forget to convert remain take_over_console's users to new API
do_take_over_console, this patch fix it.

Signed-off-by: Wang YanQing 
---
  Sorry for my mistake, I believe I do the full
  kernel source find|grep, but the reality is I
  forget  take_over_console.


I would rather revert dc9641895abb which purported to delete
_unneeded_ functions than have this. Obviously the functions
were needed.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Peter Hurley

On 05/21/2013 01:15 AM, Wang YanQing wrote:

Impact:
1:convert all remain take_over_console to do_take_over_console
2:update take_over_console to do_take_over_console in comment

Commit dc9641895abb (vt: delete unneeded functions
register_con_driver|take_over_console) delete take_over_console,
but forget to convert remain take_over_console's users to new API
do_take_over_console, this patch fix it.

Signed-off-by: Wang YanQing udkni...@gmail.com
---
  Sorry for my mistake, I believe I do the full
  kernel source find|grep, but the reality is I
  forget  take_over_console.


I would rather revert dc9641895abb which purported to delete
_unneeded_ functions than have this. Obviously the functions
were needed.

Regards,
Peter Hurley
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Wang YanQing
On Tue, May 21, 2013 at 09:10:33AM -0400, Peter Hurley wrote:
 I would rather revert dc9641895abb which purported to delete
 _unneeded_ functions than have this. Obviously the functions
 were needed.
 

Hi Peter, this series patches' goal is to reduce codes'
redundance and function duplication. But if we keep take_over_console, 
then we have to rewrite it as a trivial wrapper over do_take_over_console,
or we have to keep bind_con_driver and register_con_driver, and this
will bring use codes' redundance.

And if we rewrite take_over_console as a wrapper over
do_take_over_console, it is so trivial, delete it and let kernel
use the unified version of APIs will simplify the APIs.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Peter Hurley

On 05/21/2013 10:42 AM, Wang YanQing wrote:

On Tue, May 21, 2013 at 09:10:33AM -0400, Peter Hurley wrote:

I would rather revert dc9641895abb which purported to delete
_unneeded_ functions than have this. Obviously the functions
were needed.



Hi Peter, this series patches' goal is to reduce codes'
redundance and function duplication. But if we keep take_over_console,
then we have to rewrite it as a trivial wrapper over do_take_over_console,
or we have to keep bind_con_driver and register_con_driver, and this
will bring use codes' redundance.

And if we rewrite take_over_console as a wrapper over
do_take_over_console, it is so trivial, delete it and let kernel
use the unified version of APIs will simplify the APIs.


Except now you're spreading the brokenness that is console_lock()
over many more source files than the single-use case of
do_take_over_console().

The actual interface is take_over_console(); the _workaround_ is
exposing do_take_over_console() for fbcon to wrap.

Regards,
Peter Hurley

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Wang YanQing
On Tue, May 21, 2013 at 11:48:58AM -0400, Peter Hurley wrote:
 On 05/21/2013 10:42 AM, Wang YanQing wrote:
  On Tue, May 21, 2013 at 09:10:33AM -0400, Peter Hurley wrote:
  I would rather revert dc9641895abb which purported to delete
  _unneeded_ functions than have this. Obviously the functions
  were needed.
 
 
  Hi Peter, this series patches' goal is to reduce codes'
  redundance and function duplication. But if we keep take_over_console,
  then we have to rewrite it as a trivial wrapper over do_take_over_console,
  or we have to keep bind_con_driver and register_con_driver, and this
  will bring use codes' redundance.
 
  And if we rewrite take_over_console as a wrapper over
  do_take_over_console, it is so trivial, delete it and let kernel
  use the unified version of APIs will simplify the APIs.
 
 Except now you're spreading the brokenness that is console_lock()
 over many more source files than the single-use case of
 do_take_over_console().

 The actual interface is take_over_console(); the _workaround_ is
 exposing do_take_over_console() for fbcon to wrap.

This _workaround_ willn't work, take_over_console will hold console_lock 
internal,
but do_take_over_console need caller hold console_lock, then we can't rewrite 
do_take_over_console as a wrap base on take_over_console. 

But the reverse is ok. So if we have to do it, then the actual interface 
is do_take_over_console, and the _workaround_ is exposing take_over_console 
as a wrap base on do_take_over_console.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Wang YanQing
On Wed, May 22, 2013 at 12:18:49AM +0800, Wang YanQing wrote:
  Except now you're spreading the brokenness that is console_lock()
  over many more source files than the single-use case of
  do_take_over_console().
 
  The actual interface is take_over_console(); the _workaround_ is
  exposing do_take_over_console() for fbcon to wrap.
 
 This _workaround_ willn't work, take_over_console will hold console_lock 
 internal,
 but do_take_over_console need caller hold console_lock, then we can't rewrite 
 do_take_over_console as a wrap base on take_over_console. 
 
 But the reverse is ok. So if we have to do it, then the actual interface 
 is do_take_over_console, and the _workaround_ is exposing take_over_console 
 as a wrap base on do_take_over_console.

But if we do this, then we have two version functions do the same thing except
caller/callee hold lock, I can't see much sense to have them at the same time.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] TTY:vt: convert remain take_over_console's users to do_take_over_console

2013-05-21 Thread Peter Hurley

On 05/21/2013 12:29 PM, Wang YanQing wrote:

On Wed, May 22, 2013 at 12:18:49AM +0800, Wang YanQing wrote:

Except now you're spreading the brokenness that is console_lock()
over many more source files than the single-use case of
do_take_over_console().



The actual interface is take_over_console(); the _workaround_ is
exposing do_take_over_console() for fbcon to wrap.


This _workaround_ willn't work, take_over_console will hold console_lock 
internal,
but do_take_over_console need caller hold console_lock, then we can't rewrite
do_take_over_console as a wrap base on take_over_console.


The workaround I'm referring to is commit 50e244cc793 which
exposed do_take_over_console() as an interface to band aid the lock
problems.

I'm ok with take_over_console() being a lock wrapper around
do_take_over_console(), if you are trying to preserve your
other changes.

Regards,
Peter Hurley



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/