| Message ID | 20200223020611.1802-20-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
| Series | xfs: Delayed Ready Attrs | expand |
On Sun, Feb 23, 2020 at 4:07 AM Allison Collins <allison.henderson@oracle.com> wrote: > > xfs_attr_rmtval_remove is no longer used. Clear it out now > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > --- Patch 12/19 add a new function similar to this one called xfs_attr_rmtval_unmap() and now this function is removed. I wonder if it wouldn't have been simpler to keep the original function name and change its behavior to that of xfs_attr_rmtval_unmap(). Unless the function name change makes the logic change more clear for the future users??? > fs/xfs/libxfs/xfs_attr_remote.c | 42 ----------------------------------------- > fs/xfs/xfs_trace.h | 1 - > 2 files changed, 43 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > index a0e79db..0cc0ec1 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.c > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > @@ -734,48 +734,6 @@ xfs_attr_rmtval_invalidate( > } > > /* > - * Remove the value associated with an attribute by deleting the > - * out-of-line buffer that it is stored on. > - */ > -int > -xfs_attr_rmtval_remove( > - struct xfs_da_args *args) > -{ > - xfs_dablk_t lblkno; > - int blkcnt; > - int error = 0; > - int done = 0; > - > - trace_xfs_attr_rmtval_remove(args); > - > - error = xfs_attr_rmtval_invalidate(args); > - if (error) > - return error; > - /* > - * Keep de-allocating extents until the remote-value region is gone. > - */ > - lblkno = args->rmtblkno; > - blkcnt = args->rmtblkcnt; > - while (!done) { > - error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt, > - XFS_BMAPI_ATTRFORK, 1, &done); > - if (error) > - return error; > - error = xfs_defer_finish(&args->trans); > - if (error) > - return error; > - > - /* > - * Close out trans and start the next one in the chain. > - */ > - error = xfs_trans_roll_inode(&args->trans, args->dp); > - if (error) > - return error; > - } > - return 0; > -} > - > -/* > * Remove the value associated with an attribute by deleting the out-of-line > * buffer that it is stored on. Returns EAGAIN for the caller to refresh the > * transaction and recall the function > diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h > index 159b8af..bf9a683 100644 > --- a/fs/xfs/xfs_trace.h > +++ b/fs/xfs/xfs_trace.h > @@ -1775,7 +1775,6 @@ DEFINE_ATTR_EVENT(xfs_attr_refillstate); > > DEFINE_ATTR_EVENT(xfs_attr_rmtval_get); > DEFINE_ATTR_EVENT(xfs_attr_rmtval_set); > -DEFINE_ATTR_EVENT(xfs_attr_rmtval_remove); > > #define DEFINE_DA_EVENT(name) \ > DEFINE_EVENT(xfs_da_class, name, \ > -- > 2.7.4 >
On 2/23/20 6:54 AM, Amir Goldstein wrote: > On Sun, Feb 23, 2020 at 4:07 AM Allison Collins > <allison.henderson@oracle.com> wrote: >> >> xfs_attr_rmtval_remove is no longer used. Clear it out now >> >> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >> --- > > Patch 12/19 add a new function similar to this one called > xfs_attr_rmtval_unmap() and now this function is removed. > I wonder if it wouldn't have been simpler to keep the original function > name and change its behavior to that of xfs_attr_rmtval_unmap(). I thought about this, but then decided against it. Really what's happening is that the original function disappears across patches 13 and 14, and I wanted to keep those patches focused on the state machine rather than lumping the helpers in with it. > > Unless the function name change makes the logic change more clear > for the future users??? I think they are about equivalent myself. But the name difference helps the old function to stick around until it is completely phased out. If folks feel strongly though, please chime in :-) Thanks for the reviews! I know it's a lot! Allison > >> fs/xfs/libxfs/xfs_attr_remote.c | 42 ----------------------------------------- >> fs/xfs/xfs_trace.h | 1 - >> 2 files changed, 43 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c >> index a0e79db..0cc0ec1 100644 >> --- a/fs/xfs/libxfs/xfs_attr_remote.c >> +++ b/fs/xfs/libxfs/xfs_attr_remote.c >> @@ -734,48 +734,6 @@ xfs_attr_rmtval_invalidate( >> } >> >> /* >> - * Remove the value associated with an attribute by deleting the >> - * out-of-line buffer that it is stored on. >> - */ >> -int >> -xfs_attr_rmtval_remove( >> - struct xfs_da_args *args) >> -{ >> - xfs_dablk_t lblkno; >> - int blkcnt; >> - int error = 0; >> - int done = 0; >> - >> - trace_xfs_attr_rmtval_remove(args); >> - >> - error = xfs_attr_rmtval_invalidate(args); >> - if (error) >> - return error; >> - /* >> - * Keep de-allocating extents until the remote-value region is gone. >> - */ >> - lblkno = args->rmtblkno; >> - blkcnt = args->rmtblkcnt; >> - while (!done) { >> - error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt, >> - XFS_BMAPI_ATTRFORK, 1, &done); >> - if (error) >> - return error; >> - error = xfs_defer_finish(&args->trans); >> - if (error) >> - return error; >> - >> - /* >> - * Close out trans and start the next one in the chain. >> - */ >> - error = xfs_trans_roll_inode(&args->trans, args->dp); >> - if (error) >> - return error; >> - } >> - return 0; >> -} >> - >> -/* >> * Remove the value associated with an attribute by deleting the out-of-line >> * buffer that it is stored on. Returns EAGAIN for the caller to refresh the >> * transaction and recall the function >> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h >> index 159b8af..bf9a683 100644 >> --- a/fs/xfs/xfs_trace.h >> +++ b/fs/xfs/xfs_trace.h >> @@ -1775,7 +1775,6 @@ DEFINE_ATTR_EVENT(xfs_attr_refillstate); >> >> DEFINE_ATTR_EVENT(xfs_attr_rmtval_get); >> DEFINE_ATTR_EVENT(xfs_attr_rmtval_set); >> -DEFINE_ATTR_EVENT(xfs_attr_rmtval_remove); >> >> #define DEFINE_DA_EVENT(name) \ >> DEFINE_EVENT(xfs_da_class, name, \ >> -- >> 2.7.4 >>
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c index a0e79db..0cc0ec1 100644 --- a/fs/xfs/libxfs/xfs_attr_remote.c +++ b/fs/xfs/libxfs/xfs_attr_remote.c @@ -734,48 +734,6 @@ xfs_attr_rmtval_invalidate( } /* - * Remove the value associated with an attribute by deleting the - * out-of-line buffer that it is stored on. - */ -int -xfs_attr_rmtval_remove( - struct xfs_da_args *args) -{ - xfs_dablk_t lblkno; - int blkcnt; - int error = 0; - int done = 0; - - trace_xfs_attr_rmtval_remove(args); - - error = xfs_attr_rmtval_invalidate(args); - if (error) - return error; - /* - * Keep de-allocating extents until the remote-value region is gone. - */ - lblkno = args->rmtblkno; - blkcnt = args->rmtblkcnt; - while (!done) { - error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt, - XFS_BMAPI_ATTRFORK, 1, &done); - if (error) - return error; - error = xfs_defer_finish(&args->trans); - if (error) - return error; - - /* - * Close out trans and start the next one in the chain. - */ - error = xfs_trans_roll_inode(&args->trans, args->dp); - if (error) - return error; - } - return 0; -} - -/* * Remove the value associated with an attribute by deleting the out-of-line * buffer that it is stored on. Returns EAGAIN for the caller to refresh the * transaction and recall the function diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h index 159b8af..bf9a683 100644 --- a/fs/xfs/xfs_trace.h +++ b/fs/xfs/xfs_trace.h @@ -1775,7 +1775,6 @@ DEFINE_ATTR_EVENT(xfs_attr_refillstate); DEFINE_ATTR_EVENT(xfs_attr_rmtval_get); DEFINE_ATTR_EVENT(xfs_attr_rmtval_set); -DEFINE_ATTR_EVENT(xfs_attr_rmtval_remove); #define DEFINE_DA_EVENT(name) \ DEFINE_EVENT(xfs_da_class, name, \
xfs_attr_rmtval_remove is no longer used. Clear it out now Signed-off-by: Allison Collins <allison.henderson@oracle.com> --- fs/xfs/libxfs/xfs_attr_remote.c | 42 ----------------------------------------- fs/xfs/xfs_trace.h | 1 - 2 files changed, 43 deletions(-)