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
next prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.