lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 10 Jun 2021 10:38:53 +0800 From: Zhang Yi <yi.zhang@...wei.com> To: Dave Chinner <david@...morbit.com> CC: <linux-ext4@...r.kernel.org>, <tytso@....edu>, <adilger.kernel@...ger.ca>, <jack@...e.cz>, <yukuai3@...wei.com> Subject: Re: [RFC PATCH v3 5/8] jbd2,ext4: add a shrinker to release checkpointed buffers Hi, Dave On 2021/6/9 11:00, Dave Chinner wrote: > On Thu, May 27, 2021 at 09:56:38PM +0800, Zhang Yi wrote: >> +/** >> + * jbd2_journal_shrink_scan() >> + * >> + * Scan the checkpointed buffer on the checkpoint list and release the >> + * journal_head. >> + */ >> +unsigned long jbd2_journal_shrink_scan(struct shrinker *shrink, >> + struct shrink_control *sc) >> +{ >> + journal_t *journal = container_of(shrink, journal_t, j_shrinker); >> + unsigned long nr_to_scan = sc->nr_to_scan; >> + unsigned long nr_shrunk; >> + unsigned long count; >> + >> + count = percpu_counter_sum_positive(&journal->j_jh_shrink_count); >> + trace_jbd2_shrink_scan_enter(journal, sc->nr_to_scan, count); >> + >> + nr_shrunk = jbd2_journal_shrink_checkpoint_list(journal, &nr_to_scan); >> + >> + count = percpu_counter_sum_positive(&journal->j_jh_shrink_count); >> + trace_jbd2_shrink_scan_exit(journal, nr_to_scan, nr_shrunk, count); >> + >> + return nr_shrunk; >> +} >> + >> +/** >> + * jbd2_journal_shrink_scan() >> + * >> + * Count the number of checkpoint buffers on the checkpoint list. >> + */ >> +unsigned long jbd2_journal_shrink_count(struct shrinker *shrink, >> + struct shrink_control *sc) >> +{ >> + journal_t *journal = container_of(shrink, journal_t, j_shrinker); >> + unsigned long count; >> + >> + count = percpu_counter_sum_positive(&journal->j_jh_shrink_count); >> + trace_jbd2_shrink_count(journal, sc->nr_to_scan, count); >> + >> + return count; >> +} > > NACK. > > You should not be putting an expensive percpu counter sum in a > shrinker object count routine. These gets called a *lot* under > memory pressure, and summing over hundreds/thousands of CPUs on > every direct reclaim instance over every mounted filesystem > instance that uses jbd2 is really, really going to hurt system > performance under memory pressure. > > And, quite frankly, summing twice in the scan routine just to trace > the result with no other purpose is unnecessary and excessive CPU > overhead for a shrinker. > > Just use percpu_counter_read() in all cases here - it is more than > accurate enough for the purposes of both tracing and memory reclaim. OK, I missed this point, thanks for the comments, I will use percpu_counter_read_positive() in the next iteration. Thanks, Yi.
Powered by blists - more mailing lists