From: Brian Foster <bfoster@redhat.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 06/25] xfs: use ->t_firstblock in xattr ops
Date: Tue, 3 Jul 2018 13:23:00 -0400 [thread overview]
Message-ID: <20180703172319.24509-7-bfoster@redhat.com> (raw)
In-Reply-To: <20180703172319.24509-1-bfoster@redhat.com>
Similar to the dirops code, the xattr code uses an on-stack
firstblock variable for the various operations. This code rolls the
underlying transaction in various places, however, which means we
cannot simply replace the local firstblock vars with ->t_firstblock.
Doing so (without further changes) would invalidate the memory
pointed to by xfs_da_args.firstblock as soon as the first
transaction rolls.
To avoid this problem, remove xfs_da_args.firstblock and replace all
such accesses with ->t_firstblock at the same time. This ensures
that accesses to the current firstblock always occur through the
current transaction rather than a potentially invalid xfs_da_args
pointer.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/libxfs/xfs_attr.c | 37 ++++++++++++++++-----------------
fs/xfs/libxfs/xfs_attr_leaf.c | 2 --
fs/xfs/libxfs/xfs_attr_remote.c | 18 +++++++++-------
fs/xfs/libxfs/xfs_bmap.c | 1 -
fs/xfs/libxfs/xfs_da_btree.c | 7 +++----
fs/xfs/libxfs/xfs_da_btree.h | 1 -
fs/xfs/libxfs/xfs_dir2.c | 5 +----
7 files changed, 33 insertions(+), 38 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 8a7e2c0308c4..153d2e29f872 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -204,7 +204,6 @@ xfs_attr_set(
struct xfs_da_args args;
struct xfs_defer_ops dfops;
struct xfs_trans_res tres;
- xfs_fsblock_t firstblock;
int rsvd = (flags & ATTR_ROOT) != 0;
int error, err2, local;
@@ -219,7 +218,6 @@ xfs_attr_set(
args.value = value;
args.valuelen = valuelen;
- args.firstblock = &firstblock;
args.op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
args.total = xfs_attr_calc_size(&args, &local);
@@ -253,7 +251,7 @@ xfs_attr_set(
rsvd ? XFS_TRANS_RESERVE : 0, &args.trans);
if (error)
return error;
- xfs_defer_init(args.trans, &dfops, &firstblock);
+ xfs_defer_init(args.trans, &dfops, &args.trans->t_firstblock);
xfs_ilock(dp, XFS_ILOCK_EXCL);
error = xfs_trans_reserve_quota_nblks(args.trans, dp, args.total, 0,
@@ -392,7 +390,6 @@ xfs_attr_remove(
struct xfs_mount *mp = dp->i_mount;
struct xfs_da_args args;
struct xfs_defer_ops dfops;
- xfs_fsblock_t firstblock;
int error;
XFS_STATS_INC(mp, xs_attr_remove);
@@ -404,8 +401,6 @@ xfs_attr_remove(
if (error)
return error;
- args.firstblock = &firstblock;
-
/*
* we have no control over the attribute names that userspace passes us
* to remove, so we have to allow the name lookup prior to attribute
@@ -427,7 +422,7 @@ xfs_attr_remove(
&args.trans);
if (error)
return error;
- xfs_defer_init(args.trans, &dfops, &firstblock);
+ xfs_defer_init(args.trans, &dfops, &args.trans->t_firstblock);
xfs_ilock(dp, XFS_ILOCK_EXCL);
/*
@@ -598,7 +593,8 @@ xfs_attr_leaf_addname(
* Commit that transaction so that the node_addname() call
* can manage its own transactions.
*/
- xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_attr3_leaf_to_node(args);
if (error)
goto out_defer_cancel;
@@ -687,8 +683,8 @@ xfs_attr_leaf_addname(
* If the result is small enough, shrink it all into the inode.
*/
if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
- xfs_defer_init(NULL, args->trans->t_dfops,
- args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
if (error)
@@ -753,7 +749,8 @@ xfs_attr_leaf_removename(
* If the result is small enough, shrink it all into the inode.
*/
if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
- xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
if (error)
@@ -882,8 +879,8 @@ xfs_attr_node_addname(
*/
xfs_da_state_free(state);
state = NULL;
- xfs_defer_init(NULL, args->trans->t_dfops,
- args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_attr3_leaf_to_node(args);
if (error)
goto out_defer_cancel;
@@ -910,7 +907,8 @@ xfs_attr_node_addname(
* in the index/blkno/rmtblkno/rmtblkcnt fields and
* in the index2/blkno2/rmtblkno2/rmtblkcnt2 fields.
*/
- xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_da3_split(state);
if (error)
goto out_defer_cancel;
@@ -1008,8 +1006,8 @@ xfs_attr_node_addname(
* Check to see if the tree needs to be collapsed.
*/
if (retval && (state->path.active > 1)) {
- xfs_defer_init(NULL, args->trans->t_dfops,
- args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_da3_join(state);
if (error)
goto out_defer_cancel;
@@ -1134,7 +1132,8 @@ xfs_attr_node_removename(
* Check to see if the tree needs to be collapsed.
*/
if (retval && (state->path.active > 1)) {
- xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_da3_join(state);
if (error)
goto out_defer_cancel;
@@ -1166,8 +1165,8 @@ xfs_attr_node_removename(
goto out;
if ((forkoff = xfs_attr_shortform_allfit(bp, dp))) {
- xfs_defer_init(NULL, args->trans->t_dfops,
- args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_attr3_leaf_to_shortform(bp, args, forkoff);
/* bp is gone due to xfs_da_shrink_inode */
if (error)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index c131469db0f1..251304f3bc5d 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -802,7 +802,6 @@ xfs_attr_shortform_to_leaf(
memset((char *)&nargs, 0, sizeof(nargs));
nargs.dp = dp;
nargs.geo = args->geo;
- nargs.firstblock = args->firstblock;
nargs.total = args->total;
nargs.whichfork = XFS_ATTR_FORK;
nargs.trans = args->trans;
@@ -1005,7 +1004,6 @@ xfs_attr3_leaf_to_shortform(
memset((char *)&nargs, 0, sizeof(nargs));
nargs.geo = args->geo;
nargs.dp = dp;
- nargs.firstblock = args->firstblock;
nargs.total = args->total;
nargs.whichfork = XFS_ATTR_FORK;
nargs.trans = args->trans;
diff --git a/fs/xfs/libxfs/xfs_attr_remote.c b/fs/xfs/libxfs/xfs_attr_remote.c
index ab7c2755ad8c..205098aeb4bc 100644
--- a/fs/xfs/libxfs/xfs_attr_remote.c
+++ b/fs/xfs/libxfs/xfs_attr_remote.c
@@ -480,11 +480,13 @@ xfs_attr_rmtval_set(
* extent and then crash then the block may not contain the
* correct metadata after log recovery occurs.
*/
- xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
nmap = 1;
error = xfs_bmapi_write(args->trans, dp, (xfs_fileoff_t)lblkno,
- blkcnt, XFS_BMAPI_ATTRFORK, args->firstblock,
- args->total, &map, &nmap);
+ blkcnt, XFS_BMAPI_ATTRFORK,
+ &args->trans->t_firstblock, args->total, &map,
+ &nmap);
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, dp);
@@ -522,7 +524,8 @@ xfs_attr_rmtval_set(
ASSERT(blkcnt > 0);
- xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
nmap = 1;
error = xfs_bmapi_read(dp, (xfs_fileoff_t)lblkno,
blkcnt, &map, &nmap,
@@ -626,10 +629,11 @@ xfs_attr_rmtval_remove(
blkcnt = args->rmtblkcnt;
done = 0;
while (!done) {
- xfs_defer_init(NULL, args->trans->t_dfops, args->firstblock);
+ xfs_defer_init(args->trans, args->trans->t_dfops,
+ &args->trans->t_firstblock);
error = xfs_bunmapi(args->trans, args->dp, lblkno, blkcnt,
- XFS_BMAPI_ATTRFORK, 1, args->firstblock,
- &done);
+ XFS_BMAPI_ATTRFORK, 1,
+ &args->trans->t_firstblock, &done);
if (error)
goto out_defer_cancel;
xfs_defer_ijoin(args->trans->t_dfops, args->dp);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index fcd10b47044a..86097be47ae9 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1000,7 +1000,6 @@ xfs_bmap_add_attrfork_local(
memset(&dargs, 0, sizeof(dargs));
dargs.geo = ip->i_mount->m_dir_geo;
dargs.dp = ip;
- dargs.firstblock = &tp->t_firstblock;
dargs.total = dargs.geo->fsbcount;
dargs.whichfork = XFS_DATA_FORK;
dargs.trans = tp;
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 68a72e3d9f53..2f2be86c10dc 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -2059,10 +2059,9 @@ xfs_da_grow_inode_int(
* Try mapping it in one filesystem block.
*/
nmap = 1;
- ASSERT(args->firstblock != NULL);
error = xfs_bmapi_write(tp, dp, *bno, count,
xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA|XFS_BMAPI_CONTIG,
- args->firstblock, args->total, &map, &nmap);
+ &tp->t_firstblock, args->total, &map, &nmap);
if (error)
return error;
@@ -2084,7 +2083,7 @@ xfs_da_grow_inode_int(
c = (int)(*bno + count - b);
error = xfs_bmapi_write(tp, dp, b, c,
xfs_bmapi_aflag(w)|XFS_BMAPI_METADATA,
- args->firstblock, args->total,
+ &tp->t_firstblock, args->total,
&mapp[mapi], &nmap);
if (error)
goto out_free_map;
@@ -2394,7 +2393,7 @@ xfs_da_shrink_inode(
* the last block to the place we want to kill.
*/
error = xfs_bunmapi(tp, dp, dead_blkno, count,
- xfs_bmapi_aflag(w), 0, args->firstblock,
+ xfs_bmapi_aflag(w), 0, &tp->t_firstblock,
&done);
if (error == -ENOSPC) {
if (w != XFS_DATA_FORK)
diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
index 6b8a04f3f162..59e290ef334f 100644
--- a/fs/xfs/libxfs/xfs_da_btree.h
+++ b/fs/xfs/libxfs/xfs_da_btree.h
@@ -57,7 +57,6 @@ typedef struct xfs_da_args {
xfs_dahash_t hashval; /* hash value of name */
xfs_ino_t inumber; /* input/output inode number */
struct xfs_inode *dp; /* directory inode to manipulate */
- xfs_fsblock_t *firstblock; /* ptr to firstblock for bmap calls */
struct xfs_trans *trans; /* current trans (changes over time) */
xfs_extlen_t total; /* total blocks needed, for 1st bmap */
int whichfork; /* data or attribute fork */
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index 8a55990ca3a5..53e46fda8df4 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -271,7 +271,6 @@ xfs_dir_createname(
args->total = total;
args->whichfork = XFS_DATA_FORK;
args->trans = tp;
- args->firstblock = &tp->t_firstblock;
args->op_flags = XFS_DA_OP_ADDNAME | XFS_DA_OP_OKNOENT;
if (!inum)
args->op_flags |= XFS_DA_OP_JUSTCHECK;
@@ -437,7 +436,6 @@ xfs_dir_removename(
args->hashval = dp->i_mount->m_dirnameops->hashname(name);
args->inumber = ino;
args->dp = dp;
- args->firstblock = &tp->t_firstblock;
args->total = total;
args->whichfork = XFS_DATA_FORK;
args->trans = tp;
@@ -500,7 +498,6 @@ xfs_dir_replace(
args->hashval = dp->i_mount->m_dirnameops->hashname(name);
args->inumber = inum;
args->dp = dp;
- args->firstblock = &tp->t_firstblock;
args->total = total;
args->whichfork = XFS_DATA_FORK;
args->trans = tp;
@@ -659,7 +656,7 @@ xfs_dir2_shrink_inode(
/* Unmap the fsblock(s). */
error = xfs_bunmapi(tp, dp, da, args->geo->fsbcount, 0, 0,
- args->firstblock, &done);
+ &tp->t_firstblock, &done);
if (error) {
/*
* ENOSPC actually can happen if we're in a removename with no
--
2.17.1
next prev parent reply other threads:[~2018-07-03 17:23 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-03 17:22 [PATCH 00/25] xfs: embed firstblock in xfs_trans Brian Foster
2018-07-03 17:22 ` [PATCH 01/25] xfs: allow null firstblock in xfs_bmapi_write() when tp is null Brian Foster
2018-07-04 0:24 ` Darrick J. Wong
2018-07-08 15:26 ` Christoph Hellwig
2018-07-03 17:22 ` [PATCH 02/25] xfs: add firstblock field to xfs_trans Brian Foster
2018-07-04 0:41 ` Darrick J. Wong
2018-07-08 15:26 ` Christoph Hellwig
2018-07-03 17:22 ` [PATCH 03/25] xfs: use ->t_firstblock in dir ops Brian Foster
2018-07-04 0:42 ` Darrick J. Wong
2018-07-03 17:22 ` [PATCH 04/25] xfs: remove firstblock param from xfs " Brian Foster
2018-07-03 18:06 ` Darrick J. Wong
2018-07-03 18:15 ` Brian Foster
2018-07-03 17:22 ` [PATCH 05/25] xfs: use ->t_firstblock in attrfork add Brian Foster
2018-07-04 0:43 ` Darrick J. Wong
2018-07-03 17:23 ` Brian Foster [this message]
2018-07-04 0:45 ` [PATCH 06/25] xfs: use ->t_firstblock in xattr ops Darrick J. Wong
2018-07-03 17:23 ` [PATCH 07/25] xfs: use ->t_firstblock for all xfs_bmapi_write() callers Brian Foster
2018-07-04 0:47 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 08/25] xfs: use ->t_firstblock for all xfs_bunmapi() callers Brian Foster
2018-07-04 0:47 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 09/25] xfs: use ->t_firstblock in xfs_bmapi_remap() Brian Foster
2018-07-04 0:47 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 10/25] xfs: use ->t_firstblock in insert/collapse range Brian Foster
2018-07-04 0:48 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 11/25] xfs: remove xfs_bmapi_write() firstblock param Brian Foster
2018-07-04 0:50 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 12/25] xfs: remove xfs_bunmapi() " Brian Foster
2018-07-04 0:51 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 13/25] xfs: remove bmap insert/collapse " Brian Foster
2018-07-04 0:51 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 14/25] xfs: use ->t_firstblock in bmap extent split Brian Foster
2018-07-04 0:51 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 15/25] xfs: remove xfs_bmalloca firstblock field Brian Foster
2018-07-04 0:52 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 16/25] xfs: remove bmap extent add helper firstblock params Brian Foster
2018-07-04 0:52 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 17/25] xfs: remove bmap format helpers " Brian Foster
2018-07-04 0:53 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 18/25] xfs: remove xfs_btree_cur private firstblock field Brian Foster
2018-07-04 0:54 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 19/25] xfs: remove xfs_alloc_arg " Brian Foster
2018-07-04 0:54 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 20/25] xfs: use ->t_firstblock in dq alloc Brian Foster
2018-07-04 0:54 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 21/25] xfs: replace no-op firstblock init with ->t_firstblock Brian Foster
2018-07-04 0:54 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 22/25] xfs: use ->t_firstblock in reflink cow block cancel Brian Foster
2018-07-04 0:55 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 23/25] xfs: use ->t_firstblock in extent swap Brian Foster
2018-07-04 0:55 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 24/25] xfs: use ->t_firstblock in inode inactivate Brian Foster
2018-07-04 0:55 ` Darrick J. Wong
2018-07-03 17:23 ` [PATCH 25/25] xfs: remove xfs_defer_init() firstblock param Brian Foster
2018-07-04 1:27 ` Darrick J. Wong
2018-07-08 15:37 ` Christoph Hellwig
2018-07-08 16:34 ` Darrick J. Wong
2018-07-10 1:07 ` Brian Foster
2018-07-10 7:11 ` [PATCH 00/25] xfs: embed firstblock in xfs_trans Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180703172319.24509-7-bfoster@redhat.com \
--to=bfoster@redhat.com \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).