On Sat, Jun 06, 2020 at 07:07:46AM +1000, Dave Chinner wrote: > On Fri, Jun 05, 2020 at 12:26:11PM -0400, Brian Foster wrote: > > On Thu, Jun 04, 2020 at 05:45:55PM +1000, Dave Chinner wrote: > > > From: Dave Chinner <dchinner@xxxxxxxxxx> > > > > > > Inode reclaim will still throttle direct reclaim on the per-ag > > > reclaim locks. This is no longer necessary as reclaim can run > > > non-blocking now. Hence we can remove these locks so that we don't > > > arbitrarily block reclaimers just because there are more direct > > > reclaimers than there are AGs. > > > > > > This can result in multiple reclaimers working on the same range of > > > an AG, but this doesn't cause any apparent issues. Optimising the > > > spread of concurrent reclaimers for best efficiency can be done in a > > > future patchset. > > > > > > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> > > > Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > --- > > > fs/xfs/xfs_icache.c | 31 ++++++++++++------------------- > > > fs/xfs/xfs_mount.c | 4 ---- > > > fs/xfs/xfs_mount.h | 1 - > > > 3 files changed, 12 insertions(+), 24 deletions(-) > > > > > > diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c > > > index 74032316ce5cc..c4ba8d7bc45bc 100644 > > > --- a/fs/xfs/xfs_icache.c > > > +++ b/fs/xfs/xfs_icache.c > > ... > > > @@ -1298,11 +1293,9 @@ xfs_reclaim_inodes_ag( > > > > > > } while (nr_found && !done && *nr_to_scan > 0); > > > > > > - if (trylock && !done) > > > - pag->pag_ici_reclaim_cursor = first_index; > > > - else > > > - pag->pag_ici_reclaim_cursor = 0; > > > - mutex_unlock(&pag->pag_ici_reclaim_lock); > > > + if (done) > > > + first_index = 0; > > > + WRITE_ONCE(pag->pag_ici_reclaim_cursor, first_index); > > > > I thought the [READ|WRITE]_ONCE() macros had to do with ordering, not > > necessarily atomicity. Is this write safe if we're running a 32-bit > > kernel, for example? Outside of that the broader functional change seems > > reasonable. > > They are used for documenting intentional data races now, too. > That's what these are - we don't care about serialisation, but there > are static checkers that will now spew "data race" warnings because > multiple threads can race reading and writing unserialised > variables. > I wasn't aware of that. I'm not sure how widely known that is so it might be worth a one liner comment to ensure these are preserved (if they survive the end of the series). > It is safe on 32 bit machines because these variables are 32 bit on > 32 bit machines, and reads/writes of 32 bit variables on 32 bit > machines are atomic (though not serialised). > Ah, right. I was thinking they were always 64-bit but that is not the case. With that: Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx> > Cheers, > > Dave. > -- > Dave Chinner > david@xxxxxxxxxxxxx >
