* [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
@ 2019-05-16 17:43 ` Eric Sandeen
2019-05-17 20:49 ` Allison Collins
2019-05-16 17:45 ` [PATCH 2/3] libxfs: remove libxfs API #defines for unexported xfs functions Eric Sandeen
` (9 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 17:43 UTC (permalink / raw)
To: linux-xfs
The libxfs_* function namespace has gotten a bit confused and
muddled; the general idea is that functions called from userspace
utilities should use the libxfs_* namespace. In many cases
we use #defines to define xfs_* namespace to libxfs_*; in other
cases we have explicitly defined libxfs_* functions which are clear
counterparts or even clones of kernel libxfs/* functions.
For any function definitions within libxfs/* which match kernel
names, give them standard xfs_* names to further reduce differnces
between userspace and kernel libxfs/* code.
Then add #defines to libxfs_* for any functions which are needed
by utilities, as is done with other core functionality.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/include/libxfs.h b/include/libxfs.h
index 230bc3e8..ceebccdc 100644
--- a/include/libxfs.h
+++ b/include/libxfs.h
@@ -151,7 +151,7 @@ extern int libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
/* Shared utility routines */
-extern int libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
+extern int xfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
xfs_off_t, int, int);
/* XXX: this is messy and needs fixing */
diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 88b58ac3..3e7e80ea 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -139,21 +139,21 @@ typedef struct cred {
gid_t cr_gid;
} cred_t;
-extern int libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
+extern int xfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
mode_t, nlink_t, xfs_dev_t, struct cred *,
struct fsxattr *, struct xfs_inode **);
-extern void libxfs_trans_inode_alloc_buf (struct xfs_trans *,
+extern void xfs_trans_inode_alloc_buf (struct xfs_trans *,
struct xfs_buf *);
-extern void libxfs_trans_ichgtime(struct xfs_trans *,
+extern void xfs_trans_ichgtime(struct xfs_trans *,
struct xfs_inode *, int);
-extern int libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
+extern int xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
/* Inode Cache Interfaces */
-extern bool libxfs_inode_verify_forks(struct xfs_inode *ip);
-extern int libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
+extern bool xfs_inode_verify_forks(struct xfs_inode *ip);
+extern int xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
uint, struct xfs_inode **,
struct xfs_ifork_ops *);
-extern void libxfs_irele(struct xfs_inode *ip);
+extern void xfs_irele(struct xfs_inode *ip);
#endif /* __XFS_INODE_H__ */
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 10b74538..d32acc9e 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -75,46 +75,46 @@ typedef struct xfs_trans {
void xfs_trans_init(struct xfs_mount *);
int xfs_trans_roll(struct xfs_trans **);
-int libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
+int xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
uint blocks, uint rtextents, uint flags,
struct xfs_trans **tpp);
int libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
struct xfs_trans **tpp);
-int libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
-int libxfs_trans_commit(struct xfs_trans *);
-void libxfs_trans_cancel(struct xfs_trans *);
+int xfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
+int xfs_trans_commit(struct xfs_trans *);
+void xfs_trans_cancel(struct xfs_trans *);
/* cancel dfops associated with a transaction */
void xfs_defer_cancel(struct xfs_trans *);
-struct xfs_buf *libxfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
+struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
-void libxfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
-void libxfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
+void xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
+void xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
uint);
-int libxfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
-
-void libxfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
-void libxfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
-void libxfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
-void libxfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
-void libxfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
-void libxfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
+int xfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
+
+void xfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
+void xfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
+void xfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
+void xfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
+void xfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
+void xfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
uint, uint);
-bool libxfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
+bool xfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
-struct xfs_buf *libxfs_trans_get_buf_map(struct xfs_trans *tp,
+struct xfs_buf *xfs_trans_get_buf_map(struct xfs_trans *tp,
struct xfs_buftarg *btp,
struct xfs_buf_map *map, int nmaps,
uint flags);
-int libxfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
+int xfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
struct xfs_buftarg *btp,
struct xfs_buf_map *map, int nmaps,
uint flags, struct xfs_buf **bpp,
const struct xfs_buf_ops *ops);
static inline struct xfs_buf *
-libxfs_trans_get_buf(
+xfs_trans_get_buf(
struct xfs_trans *tp,
struct xfs_buftarg *btp,
xfs_daddr_t blkno,
@@ -122,11 +122,11 @@ libxfs_trans_get_buf(
uint flags)
{
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return libxfs_trans_get_buf_map(tp, btp, &map, 1, flags);
+ return xfs_trans_get_buf_map(tp, btp, &map, 1, flags);
}
static inline int
-libxfs_trans_read_buf(
+xfs_trans_read_buf(
struct xfs_mount *mp,
struct xfs_trans *tp,
struct xfs_buftarg *btp,
@@ -137,7 +137,7 @@ libxfs_trans_read_buf(
const struct xfs_buf_ops *ops)
{
DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
- return libxfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
+ return xfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
}
#endif /* __XFS_TRANS_H__ */
diff --git a/libxfs/init.c b/libxfs/init.c
index 2f6decc8..4b402a6e 100644
--- a/libxfs/init.c
+++ b/libxfs/init.c
@@ -449,7 +449,7 @@ rtmount_init(
}
static int
-libxfs_initialize_perag(
+xfs_initialize_perag(
xfs_mount_t *mp,
xfs_agnumber_t agcount,
xfs_agnumber_t *maxagi)
@@ -790,7 +790,7 @@ libxfs_mount(
}
/*
- * libxfs_initialize_perag will allocate a perag structure for each ag.
+ * xfs_initialize_perag will allocate a perag structure for each ag.
* If agcount is corrupted and insanely high, this will OOM the box.
* If the agount seems (arbitrarily) high, try to read what would be
* the last AG, and if that fails for a relatively high agcount, just
@@ -812,7 +812,7 @@ libxfs_mount(
libxfs_putbuf(bp);
}
- error = libxfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
+ error = xfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
if (error) {
fprintf(stderr, _("%s: perag init failed\n"),
progname);
@@ -826,9 +826,9 @@ void
libxfs_rtmount_destroy(xfs_mount_t *mp)
{
if (mp->m_rsumip)
- libxfs_irele(mp->m_rsumip);
+ xfs_irele(mp->m_rsumip);
if (mp->m_rbmip)
- libxfs_irele(mp->m_rbmip);
+ xfs_irele(mp->m_rbmip);
mp->m_rsumip = mp->m_rbmip = NULL;
}
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 1150ec93..2b8ac5ab 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -17,6 +17,7 @@
#define xfs_highbit64 libxfs_highbit64
#define xfs_trans_alloc libxfs_trans_alloc
+#define xfs_trans_alloc_rollable libxfs_trans_alloc_rollable
#define xfs_trans_alloc_empty libxfs_trans_alloc_empty
#define xfs_trans_add_item libxfs_trans_add_item
#define xfs_trans_bhold libxfs_trans_bhold
@@ -90,6 +91,9 @@
#define xfs_idestroy_fork libxfs_idestroy_fork
#define xfs_inode_validate_extsize libxfs_inode_validate_extsize
#define xfs_inode_validate_cowextsize libxfs_inode_validate_cowextsize
+#define xfs_inode_alloc libxfs_inode_alloc
+#define xfs_iflush_int libxfs_iflush_int
+#define xfs_alloc_file_space libxfs_alloc_file_space
#define xfs_rmap_alloc libxfs_rmap_alloc
#define xfs_rmap_query_range libxfs_rmap_query_range
@@ -144,4 +148,9 @@
#define xfs_fs_geometry libxfs_fs_geometry
#define xfs_init_local_fork libxfs_init_local_fork
+#define xfs_getsb libxfs_getsb
+#define xfs_irele libxfs_irele
+#define xfs_iget libxfs_iget
+#define xfs_inode_verify_forks libxfs_inode_verify_forks
+
#endif /* __LIBXFS_API_DEFS_H__ */
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 79ffd470..8fa2c2c5 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -177,7 +177,7 @@ extern void libxfs_putbuf (xfs_buf_t *);
extern void libxfs_readbuf_verify(struct xfs_buf *bp,
const struct xfs_buf_ops *ops);
-extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
+extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
extern void libxfs_bcache_purge(void);
extern void libxfs_bcache_free(void);
extern void libxfs_bcache_flush(void);
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index d668a157..157a99d6 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -559,7 +559,7 @@ typedef int (*xfs_rtalloc_query_range_fn)(
struct xfs_rtalloc_rec *rec,
void *priv);
-int libxfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
+int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
xfs_off_t count_fsb);
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index e3ff5842..8d6f958a 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -476,7 +476,7 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
xfs_buf_t *
-libxfs_getsb(xfs_mount_t *mp, int flags)
+xfs_getsb(xfs_mount_t *mp, int flags)
{
return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
@@ -1374,7 +1374,7 @@ extern kmem_zone_t *xfs_ili_zone;
* make sure they're not corrupt.
*/
bool
-libxfs_inode_verify_forks(
+xfs_inode_verify_forks(
struct xfs_inode *ip)
{
struct xfs_ifork *ifp;
@@ -1403,7 +1403,7 @@ libxfs_inode_verify_forks(
}
int
-libxfs_iget(
+xfs_iget(
struct xfs_mount *mp,
struct xfs_trans *tp,
xfs_ino_t ino,
@@ -1428,8 +1428,8 @@ libxfs_iget(
}
ip->i_fork_ops = ifork_ops;
- if (!libxfs_inode_verify_forks(ip)) {
- libxfs_irele(ip);
+ if (!xfs_inode_verify_forks(ip)) {
+ xfs_irele(ip);
return -EFSCORRUPTED;
}
@@ -1446,26 +1446,26 @@ libxfs_iget(
}
static void
-libxfs_idestroy(xfs_inode_t *ip)
+xfs_idestroy(xfs_inode_t *ip)
{
switch (VFS_I(ip)->i_mode & S_IFMT) {
case S_IFREG:
case S_IFDIR:
case S_IFLNK:
- libxfs_idestroy_fork(ip, XFS_DATA_FORK);
+ xfs_idestroy_fork(ip, XFS_DATA_FORK);
break;
}
if (ip->i_afp)
- libxfs_idestroy_fork(ip, XFS_ATTR_FORK);
+ xfs_idestroy_fork(ip, XFS_ATTR_FORK);
if (ip->i_cowfp)
xfs_idestroy_fork(ip, XFS_COW_FORK);
}
void
-libxfs_irele(
+xfs_irele(
struct xfs_inode *ip)
{
ASSERT(ip->i_itemp == NULL);
- libxfs_idestroy(ip);
+ xfs_idestroy(ip);
kmem_zone_free(xfs_inode_zone, ip);
}
diff --git a/libxfs/trans.c b/libxfs/trans.c
index cb15552c..5ef0c055 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -36,7 +36,7 @@ kmem_zone_t *xfs_trans_zone;
* in the mount structure.
*/
void
-libxfs_trans_init(
+xfs_trans_init(
struct xfs_mount *mp)
{
xfs_trans_resv_calc(mp, &mp->m_resv);
@@ -46,7 +46,7 @@ libxfs_trans_init(
* Add the given log item to the transaction's list of log items.
*/
void
-libxfs_trans_add_item(
+xfs_trans_add_item(
struct xfs_trans *tp,
struct xfs_log_item *lip)
{
@@ -62,7 +62,7 @@ libxfs_trans_add_item(
* Unlink and free the given descriptor.
*/
void
-libxfs_trans_del_item(
+xfs_trans_del_item(
struct xfs_log_item *lip)
{
clear_bit(XFS_LI_DIRTY, &lip->li_flags);
@@ -77,7 +77,7 @@ libxfs_trans_del_item(
* chunk we've been working on and get a new transaction to continue.
*/
int
-libxfs_trans_roll(
+xfs_trans_roll(
struct xfs_trans **tpp)
{
struct xfs_trans *trans = *tpp;
@@ -245,7 +245,7 @@ undo_blocks:
}
int
-libxfs_trans_alloc(
+xfs_trans_alloc(
struct xfs_mount *mp,
struct xfs_trans_res *resp,
unsigned int blocks,
@@ -289,7 +289,7 @@ libxfs_trans_alloc(
* without any dirty data.
*/
int
-libxfs_trans_alloc_empty(
+xfs_trans_alloc_empty(
struct xfs_mount *mp,
struct xfs_trans **tpp)
{
@@ -304,17 +304,17 @@ libxfs_trans_alloc_empty(
* permanent log reservation flag to avoid blowing asserts.
*/
int
-libxfs_trans_alloc_rollable(
+xfs_trans_alloc_rollable(
struct xfs_mount *mp,
unsigned int blocks,
struct xfs_trans **tpp)
{
- return libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
+ return xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
0, 0, tpp);
}
void
-libxfs_trans_cancel(
+xfs_trans_cancel(
struct xfs_trans *tp)
{
#ifdef XACT_DEBUG
@@ -337,7 +337,7 @@ out:
}
void
-libxfs_trans_ijoin(
+xfs_trans_ijoin(
xfs_trans_t *tp,
xfs_inode_t *ip,
uint lock_flags)
@@ -360,7 +360,7 @@ libxfs_trans_ijoin(
}
void
-libxfs_trans_inode_alloc_buf(
+xfs_trans_inode_alloc_buf(
xfs_trans_t *tp,
xfs_buf_t *bp)
{
@@ -407,7 +407,7 @@ xfs_trans_log_inode(
}
int
-libxfs_trans_roll_inode(
+xfs_trans_roll_inode(
struct xfs_trans **tpp,
struct xfs_inode *ip)
{
@@ -425,7 +425,7 @@ libxfs_trans_roll_inode(
* Mark a buffer dirty in the transaction.
*/
void
-libxfs_trans_dirty_buf(
+xfs_trans_dirty_buf(
struct xfs_trans *tp,
struct xfs_buf *bp)
{
@@ -451,7 +451,7 @@ libxfs_trans_dirty_buf(
* value of b_blkno.
*/
void
-libxfs_trans_log_buf(
+xfs_trans_log_buf(
struct xfs_trans *tp,
struct xfs_buf *bp,
uint first,
@@ -473,7 +473,7 @@ libxfs_trans_log_buf(
* If the buffer is already dirty, trigger the "already logged" return condition.
*/
bool
-libxfs_trans_ordered_buf(
+xfs_trans_ordered_buf(
struct xfs_trans *tp,
struct xfs_buf *bp)
{
@@ -481,7 +481,7 @@ libxfs_trans_ordered_buf(
bool ret;
ret = test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
- libxfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
+ xfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
return ret;
}
@@ -496,7 +496,7 @@ xfs_buf_item_put(
}
void
-libxfs_trans_brelse(
+xfs_trans_brelse(
xfs_trans_t *tp,
xfs_buf_t *bp)
{
@@ -531,7 +531,7 @@ libxfs_trans_brelse(
}
void
-libxfs_trans_binval(
+xfs_trans_binval(
xfs_trans_t *tp,
xfs_buf_t *bp)
{
@@ -556,7 +556,7 @@ libxfs_trans_binval(
}
void
-libxfs_trans_bjoin(
+xfs_trans_bjoin(
xfs_trans_t *tp,
xfs_buf_t *bp)
{
@@ -574,7 +574,7 @@ libxfs_trans_bjoin(
}
void
-libxfs_trans_bhold(
+xfs_trans_bhold(
xfs_trans_t *tp,
xfs_buf_t *bp)
{
@@ -590,7 +590,7 @@ libxfs_trans_bhold(
}
xfs_buf_t *
-libxfs_trans_get_buf_map(
+xfs_trans_get_buf_map(
xfs_trans_t *tp,
struct xfs_buftarg *btp,
struct xfs_buf_map *map,
@@ -619,14 +619,14 @@ libxfs_trans_get_buf_map(
fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
#endif
- libxfs_trans_bjoin(tp, bp);
+ xfs_trans_bjoin(tp, bp);
bip = bp->b_log_item;
bip->bli_recur = 0;
return bp;
}
xfs_buf_t *
-libxfs_trans_getsb(
+xfs_trans_getsb(
xfs_trans_t *tp,
xfs_mount_t *mp,
int flags)
@@ -637,7 +637,7 @@ libxfs_trans_getsb(
DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
if (tp == NULL)
- return libxfs_getsb(mp, flags);
+ return xfs_getsb(mp, flags);
bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
if (bp != NULL) {
@@ -648,19 +648,19 @@ libxfs_trans_getsb(
return bp;
}
- bp = libxfs_getsb(mp, flags);
+ bp = xfs_getsb(mp, flags);
#ifdef XACT_DEBUG
fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
#endif
- libxfs_trans_bjoin(tp, bp);
+ xfs_trans_bjoin(tp, bp);
bip = bp->b_log_item;
bip->bli_recur = 0;
return bp;
}
int
-libxfs_trans_read_buf_map(
+xfs_trans_read_buf_map(
xfs_mount_t *mp,
xfs_trans_t *tp,
struct xfs_buftarg *btp,
@@ -728,7 +728,7 @@ out_relse:
* Originally derived from xfs_trans_mod_sb().
*/
void
-libxfs_trans_mod_sb(
+xfs_trans_mod_sb(
xfs_trans_t *tp,
uint field,
long delta)
@@ -814,7 +814,7 @@ inode_item_done(
* of whether the flush succeed or not. If we fail the flush, make sure
* we still release the buffer reference we currently hold.
*/
- error = libxfs_iflush_int(ip, bp);
+ error = xfs_iflush_int(ip, bp);
bp->b_transp = NULL; /* remove xact ptr */
if (error) {
@@ -989,7 +989,7 @@ out_unreserve:
}
int
-libxfs_trans_commit(
+xfs_trans_commit(
struct xfs_trans *tp)
{
return __xfs_trans_commit(tp, false);
diff --git a/libxfs/util.c b/libxfs/util.c
index 8c9954f6..4901123a 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -143,7 +143,7 @@ xfs_log_calc_unit_res(
* where it's no longer worth the hassle of maintaining common code.
*/
void
-libxfs_trans_ichgtime(
+xfs_trans_ichgtime(
struct xfs_trans *tp,
struct xfs_inode *ip,
int flags)
@@ -232,7 +232,7 @@ xfs_flags2diflags2(
* where it's no longer worth the hassle of maintaining common code.
*/
static int
-libxfs_ialloc(
+xfs_ialloc(
xfs_trans_t *tp,
xfs_inode_t *pip,
mode_t mode,
@@ -262,7 +262,7 @@ libxfs_ialloc(
}
ASSERT(*ialloc_context == NULL);
- error = libxfs_iget(tp->t_mountp, tp, ino, 0, &ip,
+ error = xfs_iget(tp->t_mountp, tp, ino, 0, &ip,
&xfs_default_ifork_ops);
if (error != 0)
return error;
@@ -388,7 +388,7 @@ libxfs_ialloc(
* Originally based on xfs_iflush_int() from xfs_inode.c in the kernel.
*/
int
-libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
+xfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
{
xfs_inode_log_item_t *iip;
xfs_dinode_t *dip;
@@ -421,7 +421,7 @@ libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
VFS_I(ip)->i_version++;
/* Check the inline fork data before we write out. */
- if (!libxfs_inode_verify_forks(ip))
+ if (!xfs_inode_verify_forks(ip))
return -EFSCORRUPTED;
/*
@@ -467,10 +467,9 @@ libxfs_mod_incore_sb(
/*
* This routine allocates disk space for the given file.
- * Originally derived from xfs_alloc_file_space().
*/
int
-libxfs_alloc_file_space(
+xfs_alloc_file_space(
xfs_inode_t *ip,
xfs_off_t offset,
xfs_off_t len,
@@ -547,14 +546,14 @@ error0: /* Cancel bmap, cancel trans */
}
/*
- * Wrapper around call to libxfs_ialloc. Takes care of committing and
+ * Wrapper around call to xfs_ialloc. Takes care of committing and
* allocating a new transaction as needed.
*
* Originally there were two copies of this code - one in mkfs, the
* other in repair - now there is just the one.
*/
int
-libxfs_inode_alloc(
+xfs_inode_alloc(
xfs_trans_t **tp,
xfs_inode_t *pip,
mode_t mode,
@@ -569,7 +568,7 @@ libxfs_inode_alloc(
int error;
ialloc_context = (xfs_buf_t *)0;
- error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
+ error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
&ialloc_context, &ip);
if (error) {
*ipp = NULL;
@@ -591,7 +590,7 @@ libxfs_inode_alloc(
exit(1);
}
xfs_trans_bjoin(*tp, ialloc_context);
- error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
+ error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
fsx, &ialloc_context, &ip);
if (!ip)
error = -ENOSPC;
@@ -712,7 +711,7 @@ xfs_fsb_to_db(struct xfs_inode *ip, xfs_fsblock_t fsb)
}
int
-libxfs_zero_extent(
+xfs_zero_extent(
struct xfs_inode *ip,
xfs_fsblock_t start_fsb,
xfs_off_t count_fsb)
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_
2019-05-16 17:43 ` [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_ Eric Sandeen
@ 2019-05-17 20:49 ` Allison Collins
2019-05-17 21:00 ` Eric Sandeen
0 siblings, 1 reply; 29+ messages in thread
From: Allison Collins @ 2019-05-17 20:49 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
On 5/16/19 10:43 AM, Eric Sandeen wrote:
> The libxfs_* function namespace has gotten a bit confused and
> muddled; the general idea is that functions called from userspace
> utilities should use the libxfs_* namespace. In many cases
> we use #defines to define xfs_* namespace to libxfs_*; in other
> cases we have explicitly defined libxfs_* functions which are clear
> counterparts or even clones of kernel libxfs/* functions.
>
> For any function definitions within libxfs/* which match kernel
> names, give them standard xfs_* names to further reduce differnces
> between userspace and kernel libxfs/* code.
>
> Then add #defines to libxfs_* for any functions which are needed
> by utilities, as is done with other core functionality.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> diff --git a/include/libxfs.h b/include/libxfs.h
> index 230bc3e8..ceebccdc 100644
> --- a/include/libxfs.h
> +++ b/include/libxfs.h
> @@ -151,7 +151,7 @@ extern int libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
>
> /* Shared utility routines */
>
> -extern int libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
> +extern int xfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
> xfs_off_t, int, int);
>
> /* XXX: this is messy and needs fixing */
> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
> index 88b58ac3..3e7e80ea 100644
> --- a/include/xfs_inode.h
> +++ b/include/xfs_inode.h
> @@ -139,21 +139,21 @@ typedef struct cred {
> gid_t cr_gid;
> } cred_t;
>
> -extern int libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
> +extern int xfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
> mode_t, nlink_t, xfs_dev_t, struct cred *,
> struct fsxattr *, struct xfs_inode **);
> -extern void libxfs_trans_inode_alloc_buf (struct xfs_trans *,
> +extern void xfs_trans_inode_alloc_buf (struct xfs_trans *,
> struct xfs_buf *);
>
> -extern void libxfs_trans_ichgtime(struct xfs_trans *,
> +extern void xfs_trans_ichgtime(struct xfs_trans *,
> struct xfs_inode *, int);
> -extern int libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
> +extern int xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>
> /* Inode Cache Interfaces */
> -extern bool libxfs_inode_verify_forks(struct xfs_inode *ip);
> -extern int libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
> +extern bool xfs_inode_verify_forks(struct xfs_inode *ip);
> +extern int xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
> uint, struct xfs_inode **,
> struct xfs_ifork_ops *);
> -extern void libxfs_irele(struct xfs_inode *ip);
> +extern void xfs_irele(struct xfs_inode *ip);
>
> #endif /* __XFS_INODE_H__ */
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index 10b74538..d32acc9e 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -75,46 +75,46 @@ typedef struct xfs_trans {
> void xfs_trans_init(struct xfs_mount *);
> int xfs_trans_roll(struct xfs_trans **);
>
> -int libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
> +int xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
> uint blocks, uint rtextents, uint flags,
> struct xfs_trans **tpp);
> int libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
> struct xfs_trans **tpp);
Did you mean to rename libxfs_trans_alloc_rollable too? I notice the
function name is changed later down in the patch.
I think the rest of it looks pretty straight forward though.
Allison
> -int libxfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
> -int libxfs_trans_commit(struct xfs_trans *);
> -void libxfs_trans_cancel(struct xfs_trans *);
> +int xfs_trans_alloc_empty(struct xfs_mount *mp, struct xfs_trans **tpp);
> +int xfs_trans_commit(struct xfs_trans *);
> +void xfs_trans_cancel(struct xfs_trans *);
>
> /* cancel dfops associated with a transaction */
> void xfs_defer_cancel(struct xfs_trans *);
>
> -struct xfs_buf *libxfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
> +struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
>
> -void libxfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
> -void libxfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
> +void xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
> +void xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
> uint);
> -int libxfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
> -
> -void libxfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
> -void libxfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
> -void libxfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
> -void libxfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
> -void libxfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
> -void libxfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
> +int xfs_trans_roll_inode (struct xfs_trans **, struct xfs_inode *);
> +
> +void xfs_trans_brelse(struct xfs_trans *, struct xfs_buf *);
> +void xfs_trans_binval(struct xfs_trans *, struct xfs_buf *);
> +void xfs_trans_bjoin(struct xfs_trans *, struct xfs_buf *);
> +void xfs_trans_bhold(struct xfs_trans *, struct xfs_buf *);
> +void xfs_trans_dirty_buf(struct xfs_trans *, struct xfs_buf *);
> +void xfs_trans_log_buf(struct xfs_trans *, struct xfs_buf *,
> uint, uint);
> -bool libxfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
> +bool xfs_trans_ordered_buf(xfs_trans_t *, struct xfs_buf *);
>
> -struct xfs_buf *libxfs_trans_get_buf_map(struct xfs_trans *tp,
> +struct xfs_buf *xfs_trans_get_buf_map(struct xfs_trans *tp,
> struct xfs_buftarg *btp,
> struct xfs_buf_map *map, int nmaps,
> uint flags);
>
> -int libxfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
> +int xfs_trans_read_buf_map(struct xfs_mount *mp, struct xfs_trans *tp,
> struct xfs_buftarg *btp,
> struct xfs_buf_map *map, int nmaps,
> uint flags, struct xfs_buf **bpp,
> const struct xfs_buf_ops *ops);
> static inline struct xfs_buf *
> -libxfs_trans_get_buf(
> +xfs_trans_get_buf(
> struct xfs_trans *tp,
> struct xfs_buftarg *btp,
> xfs_daddr_t blkno,
> @@ -122,11 +122,11 @@ libxfs_trans_get_buf(
> uint flags)
> {
> DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
> - return libxfs_trans_get_buf_map(tp, btp, &map, 1, flags);
> + return xfs_trans_get_buf_map(tp, btp, &map, 1, flags);
> }
>
> static inline int
> -libxfs_trans_read_buf(
> +xfs_trans_read_buf(
> struct xfs_mount *mp,
> struct xfs_trans *tp,
> struct xfs_buftarg *btp,
> @@ -137,7 +137,7 @@ libxfs_trans_read_buf(
> const struct xfs_buf_ops *ops)
> {
> DEFINE_SINGLE_BUF_MAP(map, blkno, numblks);
> - return libxfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
> + return xfs_trans_read_buf_map(mp, tp, btp, &map, 1, flags, bpp, ops);
> }
>
> #endif /* __XFS_TRANS_H__ */
> diff --git a/libxfs/init.c b/libxfs/init.c
> index 2f6decc8..4b402a6e 100644
> --- a/libxfs/init.c
> +++ b/libxfs/init.c
> @@ -449,7 +449,7 @@ rtmount_init(
> }
>
> static int
> -libxfs_initialize_perag(
> +xfs_initialize_perag(
> xfs_mount_t *mp,
> xfs_agnumber_t agcount,
> xfs_agnumber_t *maxagi)
> @@ -790,7 +790,7 @@ libxfs_mount(
> }
>
> /*
> - * libxfs_initialize_perag will allocate a perag structure for each ag.
> + * xfs_initialize_perag will allocate a perag structure for each ag.
> * If agcount is corrupted and insanely high, this will OOM the box.
> * If the agount seems (arbitrarily) high, try to read what would be
> * the last AG, and if that fails for a relatively high agcount, just
> @@ -812,7 +812,7 @@ libxfs_mount(
> libxfs_putbuf(bp);
> }
>
> - error = libxfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
> + error = xfs_initialize_perag(mp, sbp->sb_agcount, &mp->m_maxagi);
> if (error) {
> fprintf(stderr, _("%s: perag init failed\n"),
> progname);
> @@ -826,9 +826,9 @@ void
> libxfs_rtmount_destroy(xfs_mount_t *mp)
> {
> if (mp->m_rsumip)
> - libxfs_irele(mp->m_rsumip);
> + xfs_irele(mp->m_rsumip);
> if (mp->m_rbmip)
> - libxfs_irele(mp->m_rbmip);
> + xfs_irele(mp->m_rbmip);
> mp->m_rsumip = mp->m_rbmip = NULL;
> }
>
> diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
> index 1150ec93..2b8ac5ab 100644
> --- a/libxfs/libxfs_api_defs.h
> +++ b/libxfs/libxfs_api_defs.h
> @@ -17,6 +17,7 @@
> #define xfs_highbit64 libxfs_highbit64
>
> #define xfs_trans_alloc libxfs_trans_alloc
> +#define xfs_trans_alloc_rollable libxfs_trans_alloc_rollable
> #define xfs_trans_alloc_empty libxfs_trans_alloc_empty
> #define xfs_trans_add_item libxfs_trans_add_item
> #define xfs_trans_bhold libxfs_trans_bhold
> @@ -90,6 +91,9 @@
> #define xfs_idestroy_fork libxfs_idestroy_fork
> #define xfs_inode_validate_extsize libxfs_inode_validate_extsize
> #define xfs_inode_validate_cowextsize libxfs_inode_validate_cowextsize
> +#define xfs_inode_alloc libxfs_inode_alloc
> +#define xfs_iflush_int libxfs_iflush_int
> +#define xfs_alloc_file_space libxfs_alloc_file_space
>
> #define xfs_rmap_alloc libxfs_rmap_alloc
> #define xfs_rmap_query_range libxfs_rmap_query_range
> @@ -144,4 +148,9 @@
> #define xfs_fs_geometry libxfs_fs_geometry
> #define xfs_init_local_fork libxfs_init_local_fork
>
> +#define xfs_getsb libxfs_getsb
> +#define xfs_irele libxfs_irele
> +#define xfs_iget libxfs_iget
> +#define xfs_inode_verify_forks libxfs_inode_verify_forks
> +
> #endif /* __LIBXFS_API_DEFS_H__ */
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 79ffd470..8fa2c2c5 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -177,7 +177,7 @@ extern void libxfs_putbuf (xfs_buf_t *);
>
> extern void libxfs_readbuf_verify(struct xfs_buf *bp,
> const struct xfs_buf_ops *ops);
> -extern xfs_buf_t *libxfs_getsb(struct xfs_mount *, int);
> +extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
> extern void libxfs_bcache_purge(void);
> extern void libxfs_bcache_free(void);
> extern void libxfs_bcache_flush(void);
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index d668a157..157a99d6 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -559,7 +559,7 @@ typedef int (*xfs_rtalloc_query_range_fn)(
> struct xfs_rtalloc_rec *rec,
> void *priv);
>
> -int libxfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
> +int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
> xfs_off_t count_fsb);
>
>
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index e3ff5842..8d6f958a 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -476,7 +476,7 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
>
>
> xfs_buf_t *
> -libxfs_getsb(xfs_mount_t *mp, int flags)
> +xfs_getsb(xfs_mount_t *mp, int flags)
> {
> return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
> XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
> @@ -1374,7 +1374,7 @@ extern kmem_zone_t *xfs_ili_zone;
> * make sure they're not corrupt.
> */
> bool
> -libxfs_inode_verify_forks(
> +xfs_inode_verify_forks(
> struct xfs_inode *ip)
> {
> struct xfs_ifork *ifp;
> @@ -1403,7 +1403,7 @@ libxfs_inode_verify_forks(
> }
>
> int
> -libxfs_iget(
> +xfs_iget(
> struct xfs_mount *mp,
> struct xfs_trans *tp,
> xfs_ino_t ino,
> @@ -1428,8 +1428,8 @@ libxfs_iget(
> }
>
> ip->i_fork_ops = ifork_ops;
> - if (!libxfs_inode_verify_forks(ip)) {
> - libxfs_irele(ip);
> + if (!xfs_inode_verify_forks(ip)) {
> + xfs_irele(ip);
> return -EFSCORRUPTED;
> }
>
> @@ -1446,26 +1446,26 @@ libxfs_iget(
> }
>
> static void
> -libxfs_idestroy(xfs_inode_t *ip)
> +xfs_idestroy(xfs_inode_t *ip)
> {
> switch (VFS_I(ip)->i_mode & S_IFMT) {
> case S_IFREG:
> case S_IFDIR:
> case S_IFLNK:
> - libxfs_idestroy_fork(ip, XFS_DATA_FORK);
> + xfs_idestroy_fork(ip, XFS_DATA_FORK);
> break;
> }
> if (ip->i_afp)
> - libxfs_idestroy_fork(ip, XFS_ATTR_FORK);
> + xfs_idestroy_fork(ip, XFS_ATTR_FORK);
> if (ip->i_cowfp)
> xfs_idestroy_fork(ip, XFS_COW_FORK);
> }
>
> void
> -libxfs_irele(
> +xfs_irele(
> struct xfs_inode *ip)
> {
> ASSERT(ip->i_itemp == NULL);
> - libxfs_idestroy(ip);
> + xfs_idestroy(ip);
> kmem_zone_free(xfs_inode_zone, ip);
> }
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index cb15552c..5ef0c055 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -36,7 +36,7 @@ kmem_zone_t *xfs_trans_zone;
> * in the mount structure.
> */
> void
> -libxfs_trans_init(
> +xfs_trans_init(
> struct xfs_mount *mp)
> {
> xfs_trans_resv_calc(mp, &mp->m_resv);
> @@ -46,7 +46,7 @@ libxfs_trans_init(
> * Add the given log item to the transaction's list of log items.
> */
> void
> -libxfs_trans_add_item(
> +xfs_trans_add_item(
> struct xfs_trans *tp,
> struct xfs_log_item *lip)
> {
> @@ -62,7 +62,7 @@ libxfs_trans_add_item(
> * Unlink and free the given descriptor.
> */
> void
> -libxfs_trans_del_item(
> +xfs_trans_del_item(
> struct xfs_log_item *lip)
> {
> clear_bit(XFS_LI_DIRTY, &lip->li_flags);
> @@ -77,7 +77,7 @@ libxfs_trans_del_item(
> * chunk we've been working on and get a new transaction to continue.
> */
> int
> -libxfs_trans_roll(
> +xfs_trans_roll(
> struct xfs_trans **tpp)
> {
> struct xfs_trans *trans = *tpp;
> @@ -245,7 +245,7 @@ undo_blocks:
> }
>
> int
> -libxfs_trans_alloc(
> +xfs_trans_alloc(
> struct xfs_mount *mp,
> struct xfs_trans_res *resp,
> unsigned int blocks,
> @@ -289,7 +289,7 @@ libxfs_trans_alloc(
> * without any dirty data.
> */
> int
> -libxfs_trans_alloc_empty(
> +xfs_trans_alloc_empty(
> struct xfs_mount *mp,
> struct xfs_trans **tpp)
> {
> @@ -304,17 +304,17 @@ libxfs_trans_alloc_empty(
> * permanent log reservation flag to avoid blowing asserts.
> */
> int
> -libxfs_trans_alloc_rollable(
> +xfs_trans_alloc_rollable(
> struct xfs_mount *mp,
> unsigned int blocks,
> struct xfs_trans **tpp)
> {
> - return libxfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
> + return xfs_trans_alloc(mp, &M_RES(mp)->tr_itruncate, blocks,
> 0, 0, tpp);
> }
>
> void
> -libxfs_trans_cancel(
> +xfs_trans_cancel(
> struct xfs_trans *tp)
> {
> #ifdef XACT_DEBUG
> @@ -337,7 +337,7 @@ out:
> }
>
> void
> -libxfs_trans_ijoin(
> +xfs_trans_ijoin(
> xfs_trans_t *tp,
> xfs_inode_t *ip,
> uint lock_flags)
> @@ -360,7 +360,7 @@ libxfs_trans_ijoin(
> }
>
> void
> -libxfs_trans_inode_alloc_buf(
> +xfs_trans_inode_alloc_buf(
> xfs_trans_t *tp,
> xfs_buf_t *bp)
> {
> @@ -407,7 +407,7 @@ xfs_trans_log_inode(
> }
>
> int
> -libxfs_trans_roll_inode(
> +xfs_trans_roll_inode(
> struct xfs_trans **tpp,
> struct xfs_inode *ip)
> {
> @@ -425,7 +425,7 @@ libxfs_trans_roll_inode(
> * Mark a buffer dirty in the transaction.
> */
> void
> -libxfs_trans_dirty_buf(
> +xfs_trans_dirty_buf(
> struct xfs_trans *tp,
> struct xfs_buf *bp)
> {
> @@ -451,7 +451,7 @@ libxfs_trans_dirty_buf(
> * value of b_blkno.
> */
> void
> -libxfs_trans_log_buf(
> +xfs_trans_log_buf(
> struct xfs_trans *tp,
> struct xfs_buf *bp,
> uint first,
> @@ -473,7 +473,7 @@ libxfs_trans_log_buf(
> * If the buffer is already dirty, trigger the "already logged" return condition.
> */
> bool
> -libxfs_trans_ordered_buf(
> +xfs_trans_ordered_buf(
> struct xfs_trans *tp,
> struct xfs_buf *bp)
> {
> @@ -481,7 +481,7 @@ libxfs_trans_ordered_buf(
> bool ret;
>
> ret = test_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
> - libxfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
> + xfs_trans_log_buf(tp, bp, 0, bp->b_bcount);
> return ret;
> }
>
> @@ -496,7 +496,7 @@ xfs_buf_item_put(
> }
>
> void
> -libxfs_trans_brelse(
> +xfs_trans_brelse(
> xfs_trans_t *tp,
> xfs_buf_t *bp)
> {
> @@ -531,7 +531,7 @@ libxfs_trans_brelse(
> }
>
> void
> -libxfs_trans_binval(
> +xfs_trans_binval(
> xfs_trans_t *tp,
> xfs_buf_t *bp)
> {
> @@ -556,7 +556,7 @@ libxfs_trans_binval(
> }
>
> void
> -libxfs_trans_bjoin(
> +xfs_trans_bjoin(
> xfs_trans_t *tp,
> xfs_buf_t *bp)
> {
> @@ -574,7 +574,7 @@ libxfs_trans_bjoin(
> }
>
> void
> -libxfs_trans_bhold(
> +xfs_trans_bhold(
> xfs_trans_t *tp,
> xfs_buf_t *bp)
> {
> @@ -590,7 +590,7 @@ libxfs_trans_bhold(
> }
>
> xfs_buf_t *
> -libxfs_trans_get_buf_map(
> +xfs_trans_get_buf_map(
> xfs_trans_t *tp,
> struct xfs_buftarg *btp,
> struct xfs_buf_map *map,
> @@ -619,14 +619,14 @@ libxfs_trans_get_buf_map(
> fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
> #endif
>
> - libxfs_trans_bjoin(tp, bp);
> + xfs_trans_bjoin(tp, bp);
> bip = bp->b_log_item;
> bip->bli_recur = 0;
> return bp;
> }
>
> xfs_buf_t *
> -libxfs_trans_getsb(
> +xfs_trans_getsb(
> xfs_trans_t *tp,
> xfs_mount_t *mp,
> int flags)
> @@ -637,7 +637,7 @@ libxfs_trans_getsb(
> DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
>
> if (tp == NULL)
> - return libxfs_getsb(mp, flags);
> + return xfs_getsb(mp, flags);
>
> bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
> if (bp != NULL) {
> @@ -648,19 +648,19 @@ libxfs_trans_getsb(
> return bp;
> }
>
> - bp = libxfs_getsb(mp, flags);
> + bp = xfs_getsb(mp, flags);
> #ifdef XACT_DEBUG
> fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
> #endif
>
> - libxfs_trans_bjoin(tp, bp);
> + xfs_trans_bjoin(tp, bp);
> bip = bp->b_log_item;
> bip->bli_recur = 0;
> return bp;
> }
>
> int
> -libxfs_trans_read_buf_map(
> +xfs_trans_read_buf_map(
> xfs_mount_t *mp,
> xfs_trans_t *tp,
> struct xfs_buftarg *btp,
> @@ -728,7 +728,7 @@ out_relse:
> * Originally derived from xfs_trans_mod_sb().
> */
> void
> -libxfs_trans_mod_sb(
> +xfs_trans_mod_sb(
> xfs_trans_t *tp,
> uint field,
> long delta)
> @@ -814,7 +814,7 @@ inode_item_done(
> * of whether the flush succeed or not. If we fail the flush, make sure
> * we still release the buffer reference we currently hold.
> */
> - error = libxfs_iflush_int(ip, bp);
> + error = xfs_iflush_int(ip, bp);
> bp->b_transp = NULL; /* remove xact ptr */
>
> if (error) {
> @@ -989,7 +989,7 @@ out_unreserve:
> }
>
> int
> -libxfs_trans_commit(
> +xfs_trans_commit(
> struct xfs_trans *tp)
> {
> return __xfs_trans_commit(tp, false);
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 8c9954f6..4901123a 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -143,7 +143,7 @@ xfs_log_calc_unit_res(
> * where it's no longer worth the hassle of maintaining common code.
> */
> void
> -libxfs_trans_ichgtime(
> +xfs_trans_ichgtime(
> struct xfs_trans *tp,
> struct xfs_inode *ip,
> int flags)
> @@ -232,7 +232,7 @@ xfs_flags2diflags2(
> * where it's no longer worth the hassle of maintaining common code.
> */
> static int
> -libxfs_ialloc(
> +xfs_ialloc(
> xfs_trans_t *tp,
> xfs_inode_t *pip,
> mode_t mode,
> @@ -262,7 +262,7 @@ libxfs_ialloc(
> }
> ASSERT(*ialloc_context == NULL);
>
> - error = libxfs_iget(tp->t_mountp, tp, ino, 0, &ip,
> + error = xfs_iget(tp->t_mountp, tp, ino, 0, &ip,
> &xfs_default_ifork_ops);
> if (error != 0)
> return error;
> @@ -388,7 +388,7 @@ libxfs_ialloc(
> * Originally based on xfs_iflush_int() from xfs_inode.c in the kernel.
> */
> int
> -libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
> +xfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
> {
> xfs_inode_log_item_t *iip;
> xfs_dinode_t *dip;
> @@ -421,7 +421,7 @@ libxfs_iflush_int(xfs_inode_t *ip, xfs_buf_t *bp)
> VFS_I(ip)->i_version++;
>
> /* Check the inline fork data before we write out. */
> - if (!libxfs_inode_verify_forks(ip))
> + if (!xfs_inode_verify_forks(ip))
> return -EFSCORRUPTED;
>
> /*
> @@ -467,10 +467,9 @@ libxfs_mod_incore_sb(
>
> /*
> * This routine allocates disk space for the given file.
> - * Originally derived from xfs_alloc_file_space().
> */
> int
> -libxfs_alloc_file_space(
> +xfs_alloc_file_space(
> xfs_inode_t *ip,
> xfs_off_t offset,
> xfs_off_t len,
> @@ -547,14 +546,14 @@ error0: /* Cancel bmap, cancel trans */
> }
>
> /*
> - * Wrapper around call to libxfs_ialloc. Takes care of committing and
> + * Wrapper around call to xfs_ialloc. Takes care of committing and
> * allocating a new transaction as needed.
> *
> * Originally there were two copies of this code - one in mkfs, the
> * other in repair - now there is just the one.
> */
> int
> -libxfs_inode_alloc(
> +xfs_inode_alloc(
> xfs_trans_t **tp,
> xfs_inode_t *pip,
> mode_t mode,
> @@ -569,7 +568,7 @@ libxfs_inode_alloc(
> int error;
>
> ialloc_context = (xfs_buf_t *)0;
> - error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
> + error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr, fsx,
> &ialloc_context, &ip);
> if (error) {
> *ipp = NULL;
> @@ -591,7 +590,7 @@ libxfs_inode_alloc(
> exit(1);
> }
> xfs_trans_bjoin(*tp, ialloc_context);
> - error = libxfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
> + error = xfs_ialloc(*tp, pip, mode, nlink, rdev, cr,
> fsx, &ialloc_context, &ip);
> if (!ip)
> error = -ENOSPC;
> @@ -712,7 +711,7 @@ xfs_fsb_to_db(struct xfs_inode *ip, xfs_fsblock_t fsb)
> }
>
> int
> -libxfs_zero_extent(
> +xfs_zero_extent(
> struct xfs_inode *ip,
> xfs_fsblock_t start_fsb,
> xfs_off_t count_fsb)
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_
2019-05-17 20:49 ` Allison Collins
@ 2019-05-17 21:00 ` Eric Sandeen
0 siblings, 0 replies; 29+ messages in thread
From: Eric Sandeen @ 2019-05-17 21:00 UTC (permalink / raw)
To: Allison Collins, linux-xfs
On 5/17/19 3:49 PM, Allison Collins wrote:
>
>
> On 5/16/19 10:43 AM, Eric Sandeen wrote:
>> The libxfs_* function namespace has gotten a bit confused and
>> muddled; the general idea is that functions called from userspace
>> utilities should use the libxfs_* namespace. In many cases
>> we use #defines to define xfs_* namespace to libxfs_*; in other
>> cases we have explicitly defined libxfs_* functions which are clear
>> counterparts or even clones of kernel libxfs/* functions.
>>
>> For any function definitions within libxfs/* which match kernel
>> names, give them standard xfs_* names to further reduce differnces
>> between userspace and kernel libxfs/* code.
>>
>> Then add #defines to libxfs_* for any functions which are needed
>> by utilities, as is done with other core functionality.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/include/libxfs.h b/include/libxfs.h
>> index 230bc3e8..ceebccdc 100644
>> --- a/include/libxfs.h
>> +++ b/include/libxfs.h
>> @@ -151,7 +151,7 @@ extern int libxfs_log_header(char *, uuid_t *, int, int, int, xfs_lsn_t,
>> /* Shared utility routines */
>> -extern int libxfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
>> +extern int xfs_alloc_file_space (struct xfs_inode *, xfs_off_t,
>> xfs_off_t, int, int);
>> /* XXX: this is messy and needs fixing */
>> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
>> index 88b58ac3..3e7e80ea 100644
>> --- a/include/xfs_inode.h
>> +++ b/include/xfs_inode.h
>> @@ -139,21 +139,21 @@ typedef struct cred {
>> gid_t cr_gid;
>> } cred_t;
>> -extern int libxfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
>> +extern int xfs_inode_alloc (struct xfs_trans **, struct xfs_inode *,
>> mode_t, nlink_t, xfs_dev_t, struct cred *,
>> struct fsxattr *, struct xfs_inode **);
>> -extern void libxfs_trans_inode_alloc_buf (struct xfs_trans *,
>> +extern void xfs_trans_inode_alloc_buf (struct xfs_trans *,
>> struct xfs_buf *);
>> -extern void libxfs_trans_ichgtime(struct xfs_trans *,
>> +extern void xfs_trans_ichgtime(struct xfs_trans *,
>> struct xfs_inode *, int);
>> -extern int libxfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>> +extern int xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
>> /* Inode Cache Interfaces */
>> -extern bool libxfs_inode_verify_forks(struct xfs_inode *ip);
>> -extern int libxfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
>> +extern bool xfs_inode_verify_forks(struct xfs_inode *ip);
>> +extern int xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
>> uint, struct xfs_inode **,
>> struct xfs_ifork_ops *);
>> -extern void libxfs_irele(struct xfs_inode *ip);
>> +extern void xfs_irele(struct xfs_inode *ip);
>> #endif /* __XFS_INODE_H__ */
>> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
>> index 10b74538..d32acc9e 100644
>> --- a/include/xfs_trans.h
>> +++ b/include/xfs_trans.h
>> @@ -75,46 +75,46 @@ typedef struct xfs_trans {
>> void xfs_trans_init(struct xfs_mount *);
>> int xfs_trans_roll(struct xfs_trans **);
>> -int libxfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
>> +int xfs_trans_alloc(struct xfs_mount *mp, struct xfs_trans_res *resp,
>> uint blocks, uint rtextents, uint flags,
>> struct xfs_trans **tpp);
>> int libxfs_trans_alloc_rollable(struct xfs_mount *mp, uint blocks,
>> struct xfs_trans **tpp);
>
> Did you mean to rename libxfs_trans_alloc_rollable too? I notice the function name is changed later down in the patch.
Not sure - it's not actually a kernel-matching function, it should
probably get moved to util.c or something, since it's
userspace-specific.
But yeah, I guess the way I handled it is a little inconsistent,
probably saved by the #define. :)
-Eric
> I think the rest of it looks pretty straight forward though.
>
> Allison
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 2/3] libxfs: remove libxfs API #defines for unexported xfs functions
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
2019-05-16 17:43 ` [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_ Eric Sandeen
@ 2019-05-16 17:45 ` Eric Sandeen
2019-05-17 21:06 ` Allison Collins
2019-05-16 17:46 ` [PATCH 3/3] xfsprogs: remove unused flags arg from getsb interfaces Eric Sandeen
` (8 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 17:45 UTC (permalink / raw)
To: linux-xfs
We define "libxfs_*" functions for anything used by userspace,
called from outside the libxfs/ directory. However, many of the
current redefinitions are for functions only used within libxfs/*
so remove their API redefinitions.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 2b8ac5ab..a53efa68 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -18,37 +18,22 @@
#define xfs_trans_alloc libxfs_trans_alloc
#define xfs_trans_alloc_rollable libxfs_trans_alloc_rollable
-#define xfs_trans_alloc_empty libxfs_trans_alloc_empty
-#define xfs_trans_add_item libxfs_trans_add_item
#define xfs_trans_bhold libxfs_trans_bhold
#define xfs_trans_binval libxfs_trans_binval
#define xfs_trans_bjoin libxfs_trans_bjoin
#define xfs_trans_brelse libxfs_trans_brelse
#define xfs_trans_commit libxfs_trans_commit
#define xfs_trans_cancel libxfs_trans_cancel
-#define xfs_trans_del_item libxfs_trans_del_item
#define xfs_trans_get_buf libxfs_trans_get_buf
-#define xfs_trans_getsb libxfs_trans_getsb
#define xfs_trans_ichgtime libxfs_trans_ichgtime
#define xfs_trans_ijoin libxfs_trans_ijoin
-#define xfs_trans_init libxfs_trans_init
-#define xfs_trans_inode_alloc_buf libxfs_trans_inode_alloc_buf
-#define xfs_trans_dirty_buf libxfs_trans_dirty_buf
#define xfs_trans_log_buf libxfs_trans_log_buf
-#define xfs_trans_ordered_buf libxfs_trans_ordered_buf
#define xfs_trans_log_inode libxfs_trans_log_inode
#define xfs_trans_roll_inode libxfs_trans_roll_inode
-#define xfs_trans_mod_sb libxfs_trans_mod_sb
#define xfs_trans_read_buf libxfs_trans_read_buf
-#define xfs_trans_read_buf_map libxfs_trans_read_buf_map
-#define xfs_trans_roll libxfs_trans_roll
-#define xfs_trans_get_buf_map libxfs_trans_get_buf_map
-#define xfs_trans_resv_calc libxfs_trans_resv_calc
#define xfs_log_get_max_trans_res libxfs_log_get_max_trans_res
-#define xfs_attr_get libxfs_attr_get
#define xfs_attr_set libxfs_attr_set
#define xfs_attr_remove libxfs_attr_remove
-#define xfs_attr_leaf_newentsize libxfs_attr_leaf_newentsize
#define xfs_agfl_walk libxfs_agfl_walk
#define xfs_alloc_fix_freelist libxfs_alloc_fix_freelist
@@ -57,15 +42,11 @@
#define xfs_bmap_last_offset libxfs_bmap_last_offset
#define xfs_iext_lookup_extent libxfs_iext_lookup_extent
#define xfs_bmapi_write libxfs_bmapi_write
-#define xfs_bmapi_read libxfs_bmapi_read
#define xfs_bunmapi libxfs_bunmapi
#define xfs_rtfree_extent libxfs_rtfree_extent
-#define xfs_verify_rtbno libxfs_verify_rtbno
#define xfs_verify_ino libxfs_verify_ino
-#define xfs_zero_extent libxfs_zero_extent
#define xfs_defer_finish libxfs_defer_finish
-#define xfs_defer_cancel libxfs_defer_cancel
#define xfs_da_hashname libxfs_da_hashname
#define xfs_da_shrink_inode libxfs_da_shrink_inode
@@ -85,14 +66,11 @@
#define xfs_da_get_buf libxfs_da_get_buf
#define xfs_inode_from_disk libxfs_inode_from_disk
-#define xfs_inode_to_disk libxfs_inode_to_disk
#define xfs_dinode_calc_crc libxfs_dinode_calc_crc
#define xfs_idata_realloc libxfs_idata_realloc
-#define xfs_idestroy_fork libxfs_idestroy_fork
#define xfs_inode_validate_extsize libxfs_inode_validate_extsize
#define xfs_inode_validate_cowextsize libxfs_inode_validate_cowextsize
#define xfs_inode_alloc libxfs_inode_alloc
-#define xfs_iflush_int libxfs_iflush_int
#define xfs_alloc_file_space libxfs_alloc_file_space
#define xfs_rmap_alloc libxfs_rmap_alloc
@@ -107,7 +85,6 @@
#define xfs_log_sb libxfs_log_sb
#define xfs_sb_from_disk libxfs_sb_from_disk
-#define xfs_sb_quota_from_disk libxfs_sb_quota_from_disk
#define xfs_sb_to_disk libxfs_sb_to_disk
#define xfs_calc_dquots_per_chunk libxfs_calc_dquots_per_chunk
@@ -151,6 +128,5 @@
#define xfs_getsb libxfs_getsb
#define xfs_irele libxfs_irele
#define xfs_iget libxfs_iget
-#define xfs_inode_verify_forks libxfs_inode_verify_forks
#endif /* __LIBXFS_API_DEFS_H__ */
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 2/3] libxfs: remove libxfs API #defines for unexported xfs functions
2019-05-16 17:45 ` [PATCH 2/3] libxfs: remove libxfs API #defines for unexported xfs functions Eric Sandeen
@ 2019-05-17 21:06 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-05-17 21:06 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
On 5/16/19 10:45 AM, Eric Sandeen wrote:
> We define "libxfs_*" functions for anything used by userspace,
> called from outside the libxfs/ directory. However, many of the
> current redefinitions are for functions only used within libxfs/*
> so remove their API redefinitions.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
Ok, looks good to me.
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
>
> diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
> index 2b8ac5ab..a53efa68 100644
> --- a/libxfs/libxfs_api_defs.h
> +++ b/libxfs/libxfs_api_defs.h
> @@ -18,37 +18,22 @@
>
> #define xfs_trans_alloc libxfs_trans_alloc
> #define xfs_trans_alloc_rollable libxfs_trans_alloc_rollable
> -#define xfs_trans_alloc_empty libxfs_trans_alloc_empty
> -#define xfs_trans_add_item libxfs_trans_add_item
> #define xfs_trans_bhold libxfs_trans_bhold
> #define xfs_trans_binval libxfs_trans_binval
> #define xfs_trans_bjoin libxfs_trans_bjoin
> #define xfs_trans_brelse libxfs_trans_brelse
> #define xfs_trans_commit libxfs_trans_commit
> #define xfs_trans_cancel libxfs_trans_cancel
> -#define xfs_trans_del_item libxfs_trans_del_item
> #define xfs_trans_get_buf libxfs_trans_get_buf
> -#define xfs_trans_getsb libxfs_trans_getsb
> #define xfs_trans_ichgtime libxfs_trans_ichgtime
> #define xfs_trans_ijoin libxfs_trans_ijoin
> -#define xfs_trans_init libxfs_trans_init
> -#define xfs_trans_inode_alloc_buf libxfs_trans_inode_alloc_buf
> -#define xfs_trans_dirty_buf libxfs_trans_dirty_buf
> #define xfs_trans_log_buf libxfs_trans_log_buf
> -#define xfs_trans_ordered_buf libxfs_trans_ordered_buf
> #define xfs_trans_log_inode libxfs_trans_log_inode
> #define xfs_trans_roll_inode libxfs_trans_roll_inode
> -#define xfs_trans_mod_sb libxfs_trans_mod_sb
> #define xfs_trans_read_buf libxfs_trans_read_buf
> -#define xfs_trans_read_buf_map libxfs_trans_read_buf_map
> -#define xfs_trans_roll libxfs_trans_roll
> -#define xfs_trans_get_buf_map libxfs_trans_get_buf_map
> -#define xfs_trans_resv_calc libxfs_trans_resv_calc
> #define xfs_log_get_max_trans_res libxfs_log_get_max_trans_res
> -#define xfs_attr_get libxfs_attr_get
> #define xfs_attr_set libxfs_attr_set
> #define xfs_attr_remove libxfs_attr_remove
> -#define xfs_attr_leaf_newentsize libxfs_attr_leaf_newentsize
>
> #define xfs_agfl_walk libxfs_agfl_walk
> #define xfs_alloc_fix_freelist libxfs_alloc_fix_freelist
> @@ -57,15 +42,11 @@
> #define xfs_bmap_last_offset libxfs_bmap_last_offset
> #define xfs_iext_lookup_extent libxfs_iext_lookup_extent
> #define xfs_bmapi_write libxfs_bmapi_write
> -#define xfs_bmapi_read libxfs_bmapi_read
> #define xfs_bunmapi libxfs_bunmapi
> #define xfs_rtfree_extent libxfs_rtfree_extent
> -#define xfs_verify_rtbno libxfs_verify_rtbno
> #define xfs_verify_ino libxfs_verify_ino
> -#define xfs_zero_extent libxfs_zero_extent
>
> #define xfs_defer_finish libxfs_defer_finish
> -#define xfs_defer_cancel libxfs_defer_cancel
>
> #define xfs_da_hashname libxfs_da_hashname
> #define xfs_da_shrink_inode libxfs_da_shrink_inode
> @@ -85,14 +66,11 @@
> #define xfs_da_get_buf libxfs_da_get_buf
>
> #define xfs_inode_from_disk libxfs_inode_from_disk
> -#define xfs_inode_to_disk libxfs_inode_to_disk
> #define xfs_dinode_calc_crc libxfs_dinode_calc_crc
> #define xfs_idata_realloc libxfs_idata_realloc
> -#define xfs_idestroy_fork libxfs_idestroy_fork
> #define xfs_inode_validate_extsize libxfs_inode_validate_extsize
> #define xfs_inode_validate_cowextsize libxfs_inode_validate_cowextsize
> #define xfs_inode_alloc libxfs_inode_alloc
> -#define xfs_iflush_int libxfs_iflush_int
> #define xfs_alloc_file_space libxfs_alloc_file_space
>
> #define xfs_rmap_alloc libxfs_rmap_alloc
> @@ -107,7 +85,6 @@
>
> #define xfs_log_sb libxfs_log_sb
> #define xfs_sb_from_disk libxfs_sb_from_disk
> -#define xfs_sb_quota_from_disk libxfs_sb_quota_from_disk
> #define xfs_sb_to_disk libxfs_sb_to_disk
>
> #define xfs_calc_dquots_per_chunk libxfs_calc_dquots_per_chunk
> @@ -151,6 +128,5 @@
> #define xfs_getsb libxfs_getsb
> #define xfs_irele libxfs_irele
> #define xfs_iget libxfs_iget
> -#define xfs_inode_verify_forks libxfs_inode_verify_forks
>
> #endif /* __LIBXFS_API_DEFS_H__ */
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 3/3] xfsprogs: remove unused flags arg from getsb interfaces
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
2019-05-16 17:43 ` [PATCH 1/3] libxfs: rename shared kernel functions from libxfs_ to xfs_ Eric Sandeen
2019-05-16 17:45 ` [PATCH 2/3] libxfs: remove libxfs API #defines for unexported xfs functions Eric Sandeen
@ 2019-05-16 17:46 ` Eric Sandeen
2019-05-17 21:09 ` Allison Collins
2019-05-16 20:38 ` [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs Eric Sandeen
` (7 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 17:46 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
The flags value is always* passed as 0 so remove the argument.
*mkfs.xfs is the sole exception; it passes a flag to fail on read
error, but we can easily detect the error and exit from main()
manually instead.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
(This one might actually come in via a libxfs merge instead)
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index d32acc9e..953da5d1 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -87,7 +87,7 @@ void xfs_trans_cancel(struct xfs_trans *);
/* cancel dfops associated with a transaction */
void xfs_defer_cancel(struct xfs_trans *);
-struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
+struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *);
void xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
void xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
index 8fa2c2c5..4dc4d5f3 100644
--- a/libxfs/libxfs_io.h
+++ b/libxfs/libxfs_io.h
@@ -177,7 +177,7 @@ extern void libxfs_putbuf (xfs_buf_t *);
extern void libxfs_readbuf_verify(struct xfs_buf *bp,
const struct xfs_buf_ops *ops);
-extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
+extern xfs_buf_t *xfs_getsb(struct xfs_mount *);
extern void libxfs_bcache_purge(void);
extern void libxfs_bcache_free(void);
extern void libxfs_bcache_flush(void);
diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
index 8d6f958a..132ed0a9 100644
--- a/libxfs/rdwr.c
+++ b/libxfs/rdwr.c
@@ -476,10 +476,10 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
xfs_buf_t *
-xfs_getsb(xfs_mount_t *mp, int flags)
+xfs_getsb(xfs_mount_t *mp)
{
return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
- XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
+ XFS_FSS_TO_BB(mp, 1), 0, &xfs_sb_buf_ops);
}
kmem_zone_t *xfs_buf_zone;
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 5ef0c055..9e49def0 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -628,8 +628,7 @@ xfs_trans_get_buf_map(
xfs_buf_t *
xfs_trans_getsb(
xfs_trans_t *tp,
- xfs_mount_t *mp,
- int flags)
+ xfs_mount_t *mp)
{
xfs_buf_t *bp;
xfs_buf_log_item_t *bip;
@@ -637,7 +636,7 @@ xfs_trans_getsb(
DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
if (tp == NULL)
- return xfs_getsb(mp, flags);
+ return xfs_getsb(mp);
bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
if (bp != NULL) {
@@ -648,7 +647,7 @@ xfs_trans_getsb(
return bp;
}
- bp = xfs_getsb(mp, flags);
+ bp = xfs_getsb(mp);
#ifdef XACT_DEBUG
fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
#endif
diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
index f1e2ba57..a1857e10 100644
--- a/libxfs/xfs_sb.c
+++ b/libxfs/xfs_sb.c
@@ -915,7 +915,7 @@ xfs_log_sb(
struct xfs_trans *tp)
{
struct xfs_mount *mp = tp->t_mountp;
- struct xfs_buf *bp = xfs_trans_getsb(tp, mp, 0);
+ struct xfs_buf *bp = xfs_trans_getsb(tp, mp);
mp->m_sb.sb_icount = percpu_counter_sum(&mp->m_icount);
mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
@@ -1045,7 +1045,7 @@ xfs_sync_sb_buf(
if (error)
return error;
- bp = xfs_trans_getsb(tp, mp, 0);
+ bp = xfs_trans_getsb(tp, mp);
xfs_log_sb(tp);
xfs_trans_bhold(tp, bp);
xfs_trans_set_sync(tp);
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 09106648..647e2bc2 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -4123,7 +4123,11 @@ main(
/*
* Mark the filesystem ok.
*/
- buf = libxfs_getsb(mp, LIBXFS_EXIT_ON_FAILURE);
+ buf = libxfs_getsb(mp);
+ if (!buf) {
+ fprintf(stderr, _("couldn't get superblock\n"));
+ exit(1);
+ }
(XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
diff --git a/repair/phase5.c b/repair/phase5.c
index 0f6a8395..8ad32365 100644
--- a/repair/phase5.c
+++ b/repair/phase5.c
@@ -2177,7 +2177,7 @@ sync_sb(xfs_mount_t *mp)
{
xfs_buf_t *bp;
- bp = libxfs_getsb(mp, 0);
+ bp = libxfs_getsb(mp);
if (!bp)
do_error(_("couldn't get superblock\n"));
diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
index 9657503f..4000b3e6 100644
--- a/repair/xfs_repair.c
+++ b/repair/xfs_repair.c
@@ -1044,7 +1044,7 @@ _("Warning: project quota information would be cleared.\n"
/*
* Clear the quota flags if they're on.
*/
- sbp = libxfs_getsb(mp, 0);
+ sbp = libxfs_getsb(mp);
if (!sbp)
do_error(_("couldn't get superblock\n"));
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 3/3] xfsprogs: remove unused flags arg from getsb interfaces
2019-05-16 17:46 ` [PATCH 3/3] xfsprogs: remove unused flags arg from getsb interfaces Eric Sandeen
@ 2019-05-17 21:09 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-05-17 21:09 UTC (permalink / raw)
To: Eric Sandeen, Eric Sandeen, linux-xfs
On 5/16/19 10:46 AM, Eric Sandeen wrote:
> The flags value is always* passed as 0 so remove the argument.
>
> *mkfs.xfs is the sole exception; it passes a flag to fail on read
> error, but we can easily detect the error and exit from main()
> manually instead.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok to me.
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> ---
>
> (This one might actually come in via a libxfs merge instead)
>
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index d32acc9e..953da5d1 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -87,7 +87,7 @@ void xfs_trans_cancel(struct xfs_trans *);
> /* cancel dfops associated with a transaction */
> void xfs_defer_cancel(struct xfs_trans *);
>
> -struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *, int);
> +struct xfs_buf *xfs_trans_getsb(struct xfs_trans *, struct xfs_mount *);
>
> void xfs_trans_ijoin(struct xfs_trans *, struct xfs_inode *, uint);
> void xfs_trans_log_inode (struct xfs_trans *, struct xfs_inode *,
> diff --git a/libxfs/libxfs_io.h b/libxfs/libxfs_io.h
> index 8fa2c2c5..4dc4d5f3 100644
> --- a/libxfs/libxfs_io.h
> +++ b/libxfs/libxfs_io.h
> @@ -177,7 +177,7 @@ extern void libxfs_putbuf (xfs_buf_t *);
>
> extern void libxfs_readbuf_verify(struct xfs_buf *bp,
> const struct xfs_buf_ops *ops);
> -extern xfs_buf_t *xfs_getsb(struct xfs_mount *, int);
> +extern xfs_buf_t *xfs_getsb(struct xfs_mount *);
> extern void libxfs_bcache_purge(void);
> extern void libxfs_bcache_free(void);
> extern void libxfs_bcache_flush(void);
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index 8d6f958a..132ed0a9 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -476,10 +476,10 @@ libxfs_trace_putbuf(const char *func, const char *file, int line, xfs_buf_t *bp)
>
>
> xfs_buf_t *
> -xfs_getsb(xfs_mount_t *mp, int flags)
> +xfs_getsb(xfs_mount_t *mp)
> {
> return libxfs_readbuf(mp->m_ddev_targp, XFS_SB_DADDR,
> - XFS_FSS_TO_BB(mp, 1), flags, &xfs_sb_buf_ops);
> + XFS_FSS_TO_BB(mp, 1), 0, &xfs_sb_buf_ops);
> }
>
> kmem_zone_t *xfs_buf_zone;
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 5ef0c055..9e49def0 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -628,8 +628,7 @@ xfs_trans_get_buf_map(
> xfs_buf_t *
> xfs_trans_getsb(
> xfs_trans_t *tp,
> - xfs_mount_t *mp,
> - int flags)
> + xfs_mount_t *mp)
> {
> xfs_buf_t *bp;
> xfs_buf_log_item_t *bip;
> @@ -637,7 +636,7 @@ xfs_trans_getsb(
> DEFINE_SINGLE_BUF_MAP(map, XFS_SB_DADDR, len);
>
> if (tp == NULL)
> - return xfs_getsb(mp, flags);
> + return xfs_getsb(mp);
>
> bp = xfs_trans_buf_item_match(tp, mp->m_dev, &map, 1);
> if (bp != NULL) {
> @@ -648,7 +647,7 @@ xfs_trans_getsb(
> return bp;
> }
>
> - bp = xfs_getsb(mp, flags);
> + bp = xfs_getsb(mp);
> #ifdef XACT_DEBUG
> fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
> #endif
> diff --git a/libxfs/xfs_sb.c b/libxfs/xfs_sb.c
> index f1e2ba57..a1857e10 100644
> --- a/libxfs/xfs_sb.c
> +++ b/libxfs/xfs_sb.c
> @@ -915,7 +915,7 @@ xfs_log_sb(
> struct xfs_trans *tp)
> {
> struct xfs_mount *mp = tp->t_mountp;
> - struct xfs_buf *bp = xfs_trans_getsb(tp, mp, 0);
> + struct xfs_buf *bp = xfs_trans_getsb(tp, mp);
>
> mp->m_sb.sb_icount = percpu_counter_sum(&mp->m_icount);
> mp->m_sb.sb_ifree = percpu_counter_sum(&mp->m_ifree);
> @@ -1045,7 +1045,7 @@ xfs_sync_sb_buf(
> if (error)
> return error;
>
> - bp = xfs_trans_getsb(tp, mp, 0);
> + bp = xfs_trans_getsb(tp, mp);
> xfs_log_sb(tp);
> xfs_trans_bhold(tp, bp);
> xfs_trans_set_sync(tp);
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 09106648..647e2bc2 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -4123,7 +4123,11 @@ main(
> /*
> * Mark the filesystem ok.
> */
> - buf = libxfs_getsb(mp, LIBXFS_EXIT_ON_FAILURE);
> + buf = libxfs_getsb(mp);
> + if (!buf) {
> + fprintf(stderr, _("couldn't get superblock\n"));
> + exit(1);
> + }
> (XFS_BUF_TO_SBP(buf))->sb_inprogress = 0;
> libxfs_writebuf(buf, LIBXFS_EXIT_ON_FAILURE);
>
> diff --git a/repair/phase5.c b/repair/phase5.c
> index 0f6a8395..8ad32365 100644
> --- a/repair/phase5.c
> +++ b/repair/phase5.c
> @@ -2177,7 +2177,7 @@ sync_sb(xfs_mount_t *mp)
> {
> xfs_buf_t *bp;
>
> - bp = libxfs_getsb(mp, 0);
> + bp = libxfs_getsb(mp);
> if (!bp)
> do_error(_("couldn't get superblock\n"));
>
> diff --git a/repair/xfs_repair.c b/repair/xfs_repair.c
> index 9657503f..4000b3e6 100644
> --- a/repair/xfs_repair.c
> +++ b/repair/xfs_repair.c
> @@ -1044,7 +1044,7 @@ _("Warning: project quota information would be cleared.\n"
> /*
> * Clear the quota flags if they're on.
> */
> - sbp = libxfs_getsb(mp, 0);
> + sbp = libxfs_getsb(mp);
> if (!sbp)
> do_error(_("couldn't get superblock\n"));
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (2 preceding siblings ...)
2019-05-16 17:46 ` [PATCH 3/3] xfsprogs: remove unused flags arg from getsb interfaces Eric Sandeen
@ 2019-05-16 20:38 ` Eric Sandeen
2019-05-17 21:36 ` Allison Collins
2019-05-16 20:39 ` [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats Eric Sandeen
` (6 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 20:38 UTC (permalink / raw)
To: linux-xfs
Remove XACT_DEBUG #ifdefs to reduce more cosmetic differences
between userspace & kernelspace libxfs. Add in some corresponding
(stubbed-out) tracepoint calls.
If these are felt to be particularly useful, the tracepoint calls
could be fleshed out to provide similar information.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
include/xfs_trace.h | 13 +++++++
libxfs/trans.c | 85 ++++++++++++---------------------------------
2 files changed, 35 insertions(+), 63 deletions(-)
diff --git a/include/xfs_trace.h b/include/xfs_trace.h
index 793ac56e..43720040 100644
--- a/include/xfs_trace.h
+++ b/include/xfs_trace.h
@@ -161,6 +161,19 @@
#define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
#define trace_xfs_perag_put(a,b,c,d) ((c) = (c))
+#define trace_xfs_trans_alloc(a,b) ((void) 0)
+#define trace_xfs_trans_cancel(a,b) ((void) 0)
+#define trace_xfs_trans_brelse(a) ((void) 0)
+#define trace_xfs_trans_binval(a) ((void) 0)
+#define trace_xfs_trans_bjoin(a) ((void) 0)
+#define trace_xfs_trans_bhold(a) ((void) 0)
+#define trace_xfs_trans_get_buf(a) ((void) 0)
+#define trace_xfs_trans_getsb_recur(a) ((void) 0)
+#define trace_xfs_trans_getsb(a) ((void) 0)
+#define trace_xfs_trans_read_buf_recur(a) ((void) 0)
+#define trace_xfs_trans_read_buf(a) ((void) 0)
+#define trace_xfs_trans_commit(a,b) ((void) 0)
+
#define trace_xfs_defer_cancel(a,b) ((void) 0)
#define trace_xfs_defer_pending_commit(a,b) ((void) 0)
#define trace_xfs_defer_pending_abort(a,b) ((void) 0)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 9e49def0..6967a1de 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -18,6 +18,7 @@
#include "xfs_trans.h"
#include "xfs_sb.h"
#include "xfs_defer.h"
+#include "xfs_trace.h"
static void xfs_trans_free_items(struct xfs_trans *tp);
STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
@@ -269,9 +270,9 @@ xfs_trans_alloc(
xfs_trans_cancel(tp);
return error;
}
-#ifdef XACT_DEBUG
- fprintf(stderr, "allocated new transaction %p\n", tp);
-#endif
+
+ trace_xfs_trans_alloc(tp, _RET_IP_);
+
*tpp = tp;
return 0;
}
@@ -317,23 +318,16 @@ void
xfs_trans_cancel(
struct xfs_trans *tp)
{
-#ifdef XACT_DEBUG
- struct xfs_trans *otp = tp;
-#endif
+ trace_xfs_trans_cancel(tp, _RET_IP_);
+
if (tp == NULL)
- goto out;
+ return;
if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
xfs_defer_cancel(tp);
xfs_trans_free_items(tp);
xfs_trans_free(tp);
-
-out:
-#ifdef XACT_DEBUG
- fprintf(stderr, "## cancelled transaction %p\n", otp);
-#endif
- return;
}
void
@@ -353,10 +347,6 @@ xfs_trans_ijoin(
iip->ili_lock_flags = lock_flags;
xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
-
-#ifdef XACT_DEBUG
- fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
}
void
@@ -388,9 +378,6 @@ xfs_trans_log_inode(
uint flags)
{
ASSERT(ip->i_itemp != NULL);
-#ifdef XACT_DEBUG
- fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
-#endif
tp->t_flags |= XFS_TRANS_DIRTY;
set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
@@ -434,9 +421,6 @@ xfs_trans_dirty_buf(
ASSERT(bp->b_transp == tp);
ASSERT(bip != NULL);
-#ifdef XACT_DEBUG
- fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
-#endif
tp->t_flags |= XFS_TRANS_DIRTY;
set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
}
@@ -501,15 +485,14 @@ xfs_trans_brelse(
xfs_buf_t *bp)
{
xfs_buf_log_item_t *bip;
-#ifdef XACT_DEBUG
- fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
-#endif
if (tp == NULL) {
ASSERT(bp->b_transp == NULL);
libxfs_putbuf(bp);
return;
}
+
+ trace_xfs_trans_brelse(bip);
ASSERT(bp->b_transp == tp);
bip = bp->b_log_item;
ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
@@ -536,13 +519,12 @@ xfs_trans_binval(
xfs_buf_t *bp)
{
xfs_buf_log_item_t *bip = bp->b_log_item;
-#ifdef XACT_DEBUG
- fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
-#endif
ASSERT(bp->b_transp == tp);
ASSERT(bip != NULL);
+ trace_xfs_trans_binval(bip);
+
if (bip->bli_flags & XFS_BLI_STALE)
return;
XFS_BUF_UNDELAYWRITE(bp);
@@ -563,14 +545,12 @@ xfs_trans_bjoin(
xfs_buf_log_item_t *bip;
ASSERT(bp->b_transp == NULL);
-#ifdef XACT_DEBUG
- fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
-#endif
xfs_buf_item_init(bp, tp->t_mountp);
bip = bp->b_log_item;
xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
bp->b_transp = tp;
+ trace_xfs_trans_bjoin(bp->b_log_item);
}
void
@@ -582,11 +562,9 @@ xfs_trans_bhold(
ASSERT(bp->b_transp == tp);
ASSERT(bip != NULL);
-#ifdef XACT_DEBUG
- fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
-#endif
bip->bli_flags |= XFS_BLI_HOLD;
+ trace_xfs_trans_bhold(bip);
}
xfs_buf_t *
@@ -615,13 +593,11 @@ xfs_trans_get_buf_map(
bp = libxfs_getbuf_map(btp, map, nmaps, 0);
if (bp == NULL)
return NULL;
-#ifdef XACT_DEBUG
- fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
-#endif
xfs_trans_bjoin(tp, bp);
bip = bp->b_log_item;
bip->bli_recur = 0;
+ trace_xfs_trans_get_buf(bp->b_log_item);
return bp;
}
@@ -644,17 +620,16 @@ xfs_trans_getsb(
bip = bp->b_log_item;
ASSERT(bip != NULL);
bip->bli_recur++;
+ trace_xfs_trans_getsb_recur(bip);
return bp;
}
bp = xfs_getsb(mp);
-#ifdef XACT_DEBUG
- fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
-#endif
xfs_trans_bjoin(tp, bp);
bip = bp->b_log_item;
bip->bli_recur = 0;
+ trace_xfs_trans_getsb(bp->b_log_item);
return bp;
}
@@ -691,6 +666,7 @@ xfs_trans_read_buf_map(
ASSERT(bp->b_log_item != NULL);
bip = bp->b_log_item;
bip->bli_recur++;
+ trace_xfs_trans_read_buf_recur(bip);
goto done;
}
@@ -701,14 +677,11 @@ xfs_trans_read_buf_map(
if (bp->b_error)
goto out_relse;
-#ifdef XACT_DEBUG
- fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
-#endif
-
xfs_trans_bjoin(tp, bp);
bip = bp->b_log_item;
bip->bli_recur = 0;
done:
+ trace_xfs_trans_read_buf(bp->b_log_item);
*bpp = bp;
return 0;
out_relse:
@@ -824,10 +797,6 @@ inode_item_done(
}
libxfs_writebuf(bp, 0);
-#ifdef XACT_DEBUG
- fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
- ip->i_ino, bp);
-#endif
free:
xfs_inode_item_put(iip);
}
@@ -845,13 +814,8 @@ buf_item_done(
bp->b_transp = NULL; /* remove xact ptr */
hold = (bip->bli_flags & XFS_BLI_HOLD);
- if (bip->bli_flags & XFS_BLI_DIRTY) {
-#ifdef XACT_DEBUG
- fprintf(stderr, "flushing/staling buffer %p (hold=%d)\n",
- bp, hold);
-#endif
+ if (bip->bli_flags & XFS_BLI_DIRTY)
libxfs_writebuf_int(bp, 0);
- }
bip->bli_flags &= ~XFS_BLI_HOLD;
xfs_buf_item_put(bip);
@@ -937,6 +901,8 @@ __xfs_trans_commit(
struct xfs_sb *sbp;
int error = 0;
+ trace_xfs_trans_commit(tp, _RET_IP_);
+
if (tp == NULL)
return 0;
@@ -952,12 +918,8 @@ __xfs_trans_commit(
goto out_unreserve;
}
- if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
-#ifdef XACT_DEBUG
- fprintf(stderr, "committed clean transaction %p\n", tp);
-#endif
+ if (!(tp->t_flags & XFS_TRANS_DIRTY))
goto out_unreserve;
- }
if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
sbp = &(tp->t_mountp->m_sb);
@@ -972,9 +934,6 @@ __xfs_trans_commit(
xfs_log_sb(tp);
}
-#ifdef XACT_DEBUG
- fprintf(stderr, "committing dirty transaction %p\n", tp);
-#endif
trans_committed(tp);
/* That's it for the transaction structure. Free it. */
--
2.17.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs
2019-05-16 20:38 ` [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs Eric Sandeen
@ 2019-05-17 21:36 ` Allison Collins
2019-05-20 22:53 ` Darrick J. Wong
0 siblings, 1 reply; 29+ messages in thread
From: Allison Collins @ 2019-05-17 21:36 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
On 5/16/19 1:38 PM, Eric Sandeen wrote:
> Remove XACT_DEBUG #ifdefs to reduce more cosmetic differences
> between userspace & kernelspace libxfs. Add in some corresponding
> (stubbed-out) tracepoint calls.
>
> If these are felt to be particularly useful, the tracepoint calls
> could be fleshed out to provide similar information.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok. It seems a shame to loose the extra tracing, but I confess
most of the time when I'm chasing a particular bug I end up adding my
own temporary traces anyway. So I think trying to synchronize the code
spaces is more important. Do you think the stubs might be something to
add to the kernel space side too?
Allison
> ---
> include/xfs_trace.h | 13 +++++++
> libxfs/trans.c | 85 ++++++++++++---------------------------------
> 2 files changed, 35 insertions(+), 63 deletions(-)
>
> diff --git a/include/xfs_trace.h b/include/xfs_trace.h
> index 793ac56e..43720040 100644
> --- a/include/xfs_trace.h
> +++ b/include/xfs_trace.h
> @@ -161,6 +161,19 @@
> #define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
> #define trace_xfs_perag_put(a,b,c,d) ((c) = (c))
>
> +#define trace_xfs_trans_alloc(a,b) ((void) 0)
> +#define trace_xfs_trans_cancel(a,b) ((void) 0)
> +#define trace_xfs_trans_brelse(a) ((void) 0)
> +#define trace_xfs_trans_binval(a) ((void) 0)
> +#define trace_xfs_trans_bjoin(a) ((void) 0)
> +#define trace_xfs_trans_bhold(a) ((void) 0)
> +#define trace_xfs_trans_get_buf(a) ((void) 0)
> +#define trace_xfs_trans_getsb_recur(a) ((void) 0)
> +#define trace_xfs_trans_getsb(a) ((void) 0)
> +#define trace_xfs_trans_read_buf_recur(a) ((void) 0)
> +#define trace_xfs_trans_read_buf(a) ((void) 0)
> +#define trace_xfs_trans_commit(a,b) ((void) 0)
> +
> #define trace_xfs_defer_cancel(a,b) ((void) 0)
> #define trace_xfs_defer_pending_commit(a,b) ((void) 0)
> #define trace_xfs_defer_pending_abort(a,b) ((void) 0)
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 9e49def0..6967a1de 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -18,6 +18,7 @@
> #include "xfs_trans.h"
> #include "xfs_sb.h"
> #include "xfs_defer.h"
> +#include "xfs_trace.h"
>
> static void xfs_trans_free_items(struct xfs_trans *tp);
> STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
> @@ -269,9 +270,9 @@ xfs_trans_alloc(
> xfs_trans_cancel(tp);
> return error;
> }
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "allocated new transaction %p\n", tp);
> -#endif
> +
> + trace_xfs_trans_alloc(tp, _RET_IP_);
> +
> *tpp = tp;
> return 0;
> }
> @@ -317,23 +318,16 @@ void
> xfs_trans_cancel(
> struct xfs_trans *tp)
> {
> -#ifdef XACT_DEBUG
> - struct xfs_trans *otp = tp;
> -#endif
> + trace_xfs_trans_cancel(tp, _RET_IP_);
> +
> if (tp == NULL)
> - goto out;
> + return;
>
> if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
> xfs_defer_cancel(tp);
>
> xfs_trans_free_items(tp);
> xfs_trans_free(tp);
> -
> -out:
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "## cancelled transaction %p\n", otp);
> -#endif
> - return;
> }
>
> void
> @@ -353,10 +347,6 @@ xfs_trans_ijoin(
> iip->ili_lock_flags = lock_flags;
>
> xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
> -
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
> -#endif
> }
>
> void
> @@ -388,9 +378,6 @@ xfs_trans_log_inode(
> uint flags)
> {
> ASSERT(ip->i_itemp != NULL);
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
> -#endif
>
> tp->t_flags |= XFS_TRANS_DIRTY;
> set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
> @@ -434,9 +421,6 @@ xfs_trans_dirty_buf(
> ASSERT(bp->b_transp == tp);
> ASSERT(bip != NULL);
>
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
> -#endif
> tp->t_flags |= XFS_TRANS_DIRTY;
> set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
> }
> @@ -501,15 +485,14 @@ xfs_trans_brelse(
> xfs_buf_t *bp)
> {
> xfs_buf_log_item_t *bip;
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
> -#endif
>
> if (tp == NULL) {
> ASSERT(bp->b_transp == NULL);
> libxfs_putbuf(bp);
> return;
> }
> +
> + trace_xfs_trans_brelse(bip);
> ASSERT(bp->b_transp == tp);
> bip = bp->b_log_item;
> ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
> @@ -536,13 +519,12 @@ xfs_trans_binval(
> xfs_buf_t *bp)
> {
> xfs_buf_log_item_t *bip = bp->b_log_item;
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
> -#endif
>
> ASSERT(bp->b_transp == tp);
> ASSERT(bip != NULL);
>
> + trace_xfs_trans_binval(bip);
> +
> if (bip->bli_flags & XFS_BLI_STALE)
> return;
> XFS_BUF_UNDELAYWRITE(bp);
> @@ -563,14 +545,12 @@ xfs_trans_bjoin(
> xfs_buf_log_item_t *bip;
>
> ASSERT(bp->b_transp == NULL);
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
> -#endif
>
> xfs_buf_item_init(bp, tp->t_mountp);
> bip = bp->b_log_item;
> xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> bp->b_transp = tp;
> + trace_xfs_trans_bjoin(bp->b_log_item);
> }
>
> void
> @@ -582,11 +562,9 @@ xfs_trans_bhold(
>
> ASSERT(bp->b_transp == tp);
> ASSERT(bip != NULL);
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
> -#endif
>
> bip->bli_flags |= XFS_BLI_HOLD;
> + trace_xfs_trans_bhold(bip);
> }
>
> xfs_buf_t *
> @@ -615,13 +593,11 @@ xfs_trans_get_buf_map(
> bp = libxfs_getbuf_map(btp, map, nmaps, 0);
> if (bp == NULL)
> return NULL;
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
> -#endif
>
> xfs_trans_bjoin(tp, bp);
> bip = bp->b_log_item;
> bip->bli_recur = 0;
> + trace_xfs_trans_get_buf(bp->b_log_item);
> return bp;
> }
>
> @@ -644,17 +620,16 @@ xfs_trans_getsb(
> bip = bp->b_log_item;
> ASSERT(bip != NULL);
> bip->bli_recur++;
> + trace_xfs_trans_getsb_recur(bip);
> return bp;
> }
>
> bp = xfs_getsb(mp);
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
> -#endif
>
> xfs_trans_bjoin(tp, bp);
> bip = bp->b_log_item;
> bip->bli_recur = 0;
> + trace_xfs_trans_getsb(bp->b_log_item);
> return bp;
> }
>
> @@ -691,6 +666,7 @@ xfs_trans_read_buf_map(
> ASSERT(bp->b_log_item != NULL);
> bip = bp->b_log_item;
> bip->bli_recur++;
> + trace_xfs_trans_read_buf_recur(bip);
> goto done;
> }
>
> @@ -701,14 +677,11 @@ xfs_trans_read_buf_map(
> if (bp->b_error)
> goto out_relse;
>
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
> -#endif
> -
> xfs_trans_bjoin(tp, bp);
> bip = bp->b_log_item;
> bip->bli_recur = 0;
> done:
> + trace_xfs_trans_read_buf(bp->b_log_item);
> *bpp = bp;
> return 0;
> out_relse:
> @@ -824,10 +797,6 @@ inode_item_done(
> }
>
> libxfs_writebuf(bp, 0);
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
> - ip->i_ino, bp);
> -#endif
> free:
> xfs_inode_item_put(iip);
> }
> @@ -845,13 +814,8 @@ buf_item_done(
> bp->b_transp = NULL; /* remove xact ptr */
>
> hold = (bip->bli_flags & XFS_BLI_HOLD);
> - if (bip->bli_flags & XFS_BLI_DIRTY) {
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "flushing/staling buffer %p (hold=%d)\n",
> - bp, hold);
> -#endif
> + if (bip->bli_flags & XFS_BLI_DIRTY)
> libxfs_writebuf_int(bp, 0);
> - }
>
> bip->bli_flags &= ~XFS_BLI_HOLD;
> xfs_buf_item_put(bip);
> @@ -937,6 +901,8 @@ __xfs_trans_commit(
> struct xfs_sb *sbp;
> int error = 0;
>
> + trace_xfs_trans_commit(tp, _RET_IP_);
> +
> if (tp == NULL)
> return 0;
>
> @@ -952,12 +918,8 @@ __xfs_trans_commit(
> goto out_unreserve;
> }
>
> - if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "committed clean transaction %p\n", tp);
> -#endif
> + if (!(tp->t_flags & XFS_TRANS_DIRTY))
> goto out_unreserve;
> - }
>
> if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
> sbp = &(tp->t_mountp->m_sb);
> @@ -972,9 +934,6 @@ __xfs_trans_commit(
> xfs_log_sb(tp);
> }
>
> -#ifdef XACT_DEBUG
> - fprintf(stderr, "committing dirty transaction %p\n", tp);
> -#endif
> trans_committed(tp);
>
> /* That's it for the transaction structure. Free it. */
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs
2019-05-17 21:36 ` Allison Collins
@ 2019-05-20 22:53 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-20 22:53 UTC (permalink / raw)
To: Allison Collins; +Cc: Eric Sandeen, linux-xfs
On Fri, May 17, 2019 at 02:36:49PM -0700, Allison Collins wrote:
> On 5/16/19 1:38 PM, Eric Sandeen wrote:
> > Remove XACT_DEBUG #ifdefs to reduce more cosmetic differences
> > between userspace & kernelspace libxfs. Add in some corresponding
> > (stubbed-out) tracepoint calls.
> >
> > If these are felt to be particularly useful, the tracepoint calls
> > could be fleshed out to provide similar information.
I've occasionally wondered this myself.
> > Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>
> Looks ok. It seems a shame to loose the extra tracing, but I confess most
> of the time when I'm chasing a particular bug I end up adding my own
> temporary traces anyway. So I think trying to synchronize the code spaces
> is more important. Do you think the stubs might be something to add to the
> kernel space side too?
I think most of them already exist in the kernel...
--D
> Allison
>
> > ---
> > include/xfs_trace.h | 13 +++++++
> > libxfs/trans.c | 85 ++++++++++++---------------------------------
> > 2 files changed, 35 insertions(+), 63 deletions(-)
> >
> > diff --git a/include/xfs_trace.h b/include/xfs_trace.h
> > index 793ac56e..43720040 100644
> > --- a/include/xfs_trace.h
> > +++ b/include/xfs_trace.h
> > @@ -161,6 +161,19 @@
> > #define trace_xfs_perag_get_tag(a,b,c,d) ((c) = (c))
> > #define trace_xfs_perag_put(a,b,c,d) ((c) = (c))
> > +#define trace_xfs_trans_alloc(a,b) ((void) 0)
> > +#define trace_xfs_trans_cancel(a,b) ((void) 0)
> > +#define trace_xfs_trans_brelse(a) ((void) 0)
> > +#define trace_xfs_trans_binval(a) ((void) 0)
> > +#define trace_xfs_trans_bjoin(a) ((void) 0)
> > +#define trace_xfs_trans_bhold(a) ((void) 0)
> > +#define trace_xfs_trans_get_buf(a) ((void) 0)
> > +#define trace_xfs_trans_getsb_recur(a) ((void) 0)
> > +#define trace_xfs_trans_getsb(a) ((void) 0)
> > +#define trace_xfs_trans_read_buf_recur(a) ((void) 0)
> > +#define trace_xfs_trans_read_buf(a) ((void) 0)
> > +#define trace_xfs_trans_commit(a,b) ((void) 0)
> > +
> > #define trace_xfs_defer_cancel(a,b) ((void) 0)
> > #define trace_xfs_defer_pending_commit(a,b) ((void) 0)
> > #define trace_xfs_defer_pending_abort(a,b) ((void) 0)
> > diff --git a/libxfs/trans.c b/libxfs/trans.c
> > index 9e49def0..6967a1de 100644
> > --- a/libxfs/trans.c
> > +++ b/libxfs/trans.c
> > @@ -18,6 +18,7 @@
> > #include "xfs_trans.h"
> > #include "xfs_sb.h"
> > #include "xfs_defer.h"
> > +#include "xfs_trace.h"
> > static void xfs_trans_free_items(struct xfs_trans *tp);
> > STATIC struct xfs_trans *xfs_trans_dup(struct xfs_trans *tp);
> > @@ -269,9 +270,9 @@ xfs_trans_alloc(
> > xfs_trans_cancel(tp);
> > return error;
> > }
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "allocated new transaction %p\n", tp);
> > -#endif
> > +
> > + trace_xfs_trans_alloc(tp, _RET_IP_);
> > +
> > *tpp = tp;
> > return 0;
> > }
> > @@ -317,23 +318,16 @@ void
> > xfs_trans_cancel(
> > struct xfs_trans *tp)
> > {
> > -#ifdef XACT_DEBUG
> > - struct xfs_trans *otp = tp;
> > -#endif
> > + trace_xfs_trans_cancel(tp, _RET_IP_);
> > +
> > if (tp == NULL)
> > - goto out;
> > + return;
> > if (tp->t_flags & XFS_TRANS_PERM_LOG_RES)
> > xfs_defer_cancel(tp);
> > xfs_trans_free_items(tp);
> > xfs_trans_free(tp);
> > -
> > -out:
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "## cancelled transaction %p\n", otp);
> > -#endif
> > - return;
> > }
> > void
> > @@ -353,10 +347,6 @@ xfs_trans_ijoin(
> > iip->ili_lock_flags = lock_flags;
> > xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
> > -
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "ijoin'd inode %llu, transaction %p\n", ip->i_ino, tp);
> > -#endif
> > }
> > void
> > @@ -388,9 +378,6 @@ xfs_trans_log_inode(
> > uint flags)
> > {
> > ASSERT(ip->i_itemp != NULL);
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "dirtied inode %llu, transaction %p\n", ip->i_ino, tp);
> > -#endif
> > tp->t_flags |= XFS_TRANS_DIRTY;
> > set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
> > @@ -434,9 +421,6 @@ xfs_trans_dirty_buf(
> > ASSERT(bp->b_transp == tp);
> > ASSERT(bip != NULL);
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "dirtied buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > tp->t_flags |= XFS_TRANS_DIRTY;
> > set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
> > }
> > @@ -501,15 +485,14 @@ xfs_trans_brelse(
> > xfs_buf_t *bp)
> > {
> > xfs_buf_log_item_t *bip;
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "released buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > if (tp == NULL) {
> > ASSERT(bp->b_transp == NULL);
> > libxfs_putbuf(bp);
> > return;
> > }
> > +
> > + trace_xfs_trans_brelse(bip);
> > ASSERT(bp->b_transp == tp);
> > bip = bp->b_log_item;
> > ASSERT(bip->bli_item.li_type == XFS_LI_BUF);
> > @@ -536,13 +519,12 @@ xfs_trans_binval(
> > xfs_buf_t *bp)
> > {
> > xfs_buf_log_item_t *bip = bp->b_log_item;
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "binval'd buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > ASSERT(bp->b_transp == tp);
> > ASSERT(bip != NULL);
> > + trace_xfs_trans_binval(bip);
> > +
> > if (bip->bli_flags & XFS_BLI_STALE)
> > return;
> > XFS_BUF_UNDELAYWRITE(bp);
> > @@ -563,14 +545,12 @@ xfs_trans_bjoin(
> > xfs_buf_log_item_t *bip;
> > ASSERT(bp->b_transp == NULL);
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "bjoin'd buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > xfs_buf_item_init(bp, tp->t_mountp);
> > bip = bp->b_log_item;
> > xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> > bp->b_transp = tp;
> > + trace_xfs_trans_bjoin(bp->b_log_item);
> > }
> > void
> > @@ -582,11 +562,9 @@ xfs_trans_bhold(
> > ASSERT(bp->b_transp == tp);
> > ASSERT(bip != NULL);
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "bhold'd buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > bip->bli_flags |= XFS_BLI_HOLD;
> > + trace_xfs_trans_bhold(bip);
> > }
> > xfs_buf_t *
> > @@ -615,13 +593,11 @@ xfs_trans_get_buf_map(
> > bp = libxfs_getbuf_map(btp, map, nmaps, 0);
> > if (bp == NULL)
> > return NULL;
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "trans_get_buf buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > xfs_trans_bjoin(tp, bp);
> > bip = bp->b_log_item;
> > bip->bli_recur = 0;
> > + trace_xfs_trans_get_buf(bp->b_log_item);
> > return bp;
> > }
> > @@ -644,17 +620,16 @@ xfs_trans_getsb(
> > bip = bp->b_log_item;
> > ASSERT(bip != NULL);
> > bip->bli_recur++;
> > + trace_xfs_trans_getsb_recur(bip);
> > return bp;
> > }
> > bp = xfs_getsb(mp);
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "trans_get_sb buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > xfs_trans_bjoin(tp, bp);
> > bip = bp->b_log_item;
> > bip->bli_recur = 0;
> > + trace_xfs_trans_getsb(bp->b_log_item);
> > return bp;
> > }
> > @@ -691,6 +666,7 @@ xfs_trans_read_buf_map(
> > ASSERT(bp->b_log_item != NULL);
> > bip = bp->b_log_item;
> > bip->bli_recur++;
> > + trace_xfs_trans_read_buf_recur(bip);
> > goto done;
> > }
> > @@ -701,14 +677,11 @@ xfs_trans_read_buf_map(
> > if (bp->b_error)
> > goto out_relse;
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "trans_read_buf buffer %p, transaction %p\n", bp, tp);
> > -#endif
> > -
> > xfs_trans_bjoin(tp, bp);
> > bip = bp->b_log_item;
> > bip->bli_recur = 0;
> > done:
> > + trace_xfs_trans_read_buf(bp->b_log_item);
> > *bpp = bp;
> > return 0;
> > out_relse:
> > @@ -824,10 +797,6 @@ inode_item_done(
> > }
> > libxfs_writebuf(bp, 0);
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "flushing dirty inode %llu, buffer %p\n",
> > - ip->i_ino, bp);
> > -#endif
> > free:
> > xfs_inode_item_put(iip);
> > }
> > @@ -845,13 +814,8 @@ buf_item_done(
> > bp->b_transp = NULL; /* remove xact ptr */
> > hold = (bip->bli_flags & XFS_BLI_HOLD);
> > - if (bip->bli_flags & XFS_BLI_DIRTY) {
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "flushing/staling buffer %p (hold=%d)\n",
> > - bp, hold);
> > -#endif
> > + if (bip->bli_flags & XFS_BLI_DIRTY)
> > libxfs_writebuf_int(bp, 0);
> > - }
> > bip->bli_flags &= ~XFS_BLI_HOLD;
> > xfs_buf_item_put(bip);
> > @@ -937,6 +901,8 @@ __xfs_trans_commit(
> > struct xfs_sb *sbp;
> > int error = 0;
> > + trace_xfs_trans_commit(tp, _RET_IP_);
> > +
> > if (tp == NULL)
> > return 0;
> > @@ -952,12 +918,8 @@ __xfs_trans_commit(
> > goto out_unreserve;
> > }
> > - if (!(tp->t_flags & XFS_TRANS_DIRTY)) {
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "committed clean transaction %p\n", tp);
> > -#endif
> > + if (!(tp->t_flags & XFS_TRANS_DIRTY))
> > goto out_unreserve;
> > - }
> > if (tp->t_flags & XFS_TRANS_SB_DIRTY) {
> > sbp = &(tp->t_mountp->m_sb);
> > @@ -972,9 +934,6 @@ __xfs_trans_commit(
> > xfs_log_sb(tp);
> > }
> > -#ifdef XACT_DEBUG
> > - fprintf(stderr, "committing dirty transaction %p\n", tp);
> > -#endif
> > trans_committed(tp);
> > /* That's it for the transaction structure. Free it. */
> >
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (3 preceding siblings ...)
2019-05-16 20:38 ` [PATCH 4/3] libxfs: Remove XACT_DEBUG #ifdefs Eric Sandeen
@ 2019-05-16 20:39 ` Eric Sandeen
2019-05-17 22:29 ` Allison Collins
2019-05-16 20:39 ` [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code Eric Sandeen
` (5 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 20:39 UTC (permalink / raw)
To: linux-xfs
Rename the bli_format structure to __bli_format to avoid
accidently confusing them with the bli_formats pointer.
(nb: userspace currently has no bli_formats pointer)
Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
include/xfs_trans.h | 2 +-
libxfs/logitem.c | 6 +++---
libxfs/trans.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index 953da5d1..fe03ba64 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
struct xfs_buf *bli_buf; /* real buffer pointer */
unsigned int bli_flags; /* misc flags */
unsigned int bli_recur; /* recursion count */
- xfs_buf_log_format_t bli_format; /* in-log header */
+ xfs_buf_log_format_t __bli_format; /* in-log header */
} xfs_buf_log_item_t;
#define XFS_BLI_DIRTY (1<<0)
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index 4da9bc1b..e862ab4f 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -107,9 +107,9 @@ xfs_buf_item_init(
bip->bli_item.li_mountp = mp;
INIT_LIST_HEAD(&bip->bli_item.li_trans);
bip->bli_buf = bp;
- bip->bli_format.blf_type = XFS_LI_BUF;
- bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
- bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
+ bip->__bli_format.blf_type = XFS_LI_BUF;
+ bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
+ bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
bp->b_log_item = bip;
}
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 6967a1de..f3c28fa7 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -531,8 +531,8 @@ xfs_trans_binval(
xfs_buf_stale(bp);
bip->bli_flags |= XFS_BLI_STALE;
bip->bli_flags &= ~XFS_BLI_DIRTY;
- bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
- bip->bli_format.blf_flags |= XFS_BLF_CANCEL;
+ bip->__bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
+ bip->__bli_format.blf_flags |= XFS_BLF_CANCEL;
set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
tp->t_flags |= XFS_TRANS_DIRTY;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats
2019-05-16 20:39 ` [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats Eric Sandeen
@ 2019-05-17 22:29 ` Allison Collins
2019-05-17 23:01 ` Eric Sandeen
0 siblings, 1 reply; 29+ messages in thread
From: Allison Collins @ 2019-05-17 22:29 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
On 5/16/19 1:39 PM, Eric Sandeen wrote:
> Rename the bli_format structure to __bli_format to avoid
> accidently confusing them with the bli_formats pointer.
>
> (nb: userspace currently has no bli_formats pointer)
>
> Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> include/xfs_trans.h | 2 +-
> libxfs/logitem.c | 6 +++---
> libxfs/trans.c | 4 ++--
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index 953da5d1..fe03ba64 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
> struct xfs_buf *bli_buf; /* real buffer pointer */
> unsigned int bli_flags; /* misc flags */
> unsigned int bli_recur; /* recursion count */
> - xfs_buf_log_format_t bli_format; /* in-log header */
> + xfs_buf_log_format_t __bli_format; /* in-log header */
> } xfs_buf_log_item_t;
>
> #define XFS_BLI_DIRTY (1<<0)
> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
> index 4da9bc1b..e862ab4f 100644
> --- a/libxfs/logitem.c
> +++ b/libxfs/logitem.c
> @@ -107,9 +107,9 @@ xfs_buf_item_init(
> bip->bli_item.li_mountp = mp;
> INIT_LIST_HEAD(&bip->bli_item.li_trans);
> bip->bli_buf = bp;
> - bip->bli_format.blf_type = XFS_LI_BUF;
> - bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
> - bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
> + bip->__bli_format.blf_type = XFS_LI_BUF;
> + bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
> + bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
> bp->b_log_item = bip;
I had a look around this area of code, and I see where the bli_format is
getting referenced, but I don't see a bli_formats. So I feel like I'm
missing the motivation for the change. Did I miss the bli_formats
somewhere? Thanks!
Allison
> }
>
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 6967a1de..f3c28fa7 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -531,8 +531,8 @@ xfs_trans_binval(
> xfs_buf_stale(bp);
> bip->bli_flags |= XFS_BLI_STALE;
> bip->bli_flags &= ~XFS_BLI_DIRTY;
> - bip->bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
> - bip->bli_format.blf_flags |= XFS_BLF_CANCEL;
> + bip->__bli_format.blf_flags &= ~XFS_BLF_INODE_BUF;
> + bip->__bli_format.blf_flags |= XFS_BLF_CANCEL;
> set_bit(XFS_LI_DIRTY, &bip->bli_item.li_flags);
> tp->t_flags |= XFS_TRANS_DIRTY;
> }
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats
2019-05-17 22:29 ` Allison Collins
@ 2019-05-17 23:01 ` Eric Sandeen
2019-05-17 23:41 ` Allison Collins
0 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-17 23:01 UTC (permalink / raw)
To: Allison Collins, Eric Sandeen, linux-xfs
On 5/17/19 5:29 PM, Allison Collins wrote:
> On 5/16/19 1:39 PM, Eric Sandeen wrote:
>> Rename the bli_format structure to __bli_format to avoid
>> accidently confusing them with the bli_formats pointer.
>>
>> (nb: userspace currently has no bli_formats pointer)
>>
>> Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>> include/xfs_trans.h | 2 +-
>> libxfs/logitem.c | 6 +++---
>> libxfs/trans.c | 4 ++--
>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
>> index 953da5d1..fe03ba64 100644
>> --- a/include/xfs_trans.h
>> +++ b/include/xfs_trans.h
>> @@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
>> struct xfs_buf *bli_buf; /* real buffer pointer */
>> unsigned int bli_flags; /* misc flags */
>> unsigned int bli_recur; /* recursion count */
>> - xfs_buf_log_format_t bli_format; /* in-log header */
>> + xfs_buf_log_format_t __bli_format; /* in-log header */
>> } xfs_buf_log_item_t;
>> #define XFS_BLI_DIRTY (1<<0)
>> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
>> index 4da9bc1b..e862ab4f 100644
>> --- a/libxfs/logitem.c
>> +++ b/libxfs/logitem.c
>> @@ -107,9 +107,9 @@ xfs_buf_item_init(
>> bip->bli_item.li_mountp = mp;
>> INIT_LIST_HEAD(&bip->bli_item.li_trans);
>> bip->bli_buf = bp;
>> - bip->bli_format.blf_type = XFS_LI_BUF;
>> - bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>> - bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>> + bip->__bli_format.blf_type = XFS_LI_BUF;
>> + bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>> + bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>> bp->b_log_item = bip;
>
> I had a look around this area of code, and I see where the bli_format is getting referenced, but I don't see a bli_formats. So I feel like I'm missing the motivation for the change. Did I miss the bli_formats somewhere? Thanks!
see above :)
> (nb: userspace currently has no bli_formats pointer)
(I guess copying the kernel commit log added confusion even w/ the note)
-Eric
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats
2019-05-17 23:01 ` Eric Sandeen
@ 2019-05-17 23:41 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-05-17 23:41 UTC (permalink / raw)
To: Eric Sandeen, Eric Sandeen, linux-xfs
On 5/17/19 4:01 PM, Eric Sandeen wrote:
> On 5/17/19 5:29 PM, Allison Collins wrote:
>> On 5/16/19 1:39 PM, Eric Sandeen wrote:
>>> Rename the bli_format structure to __bli_format to avoid
>>> accidently confusing them with the bli_formats pointer.
>>>
>>> (nb: userspace currently has no bli_formats pointer)
>>>
>>> Source kernel commit: b94381737e9c4d014a4003e8ece9ba88670a2dd4
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>> include/xfs_trans.h | 2 +-
>>> libxfs/logitem.c | 6 +++---
>>> libxfs/trans.c | 4 ++--
>>> 3 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
>>> index 953da5d1..fe03ba64 100644
>>> --- a/include/xfs_trans.h
>>> +++ b/include/xfs_trans.h
>>> @@ -39,7 +39,7 @@ typedef struct xfs_buf_log_item {
>>> struct xfs_buf *bli_buf; /* real buffer pointer */
>>> unsigned int bli_flags; /* misc flags */
>>> unsigned int bli_recur; /* recursion count */
>>> - xfs_buf_log_format_t bli_format; /* in-log header */
>>> + xfs_buf_log_format_t __bli_format; /* in-log header */
>>> } xfs_buf_log_item_t;
>>> #define XFS_BLI_DIRTY (1<<0)
>>> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
>>> index 4da9bc1b..e862ab4f 100644
>>> --- a/libxfs/logitem.c
>>> +++ b/libxfs/logitem.c
>>> @@ -107,9 +107,9 @@ xfs_buf_item_init(
>>> bip->bli_item.li_mountp = mp;
>>> INIT_LIST_HEAD(&bip->bli_item.li_trans);
>>> bip->bli_buf = bp;
>>> - bip->bli_format.blf_type = XFS_LI_BUF;
>>> - bip->bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>>> - bip->bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>>> + bip->__bli_format.blf_type = XFS_LI_BUF;
>>> + bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
>>> + bip->__bli_format.blf_len = (unsigned short)BTOBB(bp->b_bcount);
>>> bp->b_log_item = bip;
>>
>> I had a look around this area of code, and I see where the bli_format is getting referenced, but I don't see a bli_formats. So I feel like I'm missing the motivation for the change. Did I miss the bli_formats somewhere? Thanks!
>
> see above :)
>
>> (nb: userspace currently has no bli_formats pointer)
>
> (I guess copying the kernel commit log added confusion even w/ the note)
>
> -Eric
Oh I see. No I think it's ok, I overlooked it. You can add my review :-)
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (4 preceding siblings ...)
2019-05-16 20:39 ` [PATCH 5/3] libxfs: rename bli_format to avoid confusion with bli_formats Eric Sandeen
@ 2019-05-16 20:39 ` Eric Sandeen
2019-05-17 22:56 ` Allison Collins
2019-05-20 22:56 ` Darrick J. Wong
2019-05-16 20:40 ` [PATCH 7/3] libxfs: fix argument to xfs_trans_add_item Eric Sandeen
` (4 subsequent siblings)
10 siblings, 2 replies; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 20:39 UTC (permalink / raw)
To: linux-xfs
Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
xfs_trans_getsb and xfs_trans_read_buf_map. Add a new
_xfs_trans_bjoin which can be called by all three functions.
Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Alex Elder <aelder@sgi.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 39 insertions(+), 14 deletions(-)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index f3c28fa7..f78222fd 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -537,19 +537,50 @@ xfs_trans_binval(
tp->t_flags |= XFS_TRANS_DIRTY;
}
-void
-xfs_trans_bjoin(
- xfs_trans_t *tp,
- xfs_buf_t *bp)
+/*
+ * Add the locked buffer to the transaction.
+ *
+ * The buffer must be locked, and it cannot be associated with any
+ * transaction.
+ *
+ * If the buffer does not yet have a buf log item associated with it,
+ * then allocate one for it. Then add the buf item to the transaction.
+ */
+STATIC void
+_xfs_trans_bjoin(
+ struct xfs_trans *tp,
+ struct xfs_buf *bp,
+ int reset_recur)
{
- xfs_buf_log_item_t *bip;
+ struct xfs_buf_log_item *bip;
ASSERT(bp->b_transp == NULL);
+ /*
+ * The xfs_buf_log_item pointer is stored in b_log_item. If
+ * it doesn't have one yet, then allocate one and initialize it.
+ * The checks to see if one is there are in xfs_buf_item_init().
+ */
xfs_buf_item_init(bp, tp->t_mountp);
bip = bp->b_log_item;
+ if (reset_recur)
+ bip->bli_recur = 0;
+
+ /*
+ * Attach the item to the transaction so we can find it in
+ * xfs_trans_get_buf() and friends.
+ */
xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
bp->b_transp = tp;
+
+}
+
+void
+xfs_trans_bjoin(
+ struct xfs_trans *tp,
+ struct xfs_buf *bp)
+{
+ _xfs_trans_bjoin(tp, bp, 0);
trace_xfs_trans_bjoin(bp->b_log_item);
}
@@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
if (bp == NULL)
return NULL;
- xfs_trans_bjoin(tp, bp);
- bip = bp->b_log_item;
- bip->bli_recur = 0;
+ _xfs_trans_bjoin(tp, bp, 1);
trace_xfs_trans_get_buf(bp->b_log_item);
return bp;
}
@@ -626,9 +655,7 @@ xfs_trans_getsb(
bp = xfs_getsb(mp);
- xfs_trans_bjoin(tp, bp);
- bip = bp->b_log_item;
- bip->bli_recur = 0;
+ _xfs_trans_bjoin(tp, bp, 1);
trace_xfs_trans_getsb(bp->b_log_item);
return bp;
}
@@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
if (bp->b_error)
goto out_relse;
- xfs_trans_bjoin(tp, bp);
- bip = bp->b_log_item;
- bip->bli_recur = 0;
+ _xfs_trans_bjoin(tp, bp, 1);
done:
trace_xfs_trans_read_buf(bp->b_log_item);
*bpp = bp;
--
2.17.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code
2019-05-16 20:39 ` [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code Eric Sandeen
@ 2019-05-17 22:56 ` Allison Collins
2019-05-20 22:56 ` Darrick J. Wong
1 sibling, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-05-17 22:56 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
On 5/16/19 1:39 PM, Eric Sandeen wrote:
> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
> xfs_trans_getsb and xfs_trans_read_buf_map. Add a new
> _xfs_trans_bjoin which can be called by all three functions.
>
> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Alex Elder <aelder@sgi.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks ok to me
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> ---
> libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index f3c28fa7..f78222fd 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -537,19 +537,50 @@ xfs_trans_binval(
> tp->t_flags |= XFS_TRANS_DIRTY;
> }
>
> -void
> -xfs_trans_bjoin(
> - xfs_trans_t *tp,
> - xfs_buf_t *bp)
> +/*
> + * Add the locked buffer to the transaction.
> + *
> + * The buffer must be locked, and it cannot be associated with any
> + * transaction.
> + *
> + * If the buffer does not yet have a buf log item associated with it,
> + * then allocate one for it. Then add the buf item to the transaction.
> + */
> +STATIC void
> +_xfs_trans_bjoin(
> + struct xfs_trans *tp,
> + struct xfs_buf *bp,
> + int reset_recur)
> {
> - xfs_buf_log_item_t *bip;
> + struct xfs_buf_log_item *bip;
>
> ASSERT(bp->b_transp == NULL);
>
> + /*
> + * The xfs_buf_log_item pointer is stored in b_log_item. If
> + * it doesn't have one yet, then allocate one and initialize it.
> + * The checks to see if one is there are in xfs_buf_item_init().
> + */
> xfs_buf_item_init(bp, tp->t_mountp);
> bip = bp->b_log_item;
> + if (reset_recur)
> + bip->bli_recur = 0;
> +
> + /*
> + * Attach the item to the transaction so we can find it in
> + * xfs_trans_get_buf() and friends.
> + */
> xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> bp->b_transp = tp;
> +
> +}
> +
> +void
> +xfs_trans_bjoin(
> + struct xfs_trans *tp,
> + struct xfs_buf *bp)
> +{
> + _xfs_trans_bjoin(tp, bp, 0);
> trace_xfs_trans_bjoin(bp->b_log_item);
> }
>
> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
> if (bp == NULL)
> return NULL;
>
> - xfs_trans_bjoin(tp, bp);
> - bip = bp->b_log_item;
> - bip->bli_recur = 0;
> + _xfs_trans_bjoin(tp, bp, 1);
> trace_xfs_trans_get_buf(bp->b_log_item);
> return bp;
> }
> @@ -626,9 +655,7 @@ xfs_trans_getsb(
>
> bp = xfs_getsb(mp);
>
> - xfs_trans_bjoin(tp, bp);
> - bip = bp->b_log_item;
> - bip->bli_recur = 0;
> + _xfs_trans_bjoin(tp, bp, 1);
> trace_xfs_trans_getsb(bp->b_log_item);
> return bp;
> }
> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
> if (bp->b_error)
> goto out_relse;
>
> - xfs_trans_bjoin(tp, bp);
> - bip = bp->b_log_item;
> - bip->bli_recur = 0;
> + _xfs_trans_bjoin(tp, bp, 1);
> done:
> trace_xfs_trans_read_buf(bp->b_log_item);
> *bpp = bp;
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code
2019-05-16 20:39 ` [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code Eric Sandeen
2019-05-17 22:56 ` Allison Collins
@ 2019-05-20 22:56 ` Darrick J. Wong
2019-05-20 22:58 ` Eric Sandeen
1 sibling, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-20 22:56 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Thu, May 16, 2019 at 03:39:49PM -0500, Eric Sandeen wrote:
> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
> xfs_trans_getsb and xfs_trans_read_buf_map. Add a new
> _xfs_trans_bjoin which can be called by all three functions.
>
> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Alex Elder <aelder@sgi.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 39 insertions(+), 14 deletions(-)
>
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index f3c28fa7..f78222fd 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -537,19 +537,50 @@ xfs_trans_binval(
> tp->t_flags |= XFS_TRANS_DIRTY;
> }
>
> -void
> -xfs_trans_bjoin(
> - xfs_trans_t *tp,
> - xfs_buf_t *bp)
> +/*
> + * Add the locked buffer to the transaction.
> + *
> + * The buffer must be locked, and it cannot be associated with any
> + * transaction.
> + *
> + * If the buffer does not yet have a buf log item associated with it,
> + * then allocate one for it. Then add the buf item to the transaction.
> + */
> +STATIC void
> +_xfs_trans_bjoin(
> + struct xfs_trans *tp,
> + struct xfs_buf *bp,
> + int reset_recur)
bool?
> {
> - xfs_buf_log_item_t *bip;
> + struct xfs_buf_log_item *bip;
>
> ASSERT(bp->b_transp == NULL);
>
> + /*
> + * The xfs_buf_log_item pointer is stored in b_log_item. If
> + * it doesn't have one yet, then allocate one and initialize it.
> + * The checks to see if one is there are in xfs_buf_item_init().
> + */
> xfs_buf_item_init(bp, tp->t_mountp);
> bip = bp->b_log_item;
> + if (reset_recur)
> + bip->bli_recur = 0;
> +
> + /*
> + * Attach the item to the transaction so we can find it in
> + * xfs_trans_get_buf() and friends.
> + */
> xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
Kill typedef here ^^^^^^?
--D
> bp->b_transp = tp;
> +
> +}
> +
> +void
> +xfs_trans_bjoin(
> + struct xfs_trans *tp,
> + struct xfs_buf *bp)
> +{
> + _xfs_trans_bjoin(tp, bp, 0);
> trace_xfs_trans_bjoin(bp->b_log_item);
> }
>
> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
> if (bp == NULL)
> return NULL;
>
> - xfs_trans_bjoin(tp, bp);
> - bip = bp->b_log_item;
> - bip->bli_recur = 0;
> + _xfs_trans_bjoin(tp, bp, 1);
> trace_xfs_trans_get_buf(bp->b_log_item);
> return bp;
> }
> @@ -626,9 +655,7 @@ xfs_trans_getsb(
>
> bp = xfs_getsb(mp);
>
> - xfs_trans_bjoin(tp, bp);
> - bip = bp->b_log_item;
> - bip->bli_recur = 0;
> + _xfs_trans_bjoin(tp, bp, 1);
> trace_xfs_trans_getsb(bp->b_log_item);
> return bp;
> }
> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
> if (bp->b_error)
> goto out_relse;
>
> - xfs_trans_bjoin(tp, bp);
> - bip = bp->b_log_item;
> - bip->bli_recur = 0;
> + _xfs_trans_bjoin(tp, bp, 1);
> done:
> trace_xfs_trans_read_buf(bp->b_log_item);
> *bpp = bp;
> --
> 2.17.0
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code
2019-05-20 22:56 ` Darrick J. Wong
@ 2019-05-20 22:58 ` Eric Sandeen
2019-05-20 23:12 ` Darrick J. Wong
0 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-20 22:58 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: linux-xfs
On 5/20/19 5:56 PM, Darrick J. Wong wrote:
> On Thu, May 16, 2019 at 03:39:49PM -0500, Eric Sandeen wrote:
>> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
>> xfs_trans_getsb and xfs_trans_read_buf_map. Add a new
>> _xfs_trans_bjoin which can be called by all three functions.
>>
>> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
>>
>> Signed-off-by: Christoph Hellwig <hch@lst.de>
>> Reviewed-by: Dave Chinner <david@fromorbit.com>
>> Signed-off-by: Alex Elder <aelder@sgi.com>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>> libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
>> 1 file changed, 39 insertions(+), 14 deletions(-)
>>
>> diff --git a/libxfs/trans.c b/libxfs/trans.c
>> index f3c28fa7..f78222fd 100644
>> --- a/libxfs/trans.c
>> +++ b/libxfs/trans.c
>> @@ -537,19 +537,50 @@ xfs_trans_binval(
>> tp->t_flags |= XFS_TRANS_DIRTY;
>> }
>>
>> -void
>> -xfs_trans_bjoin(
>> - xfs_trans_t *tp,
>> - xfs_buf_t *bp)
>> +/*
>> + * Add the locked buffer to the transaction.
>> + *
>> + * The buffer must be locked, and it cannot be associated with any
>> + * transaction.
>> + *
>> + * If the buffer does not yet have a buf log item associated with it,
>> + * then allocate one for it. Then add the buf item to the transaction.
>> + */
>> +STATIC void
>> +_xfs_trans_bjoin(
>> + struct xfs_trans *tp,
>> + struct xfs_buf *bp,
>> + int reset_recur)
>
> bool?
If you fix it in the kernel I'll merge it to xfsprogs ;)
>> {
>> - xfs_buf_log_item_t *bip;
>> + struct xfs_buf_log_item *bip;
>>
>> ASSERT(bp->b_transp == NULL);
>>
>> + /*
>> + * The xfs_buf_log_item pointer is stored in b_log_item. If
>> + * it doesn't have one yet, then allocate one and initialize it.
>> + * The checks to see if one is there are in xfs_buf_item_init().
>> + */
>> xfs_buf_item_init(bp, tp->t_mountp);
>> bip = bp->b_log_item;
>> + if (reset_recur)
>> + bip->bli_recur = 0;
>> +
>> + /*
>> + * Attach the item to the transaction so we can find it in
>> + * xfs_trans_get_buf() and friends.
>> + */
>> xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
>
> Kill typedef here ^^^^^^?
ditto.
Point of all this is to match the kernel for easier future syncups.
(oh wait, actually, this is fixed in a later patch, getting rid of the dumb
cast altogether)
-Eric
> --D
>
>> bp->b_transp = tp;
>> +
>> +}
>> +
>> +void
>> +xfs_trans_bjoin(
>> + struct xfs_trans *tp,
>> + struct xfs_buf *bp)
>> +{
>> + _xfs_trans_bjoin(tp, bp, 0);
>> trace_xfs_trans_bjoin(bp->b_log_item);
>> }
>>
>> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
>> if (bp == NULL)
>> return NULL;
>>
>> - xfs_trans_bjoin(tp, bp);
>> - bip = bp->b_log_item;
>> - bip->bli_recur = 0;
>> + _xfs_trans_bjoin(tp, bp, 1);
>> trace_xfs_trans_get_buf(bp->b_log_item);
>> return bp;
>> }
>> @@ -626,9 +655,7 @@ xfs_trans_getsb(
>>
>> bp = xfs_getsb(mp);
>>
>> - xfs_trans_bjoin(tp, bp);
>> - bip = bp->b_log_item;
>> - bip->bli_recur = 0;
>> + _xfs_trans_bjoin(tp, bp, 1);
>> trace_xfs_trans_getsb(bp->b_log_item);
>> return bp;
>> }
>> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
>> if (bp->b_error)
>> goto out_relse;
>>
>> - xfs_trans_bjoin(tp, bp);
>> - bip = bp->b_log_item;
>> - bip->bli_recur = 0;
>> + _xfs_trans_bjoin(tp, bp, 1);
>> done:
>> trace_xfs_trans_read_buf(bp->b_log_item);
>> *bpp = bp;
>> --
>> 2.17.0
>>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code
2019-05-20 22:58 ` Eric Sandeen
@ 2019-05-20 23:12 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:12 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Mon, May 20, 2019 at 05:58:29PM -0500, Eric Sandeen wrote:
> On 5/20/19 5:56 PM, Darrick J. Wong wrote:
> > On Thu, May 16, 2019 at 03:39:49PM -0500, Eric Sandeen wrote:
> >> Most of xfs_trans_bjoin is duplicated in xfs_trans_get_buf_map,
> >> xfs_trans_getsb and xfs_trans_read_buf_map. Add a new
> >> _xfs_trans_bjoin which can be called by all three functions.
> >>
> >> Source kernel commit: d7e84f413726876c0ec66bbf90770f69841f7663
> >>
> >> Signed-off-by: Christoph Hellwig <hch@lst.de>
> >> Reviewed-by: Dave Chinner <david@fromorbit.com>
> >> Signed-off-by: Alex Elder <aelder@sgi.com>
> >> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> >> ---
> >> libxfs/trans.c | 53 +++++++++++++++++++++++++++++++++++++-------------
> >> 1 file changed, 39 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/libxfs/trans.c b/libxfs/trans.c
> >> index f3c28fa7..f78222fd 100644
> >> --- a/libxfs/trans.c
> >> +++ b/libxfs/trans.c
> >> @@ -537,19 +537,50 @@ xfs_trans_binval(
> >> tp->t_flags |= XFS_TRANS_DIRTY;
> >> }
> >>
> >> -void
> >> -xfs_trans_bjoin(
> >> - xfs_trans_t *tp,
> >> - xfs_buf_t *bp)
> >> +/*
> >> + * Add the locked buffer to the transaction.
> >> + *
> >> + * The buffer must be locked, and it cannot be associated with any
> >> + * transaction.
> >> + *
> >> + * If the buffer does not yet have a buf log item associated with it,
> >> + * then allocate one for it. Then add the buf item to the transaction.
> >> + */
> >> +STATIC void
> >> +_xfs_trans_bjoin(
> >> + struct xfs_trans *tp,
> >> + struct xfs_buf *bp,
> >> + int reset_recur)
> >
> > bool?
>
> If you fix it in the kernel I'll merge it to xfsprogs ;)
LOL, ok.
--D
> >> {
> >> - xfs_buf_log_item_t *bip;
> >> + struct xfs_buf_log_item *bip;
> >>
> >> ASSERT(bp->b_transp == NULL);
> >>
> >> + /*
> >> + * The xfs_buf_log_item pointer is stored in b_log_item. If
> >> + * it doesn't have one yet, then allocate one and initialize it.
> >> + * The checks to see if one is there are in xfs_buf_item_init().
> >> + */
> >> xfs_buf_item_init(bp, tp->t_mountp);
> >> bip = bp->b_log_item;
> >> + if (reset_recur)
> >> + bip->bli_recur = 0;
> >> +
> >> + /*
> >> + * Attach the item to the transaction so we can find it in
> >> + * xfs_trans_get_buf() and friends.
> >> + */
> >> xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> >
> > Kill typedef here ^^^^^^?
>
> ditto.
>
> Point of all this is to match the kernel for easier future syncups.
>
> (oh wait, actually, this is fixed in a later patch, getting rid of the dumb
> cast altogether)
>
> -Eric
>
> > --D
> >
> >> bp->b_transp = tp;
> >> +
> >> +}
> >> +
> >> +void
> >> +xfs_trans_bjoin(
> >> + struct xfs_trans *tp,
> >> + struct xfs_buf *bp)
> >> +{
> >> + _xfs_trans_bjoin(tp, bp, 0);
> >> trace_xfs_trans_bjoin(bp->b_log_item);
> >> }
> >>
> >> @@ -594,9 +625,7 @@ xfs_trans_get_buf_map(
> >> if (bp == NULL)
> >> return NULL;
> >>
> >> - xfs_trans_bjoin(tp, bp);
> >> - bip = bp->b_log_item;
> >> - bip->bli_recur = 0;
> >> + _xfs_trans_bjoin(tp, bp, 1);
> >> trace_xfs_trans_get_buf(bp->b_log_item);
> >> return bp;
> >> }
> >> @@ -626,9 +655,7 @@ xfs_trans_getsb(
> >>
> >> bp = xfs_getsb(mp);
> >>
> >> - xfs_trans_bjoin(tp, bp);
> >> - bip = bp->b_log_item;
> >> - bip->bli_recur = 0;
> >> + _xfs_trans_bjoin(tp, bp, 1);
> >> trace_xfs_trans_getsb(bp->b_log_item);
> >> return bp;
> >> }
> >> @@ -677,9 +704,7 @@ xfs_trans_read_buf_map(
> >> if (bp->b_error)
> >> goto out_relse;
> >>
> >> - xfs_trans_bjoin(tp, bp);
> >> - bip = bp->b_log_item;
> >> - bip->bli_recur = 0;
> >> + _xfs_trans_bjoin(tp, bp, 1);
> >> done:
> >> trace_xfs_trans_read_buf(bp->b_log_item);
> >> *bpp = bp;
> >> --
> >> 2.17.0
> >>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 7/3] libxfs: fix argument to xfs_trans_add_item
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (5 preceding siblings ...)
2019-05-16 20:39 ` [PATCH 6/3] libxfs: factor common xfs_trans_bjoin code Eric Sandeen
@ 2019-05-16 20:40 ` Eric Sandeen
2019-05-17 22:57 ` Allison Collins
2019-05-16 20:41 ` [PATCH 8/3] xfs: factor log item initialisation Eric Sandeen
` (3 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 20:40 UTC (permalink / raw)
To: linux-xfs
The hack of casting an inode_log_item or buf_log_item to a
xfs_log_item_t is pretty gross; yes it's the first member in the
structure, but yuk. Pass in the correct structure member.
This was fixed in the kernel with commit e98c414f9
("xfs: simplify log item descriptor tracking")
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
libxfs/trans.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libxfs/trans.c b/libxfs/trans.c
index f78222fd..6ef4841f 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -346,7 +346,7 @@ xfs_trans_ijoin(
ASSERT(iip->ili_lock_flags == 0);
iip->ili_lock_flags = lock_flags;
- xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
+ xfs_trans_add_item(tp, &iip->ili_item);
}
void
@@ -570,7 +570,7 @@ _xfs_trans_bjoin(
* Attach the item to the transaction so we can find it in
* xfs_trans_get_buf() and friends.
*/
- xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
+ xfs_trans_add_item(tp, &bip->bli_item);
bp->b_transp = tp;
}
--
2.17.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 7/3] libxfs: fix argument to xfs_trans_add_item
2019-05-16 20:40 ` [PATCH 7/3] libxfs: fix argument to xfs_trans_add_item Eric Sandeen
@ 2019-05-17 22:57 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-05-17 22:57 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
On 5/16/19 1:40 PM, Eric Sandeen wrote:
> The hack of casting an inode_log_item or buf_log_item to a
> xfs_log_item_t is pretty gross; yes it's the first member in the
> structure, but yuk. Pass in the correct structure member.
>
> This was fixed in the kernel with commit e98c414f9
> ("xfs: simplify log item descriptor tracking")
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good.
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> ---
> libxfs/trans.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index f78222fd..6ef4841f 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -346,7 +346,7 @@ xfs_trans_ijoin(
> ASSERT(iip->ili_lock_flags == 0);
> iip->ili_lock_flags = lock_flags;
>
> - xfs_trans_add_item(tp, (xfs_log_item_t *)(iip));
> + xfs_trans_add_item(tp, &iip->ili_item);
> }
>
> void
> @@ -570,7 +570,7 @@ _xfs_trans_bjoin(
> * Attach the item to the transaction so we can find it in
> * xfs_trans_get_buf() and friends.
> */
> - xfs_trans_add_item(tp, (xfs_log_item_t *)bip);
> + xfs_trans_add_item(tp, &bip->bli_item);
> bp->b_transp = tp;
>
> }
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 8/3] xfs: factor log item initialisation
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (6 preceding siblings ...)
2019-05-16 20:40 ` [PATCH 7/3] libxfs: fix argument to xfs_trans_add_item Eric Sandeen
@ 2019-05-16 20:41 ` Eric Sandeen
2019-05-17 23:50 ` Allison Collins
2019-05-17 19:46 ` [PATCH 9/3] libxfs: create current_time helper and sync xfs_trans_ichgtime Eric Sandeen
` (2 subsequent siblings)
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-16 20:41 UTC (permalink / raw)
To: linux-xfs
Each log item type does manual initialisation of the log item.
Delayed logging introduces new fields that need initialisation, so
factor all the open coded initialisation into a common function
first.
Source kernel commit: 43f5efc5b59db1b66e39fe9fdfc4ba6a27152afa
Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
libxfs/libxfs_priv.h | 1 +
libxfs/logitem.c | 8 ++------
libxfs/util.c | 12 ++++++++++++
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 157a99d6..7551ed65 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -564,6 +564,7 @@ int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
bool xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
+void xfs_log_item_init(struct xfs_mount *, struct xfs_log_item *, int);
#define xfs_log_in_recovery(mp) (false)
/* xfs_icache.c */
diff --git a/libxfs/logitem.c b/libxfs/logitem.c
index e862ab4f..14c62f67 100644
--- a/libxfs/logitem.c
+++ b/libxfs/logitem.c
@@ -103,9 +103,7 @@ xfs_buf_item_init(
fprintf(stderr, "adding buf item %p for not-logged buffer %p\n",
bip, bp);
#endif
- bip->bli_item.li_type = XFS_LI_BUF;
- bip->bli_item.li_mountp = mp;
- INIT_LIST_HEAD(&bip->bli_item.li_trans);
+ xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF);
bip->bli_buf = bp;
bip->__bli_format.blf_type = XFS_LI_BUF;
bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
@@ -149,8 +147,6 @@ xfs_inode_item_init(
ip->i_ino, iip);
#endif
- iip->ili_item.li_type = XFS_LI_INODE;
- iip->ili_item.li_mountp = mp;
- INIT_LIST_HEAD(&iip->ili_item.li_trans);
+ xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE);
iip->ili_inode = ip;
}
diff --git a/libxfs/util.c b/libxfs/util.c
index 4901123a..aff91080 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -691,6 +691,18 @@ xfs_log_check_lsn(
return true;
}
+void
+xfs_log_item_init(
+ struct xfs_mount *mp,
+ struct xfs_log_item *item,
+ int type)
+{
+ item->li_mountp = mp;
+ item->li_type = type;
+
+ INIT_LIST_HEAD(&item->li_trans);
+}
+
static struct xfs_buftarg *
xfs_find_bdev_for_inode(
struct xfs_inode *ip)
--
2.17.0
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 8/3] xfs: factor log item initialisation
2019-05-16 20:41 ` [PATCH 8/3] xfs: factor log item initialisation Eric Sandeen
@ 2019-05-17 23:50 ` Allison Collins
0 siblings, 0 replies; 29+ messages in thread
From: Allison Collins @ 2019-05-17 23:50 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
On 5/16/19 1:41 PM, Eric Sandeen wrote:
> Each log item type does manual initialisation of the log item.
> Delayed logging introduces new fields that need initialisation, so
> factor all the open coded initialisation into a common function
> first.
>
> Source kernel commit: 43f5efc5b59db1b66e39fe9fdfc4ba6a27152afa
>
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Looks good to me.
Reviewed-by: Allison Collins <allison.henderson@oracle.com>
> ---
> libxfs/libxfs_priv.h | 1 +
> libxfs/logitem.c | 8 ++------
> libxfs/util.c | 12 ++++++++++++
> 3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 157a99d6..7551ed65 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -564,6 +564,7 @@ int xfs_zero_extent(struct xfs_inode *ip, xfs_fsblock_t start_fsb,
>
>
> bool xfs_log_check_lsn(struct xfs_mount *, xfs_lsn_t);
> +void xfs_log_item_init(struct xfs_mount *, struct xfs_log_item *, int);
> #define xfs_log_in_recovery(mp) (false)
>
> /* xfs_icache.c */
> diff --git a/libxfs/logitem.c b/libxfs/logitem.c
> index e862ab4f..14c62f67 100644
> --- a/libxfs/logitem.c
> +++ b/libxfs/logitem.c
> @@ -103,9 +103,7 @@ xfs_buf_item_init(
> fprintf(stderr, "adding buf item %p for not-logged buffer %p\n",
> bip, bp);
> #endif
> - bip->bli_item.li_type = XFS_LI_BUF;
> - bip->bli_item.li_mountp = mp;
> - INIT_LIST_HEAD(&bip->bli_item.li_trans);
> + xfs_log_item_init(mp, &bip->bli_item, XFS_LI_BUF);
> bip->bli_buf = bp;
> bip->__bli_format.blf_type = XFS_LI_BUF;
> bip->__bli_format.blf_blkno = (int64_t)XFS_BUF_ADDR(bp);
> @@ -149,8 +147,6 @@ xfs_inode_item_init(
> ip->i_ino, iip);
> #endif
>
> - iip->ili_item.li_type = XFS_LI_INODE;
> - iip->ili_item.li_mountp = mp;
> - INIT_LIST_HEAD(&iip->ili_item.li_trans);
> + xfs_log_item_init(mp, &iip->ili_item, XFS_LI_INODE);
> iip->ili_inode = ip;
> }
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 4901123a..aff91080 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -691,6 +691,18 @@ xfs_log_check_lsn(
> return true;
> }
>
> +void
> +xfs_log_item_init(
> + struct xfs_mount *mp,
> + struct xfs_log_item *item,
> + int type)
> +{
> + item->li_mountp = mp;
> + item->li_type = type;
> +
> + INIT_LIST_HEAD(&item->li_trans);
> +}
> +
> static struct xfs_buftarg *
> xfs_find_bdev_for_inode(
> struct xfs_inode *ip)
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH 9/3] libxfs: create current_time helper and sync xfs_trans_ichgtime
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (7 preceding siblings ...)
2019-05-16 20:41 ` [PATCH 8/3] xfs: factor log item initialisation Eric Sandeen
@ 2019-05-17 19:46 ` Eric Sandeen
2019-05-17 19:50 ` [PATCH 10/3] libxfs: share kernel's xfs_trans_inode.c Eric Sandeen
2019-05-20 23:03 ` [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Darrick J. Wong
10 siblings, 0 replies; 29+ messages in thread
From: Eric Sandeen @ 2019-05-17 19:46 UTC (permalink / raw)
To: Eric Sandeen, linux-xfs
Make xfs_trans_ichgtime() almost match kernelspace by creating a
new current_time() helper to match the kernel utility.
This reduces still more cosmetic change. We may want to sync the
creation flag over to the kernel even though it's not used today.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
diff --git a/include/xfs_inode.h b/include/xfs_inode.h
index 3e7e80ea..289d1774 100644
--- a/include/xfs_inode.h
+++ b/include/xfs_inode.h
@@ -16,6 +16,16 @@ struct xfs_mount;
struct xfs_inode_log_item;
struct xfs_dir_ops;
+/*
+ * These are not actually used, they are only for userspace build
+ * compatibility in code that looks at i_state
+ */
+#define I_DIRTY_TIME 0
+#define I_DIRTY_TIME_EXPIRED 0
+
+#define IS_I_VERSION(inode) (0)
+#define inode_maybe_inc_iversion(inode,flags) (0)
+
/*
* Inode interface. This fakes up a "VFS inode" to make the xfs_inode appear
* similar to the kernel which now is used tohold certain parts of the on-disk
@@ -25,6 +35,7 @@ struct inode {
mode_t i_mode;
uint32_t i_nlink;
xfs_dev_t i_rdev; /* This actually holds xfs_dev_t */
+ unsigned long i_state; /* Not actually used in userspace */
uint32_t i_generation;
uint64_t i_version;
struct timespec i_atime;
@@ -149,6 +160,9 @@ extern void xfs_trans_ichgtime(struct xfs_trans *,
struct xfs_inode *, int);
extern int xfs_iflush_int (struct xfs_inode *, struct xfs_buf *);
+#define timespec64 timespec
+extern struct timespec64 current_time(struct inode *inode);
+
/* Inode Cache Interfaces */
extern bool xfs_inode_verify_forks(struct xfs_inode *ip);
extern int xfs_iget(struct xfs_mount *, struct xfs_trans *, xfs_ino_t,
diff --git a/libxfs/util.c b/libxfs/util.c
index aff91080..1734ae9a 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -136,11 +136,21 @@ xfs_log_calc_unit_res(
return unit_bytes;
}
+struct timespec64
+current_time(struct inode *inode)
+{
+ struct timespec64 tv;
+ struct timeval stv;
+
+ gettimeofday(&stv, (struct timezone *)0);
+ tv.tv_sec = stv.tv_sec;
+ tv.tv_nsec = stv.tv_usec * 1000;
+
+ return tv;
+}
+
/*
* Change the requested timestamp in the given inode.
- *
- * This was once shared with the kernel, but has diverged to the point
- * where it's no longer worth the hassle of maintaining common code.
*/
void
xfs_trans_ichgtime(
@@ -148,12 +158,14 @@ xfs_trans_ichgtime(
struct xfs_inode *ip,
int flags)
{
- struct timespec tv;
- struct timeval stv;
+ struct inode *inode = VFS_I(ip);
+ struct timespec64 tv;
+
+ ASSERT(tp);
+ ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+ tv = current_time(inode);
- gettimeofday(&stv, (struct timezone *)0);
- tv.tv_sec = stv.tv_sec;
- tv.tv_nsec = stv.tv_usec * 1000;
if (flags & XFS_ICHGTIME_MOD)
VFS_I(ip)->i_mtime = tv;
if (flags & XFS_ICHGTIME_CHG)
^ permalink raw reply related [flat|nested] 29+ messages in thread* [PATCH 10/3] libxfs: share kernel's xfs_trans_inode.c
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (8 preceding siblings ...)
2019-05-17 19:46 ` [PATCH 9/3] libxfs: create current_time helper and sync xfs_trans_ichgtime Eric Sandeen
@ 2019-05-17 19:50 ` Eric Sandeen
2019-05-20 23:00 ` Darrick J. Wong
2019-05-20 23:03 ` [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Darrick J. Wong
10 siblings, 1 reply; 29+ messages in thread
From: Eric Sandeen @ 2019-05-17 19:50 UTC (permalink / raw)
To: linux-xfs
Now that the majority of cosmetic changes and compat shims
are in place, we can directly share kernelspace's
xfs_trans_inode.c with just a couple more small tweaks.
In addition to the file move,
* ili_fsync_fields is added to xfs_inode_log_item (but not used)
* test_and_set_bit() helper is created
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
To finish the sync, we probably need to move the file under
libxfs/ in kernelspace, and add XFS_ICHGTIME_CREATE handling
unless that's deemed undesirable, in which case we can probably
just carry the delta in userspace...
diff --git a/include/xfs_trans.h b/include/xfs_trans.h
index fe03ba64..cdaa69d8 100644
--- a/include/xfs_trans.h
+++ b/include/xfs_trans.h
@@ -30,8 +30,9 @@ typedef struct xfs_inode_log_item {
xfs_log_item_t ili_item; /* common portion */
struct xfs_inode *ili_inode; /* inode pointer */
unsigned short ili_lock_flags; /* lock flags */
- unsigned int ili_fields; /* fields to be logged */
unsigned int ili_last_fields; /* fields when flushed*/
+ unsigned int ili_fields; /* fields to be logged */
+ unsigned int ili_fsync_fields; /* ignored by userspace */
} xfs_inode_log_item_t;
typedef struct xfs_buf_log_item {
diff --git a/libxfs/Makefile b/libxfs/Makefile
index 160498d7..8c681e0b 100644
--- a/libxfs/Makefile
+++ b/libxfs/Makefile
@@ -93,6 +93,7 @@ CFILES = cache.c \
xfs_rtbitmap.c \
xfs_sb.c \
xfs_symlink_remote.c \
+ xfs_trans_inode.c \
xfs_trans_resv.c \
xfs_types.c
diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
index 7551ed65..fc785664 100644
--- a/libxfs/libxfs_priv.h
+++ b/libxfs/libxfs_priv.h
@@ -608,6 +608,15 @@ static inline int test_bit(int nr, const volatile unsigned long *addr)
return *p & mask;
}
+/* Sets and returns original value of the bit */
+static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
+{
+ if (test_bit(nr, addr))
+ return 1;
+ set_bit(nr, addr);
+ return 0;
+}
+
/* Keep static checkers quiet about nonstatic functions by exporting */
int xfs_inode_hasattr(struct xfs_inode *ip);
int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
diff --git a/libxfs/trans.c b/libxfs/trans.c
index 6ef4841f..29dd10f7 100644
--- a/libxfs/trans.c
+++ b/libxfs/trans.c
@@ -330,25 +330,6 @@ xfs_trans_cancel(
xfs_trans_free(tp);
}
-void
-xfs_trans_ijoin(
- xfs_trans_t *tp,
- xfs_inode_t *ip,
- uint lock_flags)
-{
- xfs_inode_log_item_t *iip;
-
- if (ip->i_itemp == NULL)
- xfs_inode_item_init(ip, ip->i_mount);
- iip = ip->i_itemp;
- ASSERT(iip->ili_inode != NULL);
-
- ASSERT(iip->ili_lock_flags == 0);
- iip->ili_lock_flags = lock_flags;
-
- xfs_trans_add_item(tp, &iip->ili_item);
-}
-
void
xfs_trans_inode_alloc_buf(
xfs_trans_t *tp,
@@ -362,52 +343,6 @@ xfs_trans_inode_alloc_buf(
xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
}
-/*
- * This is called to mark the fields indicated in fieldmask as needing
- * to be logged when the transaction is committed. The inode must
- * already be associated with the given transaction.
- *
- * The values for fieldmask are defined in xfs_log_format.h. We always
- * log all of the core inode if any of it has changed, and we always log
- * all of the inline data/extents/b-tree root if any of them has changed.
- */
-void
-xfs_trans_log_inode(
- xfs_trans_t *tp,
- xfs_inode_t *ip,
- uint flags)
-{
- ASSERT(ip->i_itemp != NULL);
-
- tp->t_flags |= XFS_TRANS_DIRTY;
- set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
-
- /*
- * Always OR in the bits from the ili_last_fields field.
- * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
- * routines in the eventual clearing of the ilf_fields bits.
- * See the big comment in xfs_iflush() for an explanation of
- * this coordination mechanism.
- */
- flags |= ip->i_itemp->ili_last_fields;
- ip->i_itemp->ili_fields |= flags;
-}
-
-int
-xfs_trans_roll_inode(
- struct xfs_trans **tpp,
- struct xfs_inode *ip)
-{
- int error;
-
- xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
- error = xfs_trans_roll(tpp);
- if (!error)
- xfs_trans_ijoin(*tpp, ip, 0);
- return error;
-}
-
-
/*
* Mark a buffer dirty in the transaction.
*/
diff --git a/libxfs/util.c b/libxfs/util.c
index 1734ae9a..5a89bd03 100644
--- a/libxfs/util.c
+++ b/libxfs/util.c
@@ -149,33 +149,6 @@ current_time(struct inode *inode)
return tv;
}
-/*
- * Change the requested timestamp in the given inode.
- */
-void
-xfs_trans_ichgtime(
- struct xfs_trans *tp,
- struct xfs_inode *ip,
- int flags)
-{
- struct inode *inode = VFS_I(ip);
- struct timespec64 tv;
-
- ASSERT(tp);
- ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
-
- tv = current_time(inode);
-
- if (flags & XFS_ICHGTIME_MOD)
- VFS_I(ip)->i_mtime = tv;
- if (flags & XFS_ICHGTIME_CHG)
- VFS_I(ip)->i_ctime = tv;
- if (flags & XFS_ICHGTIME_CREATE) {
- ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
- ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
- }
-}
-
STATIC uint16_t
xfs_flags2diflags(
struct xfs_inode *ip,
diff --git a/libxfs/xfs_trans_inode.c b/libxfs/xfs_trans_inode.c
new file mode 100644
index 00000000..87e6335b
--- /dev/null
+++ b/libxfs/xfs_trans_inode.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2000,2005 Silicon Graphics, Inc.
+ * All Rights Reserved.
+ */
+#include "libxfs_priv.h"
+#include "xfs_fs.h"
+#include "xfs_shared.h"
+#include "xfs_format.h"
+#include "xfs_log_format.h"
+#include "xfs_trans_resv.h"
+#include "xfs_mount.h"
+#include "xfs_inode.h"
+#include "xfs_trans.h"
+#include "xfs_trace.h"
+
+/*
+ * Add a locked inode to the transaction.
+ *
+ * The inode must be locked, and it cannot be associated with any transaction.
+ * If lock_flags is non-zero the inode will be unlocked on transaction commit.
+ */
+void
+xfs_trans_ijoin(
+ struct xfs_trans *tp,
+ struct xfs_inode *ip,
+ uint lock_flags)
+{
+ xfs_inode_log_item_t *iip;
+
+ ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+ if (ip->i_itemp == NULL)
+ xfs_inode_item_init(ip, ip->i_mount);
+ iip = ip->i_itemp;
+
+ ASSERT(iip->ili_lock_flags == 0);
+ iip->ili_lock_flags = lock_flags;
+
+ /*
+ * Get a log_item_desc to point at the new item.
+ */
+ xfs_trans_add_item(tp, &iip->ili_item);
+}
+
+/*
+ * Transactional inode timestamp update. Requires the inode to be locked and
+ * joined to the transaction supplied. Relies on the transaction subsystem to
+ * track dirty state and update/writeback the inode accordingly.
+ */
+void
+xfs_trans_ichgtime(
+ struct xfs_trans *tp,
+ struct xfs_inode *ip,
+ int flags)
+{
+ struct inode *inode = VFS_I(ip);
+ struct timespec64 tv;
+
+ ASSERT(tp);
+ ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+ tv = current_time(inode);
+
+ if (flags & XFS_ICHGTIME_MOD)
+ inode->i_mtime = tv;
+ if (flags & XFS_ICHGTIME_CHG)
+ inode->i_ctime = tv;
+ if (flags & XFS_ICHGTIME_CREATE) {
+ ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
+ ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
+ }
+}
+
+/*
+ * This is called to mark the fields indicated in fieldmask as needing
+ * to be logged when the transaction is committed. The inode must
+ * already be associated with the given transaction.
+ *
+ * The values for fieldmask are defined in xfs_inode_item.h. We always
+ * log all of the core inode if any of it has changed, and we always log
+ * all of the inline data/extents/b-tree root if any of them has changed.
+ */
+void
+xfs_trans_log_inode(
+ xfs_trans_t *tp,
+ xfs_inode_t *ip,
+ uint flags)
+{
+ struct inode *inode = VFS_I(ip);
+
+ ASSERT(ip->i_itemp != NULL);
+ ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+ /*
+ * Don't bother with i_lock for the I_DIRTY_TIME check here, as races
+ * don't matter - we either will need an extra transaction in 24 hours
+ * to log the timestamps, or will clear already cleared fields in the
+ * worst case.
+ */
+ if (inode->i_state & (I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED)) {
+ spin_lock(&inode->i_lock);
+ inode->i_state &= ~(I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED);
+ spin_unlock(&inode->i_lock);
+ }
+
+ /*
+ * Record the specific change for fdatasync optimisation. This
+ * allows fdatasync to skip log forces for inodes that are only
+ * timestamp dirty. We do this before the change count so that
+ * the core being logged in this case does not impact on fdatasync
+ * behaviour.
+ */
+ ip->i_itemp->ili_fsync_fields |= flags;
+
+ /*
+ * First time we log the inode in a transaction, bump the inode change
+ * counter if it is configured for this to occur. While we have the
+ * inode locked exclusively for metadata modification, we can usually
+ * avoid setting XFS_ILOG_CORE if no one has queried the value since
+ * the last time it was incremented. If we have XFS_ILOG_CORE already
+ * set however, then go ahead and bump the i_version counter
+ * unconditionally.
+ */
+ if (!test_and_set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags) &&
+ IS_I_VERSION(VFS_I(ip))) {
+ if (inode_maybe_inc_iversion(VFS_I(ip), flags & XFS_ILOG_CORE))
+ flags |= XFS_ILOG_CORE;
+ }
+
+ tp->t_flags |= XFS_TRANS_DIRTY;
+
+ /*
+ * Always OR in the bits from the ili_last_fields field.
+ * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
+ * routines in the eventual clearing of the ili_fields bits.
+ * See the big comment in xfs_iflush() for an explanation of
+ * this coordination mechanism.
+ */
+ flags |= ip->i_itemp->ili_last_fields;
+ ip->i_itemp->ili_fields |= flags;
+}
+
+int
+xfs_trans_roll_inode(
+ struct xfs_trans **tpp,
+ struct xfs_inode *ip)
+{
+ int error;
+
+ xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
+ error = xfs_trans_roll(tpp);
+ if (!error)
+ xfs_trans_ijoin(*tpp, ip, 0);
+ return error;
+}
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH 10/3] libxfs: share kernel's xfs_trans_inode.c
2019-05-17 19:50 ` [PATCH 10/3] libxfs: share kernel's xfs_trans_inode.c Eric Sandeen
@ 2019-05-20 23:00 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:00 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Fri, May 17, 2019 at 02:50:15PM -0500, Eric Sandeen wrote:
> Now that the majority of cosmetic changes and compat shims
> are in place, we can directly share kernelspace's
> xfs_trans_inode.c with just a couple more small tweaks.
> In addition to the file move,
>
> * ili_fsync_fields is added to xfs_inode_log_item (but not used)
> * test_and_set_bit() helper is created
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>
> To finish the sync, we probably need to move the file under
> libxfs/ in kernelspace, and add XFS_ICHGTIME_CREATE handling
> unless that's deemed undesirable, in which case we can probably
> just carry the delta in userspace...
I already did that as part of the metadir feature series I sent last
December... :)
Nevertheless, I eventually need xfs_trans_ichgtime to accept all four of
the MOD/CHG/CREATE/ACCESS flags, so this is fine.
--D
> diff --git a/include/xfs_trans.h b/include/xfs_trans.h
> index fe03ba64..cdaa69d8 100644
> --- a/include/xfs_trans.h
> +++ b/include/xfs_trans.h
> @@ -30,8 +30,9 @@ typedef struct xfs_inode_log_item {
> xfs_log_item_t ili_item; /* common portion */
> struct xfs_inode *ili_inode; /* inode pointer */
> unsigned short ili_lock_flags; /* lock flags */
> - unsigned int ili_fields; /* fields to be logged */
> unsigned int ili_last_fields; /* fields when flushed*/
> + unsigned int ili_fields; /* fields to be logged */
> + unsigned int ili_fsync_fields; /* ignored by userspace */
> } xfs_inode_log_item_t;
>
> typedef struct xfs_buf_log_item {
> diff --git a/libxfs/Makefile b/libxfs/Makefile
> index 160498d7..8c681e0b 100644
> --- a/libxfs/Makefile
> +++ b/libxfs/Makefile
> @@ -93,6 +93,7 @@ CFILES = cache.c \
> xfs_rtbitmap.c \
> xfs_sb.c \
> xfs_symlink_remote.c \
> + xfs_trans_inode.c \
> xfs_trans_resv.c \
> xfs_types.c
>
> diff --git a/libxfs/libxfs_priv.h b/libxfs/libxfs_priv.h
> index 7551ed65..fc785664 100644
> --- a/libxfs/libxfs_priv.h
> +++ b/libxfs/libxfs_priv.h
> @@ -608,6 +608,15 @@ static inline int test_bit(int nr, const volatile unsigned long *addr)
> return *p & mask;
> }
>
> +/* Sets and returns original value of the bit */
> +static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> +{
> + if (test_bit(nr, addr))
> + return 1;
> + set_bit(nr, addr);
> + return 0;
> +}
> +
> /* Keep static checkers quiet about nonstatic functions by exporting */
> int xfs_inode_hasattr(struct xfs_inode *ip);
> int xfs_attr_get_ilocked(struct xfs_inode *ip, struct xfs_da_args *args);
> diff --git a/libxfs/trans.c b/libxfs/trans.c
> index 6ef4841f..29dd10f7 100644
> --- a/libxfs/trans.c
> +++ b/libxfs/trans.c
> @@ -330,25 +330,6 @@ xfs_trans_cancel(
> xfs_trans_free(tp);
> }
>
> -void
> -xfs_trans_ijoin(
> - xfs_trans_t *tp,
> - xfs_inode_t *ip,
> - uint lock_flags)
> -{
> - xfs_inode_log_item_t *iip;
> -
> - if (ip->i_itemp == NULL)
> - xfs_inode_item_init(ip, ip->i_mount);
> - iip = ip->i_itemp;
> - ASSERT(iip->ili_inode != NULL);
> -
> - ASSERT(iip->ili_lock_flags == 0);
> - iip->ili_lock_flags = lock_flags;
> -
> - xfs_trans_add_item(tp, &iip->ili_item);
> -}
> -
> void
> xfs_trans_inode_alloc_buf(
> xfs_trans_t *tp,
> @@ -362,52 +343,6 @@ xfs_trans_inode_alloc_buf(
> xfs_trans_buf_set_type(tp, bp, XFS_BLFT_DINO_BUF);
> }
>
> -/*
> - * This is called to mark the fields indicated in fieldmask as needing
> - * to be logged when the transaction is committed. The inode must
> - * already be associated with the given transaction.
> - *
> - * The values for fieldmask are defined in xfs_log_format.h. We always
> - * log all of the core inode if any of it has changed, and we always log
> - * all of the inline data/extents/b-tree root if any of them has changed.
> - */
> -void
> -xfs_trans_log_inode(
> - xfs_trans_t *tp,
> - xfs_inode_t *ip,
> - uint flags)
> -{
> - ASSERT(ip->i_itemp != NULL);
> -
> - tp->t_flags |= XFS_TRANS_DIRTY;
> - set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags);
> -
> - /*
> - * Always OR in the bits from the ili_last_fields field.
> - * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
> - * routines in the eventual clearing of the ilf_fields bits.
> - * See the big comment in xfs_iflush() for an explanation of
> - * this coordination mechanism.
> - */
> - flags |= ip->i_itemp->ili_last_fields;
> - ip->i_itemp->ili_fields |= flags;
> -}
> -
> -int
> -xfs_trans_roll_inode(
> - struct xfs_trans **tpp,
> - struct xfs_inode *ip)
> -{
> - int error;
> -
> - xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
> - error = xfs_trans_roll(tpp);
> - if (!error)
> - xfs_trans_ijoin(*tpp, ip, 0);
> - return error;
> -}
> -
> -
> /*
> * Mark a buffer dirty in the transaction.
> */
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 1734ae9a..5a89bd03 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -149,33 +149,6 @@ current_time(struct inode *inode)
> return tv;
> }
>
> -/*
> - * Change the requested timestamp in the given inode.
> - */
> -void
> -xfs_trans_ichgtime(
> - struct xfs_trans *tp,
> - struct xfs_inode *ip,
> - int flags)
> -{
> - struct inode *inode = VFS_I(ip);
> - struct timespec64 tv;
> -
> - ASSERT(tp);
> - ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> -
> - tv = current_time(inode);
> -
> - if (flags & XFS_ICHGTIME_MOD)
> - VFS_I(ip)->i_mtime = tv;
> - if (flags & XFS_ICHGTIME_CHG)
> - VFS_I(ip)->i_ctime = tv;
> - if (flags & XFS_ICHGTIME_CREATE) {
> - ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
> - ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
> - }
> -}
> -
> STATIC uint16_t
> xfs_flags2diflags(
> struct xfs_inode *ip,
> diff --git a/libxfs/xfs_trans_inode.c b/libxfs/xfs_trans_inode.c
> new file mode 100644
> index 00000000..87e6335b
> --- /dev/null
> +++ b/libxfs/xfs_trans_inode.c
> @@ -0,0 +1,155 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2000,2005 Silicon Graphics, Inc.
> + * All Rights Reserved.
> + */
> +#include "libxfs_priv.h"
> +#include "xfs_fs.h"
> +#include "xfs_shared.h"
> +#include "xfs_format.h"
> +#include "xfs_log_format.h"
> +#include "xfs_trans_resv.h"
> +#include "xfs_mount.h"
> +#include "xfs_inode.h"
> +#include "xfs_trans.h"
> +#include "xfs_trace.h"
> +
> +/*
> + * Add a locked inode to the transaction.
> + *
> + * The inode must be locked, and it cannot be associated with any transaction.
> + * If lock_flags is non-zero the inode will be unlocked on transaction commit.
> + */
> +void
> +xfs_trans_ijoin(
> + struct xfs_trans *tp,
> + struct xfs_inode *ip,
> + uint lock_flags)
> +{
> + xfs_inode_log_item_t *iip;
> +
> + ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> + if (ip->i_itemp == NULL)
> + xfs_inode_item_init(ip, ip->i_mount);
> + iip = ip->i_itemp;
> +
> + ASSERT(iip->ili_lock_flags == 0);
> + iip->ili_lock_flags = lock_flags;
> +
> + /*
> + * Get a log_item_desc to point at the new item.
> + */
> + xfs_trans_add_item(tp, &iip->ili_item);
> +}
> +
> +/*
> + * Transactional inode timestamp update. Requires the inode to be locked and
> + * joined to the transaction supplied. Relies on the transaction subsystem to
> + * track dirty state and update/writeback the inode accordingly.
> + */
> +void
> +xfs_trans_ichgtime(
> + struct xfs_trans *tp,
> + struct xfs_inode *ip,
> + int flags)
> +{
> + struct inode *inode = VFS_I(ip);
> + struct timespec64 tv;
> +
> + ASSERT(tp);
> + ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> +
> + tv = current_time(inode);
> +
> + if (flags & XFS_ICHGTIME_MOD)
> + inode->i_mtime = tv;
> + if (flags & XFS_ICHGTIME_CHG)
> + inode->i_ctime = tv;
> + if (flags & XFS_ICHGTIME_CREATE) {
> + ip->i_d.di_crtime.t_sec = (int32_t)tv.tv_sec;
> + ip->i_d.di_crtime.t_nsec = (int32_t)tv.tv_nsec;
> + }
> +}
> +
> +/*
> + * This is called to mark the fields indicated in fieldmask as needing
> + * to be logged when the transaction is committed. The inode must
> + * already be associated with the given transaction.
> + *
> + * The values for fieldmask are defined in xfs_inode_item.h. We always
> + * log all of the core inode if any of it has changed, and we always log
> + * all of the inline data/extents/b-tree root if any of them has changed.
> + */
> +void
> +xfs_trans_log_inode(
> + xfs_trans_t *tp,
> + xfs_inode_t *ip,
> + uint flags)
> +{
> + struct inode *inode = VFS_I(ip);
> +
> + ASSERT(ip->i_itemp != NULL);
> + ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
> +
> + /*
> + * Don't bother with i_lock for the I_DIRTY_TIME check here, as races
> + * don't matter - we either will need an extra transaction in 24 hours
> + * to log the timestamps, or will clear already cleared fields in the
> + * worst case.
> + */
> + if (inode->i_state & (I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED)) {
> + spin_lock(&inode->i_lock);
> + inode->i_state &= ~(I_DIRTY_TIME | I_DIRTY_TIME_EXPIRED);
> + spin_unlock(&inode->i_lock);
> + }
> +
> + /*
> + * Record the specific change for fdatasync optimisation. This
> + * allows fdatasync to skip log forces for inodes that are only
> + * timestamp dirty. We do this before the change count so that
> + * the core being logged in this case does not impact on fdatasync
> + * behaviour.
> + */
> + ip->i_itemp->ili_fsync_fields |= flags;
> +
> + /*
> + * First time we log the inode in a transaction, bump the inode change
> + * counter if it is configured for this to occur. While we have the
> + * inode locked exclusively for metadata modification, we can usually
> + * avoid setting XFS_ILOG_CORE if no one has queried the value since
> + * the last time it was incremented. If we have XFS_ILOG_CORE already
> + * set however, then go ahead and bump the i_version counter
> + * unconditionally.
> + */
> + if (!test_and_set_bit(XFS_LI_DIRTY, &ip->i_itemp->ili_item.li_flags) &&
> + IS_I_VERSION(VFS_I(ip))) {
> + if (inode_maybe_inc_iversion(VFS_I(ip), flags & XFS_ILOG_CORE))
> + flags |= XFS_ILOG_CORE;
> + }
> +
> + tp->t_flags |= XFS_TRANS_DIRTY;
> +
> + /*
> + * Always OR in the bits from the ili_last_fields field.
> + * This is to coordinate with the xfs_iflush() and xfs_iflush_done()
> + * routines in the eventual clearing of the ili_fields bits.
> + * See the big comment in xfs_iflush() for an explanation of
> + * this coordination mechanism.
> + */
> + flags |= ip->i_itemp->ili_last_fields;
> + ip->i_itemp->ili_fields |= flags;
> +}
> +
> +int
> +xfs_trans_roll_inode(
> + struct xfs_trans **tpp,
> + struct xfs_inode *ip)
> +{
> + int error;
> +
> + xfs_trans_log_inode(*tpp, ip, XFS_ILOG_CORE);
> + error = xfs_trans_roll(tpp);
> + if (!error)
> + xfs_trans_ijoin(*tpp, ip, 0);
> + return error;
> +}
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning
2019-05-16 17:42 [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Eric Sandeen
` (9 preceding siblings ...)
2019-05-17 19:50 ` [PATCH 10/3] libxfs: share kernel's xfs_trans_inode.c Eric Sandeen
@ 2019-05-20 23:03 ` Darrick J. Wong
2019-05-20 23:10 ` Darrick J. Wong
10 siblings, 1 reply; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:03 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Thu, May 16, 2019 at 12:42:45PM -0500, Eric Sandeen wrote:
> It wasn't super clear before - my goal here is to keep reducing
> cosmetic differences between kernelspace & userspace libxfs/*
> files and functions.
>
> To that end, 3 more patches ... the first one may requires someone
> who groks the libxfs_* API namespace picture (looking at you, Dave!)
>
> (this abandons the "make new files" patches I sent before, at least
> for now, I'll heed dave's advice to minimize moves...)
Er, can you turn on diffstat for each patch that gets sent? That'll
make it much easier for me to figure out if any of these patches have to
go in the kernel first.
I /think/ the answer is 'no'....?
--D
> Thanks,
> -Eric
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning
2019-05-20 23:03 ` [PATCH 0/3] xfsprogs: more libxfs/ spring cleaning Darrick J. Wong
@ 2019-05-20 23:10 ` Darrick J. Wong
0 siblings, 0 replies; 29+ messages in thread
From: Darrick J. Wong @ 2019-05-20 23:10 UTC (permalink / raw)
To: Eric Sandeen; +Cc: linux-xfs
On Mon, May 20, 2019 at 04:03:11PM -0700, Darrick J. Wong wrote:
> On Thu, May 16, 2019 at 12:42:45PM -0500, Eric Sandeen wrote:
> > It wasn't super clear before - my goal here is to keep reducing
> > cosmetic differences between kernelspace & userspace libxfs/*
> > files and functions.
> >
> > To that end, 3 more patches ... the first one may requires someone
> > who groks the libxfs_* API namespace picture (looking at you, Dave!)
> >
> > (this abandons the "make new files" patches I sent before, at least
> > for now, I'll heed dave's advice to minimize moves...)
>
> Er, can you turn on diffstat for each patch that gets sent? That'll
> make it much easier for me to figure out if any of these patches have to
> go in the kernel first.
>
> I /think/ the answer is 'no'....?
Yes, this is purely userspace stuff, no kernel patches required.
For the whole series (modulo everyone else's comments),
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
--D
> --D
>
> > Thanks,
> > -Eric
^ permalink raw reply [flat|nested] 29+ messages in thread