Re: [f2fs-dev] [PATCH] f2fs: handle decompress only post processing in softirq
On Tue, Jun 14, 2022 at 10:49:37AM -0700, Daeho Jeong wrote: > > Yeah, I heard that you folks are really suffered from the scheduling > > issues. But for my own previous experience, extra memory footprints are > > really critical in Android low memory scenarios (no matter low-ended > > devices or artificial workloads), it tossed me a lot. So I finally > > ntroduced many inplace I/O to handle/minimize that, including inplace > > I/O for compressed pages and temporary pages. > > > > But I'm not quite sure what's currently happening now, since we once > > didn't have such non-deterministic workqueues, and I don't hear from > > other landed vendors. I think it'd be better to analyse what's going > > on for these kworkers from scheduling POV and why they don't schedule > > in time. > > > > I also have an idea is much like what I'm doing now for sync > > decompression, is that just before lock page and ->read_folio, we can > > trigger some decompression in addition to kworker decompression, but it > > needs some MM modification, as below: > > > >!PageUptodate(page) > > > >some callback to decompress in addition to kworker > > > >lock_page() > >->read_folio() > > > > If mm folks don't like it, I think RT thread is also fine after we > > analysed the root cause of the kworker delay I think. > > > > Thanks, > > Gao Xiang > > > > > > > > Thanks, > > I don't think this is not a problem with most devices, since the > allocated memory is not too big and it'll be kept just as long as I/O > processing is on. However, I still understand what you're worried > about, so I think I can make a new mount option like "memory=low", > which can be used to give a hint to F2FS to have a priority on as > little memory as possible. In this mode, we will try to keep minimal > memory and we can use the previous implementation for decompression. Okay, one of our previous tests was that how many applications are still there after many other applications boot. That makes sense since most users need to leave as many apps as possible, I know for now we have swap-like thing, but once it was done without swap. If you reserve too much memory (with page mempool or even for inflight I/O), it will impact the alive app numbers compared to uncompressed cases for all devices (even high-ended devices). BTW, most crypto algorithms have hardware instructions to boost up, actually we have some in-house neon lz4 assembly as well. but it still somewhat slow than crypto algorithms, not to mention some algorithms like zstd or lzma. Anyway, I personally prefer RT Thread way since it's more flexible, also for dm-verity at least try with WQ_HIGHPRI, and I've seen: https://android-review.googlesource.com/c/kernel/common/+/204421 But I'm not sure why it wasn't upstreamed though. Thanks, Gao Xiang > > Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: handle decompress only post processing in softirq
> Yeah, I heard that you folks are really suffered from the scheduling > issues. But for my own previous experience, extra memory footprints are > really critical in Android low memory scenarios (no matter low-ended > devices or artificial workloads), it tossed me a lot. So I finally > ntroduced many inplace I/O to handle/minimize that, including inplace > I/O for compressed pages and temporary pages. > > But I'm not quite sure what's currently happening now, since we once > didn't have such non-deterministic workqueues, and I don't hear from > other landed vendors. I think it'd be better to analyse what's going > on for these kworkers from scheduling POV and why they don't schedule > in time. > > I also have an idea is much like what I'm doing now for sync > decompression, is that just before lock page and ->read_folio, we can > trigger some decompression in addition to kworker decompression, but it > needs some MM modification, as below: > >!PageUptodate(page) > >some callback to decompress in addition to kworker > >lock_page() >->read_folio() > > If mm folks don't like it, I think RT thread is also fine after we > analysed the root cause of the kworker delay I think. > > Thanks, > Gao Xiang > > > > > Thanks, I don't think this is not a problem with most devices, since the allocated memory is not too big and it'll be kept just as long as I/O processing is on. However, I still understand what you're worried about, so I think I can make a new mount option like "memory=low", which can be used to give a hint to F2FS to have a priority on as little memory as possible. In this mode, we will try to keep minimal memory and we can use the previous implementation for decompression. Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: handle decompress only post processing in softirq
Hi Daeho, On Tue, Jun 14, 2022 at 09:46:50AM -0700, Daeho Jeong wrote: > > > > Some my own previous thoughts about this strategy: > > > > - If we allocate all memory and map these before I/Os, all inflight I/Os > >will keep such temporary pages all the time until decompression is > >finished. In contrast, if we allocate or reuse such pages just before > >decompression, it would minimize the memory footprints. > > > >I think it will impact the memory numbers at least on the very > >low-ended devices with bslow storage. (I've seen f2fs has some big > >mempool already) > > > > - Many compression algorithms are not suitable in the softirq contexts, > >also I vaguely remembered if softirq context lasts for > 2ms, it will > >push into ksoftirqd instead so it's actually another process context. > >And it may delay other important interrupt handling. > > > > - Go back to the non-deterministic scheduling of workqueues. I guess it > >may be just due to scheduling punishment due to a lot of CPU consuming > >due to decompression before so the priority becomes low, but that is > >just a pure guess. May be we need to use RT scheduling policy instead. > > > >At least with WQ_HIGHPRI for dm-verity at least, but I don't find > >WQ_HIGHPRI mark for dm-verity. > > > > Thanks, > > Gao Xiang > > I totally understand what you are worried about. However, in the real > world, non-determinism from workqueues is more harsh than we expected. > As you know, reading I/Os in the system are critical paths most of the > time and now I/O variations with workqueue are too bad. > > I also think it's better that we have RT scheduling like things here. > We could think about it more. Yeah, I heard that you folks are really suffered from the scheduling issues. But for my own previous experience, extra memory footprints are really critical in Android low memory scenarios (no matter low-ended devices or artificial workloads), it tossed me a lot. So I finally ntroduced many inplace I/O to handle/minimize that, including inplace I/O for compressed pages and temporary pages. But I'm not quite sure what's currently happening now, since we once didn't have such non-deterministic workqueues, and I don't hear from other landed vendors. I think it'd be better to analyse what's going on for these kworkers from scheduling POV and why they don't schedule in time. I also have an idea is much like what I'm doing now for sync decompression, is that just before lock page and ->read_folio, we can trigger some decompression in addition to kworker decompression, but it needs some MM modification, as below: !PageUptodate(page) some callback to decompress in addition to kworker lock_page() ->read_folio() If mm folks don't like it, I think RT thread is also fine after we analysed the root cause of the kworker delay I think. Thanks, Gao Xiang > > Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: handle decompress only post processing in softirq
> > Some my own previous thoughts about this strategy: > > - If we allocate all memory and map these before I/Os, all inflight I/Os >will keep such temporary pages all the time until decompression is >finished. In contrast, if we allocate or reuse such pages just before >decompression, it would minimize the memory footprints. > >I think it will impact the memory numbers at least on the very >low-ended devices with bslow storage. (I've seen f2fs has some big >mempool already) > > - Many compression algorithms are not suitable in the softirq contexts, >also I vaguely remembered if softirq context lasts for > 2ms, it will >push into ksoftirqd instead so it's actually another process context. >And it may delay other important interrupt handling. > > - Go back to the non-deterministic scheduling of workqueues. I guess it >may be just due to scheduling punishment due to a lot of CPU consuming >due to decompression before so the priority becomes low, but that is >just a pure guess. May be we need to use RT scheduling policy instead. > >At least with WQ_HIGHPRI for dm-verity at least, but I don't find >WQ_HIGHPRI mark for dm-verity. > > Thanks, > Gao Xiang I totally understand what you are worried about. However, in the real world, non-determinism from workqueues is more harsh than we expected. As you know, reading I/Os in the system are critical paths most of the time and now I/O variations with workqueue are too bad. I also think it's better that we have RT scheduling like things here. We could think about it more. Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: handle decompress only post processing in softirq
> One question: is this (the bio endio callback) actually guaranteed to be > executed from a softirq? If you look at dm-crypt's support for workqueue-less > decryption, for example, it explicitly checks 'in_hardirq() || > irqs_disabled()' > and schedules a tasklet if either of those is the case. > > - Eric Oh, you're right. Even though it's safe to defer all the release process as a work in end_io function, it's better to check the condition and process the release process right away if possible. Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: handle decompress only post processing in softirq
Hi all, On Mon, Jun 13, 2022 at 10:38:25PM -0700, Eric Biggers wrote: > [+Cc Nathan Huckleberry who is looking into a similar problem in dm-verity] > > On Mon, Jun 13, 2022 at 08:56:12AM -0700, Daeho Jeong wrote: > > From: Daeho Jeong > > > > Now decompression is being handled in workqueue and it makes read I/O > > latency non-deterministic, because of the non-deterministic scheduling > > nature of workqueues. So, I made it handled in softirq context only if > > possible. > > > > Signed-off-by: Daeho Jeong ... > > One question: is this (the bio endio callback) actually guaranteed to be > executed from a softirq? If you look at dm-crypt's support for workqueue-less > decryption, for example, it explicitly checks 'in_hardirq() || > irqs_disabled()' > and schedules a tasklet if either of those is the case. > > - Eric > Some my own previous thoughts about this strategy: - If we allocate all memory and map these before I/Os, all inflight I/Os will keep such temporary pages all the time until decompression is finished. In contrast, if we allocate or reuse such pages just before decompression, it would minimize the memory footprints. I think it will impact the memory numbers at least on the very low-ended devices with bslow storage. (I've seen f2fs has some big mempool already) - Many compression algorithms are not suitable in the softirq contexts, also I vaguely remembered if softirq context lasts for > 2ms, it will push into ksoftirqd instead so it's actually another process context. And it may delay other important interrupt handling. - Go back to the non-deterministic scheduling of workqueues. I guess it may be just due to scheduling punishment due to a lot of CPU consuming due to decompression before so the priority becomes low, but that is just a pure guess. May be we need to use RT scheduling policy instead. At least with WQ_HIGHPRI for dm-verity at least, but I don't find WQ_HIGHPRI mark for dm-verity. Thanks, Gao Xiang ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: handle decompress only post processing in softirq
[+Cc Nathan Huckleberry who is looking into a similar problem in dm-verity]
On Mon, Jun 13, 2022 at 08:56:12AM -0700, Daeho Jeong wrote:
> From: Daeho Jeong
>
> Now decompression is being handled in workqueue and it makes read I/O
> latency non-deterministic, because of the non-deterministic scheduling
> nature of workqueues. So, I made it handled in softirq context only if
> possible.
>
> Signed-off-by: Daeho Jeong
> ---
> fs/f2fs/compress.c | 145 +
> fs/f2fs/data.c | 50 ++--
> fs/f2fs/f2fs.h | 10 ++--
> 3 files changed, 119 insertions(+), 86 deletions(-)
[...]
> static void f2fs_read_end_io(struct bio *bio)
> @@ -281,16 +283,28 @@ static void f2fs_read_end_io(struct bio *bio)
> }
>
> if (bio->bi_status) {
> - f2fs_finish_read_bio(bio);
> + f2fs_finish_read_bio(bio, true);
> return;
> }
>
> - if (ctx && (ctx->enabled_steps & (STEP_DECRYPT | STEP_DECOMPRESS))) {
> - INIT_WORK(>work, f2fs_post_read_work);
> - queue_work(ctx->sbi->post_read_wq, >work);
> - } else {
> - f2fs_verify_and_finish_bio(bio);
> + if (ctx) {
> + unsigned int enabled_steps = ctx->enabled_steps &
> + (STEP_DECRYPT | STEP_DECOMPRESS);
> +
> + /*
> + * If we have only decompression step between decompression and
> + * decrypt, we don't need post processing for this.
> + */
> + if (enabled_steps == STEP_DECOMPRESS) {
> + f2fs_handle_step_decompress(ctx, true);
One question: is this (the bio endio callback) actually guaranteed to be
executed from a softirq? If you look at dm-crypt's support for workqueue-less
decryption, for example, it explicitly checks 'in_hardirq() || irqs_disabled()'
and schedules a tasklet if either of those is the case.
- Eric
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
