| Message ID | 20210525195504.7332-14-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Delay Ready Attributes | expand |
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@kernel.org> > Signed-off-by: Allison Henderson <allison.henderson@oracle.com> > --- > 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? --D > break; > } > out: > -- > 2.7.4 >
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@kernel.org> >> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >> --- >> 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. Allison > > --D > >> break; >> } >> out: >> -- >> 2.7.4 >>
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@kernel.org> >>> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> >>> --- >>> 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.
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); break; } out:
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@kernel.org> Signed-off-by: Allison Henderson <allison.henderson@oracle.com> --- fs/xfs/libxfs/xfs_attr.c | 1 - 1 file changed, 1 deletion(-)