From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay2.corp.sgi.com [137.38.102.29]) by oss.sgi.com (Postfix) with ESMTP id 97AA57F3F for ; Thu, 18 Dec 2014 03:59:29 -0600 (CST) Received: from cuda.sgi.com (cuda1.sgi.com [192.48.157.11]) by relay2.corp.sgi.com (Postfix) with ESMTP id 6878630407A for ; Thu, 18 Dec 2014 01:59:26 -0800 (PST) Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by cuda.sgi.com with ESMTP id XNAwodwoybD7fvKR (version=TLSv1 cipher=AES256-SHA bits=256 verify=NO) for ; Thu, 18 Dec 2014 01:59:24 -0800 (PST) Date: Thu, 18 Dec 2014 10:59:19 +0100 From: Jan Kara Subject: Re: [PATCH] xfs: Keep sb_bad_features2 consistent with sb_features2 Message-ID: <20141218095919.GA13705@quack.suse.cz> References: <1418848046-11265-1-git-send-email-jack@suse.cz> <20141217212255.GZ24183@dastard> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20141217212255.GZ24183@dastard> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Dave Chinner Cc: Jan Kara , xfs@oss.sgi.com On Thu 18-12-14 08:22:55, Dave Chinner wrote: > On Wed, Dec 17, 2014 at 09:27:26PM +0100, Jan Kara wrote: > > Currently when we modify sb_features2, we store the same value also in > > sb_bad_features2. However in most places we forget to mark field > > sb_bad_features2 for logging and thus it can happen that a change to it > > is lost. This results in an inconsistent sb_features2 and > > sb_bad_features2 fields e.g. after xfstests test xfs/187. > > > > Fix the problem by changing XFS_SB_FEATURES2 to actually mean both > > sb_features2 and sb_bad_features2 fields since this is always what we > > want to log. This isn't ideal because the fact that XFS_SB_FEATURES2 > > means two fields could cause some problem in future however the code is > > hopefully less error prone that it is now. > > Actually, I have patches that fix this differently that I'm planning > to push for the 3.20 cycle. They get rid of the "update random SB > fields" problem altogether simply by logging and updating the entire > SB every time. > > http://oss.sgi.com/archives/xfs/2014-09/msg00448.html OK, thanks for info. Honza > > > Signed-off-by: Jan Kara > > --- > > fs/xfs/libxfs/xfs_format.h | 10 +++++----- > > fs/xfs/xfs_mount.c | 5 ++--- > > 2 files changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h > > index fbd6da263571..476273287aaf 100644 > > --- a/fs/xfs/libxfs/xfs_format.h > > +++ b/fs/xfs/libxfs/xfs_format.h > > @@ -304,8 +304,8 @@ typedef enum { > > #define XFS_SB_ICOUNT XFS_SB_MVAL(ICOUNT) > > #define XFS_SB_IFREE XFS_SB_MVAL(IFREE) > > #define XFS_SB_FDBLOCKS XFS_SB_MVAL(FDBLOCKS) > > -#define XFS_SB_FEATURES2 XFS_SB_MVAL(FEATURES2) > > -#define XFS_SB_BAD_FEATURES2 XFS_SB_MVAL(BAD_FEATURES2) > > +#define XFS_SB_FEATURES2 (XFS_SB_MVAL(FEATURES2) | \ > > + XFS_SB_MVAL(BAD_FEATURES2)) > > That doesn't guarantee that both fields are updated together, just > that the'll get logged and written together. The code still has to > ensure both fields are updated beforehand. I have a patch to address > this as well, and will post them soon. > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com -- Jan Kara SUSE Labs, CR _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs