On 26 May 2021 at 23:43, Allison Henderson wrote: > On 5/25/21 1:52 PM, Darrick J. Wong wrote: >> On Tue, May 25, 2021 at 12:55:03PM -0700, Allison Henderson wrote: >>> This ASSERT checks for the state value of RM_SHRINK in the set path. >>> Which would be invalid, and should never happen. This change is being >>> set aside from the rest of the set for further discussion >>> >>> Suggested-by: Darrick J. Wong <djwong@xxxxxxxxxx> >>> Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx> >>> --- >>> fs/xfs/libxfs/xfs_attr.c | 1 - >>> 1 file changed, 1 deletion(-) >>> >>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c >>> index 32d451b..7294a2e 100644 >>> --- a/fs/xfs/libxfs/xfs_attr.c >>> +++ b/fs/xfs/libxfs/xfs_attr.c >>> @@ -612,7 +612,6 @@ xfs_attr_set_iter( >>> error = xfs_attr_node_addname_clear_incomplete(dac); >>> break; >>> default: >>> - ASSERT(dac->dela_state != XFS_DAS_RM_SHRINK); >> >> ASSERT(0); ? >> >> AFAICT the switch statement covers all the states mentioned in the state >> diagram for attr setting, so in theory it should be impossible to land >> in this state, correct? > Yes, that's correct, so ASSERT(0); should work too. I am fine with > this change if others are. "ASSERT(0);" looks good to me as well. -- chandan