From: kernel test robot <lkp@intel.com>
To: Roman Anasal <roman.anasal@bdsu.de>, linux-btrfs@vger.kernel.org
Cc: kbuild-all@lists.01.org, Roman Anasal <roman.anasal@bdsu.de>
Subject: Re: [PATCH 2/2] btrfs: send: fix invalid commands for inodes with changed type but same gen
Date: Tue, 12 Jan 2021 05:19:57 +0800 [thread overview]
Message-ID: <202101120515.FJAsu4W6-lkp@intel.com> (raw)
In-Reply-To: <20210111190243.4152-3-roman.anasal@bdsu.de>
[-- Attachment #1: Type: text/plain, Size: 9020 bytes --]
Hi Roman,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on btrfs/next v5.11-rc3 next-20210111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Roman-Anasal/btrfs-send-correctly-recreate-changed-inodes/20210112-030653
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/2993e0e565f9215fc3e4cedd9b1b9bc8df6dbdad
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Roman-Anasal/btrfs-send-correctly-recreate-changed-inodes/20210112-030653
git checkout 2993e0e565f9215fc3e4cedd9b1b9bc8df6dbdad
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/btrfs/send.c: In function 'changed_inode':
>> fs/btrfs/send.c:6289:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
6289 | u64 left_type = S_IFMT & btrfs_inode_mode(
| ^~~
vim +6289 fs/btrfs/send.c
6243
6244 static int changed_inode(struct send_ctx *sctx,
6245 enum btrfs_compare_tree_result result)
6246 {
6247 int ret = 0;
6248 struct btrfs_key *key = sctx->cmp_key;
6249 struct btrfs_inode_item *left_ii = NULL;
6250 struct btrfs_inode_item *right_ii = NULL;
6251 u64 left_gen = 0;
6252 u64 right_gen = 0;
6253
6254 sctx->cur_ino = key->objectid;
6255 sctx->cur_inode_recreated = 0;
6256 sctx->cur_inode_last_extent = (u64)-1;
6257 sctx->cur_inode_next_write_offset = 0;
6258 sctx->ignore_cur_inode = false;
6259
6260 /*
6261 * Set send_progress to current inode. This will tell all get_cur_xxx
6262 * functions that the current inode's refs are not updated yet. Later,
6263 * when process_recorded_refs is finished, it is set to cur_ino + 1.
6264 */
6265 sctx->send_progress = sctx->cur_ino;
6266
6267 if (result == BTRFS_COMPARE_TREE_NEW ||
6268 result == BTRFS_COMPARE_TREE_CHANGED) {
6269 left_ii = btrfs_item_ptr(sctx->left_path->nodes[0],
6270 sctx->left_path->slots[0],
6271 struct btrfs_inode_item);
6272 left_gen = btrfs_inode_generation(sctx->left_path->nodes[0],
6273 left_ii);
6274 } else {
6275 right_ii = btrfs_item_ptr(sctx->right_path->nodes[0],
6276 sctx->right_path->slots[0],
6277 struct btrfs_inode_item);
6278 right_gen = btrfs_inode_generation(sctx->right_path->nodes[0],
6279 right_ii);
6280 }
6281 if (result == BTRFS_COMPARE_TREE_CHANGED) {
6282 right_ii = btrfs_item_ptr(sctx->right_path->nodes[0],
6283 sctx->right_path->slots[0],
6284 struct btrfs_inode_item);
6285
6286 right_gen = btrfs_inode_generation(sctx->right_path->nodes[0],
6287 right_ii);
6288
> 6289 u64 left_type = S_IFMT & btrfs_inode_mode(
6290 sctx->left_path->nodes[0], left_ii);
6291 u64 right_type = S_IFMT & btrfs_inode_mode(
6292 sctx->right_path->nodes[0], right_ii);
6293
6294
6295 /*
6296 * The cur_ino = root dir case is special here. We can't treat
6297 * the inode as deleted+reused because it would generate a
6298 * stream that tries to delete/mkdir the root dir.
6299 */
6300 if ((left_gen != right_gen || left_type != right_type) &&
6301 sctx->cur_ino != BTRFS_FIRST_FREE_OBJECTID)
6302 sctx->cur_inode_recreated = 1;
6303 }
6304
6305 /*
6306 * Normally we do not find inodes with a link count of zero (orphans)
6307 * because the most common case is to create a snapshot and use it
6308 * for a send operation. However other less common use cases involve
6309 * using a subvolume and send it after turning it to RO mode just
6310 * after deleting all hard links of a file while holding an open
6311 * file descriptor against it or turning a RO snapshot into RW mode,
6312 * keep an open file descriptor against a file, delete it and then
6313 * turn the snapshot back to RO mode before using it for a send
6314 * operation. So if we find such cases, ignore the inode and all its
6315 * items completely if it's a new inode, or if it's a changed inode
6316 * make sure all its previous paths (from the parent snapshot) are all
6317 * unlinked and all other the inode items are ignored.
6318 */
6319 if (result == BTRFS_COMPARE_TREE_NEW ||
6320 result == BTRFS_COMPARE_TREE_CHANGED) {
6321 u32 nlinks;
6322
6323 nlinks = btrfs_inode_nlink(sctx->left_path->nodes[0], left_ii);
6324 if (nlinks == 0) {
6325 sctx->ignore_cur_inode = true;
6326 if (result == BTRFS_COMPARE_TREE_CHANGED)
6327 ret = btrfs_unlink_all_paths(sctx);
6328 goto out;
6329 }
6330 }
6331
6332 if (result == BTRFS_COMPARE_TREE_NEW) {
6333 sctx->cur_inode_gen = left_gen;
6334 sctx->cur_inode_new = 1;
6335 sctx->cur_inode_deleted = 0;
6336 sctx->cur_inode_size = btrfs_inode_size(
6337 sctx->left_path->nodes[0], left_ii);
6338 sctx->cur_inode_mode = btrfs_inode_mode(
6339 sctx->left_path->nodes[0], left_ii);
6340 sctx->cur_inode_rdev = btrfs_inode_rdev(
6341 sctx->left_path->nodes[0], left_ii);
6342 if (sctx->cur_ino != BTRFS_FIRST_FREE_OBJECTID)
6343 ret = send_create_inode_if_needed(sctx);
6344 } else if (result == BTRFS_COMPARE_TREE_DELETED) {
6345 sctx->cur_inode_gen = right_gen;
6346 sctx->cur_inode_new = 0;
6347 sctx->cur_inode_deleted = 1;
6348 sctx->cur_inode_size = btrfs_inode_size(
6349 sctx->right_path->nodes[0], right_ii);
6350 sctx->cur_inode_mode = btrfs_inode_mode(
6351 sctx->right_path->nodes[0], right_ii);
6352 } else if (result == BTRFS_COMPARE_TREE_CHANGED) {
6353 /*
6354 * We need to do some special handling in case the inode was
6355 * reported as changed with a changed generation number or
6356 * changed inode type. This means that the original inode was
6357 * deleted and new inode reused the same inum. So we have to
6358 * treat the old inode as deleted and the new one as new.
6359 */
6360 if (sctx->cur_inode_recreated) {
6361 /*
6362 * First, process the inode as if it was deleted.
6363 */
6364 sctx->cur_inode_gen = right_gen;
6365 sctx->cur_inode_new = 0;
6366 sctx->cur_inode_deleted = 1;
6367 sctx->cur_inode_size = btrfs_inode_size(
6368 sctx->right_path->nodes[0], right_ii);
6369 sctx->cur_inode_mode = btrfs_inode_mode(
6370 sctx->right_path->nodes[0], right_ii);
6371 ret = process_all_refs(sctx,
6372 BTRFS_COMPARE_TREE_DELETED);
6373 if (ret < 0)
6374 goto out;
6375
6376 /*
6377 * Now process the inode as if it was new.
6378 */
6379 sctx->cur_inode_gen = left_gen;
6380 sctx->cur_inode_new = 1;
6381 sctx->cur_inode_deleted = 0;
6382 sctx->cur_inode_size = btrfs_inode_size(
6383 sctx->left_path->nodes[0], left_ii);
6384 sctx->cur_inode_mode = btrfs_inode_mode(
6385 sctx->left_path->nodes[0], left_ii);
6386 sctx->cur_inode_rdev = btrfs_inode_rdev(
6387 sctx->left_path->nodes[0], left_ii);
6388 ret = send_create_inode_if_needed(sctx);
6389 if (ret < 0)
6390 goto out;
6391
6392 ret = process_all_refs(sctx, BTRFS_COMPARE_TREE_NEW);
6393 if (ret < 0)
6394 goto out;
6395 /*
6396 * Advance send_progress now as we did not get into
6397 * process_recorded_refs_if_needed in the
6398 * cur_inode_recreated case.
6399 */
6400 sctx->send_progress = sctx->cur_ino + 1;
6401
6402 /*
6403 * Now process all extents and xattrs of the inode as if
6404 * they were all new.
6405 */
6406 ret = process_all_extents(sctx);
6407 if (ret < 0)
6408 goto out;
6409 ret = process_all_new_xattrs(sctx);
6410 if (ret < 0)
6411 goto out;
6412 } else {
6413 sctx->cur_inode_gen = left_gen;
6414 sctx->cur_inode_new = 0;
6415 sctx->cur_inode_recreated = 0;
6416 sctx->cur_inode_deleted = 0;
6417 sctx->cur_inode_size = btrfs_inode_size(
6418 sctx->left_path->nodes[0], left_ii);
6419 sctx->cur_inode_mode = btrfs_inode_mode(
6420 sctx->left_path->nodes[0], left_ii);
6421 }
6422 }
6423
6424 out:
6425 return ret;
6426 }
6427
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 75883 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: kernel test robot <lkp@intel.com>
To: kbuild-all@lists.01.org
Subject: Re: [PATCH 2/2] btrfs: send: fix invalid commands for inodes with changed type but same gen
Date: Tue, 12 Jan 2021 05:19:57 +0800 [thread overview]
Message-ID: <202101120515.FJAsu4W6-lkp@intel.com> (raw)
In-Reply-To: <20210111190243.4152-3-roman.anasal@bdsu.de>
[-- Attachment #1: Type: text/plain, Size: 9246 bytes --]
Hi Roman,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on kdave/for-next]
[also build test WARNING on btrfs/next v5.11-rc3 next-20210111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Roman-Anasal/btrfs-send-correctly-recreate-changed-inodes/20210112-030653
base: https://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/2993e0e565f9215fc3e4cedd9b1b9bc8df6dbdad
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Roman-Anasal/btrfs-send-correctly-recreate-changed-inodes/20210112-030653
git checkout 2993e0e565f9215fc3e4cedd9b1b9bc8df6dbdad
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
fs/btrfs/send.c: In function 'changed_inode':
>> fs/btrfs/send.c:6289:3: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
6289 | u64 left_type = S_IFMT & btrfs_inode_mode(
| ^~~
vim +6289 fs/btrfs/send.c
6243
6244 static int changed_inode(struct send_ctx *sctx,
6245 enum btrfs_compare_tree_result result)
6246 {
6247 int ret = 0;
6248 struct btrfs_key *key = sctx->cmp_key;
6249 struct btrfs_inode_item *left_ii = NULL;
6250 struct btrfs_inode_item *right_ii = NULL;
6251 u64 left_gen = 0;
6252 u64 right_gen = 0;
6253
6254 sctx->cur_ino = key->objectid;
6255 sctx->cur_inode_recreated = 0;
6256 sctx->cur_inode_last_extent = (u64)-1;
6257 sctx->cur_inode_next_write_offset = 0;
6258 sctx->ignore_cur_inode = false;
6259
6260 /*
6261 * Set send_progress to current inode. This will tell all get_cur_xxx
6262 * functions that the current inode's refs are not updated yet. Later,
6263 * when process_recorded_refs is finished, it is set to cur_ino + 1.
6264 */
6265 sctx->send_progress = sctx->cur_ino;
6266
6267 if (result == BTRFS_COMPARE_TREE_NEW ||
6268 result == BTRFS_COMPARE_TREE_CHANGED) {
6269 left_ii = btrfs_item_ptr(sctx->left_path->nodes[0],
6270 sctx->left_path->slots[0],
6271 struct btrfs_inode_item);
6272 left_gen = btrfs_inode_generation(sctx->left_path->nodes[0],
6273 left_ii);
6274 } else {
6275 right_ii = btrfs_item_ptr(sctx->right_path->nodes[0],
6276 sctx->right_path->slots[0],
6277 struct btrfs_inode_item);
6278 right_gen = btrfs_inode_generation(sctx->right_path->nodes[0],
6279 right_ii);
6280 }
6281 if (result == BTRFS_COMPARE_TREE_CHANGED) {
6282 right_ii = btrfs_item_ptr(sctx->right_path->nodes[0],
6283 sctx->right_path->slots[0],
6284 struct btrfs_inode_item);
6285
6286 right_gen = btrfs_inode_generation(sctx->right_path->nodes[0],
6287 right_ii);
6288
> 6289 u64 left_type = S_IFMT & btrfs_inode_mode(
6290 sctx->left_path->nodes[0], left_ii);
6291 u64 right_type = S_IFMT & btrfs_inode_mode(
6292 sctx->right_path->nodes[0], right_ii);
6293
6294
6295 /*
6296 * The cur_ino = root dir case is special here. We can't treat
6297 * the inode as deleted+reused because it would generate a
6298 * stream that tries to delete/mkdir the root dir.
6299 */
6300 if ((left_gen != right_gen || left_type != right_type) &&
6301 sctx->cur_ino != BTRFS_FIRST_FREE_OBJECTID)
6302 sctx->cur_inode_recreated = 1;
6303 }
6304
6305 /*
6306 * Normally we do not find inodes with a link count of zero (orphans)
6307 * because the most common case is to create a snapshot and use it
6308 * for a send operation. However other less common use cases involve
6309 * using a subvolume and send it after turning it to RO mode just
6310 * after deleting all hard links of a file while holding an open
6311 * file descriptor against it or turning a RO snapshot into RW mode,
6312 * keep an open file descriptor against a file, delete it and then
6313 * turn the snapshot back to RO mode before using it for a send
6314 * operation. So if we find such cases, ignore the inode and all its
6315 * items completely if it's a new inode, or if it's a changed inode
6316 * make sure all its previous paths (from the parent snapshot) are all
6317 * unlinked and all other the inode items are ignored.
6318 */
6319 if (result == BTRFS_COMPARE_TREE_NEW ||
6320 result == BTRFS_COMPARE_TREE_CHANGED) {
6321 u32 nlinks;
6322
6323 nlinks = btrfs_inode_nlink(sctx->left_path->nodes[0], left_ii);
6324 if (nlinks == 0) {
6325 sctx->ignore_cur_inode = true;
6326 if (result == BTRFS_COMPARE_TREE_CHANGED)
6327 ret = btrfs_unlink_all_paths(sctx);
6328 goto out;
6329 }
6330 }
6331
6332 if (result == BTRFS_COMPARE_TREE_NEW) {
6333 sctx->cur_inode_gen = left_gen;
6334 sctx->cur_inode_new = 1;
6335 sctx->cur_inode_deleted = 0;
6336 sctx->cur_inode_size = btrfs_inode_size(
6337 sctx->left_path->nodes[0], left_ii);
6338 sctx->cur_inode_mode = btrfs_inode_mode(
6339 sctx->left_path->nodes[0], left_ii);
6340 sctx->cur_inode_rdev = btrfs_inode_rdev(
6341 sctx->left_path->nodes[0], left_ii);
6342 if (sctx->cur_ino != BTRFS_FIRST_FREE_OBJECTID)
6343 ret = send_create_inode_if_needed(sctx);
6344 } else if (result == BTRFS_COMPARE_TREE_DELETED) {
6345 sctx->cur_inode_gen = right_gen;
6346 sctx->cur_inode_new = 0;
6347 sctx->cur_inode_deleted = 1;
6348 sctx->cur_inode_size = btrfs_inode_size(
6349 sctx->right_path->nodes[0], right_ii);
6350 sctx->cur_inode_mode = btrfs_inode_mode(
6351 sctx->right_path->nodes[0], right_ii);
6352 } else if (result == BTRFS_COMPARE_TREE_CHANGED) {
6353 /*
6354 * We need to do some special handling in case the inode was
6355 * reported as changed with a changed generation number or
6356 * changed inode type. This means that the original inode was
6357 * deleted and new inode reused the same inum. So we have to
6358 * treat the old inode as deleted and the new one as new.
6359 */
6360 if (sctx->cur_inode_recreated) {
6361 /*
6362 * First, process the inode as if it was deleted.
6363 */
6364 sctx->cur_inode_gen = right_gen;
6365 sctx->cur_inode_new = 0;
6366 sctx->cur_inode_deleted = 1;
6367 sctx->cur_inode_size = btrfs_inode_size(
6368 sctx->right_path->nodes[0], right_ii);
6369 sctx->cur_inode_mode = btrfs_inode_mode(
6370 sctx->right_path->nodes[0], right_ii);
6371 ret = process_all_refs(sctx,
6372 BTRFS_COMPARE_TREE_DELETED);
6373 if (ret < 0)
6374 goto out;
6375
6376 /*
6377 * Now process the inode as if it was new.
6378 */
6379 sctx->cur_inode_gen = left_gen;
6380 sctx->cur_inode_new = 1;
6381 sctx->cur_inode_deleted = 0;
6382 sctx->cur_inode_size = btrfs_inode_size(
6383 sctx->left_path->nodes[0], left_ii);
6384 sctx->cur_inode_mode = btrfs_inode_mode(
6385 sctx->left_path->nodes[0], left_ii);
6386 sctx->cur_inode_rdev = btrfs_inode_rdev(
6387 sctx->left_path->nodes[0], left_ii);
6388 ret = send_create_inode_if_needed(sctx);
6389 if (ret < 0)
6390 goto out;
6391
6392 ret = process_all_refs(sctx, BTRFS_COMPARE_TREE_NEW);
6393 if (ret < 0)
6394 goto out;
6395 /*
6396 * Advance send_progress now as we did not get into
6397 * process_recorded_refs_if_needed in the
6398 * cur_inode_recreated case.
6399 */
6400 sctx->send_progress = sctx->cur_ino + 1;
6401
6402 /*
6403 * Now process all extents and xattrs of the inode as if
6404 * they were all new.
6405 */
6406 ret = process_all_extents(sctx);
6407 if (ret < 0)
6408 goto out;
6409 ret = process_all_new_xattrs(sctx);
6410 if (ret < 0)
6411 goto out;
6412 } else {
6413 sctx->cur_inode_gen = left_gen;
6414 sctx->cur_inode_new = 0;
6415 sctx->cur_inode_recreated = 0;
6416 sctx->cur_inode_deleted = 0;
6417 sctx->cur_inode_size = btrfs_inode_size(
6418 sctx->left_path->nodes[0], left_ii);
6419 sctx->cur_inode_mode = btrfs_inode_mode(
6420 sctx->left_path->nodes[0], left_ii);
6421 }
6422 }
6423
6424 out:
6425 return ret;
6426 }
6427
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 75883 bytes --]
next prev parent reply other threads:[~2021-01-11 21:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-11 19:02 [PATCH 0/2] btrfs: send: correctly recreate changed inodes Roman Anasal
2021-01-11 19:02 ` [PATCH 1/2] btrfs: send: rename send_ctx.cur_inode_new_gen to cur_inode_recreated Roman Anasal
2021-01-11 19:02 ` [PATCH 2/2] btrfs: send: fix invalid commands for inodes with changed type but same gen Roman Anasal
2021-01-11 19:30 ` Andrei Borzenkov
2021-01-11 20:53 ` Roman Anasal | BDSU
2021-01-12 11:27 ` Filipe Manana
2021-01-12 12:07 ` Graham Cobb
2021-01-12 12:30 ` Filipe Manana
2021-01-12 13:10 ` Roman Anasal | BDSU
2021-01-12 13:54 ` Filipe Manana
2021-01-12 14:37 ` Roman Anasal | BDSU
2021-01-12 15:08 ` Filipe Manana
2021-01-11 21:15 ` David Sterba
2021-01-11 21:31 ` Roman Anasal | BDSU
2021-01-11 21:19 ` kernel test robot [this message]
2021-01-11 21:19 ` kernel test robot
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=202101120515.FJAsu4W6-lkp@intel.com \
--to=lkp@intel.com \
--cc=kbuild-all@lists.01.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=roman.anasal@bdsu.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.