Linux Serial subsystem development
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Xin Chen <xin.chen2@oss.qualcomm.com>
Cc: jirislaby@kernel.org, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org, liulzhao@qti.qualcomm.com,
	cheng.jiang@oss.qualcomm.com, cxin@qti.qualcomm.com
Subject: Re: [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data
Date: Tue, 18 Aug 2026 09:34:31 +0200	[thread overview]
Message-ID: <2026081851-krypton-shady-b18d@gregkh> (raw)
In-Reply-To: <5c0fdf81-950c-427f-8327-a8b4b1ad922f@oss.qualcomm.com>

On Tue, Aug 18, 2026 at 02:58:41PM +0800, Xin Chen wrote:
> On Tue, Aug 18, 2026, Greg KH wrote:
> > But that's not a problem with the tty layer, if something else happens
> > to "drain" the pool again you can not create a skb.  You are not
> > solving the root problem here.
> 
> You are right that this does not prevent every possible order-0
> exhaustion. However, the specific and reproducible trigger is
> n_tty_open() consuming order-0 pages via vzalloc() immediately before
> skb_clone() runs. Eliminating that unnecessary pressure removes the
> failure in practice, even if it does not make skb_clone() immune to
> all possible memory pressure.

Then you are going to play whack-a-mole on your very memory-constrained
system in order to work around the root problem here.  Please don't do
that, solve the real problem you are having.

> > But that's not really a change, when was vmalloc() first used?
> > As nothing has changed here, then why is this suddenly showing up now?
> 
> ldata was originally allocated with kzalloc() (introduced in commit
> 70ece7a73159, "TTY: n_tty, add ldisc data to n_tty", 2012).  Commit
> ebec3f8f5271 switched it to vmalloc()/vzalloc() in 2018 as a side
> effect of fixing an echo buffer race — the allocation change was
> incidental, not intentional.  The issue surfaces now because the BT
> enable-disable sanity test exercises a back-to-back open pattern that
> was not common before serdev-based UART transports became widespread.

So this is purely because you are stress-testing the BT stack now,
right?

> > Again, that sounds like a bluetooth issue, and why can't you just
> > properly handle the skb out of memory issue?
> 
> The skb_clone() failure is silent — it returns NULL and the code
> continues without error, leaving hdev->req_skb NULL.

Why not fix that?

> By the time
> the BT layer observes the problem (a -ETIMEDOUT 10 seconds later),
> it is several layers removed from the skb_clone() failure: the
> firmware has already replied successfully, hci_req_cmd_complete()
> has already run and found req_skb NULL, and the completion callback
> was never invoked.

Again, fix that.

> At that point the BT layer has no way to
> distinguish a memory failure from a genuine firmware timeout, let
> alone recover from it. 

But that's not a TTY layer issue, it's a BT issue.  Please fix that.

> And even if the NULL req_skb were detected
> and surfaced as an error immediately, there is nothing the BT layer
> could do to recover — it cannot reclaim memory or retry the
> allocation itself.

Nor should it, it should handle the error properly and recover
correctly.

>  The only option would be to wait for the memory
> to be reclaimed and retry the entire BT enable sequence from
> userspace, which is exactly the kind of fragile error handling we
> want to avoid.

No, you need to handle the error properly because it could happen at any
point in time.

> The tty change is simpler and correct: ldata was originally a
> kzalloc() allocation and there is no reason for it to use
> vmalloc-backed pages that interfere with unrelated allocations.

It is not "correct", but rather papering over the root problem.

Fix the bluetooth stack please.

thanks,

greg k-h

  reply	other threads:[~2026-08-18  7:34 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-17 13:55 [PATCH v1] tty: n_tty: use kvzalloc/kvfree for line discipline data Xin Chen
2026-08-17 14:14 ` Greg KH
2026-08-18  3:09   ` Xin Chen
2026-08-18  6:05     ` Greg KH
2026-08-18  6:22       ` Xin Chen
2026-08-17 14:16 ` Greg KH
2026-08-17 14:49   ` Greg KH
2026-08-18  6:02     ` Xin Chen
2026-08-18  6:34       ` Greg KH
2026-08-18  6:58         ` Xin Chen
2026-08-18  7:34           ` Greg KH [this message]
2026-08-18  3:31   ` Xin Chen
2026-08-18  6:07     ` Greg KH
2026-08-18  6:39       ` Xin Chen
2026-08-18  7:03 ` [PATCH v2] " Xin Chen
2026-08-18  7:15   ` Jiri Slaby
2026-08-18  7:23     ` Xin Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026081851-krypton-shady-b18d@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=cheng.jiang@oss.qualcomm.com \
    --cc=cxin@qti.qualcomm.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=liulzhao@qti.qualcomm.com \
    --cc=xin.chen2@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox