From: Brian Foster <bfoster@redhat.com>
To: linux-xfs@vger.kernel.org
Subject: [PATCH 2/2] xfs: replace filestream item xfs_inode reference with xfs_mount
Date: Thu, 5 Apr 2018 08:11:47 -0400 [thread overview]
Message-ID: <20180405121147.60897-2-bfoster@redhat.com> (raw)
In-Reply-To: <20180405121147.60897-1-bfoster@redhat.com>
The filestreams allocator stores an xfs_fstrm_item structure in the
MRU to cache inode number to agno mappings for a particular length
of time. Each xfs_fstrm_item contains the internal MRU structure, an
inode pointer and agno value.
The inode pointer stored in the xfs_fstrm_item is not referenced,
however, which means the inode itself can be removed and reclaimed
before the MRU item is freed. If this occurs, xfs_fstrm_free_func()
can access freed or unrelated memory through xfs_fstrm_item->ip and
crash.
The obvious solution is to grab an inode reference for
xfs_fstrm_item. The filestream mechanism only actually uses the
inode pointer as a means to access the xfs_mount, however. Rather
than add unnecessary complexity, simplify the implementation to
store an xfs_mount pointer instead of the inode. This also requires
updates to the tracepoint class to provide the associated data via
parameters rather than the inode and a minor hack to peek at the MRU
key to establish the inode number at free time.
Signed-off-by: Brian Foster <bfoster@redhat.com>
---
fs/xfs/xfs_filestream.c | 12 ++++++------
fs/xfs/xfs_trace.h | 14 +++++++-------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 043ca3808ea2..585f4f25fbe5 100644
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -34,7 +34,7 @@
struct xfs_fstrm_item {
struct xfs_mru_cache_elem mru;
- struct xfs_inode *ip;
+ struct xfs_mount *mp;
xfs_agnumber_t ag; /* AG in use for this directory */
};
@@ -127,9 +127,9 @@ xfs_fstrm_free_func(
struct xfs_fstrm_item *item =
container_of(mru, struct xfs_fstrm_item, mru);
- xfs_filestream_put_ag(item->ip->i_mount, item->ag);
+ xfs_filestream_put_ag(item->mp, item->ag);
- trace_xfs_filestream_free(item->ip, item->ag);
+ trace_xfs_filestream_free(item->mp, mru->key, item->ag);
kmem_free(item);
}
@@ -165,7 +165,7 @@ xfs_filestream_pick_ag(
trylock = XFS_ALLOC_FLAG_TRYLOCK;
for (nscan = 0; 1; nscan++) {
- trace_xfs_filestream_scan(ip, ag);
+ trace_xfs_filestream_scan(mp, ip->i_ino, ag);
pag = xfs_perag_get(mp, ag);
@@ -265,7 +265,7 @@ xfs_filestream_pick_ag(
goto out_put_ag;
item->ag = *agp;
- item->ip = ip;
+ item->mp = mp;
err = xfs_mru_cache_insert(mp->m_filestream, ip->i_ino, &item->mru);
if (err) {
@@ -333,7 +333,7 @@ xfs_filestream_lookup_ag(
ag = container_of(mru, struct xfs_fstrm_item, mru)->ag;
xfs_mru_cache_done(mp->m_filestream);
- trace_xfs_filestream_lookup(ip, ag);
+ trace_xfs_filestream_lookup(mp, ip->i_ino, ag);
goto out;
}
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index a982c0b623d0..8955254b900e 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -506,8 +506,8 @@ DEFINE_BUF_ITEM_EVENT(xfs_trans_bhold_release);
DEFINE_BUF_ITEM_EVENT(xfs_trans_binval);
DECLARE_EVENT_CLASS(xfs_filestream_class,
- TP_PROTO(struct xfs_inode *ip, xfs_agnumber_t agno),
- TP_ARGS(ip, agno),
+ TP_PROTO(struct xfs_mount *mp, xfs_ino_t ino, xfs_agnumber_t agno),
+ TP_ARGS(mp, ino, agno),
TP_STRUCT__entry(
__field(dev_t, dev)
__field(xfs_ino_t, ino)
@@ -515,10 +515,10 @@ DECLARE_EVENT_CLASS(xfs_filestream_class,
__field(int, streams)
),
TP_fast_assign(
- __entry->dev = VFS_I(ip)->i_sb->s_dev;
- __entry->ino = ip->i_ino;
+ __entry->dev = mp->m_super->s_dev;
+ __entry->ino = ino;
__entry->agno = agno;
- __entry->streams = xfs_filestream_peek_ag(ip->i_mount, agno);
+ __entry->streams = xfs_filestream_peek_ag(mp, agno);
),
TP_printk("dev %d:%d ino 0x%llx agno %u streams %d",
MAJOR(__entry->dev), MINOR(__entry->dev),
@@ -528,8 +528,8 @@ DECLARE_EVENT_CLASS(xfs_filestream_class,
)
#define DEFINE_FILESTREAM_EVENT(name) \
DEFINE_EVENT(xfs_filestream_class, name, \
- TP_PROTO(struct xfs_inode *ip, xfs_agnumber_t agno), \
- TP_ARGS(ip, agno))
+ TP_PROTO(struct xfs_mount *mp, xfs_ino_t ino, xfs_agnumber_t agno), \
+ TP_ARGS(mp, ino, agno))
DEFINE_FILESTREAM_EVENT(xfs_filestream_free);
DEFINE_FILESTREAM_EVENT(xfs_filestream_lookup);
DEFINE_FILESTREAM_EVENT(xfs_filestream_scan);
--
2.13.6
next prev parent reply other threads:[~2018-04-05 12:11 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-05 12:11 [PATCH 1/2] xfs: reset xfs_inode struct on reclaim in debug mode Brian Foster
2018-04-05 12:11 ` Brian Foster [this message]
2018-04-05 17:18 ` [PATCH 2/2] xfs: replace filestream item xfs_inode reference with xfs_mount Christoph Hellwig
2018-04-05 18:13 ` Brian Foster
2018-04-06 6:59 ` Christoph Hellwig
2018-04-06 13:47 ` Brian Foster
2018-04-05 16:51 ` [PATCH 1/2] xfs: reset xfs_inode struct on reclaim in debug mode Darrick J. Wong
2018-04-05 18:12 ` Brian Foster
2018-04-06 16:16 ` Darrick J. Wong
2018-04-05 21:14 ` Dave Chinner
2018-04-06 13:28 ` Brian Foster
2018-04-06 16:13 ` Darrick J. Wong
2018-04-06 17:01 ` Brian Foster
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=20180405121147.60897-2-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.