----- Original Message ----- > On Fri, Aug 21, 2020 at 7:33 PM Bob Peterson <rpete...@redhat.com> wrote: > > Before this patch, function gfs2_evict_inode would check if i_nlink > > was non-zero, and if so, go to label out. The problem is, the evicted > > file may still have outstanding pages that need invalidating, but > > the call to truncate_inode_pages_final at label out doesn't start a > > transaction. It needs a transaction in order to write revokes for any > > pages it has to invalidate. > > This is only true for jdata inodes though, right? If so, I'd rather > just create transactions in the jdata case.
The truncate_inode_pages_final() for i_data is only for jdata, which includes directories for their hash tables. However, for regular files, evict's call to gfs2_glock_put_eventually() has the potential to be the last put for the inode's glock (in a race), which might still have pages attached (metamapping). I firmly believe this is our "nrpages" bug I've been chasing, but I haven't proven it yet because it's very hard to recreate. Afaik, some of these unresolved metadata pages may still need revokes, and we still need a transaction to do that, even if the dinode still has links. The "nrpages" problem always seems to involve the system quotas file, probably because it's jdata, but imagine a directory with a large hash table, which is modified, then is quickly evicted (without being deleted). It wasn't that long ago I was working on a patch to take glock reference even sooner than we did for f4e2f5e1a527ce58fc9f85145b03704779a3123e. I titled the patch "grab glock reference as early as possible in transactions but it was never pushed anywhere because it added a new atomic to the glock. It may be an alternative solution to the problem. My comments on that patch were: Before this patch, an additional glock reference was taken when the bufdata element, bd, was revoked. That's not early enough because the caller who created the bd (via trans_add_meta) may have already come and gone with the bd still not revoked (but in the ail). This patch takes the glock reference earlier in the process, when the first bd element is allocated for a glock. It queues the glock reference to be put when the last bd element for the glock is freed. To this end, a new atomic glock field, gl_bd_count, keeps count. Regards, Bob Peterson