From: Brian Johannesmeyer <bjohannesmeyer@gmail.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-hardening@vger.kernel.org,
Raphael Isemann <teemperor@gmail.com>,
Cristiano Giuffrida <giuffrida@cs.vu.nl>,
Herbert Bos <h.j.bos@vu.nl>,
Greg KH <gregkh@linuxfoundation.org>,
Keith Busch <kbusch@kernel.org>
Subject: Re: [RFC v2 1/2] dmapool: Move pool metadata into non-DMA memory
Date: Wed, 20 Nov 2024 16:46:40 -0700 [thread overview]
Message-ID: <CAOZ5it2KXhBy0=ktgjAHMs8ut-Go2OXOt_vnWFiUBV7uBBH5HQ@mail.gmail.com> (raw)
In-Reply-To: <Zz2tzVqql2RMSFN4@infradead.org>
> Given that you now need an array of the blocks anyway, it might make
> sense to switch from a linked list to a bitmap for tracking free state,
> which would be a lot more efficient as you only need a bit per block
> as tracking overhead instead of a two pointers and a dma_addr_t.
>
> e.g. do a find_first_zero_bit() to find the ffree slot, then calculate
> the dma_addr and virt address by simple offseting into the dma_page
> ones with bitnr * pool->size.
Thank you for the suggestion. I hacked together a bitmap-based
approach as you proposed, and while it does improve memory efficiency
by reducing the per-block metadata overhead, it unfortunately appears
to significantly impact the runtime performance.
Here are the performance results, with DMAPOOL_DEBUG disabled. The
first two sets of numbers are the same as my latest response in the
other thread (i.e., [RFC v2 0/2]), and the last set of numbers is with
the bitmap approach applied:
**Without no patches applied:**
```
dmapool test: size:16 align:16 blocks:8192 time:11860
dmapool test: size:64 align:64 blocks:8192 time:11951
dmapool test: size:256 align:256 blocks:8192 time:12287
dmapool test: size:1024 align:1024 blocks:2048 time:3134
dmapool test: size:4096 align:4096 blocks:1024 time:1686
dmapool test: size:68 align:32 blocks:8192 time:12050
```
**With the submitted patches applied:**
```
dmapool test: size:16 align:16 blocks:8192 time:34432
dmapool test: size:64 align:64 blocks:8192 time:62262
dmapool test: size:256 align:256 blocks:8192 time:238137
dmapool test: size:1024 align:1024 blocks:2048 time:61386
dmapool test: size:4096 align:4096 blocks:1024 time:75342
dmapool test: size:68 align:32 blocks:8192 time:88243
```
**With the submitted patches applied AND using a bitmap approach:**
```
dmapool test: size:16 align:16 blocks:8192 time:82733
dmapool test: size:64 align:64 blocks:8192 time:198460
dmapool test: size:256 align:256 blocks:8192 time:710316
dmapool test: size:1024 align:1024 blocks:2048 time:177801
dmapool test: size:4096 align:4096 blocks:1024 time:192297
dmapool test: size:68 align:32 blocks:8192 time:274931
```
My guess as to why: The current linked list implementation allows us
to find the next free block in constant time (`O(1)`) by directly
dereferencing `pool->next_block`, and then following the `next_block`
pointers for subsequent free blocks. In contrast, the bitmap approach
requires iterating over all pages in `page->page_list` and, for each
page, iterating through its bitmap to find the first zero bit. This
results in a worst-case complexity of `O(n * b)`, where `n` is the
number of pages and `b` is the number of bits in each page's bitmap.
If you have ideas for mitigating this runtime overhead, I’d be happy
to explore them further.
Thanks,
Brian Johannesmeyer
next prev parent reply other threads:[~2024-11-20 23:46 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-19 20:55 [RFC v2 0/2] dmapool: Mitigate device-controllable mem. corruption Brian Johannesmeyer
2024-11-19 20:55 ` [RFC v2 1/2] dmapool: Move pool metadata into non-DMA memory Brian Johannesmeyer
2024-11-20 9:37 ` Christoph Hellwig
2024-11-20 23:46 ` Brian Johannesmeyer [this message]
2024-11-21 5:03 ` Christoph Hellwig
2024-11-21 17:48 ` Brian Johannesmeyer
2024-11-19 20:55 ` [RFC v2 2/2] dmapool: Use pool_find_block() in pool_block_err() Brian Johannesmeyer
2024-11-19 22:14 ` [RFC v2 0/2] dmapool: Mitigate device-controllable mem. corruption Greg KH
2024-11-19 22:22 ` Brian Johannesmeyer
2024-11-20 9:29 ` Christoph Hellwig
2024-11-20 15:56 ` Keith Busch
2024-11-20 18:51 ` Keith Busch
2024-11-20 21:58 ` Brian Johannesmeyer
2024-11-21 3:37 ` Keith Busch
2024-11-21 17:31 ` Brian Johannesmeyer
2024-11-21 18:06 ` Keith Busch
2024-11-21 19:07 ` Brian Johannesmeyer
2024-11-22 19:19 ` Brian Johannesmeyer
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='CAOZ5it2KXhBy0=ktgjAHMs8ut-Go2OXOt_vnWFiUBV7uBBH5HQ@mail.gmail.com' \
--to=bjohannesmeyer@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=giuffrida@cs.vu.nl \
--cc=gregkh@linuxfoundation.org \
--cc=h.j.bos@vu.nl \
--cc=hch@infradead.org \
--cc=kbusch@kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=teemperor@gmail.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