Messages in this thread | | | Date | Wed, 4 Oct 2023 17:32:31 +0200 | From | Jan Kara <> | Subject | Re: [PATCH 8/8] shmem,percpu_counter: add _limited_add(fbc, limit, amount) |
| |
On Fri 29-09-23 20:42:45, Hugh Dickins wrote: > Percpu counter's compare and add are separate functions: without locking > around them (which would defeat their purpose), it has been possible to > overflow the intended limit. Imagine all the other CPUs fallocating > tmpfs huge pages to the limit, in between this CPU's compare and its add. > > I have not seen reports of that happening; but tmpfs's recent addition > of dquot_alloc_block_nodirty() in between the compare and the add makes > it even more likely, and I'd be uncomfortable to leave it unfixed. > > Introduce percpu_counter_limited_add(fbc, limit, amount) to prevent it. > > I believe this implementation is correct, and slightly more efficient > than the combination of compare and add (taking the lock once rather > than twice when nearing full - the last 128MiB of a tmpfs volume on a > machine with 128 CPUs and 4KiB pages); but it does beg for a better > design - when nearing full, there is no new batching, but the costly > percpu counter sum across CPUs still has to be done, while locked. > > Follow __percpu_counter_sum()'s example, including cpu_dying_mask as > well as cpu_online_mask: but shouldn't __percpu_counter_compare() and > __percpu_counter_limited_add() then be adding a num_dying_cpus() to > num_online_cpus(), when they calculate the maximum which could be held > across CPUs? But the times when it matters would be vanishingly rare. > > Signed-off-by: Hugh Dickins <hughd@google.com> > Cc: Tim Chen <tim.c.chen@intel.com> > Cc: Dave Chinner <dchinner@redhat.com> > Cc: Darrick J. Wong <djwong@kernel.org>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
-- Jan Kara <jack@suse.com> SUSE Labs, CR
| |