tree: https://github.com/kdave/btrfs-devel.git misc-next
head: 8e5d6a5c062f370d4d0b2dace7e95ab40c6ce3dd
commit: 6bfc5d45946acd8286fb026137f20ee8747a50f1 [149/153] btrfs: pass the dentry to btrfs_log_new_name() instead of the inode
config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20220126/[email protected]/config)
compiler: nds32le-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/kdave/btrfs-devel/commit/6bfc5d45946acd8286fb026137f20ee8747a50f1
git remote add kdave-btrfs-devel https://github.com/kdave/btrfs-devel.git
git fetch --no-tags kdave-btrfs-devel misc-next
git checkout 6bfc5d45946acd8286fb026137f20ee8747a50f1
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash fs/btrfs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> fs/btrfs/tree-log.c:6755: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Update the log after adding a new name for an inode.
vim +6755 fs/btrfs/tree-log.c
6753
6754 /**
> 6755 * Update the log after adding a new name for an inode.
6756 *
6757 * @trans: Transaction handle.
6758 * @old_dentry: The dentry associated with the old name and the old
6759 * parent directory.
6760 * @old_dir: The inode of the previous parent directory for the case
6761 * of a rename. For a link operation, it must be NULL.
6762 * @parent: The dentry associated with the directory under which the
6763 * new name is located.
6764 *
6765 * Call this after adding a new name for an inode, as a result of a link or
6766 * rename operation, and it will properly update the log to reflect the new name.
6767 */
6768 void btrfs_log_new_name(struct btrfs_trans_handle *trans,
6769 struct dentry *old_dentry, struct btrfs_inode *old_dir,
6770 struct dentry *parent)
6771 {
6772 struct btrfs_inode *inode = BTRFS_I(d_inode(old_dentry));
6773 struct btrfs_log_ctx ctx;
6774
6775 /*
6776 * this will force the logging code to walk the dentry chain
6777 * up for the file
6778 */
6779 if (!S_ISDIR(inode->vfs_inode.i_mode))
6780 inode->last_unlink_trans = trans->transid;
6781
6782 /*
6783 * if this inode hasn't been logged and directory we're renaming it
6784 * from hasn't been logged, we don't need to log it
6785 */
6786 if (!inode_logged(trans, inode) &&
6787 (!old_dir || !inode_logged(trans, old_dir)))
6788 return;
6789
6790 /*
6791 * If we are doing a rename (old_dir is not NULL) from a directory that
6792 * was previously logged, make sure the next log attempt on the directory
6793 * is not skipped and logs the inode again. This is because the log may
6794 * not currently be authoritative for a range including the old
6795 * BTRFS_DIR_INDEX_KEY key, so we want to make sure after a log replay we
6796 * do not end up with both the new and old dentries around (in case the
6797 * inode is a directory we would have a directory with two hard links and
6798 * 2 inode references for different parents). The next log attempt of
6799 * old_dir will happen at btrfs_log_all_parents(), called through
6800 * btrfs_log_inode_parent() below, because we have previously set
6801 * inode->last_unlink_trans to the current transaction ID, either here or
6802 * at btrfs_record_unlink_dir() in case the inode is a directory.
6803 */
6804 if (old_dir)
6805 old_dir->logged_trans = 0;
6806
6807 btrfs_init_log_ctx(&ctx, &inode->vfs_inode);
6808 ctx.logging_new_name = true;
6809 /*
6810 * We don't care about the return value. If we fail to log the new name
6811 * then we know the next attempt to sync the log will fallback to a full
6812 * transaction commit (due to a call to btrfs_set_log_full_commit()), so
6813 * we don't need to worry about getting a log committed that has an
6814 * inconsistent state after a rename operation.
6815 */
6816 btrfs_log_inode_parent(trans, inode, parent, LOG_INODE_EXISTS, &ctx);
6817 }
6818
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On 25/01/22 23:12, kernel test robot wrote:
> tree: https://github.com/kdave/btrfs-devel.git misc-next
> head: 8e5d6a5c062f370d4d0b2dace7e95ab40c6ce3dd
> commit: 6bfc5d45946acd8286fb026137f20ee8747a50f1 [149/153] btrfs: pass the dentry to btrfs_log_new_name() instead of the inode
> config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20220126/[email protected]/config)
> compiler: nds32le-linux-gcc (GCC) 11.2.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/kdave/btrfs-devel/commit/6bfc5d45946acd8286fb026137f20ee8747a50f1
> git remote add kdave-btrfs-devel https://github.com/kdave/btrfs-devel.git
> git fetch --no-tags kdave-btrfs-devel misc-next
> git checkout 6bfc5d45946acd8286fb026137f20ee8747a50f1
> # save the config file to linux build tree
> mkdir build_dir
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash fs/btrfs/
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
>>> fs/btrfs/tree-log.c:6755: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
> * Update the log after adding a new name for an inode.
>
>
> vim +6755 fs/btrfs/tree-log.c
>
> 6753
> 6754 /**
>> 6755 * Update the log after adding a new name for an inode.
The patch I submitted did not add the "/**":
https://lore.kernel.org/linux-btrfs/1f0d5aaf498afa64ef3582cb9d9d24bc5f888ab2.1642676248.git.fdmanana@suse.com/T/#u
However I see that David's misc-next branch, the patch adds that:
https://github.com/kdave/btrfs-devel/commit/6bfc5d45946acd8286fb026137f20ee8747a50f1
David, did you edit the patch? Why?
thanks
> 6756 *
> 6757 * @trans: Transaction handle.
> 6758 * @old_dentry: The dentry associated with the old name and the old
> 6759 * parent directory.
> 6760 * @old_dir: The inode of the previous parent directory for the case
> 6761 * of a rename. For a link operation, it must be NULL.
> 6762 * @parent: The dentry associated with the directory under which the
> 6763 * new name is located.
> 6764 *
> 6765 * Call this after adding a new name for an inode, as a result of a link or
> 6766 * rename operation, and it will properly update the log to reflect the new name.
> 6767 */
> 6768 void btrfs_log_new_name(struct btrfs_trans_handle *trans,
> 6769 struct dentry *old_dentry, struct btrfs_inode *old_dir,
> 6770 struct dentry *parent)
> 6771 {
> 6772 struct btrfs_inode *inode = BTRFS_I(d_inode(old_dentry));
> 6773 struct btrfs_log_ctx ctx;
> 6774
> 6775 /*
> 6776 * this will force the logging code to walk the dentry chain
> 6777 * up for the file
> 6778 */
> 6779 if (!S_ISDIR(inode->vfs_inode.i_mode))
> 6780 inode->last_unlink_trans = trans->transid;
> 6781
> 6782 /*
> 6783 * if this inode hasn't been logged and directory we're renaming it
> 6784 * from hasn't been logged, we don't need to log it
> 6785 */
> 6786 if (!inode_logged(trans, inode) &&
> 6787 (!old_dir || !inode_logged(trans, old_dir)))
> 6788 return;
> 6789
> 6790 /*
> 6791 * If we are doing a rename (old_dir is not NULL) from a directory that
> 6792 * was previously logged, make sure the next log attempt on the directory
> 6793 * is not skipped and logs the inode again. This is because the log may
> 6794 * not currently be authoritative for a range including the old
> 6795 * BTRFS_DIR_INDEX_KEY key, so we want to make sure after a log replay we
> 6796 * do not end up with both the new and old dentries around (in case the
> 6797 * inode is a directory we would have a directory with two hard links and
> 6798 * 2 inode references for different parents). The next log attempt of
> 6799 * old_dir will happen at btrfs_log_all_parents(), called through
> 6800 * btrfs_log_inode_parent() below, because we have previously set
> 6801 * inode->last_unlink_trans to the current transaction ID, either here or
> 6802 * at btrfs_record_unlink_dir() in case the inode is a directory.
> 6803 */
> 6804 if (old_dir)
> 6805 old_dir->logged_trans = 0;
> 6806
> 6807 btrfs_init_log_ctx(&ctx, &inode->vfs_inode);
> 6808 ctx.logging_new_name = true;
> 6809 /*
> 6810 * We don't care about the return value. If we fail to log the new name
> 6811 * then we know the next attempt to sync the log will fallback to a full
> 6812 * transaction commit (due to a call to btrfs_set_log_full_commit()), so
> 6813 * we don't need to worry about getting a log committed that has an
> 6814 * inconsistent state after a rename operation.
> 6815 */
> 6816 btrfs_log_inode_parent(trans, inode, parent, LOG_INODE_EXISTS, &ctx);
> 6817 }
> 6818
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/[email protected]
>
On Wed, Jan 26, 2022 at 10:31:43AM +0000, Filipe Manana wrote:
>
>
> On 25/01/22 23:12, kernel test robot wrote:
> > tree: https://github.com/kdave/btrfs-devel.git misc-next
> > head: 8e5d6a5c062f370d4d0b2dace7e95ab40c6ce3dd
> > commit: 6bfc5d45946acd8286fb026137f20ee8747a50f1 [149/153] btrfs: pass the dentry to btrfs_log_new_name() instead of the inode
> > config: nds32-allyesconfig (https://download.01.org/0day-ci/archive/20220126/[email protected]/config)
> > compiler: nds32le-linux-gcc (GCC) 11.2.0
> > reproduce (this is a W=1 build):
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # https://github.com/kdave/btrfs-devel/commit/6bfc5d45946acd8286fb026137f20ee8747a50f1
> > git remote add kdave-btrfs-devel https://github.com/kdave/btrfs-devel.git
> > git fetch --no-tags kdave-btrfs-devel misc-next
> > git checkout 6bfc5d45946acd8286fb026137f20ee8747a50f1
> > # save the config file to linux build tree
> > mkdir build_dir
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nds32 SHELL=/bin/bash fs/btrfs/
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <[email protected]>
> >
> > All warnings (new ones prefixed by >>):
> >
> >>> fs/btrfs/tree-log.c:6755: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
> > * Update the log after adding a new name for an inode.
> >
> >
> > vim +6755 fs/btrfs/tree-log.c
> >
> > 6753
> > 6754 /**
> >> 6755 * Update the log after adding a new name for an inode.
>
> The patch I submitted did not add the "/**":
>
> https://lore.kernel.org/linux-btrfs/1f0d5aaf498afa64ef3582cb9d9d24bc5f888ab2.1642676248.git.fdmanana@suse.com/T/#u
>
> However I see that David's misc-next branch, the patch adds that:
>
> https://github.com/kdave/btrfs-devel/commit/6bfc5d45946acd8286fb026137f20ee8747a50f1
>
> David, did you edit the patch? Why?
Yes I did that, because you've added the parameter descriptions and with
the /** we can ask the kdoc script to verify that they're in order as
in the function and that the list is complete. That's all what we need
from kdoc but it mandates a particular format on the first line that
does not make sense for internal helpers.