[Cluster-devel] [GFS2 PATCH 06/12] gfs2: Create transaction for inodes with i_nlink != 0

[Bob Peterson <rpeterso@redhat.com>]

----- Original Message -----
> On Fri, Aug 21, 2020 at 7:33 PM Bob Peterson <rpeterso@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