You need to turn on the debugging options Vlastimil mentioned and try to
figure out what nvme is doing wrong.
Agree, looks like some error path going wrong?
Since there seems to be actual non-large kmalloc usage involved, another
debug parameter that could help: CONFIG_SLUB_DEBUG=y, and boot with
"slab_debug=FZPU,kmalloc-*"
Presumably that's __iov_iter_get_pages_alloc() doing get_page() either in
the " if (iov_iter_is_bvec(i)) " branch or via iter_folioq_get_pages()?
Looks like it.
quoted
Which doesn't work for a sub-size kmalloc() from a slab folio, which after
the frozen refcount conversion no longer supports get_page().
The question is if this is a mistake specific for this path that's easy to
fix or there are more paths that do this. At the very least the pinning of
page through a kmalloc() allocation from it is useless - the object itself
has to be kfree()'d and that would never happen through a put_page()
reaching zero.
Looks like a specific mistake.
tls_sw is the only user of sk_msg_zerocopy_from_iter()
(which is calling into __iov_iter_get_pages_alloc()).
And, more to the point, tls_sw messes up iov pacing coming in from
the upper layers.
So even if the upper layers send individual iovs (where each iov might
contain different allocation types), tls_sw is packing them together
into full records. So it might end up with iovs having _different_
allocations.
Which would explain why we only see it with TLS, but not with normal
connections.
Or so my reasoning goes. Not sure if that's correct.
So I'd be happy with an 'easy' fix for now. Obviously :-)
Cheers,
Hannes
You need to turn on the debugging options Vlastimil mentioned and try to
figure out what nvme is doing wrong.
Agree, looks like some error path going wrong?
Since there seems to be actual non-large kmalloc usage involved, another
debug parameter that could help: CONFIG_SLUB_DEBUG=y, and boot with
"slab_debug=FZPU,kmalloc-*"
Presumably that's __iov_iter_get_pages_alloc() doing get_page() either in
the " if (iov_iter_is_bvec(i)) " branch or via iter_folioq_get_pages()?
Looks like it.
quoted
Which doesn't work for a sub-size kmalloc() from a slab folio, which after
the frozen refcount conversion no longer supports get_page().
The question is if this is a mistake specific for this path that's easy to
fix or there are more paths that do this. At the very least the pinning of
page through a kmalloc() allocation from it is useless - the object itself
has to be kfree()'d and that would never happen through a put_page()
reaching zero.
Looks like a specific mistake.
tls_sw is the only user of sk_msg_zerocopy_from_iter()
(which is calling into __iov_iter_get_pages_alloc()).
That's from tls_sw_sendmsg_locked(), right? But that's under:
if (!is_kvec && (full_record || eor) && !async_capable) {
Shouldn't is_kvec be true if we're dealing a kernel buffer (kmalloc()) there?
quoted
And, more to the point, tls_sw messes up iov pacing coming in from
the upper layers.
So even if the upper layers send individual iovs (where each iov might
contain different allocation types), tls_sw is packing them together
into full records. So it might end up with iovs having _different_
allocations.
Which would explain why we only see it with TLS, but not with normal
connections.
Or so my reasoning goes. Not sure if that's correct.
So I'd be happy with an 'easy' fix for now. Obviously :-)
Cheers,
Hannes
Presumably that's __iov_iter_get_pages_alloc() doing get_page() either in
the " if (iov_iter_is_bvec(i)) " branch or via iter_folioq_get_pages()?
It's the bvec path:
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, len);
quoted
quoted
Which doesn't work for a sub-size kmalloc() from a slab folio, which after
the frozen refcount conversion no longer supports get_page().
The question is if this is a mistake specific for this path that's easy to
fix or there are more paths that do this. At the very least the pinning of
page through a kmalloc() allocation from it is useless - the object itself
has to be kfree()'d and that would never happen through a put_page()
reaching zero.
Looks like a specific mistake.
tls_sw is the only user of sk_msg_zerocopy_from_iter()
(which is calling into __iov_iter_get_pages_alloc()).
And, more to the point, tls_sw messes up iov pacing coming in from
the upper layers.
So even if the upper layers send individual iovs (where each iov might
contain different allocation types), tls_sw is packing them together
into full records. So it might end up with iovs having _different_
allocations.
Which would explain why we only see it with TLS, but not with normal
connections.
I thought we'd done all the work needed to get rid of these pointless
refcount bumps. Turns out that's only on the block side (eg commit
e4cc64657bec). So what does networking need in order to understand
that some iovecs do not need to mess with the refcount?
You need to turn on the debugging options Vlastimil mentioned and try to
figure out what nvme is doing wrong.
Agree, looks like some error path going wrong?
Since there seems to be actual non-large kmalloc usage involved, another
debug parameter that could help: CONFIG_SLUB_DEBUG=y, and boot with
"slab_debug=FZPU,kmalloc-*"
Presumably that's __iov_iter_get_pages_alloc() doing get_page() either in
the " if (iov_iter_is_bvec(i)) " branch or via iter_folioq_get_pages()?
Looks like it.
quoted
Which doesn't work for a sub-size kmalloc() from a slab folio, which after
the frozen refcount conversion no longer supports get_page().
The question is if this is a mistake specific for this path that's easy to
fix or there are more paths that do this. At the very least the pinning of
page through a kmalloc() allocation from it is useless - the object itself
has to be kfree()'d and that would never happen through a put_page()
reaching zero.
Looks like a specific mistake.
tls_sw is the only user of sk_msg_zerocopy_from_iter()
(which is calling into __iov_iter_get_pages_alloc()).
That's from tls_sw_sendmsg_locked(), right? But that's under:
if (!is_kvec && (full_record || eor) && !async_capable) {
Shouldn't is_kvec be true if we're dealing a kernel buffer (kmalloc()) there?
Yes, and no.
We're initializing the iter in nvme_tcp_try_send_data():
bvec_set_page(&bvec, page, len, offset);
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, len);
and 'page' is coming from bio bvec. So the bv_page could refer to a
kmalloced page, the bvec is still that, a bvec.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Presumably that's __iov_iter_get_pages_alloc() doing get_page() either in
the " if (iov_iter_is_bvec(i)) " branch or via iter_folioq_get_pages()?
It's the bvec path:
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, len);
quoted
quoted
quoted
Which doesn't work for a sub-size kmalloc() from a slab folio, which after
the frozen refcount conversion no longer supports get_page().
The question is if this is a mistake specific for this path that's easy to
fix or there are more paths that do this. At the very least the pinning of
page through a kmalloc() allocation from it is useless - the object itself
has to be kfree()'d and that would never happen through a put_page()
reaching zero.
Looks like a specific mistake.
tls_sw is the only user of sk_msg_zerocopy_from_iter()
(which is calling into __iov_iter_get_pages_alloc()).
And, more to the point, tls_sw messes up iov pacing coming in from
the upper layers.
So even if the upper layers send individual iovs (where each iov might
contain different allocation types), tls_sw is packing them together
into full records. So it might end up with iovs having _different_
allocations.
Which would explain why we only see it with TLS, but not with normal
connections.
I thought we'd done all the work needed to get rid of these pointless
refcount bumps. Turns out that's only on the block side (eg commit
e4cc64657bec). So what does networking need in order to understand
that some iovecs do not need to mess with the refcount?
The network stack needs to get hold of the page while transmission is
ongoing, as there is potentially rather deep queueing involved,
requiring several calls to sendmsg() and friends before the page is
finally transmitted. And maybe some post-processing (checksums,
digests, you name it), too, all of which require the page to be there.
It's all so jumbled up ... personally, I would _love_ to do away with
__iov_iter_get_pages_alloc(). Allocating a page array? Seriously?
And the problem with that is that it's always takes a page(!) reference,
completely oblivious to the fact whether you even _can_ take a page
reference (eg for tail pages); we've hit this problem several times now
(check for sendpage_ok() ...).
But that's not the real issue; real issue is that the page reference is
taken down in the very bowels of __iov_iter_get_pages_alloc(), but needs
to be undone by the _caller_. Who might (or might not) have an idea
that he needs to drop the reference here.
That's why there is no straightforward conversion; you need to audit
each and every caller and try to find out where the page reference (if
any) is dropped.
Bah.
Can't we (at the very least) leave it to the caller of
__iov_iter_get_pages() to get a page reference (he has access to the
page array, after all ...)? That would make the interface slightly
better, and it'll be far more obvious to the caller what needs
to be done.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.com +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
From: Matthew Wilcox <willy@infradead.org> Date: 2025-03-04 16:53:16
On Tue, Mar 04, 2025 at 05:32:32PM +0100, Hannes Reinecke wrote:
On 3/4/25 17:14, Matthew Wilcox wrote:
quoted
I thought we'd done all the work needed to get rid of these pointless
refcount bumps. Turns out that's only on the block side (eg commit
e4cc64657bec). So what does networking need in order to understand
that some iovecs do not need to mess with the refcount?
The network stack needs to get hold of the page while transmission is
ongoing, as there is potentially rather deep queueing involved,
requiring several calls to sendmsg() and friends before the page is finally
transmitted. And maybe some post-processing (checksums,
digests, you name it), too, all of which require the page to be there.
It's all so jumbled up ... personally, I would _love_ to do away with
__iov_iter_get_pages_alloc(). Allocating a page array? Seriously?
And the problem with that is that it's always takes a page(!) reference,
completely oblivious to the fact whether you even _can_ take a page
reference (eg for tail pages); we've hit this problem several times now
(check for sendpage_ok() ...).
Calling get_page() / put_page() on a tail page is fine -- that just
redirects to the head page. But calling it on a slab never made any
sense; at best it gets you the equivalent of TYPESAFE_BY_RCU -- that is,
the object can be freed and reallocated, but the underlying slab will
not be reallocated to some other purpose.
But that's not the real issue; real issue is that the page reference is
taken down in the very bowels of __iov_iter_get_pages_alloc(), but needs
to be undone by the _caller_. Who might (or might not) have an idea
that he needs to drop the reference here.
That's why there is no straightforward conversion; you need to audit
each and every caller and try to find out where the page reference (if any)
is dropped.
Bah.
Can't we (at the very least) leave it to the caller of
__iov_iter_get_pages() to get a page reference (he has access to the page
array, after all ...)? That would make the interface slightly
better, and it'll be far more obvious to the caller what needs
to be done.
Right, that's what happened in the block layer. We mark the bio with
BIO_PAGE_PINNED if the pincount needs to be dropped. As a transitional
period, we had BIO_PAGE_REFFED which indicated that the page refcount
needed to be dropped. Perhaps there's something similar that network
could be doing.
From: Matthew Wilcox <willy@infradead.org> Date: 2025-03-04 18:06:41
On Tue, Mar 04, 2025 at 04:53:09PM +0000, Matthew Wilcox wrote:
Right, that's what happened in the block layer. We mark the bio with
BIO_PAGE_PINNED if the pincount needs to be dropped. As a transitional
period, we had BIO_PAGE_REFFED which indicated that the page refcount
needed to be dropped. Perhaps there's something similar that network
could be doing.
Until that time ... how does this look as a quick hack to avoid
reverting the slab change?
On Tue, Mar 04, 2025 at 04:53:09PM +0000, Matthew Wilcox wrote:
quoted
Right, that's what happened in the block layer. We mark the bio with
BIO_PAGE_PINNED if the pincount needs to be dropped. As a transitional
period, we had BIO_PAGE_REFFED which indicated that the page refcount
needed to be dropped. Perhaps there's something similar that network
could be doing.
Until that time ... how does this look as a quick hack to avoid
reverting the slab change?
And that means knowing where the put_page() side is and skipping it for slab
pages like for the get_page() side below. Hannes could you apply this patch
but also with WARN_ON_ONCE here in put_page()? And tell us where the
put_page() splats. Thanks!
From: Hannes Reinecke <hare@suse.de> Date: 2025-03-04 19:39:53
On 3/4/25 19:05, Matthew Wilcox wrote:
quoted hunk
On Tue, Mar 04, 2025 at 04:53:09PM +0000, Matthew Wilcox wrote:
quoted
Right, that's what happened in the block layer. We mark the bio with
BIO_PAGE_PINNED if the pincount needs to be dropped. As a transitional
period, we had BIO_PAGE_REFFED which indicated that the page refcount
needed to be dropped. Perhaps there's something similar that network
could be doing.
Until that time ... how does this look as a quick hack to avoid
reverting the slab change?
Good news and bad news ...
Good news: TLS works again!
Bad news: no errors.
Question to the wise: this is not the only place in iov_iter.c where we
do a 'get_page()'. Do we leave them and wait for others to report
regressions, knowing fully well that the current code _has_ issues?
Or shouldn't we rather clean them up?
I guess the real fix would be to fiddle with the 'bio_add_page()' logic;
we are always adding a 'page' reference to the bio, completely ignoring
whether this page is a slab page or a normal one.
Discussion at LSF, maybe?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
On Tue, Mar 04, 2025 at 04:53:09PM +0000, Matthew Wilcox wrote:
quoted
Right, that's what happened in the block layer. We mark the bio with
BIO_PAGE_PINNED if the pincount needs to be dropped. As a transitional
period, we had BIO_PAGE_REFFED which indicated that the page refcount
needed to be dropped. Perhaps there's something similar that network
could be doing.
Until that time ... how does this look as a quick hack to avoid
reverting the slab change?
Good news and bad news ...
Good news: TLS works again!
Bad news: no errors.
Wait, did you add a WARN_ON_ONCE() to the put_page() as I suggested? If yes
and there was no error, it would have to be leaking the page. Or the path
uses folio_put() and we'd need to put the warning there.
Question to the wise: this is not the only place in iov_iter.c where we
do a 'get_page()'. Do we leave them and wait for others to report
regressions, knowing fully well that the current code _has_ issues?
Or shouldn't we rather clean them up?
I guess the real fix would be to fiddle with the 'bio_add_page()' logic;
we are always adding a 'page' reference to the bio, completely ignoring
whether this page is a slab page or a normal one.
Discussion at LSF, maybe?
Cheers,
Hannes
From: Hannes Reinecke <hare@suse.de> Date: 2025-03-05 07:14:56
On 3/4/25 20:44, Vlastimil Babka wrote:
On 3/4/25 20:39, Hannes Reinecke wrote:
quoted
On 3/4/25 19:05, Matthew Wilcox wrote:
quoted
On Tue, Mar 04, 2025 at 04:53:09PM +0000, Matthew Wilcox wrote:
quoted
Right, that's what happened in the block layer. We mark the bio with
BIO_PAGE_PINNED if the pincount needs to be dropped. As a transitional
period, we had BIO_PAGE_REFFED which indicated that the page refcount
needed to be dropped. Perhaps there's something similar that network
could be doing.
Until that time ... how does this look as a quick hack to avoid
reverting the slab change?
Good news and bad news ...
Good news: TLS works again!
Bad news: no errors.
Wait, did you add a WARN_ON_ONCE() to the put_page() as I suggested? If yes
and there was no error, it would have to be leaking the page. Or the path
uses folio_put() and we'd need to put the warning there.
Oh, no, I didn't. Just added the WARN_ON to get_page().
Let me try ...
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
From: Hannes Reinecke <hare@suse.com> Date: 2025-03-05 08:20:57
On 3/4/25 20:44, Vlastimil Babka wrote:
On 3/4/25 20:39, Hannes Reinecke wrote:
[ .. ]
quoted
Good news and bad news ...
Good news: TLS works again!
Bad news: no errors.
Wait, did you add a WARN_ON_ONCE() to the put_page() as I suggested? If yes
and there was no error, it would have to be leaking the page. Or the path
uses folio_put() and we'd need to put the warning there.
Good news and bad news ...
Good news: TLS works again!
Bad news: no errors.
Wait, did you add a WARN_ON_ONCE() to the put_page() as I suggested? If yes
and there was no error, it would have to be leaking the page. Or the path
uses folio_put() and we'd need to put the warning there.
That triggers:
...
Not surprisingly, though, as the original code did a get_page(), so
there had to be a corresponding put_page() somewhere.
Is is this one? If there's no more warning afterwards, that should be it.
@@ -182,9 +182,14 @@ static int sk_msg_free_elem(struct sock *sk, struct sk_msg *msg, u32 i,/* When the skb owns the memory we free it from consume_skb path. */if(!msg->skb){+structfolio*folio;+if(charge)sk_mem_uncharge(sk,len);-put_page(sg_page(sge));++folio=page_folio(sg_page(sge));+if(!folio_test_slab(folio))+folio_put(folio);}memset(sge,0,sizeof(*sge));returnlen;
From: Hannes Reinecke <hare@suse.de> Date: 2025-03-05 11:43:05
On 3/5/25 09:58, Vlastimil Babka wrote:
quoted hunk
On 3/5/25 09:20, Hannes Reinecke wrote:
quoted
On 3/4/25 20:44, Vlastimil Babka wrote:
quoted
On 3/4/25 20:39, Hannes Reinecke wrote:
[ .. ]
quoted
quoted
Good news and bad news ...
Good news: TLS works again!
Bad news: no errors.
Wait, did you add a WARN_ON_ONCE() to the put_page() as I suggested? If yes
and there was no error, it would have to be leaking the page. Or the path
uses folio_put() and we'd need to put the warning there.
That triggers:
...
quoted
Not surprisingly, though, as the original code did a get_page(), so
there had to be a corresponding put_page() somewhere.
Is is this one? If there's no more warning afterwards, that should be it.
@@ -182,9 +182,14 @@ static int sk_msg_free_elem(struct sock *sk, struct sk_msg *msg, u32 i,/* When the skb owns the memory we free it from consume_skb path. */if(!msg->skb){+structfolio*folio;+if(charge)sk_mem_uncharge(sk,len);-put_page(sg_page(sge));++folio=page_folio(sg_page(sge));+if(!folio_test_slab(folio))+folio_put(folio);}memset(sge,0,sizeof(*sge));returnlen;
Oh, sure. But what annoys me: why do we have to care?
When doing I/O _all_ data is stuffed into bvecs via
bio_add_page(), and after that information about the
origin is lost; any iteration on the bio will be a bvec
iteration.
Previously we could just do a bvec iteration, get a reference
for each page, and start processing.
Now suddenly the caller has to check if it's a slab page and don't
get a reference for that. Not only that, he also has to remember
to _not_ drop the reference when he's done.
And, of course, tracing get_page() and the corresponding put_page()
calls through all the layers.
Really?
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
From: Matthew Wilcox <willy@infradead.org> Date: 2025-03-05 18:11:29
On Wed, Mar 05, 2025 at 12:43:02PM +0100, Hannes Reinecke wrote:
Oh, sure. But what annoys me: why do we have to care?
When doing I/O _all_ data is stuffed into bvecs via
bio_add_page(), and after that information about the
origin is lost; any iteration on the bio will be a bvec
iteration.
Previously we could just do a bvec iteration, get a reference
for each page, and start processing.
Now suddenly the caller has to check if it's a slab page and don't
get a reference for that. Not only that, he also has to remember
to _not_ drop the reference when he's done.
And, of course, tracing get_page() and the corresponding put_page()
calls through all the layers.
Networking needs to follow block's lead and STOP GETTING REFCOUNTS ON
PAGES. That will speed up networking (eliminates two atomic operations per
page). And of course, it will eliminate this hack in the MM. I think
we do need to put this hack into the MM for now, but it needs to go away
again as quickly as possible.
What worries me is that nobody in networking has replied to this thread
yet. Do they not care? Let's see if a subject line change will help
with that.
From: Cong Wang <hidden> Date: 2025-03-06 00:46:53
On Wed, Mar 05, 2025 at 06:11:24PM +0000, Matthew Wilcox wrote:
On Wed, Mar 05, 2025 at 12:43:02PM +0100, Hannes Reinecke wrote:
quoted
Oh, sure. But what annoys me: why do we have to care?
When doing I/O _all_ data is stuffed into bvecs via
bio_add_page(), and after that information about the
origin is lost; any iteration on the bio will be a bvec
iteration.
Previously we could just do a bvec iteration, get a reference
for each page, and start processing.
Now suddenly the caller has to check if it's a slab page and don't
get a reference for that. Not only that, he also has to remember
to _not_ drop the reference when he's done.
And, of course, tracing get_page() and the corresponding put_page()
calls through all the layers.
Networking needs to follow block's lead and STOP GETTING REFCOUNTS ON
PAGES. That will speed up networking (eliminates two atomic operations per
page). And of course, it will eliminate this hack in the MM. I think
we do need to put this hack into the MM for now, but it needs to go away
again as quickly as possible.
What worries me is that nobody in networking has replied to this thread
yet. Do they not care? Let's see if a subject line change will help
with that.
Since it triggered a kernel crash, I am pretty sure people care. How
about sending out a patch to get more attentions?
I am not sure what patterns here you are suggesting to change w.r.t page
refcount, but at least using AI copilot or whatever automation tool should
be very handy.
Thanks.
Good news and bad news ...
Good news: TLS works again!
Bad news: no errors.
Wait, did you add a WARN_ON_ONCE() to the put_page() as I suggested? If yes
and there was no error, it would have to be leaking the page. Or the path
uses folio_put() and we'd need to put the warning there.
That triggers:
...
quoted
Not surprisingly, though, as the original code did a get_page(), so
there had to be a corresponding put_page() somewhere.
Is is this one? If there's no more warning afterwards, that should be it.
@@ -182,9 +182,14 @@ static int sk_msg_free_elem(struct sock *sk, struct sk_msg *msg, u32 i,/* When the skb owns the memory we free it from consume_skb path. */if(!msg->skb){+structfolio*folio;+if(charge)sk_mem_uncharge(sk,len);-put_page(sg_page(sge));++folio=page_folio(sg_page(sge));+if(!folio_test_slab(folio))+folio_put(folio);}memset(sge,0,sizeof(*sge));returnlen;
Oh, sure. But what annoys me: why do we have to care?
When doing I/O _all_ data is stuffed into bvecs via
bio_add_page(), and after that information about the
origin is lost; any iteration on the bio will be a bvec
iteration.
Previously we could just do a bvec iteration, get a reference
for each page, and start processing.
AFAIU there's BIO_PAGE_PINNED that controls whether the pages are pinned, as
there are usecases where it makes sense to do that (userspace pages?). And
__bio_release_pages() can be removing the last pin and freeing the pages.
But this is a case where the buffer is a kmalloc() allocation, so somebody
has to do the corresponding kfree() when the messages are processed. A pin
on the slab folio where the kmalloc() resides helps nothing and as willy
says it's just unnecessary overhead of atomic allocations.
Now suddenly the caller has to check if it's a slab page and don't
get a reference for that. Not only that, he also has to remember
to _not_ drop the reference when he's done.
The caller did kmalloc() and will have to do kfree(). I guess it's about
telling the intermediate layers via something similar like BIO_PAGE_PINNED
whether the pages should be pinned or not.
And, of course, tracing get_page() and the corresponding put_page()
calls through all the layers.
Really?
Cheers,
Hannes
From: Christoph Hellwig <hch@infradead.org> Date: 2025-03-12 15:09:24
On Wed, Mar 05, 2025 at 06:11:24PM +0000, Matthew Wilcox wrote:
Networking needs to follow block's lead and STOP GETTING REFCOUNTS ON
PAGES.
The block layer never took references on pages. The direct I/O helpers
that just happened to set in block/ did hold references and abused some
field in the bio for it (and still do for the pinning), but the reference
was (and the pin now is) owned by the submitter.
The block layer model has always been that the submitter needs to ensure
memory stays allocated until the I/O has completed. Which IMHO is the
only sane model for dealing with memory lifetimes vs I/O, and something
networking absolutely should follow.
From: James R. Bergsten <hidden> Date: 2025-03-12 18:34:58
OK another "unnecessary" old-timer storage/network story/disruption to your otherwise relevant discussions (thank the subject line). If you're too busy, just don't read it. 😊
Around 1985, Gene Amdahl founded a company called Andor. Its original purpose (as was with everything Gene did) was to build the smallest plug-compatible mainframe. When it was designed, someone noted it had no physical room for the humongous "Bus and Tag" cables needed for peripherals, so Gene raised a bit more money and started a storage project too.
When the Loma Prieta earthquake happened in 1989, PG&E, the local utility, lost the datacenter containing all of the information needed to repair their utilities, so the service people had to do this from memory. The Public Utilities Commission didn't find this terribly funny, so they said PG&E had to create a second datacenter out of the area immediately and have backups there within about 24 hours, shorter as time went on. So, they shut down the primary site every night, dumped to tape, then drove it up to Sacramento where these were restored. They named this CTAM for "Chevy Truck Access Method."
Somehow Gene and friends heard about this and, as they already had a processor, device simulation and devices, if they added some sort of networking interface, they could have a local unit and a remote unit doing this backup, eliminating the truck. BTW the "front end" storage group all came from Memorex. The "back end" group mostly from Amdahl.
This actually (somewhat) worked, and a couple of units were installed in beta sites. Sadly, Gene ran out of money (or at least didn't accept the terms offered) and buggered off to start yet another mainframe company which never shipped anything.
I was the last Engineering VP at Andor, so when it folded, I grabbed a few of the people and started a similar company but for the open systems market instead. We named it "Ark" at my wife's suggestion as was like Noah's Ark - "disaster recovery" and "two of everything." We mostly bootstrapped, did ship product, and were acquired by LSI Logic who were getting beaten around the head as EMC had a remote solution, but LSI didn't. I got about a dozen US Patents Issued and enough money to finally buy a house in Silicon Valley.
Our (SCSI-based) device had front end ports for the host(s), back-end ones for the devices, and side ones for the networking. Lots of features, some you folks are only doing recently. Looked like devices to hosts, hosts to devices.
Anyway, the point of all of this is that when we sold it to customers, the storage people looked at the network ports with confusion and dismay (some hadn't even ordered the network lines and caused months of delay), while the network people looked at the device ports as if they were full of Tasmanian devils.
Turned out, both network and storage expertise were very rare commodities. This was largely why most iSCSI startups failed, they either did a storage product or a networking product. We pilled this off because I am stupid but stubborn and wrote the RTOS myself (Linux was in its infancy and the other RTOS's sucked). Seemed a good idea at the time. Have white papers online if anybody is interested.
So, networking people may smell funny, but to them storage people come from another galaxy. Working in this industry at all could be considered a poor life choice but that's for another time.
Sorry. You can go back to work now.
Jim B
-----Original Message-----
From: Linux-nvme <redacted> On Behalf Of Christoph Hellwig
Sent: Wednesday, March 12, 2025 8:09 AM
To: Matthew Wilcox <willy@infradead.org>
Cc: Hannes Reinecke <hare@suse.de>; Vlastimil Babka <redacted>; Hannes Reinecke <hare@suse.com>; Boris Pismenny <borisp@nvidia.com>; John Fastabend <john.fastabend@gmail.com>; Jakub Kicinski <kuba@kernel.org>; Sagi Grimberg <sagi@grimberg.me>; linux-nvme@lists.infradead.org; linux-block@vger.kernel.org; linux-mm@kvack.org; Harry Yoo <redacted>; netdev@vger.kernel.org
Subject: Re: Networking people smell funny and make poor life choices
On Wed, Mar 05, 2025 at 06:11:24PM +0000, Matthew Wilcox wrote:
Networking needs to follow block's lead and STOP GETTING REFCOUNTS ON
PAGES.
The block layer never took references on pages. The direct I/O helpers that just happened to set in block/ did hold references and abused some field in the bio for it (and still do for the pinning), but the reference was (and the pin now is) owned by the submitter.
The block layer model has always been that the submitter needs to ensure memory stays allocated until the I/O has completed. Which IMHO is the only sane model for dealing with memory lifetimes vs I/O, and something networking absolutely should follow.