* attr cleanups
@ 2014-05-03 15:20 Christoph Hellwig
2014-05-03 16:04 ` Mark Tinguely
0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2014-05-03 15:20 UTC (permalink / raw
To: xfs
A couple random cleanups for the attr code that I had sitting in my tree
for a long time.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: attr cleanups
2014-05-03 15:20 attr cleanups Christoph Hellwig
@ 2014-05-03 16:04 ` Mark Tinguely
2014-05-04 10:16 ` Christoph Hellwig
0 siblings, 1 reply; 31+ messages in thread
From: Mark Tinguely @ 2014-05-03 16:04 UTC (permalink / raw
To: Christoph Hellwig; +Cc: xfs
On 05/03/14 10:20, Christoph Hellwig wrote:
> A couple random cleanups for the attr code that I had sitting in my tree
> for a long time.
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
Depends on how parent inode pointers are implemented, this folding the
internal version of get and set attributes could be undone.
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: attr cleanups
2014-05-03 16:04 ` Mark Tinguely
@ 2014-05-04 10:16 ` Christoph Hellwig
2014-05-04 21:52 ` Dave Chinner
2014-05-05 13:24 ` Mark Tinguely
0 siblings, 2 replies; 31+ messages in thread
From: Christoph Hellwig @ 2014-05-04 10:16 UTC (permalink / raw
To: Mark Tinguely; +Cc: Christoph Hellwig, xfs
On Sat, May 03, 2014 at 11:04:05AM -0500, Mark Tinguely wrote:
> Depends on how parent inode pointers are implemented, this folding the
> internal version of get and set attributes could be undone.
We might have to introduce _locked version at that point. But I'd like
to keep the xfs_name removal and other assorted cleanups.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: attr cleanups
2014-05-04 10:16 ` Christoph Hellwig
@ 2014-05-04 21:52 ` Dave Chinner
2014-05-05 13:24 ` Mark Tinguely
1 sibling, 0 replies; 31+ messages in thread
From: Dave Chinner @ 2014-05-04 21:52 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Mark Tinguely, xfs
On Sun, May 04, 2014 at 12:16:23PM +0200, Christoph Hellwig wrote:
> On Sat, May 03, 2014 at 11:04:05AM -0500, Mark Tinguely wrote:
> > Depends on how parent inode pointers are implemented, this folding the
> > internal version of get and set attributes could be undone.
>
> We might have to introduce _locked version at that point. But I'd like
> to keep the xfs_name removal and other assorted cleanups.
*nod*
If we need to factor the code for new additions, then we should do
that appropriately for the new code when it arrives.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: attr cleanups
2014-05-04 10:16 ` Christoph Hellwig
2014-05-04 21:52 ` Dave Chinner
@ 2014-05-05 13:24 ` Mark Tinguely
2014-05-05 20:55 ` Dave Chinner
1 sibling, 1 reply; 31+ messages in thread
From: Mark Tinguely @ 2014-05-05 13:24 UTC (permalink / raw
To: Christoph Hellwig; +Cc: xfs
On 05/04/14 05:16, Christoph Hellwig wrote:
> On Sat, May 03, 2014 at 11:04:05AM -0500, Mark Tinguely wrote:
>> Depends on how parent inode pointers are implemented, this folding the
>> internal version of get and set attributes could be undone.
>
> We might have to introduce _locked version at that point. But I'd like
> to keep the xfs_name removal and other assorted cleanups.
>
locking is only one issue, xfs_attr_(get/set/remove) are asciii only
whereas the xfs_attr_(get/set/remove)_int versions are more generic. I
am thinking of not just parent inode pointers but a non-ascii character set.
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: attr cleanups
2014-05-05 13:24 ` Mark Tinguely
@ 2014-05-05 20:55 ` Dave Chinner
2014-05-06 19:40 ` Mark Tinguely
0 siblings, 1 reply; 31+ messages in thread
From: Dave Chinner @ 2014-05-05 20:55 UTC (permalink / raw
To: Mark Tinguely; +Cc: Christoph Hellwig, xfs
On Mon, May 05, 2014 at 08:24:35AM -0500, Mark Tinguely wrote:
> On 05/04/14 05:16, Christoph Hellwig wrote:
> >On Sat, May 03, 2014 at 11:04:05AM -0500, Mark Tinguely wrote:
> >>Depends on how parent inode pointers are implemented, this folding the
> >>internal version of get and set attributes could be undone.
> >
> >We might have to introduce _locked version at that point. But I'd like
> >to keep the xfs_name removal and other assorted cleanups.
> >
>
> locking is only one issue, xfs_attr_(get/set/remove) are asciii only
> whereas the xfs_attr_(get/set/remove)_int versions are more generic.
> I am thinking of not just parent inode pointers but a non-ascii
> character set.
I fail to see how they are different. The attribute name is just an
opaque binary blob - only when it is compared externally does it
have any meaning at all.
Character sets are meaningless unless you are doing case
manipulation, in which case we would need to apply the same
treatment as the directory code deep in the internal attribute cod.
i.e It needs case aware compare and hash algorithms. However, the
outer layers are completely unchanged - they just pass through the
blob that was passed to them...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: attr cleanups
2014-05-05 20:55 ` Dave Chinner
@ 2014-05-06 19:40 ` Mark Tinguely
2014-05-06 20:51 ` Dave Chinner
0 siblings, 1 reply; 31+ messages in thread
From: Mark Tinguely @ 2014-05-06 19:40 UTC (permalink / raw
To: Dave Chinner; +Cc: Christoph Hellwig, xfs
On 05/05/14 15:55, Dave Chinner wrote:
> On Mon, May 05, 2014 at 08:24:35AM -0500, Mark Tinguely wrote:
>> On 05/04/14 05:16, Christoph Hellwig wrote:
>>> On Sat, May 03, 2014 at 11:04:05AM -0500, Mark Tinguely wrote:
>>>> Depends on how parent inode pointers are implemented, this folding the
>>>> internal version of get and set attributes could be undone.
>>>
>>> We might have to introduce _locked version at that point. But I'd like
>>> to keep the xfs_name removal and other assorted cleanups.
>>>
>>
>> locking is only one issue, xfs_attr_(get/set/remove) are asciii only
>> whereas the xfs_attr_(get/set/remove)_int versions are more generic.
>> I am thinking of not just parent inode pointers but a non-ascii
>> character set.
>
> I fail to see how they are different. The attribute name is just an
> opaque binary blob - only when it is compared externally does it
> have any meaning at all.
>
> Character sets are meaningless unless you are doing case
> manipulation, in which case we would need to apply the same
> treatment as the directory code deep in the internal attribute cod.
> i.e It needs case aware compare and hash algorithms. However, the
> outer layers are completely unchanged - they just pass through the
> blob that was passed to them...
>
> Cheers,
>
> Dave.
Right now xfs_attr_get(), xfs_attr_set, xfs_attr_remove() expect the
name to be a NULL terminate character array.
The internal versions of these functions have a more useful interface,
they use a blob for the name (the array/length xfs_name structure).
--Mark.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: attr cleanups
2014-05-06 19:40 ` Mark Tinguely
@ 2014-05-06 20:51 ` Dave Chinner
0 siblings, 0 replies; 31+ messages in thread
From: Dave Chinner @ 2014-05-06 20:51 UTC (permalink / raw
To: Mark Tinguely; +Cc: Christoph Hellwig, xfs
On Tue, May 06, 2014 at 02:40:56PM -0500, Mark Tinguely wrote:
> On 05/05/14 15:55, Dave Chinner wrote:
> >On Mon, May 05, 2014 at 08:24:35AM -0500, Mark Tinguely wrote:
> >>On 05/04/14 05:16, Christoph Hellwig wrote:
> >>>On Sat, May 03, 2014 at 11:04:05AM -0500, Mark Tinguely wrote:
> >>>>Depends on how parent inode pointers are implemented, this folding the
> >>>>internal version of get and set attributes could be undone.
> >>>
> >>>We might have to introduce _locked version at that point. But I'd like
> >>>to keep the xfs_name removal and other assorted cleanups.
> >>>
> >>
> >>locking is only one issue, xfs_attr_(get/set/remove) are asciii only
> >>whereas the xfs_attr_(get/set/remove)_int versions are more generic.
> >>I am thinking of not just parent inode pointers but a non-ascii
> >>character set.
> >
> >I fail to see how they are different. The attribute name is just an
> >opaque binary blob - only when it is compared externally does it
> >have any meaning at all.
> >
> >Character sets are meaningless unless you are doing case
> >manipulation, in which case we would need to apply the same
> >treatment as the directory code deep in the internal attribute cod.
> >i.e It needs case aware compare and hash algorithms. However, the
> >outer layers are completely unchanged - they just pass through the
> >blob that was passed to them...
> >
> >Cheers,
> >
> >Dave.
>
> Right now xfs_attr_get(), xfs_attr_set, xfs_attr_remove() expect the
> name to be a NULL terminate character array.
Sure. The obvious solution is simply moving xfs_attr_name_to_xname()
to the callers and passing an xfs_name rather than a char *, as you
have pointed out:
> The internal versions of these functions have a more useful
> interface, they use a blob for the name (the array/length xfs_name
> structure).
... there is nothing that inherently makes those functions only
support character strings.
Like I said, it needs to become more like the directory code - we do
this "convert to opaque xfs_name" via xfs_dentry_to_name() at the
VFS entry layer for directory operations (see xfs_iops.c), and all
the code that uses xattrs should do the same...
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 31+ messages in thread
* attr cleanups
@ 2023-12-17 17:03 Christoph Hellwig
2023-12-17 17:03 ` [PATCH 1/8] xfs: make if_data a void pointer Christoph Hellwig
` (7 more replies)
0 siblings, 8 replies; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
Hi all,
this series started by trying to remove xfs_attr_shortform as sparse
complains about it due using a variable sized array in struct using in a
variable sized array. I ended up cleaning a lot more code around it
once I started looking into it, including some basic cleanups for the
inode fork inline data memory management (I'll have another series for
more work there at a later time).
Note that the dir2 equivalent for the structure has already been removed
long time ago.
Diffstat:
libxfs/xfs_attr.c | 28 ++---
libxfs/xfs_attr_leaf.c | 215 ++++++++++++++------------------------------
libxfs/xfs_attr_leaf.h | 7 -
libxfs/xfs_attr_sf.h | 23 ++--
libxfs/xfs_bmap.c | 4
libxfs/xfs_da_format.h | 30 +++---
libxfs/xfs_dir2.c | 2
libxfs/xfs_dir2_block.c | 6 -
libxfs/xfs_dir2_sf.c | 78 ++++++---------
libxfs/xfs_iext_tree.c | 36 +++----
libxfs/xfs_inode_fork.c | 70 ++++++--------
libxfs/xfs_inode_fork.h | 10 --
libxfs/xfs_ondisk.h | 14 +-
libxfs/xfs_symlink_remote.c | 4
scrub/attr.c | 17 +--
scrub/inode_repair.c | 4
scrub/readdir.c | 6 -
scrub/symlink.c | 2
xfs_attr_list.c | 13 +-
xfs_dir2_readdir.c | 6 -
xfs_inode.c | 6 -
xfs_inode_item.c | 10 --
xfs_symlink.c | 4
23 files changed, 239 insertions(+), 356 deletions(-)
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/8] xfs: make if_data a void pointer
2023-12-17 17:03 attr cleanups Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-18 22:31 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 2/8] xfs: return if_data from xfs_idata_realloc Christoph Hellwig
` (6 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 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>
---
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] 31+ messages in thread
* [PATCH 2/8] xfs: return if_data from xfs_idata_realloc
2023-12-17 17:03 attr cleanups Christoph Hellwig
2023-12-17 17:03 ` [PATCH 1/8] xfs: make if_data a void pointer Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-18 22:29 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint Christoph Hellwig
` (5 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 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>
---
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] 31+ messages in thread
* [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint
2023-12-17 17:03 attr cleanups Christoph Hellwig
2023-12-17 17:03 ` [PATCH 1/8] xfs: make if_data a void pointer Christoph Hellwig
2023-12-17 17:03 ` [PATCH 2/8] xfs: return if_data from xfs_idata_realloc Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-18 22:39 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 4/8] xfs: simplify xfs_attr_sf_findname Christoph Hellwig
` (4 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 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>
---
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] 31+ messages in thread
* [PATCH 4/8] xfs: simplify xfs_attr_sf_findname
2023-12-17 17:03 attr cleanups Christoph Hellwig
` (2 preceding siblings ...)
2023-12-17 17:03 ` [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-18 22:35 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup Christoph Hellwig
` (3 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 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>
---
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 | 6 +++
4 files changed, 47 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..db5819cbea0f91 100644
--- a/fs/xfs/libxfs/xfs_attr_sf.h
+++ b/fs/xfs/libxfs/xfs_attr_sf.h
@@ -48,4 +48,10 @@ xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
return (void *)sfep + xfs_attr_sf_entsize(sfep);
}
+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] 31+ messages in thread
* [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup
2023-12-17 17:03 attr cleanups Christoph Hellwig
` (3 preceding siblings ...)
2023-12-17 17:03 ` [PATCH 4/8] xfs: simplify xfs_attr_sf_findname Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-18 22:37 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue Christoph Hellwig
` (2 subsequent siblings)
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 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>
---
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] 31+ messages in thread
* [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue
2023-12-17 17:03 attr cleanups Christoph Hellwig
` (4 preceding siblings ...)
2023-12-17 17:03 ` [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-18 22:37 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 7/8] xfs: remove struct xfs_attr_shortform Christoph Hellwig
2023-12-17 17:03 ` [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t Christoph Hellwig
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 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>
---
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] 31+ messages in thread
* [PATCH 7/8] xfs: remove struct xfs_attr_shortform
2023-12-17 17:03 attr cleanups Christoph Hellwig
` (5 preceding siblings ...)
2023-12-17 17:03 ` [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-17 21:12 ` Dave Chinner
2023-12-17 17:03 ` [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t Christoph Hellwig
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
sparse complains about struct xfs_attr_shortform because it embedds a
structure with a variable sized array in a variable sized array.
Given that xfs_attr_shortform is not a very useful struture, and the dir2
equivalent has been removed a long time ago, remove it as well and
instead provide a xfs_attr_sf_firstentry helper that returns the first
xfs_attr_sf_entry behind a xfs_attr_sf_hdr.
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 | 23 ++++++++++-----------
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, 63 insertions(+), 60 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 db5819cbea0f91..a1d5ef88ca2673 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)
{
@@ -49,9 +56,9 @@ xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
}
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..650fedce40449e 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -580,18 +580,17 @@ xfs_dir2_block_leaf_p(struct xfs_dir2_block_tail *btp)
/*
* 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 */
+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 (see xfs_attr_leaf.h) */
+ __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] 31+ messages in thread
* [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t
2023-12-17 17:03 attr cleanups Christoph Hellwig
` (6 preceding siblings ...)
2023-12-17 17:03 ` [PATCH 7/8] xfs: remove struct xfs_attr_shortform Christoph Hellwig
@ 2023-12-17 17:03 ` Christoph Hellwig
2023-12-18 22:39 ` Darrick J. Wong
7 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-17 17:03 UTC (permalink / raw
To: Chandan Babu R; +Cc: Darrick J. Wong, linux-xfs
Remove the last two users of the typedef and move the comment next to
its declaration to a more useful place.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++--
fs/xfs/libxfs/xfs_attr_sf.h | 8 --------
fs/xfs/libxfs/xfs_da_format.h | 5 ++++-
3 files changed, 6 insertions(+), 11 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 a1d5ef88ca2673..0600b4e408fa36 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.
*/
diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
index 650fedce40449e..dcfe2fe9edc385 100644
--- a/fs/xfs/libxfs/xfs_da_format.h
+++ b/fs/xfs/libxfs/xfs_da_format.h
@@ -578,7 +578,10 @@ 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.
+ * 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.
*/
struct xfs_attr_sf_hdr { /* constant-structure header block */
__be16 totsize; /* total bytes in shortform list */
--
2.39.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 7/8] xfs: remove struct xfs_attr_shortform
2023-12-17 17:03 ` [PATCH 7/8] xfs: remove struct xfs_attr_shortform Christoph Hellwig
@ 2023-12-17 21:12 ` Dave Chinner
2023-12-18 4:30 ` Christoph Hellwig
0 siblings, 1 reply; 31+ messages in thread
From: Dave Chinner @ 2023-12-17 21:12 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, Darrick J. Wong, linux-xfs
On Sun, Dec 17, 2023 at 06:03:49PM +0100, Christoph Hellwig wrote:
> sparse complains about struct xfs_attr_shortform because it embedds a
embeds
> structure with a variable sized array in a variable sized array.
>
> Given that xfs_attr_shortform is not a very useful struture, and the dir2
> equivalent has been removed a long time ago, remove it as well and
> instead provide a xfs_attr_sf_firstentry helper that returns the first
> xfs_attr_sf_entry behind a xfs_attr_sf_hdr.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
....
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index f9015f88eca706..650fedce40449e 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -580,18 +580,17 @@ xfs_dir2_block_leaf_p(struct xfs_dir2_block_tail *btp)
> /*
> * 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 */
> +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 (see xfs_attr_leaf.h) */
May as well correct the comment while you are touching this
structure; xfs_attr_leaf.h has not existed for a long time. Perhaps
just "/* XFS_ATTR_* flags */" as they are defined a little further
down this same file...
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/8] xfs: remove struct xfs_attr_shortform
2023-12-17 21:12 ` Dave Chinner
@ 2023-12-18 4:30 ` Christoph Hellwig
2023-12-18 22:41 ` Darrick J. Wong
0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-18 4:30 UTC (permalink / raw
To: Dave Chinner
Cc: Christoph Hellwig, Chandan Babu R, Darrick J. Wong, linux-xfs
On Mon, Dec 18, 2023 at 08:12:12AM +1100, Dave Chinner wrote:
> > +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 (see xfs_attr_leaf.h) */
>
> May as well correct the comment while you are touching this
> structure; xfs_attr_leaf.h has not existed for a long time. Perhaps
> just "/* XFS_ATTR_* flags */" as they are defined a little further
> down this same file...
Yeah, I'll update it for the next version.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/8] xfs: return if_data from xfs_idata_realloc
2023-12-17 17:03 ` [PATCH 2/8] xfs: return if_data from xfs_idata_realloc Christoph Hellwig
@ 2023-12-18 22:29 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:29 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Dec 17, 2023 at 06:03:44PM +0100, Christoph Hellwig wrote:
> 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>
Nice cleanup!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 [flat|nested] 31+ messages in thread
* Re: [PATCH 1/8] xfs: make if_data a void pointer
2023-12-17 17:03 ` [PATCH 1/8] xfs: make if_data a void pointer Christoph Hellwig
@ 2023-12-18 22:31 ` Darrick J. Wong
2023-12-19 4:20 ` Christoph Hellwig
0 siblings, 1 reply; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:31 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Dec 17, 2023 at 06:03:43PM +0100, Christoph Hellwig wrote:
> 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>
Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
(Does if_bytes really need to be int64_t? I don't think we can
realistically allocate that much space...)
--D
> ---
> 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 [flat|nested] 31+ messages in thread
* Re: [PATCH 4/8] xfs: simplify xfs_attr_sf_findname
2023-12-17 17:03 ` [PATCH 4/8] xfs: simplify xfs_attr_sf_findname Christoph Hellwig
@ 2023-12-18 22:35 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:35 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Dec 17, 2023 at 06:03:46PM +0100, Christoph Hellwig wrote:
> 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>
I like the simplification of xfs_attr_sf_findname's return value.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 | 6 +++
> 4 files changed, 47 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..db5819cbea0f91 100644
> --- a/fs/xfs/libxfs/xfs_attr_sf.h
> +++ b/fs/xfs/libxfs/xfs_attr_sf.h
> @@ -48,4 +48,10 @@ xfs_attr_sf_nextentry(struct xfs_attr_sf_entry *sfep)
> return (void *)sfep + xfs_attr_sf_entsize(sfep);
> }
>
> +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 [flat|nested] 31+ messages in thread
* Re: [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup
2023-12-17 17:03 ` [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup Christoph Hellwig
@ 2023-12-18 22:37 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:37 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Dec 17, 2023 at 06:03:47PM +0100, Christoph Hellwig wrote:
> 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>
Looks ok,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue
2023-12-17 17:03 ` [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue Christoph Hellwig
@ 2023-12-18 22:37 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:37 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Dec 17, 2023 at 06:03:48PM +0100, Christoph Hellwig wrote:
> xfs_attr_shortform_getvalue duplicates the logic in xfs_attr_sf_findname.
> Use the helper instead.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Nice cleanup!
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> 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 [flat|nested] 31+ messages in thread
* Re: [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint
2023-12-17 17:03 ` [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint Christoph Hellwig
@ 2023-12-18 22:39 ` Darrick J. Wong
2023-12-19 4:21 ` Christoph Hellwig
0 siblings, 1 reply; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:39 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Dec 17, 2023 at 06:03:45PM +0100, Christoph Hellwig wrote:
> 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>
> ---
> 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);
Shouldn't this get renamed to trace_xfs_attr_shortform_getvalue to match
the function? Especially since xfs_attr_shortform_lookup disappears
later, AFAICT.
--D
> +
> sfe = &sf->list[0];
> for (i = 0; i < sf->hdr.count;
> sfe = xfs_attr_sf_nextentry(sfe), i++) {
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t
2023-12-17 17:03 ` [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t Christoph Hellwig
@ 2023-12-18 22:39 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:39 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Sun, Dec 17, 2023 at 06:03:50PM +0100, Christoph Hellwig wrote:
> Remove the last two users of the typedef and move the comment next to
> its declaration to a more useful place.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Woo, fewer typedefs.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
> ---
> fs/xfs/libxfs/xfs_attr_leaf.c | 4 ++--
> fs/xfs/libxfs/xfs_attr_sf.h | 8 --------
> fs/xfs/libxfs/xfs_da_format.h | 5 ++++-
> 3 files changed, 6 insertions(+), 11 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 a1d5ef88ca2673..0600b4e408fa36 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.
> */
> diff --git a/fs/xfs/libxfs/xfs_da_format.h b/fs/xfs/libxfs/xfs_da_format.h
> index 650fedce40449e..dcfe2fe9edc385 100644
> --- a/fs/xfs/libxfs/xfs_da_format.h
> +++ b/fs/xfs/libxfs/xfs_da_format.h
> @@ -578,7 +578,10 @@ 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.
> + * 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.
> */
> struct xfs_attr_sf_hdr { /* constant-structure header block */
> __be16 totsize; /* total bytes in shortform list */
> --
> 2.39.2
>
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/8] xfs: remove struct xfs_attr_shortform
2023-12-18 4:30 ` Christoph Hellwig
@ 2023-12-18 22:41 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-18 22:41 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Dave Chinner, Chandan Babu R, linux-xfs
On Mon, Dec 18, 2023 at 05:30:38AM +0100, Christoph Hellwig wrote:
> On Mon, Dec 18, 2023 at 08:12:12AM +1100, Dave Chinner wrote:
> > > +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 (see xfs_attr_leaf.h) */
> >
> > May as well correct the comment while you are touching this
> > structure; xfs_attr_leaf.h has not existed for a long time. Perhaps
> > just "/* XFS_ATTR_* flags */" as they are defined a little further
> > down this same file...
>
> Yeah, I'll update it for the next version.
Also, could you add a comment somewhere that the ondisk format is one
struct xfs_attr_sf_hdr followed by a variable number of struct
xfs_attr_sf_entry objects? That much was clear with the old structure,
even if the C linters can't make sense of it.
--D
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/8] xfs: make if_data a void pointer
2023-12-18 22:31 ` Darrick J. Wong
@ 2023-12-19 4:20 ` Christoph Hellwig
2023-12-19 4:48 ` Darrick J. Wong
0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-19 4:20 UTC (permalink / raw
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Mon, Dec 18, 2023 at 02:31:54PM -0800, Darrick J. Wong wrote:
> (Does if_bytes really need to be int64_t? I don't think we can
> realistically allocate that much space...)
Ѕee commit 3f8a4f1d876d3e3e49e50b0396eaffcc4ba71b08 for Dave's detailed
explanation.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint
2023-12-18 22:39 ` Darrick J. Wong
@ 2023-12-19 4:21 ` Christoph Hellwig
2023-12-19 4:52 ` Darrick J. Wong
0 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2023-12-19 4:21 UTC (permalink / raw
To: Darrick J. Wong; +Cc: Christoph Hellwig, Chandan Babu R, linux-xfs
On Mon, Dec 18, 2023 at 02:39:02PM -0800, Darrick J. Wong wrote:
> > - 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);
>
> Shouldn't this get renamed to trace_xfs_attr_shortform_getvalue to match
> the function? Especially since xfs_attr_shortform_lookup disappears
> later, AFAICT.
If we value accurate naming over being able to use a historical
trace point: yes. Although in that case I'd probably structure it
as a patch adding the new xfs_attr_shortform_getvalue tracepoint only,
and removing the xfs_attr_sf_lookup one with the function.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/8] xfs: make if_data a void pointer
2023-12-19 4:20 ` Christoph Hellwig
@ 2023-12-19 4:48 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-19 4:48 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Tue, Dec 19, 2023 at 05:20:08AM +0100, Christoph Hellwig wrote:
> On Mon, Dec 18, 2023 at 02:31:54PM -0800, Darrick J. Wong wrote:
> > (Does if_bytes really need to be int64_t? I don't think we can
> > realistically allocate that much space...)
>
> Ѕee commit 3f8a4f1d876d3e3e49e50b0396eaffcc4ba71b08 for Dave's detailed
> explanation.
Heh, thanks for the reminder!
--D
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint
2023-12-19 4:21 ` Christoph Hellwig
@ 2023-12-19 4:52 ` Darrick J. Wong
0 siblings, 0 replies; 31+ messages in thread
From: Darrick J. Wong @ 2023-12-19 4:52 UTC (permalink / raw
To: Christoph Hellwig; +Cc: Chandan Babu R, linux-xfs
On Tue, Dec 19, 2023 at 05:21:39AM +0100, Christoph Hellwig wrote:
> On Mon, Dec 18, 2023 at 02:39:02PM -0800, Darrick J. Wong wrote:
> > > - 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);
> >
> > Shouldn't this get renamed to trace_xfs_attr_shortform_getvalue to match
> > the function? Especially since xfs_attr_shortform_lookup disappears
> > later, AFAICT.
>
> If we value accurate naming over being able to use a historical
> trace point: yes. Although in that case I'd probably structure it
> as a patch adding the new xfs_attr_shortform_getvalue tracepoint only,
> and removing the xfs_attr_sf_lookup one with the function.
Eh, tracepoint names are greppable enough.
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
--D
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2023-12-19 4:52 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-17 17:03 attr cleanups Christoph Hellwig
2023-12-17 17:03 ` [PATCH 1/8] xfs: make if_data a void pointer Christoph Hellwig
2023-12-18 22:31 ` Darrick J. Wong
2023-12-19 4:20 ` Christoph Hellwig
2023-12-19 4:48 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 2/8] xfs: return if_data from xfs_idata_realloc Christoph Hellwig
2023-12-18 22:29 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 3/8] xfs: move the xfs_attr_sf_lookup tracepoint Christoph Hellwig
2023-12-18 22:39 ` Darrick J. Wong
2023-12-19 4:21 ` Christoph Hellwig
2023-12-19 4:52 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 4/8] xfs: simplify xfs_attr_sf_findname Christoph Hellwig
2023-12-18 22:35 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 5/8] xfs: remove xfs_attr_shortform_lookup Christoph Hellwig
2023-12-18 22:37 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 6/8] xfs: use xfs_attr_sf_findname in xfs_attr_shortform_getvalue Christoph Hellwig
2023-12-18 22:37 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 7/8] xfs: remove struct xfs_attr_shortform Christoph Hellwig
2023-12-17 21:12 ` Dave Chinner
2023-12-18 4:30 ` Christoph Hellwig
2023-12-18 22:41 ` Darrick J. Wong
2023-12-17 17:03 ` [PATCH 8/8] xfs: remove xfs_attr_sf_hdr_t Christoph Hellwig
2023-12-18 22:39 ` Darrick J. Wong
-- strict thread matches above, loose matches on Subject: below --
2014-05-03 15:20 attr cleanups Christoph Hellwig
2014-05-03 16:04 ` Mark Tinguely
2014-05-04 10:16 ` Christoph Hellwig
2014-05-04 21:52 ` Dave Chinner
2014-05-05 13:24 ` Mark Tinguely
2014-05-05 20:55 ` Dave Chinner
2014-05-06 19:40 ` Mark Tinguely
2014-05-06 20:51 ` Dave Chinner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).