[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Thu, 31 Dec 2020 17:37:44 -0800
From: Xie He <xie.he.0141@...il.com>
To: Gao Yan <gao.yanB@....com>
Cc: paulus@...ba.org, davem@...emloft.net, kuba@...nel.org,
linux-ppp@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: remove disc_data_lock in ppp line discipline
> In tty layer, it use tty->ldisc_sem to proect tty_ldisc_ops.
> So I think tty->ldisc_sem can also protect tty->disc_data;
It might help by CC'ing TTY people, so that we could get this reviewed by
people who are familiar with TTY code.
Greg Kroah-Hartman <gregkh@...uxfoundation.org> (supporter:TTY LAYER)
Jiri Slaby <jirislaby@...nel.org> (supporter:TTY LAYER)
Thanks!
> For examlpe,
> When cpu A is running ppp_synctty_ioctl that hold the tty->ldisc_sem,
> at the same time if cpu B calls ppp_synctty_close, it will wait until
> cpu A release tty->ldisc_sem. So I think it is unnecessary to have the
> disc_data_lock;
>
> cpu A cpu B
> tty_ioctl tty_reopen
> ->hold tty->ldisc_sem ->hold tty->ldisc_sem(write), failed
> ->ld->ops->ioctl ->wait...
> ->release tty->ldisc_sem ->wait...OK,hold tty->ldisc_sem
> ->tty_ldisc_reinit
> ->tty_ldisc_close
> ->ld->ops->close
IMHO an example might not be necessary. Examples are useful to show
incorrectness. But we cannot show correctness by examples because
examples are not exhaustive.
BTW, there're some typos:
"proect" -> "protect"
"examlpe" -> "example"
"that hold ..." -> "that holds ..."
"cpu A release ..." -> "cpu A releases ..."
> * FIXME: this is no longer true. The _close path for the ldisc is
> * now guaranteed to be sane.
> */
> *
> * FIXME: Fixed in tty_io nowadays.
> */
Since you are removing "disc_data_lock", please update (or remove) these
two comments. Thanks!
Powered by blists - more mailing lists