linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Allison Henderson <allison.henderson@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 13/17] xfs: add parent attributes to link
Date: Fri, 20 Oct 2017 18:12:17 -0700	[thread overview]
Message-ID: <f3fad097-0d0a-f454-f443-fa2aba9419a6@oracle.com> (raw)
In-Reply-To: <20171019194015.GH4755@magnolia>

On 10/19/2017 12:40 PM, Darrick J. Wong wrote:

> On Wed, Oct 18, 2017 at 03:55:29PM -0700, Allison Henderson wrote:
>> From: Dave Chinner<dchinner@redhat.com>
> Needs a description of what w're doing and maybe why...
>
Sure, I think initially these had quick one line descriptions that
got formatted into the subject line when I created the patch set

>> [bfoster: rebase, use VFS inode fields, fix xfs_bmap_finish() usage]
>> [achender: rebased, changed __unint32_t to xfs_dir2_dataptr_t,
>> 	   fixed null pointer bugs]
>>
>> Signed-off-by: Dave Chinner<dchinner@redhat.com>
>> Signed-off-by: Allison Henderson<allison.henderson@oracle.com>
>> ---
>>   fs/xfs/libxfs/xfs_attr.c   | 20 +++++++++++++-
>>   fs/xfs/libxfs/xfs_parent.c | 43 ++++++++++++++++++++++++++++++
>>   fs/xfs/xfs_attr.h          | 10 +++++++
>>   fs/xfs/xfs_inode.c         | 66 ++++++++++++++++++++++++++++++++++++----------
>>   4 files changed, 124 insertions(+), 15 deletions(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>> index 8aad242..e7692ef 100644
>> --- a/fs/xfs/libxfs/xfs_attr.c
>> +++ b/fs/xfs/libxfs/xfs_attr.c
>> @@ -35,6 +35,7 @@
>>   #include "xfs_bmap_util.h"
>>   #include "xfs_bmap_btree.h"
>>   #include "xfs_attr.h"
>> +#include "xfs_attr_sf.h"
>>   #include "xfs_attr_leaf.h"
>>   #include "xfs_attr_remote.h"
>>   #include "xfs_error.h"
>> @@ -266,6 +267,23 @@ xfs_attr_set_first_parent(
>>   	return error;
>>   }
>>   
>> +int
>> +xfs_attr_set_parent(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_inode	*ip,
>> +	struct xfs_parent_name_rec *rec,
>> +	int			reclen,
>> +	const char		*value,
>> +	int			valuelen,
>> +	struct xfs_defer_ops	*dfops,
>> +	xfs_fsblock_t		*firstblock)
>> +{
>> +	int                     flags = ATTR_PARENT;
>> +
>> +	return xfs_attr_set_deferred(ip, dfops, (char *)rec, reclen,
>> +				    (char *)value, valuelen, flags);
>> +}
>> +
>>   /*
>>    * set the attribute specified in @args. In the case of the parent attribute
>>    * being set, we do not want to roll the transaction on shortform-to-leaf
>> @@ -512,8 +530,8 @@ xfs_attr_set(
>>   	 */
>>   	xfs_trans_log_inode(args.trans, dp, XFS_ILOG_CORE);
>>   	error = xfs_trans_commit(args.trans);
>> -	xfs_iunlock(dp, XFS_ILOCK_EXCL);
>>   
>> +	xfs_iunlock(dp, XFS_ILOCK_EXCL);
>>   	return error;
>>   
>>   out_defer_cancel:
>> diff --git a/fs/xfs/libxfs/xfs_parent.c b/fs/xfs/libxfs/xfs_parent.c
>> index 88f7edc..0707336 100644
>> --- a/fs/xfs/libxfs/xfs_parent.c
>> +++ b/fs/xfs/libxfs/xfs_parent.c
>> @@ -96,3 +96,46 @@ xfs_parent_create(
>>   
>>   	return xfs_parent_create_nrec(tp, child, &nrec, dfops, firstblock);
>>   }
>> +
>> +static int
>> +xfs_parent_add_nrec(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_inode	*child,
>> +	struct xfs_parent_name_irec *nrec,
>> +	struct xfs_defer_ops	*dfops,
>> +	xfs_fsblock_t		*firstblock)
>> +{
>> +	struct xfs_parent_name_rec rec;
>> +
>> +	rec.p_ino = cpu_to_be64(nrec->p_ino);
>> +	rec.p_gen = cpu_to_be32(nrec->p_gen);
>> +	rec.p_diroffset = cpu_to_be32(nrec->p_diroffset);
>> +
>> +	return xfs_attr_set_parent(tp, child, &rec, sizeof(rec),
>> +				   nrec->p_name, nrec->p_namelen,
>> +				   dfops, firstblock);
>> +}
>> +
>> +/*
>> + * Add a parent record to an inode with existing parent records.
>> + */
>> +int
>> +xfs_parent_add(
>> +	struct xfs_trans	*tp,
>> +	struct xfs_inode	*parent,
>> +	struct xfs_inode	*child,
>> +	struct xfs_name		*child_name,
>> +	uint32_t		diroffset,
>> +	struct xfs_defer_ops	*dfops,
>> +	xfs_fsblock_t		*firstblock)
>> +{
>> +	struct xfs_parent_name_irec nrec;
>> +
>> +	nrec.p_ino = parent->i_ino;
>> +	nrec.p_gen = VFS_I(parent)->i_generation;
>> +	nrec.p_diroffset = diroffset;
>> +	nrec.p_name = child_name->name;
>> +	nrec.p_namelen = child_name->len;
>> +
>> +	return xfs_parent_add_nrec(tp, child, &nrec, dfops, firstblock);
>> +}
>> diff --git a/fs/xfs/xfs_attr.h b/fs/xfs/xfs_attr.h
>> index b48e31b..acb6157 100644
>> --- a/fs/xfs/xfs_attr.h
>> +++ b/fs/xfs/xfs_attr.h
>> @@ -197,4 +197,14 @@ int xfs_attr_set_first_parent(struct xfs_trans *tp, struct xfs_inode *ip,
>>   			      const char *value, int valuelen,
>>   			      struct xfs_defer_ops *dfops,
>>   			      xfs_fsblock_t *firstblock);
>> +
>> +int xfs_parent_add(struct xfs_trans *tp, struct xfs_inode *parent,
>> +		   struct xfs_inode *child, struct xfs_name *child_name,
>> +		   xfs_dir2_dataptr_t diroffset, struct xfs_defer_ops *dfops,
>> +		   xfs_fsblock_t *firstblock);
>> +int xfs_attr_set_parent(struct xfs_trans *tp, struct xfs_inode *ip,
>> +			struct xfs_parent_name_rec *rec, int reclen,
>> +			const char *value, int valuelen,
>> +			struct xfs_defer_ops *dfops, xfs_fsblock_t *firstblock);
>> +
>>   #endif	/* __XFS_ATTR_H__ */
>> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
>> index 4396561..51b623b 100644
>> --- a/fs/xfs/xfs_inode.c
>> +++ b/fs/xfs/xfs_inode.c
>> @@ -1451,6 +1451,8 @@ xfs_link(
>>   	struct xfs_defer_ops	dfops;
>>   	xfs_fsblock_t           first_block;
>>   	int			resblks;
>> +	uint32_t		diroffset;
>> +	bool			first_parent = false;
>>   
>>   	trace_xfs_link(tdp, target_name);
>>   
>> @@ -1467,6 +1469,25 @@ xfs_link(
>>   	if (error)
>>   		goto std_return;
>>   
>> +	/*
>> +	 * If we have parent pointers and there is no attribute fork (i.e. we
>> +	 * are linking in a O_TMPFILE created inode) we need to add the
>> +	 * attribute fork to the inode. Because we may have an existing data
>> +	 * fork, we do this before we start the link transaction as adding an
>> +	 * attribute fork requires it's own transaction.
> Ok, so an inode that isn't pointed to by any directory will have zero
> parent link attributes and possibly not even an attr fork.  Got it.
>
> --D
>
>> +	 */
>> +	if (xfs_sb_version_hasparent(&mp->m_sb) && !xfs_inode_hasattr(sip)) {
>> +		int sf_size = sizeof(struct xfs_attr_sf_hdr) +
>> +				XFS_ATTR_SF_ENTSIZE_BYNAME(
>> +					sizeof(struct xfs_parent_name_rec),
>> +					target_name->len);
>> +		ASSERT(VFS_I(sip)->i_nlink == 0);
>> +		error = xfs_bmap_add_attrfork(sip, sf_size, 0);
>> +		if (error)
>> +			goto std_return;
>> +		first_parent = true;
>> +	}
>> +
>>   	resblks = XFS_LINK_SPACE_RES(mp, target_name->len);
>>   	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_link, resblks, 0, 0, &tp);
>>   	if (error == -ENOSPC) {
>> @@ -1498,8 +1519,6 @@ xfs_link(
>>   			goto error_return;
>>   	}
>>   
>> -	xfs_defer_init(&dfops, &first_block);
>> -
>>   	/*
>>   	 * Handle initial link state of O_TMPFILE inode
>>   	 */
>> @@ -1509,36 +1528,55 @@ xfs_link(
>>   			goto error_return;
>>   	}
>>   
>> +	xfs_defer_init(&dfops, &first_block);
>>   	error = xfs_dir_createname(tp, tdp, target_name, sip->i_ino,
>> -				   &first_block, &dfops, resblks, NULL);
>> +				   &first_block, &dfops, resblks, &diroffset);
>>   	if (error)
>> -		goto error_return;
>> +		goto out_defer_cancel;
>>   	xfs_trans_ichgtime(tp, tdp, XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG);
>>   	xfs_trans_log_inode(tp, tdp, XFS_ILOG_CORE);
>>   
>>   	error = xfs_bumplink(tp, sip);
>>   	if (error)
>> -		goto error_return;
>> +		goto out_defer_cancel;
>>   
>>   	/*
>> -	 * If this is a synchronous mount, make sure that the
>> -	 * link transaction goes to disk before returning to
>> -	 * the user.
>> +	 * If we have parent pointers, we now need to add the parent record to
>> +	 * the attribute fork of the inode. If this is the initial parent
>> +	 * atribute, we need to create it correctly, otherwise we can just add
>> +	 * the parent to the inode.
>> +	 */
>> +	if (xfs_sb_version_hasparent(&mp->m_sb)) {
>> +		if (first_parent)
>> +			error = xfs_parent_create(tp, tdp, sip, target_name,
>> +						  diroffset, &dfops,
>> +						  &first_block);
>> +		else
>> +			error = xfs_parent_add(tp, tdp, sip, target_name,
>> +					       diroffset, &dfops,
>> +					       &first_block);
>> +		if (error)
>> +			goto out_defer_cancel;
>> +	}
>> +
>> +	/*
>> +	 * If this is a synchronous mount, make sure that the link transaction
>> +	 * goes to disk before returning to the user.
>>   	 */
>>   	if (mp->m_flags & (XFS_MOUNT_WSYNC|XFS_MOUNT_DIRSYNC))
>>   		xfs_trans_set_sync(tp);
>>   
>>   	error = xfs_defer_finish(&tp, &dfops);
>> -	if (error) {
>> -		xfs_defer_cancel(&dfops);
>> -		goto error_return;
>> -	}
>> +	if (error)
>> +		goto out_defer_cancel;
>>   
>>   	return xfs_trans_commit(tp);
>>   
>> - error_return:
>> +out_defer_cancel:
>> +	xfs_defer_cancel(&dfops);
>> +error_return:
>>   	xfs_trans_cancel(tp);
>> - std_return:
>> +std_return:
>>   	return error;
>>   }
>>   
>> -- 
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message tomajordomo@vger.kernel.org
>> More majordomo info athttp://vger.kernel.org/majordomo-info.html


  reply	other threads:[~2017-10-21  1:12 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-18 22:55 [PATCH 00/17] Parent Pointers V3 Allison Henderson
2017-10-18 22:55 ` [PATCH 01/17] Add helper functions xfs_attr_set_args and xfs_attr_remove_args Allison Henderson
2017-10-19 20:03   ` Darrick J. Wong
2017-10-21  1:14     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 02/17] Set up infastructure for deferred attribute operations Allison Henderson
2017-10-19 19:02   ` Darrick J. Wong
2017-10-21  1:08     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 03/17] Add xfs_attr_set_defered and xfs_attr_remove_defered Allison Henderson
2017-10-19 19:13   ` Darrick J. Wong
2017-10-21  1:08     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 04/17] Remove all strlen calls in all xfs_attr_* functions for attr names Allison Henderson
2017-10-19 19:15   ` Darrick J. Wong
2017-10-21  1:10     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 05/17] xfs: get directory offset when adding directory name Allison Henderson
2017-10-18 22:55 ` [PATCH 06/17] xfs: get directory offset when removing " Allison Henderson
2017-10-19 19:17   ` Darrick J. Wong
2017-10-21  1:11     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 07/17] xfs: get directory offset when replacing a " Allison Henderson
2017-10-18 22:55 ` [PATCH 08/17] xfs: add parent pointer support to attribute code Allison Henderson
2017-10-18 22:55 ` [PATCH 09/17] xfs: define parent pointer xattr format Allison Henderson
2017-10-18 22:55 ` [PATCH 10/17] :xfs: extent transaction reservations for parent attributes Allison Henderson
2017-10-19 18:24   ` Darrick J. Wong
     [not found]     ` <8680e0c1-ada8-06e3-e397-61a5076030be@oracle.com>
2017-10-20 23:45       ` Darrick J. Wong
2017-10-21  0:12         ` Allison Henderson
2017-10-21  1:07     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 11/17] Add the extra space requirements for parent pointer attributes when calculating the minimum log size during mkfs Allison Henderson
2017-10-19 18:13   ` Darrick J. Wong
2017-10-21  1:07     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 12/17] xfs: parent pointer attribute creation Allison Henderson
2017-10-19 19:36   ` Darrick J. Wong
     [not found]     ` <9185d3e8-4b41-b2d8-294b-934f7d3409f0@oracle.com>
2017-10-21  0:03       ` Darrick J. Wong
2017-10-21  1:11     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 13/17] xfs: add parent attributes to link Allison Henderson
2017-10-19 19:40   ` Darrick J. Wong
2017-10-21  1:12     ` Allison Henderson [this message]
2017-10-18 22:55 ` [PATCH 14/17] xfs: remove parent pointers in unlink Allison Henderson
2017-10-19 19:43   ` Darrick J. Wong
2017-10-21  1:12     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 15/17] xfs_bmap_add_attrfork(): re-add error handling from set_attrforkoff() call Allison Henderson
2017-10-19 19:43   ` Darrick J. Wong
2017-10-21  1:13     ` Allison Henderson
2017-10-18 22:55 ` [PATCH 16/17] Add parent pointers to rename Allison Henderson
2017-10-18 22:55 ` [PATCH 17/17] Add the parent pointer support to the superblock version 5 Allison Henderson
2017-10-19  3:57   ` Amir Goldstein
2017-10-19 20:06     ` Darrick J. Wong
2017-10-20  3:18       ` Amir Goldstein
2017-10-19 19:45   ` Darrick J. Wong
2017-10-21  1:13     ` Allison Henderson
2017-10-19  4:11 ` [PATCH 00/17] Parent Pointers V3 Amir Goldstein
2017-10-20  3:22   ` Amir Goldstein
2017-10-21  1:06     ` Allison Henderson
2017-10-20 22:41   ` Dave Chinner
2017-10-21  7:34     ` Amir Goldstein
2017-10-22 23:27       ` Dave Chinner
2017-10-23  4:30         ` Amir Goldstein
2017-10-23  5:32           ` Dave Chinner
2017-10-23  6:48             ` Amir Goldstein
2017-10-23  8:40               ` Dave Chinner
2017-10-23  9:06                 ` Amir Goldstein
2017-10-23 17:14                   ` Darrick J. Wong
2017-10-23 19:20                     ` Amir Goldstein
  -- strict thread matches above, loose matches on Subject: below --
2017-10-06 22:05 [PATCH 00/17] Parent Pointers V2 Allison Henderson
2017-10-06 22:05 ` [PATCH 13/17] xfs: add parent attributes to link Allison Henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f3fad097-0d0a-f454-f443-fa2aba9419a6@oracle.com \
    --to=allison.henderson@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).