* [PATCH 1/8] xfs: make if_data a void pointer
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
2023-12-19 12:08 ` [PATCH 2/8] xfs: return if_data from xfs_idata_realloc Christoph Hellwig
` (6 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
The xfs_ifork structure currently has a union of the if_root void pointer
and the if_data char pointer. In either case it is an opaque pointer
that depends on the fork format. Replace the union with a single if_data
void pointer as that is what almost all callers want. Only the symlink
NULL termination code in xfs_init_local_fork actually needs a new local
variable now.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr.c | 3 +-
fs/xfs/libxfs/xfs_attr_leaf.c | 62 ++++++++++++------------------
fs/xfs/libxfs/xfs_bmap.c | 4 +-
fs/xfs/libxfs/xfs_dir2.c | 2 +-
fs/xfs/libxfs/xfs_dir2_block.c | 6 +--
fs/xfs/libxfs/xfs_dir2_sf.c | 61 ++++++++++++-----------------
fs/xfs/libxfs/xfs_iext_tree.c | 36 ++++++++---------
fs/xfs/libxfs/xfs_inode_fork.c | 53 ++++++++++++-------------
fs/xfs/libxfs/xfs_inode_fork.h | 8 ++--
fs/xfs/libxfs/xfs_symlink_remote.c | 4 +-
fs/xfs/scrub/attr.c | 10 ++---
fs/xfs/scrub/readdir.c | 6 +--
fs/xfs/scrub/symlink.c | 2 +-
fs/xfs/xfs_attr_list.c | 3 +-
fs/xfs/xfs_dir2_readdir.c | 6 +--
fs/xfs/xfs_inode.c | 6 +--
fs/xfs/xfs_inode_item.c | 10 ++---
fs/xfs/xfs_symlink.c | 4 +-
18 files changed, 119 insertions(+), 167 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index fa49c795f40745..7f822e72dfcd3e 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1049,9 +1049,8 @@ xfs_attr_set(
static inline int xfs_attr_sf_totsize(struct xfs_inode *dp)
{
- struct xfs_attr_shortform *sf;
+ struct xfs_attr_shortform *sf = dp->i_af.if_data;
- sf = (struct xfs_attr_shortform *)dp->i_af.if_u1.if_data;
return be16_to_cpu(sf->hdr.totsize);
}
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 5d1ab4978f3293..3e5377fd498471 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -691,7 +691,7 @@ xfs_attr_shortform_create(
if (ifp->if_format == XFS_DINODE_FMT_EXTENTS)
ifp->if_format = XFS_DINODE_FMT_LOCAL;
xfs_idata_realloc(dp, sizeof(*hdr), XFS_ATTR_FORK);
- hdr = (struct xfs_attr_sf_hdr *)ifp->if_u1.if_data;
+ hdr = ifp->if_data;
memset(hdr, 0, sizeof(*hdr));
hdr->totsize = cpu_to_be16(sizeof(*hdr));
xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_ADATA);
@@ -712,14 +712,13 @@ xfs_attr_sf_findname(
struct xfs_attr_sf_entry **sfep,
unsigned int *basep)
{
- struct xfs_attr_shortform *sf;
+ struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
unsigned int base = sizeof(struct xfs_attr_sf_hdr);
int size = 0;
int end;
int i;
- sf = (struct xfs_attr_shortform *)args->dp->i_af.if_u1.if_data;
sfe = &sf->list[0];
end = sf->hdr.count;
for (i = 0; i < end; sfe = xfs_attr_sf_nextentry(sfe),
@@ -751,29 +750,25 @@ xfs_attr_shortform_add(
struct xfs_da_args *args,
int forkoff)
{
- struct xfs_attr_shortform *sf;
+ struct xfs_inode *dp = args->dp;
+ struct xfs_mount *mp = dp->i_mount;
+ struct xfs_ifork *ifp = &dp->i_af;
+ struct xfs_attr_shortform *sf = ifp->if_data;
struct xfs_attr_sf_entry *sfe;
int offset, size;
- struct xfs_mount *mp;
- struct xfs_inode *dp;
- struct xfs_ifork *ifp;
trace_xfs_attr_sf_add(args);
- dp = args->dp;
- mp = dp->i_mount;
dp->i_forkoff = forkoff;
- ifp = &dp->i_af;
ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
- sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
if (xfs_attr_sf_findname(args, &sfe, NULL) == -EEXIST)
ASSERT(0);
offset = (char *)sfe - (char *)sf;
size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
- sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
+ sf = ifp->if_data;
sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
sfe->namelen = args->namelen;
@@ -811,20 +806,16 @@ int
xfs_attr_sf_removename(
struct xfs_da_args *args)
{
- struct xfs_attr_shortform *sf;
+ struct xfs_inode *dp = args->dp;
+ struct xfs_mount *mp = dp->i_mount;
+ struct xfs_attr_shortform *sf = dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
int size = 0, end, totsize;
unsigned int base;
- struct xfs_mount *mp;
- struct xfs_inode *dp;
int error;
trace_xfs_attr_sf_remove(args);
- dp = args->dp;
- mp = dp->i_mount;
- sf = (struct xfs_attr_shortform *)dp->i_af.if_u1.if_data;
-
error = xfs_attr_sf_findname(args, &sfe, &base);
/*
@@ -878,18 +869,17 @@ xfs_attr_sf_removename(
*/
/*ARGSUSED*/
int
-xfs_attr_shortform_lookup(xfs_da_args_t *args)
+xfs_attr_shortform_lookup(
+ struct xfs_da_args *args)
{
- struct xfs_attr_shortform *sf;
- struct xfs_attr_sf_entry *sfe;
- int i;
- struct xfs_ifork *ifp;
+ struct xfs_ifork *ifp = &args->dp->i_af;
+ struct xfs_attr_shortform *sf = ifp->if_data;
+ struct xfs_attr_sf_entry *sfe;
+ int i;
trace_xfs_attr_sf_lookup(args);
- ifp = &args->dp->i_af;
ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
- sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
sfe = &sf->list[0];
for (i = 0; i < sf->hdr.count;
sfe = xfs_attr_sf_nextentry(sfe), i++) {
@@ -909,14 +899,13 @@ xfs_attr_shortform_lookup(xfs_da_args_t *args)
*/
int
xfs_attr_shortform_getvalue(
- struct xfs_da_args *args)
+ struct xfs_da_args *args)
{
- struct xfs_attr_shortform *sf;
- struct xfs_attr_sf_entry *sfe;
- int i;
+ struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
+ struct xfs_attr_sf_entry *sfe;
+ int i;
ASSERT(args->dp->i_af.if_format == XFS_DINODE_FMT_LOCAL);
- sf = (struct xfs_attr_shortform *)args->dp->i_af.if_u1.if_data;
sfe = &sf->list[0];
for (i = 0; i < sf->hdr.count;
sfe = xfs_attr_sf_nextentry(sfe), i++) {
@@ -933,25 +922,22 @@ int
xfs_attr_shortform_to_leaf(
struct xfs_da_args *args)
{
- struct xfs_inode *dp;
- struct xfs_attr_shortform *sf;
+ struct xfs_inode *dp = args->dp;
+ struct xfs_ifork *ifp = &dp->i_af;
+ struct xfs_attr_shortform *sf = ifp->if_data;
struct xfs_attr_sf_entry *sfe;
struct xfs_da_args nargs;
char *tmpbuffer;
int error, i, size;
xfs_dablk_t blkno;
struct xfs_buf *bp;
- struct xfs_ifork *ifp;
trace_xfs_attr_sf_to_leaf(args);
- dp = args->dp;
- ifp = &dp->i_af;
- sf = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
size = be16_to_cpu(sf->hdr.totsize);
tmpbuffer = kmem_alloc(size, 0);
ASSERT(tmpbuffer != NULL);
- memcpy(tmpbuffer, ifp->if_u1.if_data, size);
+ memcpy(tmpbuffer, ifp->if_data, size);
sf = (struct xfs_attr_shortform *)tmpbuffer;
xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 523926fe50eb0a..3ed01c178b7baa 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -747,7 +747,7 @@ xfs_bmap_local_to_extents_empty(
ASSERT(ifp->if_nextents == 0);
xfs_bmap_forkoff_reset(ip, whichfork);
- ifp->if_u1.if_root = NULL;
+ ifp->if_data = NULL;
ifp->if_height = 0;
ifp->if_format = XFS_DINODE_FMT_EXTENTS;
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
@@ -832,7 +832,7 @@ xfs_bmap_local_to_extents(
xfs_bmap_local_to_extents_empty(tp, ip, whichfork);
flags |= XFS_ILOG_CORE;
- ifp->if_u1.if_root = NULL;
+ ifp->if_data = NULL;
ifp->if_height = 0;
rec.br_startoff = 0;
diff --git a/fs/xfs/libxfs/xfs_dir2.c b/fs/xfs/libxfs/xfs_dir2.c
index f5462fd582d502..a7667328151450 100644
--- a/fs/xfs/libxfs/xfs_dir2.c
+++ b/fs/xfs/libxfs/xfs_dir2.c
@@ -196,7 +196,7 @@ xfs_dir_isempty(
return 1;
if (dp->i_disk_size > xfs_inode_data_fork_size(dp))
return 0;
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
return !sfp->count;
}
diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
index 00f960a703b2ef..3c256d4cc40b48 100644
--- a/fs/xfs/libxfs/xfs_dir2_block.c
+++ b/fs/xfs/libxfs/xfs_dir2_block.c
@@ -1089,7 +1089,7 @@ xfs_dir2_sf_to_block(
int newoffset; /* offset from current entry */
unsigned int offset = geo->data_entry_offset;
xfs_dir2_sf_entry_t *sfep; /* sf entry pointer */
- xfs_dir2_sf_hdr_t *oldsfp; /* old shortform header */
+ struct xfs_dir2_sf_hdr *oldsfp = ifp->if_data;
xfs_dir2_sf_hdr_t *sfp; /* shortform header */
__be16 *tagp; /* end of data entry */
struct xfs_name name;
@@ -1099,10 +1099,8 @@ xfs_dir2_sf_to_block(
ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
- oldsfp = (xfs_dir2_sf_hdr_t *)ifp->if_u1.if_data;
-
ASSERT(ifp->if_bytes == dp->i_disk_size);
- ASSERT(ifp->if_u1.if_data != NULL);
+ ASSERT(oldsfp != NULL);
ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(oldsfp->i8count));
ASSERT(dp->i_df.if_nextents == 0);
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 870ef1d1ebe4a2..0b63138d2b9f0e 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -364,25 +364,23 @@ int /* error */
xfs_dir2_sf_addname(
xfs_da_args_t *args) /* operation arguments */
{
- xfs_inode_t *dp; /* incore directory inode */
+ struct xfs_inode *dp = args->dp;
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
int error; /* error return value */
int incr_isize; /* total change in size */
int new_isize; /* size after adding name */
int objchange; /* changing to 8-byte inodes */
xfs_dir2_data_aoff_t offset = 0; /* offset for new entry */
int pick; /* which algorithm to use */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
xfs_dir2_sf_entry_t *sfep = NULL; /* shortform entry */
trace_xfs_dir2_sf_addname(args);
ASSERT(xfs_dir2_sf_lookup(args) == -ENOENT);
- dp = args->dp;
ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
- ASSERT(dp->i_df.if_u1.if_data != NULL);
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ ASSERT(sfp != NULL);
ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(sfp->i8count));
/*
* Compute entry (and change in) size.
@@ -462,11 +460,9 @@ xfs_dir2_sf_addname_easy(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
- int byteoff; /* byte offset in sf dir */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
+ int byteoff = (int)((char *)sfep - (char *)sfp);
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
- byteoff = (int)((char *)sfep - (char *)sfp);
/*
* Grow the in-inode space.
*/
@@ -475,7 +471,7 @@ xfs_dir2_sf_addname_easy(
/*
* Need to set up again due to realloc of the inode data.
*/
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
sfep = (xfs_dir2_sf_entry_t *)((char *)sfp + byteoff);
/*
* Fill in the new entry.
@@ -528,11 +524,10 @@ xfs_dir2_sf_addname_hard(
/*
* Copy the old directory to the stack buffer.
*/
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
old_isize = (int)dp->i_disk_size;
buf = kmem_alloc(old_isize, 0);
oldsfp = (xfs_dir2_sf_hdr_t *)buf;
- memcpy(oldsfp, sfp, old_isize);
+ memcpy(oldsfp, dp->i_df.if_data, old_isize);
/*
* Loop over the old directory finding the place we're going
* to insert the new entry.
@@ -560,7 +555,7 @@ xfs_dir2_sf_addname_hard(
/*
* Reset the pointer since the buffer was reallocated.
*/
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
/*
* Copy the first part of the directory, including the header.
*/
@@ -610,11 +605,10 @@ xfs_dir2_sf_addname_pick(
int i; /* entry number */
xfs_dir2_data_aoff_t offset; /* data block offset */
xfs_dir2_sf_entry_t *sfep; /* shortform entry */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
int size; /* entry's data size */
int used; /* data bytes used */
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
size = xfs_dir2_data_entsize(mp, args->namelen);
offset = args->geo->data_first_offset;
sfep = xfs_dir2_sf_firstentry(sfp);
@@ -673,14 +667,13 @@ xfs_dir2_sf_check(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
int i; /* entry number */
int i8count; /* number of big inode#s */
xfs_ino_t ino; /* entry inode number */
int offset; /* data offset */
xfs_dir2_sf_entry_t *sfep; /* shortform dir entry */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
offset = args->geo->data_first_offset;
ino = xfs_dir2_sf_get_parent_ino(sfp);
i8count = ino > XFS_DIR2_MAX_SHORT_INUM;
@@ -834,7 +827,7 @@ xfs_dir2_sf_create(
/*
* Fill in the header,
*/
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
sfp->i8count = i8count;
/*
* Now can put in the inode number, since i8count is set.
@@ -857,9 +850,9 @@ xfs_dir2_sf_lookup(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
int i; /* entry index */
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
enum xfs_dacmp cmp; /* comparison result */
xfs_dir2_sf_entry_t *ci_sfep; /* case-insens. entry */
@@ -870,8 +863,7 @@ xfs_dir2_sf_lookup(
ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
- ASSERT(dp->i_df.if_u1.if_data != NULL);
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ ASSERT(sfp != NULL);
ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(sfp->i8count));
/*
* Special case for .
@@ -933,13 +925,13 @@ xfs_dir2_sf_removename(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
int byteoff; /* offset of removed entry */
int entsize; /* this entry's size */
int i; /* shortform entry index */
int newsize; /* new inode size */
int oldsize; /* old inode size */
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
trace_xfs_dir2_sf_removename(args);
@@ -947,8 +939,7 @@ xfs_dir2_sf_removename(
oldsize = (int)dp->i_disk_size;
ASSERT(oldsize >= offsetof(struct xfs_dir2_sf_hdr, parent));
ASSERT(dp->i_df.if_bytes == oldsize);
- ASSERT(dp->i_df.if_u1.if_data != NULL);
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ ASSERT(sfp != NULL);
ASSERT(oldsize >= xfs_dir2_sf_hdr_size(sfp->i8count));
/*
* Loop over the old directory entries.
@@ -989,7 +980,7 @@ xfs_dir2_sf_removename(
* Reallocate, making it smaller.
*/
xfs_idata_realloc(dp, newsize - oldsize, XFS_DATA_FORK);
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
/*
* Are we changing inode number size?
*/
@@ -1012,13 +1003,12 @@ xfs_dir2_sf_replace_needblock(
struct xfs_inode *dp,
xfs_ino_t inum)
{
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
int newsize;
- struct xfs_dir2_sf_hdr *sfp;
if (dp->i_df.if_format != XFS_DINODE_FMT_LOCAL)
return false;
- sfp = (struct xfs_dir2_sf_hdr *)dp->i_df.if_u1.if_data;
newsize = dp->i_df.if_bytes + (sfp->count + 1) * XFS_INO64_DIFF;
return inum > XFS_DIR2_MAX_SHORT_INUM &&
@@ -1034,19 +1024,18 @@ xfs_dir2_sf_replace(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
int i; /* entry index */
xfs_ino_t ino=0; /* entry old inode number */
int i8elevated; /* sf_toino8 set i8count=1 */
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
trace_xfs_dir2_sf_replace(args);
ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
- ASSERT(dp->i_df.if_u1.if_data != NULL);
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ ASSERT(sfp != NULL);
ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(sfp->i8count));
/*
@@ -1069,7 +1058,7 @@ xfs_dir2_sf_replace(
*/
xfs_dir2_sf_toino8(args);
i8elevated = 1;
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
} else
i8elevated = 0;
@@ -1150,11 +1139,11 @@ xfs_dir2_sf_toino4(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
+ struct xfs_dir2_sf_hdr *oldsfp = dp->i_df.if_data;
char *buf; /* old dir's buffer */
int i; /* entry index */
int newsize; /* new inode size */
xfs_dir2_sf_entry_t *oldsfep; /* old sf entry */
- xfs_dir2_sf_hdr_t *oldsfp; /* old sf directory */
int oldsize; /* old inode size */
xfs_dir2_sf_entry_t *sfep; /* new sf entry */
xfs_dir2_sf_hdr_t *sfp; /* new sf directory */
@@ -1168,7 +1157,6 @@ xfs_dir2_sf_toino4(
*/
oldsize = dp->i_df.if_bytes;
buf = kmem_alloc(oldsize, 0);
- oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
ASSERT(oldsfp->i8count == 1);
memcpy(buf, oldsfp, oldsize);
/*
@@ -1181,7 +1169,7 @@ xfs_dir2_sf_toino4(
* Reset our pointers, the data has moved.
*/
oldsfp = (xfs_dir2_sf_hdr_t *)buf;
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
/*
* Fill in the new header.
*/
@@ -1223,11 +1211,11 @@ xfs_dir2_sf_toino8(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
+ struct xfs_dir2_sf_hdr *oldsfp = dp->i_df.if_data;
char *buf; /* old dir's buffer */
int i; /* entry index */
int newsize; /* new inode size */
xfs_dir2_sf_entry_t *oldsfep; /* old sf entry */
- xfs_dir2_sf_hdr_t *oldsfp; /* old sf directory */
int oldsize; /* old inode size */
xfs_dir2_sf_entry_t *sfep; /* new sf entry */
xfs_dir2_sf_hdr_t *sfp; /* new sf directory */
@@ -1241,7 +1229,6 @@ xfs_dir2_sf_toino8(
*/
oldsize = dp->i_df.if_bytes;
buf = kmem_alloc(oldsize, 0);
- oldsfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
ASSERT(oldsfp->i8count == 0);
memcpy(buf, oldsfp, oldsize);
/*
@@ -1254,7 +1241,7 @@ xfs_dir2_sf_toino8(
* Reset our pointers, the data has moved.
*/
oldsfp = (xfs_dir2_sf_hdr_t *)buf;
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ sfp = dp->i_df.if_data;
/*
* Fill in the new header.
*/
diff --git a/fs/xfs/libxfs/xfs_iext_tree.c b/fs/xfs/libxfs/xfs_iext_tree.c
index d062794cc79575..f4e6b200cdf8c1 100644
--- a/fs/xfs/libxfs/xfs_iext_tree.c
+++ b/fs/xfs/libxfs/xfs_iext_tree.c
@@ -158,7 +158,7 @@ static void *
xfs_iext_find_first_leaf(
struct xfs_ifork *ifp)
{
- struct xfs_iext_node *node = ifp->if_u1.if_root;
+ struct xfs_iext_node *node = ifp->if_data;
int height;
if (!ifp->if_height)
@@ -176,7 +176,7 @@ static void *
xfs_iext_find_last_leaf(
struct xfs_ifork *ifp)
{
- struct xfs_iext_node *node = ifp->if_u1.if_root;
+ struct xfs_iext_node *node = ifp->if_data;
int height, i;
if (!ifp->if_height)
@@ -306,7 +306,7 @@ xfs_iext_find_level(
xfs_fileoff_t offset,
int level)
{
- struct xfs_iext_node *node = ifp->if_u1.if_root;
+ struct xfs_iext_node *node = ifp->if_data;
int height, i;
if (!ifp->if_height)
@@ -402,12 +402,12 @@ xfs_iext_grow(
int i;
if (ifp->if_height == 1) {
- struct xfs_iext_leaf *prev = ifp->if_u1.if_root;
+ struct xfs_iext_leaf *prev = ifp->if_data;
node->keys[0] = xfs_iext_leaf_key(prev, 0);
node->ptrs[0] = prev;
} else {
- struct xfs_iext_node *prev = ifp->if_u1.if_root;
+ struct xfs_iext_node *prev = ifp->if_data;
ASSERT(ifp->if_height > 1);
@@ -418,7 +418,7 @@ xfs_iext_grow(
for (i = 1; i < KEYS_PER_NODE; i++)
node->keys[i] = XFS_IEXT_KEY_INVALID;
- ifp->if_u1.if_root = node;
+ ifp->if_data = node;
ifp->if_height++;
}
@@ -430,7 +430,7 @@ xfs_iext_update_node(
int level,
void *ptr)
{
- struct xfs_iext_node *node = ifp->if_u1.if_root;
+ struct xfs_iext_node *node = ifp->if_data;
int height, i;
for (height = ifp->if_height; height > level; height--) {
@@ -583,11 +583,11 @@ xfs_iext_alloc_root(
{
ASSERT(ifp->if_bytes == 0);
- ifp->if_u1.if_root = kmem_zalloc(sizeof(struct xfs_iext_rec), KM_NOFS);
+ ifp->if_data = kmem_zalloc(sizeof(struct xfs_iext_rec), KM_NOFS);
ifp->if_height = 1;
/* now that we have a node step into it */
- cur->leaf = ifp->if_u1.if_root;
+ cur->leaf = ifp->if_data;
cur->pos = 0;
}
@@ -603,9 +603,9 @@ xfs_iext_realloc_root(
if (new_size / sizeof(struct xfs_iext_rec) == RECS_PER_LEAF)
new_size = NODE_SIZE;
- new = krealloc(ifp->if_u1.if_root, new_size, GFP_NOFS | __GFP_NOFAIL);
+ new = krealloc(ifp->if_data, new_size, GFP_NOFS | __GFP_NOFAIL);
memset(new + ifp->if_bytes, 0, new_size - ifp->if_bytes);
- ifp->if_u1.if_root = new;
+ ifp->if_data = new;
cur->leaf = new;
}
@@ -786,8 +786,8 @@ xfs_iext_remove_node(
* If we are at the root and only one entry is left we can just
* free this node and update the root pointer.
*/
- ASSERT(node == ifp->if_u1.if_root);
- ifp->if_u1.if_root = node->ptrs[0];
+ ASSERT(node == ifp->if_data);
+ ifp->if_data = node->ptrs[0];
ifp->if_height--;
kmem_free(node);
}
@@ -863,8 +863,8 @@ xfs_iext_free_last_leaf(
struct xfs_ifork *ifp)
{
ifp->if_height--;
- kmem_free(ifp->if_u1.if_root);
- ifp->if_u1.if_root = NULL;
+ kmem_free(ifp->if_data);
+ ifp->if_data = NULL;
}
void
@@ -881,7 +881,7 @@ xfs_iext_remove(
trace_xfs_iext_remove(ip, cur, state, _RET_IP_);
ASSERT(ifp->if_height > 0);
- ASSERT(ifp->if_u1.if_root != NULL);
+ ASSERT(ifp->if_data != NULL);
ASSERT(xfs_iext_valid(ifp, cur));
xfs_iext_inc_seq(ifp);
@@ -1051,9 +1051,9 @@ void
xfs_iext_destroy(
struct xfs_ifork *ifp)
{
- xfs_iext_destroy_node(ifp->if_u1.if_root, ifp->if_height);
+ xfs_iext_destroy_node(ifp->if_data, ifp->if_height);
ifp->if_bytes = 0;
ifp->if_height = 0;
- ifp->if_u1.if_root = NULL;
+ ifp->if_data = NULL;
}
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index b86d57589f67e6..d23910e503a1ae 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -50,12 +50,15 @@ xfs_init_local_fork(
mem_size++;
if (size) {
- ifp->if_u1.if_data = kmem_alloc(mem_size, KM_NOFS);
- memcpy(ifp->if_u1.if_data, data, size);
+ char *new_data = kmem_alloc(mem_size, KM_NOFS);
+
+ memcpy(new_data, data, size);
if (zero_terminate)
- ifp->if_u1.if_data[size] = '\0';
+ new_data[size] = '\0';
+
+ ifp->if_data = new_data;
} else {
- ifp->if_u1.if_data = NULL;
+ ifp->if_data = NULL;
}
ifp->if_bytes = size;
@@ -125,7 +128,7 @@ xfs_iformat_extents(
}
ifp->if_bytes = 0;
- ifp->if_u1.if_root = NULL;
+ ifp->if_data = NULL;
ifp->if_height = 0;
if (size) {
dp = (xfs_bmbt_rec_t *) XFS_DFORK_PTR(dip, whichfork);
@@ -212,7 +215,7 @@ xfs_iformat_btree(
ifp->if_broot, size);
ifp->if_bytes = 0;
- ifp->if_u1.if_root = NULL;
+ ifp->if_data = NULL;
ifp->if_height = 0;
return 0;
}
@@ -509,14 +512,14 @@ xfs_idata_realloc(
return;
if (new_size == 0) {
- kmem_free(ifp->if_u1.if_data);
- ifp->if_u1.if_data = NULL;
+ kmem_free(ifp->if_data);
+ ifp->if_data = NULL;
ifp->if_bytes = 0;
return;
}
- ifp->if_u1.if_data = krealloc(ifp->if_u1.if_data, new_size,
- GFP_NOFS | __GFP_NOFAIL);
+ ifp->if_data = krealloc(ifp->if_data, new_size,
+ GFP_NOFS | __GFP_NOFAIL);
ifp->if_bytes = new_size;
}
@@ -532,8 +535,8 @@ xfs_idestroy_fork(
switch (ifp->if_format) {
case XFS_DINODE_FMT_LOCAL:
- kmem_free(ifp->if_u1.if_data);
- ifp->if_u1.if_data = NULL;
+ kmem_free(ifp->if_data);
+ ifp->if_data = NULL;
break;
case XFS_DINODE_FMT_EXTENTS:
case XFS_DINODE_FMT_BTREE:
@@ -626,9 +629,9 @@ xfs_iflush_fork(
case XFS_DINODE_FMT_LOCAL:
if ((iip->ili_fields & dataflag[whichfork]) &&
(ifp->if_bytes > 0)) {
- ASSERT(ifp->if_u1.if_data != NULL);
+ ASSERT(ifp->if_data != NULL);
ASSERT(ifp->if_bytes <= xfs_inode_fork_size(ip, whichfork));
- memcpy(cp, ifp->if_u1.if_data, ifp->if_bytes);
+ memcpy(cp, ifp->if_data, ifp->if_bytes);
}
break;
@@ -706,17 +709,15 @@ xfs_ifork_verify_local_data(
case S_IFDIR: {
struct xfs_mount *mp = ip->i_mount;
struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
- struct xfs_dir2_sf_hdr *sfp;
+ struct xfs_dir2_sf_hdr *sfp = ifp->if_data;
- sfp = (struct xfs_dir2_sf_hdr *)ifp->if_u1.if_data;
fa = xfs_dir2_sf_verify(mp, sfp, ifp->if_bytes);
break;
}
case S_IFLNK: {
struct xfs_ifork *ifp = xfs_ifork_ptr(ip, XFS_DATA_FORK);
- fa = xfs_symlink_shortform_verify(ifp->if_u1.if_data,
- ifp->if_bytes);
+ fa = xfs_symlink_shortform_verify(ifp->if_data, ifp->if_bytes);
break;
}
default:
@@ -725,7 +726,7 @@ xfs_ifork_verify_local_data(
if (fa) {
xfs_inode_verifier_error(ip, -EFSCORRUPTED, "data fork",
- ip->i_df.if_u1.if_data, ip->i_df.if_bytes, fa);
+ ip->i_df.if_data, ip->i_df.if_bytes, fa);
return -EFSCORRUPTED;
}
@@ -743,20 +744,14 @@ xfs_ifork_verify_local_attr(
if (!xfs_inode_has_attr_fork(ip)) {
fa = __this_address;
} else {
- struct xfs_attr_shortform *sfp;
- struct xfs_ifork *ifp;
- int64_t size;
-
- ASSERT(ip->i_af.if_format == XFS_DINODE_FMT_LOCAL);
- ifp = xfs_ifork_ptr(ip, XFS_ATTR_FORK);
- sfp = (struct xfs_attr_shortform *)ifp->if_u1.if_data;
- size = ifp->if_bytes;
+ struct xfs_ifork *ifp = &ip->i_af;
- fa = xfs_attr_shortform_verify(sfp, size);
+ ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
+ fa = xfs_attr_shortform_verify(ifp->if_data, ifp->if_bytes);
}
if (fa) {
xfs_inode_verifier_error(ip, -EFSCORRUPTED, "attr fork",
- ifp->if_u1.if_data, ifp->if_bytes, fa);
+ ifp->if_data, ifp->if_bytes, fa);
return -EFSCORRUPTED;
}
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 535be5c036899c..7edcf0e8cd5388 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -13,14 +13,12 @@ struct xfs_dinode;
* File incore extent information, present for each of data & attr forks.
*/
struct xfs_ifork {
- int64_t if_bytes; /* bytes in if_u1 */
+ int64_t if_bytes; /* bytes in if_data */
struct xfs_btree_block *if_broot; /* file's incore btree root */
unsigned int if_seq; /* fork mod counter */
int if_height; /* height of the extent tree */
- union {
- void *if_root; /* extent tree root */
- char *if_data; /* inline file data */
- } if_u1;
+ void *if_data; /* extent tree root or
+ inline data */
xfs_extnum_t if_nextents; /* # of extents in this fork */
short if_broot_bytes; /* bytes allocated for root */
int8_t if_format; /* format of this fork */
diff --git a/fs/xfs/libxfs/xfs_symlink_remote.c b/fs/xfs/libxfs/xfs_symlink_remote.c
index 3c96d1d617fb0b..160aa20aa44139 100644
--- a/fs/xfs/libxfs/xfs_symlink_remote.c
+++ b/fs/xfs/libxfs/xfs_symlink_remote.c
@@ -175,7 +175,7 @@ xfs_symlink_local_to_remote(
if (!xfs_has_crc(mp)) {
bp->b_ops = NULL;
- memcpy(bp->b_addr, ifp->if_u1.if_data, ifp->if_bytes);
+ memcpy(bp->b_addr, ifp->if_data, ifp->if_bytes);
xfs_trans_log_buf(tp, bp, 0, ifp->if_bytes - 1);
return;
}
@@ -191,7 +191,7 @@ xfs_symlink_local_to_remote(
buf = bp->b_addr;
buf += xfs_symlink_hdr_set(mp, ip->i_ino, 0, ifp->if_bytes, bp);
- memcpy(buf, ifp->if_u1.if_data, ifp->if_bytes);
+ memcpy(buf, ifp->if_data, ifp->if_bytes);
xfs_trans_log_buf(tp, bp, 0, sizeof(struct xfs_dsymlink_hdr) +
ifp->if_bytes - 1);
}
diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index 6c16d9530ccaca..bac6fb2f01d880 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -527,19 +527,15 @@ xchk_xattr_check_sf(
struct xfs_scrub *sc)
{
struct xchk_xattr_buf *ab = sc->buf;
- struct xfs_attr_shortform *sf;
+ struct xfs_ifork *ifp = &sc->ip->i_af;
+ struct xfs_attr_shortform *sf = ifp->if_data;
struct xfs_attr_sf_entry *sfe;
struct xfs_attr_sf_entry *next;
- struct xfs_ifork *ifp;
- unsigned char *end;
+ unsigned char *end = ifp->if_data + ifp->if_bytes;
int i;
int error = 0;
- ifp = xfs_ifork_ptr(sc->ip, XFS_ATTR_FORK);
-
bitmap_zero(ab->usedmap, ifp->if_bytes);
- sf = (struct xfs_attr_shortform *)sc->ip->i_af.if_u1.if_data;
- end = (unsigned char *)ifp->if_u1.if_data + ifp->if_bytes;
xchk_xattr_set_map(sc, ab->usedmap, 0, sizeof(sf->hdr));
sfe = &sf->list[0];
diff --git a/fs/xfs/scrub/readdir.c b/fs/xfs/scrub/readdir.c
index e51c1544be6323..16462332c897b1 100644
--- a/fs/xfs/scrub/readdir.c
+++ b/fs/xfs/scrub/readdir.c
@@ -36,16 +36,14 @@ xchk_dir_walk_sf(
struct xfs_mount *mp = dp->i_mount;
struct xfs_da_geometry *geo = mp->m_dir_geo;
struct xfs_dir2_sf_entry *sfep;
- struct xfs_dir2_sf_hdr *sfp;
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
xfs_ino_t ino;
xfs_dir2_dataptr_t dapos;
unsigned int i;
int error;
ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
- ASSERT(dp->i_df.if_u1.if_data != NULL);
-
- sfp = (struct xfs_dir2_sf_hdr *)dp->i_df.if_u1.if_data;
+ ASSERT(sfp != NULL);
/* dot entry */
dapos = xfs_dir2_db_off_to_dataptr(geo, geo->datablk,
diff --git a/fs/xfs/scrub/symlink.c b/fs/xfs/scrub/symlink.c
index 60643d791d4a22..ddff86713df353 100644
--- a/fs/xfs/scrub/symlink.c
+++ b/fs/xfs/scrub/symlink.c
@@ -61,7 +61,7 @@ xchk_symlink(
/* Inline symlink? */
if (ifp->if_format == XFS_DINODE_FMT_LOCAL) {
if (len > xfs_inode_data_fork_size(ip) ||
- len > strnlen(ifp->if_u1.if_data, xfs_inode_data_fork_size(ip)))
+ len > strnlen(ifp->if_data, xfs_inode_data_fork_size(ip)))
xchk_fblock_set_corrupt(sc, XFS_DATA_FORK, 0);
return 0;
}
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 99bbbe1a0e4478..8700b00e154c98 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -56,12 +56,11 @@ xfs_attr_shortform_list(
struct xfs_attrlist_cursor_kern *cursor = &context->cursor;
struct xfs_inode *dp = context->dp;
struct xfs_attr_sf_sort *sbuf, *sbp;
- struct xfs_attr_shortform *sf;
+ struct xfs_attr_shortform *sf = dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
int sbsize, nsbuf, count, i;
int error = 0;
- sf = (struct xfs_attr_shortform *)dp->i_af.if_u1.if_data;
ASSERT(sf != NULL);
if (!sf->hdr.count)
return 0;
diff --git a/fs/xfs/xfs_dir2_readdir.c b/fs/xfs/xfs_dir2_readdir.c
index 57f42c2af0a316..cc6dc56f455d0c 100644
--- a/fs/xfs/xfs_dir2_readdir.c
+++ b/fs/xfs/xfs_dir2_readdir.c
@@ -52,7 +52,7 @@ xfs_dir2_sf_getdents(
struct xfs_mount *mp = dp->i_mount;
xfs_dir2_dataptr_t off; /* current entry's offset */
xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
- xfs_dir2_sf_hdr_t *sfp; /* shortform structure */
+ struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
xfs_dir2_dataptr_t dot_offset;
xfs_dir2_dataptr_t dotdot_offset;
xfs_ino_t ino;
@@ -60,9 +60,7 @@ xfs_dir2_sf_getdents(
ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
- ASSERT(dp->i_df.if_u1.if_data != NULL);
-
- sfp = (xfs_dir2_sf_hdr_t *)dp->i_df.if_u1.if_data;
+ ASSERT(sfp != NULL);
/*
* If the block number in the offset is out of range, we're done.
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 1ffc8dfa2a52ce..1fd94958aa97aa 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -872,7 +872,7 @@ xfs_init_new_inode(
case S_IFLNK:
ip->i_df.if_format = XFS_DINODE_FMT_EXTENTS;
ip->i_df.if_bytes = 0;
- ip->i_df.if_u1.if_root = NULL;
+ ip->i_df.if_data = NULL;
break;
default:
ASSERT(0);
@@ -2378,8 +2378,8 @@ xfs_ifree(
* already been freed by xfs_attr_inactive.
*/
if (ip->i_df.if_format == XFS_DINODE_FMT_LOCAL) {
- kmem_free(ip->i_df.if_u1.if_data);
- ip->i_df.if_u1.if_data = NULL;
+ kmem_free(ip->i_df.if_data);
+ ip->i_df.if_data = NULL;
ip->i_df.if_bytes = 0;
}
diff --git a/fs/xfs/xfs_inode_item.c b/fs/xfs/xfs_inode_item.c
index b35335e20342c7..0aee97ba0be81f 100644
--- a/fs/xfs/xfs_inode_item.c
+++ b/fs/xfs/xfs_inode_item.c
@@ -352,11 +352,10 @@ xfs_inode_item_format_data_fork(
~(XFS_ILOG_DEXT | XFS_ILOG_DBROOT | XFS_ILOG_DEV);
if ((iip->ili_fields & XFS_ILOG_DDATA) &&
ip->i_df.if_bytes > 0) {
- ASSERT(ip->i_df.if_u1.if_data != NULL);
+ ASSERT(ip->i_df.if_data != NULL);
ASSERT(ip->i_disk_size > 0);
xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_ILOCAL,
- ip->i_df.if_u1.if_data,
- ip->i_df.if_bytes);
+ ip->i_df.if_data, ip->i_df.if_bytes);
ilf->ilf_dsize = (unsigned)ip->i_df.if_bytes;
ilf->ilf_size++;
} else {
@@ -431,10 +430,9 @@ xfs_inode_item_format_attr_fork(
if ((iip->ili_fields & XFS_ILOG_ADATA) &&
ip->i_af.if_bytes > 0) {
- ASSERT(ip->i_af.if_u1.if_data != NULL);
+ ASSERT(ip->i_af.if_data != NULL);
xlog_copy_iovec(lv, vecp, XLOG_REG_TYPE_IATTR_LOCAL,
- ip->i_af.if_u1.if_data,
- ip->i_af.if_bytes);
+ ip->i_af.if_data, ip->i_af.if_bytes);
ilf->ilf_asize = (unsigned)ip->i_af.if_bytes;
ilf->ilf_size++;
} else {
diff --git a/fs/xfs/xfs_symlink.c b/fs/xfs/xfs_symlink.c
index 7c713727f7fd37..92974a4414c832 100644
--- a/fs/xfs/xfs_symlink.c
+++ b/fs/xfs/xfs_symlink.c
@@ -131,10 +131,10 @@ xfs_readlink(
* The VFS crashes on a NULL pointer, so return -EFSCORRUPTED
* if if_data is junk.
*/
- if (XFS_IS_CORRUPT(ip->i_mount, !ip->i_df.if_u1.if_data))
+ if (XFS_IS_CORRUPT(ip->i_mount, !ip->i_df.if_data))
goto out;
- memcpy(link, ip->i_df.if_u1.if_data, pathlen + 1);
+ memcpy(link, ip->i_df.if_data, pathlen + 1);
error = 0;
} else {
error = xfs_readlink_bmap_ilocked(ip, link);
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 2/8] xfs: return if_data from xfs_idata_realloc
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
2023-12-19 12:08 ` [PATCH 1/8] xfs: make if_data a void pointer Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
2023-12-19 12:08 ` [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint Christoph Hellwig
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
Many of the xfs_idata_realloc callers need to set a local pointer to the
just reallocated if_data memory. Return the pointer to simplify them a
bit and use the opportunity to re-use krealloc for freeing if_data if the
size hits 0.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 7 +++----
fs/xfs/libxfs/xfs_dir2_sf.c | 25 ++++++++++---------------
fs/xfs/libxfs/xfs_inode_fork.c | 20 ++++++++------------
fs/xfs/libxfs/xfs_inode_fork.h | 2 +-
4 files changed, 22 insertions(+), 32 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 3e5377fd498471..2e3334ac32287a 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -690,8 +690,8 @@ xfs_attr_shortform_create(
ASSERT(ifp->if_bytes == 0);
if (ifp->if_format == XFS_DINODE_FMT_EXTENTS)
ifp->if_format = XFS_DINODE_FMT_LOCAL;
- xfs_idata_realloc(dp, sizeof(*hdr), XFS_ATTR_FORK);
- hdr = ifp->if_data;
+
+ hdr = xfs_idata_realloc(dp, sizeof(*hdr), XFS_ATTR_FORK);
memset(hdr, 0, sizeof(*hdr));
hdr->totsize = cpu_to_be16(sizeof(*hdr));
xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_ADATA);
@@ -767,8 +767,7 @@ xfs_attr_shortform_add(
offset = (char *)sfe - (char *)sf;
size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
- xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
- sf = ifp->if_data;
+ sf = xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
sfe->namelen = args->namelen;
diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
index 0b63138d2b9f0e..e1f83fc7b6ad11 100644
--- a/fs/xfs/libxfs/xfs_dir2_sf.c
+++ b/fs/xfs/libxfs/xfs_dir2_sf.c
@@ -466,12 +466,11 @@ xfs_dir2_sf_addname_easy(
/*
* Grow the in-inode space.
*/
- xfs_idata_realloc(dp, xfs_dir2_sf_entsize(mp, sfp, args->namelen),
+ sfp = xfs_idata_realloc(dp, xfs_dir2_sf_entsize(mp, sfp, args->namelen),
XFS_DATA_FORK);
/*
* Need to set up again due to realloc of the inode data.
*/
- sfp = dp->i_df.if_data;
sfep = (xfs_dir2_sf_entry_t *)((char *)sfp + byteoff);
/*
* Fill in the new entry.
@@ -551,11 +550,8 @@ xfs_dir2_sf_addname_hard(
* the data.
*/
xfs_idata_realloc(dp, -old_isize, XFS_DATA_FORK);
- xfs_idata_realloc(dp, new_isize, XFS_DATA_FORK);
- /*
- * Reset the pointer since the buffer was reallocated.
- */
- sfp = dp->i_df.if_data;
+ sfp = xfs_idata_realloc(dp, new_isize, XFS_DATA_FORK);
+
/*
* Copy the first part of the directory, including the header.
*/
@@ -820,15 +816,13 @@ xfs_dir2_sf_create(
ASSERT(dp->i_df.if_bytes == 0);
i8count = pino > XFS_DIR2_MAX_SHORT_INUM;
size = xfs_dir2_sf_hdr_size(i8count);
+
/*
- * Make a buffer for the data.
- */
- xfs_idata_realloc(dp, size, XFS_DATA_FORK);
- /*
- * Fill in the header,
+ * Make a buffer for the data and fill in the header.
*/
- sfp = dp->i_df.if_data;
+ sfp = xfs_idata_realloc(dp, size, XFS_DATA_FORK);
sfp->i8count = i8count;
+
/*
* Now can put in the inode number, since i8count is set.
*/
@@ -976,11 +970,12 @@ xfs_dir2_sf_removename(
*/
sfp->count--;
dp->i_disk_size = newsize;
+
/*
* Reallocate, making it smaller.
*/
- xfs_idata_realloc(dp, newsize - oldsize, XFS_DATA_FORK);
- sfp = dp->i_df.if_data;
+ sfp = xfs_idata_realloc(dp, newsize - oldsize, XFS_DATA_FORK);
+
/*
* Are we changing inode number size?
*/
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index d23910e503a1ae..d8405a8d3c14f9 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -496,7 +496,7 @@ xfs_iroot_realloc(
* byte_diff -- the change in the number of bytes, positive or negative,
* requested for the if_data array.
*/
-void
+void *
xfs_idata_realloc(
struct xfs_inode *ip,
int64_t byte_diff,
@@ -508,19 +508,15 @@ xfs_idata_realloc(
ASSERT(new_size >= 0);
ASSERT(new_size <= xfs_inode_fork_size(ip, whichfork));
- if (byte_diff == 0)
- return;
-
- if (new_size == 0) {
- kmem_free(ifp->if_data);
- ifp->if_data = NULL;
- ifp->if_bytes = 0;
- return;
+ if (byte_diff) {
+ ifp->if_data = krealloc(ifp->if_data, new_size,
+ GFP_NOFS | __GFP_NOFAIL);
+ if (new_size == 0)
+ ifp->if_data = NULL;
+ ifp->if_bytes = new_size;
}
- ifp->if_data = krealloc(ifp->if_data, new_size,
- GFP_NOFS | __GFP_NOFAIL);
- ifp->if_bytes = new_size;
+ return ifp->if_data;
}
/* Free all memory and reset a fork back to its initial state. */
diff --git a/fs/xfs/libxfs/xfs_inode_fork.h b/fs/xfs/libxfs/xfs_inode_fork.h
index 7edcf0e8cd5388..96303249d28ab4 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.h
+++ b/fs/xfs/libxfs/xfs_inode_fork.h
@@ -168,7 +168,7 @@ int xfs_iformat_attr_fork(struct xfs_inode *, struct xfs_dinode *);
void xfs_iflush_fork(struct xfs_inode *, struct xfs_dinode *,
struct xfs_inode_log_item *, int);
void xfs_idestroy_fork(struct xfs_ifork *ifp);
-void xfs_idata_realloc(struct xfs_inode *ip, int64_t byte_diff,
+void * xfs_idata_realloc(struct xfs_inode *ip, int64_t byte_diff,
int whichfork);
void xfs_iroot_realloc(struct xfs_inode *, int, int);
int xfs_iread_extents(struct xfs_trans *, struct xfs_inode *, int);
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
2023-12-19 12:08 ` [PATCH 1/8] xfs: make if_data a void pointer Christoph Hellwig
2023-12-19 12:08 ` [PATCH 2/8] xfs: return if_data from xfs_idata_realloc Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
2023-12-19 12:08 ` [PATCH 4/8] xfs: simplify xfs_attr_sf_findname Christoph Hellwig
` (4 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
trace_xfs_attr_sf_lookup is currently only called by
xfs_attr_shortform_lookup, which despit it's name is a simple helper for
xfs_attr_shortform_addname, which has it's own tracing. Move the
callsite to xfs_attr_shortform_getvalue, which is the closest thing to
a high level lookup we have for the Linux xattr API.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 2e3334ac32287a..37474af8ee4633 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -876,8 +876,6 @@ xfs_attr_shortform_lookup(
struct xfs_attr_sf_entry *sfe;
int i;
- trace_xfs_attr_sf_lookup(args);
-
ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
sfe = &sf->list[0];
for (i = 0; i < sf->hdr.count;
@@ -905,6 +903,9 @@ xfs_attr_shortform_getvalue(
int i;
ASSERT(args->dp->i_af.if_format == XFS_DINODE_FMT_LOCAL);
+
+ trace_xfs_attr_sf_lookup(args);
+
sfe = &sf->list[0];
for (i = 0; i < sf->hdr.count;
sfe = xfs_attr_sf_nextentry(sfe), i++) {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 4/8] xfs: simplify xfs_attr_sf_findname
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
` (2 preceding siblings ...)
2023-12-19 12:08 ` [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
2023-12-19 12:08 ` [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup Christoph Hellwig
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
xfs_attr_sf_findname has the simple job of finding a xfs_attr_sf_entry in
the attr fork, but the convoluted calling convention obfuscates that.
Return the found entry as the return value instead of an pointer
argument, as the -ENOATTR/-EEXIST can be trivally derived from that, and
remove the basep argument, as it is equivalent of the offset of sfe in
the data for if an sfe was found, or an offset of totsize if not was
found. To simplify the totsize computation add a xfs_attr_sf_endptr
helper that returns the imaginative xfs_attr_sf_entry at the end of
the current attrs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr.c | 7 ++-
fs/xfs/libxfs/xfs_attr_leaf.c | 96 +++++++++++++----------------------
fs/xfs/libxfs/xfs_attr_leaf.h | 4 +-
fs/xfs/libxfs/xfs_attr_sf.h | 7 +++
4 files changed, 48 insertions(+), 66 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 7f822e72dfcd3e..bcf8748cb1a333 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -862,8 +862,11 @@ xfs_attr_lookup(
if (!xfs_inode_hasattr(dp))
return -ENOATTR;
- if (dp->i_af.if_format == XFS_DINODE_FMT_LOCAL)
- return xfs_attr_sf_findname(args, NULL, NULL);
+ if (dp->i_af.if_format == XFS_DINODE_FMT_LOCAL) {
+ if (xfs_attr_sf_findname(args))
+ return -EEXIST;
+ return -ENOATTR;
+ }
if (xfs_attr_is_leaf(dp)) {
error = xfs_attr_leaf_hasname(args, &bp);
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 37474af8ee4633..7a623efd23a6a4 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -698,47 +698,24 @@ xfs_attr_shortform_create(
}
/*
- * Return -EEXIST if attr is found, or -ENOATTR if not
- * args: args containing attribute name and namelen
- * sfep: If not null, pointer will be set to the last attr entry found on
- -EEXIST. On -ENOATTR pointer is left at the last entry in the list
- * basep: If not null, pointer is set to the byte offset of the entry in the
- * list on -EEXIST. On -ENOATTR, pointer is left at the byte offset of
- * the last entry in the list
+ * Return the entry if the attr in args is found, or NULL if not.
*/
-int
+struct xfs_attr_sf_entry *
xfs_attr_sf_findname(
- struct xfs_da_args *args,
- struct xfs_attr_sf_entry **sfep,
- unsigned int *basep)
+ struct xfs_da_args *args)
{
- struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
- struct xfs_attr_sf_entry *sfe;
- unsigned int base = sizeof(struct xfs_attr_sf_hdr);
- int size = 0;
- int end;
- int i;
+ struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
+ struct xfs_attr_sf_entry *sfe;
- sfe = &sf->list[0];
- end = sf->hdr.count;
- for (i = 0; i < end; sfe = xfs_attr_sf_nextentry(sfe),
- base += size, i++) {
- size = xfs_attr_sf_entsize(sfe);
- if (!xfs_attr_match(args, sfe->namelen, sfe->nameval,
- sfe->flags))
- continue;
- break;
+ for (sfe = &sf->list[0];
+ sfe < xfs_attr_sf_endptr(sf);
+ sfe = xfs_attr_sf_nextentry(sfe)) {
+ if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
+ sfe->flags))
+ return sfe;
}
- if (sfep != NULL)
- *sfep = sfe;
-
- if (basep != NULL)
- *basep = base;
-
- if (i == end)
- return -ENOATTR;
- return -EEXIST;
+ return NULL;
}
/*
@@ -755,21 +732,19 @@ xfs_attr_shortform_add(
struct xfs_ifork *ifp = &dp->i_af;
struct xfs_attr_shortform *sf = ifp->if_data;
struct xfs_attr_sf_entry *sfe;
- int offset, size;
+ int size;
trace_xfs_attr_sf_add(args);
dp->i_forkoff = forkoff;
ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
- if (xfs_attr_sf_findname(args, &sfe, NULL) == -EEXIST)
- ASSERT(0);
+ ASSERT(!xfs_attr_sf_findname(args));
- offset = (char *)sfe - (char *)sf;
size = xfs_attr_sf_entsize_byname(args->namelen, args->valuelen);
sf = xfs_idata_realloc(dp, size, XFS_ATTR_FORK);
- sfe = (struct xfs_attr_sf_entry *)((char *)sf + offset);
+ sfe = xfs_attr_sf_endptr(sf);
sfe->namelen = args->namelen;
sfe->valuelen = args->valuelen;
sfe->flags = args->attr_filter;
@@ -809,39 +784,38 @@ xfs_attr_sf_removename(
struct xfs_mount *mp = dp->i_mount;
struct xfs_attr_shortform *sf = dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
- int size = 0, end, totsize;
- unsigned int base;
- int error;
+ uint16_t totsize = be16_to_cpu(sf->hdr.totsize);
+ void *next, *end;
+ int size = 0;
trace_xfs_attr_sf_remove(args);
- error = xfs_attr_sf_findname(args, &sfe, &base);
-
- /*
- * If we are recovering an operation, finding nothing to
- * remove is not an error - it just means there was nothing
- * to clean up.
- */
- if (error == -ENOATTR && (args->op_flags & XFS_DA_OP_RECOVERY))
- return 0;
- if (error != -EEXIST)
- return error;
- size = xfs_attr_sf_entsize(sfe);
+ sfe = xfs_attr_sf_findname(args);
+ if (!sfe) {
+ /*
+ * If we are recovering an operation, finding nothing to remove
+ * is not an error, it just means there was nothing to clean up.
+ */
+ if (args->op_flags & XFS_DA_OP_RECOVERY)
+ return 0;
+ return -ENOATTR;
+ }
/*
* Fix up the attribute fork data, covering the hole
*/
- end = base + size;
- totsize = be16_to_cpu(sf->hdr.totsize);
- if (end != totsize)
- memmove(&((char *)sf)[base], &((char *)sf)[end], totsize - end);
+ size = xfs_attr_sf_entsize(sfe);
+ next = xfs_attr_sf_nextentry(sfe);
+ end = xfs_attr_sf_endptr(sf);
+ if (next < end)
+ memmove(sfe, next, end - next);
sf->hdr.count--;
- be16_add_cpu(&sf->hdr.totsize, -size);
+ totsize -= size;
+ sf->hdr.totsize = cpu_to_be16(totsize);
/*
* Fix up the start offset of the attribute fork
*/
- totsize -= size;
if (totsize == sizeof(xfs_attr_sf_hdr_t) && xfs_has_attr2(mp) &&
(dp->i_df.if_format != XFS_DINODE_FMT_BTREE) &&
!(args->op_flags & (XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE))) {
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index ce6743463c8681..56fcd689eedfe7 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -51,9 +51,7 @@ int xfs_attr_shortform_lookup(struct xfs_da_args *args);
int xfs_attr_shortform_getvalue(struct xfs_da_args *args);
int xfs_attr_shortform_to_leaf(struct xfs_da_args *args);
int xfs_attr_sf_removename(struct xfs_da_args *args);
-int xfs_attr_sf_findname(struct xfs_da_args *args,
- struct xfs_attr_sf_entry **sfep,
- unsigned int *basep);
+struct xfs_attr_sf_entry *xfs_attr_sf_findname(struct xfs_da_args *args);
int xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
int xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes);
xfs_failaddr_t xfs_attr_shortform_verify(struct xfs_attr_shortform *sfp,
diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
index 37578b369d9b98..a774d4d8776354 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -48,4 +48,11 @@ xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
return (void *)sfep + xfs_attr_sf_entsize(sfep);
}
+/* pointer to the space after the last entry, e.g. for adding a new one */
+static inline struct xfs_attr_sf_entry *
+xfs_attr_sf_endptr(struct xfs_attr_shortform *sf)
+{
+ return (void *)sf + be16_to_cpu(sf->hdr.totsize);
+}
+
#endif /* __XFS_ATTR_SF_H__ */
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
` (3 preceding siblings ...)
2023-12-19 12:08 ` [PATCH 4/8] xfs: simplify xfs_attr_sf_findname Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
2023-12-19 14:46 ` Christoph Hellwig
2023-12-19 12:08 ` [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue Christoph Hellwig
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
xfs_attr_shortform_lookup is only used by xfs_attr_shortform_addname,
which is much better served by calling xfs_attr_sf_findname. Switch
it over and remove xfs_attr_shortform_lookup.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr.c | 16 ++++------------
fs/xfs/libxfs/xfs_attr_leaf.c | 24 ------------------------
fs/xfs/libxfs/xfs_attr_leaf.h | 1 -
3 files changed, 4 insertions(+), 37 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index bcf8748cb1a333..d6173888ed0d56 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1070,13 +1070,7 @@ xfs_attr_shortform_addname(
trace_xfs_attr_sf_addname(args);
- error = xfs_attr_shortform_lookup(args);
- switch (error) {
- case -ENOATTR:
- if (args->op_flags & XFS_DA_OP_REPLACE)
- return error;
- break;
- case -EEXIST:
+ if (xfs_attr_sf_findname(args)) {
if (!(args->op_flags & XFS_DA_OP_REPLACE))
return error;
@@ -1091,11 +1085,9 @@ xfs_attr_shortform_addname(
* around.
*/
args->op_flags &= ~XFS_DA_OP_REPLACE;
- break;
- case 0:
- break;
- default:
- return error;
+ } else {
+ if (args->op_flags & XFS_DA_OP_REPLACE)
+ return error;
}
if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 7a623efd23a6a4..75c597805ffa8b 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -837,30 +837,6 @@ xfs_attr_sf_removename(
return 0;
}
-/*
- * Look up a name in a shortform attribute list structure.
- */
-/*ARGSUSED*/
-int
-xfs_attr_shortform_lookup(
- struct xfs_da_args *args)
-{
- struct xfs_ifork *ifp = &args->dp->i_af;
- struct xfs_attr_shortform *sf = ifp->if_data;
- struct xfs_attr_sf_entry *sfe;
- int i;
-
- ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
- sfe = &sf->list[0];
- for (i = 0; i < sf->hdr.count;
- sfe = xfs_attr_sf_nextentry(sfe), i++) {
- if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
- sfe->flags))
- return -EEXIST;
- }
- return -ENOATTR;
-}
-
/*
* Retrieve the attribute value and length.
*
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index 56fcd689eedfe7..35e668ae744fb1 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -47,7 +47,6 @@ struct xfs_attr3_icleaf_hdr {
*/
void xfs_attr_shortform_create(struct xfs_da_args *args);
void xfs_attr_shortform_add(struct xfs_da_args *args, int forkoff);
-int xfs_attr_shortform_lookup(struct xfs_da_args *args);
int xfs_attr_shortform_getvalue(struct xfs_da_args *args);
int xfs_attr_shortform_to_leaf(struct xfs_da_args *args);
int xfs_attr_sf_removename(struct xfs_da_args *args);
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup
2023-12-19 12:08 ` [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup Christoph Hellwig
@ 2023-12-19 14:46 ` Christoph Hellwig
2023-12-19 17:45 ` Darrick J. Wong
0 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 14:46 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
So, the buildbot rightly complained that this can return an
uninitialized error variable in xfs_attr_shortform_addname now
(why are we disabling the goddam use of uninitialized variable
warnings in Linux again, sigh..).
I then not only created the trivial fix, but also wrote a simple wrapper
for the setxattr syscall as the existing setfattr and attr tools don't
allow to control the flag, which I assumed means xfstests didn't really
test this as much as it should. But that little test showed we're still
getting the right errno values even with the unininitialized variable
returns, which seemed odd.
It turns out we're not even exercising this code any more, as
xfs_attr_set already does a xfs_attr_lookup lookup first and has a
copy of this logic executed much earlier (and I should have really though
about that because I got very close to that code for the defer ops
cleanup).
So.. I'm tempted to just turn these checks into asserts with something
like the below on top of this patch, I'll just need to see if it survives
testing:
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d6173888ed0d56..abdc58f286154a 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1066,13 +1066,13 @@ xfs_attr_shortform_addname(
struct xfs_da_args *args)
{
int newsize, forkoff;
- int error;
trace_xfs_attr_sf_addname(args);
if (xfs_attr_sf_findname(args)) {
- if (!(args->op_flags & XFS_DA_OP_REPLACE))
- return error;
+ int error;
+
+ ASSERT(args->op_flags & XFS_DA_OP_REPLACE);
error = xfs_attr_sf_removename(args);
if (error)
@@ -1086,8 +1086,7 @@ xfs_attr_shortform_addname(
*/
args->op_flags &= ~XFS_DA_OP_REPLACE;
} else {
- if (args->op_flags & XFS_DA_OP_REPLACE)
- return error;
+ ASSERT(!(args->op_flags & XFS_DA_OP_REPLACE));
}
if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup
2023-12-19 14:46 ` Christoph Hellwig
@ 2023-12-19 17:45 ` Darrick J. Wong
2023-12-20 3:55 ` Christoph Hellwig
0 siblings, 1 reply; 15+ messages in thread
From: Darrick J. Wong @ 2023-12-19 17:45 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Tue, Dec 19, 2023 at 03:46:27PM +0100, Christoph Hellwig wrote:
> So, the buildbot rightly complained that this can return an
> uninitialized error variable in xfs_attr_shortform_addname now
> (why are we disabling the goddam use of uninitialized variable
> warnings in Linux again, sigh..).
>
> I then not only created the trivial fix, but also wrote a simple wrapper
> for the setxattr syscall as the existing setfattr and attr tools don't
> allow to control the flag, which I assumed means xfstests didn't really
> test this as much as it should. But that little test showed we're still
> getting the right errno values even with the unininitialized variable
> returns, which seemed odd.
>
> It turns out we're not even exercising this code any more, as
> xfs_attr_set already does a xfs_attr_lookup lookup first and has a
> copy of this logic executed much earlier (and I should have really though
> about that because I got very close to that code for the defer ops
> cleanup).
Eh, there's lots of, uh, cleanup opportunities in the xattr code. ;)
The changes below look reasonable, but I wonder -- the leaf and node add
functions do a similar thing; can they go too?
I'm assuming those can't go away because they actually set @args->index
and @args->rmt* and we might've blown that away after the initial lookup
in xfs_attr_set? But maybe they can? Insofar as figuring all that out
is probably an entire campaign on its own.
> So.. I'm tempted to just turn these checks into asserts with something
> like the below on top of this patch, I'll just need to see if it survives
> testing:
I'll await your return then. :)
--D
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index d6173888ed0d56..abdc58f286154a 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1066,13 +1066,13 @@ xfs_attr_shortform_addname(
> struct xfs_da_args *args)
> {
> int newsize, forkoff;
> - int error;
>
> trace_xfs_attr_sf_addname(args);
>
> if (xfs_attr_sf_findname(args)) {
> - if (!(args->op_flags & XFS_DA_OP_REPLACE))
> - return error;
> + int error;
> +
> + ASSERT(args->op_flags & XFS_DA_OP_REPLACE);
>
> error = xfs_attr_sf_removename(args);
> if (error)
> @@ -1086,8 +1086,7 @@ xfs_attr_shortform_addname(
> */
> args->op_flags &= ~XFS_DA_OP_REPLACE;
> } else {
> - if (args->op_flags & XFS_DA_OP_REPLACE)
> - return error;
> + ASSERT(!(args->op_flags & XFS_DA_OP_REPLACE));
> }
>
> if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup
2023-12-19 17:45 ` Darrick J. Wong
@ 2023-12-20 3:55 ` Christoph Hellwig
0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-20 3:55 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Tue, Dec 19, 2023 at 09:45:05AM -0800, Darrick J. Wong wrote:
> Eh, there's lots of, uh, cleanup opportunities in the xattr code. ;)
>
> The changes below look reasonable, but I wonder -- the leaf and node add
> functions do a similar thing; can they go too?
>
> I'm assuming those can't go away because they actually set @args->index
> and @args->rmt* and we might've blown that away after the initial lookup
> in xfs_attr_set? But maybe they can? Insofar as figuring all that out
> is probably an entire campaign on its own.
Yeah, this looks pretty scary to touch for a cleanup series that's
already gone kinda out of bounds..
>
> > So.. I'm tempted to just turn these checks into asserts with something
> > like the below on top of this patch, I'll just need to see if it survives
> > testing:
>
> I'll await your return then. :)
It has been surviving testing just fine over night.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
` (4 preceding siblings ...)
2023-12-19 12:08 ` [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
2023-12-19 12:08 ` [PATCH 7/8] xfs: remove struct xfs_attr_shortform Christoph Hellwig
2023-12-19 12:08 ` [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t Christoph Hellwig
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
xfs_attr_shortform_getvalue duplicates the logic in xfs_attr_sf_findname.
Use the helper instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 75c597805ffa8b..82e1830334160b 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -848,23 +848,17 @@ int
xfs_attr_shortform_getvalue(
struct xfs_da_args *args)
{
- struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
- int i;
ASSERT(args->dp->i_af.if_format == XFS_DINODE_FMT_LOCAL);
trace_xfs_attr_sf_lookup(args);
- sfe = &sf->list[0];
- for (i = 0; i < sf->hdr.count;
- sfe = xfs_attr_sf_nextentry(sfe), i++) {
- if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
- sfe->flags))
- return xfs_attr_copy_value(args,
- &sfe->nameval[args->namelen], sfe->valuelen);
- }
- return -ENOATTR;
+ sfe = xfs_attr_sf_findname(args);
+ if (!sfe)
+ return -ENOATTR;
+ return xfs_attr_copy_value(args, &sfe->nameval[args->namelen],
+ sfe->valuelen);
}
/* Convert from using the shortform to the leaf format. */
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 7/8] xfs: remove struct xfs_attr_shortform
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
` (5 preceding siblings ...)
2023-12-19 12:08 ` [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
2023-12-19 17:35 ` Darrick J. Wong
2023-12-19 12:08 ` [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t Christoph Hellwig
7 siblings, 1 reply; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
sparse complains about struct xfs_attr_shortform because it embeds a
structure with a variable sized array in a variable sized array.
Given that xfs_attr_shortform is not a very useful structure, and the
dir2 equivalent has been removed a long time ago, remove it as well.
Provide a xfs_attr_sf_firstentry helper that returns the first
xfs_attr_sf_entry behind a xfs_attr_sf_hdr to replace the structure
dereference.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_attr.c | 4 ++--
fs/xfs/libxfs/xfs_attr_leaf.c | 37 +++++++++++++++++-----------------
fs/xfs/libxfs/xfs_attr_leaf.h | 2 +-
fs/xfs/libxfs/xfs_attr_sf.h | 13 +++++++++---
fs/xfs/libxfs/xfs_da_format.h | 33 +++++++++++++++++-------------
fs/xfs/libxfs/xfs_inode_fork.c | 5 ++---
fs/xfs/libxfs/xfs_ondisk.h | 14 ++++++-------
fs/xfs/scrub/attr.c | 9 ++++-----
fs/xfs/scrub/inode_repair.c | 4 ++--
fs/xfs/xfs_attr_list.c | 12 +++++------
10 files changed, 71 insertions(+), 62 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index d6173888ed0d56..846555de1cfccb 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -1052,9 +1052,9 @@ xfs_attr_set(
static inline int xfs_attr_sf_totsize(struct xfs_inode *dp)
{
- struct xfs_attr_shortform *sf = dp->i_af.if_data;
+ struct xfs_attr_sf_hdr *sf = dp->i_af.if_data;
- return be16_to_cpu(sf->hdr.totsize);
+ return be16_to_cpu(sf->totsize);
}
/*
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 82e1830334160b..e1281ab413c832 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -704,10 +704,10 @@ struct xfs_attr_sf_entry *
xfs_attr_sf_findname(
struct xfs_da_args *args)
{
- struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
+ struct xfs_attr_sf_hdr *sf = args->dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
- for (sfe = &sf->list[0];
+ for (sfe = xfs_attr_sf_firstentry(sf);
sfe < xfs_attr_sf_endptr(sf);
sfe = xfs_attr_sf_nextentry(sfe)) {
if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
@@ -730,7 +730,7 @@ xfs_attr_shortform_add(
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
struct xfs_ifork *ifp = &dp->i_af;
- struct xfs_attr_shortform *sf = ifp->if_data;
+ struct xfs_attr_sf_hdr *sf = ifp->if_data;
struct xfs_attr_sf_entry *sfe;
int size;
@@ -750,8 +750,8 @@ xfs_attr_shortform_add(
sfe->flags = args->attr_filter;
memcpy(sfe->nameval, args->name, args->namelen);
memcpy(&sfe->nameval[args->namelen], args->value, args->valuelen);
- sf->hdr.count++;
- be16_add_cpu(&sf->hdr.totsize, size);
+ sf->count++;
+ be16_add_cpu(&sf->totsize, size);
xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_ADATA);
xfs_sbversion_add_attr2(mp, args->trans);
@@ -782,9 +782,9 @@ xfs_attr_sf_removename(
{
struct xfs_inode *dp = args->dp;
struct xfs_mount *mp = dp->i_mount;
- struct xfs_attr_shortform *sf = dp->i_af.if_data;
+ struct xfs_attr_sf_hdr *sf = dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
- uint16_t totsize = be16_to_cpu(sf->hdr.totsize);
+ uint16_t totsize = be16_to_cpu(sf->totsize);
void *next, *end;
int size = 0;
@@ -809,9 +809,9 @@ xfs_attr_sf_removename(
end = xfs_attr_sf_endptr(sf);
if (next < end)
memmove(sfe, next, end - next);
- sf->hdr.count--;
+ sf->count--;
totsize -= size;
- sf->hdr.totsize = cpu_to_be16(totsize);
+ sf->totsize = cpu_to_be16(totsize);
/*
* Fix up the start offset of the attribute fork
@@ -868,21 +868,21 @@ xfs_attr_shortform_to_leaf(
{
struct xfs_inode *dp = args->dp;
struct xfs_ifork *ifp = &dp->i_af;
- struct xfs_attr_shortform *sf = ifp->if_data;
+ struct xfs_attr_sf_hdr *sf = ifp->if_data;
struct xfs_attr_sf_entry *sfe;
+ int size = be16_to_cpu(sf->totsize);
struct xfs_da_args nargs;
char *tmpbuffer;
- int error, i, size;
+ int error, i;
xfs_dablk_t blkno;
struct xfs_buf *bp;
trace_xfs_attr_sf_to_leaf(args);
- size = be16_to_cpu(sf->hdr.totsize);
tmpbuffer = kmem_alloc(size, 0);
ASSERT(tmpbuffer != NULL);
memcpy(tmpbuffer, ifp->if_data, size);
- sf = (struct xfs_attr_shortform *)tmpbuffer;
+ sf = (struct xfs_attr_sf_hdr *)tmpbuffer;
xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
xfs_bmap_local_to_extents_empty(args->trans, dp, XFS_ATTR_FORK);
@@ -905,8 +905,8 @@ xfs_attr_shortform_to_leaf(
nargs.trans = args->trans;
nargs.op_flags = XFS_DA_OP_OKNOENT;
- sfe = &sf->list[0];
- for (i = 0; i < sf->hdr.count; i++) {
+ sfe = xfs_attr_sf_firstentry(sf);
+ for (i = 0; i < sf->count; i++) {
nargs.name = sfe->nameval;
nargs.namelen = sfe->namelen;
nargs.value = &sfe->nameval[nargs.namelen];
@@ -973,10 +973,10 @@ xfs_attr_shortform_allfit(
/* Verify the consistency of a raw inline attribute fork. */
xfs_failaddr_t
xfs_attr_shortform_verify(
- struct xfs_attr_shortform *sfp,
+ struct xfs_attr_sf_hdr *sfp,
size_t size)
{
- struct xfs_attr_sf_entry *sfep;
+ struct xfs_attr_sf_entry *sfep = xfs_attr_sf_firstentry(sfp);
struct xfs_attr_sf_entry *next_sfep;
char *endp;
int i;
@@ -990,8 +990,7 @@ xfs_attr_shortform_verify(
endp = (char *)sfp + size;
/* Check all reported entries */
- sfep = &sfp->list[0];
- for (i = 0; i < sfp->hdr.count; i++) {
+ for (i = 0; i < sfp->count; i++) {
/*
* struct xfs_attr_sf_entry has a variable length.
* Check the fixed-offset parts of the structure are
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index 35e668ae744fb1..9b9948639c0fb3 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -53,7 +53,7 @@ int xfs_attr_sf_removename(struct xfs_da_args *args);
struct xfs_attr_sf_entry *xfs_attr_sf_findname(struct xfs_da_args *args);
int xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
int xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes);
-xfs_failaddr_t xfs_attr_shortform_verify(struct xfs_attr_shortform *sfp,
+xfs_failaddr_t xfs_attr_shortform_verify(struct xfs_attr_sf_hdr *sfp,
size_t size);
void xfs_attr_fork_remove(struct xfs_inode *ip, struct xfs_trans *tp);
diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
index a774d4d8776354..9abf7de95465f5 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -41,7 +41,14 @@ static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep)
return struct_size(sfep, nameval, sfep->namelen + sfep->valuelen);
}
-/* next entry in struct */
+/* first entry in the SF attr fork */
+static inline struct xfs_attr_sf_entry *
+xfs_attr_sf_firstentry(struct xfs_attr_sf_hdr *hdr)
+{
+ return (struct xfs_attr_sf_entry *)(hdr + 1);
+}
+
+/* next entry after sfep */
static inline struct xfs_attr_sf_entry *
xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
{
@@ -50,9 +57,9 @@ xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
/* pointer to the space after the last entry, e.g. for adding a new one */
static inline struct xfs_attr_sf_entry *
-xfs_attr_sf_endptr(struct xfs_attr_shortform *sf)
+xfs_attr_sf_endptr(struct xfs_attr_sf_hdr *sf)
{
- return (void *)sf + be16_to_cpu(sf->hdr.totsize);
+ return (void *)sf + be16_to_cpu(sf->totsize);
}
#endif /* __XFS_ATTR_SF_H__ */
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index f9015f88eca706..24f9d1461f9a6f 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -578,20 +578,25 @@ xfs_dir2_block_leaf_p(struct xfs_dir2_block_tail *btp)
#define XFS_ATTR_LEAF_MAPSIZE 3 /* how many freespace slots */
/*
- * Entries are packed toward the top as tight as possible.
- */
-struct xfs_attr_shortform {
- struct xfs_attr_sf_hdr { /* constant-structure header block */
- __be16 totsize; /* total bytes in shortform list */
- __u8 count; /* count of active entries */
- __u8 padding;
- } hdr;
- struct xfs_attr_sf_entry {
- uint8_t namelen; /* actual length of name (no NULL) */
- uint8_t valuelen; /* actual length of value (no NULL) */
- uint8_t flags; /* flags bits (see xfs_attr_leaf.h) */
- uint8_t nameval[]; /* name & value bytes concatenated */
- } list[]; /* variable sized array */
+ * Attribute storage when stored inside the inode.
+ *
+ * Small attribute lists are packed as tightly as possible so as to fit into the
+ * literal area of the inode.
+ *
+ * These "shortform" attribute forks consist of a single xfs_attr_sf_hdr header
+ * followed by zero or more xfs_attr_sf_entry structures.
+ */
+struct xfs_attr_sf_hdr { /* constant-structure header block */
+ __be16 totsize; /* total bytes in shortform list */
+ __u8 count; /* count of active entries */
+ __u8 padding;
+};
+
+struct xfs_attr_sf_entry {
+ __u8 namelen; /* actual length of name (no NULL) */
+ __u8 valuelen; /* actual length of value (no NULL) */
+ __u8 flags; /* flags bits (XFS_ATTR_*) */
+ __u8 nameval[]; /* name & value bytes concatenated */
};
typedef struct xfs_attr_leaf_map { /* RLE map of free bytes */
diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
index d8405a8d3c14f9..f4569e18a8d0ea 100644
--- a/fs/xfs/libxfs/xfs_inode_fork.c
+++ b/fs/xfs/libxfs/xfs_inode_fork.c
@@ -279,10 +279,9 @@ static uint16_t
xfs_dfork_attr_shortform_size(
struct xfs_dinode *dip)
{
- struct xfs_attr_shortform *atp =
- (struct xfs_attr_shortform *)XFS_DFORK_APTR(dip);
+ struct xfs_attr_sf_hdr *sf = XFS_DFORK_APTR(dip);
- return be16_to_cpu(atp->hdr.totsize);
+ return be16_to_cpu(sf->totsize);
}
void
diff --git a/fs/xfs/libxfs/xfs_ondisk.h b/fs/xfs/libxfs/xfs_ondisk.h
index d9c988c5ad692e..81885a6a028ed7 100644
--- a/fs/xfs/libxfs/xfs_ondisk.h
+++ b/fs/xfs/libxfs/xfs_ondisk.h
@@ -93,13 +93,13 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen, 8);
XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name, 9);
XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 32);
- XFS_CHECK_STRUCT_SIZE(struct xfs_attr_shortform, 4);
- XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0);
- XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count, 2);
- XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen, 4);
- XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].valuelen, 5);
- XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].flags, 6);
- XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].nameval, 7);
+ XFS_CHECK_STRUCT_SIZE(struct xfs_attr_sf_hdr, 4);
+ XFS_CHECK_OFFSET(struct xfs_attr_sf_hdr, totsize, 0);
+ XFS_CHECK_OFFSET(struct xfs_attr_sf_hdr, count, 2);
+ XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, namelen, 0);
+ XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, valuelen, 1);
+ XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, flags, 2);
+ XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, nameval, 3);
XFS_CHECK_STRUCT_SIZE(xfs_da_blkinfo_t, 12);
XFS_CHECK_STRUCT_SIZE(xfs_da_intnode_t, 16);
XFS_CHECK_STRUCT_SIZE(xfs_da_node_entry_t, 8);
diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
index bac6fb2f01d880..83c7feb387147a 100644
--- a/fs/xfs/scrub/attr.c
+++ b/fs/xfs/scrub/attr.c
@@ -528,23 +528,22 @@ xchk_xattr_check_sf(
{
struct xchk_xattr_buf *ab = sc->buf;
struct xfs_ifork *ifp = &sc->ip->i_af;
- struct xfs_attr_shortform *sf = ifp->if_data;
- struct xfs_attr_sf_entry *sfe;
+ struct xfs_attr_sf_hdr *sf = ifp->if_data;
+ struct xfs_attr_sf_entry *sfe = xfs_attr_sf_firstentry(sf);
struct xfs_attr_sf_entry *next;
unsigned char *end = ifp->if_data + ifp->if_bytes;
int i;
int error = 0;
bitmap_zero(ab->usedmap, ifp->if_bytes);
- xchk_xattr_set_map(sc, ab->usedmap, 0, sizeof(sf->hdr));
+ xchk_xattr_set_map(sc, ab->usedmap, 0, sizeof(*sf));
- sfe = &sf->list[0];
if ((unsigned char *)sfe > end) {
xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0);
return 0;
}
- for (i = 0; i < sf->hdr.count; i++) {
+ for (i = 0; i < sf->count; i++) {
unsigned char *name = sfe->nameval;
unsigned char *value = &sfe->nameval[sfe->namelen];
diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
index 66949cc3d7cc9e..0ca62d59f84ad1 100644
--- a/fs/xfs/scrub/inode_repair.c
+++ b/fs/xfs/scrub/inode_repair.c
@@ -760,7 +760,7 @@ xrep_dinode_check_afork(
struct xfs_scrub *sc,
struct xfs_dinode *dip)
{
- struct xfs_attr_shortform *afork_ptr;
+ struct xfs_attr_sf_hdr *afork_ptr;
size_t attr_size;
unsigned int afork_size;
@@ -778,7 +778,7 @@ xrep_dinode_check_afork(
return true;
/* xattr structure cannot be larger than the fork */
- attr_size = be16_to_cpu(afork_ptr->hdr.totsize);
+ attr_size = be16_to_cpu(afork_ptr->totsize);
if (attr_size > afork_size)
return true;
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 8700b00e154c98..e368ad671e261e 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -56,13 +56,13 @@ xfs_attr_shortform_list(
struct xfs_attrlist_cursor_kern *cursor = &context->cursor;
struct xfs_inode *dp = context->dp;
struct xfs_attr_sf_sort *sbuf, *sbp;
- struct xfs_attr_shortform *sf = dp->i_af.if_data;
+ struct xfs_attr_sf_hdr *sf = dp->i_af.if_data;
struct xfs_attr_sf_entry *sfe;
int sbsize, nsbuf, count, i;
int error = 0;
ASSERT(sf != NULL);
- if (!sf->hdr.count)
+ if (!sf->count)
return 0;
trace_xfs_attr_list_sf(context);
@@ -78,8 +78,8 @@ xfs_attr_shortform_list(
*/
if (context->bufsize == 0 ||
(XFS_ISRESET_CURSOR(cursor) &&
- (dp->i_af.if_bytes + sf->hdr.count * 16) < context->bufsize)) {
- for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
+ (dp->i_af.if_bytes + sf->count * 16) < context->bufsize)) {
+ for (i = 0, sfe = xfs_attr_sf_firstentry(sf); i < sf->count; i++) {
if (XFS_IS_CORRUPT(context->dp->i_mount,
!xfs_attr_namecheck(sfe->nameval,
sfe->namelen)))
@@ -108,7 +108,7 @@ xfs_attr_shortform_list(
/*
* It didn't all fit, so we have to sort everything on hashval.
*/
- sbsize = sf->hdr.count * sizeof(*sbuf);
+ sbsize = sf->count * sizeof(*sbuf);
sbp = sbuf = kmem_alloc(sbsize, KM_NOFS);
/*
@@ -116,7 +116,7 @@ xfs_attr_shortform_list(
* the relevant info from only those that match into a buffer.
*/
nsbuf = 0;
- for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
+ for (i = 0, sfe = xfs_attr_sf_firstentry(sf); i < sf->count; i++) {
if (unlikely(
((char *)sfe < (char *)sf) ||
((char *)sfe >= ((char *)sf + dp->i_af.if_bytes)))) {
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 7/8] xfs: remove struct xfs_attr_shortform
2023-12-19 12:08 ` [PATCH 7/8] xfs: remove struct xfs_attr_shortform Christoph Hellwig
@ 2023-12-19 17:35 ` Darrick J. Wong
0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2023-12-19 17:35 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Tue, Dec 19, 2023 at 01:08:16PM +0100, Christoph Hellwig wrote:
> sparse complains about struct xfs_attr_shortform because it embeds a
> structure with a variable sized array in a variable sized array.
>
> Given that xfs_attr_shortform is not a very useful structure, and the
> dir2 equivalent has been removed a long time ago, remove it as well.
>
> Provide a xfs_attr_sf_firstentry helper that returns the first
> xfs_attr_sf_entry behind a xfs_attr_sf_hdr to replace the structure
> dereference.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_attr.c | 4 ++--
> fs/xfs/libxfs/xfs_attr_leaf.c | 37 +++++++++++++++++-----------------
> fs/xfs/libxfs/xfs_attr_leaf.h | 2 +-
> fs/xfs/libxfs/xfs_attr_sf.h | 13 +++++++++---
> fs/xfs/libxfs/xfs_da_format.h | 33 +++++++++++++++++-------------
> fs/xfs/libxfs/xfs_inode_fork.c | 5 ++---
> fs/xfs/libxfs/xfs_ondisk.h | 14 ++++++-------
> fs/xfs/scrub/attr.c | 9 ++++-----
> fs/xfs/scrub/inode_repair.c | 4 ++--
> fs/xfs/xfs_attr_list.c | 12 +++++------
> 10 files changed, 71 insertions(+), 62 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
> index d6173888ed0d56..846555de1cfccb 100644
> --- a/fs/xfs/libxfs/xfs_attr.c
> +++ b/fs/xfs/libxfs/xfs_attr.c
> @@ -1052,9 +1052,9 @@ xfs_attr_set(
>
> static inline int xfs_attr_sf_totsize(struct xfs_inode *dp)
> {
> - struct xfs_attr_shortform *sf = dp->i_af.if_data;
> + struct xfs_attr_sf_hdr *sf = dp->i_af.if_data;
>
> - return be16_to_cpu(sf->hdr.totsize);
> + return be16_to_cpu(sf->totsize);
> }
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 82e1830334160b..e1281ab413c832 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -704,10 +704,10 @@ struct xfs_attr_sf_entry *
> xfs_attr_sf_findname(
> struct xfs_da_args *args)
> {
> - struct xfs_attr_shortform *sf = args->dp->i_af.if_data;
> + struct xfs_attr_sf_hdr *sf = args->dp->i_af.if_data;
> struct xfs_attr_sf_entry *sfe;
>
> - for (sfe = &sf->list[0];
> + for (sfe = xfs_attr_sf_firstentry(sf);
> sfe < xfs_attr_sf_endptr(sf);
> sfe = xfs_attr_sf_nextentry(sfe)) {
> if (xfs_attr_match(args, sfe->namelen, sfe->nameval,
> @@ -730,7 +730,7 @@ xfs_attr_shortform_add(
> struct xfs_inode *dp = args->dp;
> struct xfs_mount *mp = dp->i_mount;
> struct xfs_ifork *ifp = &dp->i_af;
> - struct xfs_attr_shortform *sf = ifp->if_data;
> + struct xfs_attr_sf_hdr *sf = ifp->if_data;
> struct xfs_attr_sf_entry *sfe;
> int size;
>
> @@ -750,8 +750,8 @@ xfs_attr_shortform_add(
> sfe->flags = args->attr_filter;
> memcpy(sfe->nameval, args->name, args->namelen);
> memcpy(&sfe->nameval[args->namelen], args->value, args->valuelen);
> - sf->hdr.count++;
> - be16_add_cpu(&sf->hdr.totsize, size);
> + sf->count++;
> + be16_add_cpu(&sf->totsize, size);
> xfs_trans_log_inode(args->trans, dp, XFS_ILOG_CORE | XFS_ILOG_ADATA);
>
> xfs_sbversion_add_attr2(mp, args->trans);
> @@ -782,9 +782,9 @@ xfs_attr_sf_removename(
> {
> struct xfs_inode *dp = args->dp;
> struct xfs_mount *mp = dp->i_mount;
> - struct xfs_attr_shortform *sf = dp->i_af.if_data;
> + struct xfs_attr_sf_hdr *sf = dp->i_af.if_data;
> struct xfs_attr_sf_entry *sfe;
> - uint16_t totsize = be16_to_cpu(sf->hdr.totsize);
> + uint16_t totsize = be16_to_cpu(sf->totsize);
> void *next, *end;
> int size = 0;
>
> @@ -809,9 +809,9 @@ xfs_attr_sf_removename(
> end = xfs_attr_sf_endptr(sf);
> if (next < end)
> memmove(sfe, next, end - next);
> - sf->hdr.count--;
> + sf->count--;
> totsize -= size;
> - sf->hdr.totsize = cpu_to_be16(totsize);
> + sf->totsize = cpu_to_be16(totsize);
>
> /*
> * Fix up the start offset of the attribute fork
> @@ -868,21 +868,21 @@ xfs_attr_shortform_to_leaf(
> {
> struct xfs_inode *dp = args->dp;
> struct xfs_ifork *ifp = &dp->i_af;
> - struct xfs_attr_shortform *sf = ifp->if_data;
> + struct xfs_attr_sf_hdr *sf = ifp->if_data;
> struct xfs_attr_sf_entry *sfe;
> + int size = be16_to_cpu(sf->totsize);
> struct xfs_da_args nargs;
> char *tmpbuffer;
> - int error, i, size;
> + int error, i;
> xfs_dablk_t blkno;
> struct xfs_buf *bp;
>
> trace_xfs_attr_sf_to_leaf(args);
>
> - size = be16_to_cpu(sf->hdr.totsize);
> tmpbuffer = kmem_alloc(size, 0);
> ASSERT(tmpbuffer != NULL);
> memcpy(tmpbuffer, ifp->if_data, size);
> - sf = (struct xfs_attr_shortform *)tmpbuffer;
> + sf = (struct xfs_attr_sf_hdr *)tmpbuffer;
>
> xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
> xfs_bmap_local_to_extents_empty(args->trans, dp, XFS_ATTR_FORK);
> @@ -905,8 +905,8 @@ xfs_attr_shortform_to_leaf(
> nargs.trans = args->trans;
> nargs.op_flags = XFS_DA_OP_OKNOENT;
>
> - sfe = &sf->list[0];
> - for (i = 0; i < sf->hdr.count; i++) {
> + sfe = xfs_attr_sf_firstentry(sf);
> + for (i = 0; i < sf->count; i++) {
> nargs.name = sfe->nameval;
> nargs.namelen = sfe->namelen;
> nargs.value = &sfe->nameval[nargs.namelen];
> @@ -973,10 +973,10 @@ xfs_attr_shortform_allfit(
> /* Verify the consistency of a raw inline attribute fork. */
> xfs_failaddr_t
> xfs_attr_shortform_verify(
> - struct xfs_attr_shortform *sfp,
> + struct xfs_attr_sf_hdr *sfp,
> size_t size)
> {
> - struct xfs_attr_sf_entry *sfep;
> + struct xfs_attr_sf_entry *sfep = xfs_attr_sf_firstentry(sfp);
> struct xfs_attr_sf_entry *next_sfep;
> char *endp;
> int i;
> @@ -990,8 +990,7 @@ xfs_attr_shortform_verify(
> endp = (char *)sfp + size;
>
> /* Check all reported entries */
> - sfep = &sfp->list[0];
> - for (i = 0; i < sfp->hdr.count; i++) {
> + for (i = 0; i < sfp->count; i++) {
> /*
> * struct xfs_attr_sf_entry has a variable length.
> * Check the fixed-offset parts of the structure are
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
> index 35e668ae744fb1..9b9948639c0fb3 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.h
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.h
> @@ -53,7 +53,7 @@ int xfs_attr_sf_removename(struct xfs_da_args *args);
> struct xfs_attr_sf_entry *xfs_attr_sf_findname(struct xfs_da_args *args);
> int xfs_attr_shortform_allfit(struct xfs_buf *bp, struct xfs_inode *dp);
> int xfs_attr_shortform_bytesfit(struct xfs_inode *dp, int bytes);
> -xfs_failaddr_t xfs_attr_shortform_verify(struct xfs_attr_shortform *sfp,
> +xfs_failaddr_t xfs_attr_shortform_verify(struct xfs_attr_sf_hdr *sfp,
> size_t size);
> void xfs_attr_fork_remove(struct xfs_inode *ip, struct xfs_trans *tp);
>
> diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
> index a774d4d8776354..9abf7de95465f5 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -41,7 +41,14 @@ static inline int xfs_attr_sf_entsize(struct xfs_attr_sf_entry *sfep)
> return struct_size(sfep, nameval, sfep->namelen + sfep->valuelen);
> }
>
> -/* next entry in struct */
> +/* first entry in the SF attr fork */
> +static inline struct xfs_attr_sf_entry *
> +xfs_attr_sf_firstentry(struct xfs_attr_sf_hdr *hdr)
> +{
> + return (struct xfs_attr_sf_entry *)(hdr + 1);
> +}
> +
> +/* next entry after sfep */
> static inline struct xfs_attr_sf_entry *
> xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
> {
> @@ -50,9 +57,9 @@ xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
>
> /* pointer to the space after the last entry, e.g. for adding a new one */
> static inline struct xfs_attr_sf_entry *
> -xfs_attr_sf_endptr(struct xfs_attr_shortform *sf)
> +xfs_attr_sf_endptr(struct xfs_attr_sf_hdr *sf)
> {
> - return (void *)sf + be16_to_cpu(sf->hdr.totsize);
> + return (void *)sf + be16_to_cpu(sf->totsize);
> }
>
> #endif /* __XFS_ATTR_SF_H__ */
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index f9015f88eca706..24f9d1461f9a6f 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -578,20 +578,25 @@ xfs_dir2_block_leaf_p(struct xfs_dir2_block_tail *btp)
> #define XFS_ATTR_LEAF_MAPSIZE 3 /* how many freespace slots */
>
> /*
> - * Entries are packed toward the top as tight as possible.
> - */
> -struct xfs_attr_shortform {
> - struct xfs_attr_sf_hdr { /* constant-structure header block */
> - __be16 totsize; /* total bytes in shortform list */
> - __u8 count; /* count of active entries */
> - __u8 padding;
> - } hdr;
> - struct xfs_attr_sf_entry {
> - uint8_t namelen; /* actual length of name (no NULL) */
> - uint8_t valuelen; /* actual length of value (no NULL) */
> - uint8_t flags; /* flags bits (see xfs_attr_leaf.h) */
> - uint8_t nameval[]; /* name & value bytes concatenated */
> - } list[]; /* variable sized array */
> + * Attribute storage when stored inside the inode.
> + *
> + * Small attribute lists are packed as tightly as possible so as to fit into the
> + * literal area of the inode.
> + *
> + * These "shortform" attribute forks consist of a single xfs_attr_sf_hdr header
> + * followed by zero or more xfs_attr_sf_entry structures.
> + */
> +struct xfs_attr_sf_hdr { /* constant-structure header block */
> + __be16 totsize; /* total bytes in shortform list */
> + __u8 count; /* count of active entries */
> + __u8 padding;
> +};
> +
> +struct xfs_attr_sf_entry {
> + __u8 namelen; /* actual length of name (no NULL) */
> + __u8 valuelen; /* actual length of value (no NULL) */
> + __u8 flags; /* flags bits (XFS_ATTR_*) */
> + __u8 nameval[]; /* name & value bytes concatenated */
> };
>
> typedef struct xfs_attr_leaf_map { /* RLE map of free bytes */
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index d8405a8d3c14f9..f4569e18a8d0ea 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -279,10 +279,9 @@ static uint16_t
> xfs_dfork_attr_shortform_size(
> struct xfs_dinode *dip)
> {
> - struct xfs_attr_shortform *atp =
> - (struct xfs_attr_shortform *)XFS_DFORK_APTR(dip);
> + struct xfs_attr_sf_hdr *sf = XFS_DFORK_APTR(dip);
>
> - return be16_to_cpu(atp->hdr.totsize);
> + return be16_to_cpu(sf->totsize);
> }
>
> void
> diff --git a/fs/xfs/libxfs/xfs_ondisk.h b/fs/xfs/libxfs/xfs_ondisk.h
> index d9c988c5ad692e..81885a6a028ed7 100644
> --- a/fs/xfs/libxfs/xfs_ondisk.h
> +++ b/fs/xfs/libxfs/xfs_ondisk.h
> @@ -93,13 +93,13 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, namelen, 8);
> XFS_CHECK_OFFSET(xfs_attr_leaf_name_remote_t, name, 9);
> XFS_CHECK_STRUCT_SIZE(xfs_attr_leafblock_t, 32);
> - XFS_CHECK_STRUCT_SIZE(struct xfs_attr_shortform, 4);
> - XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.totsize, 0);
> - XFS_CHECK_OFFSET(struct xfs_attr_shortform, hdr.count, 2);
> - XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].namelen, 4);
> - XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].valuelen, 5);
> - XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].flags, 6);
> - XFS_CHECK_OFFSET(struct xfs_attr_shortform, list[0].nameval, 7);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_attr_sf_hdr, 4);
> + XFS_CHECK_OFFSET(struct xfs_attr_sf_hdr, totsize, 0);
> + XFS_CHECK_OFFSET(struct xfs_attr_sf_hdr, count, 2);
> + XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, namelen, 0);
> + XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, valuelen, 1);
> + XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, flags, 2);
> + XFS_CHECK_OFFSET(struct xfs_attr_sf_entry, nameval, 3);
> XFS_CHECK_STRUCT_SIZE(xfs_da_blkinfo_t, 12);
> XFS_CHECK_STRUCT_SIZE(xfs_da_intnode_t, 16);
> XFS_CHECK_STRUCT_SIZE(xfs_da_node_entry_t, 8);
> diff --git a/fs/xfs/scrub/attr.c b/fs/xfs/scrub/attr.c
> index bac6fb2f01d880..83c7feb387147a 100644
> --- a/fs/xfs/scrub/attr.c
> +++ b/fs/xfs/scrub/attr.c
> @@ -528,23 +528,22 @@ xchk_xattr_check_sf(
> {
> struct xchk_xattr_buf *ab = sc->buf;
> struct xfs_ifork *ifp = &sc->ip->i_af;
> - struct xfs_attr_shortform *sf = ifp->if_data;
> - struct xfs_attr_sf_entry *sfe;
> + struct xfs_attr_sf_hdr *sf = ifp->if_data;
> + struct xfs_attr_sf_entry *sfe = xfs_attr_sf_firstentry(sf);
> struct xfs_attr_sf_entry *next;
> unsigned char *end = ifp->if_data + ifp->if_bytes;
> int i;
> int error = 0;
>
> bitmap_zero(ab->usedmap, ifp->if_bytes);
> - xchk_xattr_set_map(sc, ab->usedmap, 0, sizeof(sf->hdr));
> + xchk_xattr_set_map(sc, ab->usedmap, 0, sizeof(*sf));
>
> - sfe = &sf->list[0];
> if ((unsigned char *)sfe > end) {
> xchk_fblock_set_corrupt(sc, XFS_ATTR_FORK, 0);
> return 0;
> }
>
> - for (i = 0; i < sf->hdr.count; i++) {
> + for (i = 0; i < sf->count; i++) {
> unsigned char *name = sfe->nameval;
> unsigned char *value = &sfe->nameval[sfe->namelen];
>
> diff --git a/fs/xfs/scrub/inode_repair.c b/fs/xfs/scrub/inode_repair.c
> index 66949cc3d7cc9e..0ca62d59f84ad1 100644
> --- a/fs/xfs/scrub/inode_repair.c
> +++ b/fs/xfs/scrub/inode_repair.c
> @@ -760,7 +760,7 @@ xrep_dinode_check_afork(
> struct xfs_scrub *sc,
> struct xfs_dinode *dip)
> {
> - struct xfs_attr_shortform *afork_ptr;
> + struct xfs_attr_sf_hdr *afork_ptr;
> size_t attr_size;
> unsigned int afork_size;
>
> @@ -778,7 +778,7 @@ xrep_dinode_check_afork(
> return true;
>
> /* xattr structure cannot be larger than the fork */
> - attr_size = be16_to_cpu(afork_ptr->hdr.totsize);
> + attr_size = be16_to_cpu(afork_ptr->totsize);
> if (attr_size > afork_size)
> return true;
>
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 8700b00e154c98..e368ad671e261e 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -56,13 +56,13 @@ xfs_attr_shortform_list(
> struct xfs_attrlist_cursor_kern *cursor = &context->cursor;
> struct xfs_inode *dp = context->dp;
> struct xfs_attr_sf_sort *sbuf, *sbp;
> - struct xfs_attr_shortform *sf = dp->i_af.if_data;
> + struct xfs_attr_sf_hdr *sf = dp->i_af.if_data;
> struct xfs_attr_sf_entry *sfe;
> int sbsize, nsbuf, count, i;
> int error = 0;
>
> ASSERT(sf != NULL);
> - if (!sf->hdr.count)
> + if (!sf->count)
> return 0;
>
> trace_xfs_attr_list_sf(context);
> @@ -78,8 +78,8 @@ xfs_attr_shortform_list(
> */
> if (context->bufsize == 0 ||
> (XFS_ISRESET_CURSOR(cursor) &&
> - (dp->i_af.if_bytes + sf->hdr.count * 16) < context->bufsize)) {
> - for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
> + (dp->i_af.if_bytes + sf->count * 16) < context->bufsize)) {
> + for (i = 0, sfe = xfs_attr_sf_firstentry(sf); i < sf->count; i++) {
> if (XFS_IS_CORRUPT(context->dp->i_mount,
> !xfs_attr_namecheck(sfe->nameval,
> sfe->namelen)))
> @@ -108,7 +108,7 @@ xfs_attr_shortform_list(
> /*
> * It didn't all fit, so we have to sort everything on hashval.
> */
> - sbsize = sf->hdr.count * sizeof(*sbuf);
> + sbsize = sf->count * sizeof(*sbuf);
> sbp = sbuf = kmem_alloc(sbsize, KM_NOFS);
>
> /*
> @@ -116,7 +116,7 @@ xfs_attr_shortform_list(
> * the relevant info from only those that match into a buffer.
> */
> nsbuf = 0;
> - for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
> + for (i = 0, sfe = xfs_attr_sf_firstentry(sf); i < sf->count; i++) {
> if (unlikely(
> ((char *)sfe < (char *)sf) ||
> ((char *)sfe >= ((char *)sf + dp->i_af.if_bytes)))) {
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t
2023-12-19 12:08 attr cleanups v2 Christoph Hellwig
` (6 preceding siblings ...)
2023-12-19 12:08 ` [PATCH 7/8] xfs: remove struct xfs_attr_shortform Christoph Hellwig
@ 2023-12-19 12:08 ` Christoph Hellwig
7 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2023-12-19 12:08 UTC (permalink / raw)
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
Remove the last two users of the typedef.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++--
fs/xfs/libxfs/xfs_attr_sf.h | 8 --------
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index e1281ab413c832..6374bf10724207 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -816,7 +816,7 @@ xfs_attr_sf_removename(
/*
* Fix up the start offset of the attribute fork
*/
- if (totsize == sizeof(xfs_attr_sf_hdr_t) && xfs_has_attr2(mp) &&
+ if (totsize == sizeof(struct xfs_attr_sf_hdr) && xfs_has_attr2(mp) &&
(dp->i_df.if_format != XFS_DINODE_FMT_BTREE) &&
!(args->op_flags & (XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE))) {
xfs_attr_fork_remove(dp, args->trans);
@@ -824,7 +824,7 @@ xfs_attr_sf_removename(
xfs_idata_realloc(dp, -size, XFS_ATTR_FORK);
dp->i_forkoff = xfs_attr_shortform_bytesfit(dp, totsize);
ASSERT(dp->i_forkoff);
- ASSERT(totsize > sizeof(xfs_attr_sf_hdr_t) ||
+ ASSERT(totsize > sizeof(struct xfs_attr_sf_hdr) ||
(args->op_flags & XFS_DA_OP_ADDNAME) ||
!xfs_has_attr2(mp) ||
dp->i_df.if_format == XFS_DINODE_FMT_BTREE);
diff --git a/fs/xfs/libxfs/xfs_attr_sf.h b/fs/xfs/libxfs/xfs_attr_sf.h
index 9abf7de95465f5..bc442222302486 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -6,14 +6,6 @@
#ifndef __XFS_ATTR_SF_H__
#define __XFS_ATTR_SF_H__
-/*
- * Attribute storage when stored inside the inode.
- *
- * Small attribute lists are packed as tightly as possible so as
- * to fit into the literal area of the inode.
- */
-typedef struct xfs_attr_sf_hdr xfs_attr_sf_hdr_t;
-
/*
* We generate this then sort it, attr_list() must return things in hash-order.
*/
--
2.39.2
^ permalink raw reply related [flat|nested] 15+ messages in thread