[PATCH] ttyprintk: optimize tpk_close flush code

[PATCH] ttyprintk: optimize tpk_close flush code

[Junyong Sun]

tpk_printk(NULL,0) do nothing but call tpk_flush to
flush buffer, so why don't use tpk_flush diretcly?
this is a small optimization.

Signed-off-by: Junyong Sun <sunjunyong@xiaomi.com>
---
 drivers/char/ttyprintk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c
index 6a0059e..2ce78b3 100644
--- a/drivers/char/ttyprintk.c
+++ b/drivers/char/ttyprintk.c
@@ -104,7 +104,7 @@ static void tpk_close(struct tty_struct *tty, struct file *filp)
 
 	spin_lock_irqsave(&tpkp->spinlock, flags);
 	/* flush tpk_printk buffer */
-	tpk_printk(NULL, 0);
+	tpk_flush();
 	spin_unlock_irqrestore(&tpkp->spinlock, flags);
 
 	tty_port_close(&tpkp->port, tty, filp);
-- 
2.7.4

Re: [PATCH] ttyprintk: optimize tpk_close flush code

[Greg KH]

On Wed, Nov 04, 2020 at 02:02:24PM +0800, Junyong Sun wrote:
> tpk_printk(NULL,0) do nothing but call tpk_flush to
> flush buffer, so why don't use tpk_flush diretcly?
> this is a small optimization.
> 
> Signed-off-by: Junyong Sun <sunjunyong@xiaomi.com>
> ---
>  drivers/char/ttyprintk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c
> index 6a0059e..2ce78b3 100644
> --- a/drivers/char/ttyprintk.c
> +++ b/drivers/char/ttyprintk.c
> @@ -104,7 +104,7 @@ static void tpk_close(struct tty_struct *tty, struct file *filp)
>  
>  	spin_lock_irqsave(&tpkp->spinlock, flags);
>  	/* flush tpk_printk buffer */
> -	tpk_printk(NULL, 0);
> +	tpk_flush();

If you do this, then please remove the logic in tpk_flush() that handles
NULL as now that logic will not be needed at all, right?

Also the comment here wouldn't be needed as the code obviously does that
based on the function call being made :)

thanks,

greg k-h

Re: [PATCH] ttyprintk: optimize tpk_close flush code

[sunjunyong]

Hi Greg:
It have no logic that handles NULL in tpk_flush() but tpk_printk().
Do you mean that if i understand correctly?!I think we should not remove 
the logic that handles NULL in tpk_printk() as we don't know if the buf 
from parent caller is null or not.But we transfer a null buf to tpk_printk()
 for previous version of tpk_close, i think it's redundant.

The comment is a hitory tip for other guys to understand.I suggest to 
keep it as before if you like.

thanks
JY
On Thu, Nov 12, 2020 at 08:52:23AM +0100, Greg KH wrote:
> On Wed, Nov 04, 2020 at 02:02:24PM +0800, Junyong Sun wrote:
> > tpk_printk(NULL,0) do nothing but call tpk_flush to
> > flush buffer, so why don't use tpk_flush diretcly?
> > this is a small optimization.
> > 
> > Signed-off-by: Junyong Sun <sunjunyong@xiaomi.com>
> > ---
> >  drivers/char/ttyprintk.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/char/ttyprintk.c b/drivers/char/ttyprintk.c
> > index 6a0059e..2ce78b3 100644
> > --- a/drivers/char/ttyprintk.c
> > +++ b/drivers/char/ttyprintk.c
> > @@ -104,7 +104,7 @@ static void tpk_close(struct tty_struct *tty, struct file *filp)
> >  
> >  	spin_lock_irqsave(&tpkp->spinlock, flags);
> >  	/* flush tpk_printk buffer */
> > -	tpk_printk(NULL, 0);
> > +	tpk_flush();
> 
> If you do this, then please remove the logic in tpk_flush() that handles
> NULL as now that logic will not be needed at all, right?
> 
> Also the comment here wouldn't be needed as the code obviously does that
> based on the function call being made :)
> 
> thanks,
> 
> greg k-h

Re: [PATCH] ttyprintk: optimize tpk_close flush code

[Greg KH]

A: http://en.wikipedia.org/wiki/Top_post
Q: Were do I find info about this thing called top-posting?
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Nov 12, 2020 at 09:41:22PM +0800, sunjunyong wrote:
> Hi Greg:
> It have no logic that handles NULL in tpk_flush() but tpk_printk().

Sorry, yes, that is what I mean.

> Do you mean that if i understand correctly?!I think we should not remove 
> the logic that handles NULL in tpk_printk() as we don't know if the buf 
> from parent caller is null or not.

Yes you do, you control that buffer right?  This is a static function,
you know what is happening here.

> But we transfer a null buf to tpk_printk()
>  for previous version of tpk_close, i think it's redundant.
> 
> The comment is a hitory tip for other guys to understand.I suggest to 
> keep it as before if you like.

The call to flush makes it obvious, no need to keep it.  And we have git
history for people to look at if they are curious about past versions.

thanks,

greg k-h

Re: [PATCH] ttyprintk: optimize tpk_close flush code

[sunjunyong]

Hi greg:

> The call to flush makes it obvious, no need to keep it.  And we have git
> history for people to look at if they are curious about past versions.
you are right. I would remove it in next version.

Any other suggestions ?

thanks!
JY.