xfs
[Top] [All Lists]

Re: [PATCH 16/71] xfs: log refcount intent items

To: "Darrick J. Wong" <darrick.wong@xxxxxxxxxx>
Subject: Re: [PATCH 16/71] xfs: log refcount intent items
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 9 Sep 2016 09:13:26 +1000
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, linux-xfs@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20160908191404.GB8969@xxxxxxxxxxxxxxxx>
References: <147216791538.867.12413509832420924168.stgit@xxxxxxxxxxxxxxxx> <147216802075.867.12945255918683675311.stgit@xxxxxxxxxxxxxxxx> <20160906152155.GJ24287@xxxxxxxxxxxxx> <20160908191404.GB8969@xxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Sep 08, 2016 at 12:14:04PM -0700, Darrick J. Wong wrote:
> On Tue, Sep 06, 2016 at 08:21:55AM -0700, Christoph Hellwig wrote:
> > > + __uint64_t                      cui_id;
> > > + struct xfs_ail_cursor           cur;
> > > + struct xfs_ail                  *ailp = log->l_ailp;
> > > +
> > > + cud_formatp = item->ri_buf[0].i_addr;
> > > + ASSERT(item->ri_buf[0].i_len == sizeof(struct xfs_cud_log_format));
> > 
> > Should we return -EFSCORRUPTED here instead?
> 
> Yes.  The RUD recovery routine should probably get that change too.
> 
> > > + /* XXX: do nothing for now */
> > 
> > What else would be do in the future here?
> > 
> > > +static void
> > > +xfs_trans_set_refcount_flags(
> > > + struct xfs_phys_extent          *refc,
> > > + enum xfs_refcount_intent_type   type)
> > > +{
> > > + refc->pe_flags = 0;
> > > + switch (type) {
> > > + case XFS_REFCOUNT_INCREASE:
> > > +         refc->pe_flags |= XFS_REFCOUNT_EXTENT_INCREASE;
> > > +         break;
> > > + case XFS_REFCOUNT_DECREASE:
> > > +         refc->pe_flags |= XFS_REFCOUNT_EXTENT_DECREASE;
> > > +         break;
> > > + case XFS_REFCOUNT_ALLOC_COW:
> > > +         refc->pe_flags |= XFS_REFCOUNT_EXTENT_ALLOC_COW;
> > > +         break;
> > > + case XFS_REFCOUNT_FREE_COW:
> > > +         refc->pe_flags |= XFS_REFCOUNT_EXTENT_FREE_COW;
> > > +         break;
> > 
> > Is there any good reasons to use a type enum in core, but flags on
> > disk?
> 
> I suppose since log structures aren't guaranteed to be platform agnostic
> it's fine to just copy in the in-core enum here.

Carfeul there - enums are not defined to have a fixed size and so
can change from compiler version to compiler version. IOWs, the
enum values can be written idirectly to an on-disk structure, but
the on-disk structure should not be using the enum as the type
definition for whatever gets stored on disk.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>