| Message ID | 20200430225016.4287-11-allison.henderson@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xfs: Delay Ready Attributes | expand |
On Thu, Apr 30, 2020 at 03:50:02PM -0700, Allison Collins wrote: > This function is similar to xfs_attr_rmtval_remove, but adapted to > return EAGAIN for new transactions. We will use this later when we > introduce delayed attributes. This function will eventually replace > xfs_attr_rmtval_remove > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com> > --- Looks like the commit log needs some rewording now that this is a refactor patch. With that fixed: Reviewed-by: Brian Foster <bfoster@redhat.com> > fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++--------- > fs/xfs/libxfs/xfs_attr_remote.h | 1 + > 2 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > index 4d51969..02d1a44 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.c > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( > xfs_dablk_t lblkno; > int blkcnt; > int error = 0; > - int done = 0; > + int retval = 0; > > trace_xfs_attr_rmtval_remove(args); > > @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( > */ > 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; > + do { > + retval = __xfs_attr_rmtval_remove(args); > + if (retval && retval != EAGAIN) > + return retval; > > /* > * Close out trans and start the next one in the chain. > @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( > error = xfs_trans_roll_inode(&args->trans, args->dp); > if (error) > return error; > - } > + } while (retval == -EAGAIN); > + > 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 > + */ > +int > +__xfs_attr_rmtval_remove( > + struct xfs_da_args *args) > +{ > + int error, done; > + > + /* > + * Unmap value blocks for this attr. > + */ > + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, > + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); > + if (error) > + return error; > + > + error = xfs_defer_finish(&args->trans); > + if (error) > + return error; > + > + if (!done) > + return -EAGAIN; > + > + return error; > +} > diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h > index eff5f95..ee3337b 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.h > +++ b/fs/xfs/libxfs/xfs_attr_remote.h > @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); > int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, > xfs_buf_flags_t incore_flags); > int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); > +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); > #endif /* __XFS_ATTR_REMOTE_H__ */ > -- > 2.7.4 >
On Thu, Apr 30, 2020 at 03:50:02PM -0700, Allison Collins wrote: > This function is similar to xfs_attr_rmtval_remove, but adapted to > return EAGAIN for new transactions. We will use this later when we > introduce delayed attributes. This function will eventually replace > xfs_attr_rmtval_remove Like Brian suggested, this changelog could probably just say that we're hoisting the loop body into a separate function so that delayed attrs can manage the transaction rolling. > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com> > --- > fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++--------- > fs/xfs/libxfs/xfs_attr_remote.h | 1 + > 2 files changed, 37 insertions(+), 10 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > index 4d51969..02d1a44 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.c > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( > xfs_dablk_t lblkno; > int blkcnt; > int error = 0; > - int done = 0; > + int retval = 0; > > trace_xfs_attr_rmtval_remove(args); > > @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( > */ > 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; > + do { > + retval = __xfs_attr_rmtval_remove(args); > + if (retval && retval != EAGAIN) > + return retval; > > /* > * Close out trans and start the next one in the chain. > @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( > error = xfs_trans_roll_inode(&args->trans, args->dp); > if (error) > return error; > - } > + } while (retval == -EAGAIN); > + > 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 recall the function...? Oh. "re-call" the function. > + */ > +int > +__xfs_attr_rmtval_remove( xfs_attr_rmtval_remove_extent() ? --D > + struct xfs_da_args *args) > +{ > + int error, done; > + > + /* > + * Unmap value blocks for this attr. > + */ > + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, > + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); > + if (error) > + return error; > + > + error = xfs_defer_finish(&args->trans); > + if (error) > + return error; > + > + if (!done) > + return -EAGAIN; > + > + return error; > +} > diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h > index eff5f95..ee3337b 100644 > --- a/fs/xfs/libxfs/xfs_attr_remote.h > +++ b/fs/xfs/libxfs/xfs_attr_remote.h > @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); > int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, > xfs_buf_flags_t incore_flags); > int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); > +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); > #endif /* __XFS_ATTR_REMOTE_H__ */ > -- > 2.7.4 >
On 5/4/20 6:27 AM, Brian Foster wrote: > On Thu, Apr 30, 2020 at 03:50:02PM -0700, Allison Collins wrote: >> This function is similar to xfs_attr_rmtval_remove, but adapted to >> return EAGAIN for new transactions. We will use this later when we >> introduce delayed attributes. This function will eventually replace >> xfs_attr_rmtval_remove >> >> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >> Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com> >> --- > > Looks like the commit log needs some rewording now that this is a > refactor patch. With that fixed: Ok, maybe just an extra line like "Refactor xfs_attr_rmtval_remove to add helper function __xfs_attr_rmtval_remove" ? > > Reviewed-by: Brian Foster <bfoster@redhat.com> Alrighty, thank you! Allison > >> fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++--------- >> fs/xfs/libxfs/xfs_attr_remote.h | 1 + >> 2 files changed, 37 insertions(+), 10 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c >> index 4d51969..02d1a44 100644 >> --- a/fs/xfs/libxfs/xfs_attr_remote.c >> +++ b/fs/xfs/libxfs/xfs_attr_remote.c >> @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( >> xfs_dablk_t lblkno; >> int blkcnt; >> int error = 0; >> - int done = 0; >> + int retval = 0; >> >> trace_xfs_attr_rmtval_remove(args); >> >> @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( >> */ >> 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; >> + do { >> + retval = __xfs_attr_rmtval_remove(args); >> + if (retval && retval != EAGAIN) >> + return retval; >> >> /* >> * Close out trans and start the next one in the chain. >> @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( >> error = xfs_trans_roll_inode(&args->trans, args->dp); >> if (error) >> return error; >> - } >> + } while (retval == -EAGAIN); >> + >> 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 >> + */ >> +int >> +__xfs_attr_rmtval_remove( >> + struct xfs_da_args *args) >> +{ >> + int error, done; >> + >> + /* >> + * Unmap value blocks for this attr. >> + */ >> + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, >> + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); >> + if (error) >> + return error; >> + >> + error = xfs_defer_finish(&args->trans); >> + if (error) >> + return error; >> + >> + if (!done) >> + return -EAGAIN; >> + >> + return error; >> +} >> diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h >> index eff5f95..ee3337b 100644 >> --- a/fs/xfs/libxfs/xfs_attr_remote.h >> +++ b/fs/xfs/libxfs/xfs_attr_remote.h >> @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); >> int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, >> xfs_buf_flags_t incore_flags); >> int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); >> +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); >> #endif /* __XFS_ATTR_REMOTE_H__ */ >> -- >> 2.7.4 >> >
On 5/4/20 10:41 AM, Darrick J. Wong wrote: > On Thu, Apr 30, 2020 at 03:50:02PM -0700, Allison Collins wrote: >> This function is similar to xfs_attr_rmtval_remove, but adapted to >> return EAGAIN for new transactions. We will use this later when we >> introduce delayed attributes. This function will eventually replace >> xfs_attr_rmtval_remove > > Like Brian suggested, this changelog could probably just say that we're > hoisting the loop body into a separate function so that delayed attrs > can manage the transaction rolling. > >> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >> Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com> >> --- >> fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++--------- >> fs/xfs/libxfs/xfs_attr_remote.h | 1 + >> 2 files changed, 37 insertions(+), 10 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c >> index 4d51969..02d1a44 100644 >> --- a/fs/xfs/libxfs/xfs_attr_remote.c >> +++ b/fs/xfs/libxfs/xfs_attr_remote.c >> @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( >> xfs_dablk_t lblkno; >> int blkcnt; >> int error = 0; >> - int done = 0; >> + int retval = 0; >> >> trace_xfs_attr_rmtval_remove(args); >> >> @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( >> */ >> 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; >> + do { >> + retval = __xfs_attr_rmtval_remove(args); >> + if (retval && retval != EAGAIN) >> + return retval; >> >> /* >> * Close out trans and start the next one in the chain. >> @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( >> error = xfs_trans_roll_inode(&args->trans, args->dp); >> if (error) >> return error; >> - } >> + } while (retval == -EAGAIN); >> + >> 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 > > recall the function...? > > Oh. "re-call" the function. Will fix > >> + */ >> +int >> +__xfs_attr_rmtval_remove( > > xfs_attr_rmtval_remove_extent() ? Well, this would be the third rename for this function though. Initially I think we named it xfs_attr_rmtval_unmap, and then later we changed it to __xfs_attr_rmtval_remove. Ultimatly, it really doesn't matter what the name is, because it's going to get renamed at the end of the set anyway. Eventually it replaces the calling function and becomes xfs_attr_rmtval_remove. So the name it gets here is just sort of a transient name, and it doesnt effect much in the greater scheme of things. If people really feel strongly about it though, it wont hurt much to change it again. I do try to point out the history of it though to avoid too much churn. :-) Allison > > --D > >> + struct xfs_da_args *args) >> +{ >> + int error, done; >> + >> + /* >> + * Unmap value blocks for this attr. >> + */ >> + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, >> + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); >> + if (error) >> + return error; >> + >> + error = xfs_defer_finish(&args->trans); >> + if (error) >> + return error; >> + >> + if (!done) >> + return -EAGAIN; >> + >> + return error; >> +} >> diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h >> index eff5f95..ee3337b 100644 >> --- a/fs/xfs/libxfs/xfs_attr_remote.h >> +++ b/fs/xfs/libxfs/xfs_attr_remote.h >> @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); >> int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, >> xfs_buf_flags_t incore_flags); >> int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); >> +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); >> #endif /* __XFS_ATTR_REMOTE_H__ */ >> -- >> 2.7.4 >>
On Mon, May 04, 2020 at 03:53:25PM -0700, Allison Collins wrote: > > > On 5/4/20 10:41 AM, Darrick J. Wong wrote: > > On Thu, Apr 30, 2020 at 03:50:02PM -0700, Allison Collins wrote: > > > This function is similar to xfs_attr_rmtval_remove, but adapted to > > > return EAGAIN for new transactions. We will use this later when we > > > introduce delayed attributes. This function will eventually replace > > > xfs_attr_rmtval_remove > > > > Like Brian suggested, this changelog could probably just say that we're > > hoisting the loop body into a separate function so that delayed attrs > > can manage the transaction rolling. > > > > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > > > Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com> > > > --- > > > fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++--------- > > > fs/xfs/libxfs/xfs_attr_remote.h | 1 + > > > 2 files changed, 37 insertions(+), 10 deletions(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > > > index 4d51969..02d1a44 100644 > > > --- a/fs/xfs/libxfs/xfs_attr_remote.c > > > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > > > @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( > > > xfs_dablk_t lblkno; > > > int blkcnt; > > > int error = 0; > > > - int done = 0; > > > + int retval = 0; > > > trace_xfs_attr_rmtval_remove(args); > > > @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( > > > */ > > > 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; > > > + do { > > > + retval = __xfs_attr_rmtval_remove(args); > > > + if (retval && retval != EAGAIN) > > > + return retval; > > > /* > > > * Close out trans and start the next one in the chain. > > > @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( > > > error = xfs_trans_roll_inode(&args->trans, args->dp); > > > if (error) > > > return error; > > > - } > > > + } while (retval == -EAGAIN); > > > + > > > 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 > > > > recall the function...? > > > > Oh. "re-call" the function. > Will fix > > > > > > + */ > > > +int > > > +__xfs_attr_rmtval_remove( > > > > xfs_attr_rmtval_remove_extent() ? > Well, this would be the third rename for this function though. Initially I > think we named it xfs_attr_rmtval_unmap, and then later we changed it to > __xfs_attr_rmtval_remove. Ultimatly, it really doesn't matter what the name > is, because it's going to get renamed at the end of the set anyway. > Eventually it replaces the calling function and becomes > xfs_attr_rmtval_remove. > > So the name it gets here is just sort of a transient name, and it doesnt > effect much in the greater scheme of things. If people really feel strongly > about it though, it wont hurt much to change it again. I do try to point > out the history of it though to avoid too much churn. :-) Heh, ok. /I/ don't care that much. :) --D > Allison > > > > > --D > > > > > + struct xfs_da_args *args) > > > +{ > > > + int error, done; > > > + > > > + /* > > > + * Unmap value blocks for this attr. > > > + */ > > > + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, > > > + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); > > > + if (error) > > > + return error; > > > + > > > + error = xfs_defer_finish(&args->trans); > > > + if (error) > > > + return error; > > > + > > > + if (!done) > > > + return -EAGAIN; > > > + > > > + return error; > > > +} > > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h > > > index eff5f95..ee3337b 100644 > > > --- a/fs/xfs/libxfs/xfs_attr_remote.h > > > +++ b/fs/xfs/libxfs/xfs_attr_remote.h > > > @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); > > > int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, > > > xfs_buf_flags_t incore_flags); > > > int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); > > > +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); > > > #endif /* __XFS_ATTR_REMOTE_H__ */ > > > -- > > > 2.7.4 > > >
On Mon, May 04, 2020 at 02:36:39PM -0700, Allison Collins wrote: > > > On 5/4/20 6:27 AM, Brian Foster wrote: > > On Thu, Apr 30, 2020 at 03:50:02PM -0700, Allison Collins wrote: > > > This function is similar to xfs_attr_rmtval_remove, but adapted to > > > return EAGAIN for new transactions. We will use this later when we > > > introduce delayed attributes. This function will eventually replace > > > xfs_attr_rmtval_remove > > > > > > Signed-off-by: Allison Collins <allison.henderson@oracle.com> > > > Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com> > > > --- > > > > Looks like the commit log needs some rewording now that this is a > > refactor patch. With that fixed: > Ok, maybe just an extra line like "Refactor xfs_attr_rmtval_remove to add > helper function __xfs_attr_rmtval_remove" ? > I'd update the first sentence to say something like that instead of how the function is similar to xfs_attr_rmtval_remove(). Brian > > > > Reviewed-by: Brian Foster <bfoster@redhat.com> > Alrighty, thank you! > > Allison > > > > > > fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++--------- > > > fs/xfs/libxfs/xfs_attr_remote.h | 1 + > > > 2 files changed, 37 insertions(+), 10 deletions(-) > > > > > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c > > > index 4d51969..02d1a44 100644 > > > --- a/fs/xfs/libxfs/xfs_attr_remote.c > > > +++ b/fs/xfs/libxfs/xfs_attr_remote.c > > > @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( > > > xfs_dablk_t lblkno; > > > int blkcnt; > > > int error = 0; > > > - int done = 0; > > > + int retval = 0; > > > trace_xfs_attr_rmtval_remove(args); > > > @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( > > > */ > > > 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; > > > + do { > > > + retval = __xfs_attr_rmtval_remove(args); > > > + if (retval && retval != EAGAIN) > > > + return retval; > > > /* > > > * Close out trans and start the next one in the chain. > > > @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( > > > error = xfs_trans_roll_inode(&args->trans, args->dp); > > > if (error) > > > return error; > > > - } > > > + } while (retval == -EAGAIN); > > > + > > > 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 > > > + */ > > > +int > > > +__xfs_attr_rmtval_remove( > > > + struct xfs_da_args *args) > > > +{ > > > + int error, done; > > > + > > > + /* > > > + * Unmap value blocks for this attr. > > > + */ > > > + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, > > > + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); > > > + if (error) > > > + return error; > > > + > > > + error = xfs_defer_finish(&args->trans); > > > + if (error) > > > + return error; > > > + > > > + if (!done) > > > + return -EAGAIN; > > > + > > > + return error; > > > +} > > > diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h > > > index eff5f95..ee3337b 100644 > > > --- a/fs/xfs/libxfs/xfs_attr_remote.h > > > +++ b/fs/xfs/libxfs/xfs_attr_remote.h > > > @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); > > > int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, > > > xfs_buf_flags_t incore_flags); > > > int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); > > > +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); > > > #endif /* __XFS_ATTR_REMOTE_H__ */ > > > -- > > > 2.7.4 > > > > > >
On 5/5/20 5:03 AM, Brian Foster wrote: > On Mon, May 04, 2020 at 02:36:39PM -0700, Allison Collins wrote: >> >> >> On 5/4/20 6:27 AM, Brian Foster wrote: >>> On Thu, Apr 30, 2020 at 03:50:02PM -0700, Allison Collins wrote: >>>> This function is similar to xfs_attr_rmtval_remove, but adapted to >>>> return EAGAIN for new transactions. We will use this later when we >>>> introduce delayed attributes. This function will eventually replace >>>> xfs_attr_rmtval_remove >>>> >>>> Signed-off-by: Allison Collins <allison.henderson@oracle.com> >>>> Reviewed-by: Chandan Rajendra <chandanrlinux@gmail.com> >>>> --- >>> >>> Looks like the commit log needs some rewording now that this is a >>> refactor patch. With that fixed: >> Ok, maybe just an extra line like "Refactor xfs_attr_rmtval_remove to add >> helper function __xfs_attr_rmtval_remove" ? >> > > I'd update the first sentence to say something like that instead of how > the function is similar to xfs_attr_rmtval_remove(). > > Brian Ok then, will update. Allison > >>> >>> Reviewed-by: Brian Foster <bfoster@redhat.com> >> Alrighty, thank you! >> >> Allison >> >>> >>>> fs/xfs/libxfs/xfs_attr_remote.c | 46 ++++++++++++++++++++++++++++++++--------- >>>> fs/xfs/libxfs/xfs_attr_remote.h | 1 + >>>> 2 files changed, 37 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c >>>> index 4d51969..02d1a44 100644 >>>> --- a/fs/xfs/libxfs/xfs_attr_remote.c >>>> +++ b/fs/xfs/libxfs/xfs_attr_remote.c >>>> @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( >>>> xfs_dablk_t lblkno; >>>> int blkcnt; >>>> int error = 0; >>>> - int done = 0; >>>> + int retval = 0; >>>> trace_xfs_attr_rmtval_remove(args); >>>> @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( >>>> */ >>>> 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; >>>> + do { >>>> + retval = __xfs_attr_rmtval_remove(args); >>>> + if (retval && retval != EAGAIN) >>>> + return retval; >>>> /* >>>> * Close out trans and start the next one in the chain. >>>> @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( >>>> error = xfs_trans_roll_inode(&args->trans, args->dp); >>>> if (error) >>>> return error; >>>> - } >>>> + } while (retval == -EAGAIN); >>>> + >>>> 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 >>>> + */ >>>> +int >>>> +__xfs_attr_rmtval_remove( >>>> + struct xfs_da_args *args) >>>> +{ >>>> + int error, done; >>>> + >>>> + /* >>>> + * Unmap value blocks for this attr. >>>> + */ >>>> + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, >>>> + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); >>>> + if (error) >>>> + return error; >>>> + >>>> + error = xfs_defer_finish(&args->trans); >>>> + if (error) >>>> + return error; >>>> + >>>> + if (!done) >>>> + return -EAGAIN; >>>> + >>>> + return error; >>>> +} >>>> diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h >>>> index eff5f95..ee3337b 100644 >>>> --- a/fs/xfs/libxfs/xfs_attr_remote.h >>>> +++ b/fs/xfs/libxfs/xfs_attr_remote.h >>>> @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); >>>> int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, >>>> xfs_buf_flags_t incore_flags); >>>> int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); >>>> +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); >>>> #endif /* __XFS_ATTR_REMOTE_H__ */ >>>> -- >>>> 2.7.4 >>>> >>> >> >
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c index 4d51969..02d1a44 100644 --- a/fs/xfs/libxfs/xfs_attr_remote.c +++ b/fs/xfs/libxfs/xfs_attr_remote.c @@ -681,7 +681,7 @@ xfs_attr_rmtval_remove( xfs_dablk_t lblkno; int blkcnt; int error = 0; - int done = 0; + int retval = 0; trace_xfs_attr_rmtval_remove(args); @@ -693,14 +693,10 @@ xfs_attr_rmtval_remove( */ 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; + do { + retval = __xfs_attr_rmtval_remove(args); + if (retval && retval != EAGAIN) + return retval; /* * Close out trans and start the next one in the chain. @@ -708,6 +704,36 @@ xfs_attr_rmtval_remove( error = xfs_trans_roll_inode(&args->trans, args->dp); if (error) return error; - } + } while (retval == -EAGAIN); + 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 + */ +int +__xfs_attr_rmtval_remove( + struct xfs_da_args *args) +{ + int error, done; + + /* + * Unmap value blocks for this attr. + */ + error = xfs_bunmapi(args->trans, args->dp, args->rmtblkno, + args->rmtblkcnt, XFS_BMAPI_ATTRFORK, 1, &done); + if (error) + return error; + + error = xfs_defer_finish(&args->trans); + if (error) + return error; + + if (!done) + return -EAGAIN; + + return error; +} diff --git a/fs/xfs/libxfs/xfs_attr_remote.h b/fs/xfs/libxfs/xfs_attr_remote.h index eff5f95..ee3337b 100644 --- a/fs/xfs/libxfs/xfs_attr_remote.h +++ b/fs/xfs/libxfs/xfs_attr_remote.h @@ -14,4 +14,5 @@ int xfs_attr_rmtval_remove(struct xfs_da_args *args); int xfs_attr_rmtval_stale(struct xfs_inode *ip, struct xfs_bmbt_irec *map, xfs_buf_flags_t incore_flags); int xfs_attr_rmtval_invalidate(struct xfs_da_args *args); +int __xfs_attr_rmtval_remove(struct xfs_da_args *args); #endif /* __XFS_ATTR_REMOTE_H__ */