Re: [PATCH 1/1] tty: n_gsm: Avoid sleeping during .write() whilst atomic

[Greg Kroah-Hartman]

On Thu, Oct 05, 2023 at 01:17:56PM +0100, Lee Jones wrote:
> On Thu, 05 Oct 2023, Greg Kroah-Hartman wrote:
> 
> > On Thu, Oct 05, 2023 at 12:46:32PM +0100, Lee Jones wrote:
> > > On Thu, 05 Oct 2023, Starke, Daniel wrote:
> > > 
> > > > > > Would something like this tick that box?
> > > > > > 
> > > > > > diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c index 
> > > > > > 1f3aba607cd51..5c1d2fcd5d9e2 100644
> > > > > > --- a/drivers/tty/n_gsm.c
> > > > > > +++ b/drivers/tty/n_gsm.c
> > > > > > @@ -3716,6 +3716,10 @@ static ssize_t gsmld_write(struct tty_struct *tty, struct file *file,
> > > > > >         if (!gsm)
> > > > > >                 return -ENODEV;
> > > > > >  
> > > > > > +       /* The GSM line discipline does not support binding to console */
> > > > > > +       if (strncmp(tty->name, "tty", 3))
> > > > > > +               return -EINVAL;
> > > > > 
> > > > > No, that's not going to work, some consoles do not start with "tty" :(
> > > 
> > > Ah, you mean there are others that we need to consider?
> > > 
> > > I was just covering off con_write() from drivers/tty/vt/vt.c.
> > > 
> > > Does anyone have a counter proposal?
> > 
> > consoles are dynamically assigned, the device knows if it is a console
> > or not, so there is a way to determine this at runtime.  It's not a
> > device naming thing at all.
> 
> It's not a device naming thing, but it is a ... :)
> 
> Okay, here's another uninformed stab in the dark:
> 
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index 1f3aba607cd51..ddf00f6a4141d 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -3629,6 +3629,10 @@ static int gsmld_open(struct tty_struct *tty)
>         if (tty->ops->write == NULL)
>                 return -EINVAL;
> 
> +       /* The GSM line discipline does not support binding to console */
> +       if (tty->driver->type == TTY_DRIVER_TYPE_CONSOLE)
> +               return -EINVAL;
> +
>         /* Attach our ldisc data */
>         gsm = gsm_alloc_mux();
>         if (gsm == NULL)
> 
> This seems to allow for the real serial devices (TTY_DRIVER_TYPE_SERIAL)
> suggested by Daniel.

Close, but not quite.

Driver "types" can say if they are a console or not, but that doesn't
mean you can't run the console over a serial port as well, right?

Sorry, I don't have a real solution right now, and wouldn't wish the
phrase "just dig through the tty console code!" on anyone, but that's
what it is going to take to figure it out somehow, I can't remember the
exact way consoles are hooked up at the moment (I conviently skipped
that portion of the tty layer in my Embedded Recipies talk last week,
saying "it's magic...")

thanks,

greg k-h