On Fri, Apr 17, 2015 at 03:15:21PM +0800, çæ wrote:
> On Fri, Apr 17, 2015 at 12:06 PM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
> > On Fri, Apr 17, 2015 at 10:22:24AM +0800, xuw2015@xxxxxxxxx wrote:
> >> From: George Wang <xuw2015@xxxxxxxxx>
> >> Function percpu_counter_read just return the current counter, regardless of
> >> every cpu's count. This counter can be negative value, which will cause the
> >> checking of "allocated inode counts <= m_maxicount" false positive.
> > Have you actually seen this, or is it just theoretical?
> I produced it by running unionmount-testsuites for overlay, based xfs as upper
> and lowerdir. I think I can extract the procedure to reproduce it.
Inteesting. I've never been able to get the unionmount-testsuit to
actually be able to run, let alone had it find problems.
> >> Commit 501ab3238753 "xfs: use generic percpu counters for inode counter
> > The correct fix is to use percpu_counter_read_positive(), because in
> > the majority of cases args.mp->m_maxicount is orders of magnitude
> > larger (20 million inodes per 100GB of fs space for small filesystems)
> > than the unaggregated per-cpu counts can cause the sum to go
> > negative. Hence if it is negative, it may as well be zero because it
> > makes no difference to the default threshold configurations.
> Thanks for the explanation, and I agree that totally.
> Besides, shall we use the exact count in xfs_fs_counts? Because it's a ioctl
> function, not so much sensitive performance.
Right, but that makes percpu_counter_read_positive is perfect for
ioctls like this: what is reported to userspace is never
perfectly accurate on a busy filesystem. Also, we don't what someone
repeatedly running that ioctl to adversely affect the performance of
the filesystem, which is what would happen if it was an accurate