From: Changwei Ge <ge.changwei@h3c.com>
To: Dan Carpenter <dan.carpenter@oracle.com>,
"kbuild@01.org" <kbuild@01.org>
Cc: "kbuild-all@01.org" <kbuild-all@01.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [linux-next:master 10644/11012] fs/ocfs2/alloc.c:6761 ocfs2_reuse_blk_from_dealloc() warn: potentially one past the end of array 'new_eb_bh[i]'
Date: Wed, 31 Jan 2018 13:22:45 +0000 [thread overview]
Message-ID: <63ADC13FD55D6546B7DECE290D39E373F29196AE@H3CMLB12-EX.srv.huawei-3com.com> (raw)
In-Reply-To: <20180131105004.xdig2mzgrmiagf5l@mwanda>
Hi Dan,
In order to make static checker happy, I wanna give a fix to silence those warning.
Could you please help to trigger the checker again to see if they are gone.
If any further warning shows up, please feel free to let me know.
Subject: [PATCH] ocfs2: fix static checker warnning
Signed-off-by: Changwei Ge <ge.changwei@h3c.com>
---
fs/ocfs2/alloc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c
index ec1ebbf..084b8b9 100644
--- a/fs/ocfs2/alloc.c
+++ b/fs/ocfs2/alloc.c
@@ -6765,8 +6765,8 @@ static int ocfs2_reuse_blk_from_dealloc(handle_t *handle,
*blk_given = i;
bail:
- if (unlikely(status)) {
- for (; i >= 0; i--) {
+ if (unlikely(status < 0)) {
+ for (i = 0; i < blk_wanted; i++) {
if (new_eb_bh[i])
brelse(new_eb_bh[i]);
}
--
2.7.4
On 2018/1/31 18:55, Dan Carpenter wrote:
>
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head: 761914dd2975bc443024f0ec10a66a26b7186ec2
> commit: 0d3e622b2ac768ac5a94f8d9ede80a051154ea9e [10644/11012] ocfs2: try to reuse extent block in dealloc without meta_alloc
>
> New smatch warnings:
> fs/ocfs2/alloc.c:6761 ocfs2_reuse_blk_from_dealloc() warn: potentially one past the end of array 'new_eb_bh[i]'
> fs/ocfs2/alloc.c:6761 ocfs2_reuse_blk_from_dealloc() warn: potentially one past the end of array 'new_eb_bh[i]'
>
> Old smatch warnings:
> fs/ocfs2/alloc.c:6762 ocfs2_reuse_blk_from_dealloc() warn: potentially one past the end of array 'new_eb_bh[i]'
> fs/ocfs2/alloc.c:6762 ocfs2_reuse_blk_from_dealloc() warn: potentially one past the end of array 'new_eb_bh[i]'
> fs/ocfs2/alloc.c:6887 ocfs2_zero_cluster_pages() warn: should '(page->index + 1) << 12' be a 64 bit type?
>
> # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=0d3e622b2ac768ac5a94f8d9ede80a051154ea9e
> git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> git remote update linux-next
> git checkout 0d3e622b2ac768ac5a94f8d9ede80a051154ea9e
> vim +6761 fs/ocfs2/alloc.c
>
> 0d3e622b Changwei Ge 2018-01-19 6662
> 0d3e622b Changwei Ge 2018-01-19 6663 /* If extent was deleted from tree due to extent rotation and merging, and
> 0d3e622b Changwei Ge 2018-01-19 6664 * no metadata is reserved ahead of time. Try to reuse some extents
> 0d3e622b Changwei Ge 2018-01-19 6665 * just deleted. This is only used to reuse extent blocks.
> 0d3e622b Changwei Ge 2018-01-19 6666 * It is supposed to find enough extent blocks in dealloc if our estimation
> 0d3e622b Changwei Ge 2018-01-19 6667 * on metadata is accurate.
> 0d3e622b Changwei Ge 2018-01-19 6668 */
> 0d3e622b Changwei Ge 2018-01-19 6669 static int ocfs2_reuse_blk_from_dealloc(handle_t *handle,
> 0d3e622b Changwei Ge 2018-01-19 6670 struct ocfs2_extent_tree *et,
> 0d3e622b Changwei Ge 2018-01-19 6671 struct buffer_head **new_eb_bh,
> 0d3e622b Changwei Ge 2018-01-19 6672 int blk_wanted, int *blk_given)
> 0d3e622b Changwei Ge 2018-01-19 6673 {
> 0d3e622b Changwei Ge 2018-01-19 6674 int i, status = 0, real_slot;
> 0d3e622b Changwei Ge 2018-01-19 6675 struct ocfs2_cached_dealloc_ctxt *dealloc;
> 0d3e622b Changwei Ge 2018-01-19 6676 struct ocfs2_per_slot_free_list *fl;
> 0d3e622b Changwei Ge 2018-01-19 6677 struct ocfs2_cached_block_free *bf;
> 0d3e622b Changwei Ge 2018-01-19 6678 struct ocfs2_extent_block *eb;
> 0d3e622b Changwei Ge 2018-01-19 6679 struct ocfs2_super *osb =
> 0d3e622b Changwei Ge 2018-01-19 6680 OCFS2_SB(ocfs2_metadata_cache_get_super(et->et_ci));
> 0d3e622b Changwei Ge 2018-01-19 6681
> 0d3e622b Changwei Ge 2018-01-19 6682 *blk_given = 0;
> 0d3e622b Changwei Ge 2018-01-19 6683
> 0d3e622b Changwei Ge 2018-01-19 6684 /* If extent tree doesn't have a dealloc, this is not faulty. Just
> 0d3e622b Changwei Ge 2018-01-19 6685 * tell upper caller dealloc can't provide any block and it should
> 0d3e622b Changwei Ge 2018-01-19 6686 * ask for alloc to claim more space.
> 0d3e622b Changwei Ge 2018-01-19 6687 */
> 0d3e622b Changwei Ge 2018-01-19 6688 dealloc = et->et_dealloc;
> 0d3e622b Changwei Ge 2018-01-19 6689 if (!dealloc)
> 0d3e622b Changwei Ge 2018-01-19 6690 goto bail;
> 0d3e622b Changwei Ge 2018-01-19 6691
> 0d3e622b Changwei Ge 2018-01-19 6692 for (i = 0; i < blk_wanted; i++) {
> 0d3e622b Changwei Ge 2018-01-19 6693 /* Prefer to use local slot */
> 0d3e622b Changwei Ge 2018-01-19 6694 fl = ocfs2_find_preferred_free_list(EXTENT_ALLOC_SYSTEM_INODE,
> 0d3e622b Changwei Ge 2018-01-19 6695 osb->slot_num, &real_slot,
> 0d3e622b Changwei Ge 2018-01-19 6696 dealloc);
> 0d3e622b Changwei Ge 2018-01-19 6697 /* If no more block can be reused, we should claim more
> 0d3e622b Changwei Ge 2018-01-19 6698 * from alloc. Just return here normally.
> 0d3e622b Changwei Ge 2018-01-19 6699 */
> 0d3e622b Changwei Ge 2018-01-19 6700 if (!fl) {
> 0d3e622b Changwei Ge 2018-01-19 6701 status = 0;
> 0d3e622b Changwei Ge 2018-01-19 6702 break;
> 0d3e622b Changwei Ge 2018-01-19 6703 }
> 0d3e622b Changwei Ge 2018-01-19 6704
> 0d3e622b Changwei Ge 2018-01-19 6705 bf = fl->f_first;
> 0d3e622b Changwei Ge 2018-01-19 6706 fl->f_first = bf->free_next;
> 0d3e622b Changwei Ge 2018-01-19 6707
> 0d3e622b Changwei Ge 2018-01-19 6708 new_eb_bh[i] = sb_getblk(osb->sb, bf->free_blk);
> 0d3e622b Changwei Ge 2018-01-19 6709 if (new_eb_bh[i] == NULL) {
> 0d3e622b Changwei Ge 2018-01-19 6710 status = -ENOMEM;
> 0d3e622b Changwei Ge 2018-01-19 6711 mlog_errno(status);
> 0d3e622b Changwei Ge 2018-01-19 6712 goto bail;
> 0d3e622b Changwei Ge 2018-01-19 6713 }
> 0d3e622b Changwei Ge 2018-01-19 6714
> 0d3e622b Changwei Ge 2018-01-19 6715 mlog(0, "Reusing block(%llu) from "
> 0d3e622b Changwei Ge 2018-01-19 6716 "dealloc(local slot:%d, real slot:%d)\n",
> 0d3e622b Changwei Ge 2018-01-19 6717 bf->free_blk, osb->slot_num, real_slot);
> 0d3e622b Changwei Ge 2018-01-19 6718
> 0d3e622b Changwei Ge 2018-01-19 6719 ocfs2_set_new_buffer_uptodate(et->et_ci, new_eb_bh[i]);
> 0d3e622b Changwei Ge 2018-01-19 6720
> 0d3e622b Changwei Ge 2018-01-19 6721 status = ocfs2_journal_access_eb(handle, et->et_ci,
> 0d3e622b Changwei Ge 2018-01-19 6722 new_eb_bh[i],
> 0d3e622b Changwei Ge 2018-01-19 6723 OCFS2_JOURNAL_ACCESS_CREATE);
> 0d3e622b Changwei Ge 2018-01-19 6724 if (status < 0) {
> ^^^^^^^^^^
> The warning is a false positive. It's caused because the check here is
> for less than zero and the check at the end is for non-zero. The static
> checker is thinking that status can be > 0 here.
>
> If both checks were written the same way, that would silence the
> warning.
>
> Also if you rebuild your cross function DB a bunch of time that silences
> the warning because then Smatch know that ocfs2_journal_access_eb()
> returns (-30),(-22),(-5), or 0. I rebuild my DB every morning on the
> latest linux-next so I don't see this warning on my system.
>
> 0d3e622b Changwei Ge 2018-01-19 6725 mlog_errno(status);
> 0d3e622b Changwei Ge 2018-01-19 6726 goto bail;
> 0d3e622b Changwei Ge 2018-01-19 6727 }
> 0d3e622b Changwei Ge 2018-01-19 6728
> 0d3e622b Changwei Ge 2018-01-19 6729 memset(new_eb_bh[i]->b_data, 0, osb->sb->s_blocksize);
> 0d3e622b Changwei Ge 2018-01-19 6730 eb = (struct ocfs2_extent_block *) new_eb_bh[i]->b_data;
> 0d3e622b Changwei Ge 2018-01-19 6731
> 0d3e622b Changwei Ge 2018-01-19 6732 /* We can't guarantee that buffer head is still cached, so
> 0d3e622b Changwei Ge 2018-01-19 6733 * polutlate the extent block again.
> 0d3e622b Changwei Ge 2018-01-19 6734 */
> 0d3e622b Changwei Ge 2018-01-19 6735 strcpy(eb->h_signature, OCFS2_EXTENT_BLOCK_SIGNATURE);
> 0d3e622b Changwei Ge 2018-01-19 6736 eb->h_blkno = cpu_to_le64(bf->free_blk);
> 0d3e622b Changwei Ge 2018-01-19 6737 eb->h_fs_generation = cpu_to_le32(osb->fs_generation);
> 0d3e622b Changwei Ge 2018-01-19 6738 eb->h_suballoc_slot = cpu_to_le16(real_slot);
> 0d3e622b Changwei Ge 2018-01-19 6739 eb->h_suballoc_loc = cpu_to_le64(bf->free_bg);
> 0d3e622b Changwei Ge 2018-01-19 6740 eb->h_suballoc_bit = cpu_to_le16(bf->free_bit);
> 0d3e622b Changwei Ge 2018-01-19 6741 eb->h_list.l_count =
> 0d3e622b Changwei Ge 2018-01-19 6742 cpu_to_le16(ocfs2_extent_recs_per_eb(osb->sb));
> 0d3e622b Changwei Ge 2018-01-19 6743
> 0d3e622b Changwei Ge 2018-01-19 6744 /* We'll also be dirtied by the caller, so
> 0d3e622b Changwei Ge 2018-01-19 6745 * this isn't absolutely necessary.
> 0d3e622b Changwei Ge 2018-01-19 6746 */
> 0d3e622b Changwei Ge 2018-01-19 6747 ocfs2_journal_dirty(handle, new_eb_bh[i]);
> 0d3e622b Changwei Ge 2018-01-19 6748
> 0d3e622b Changwei Ge 2018-01-19 6749 if (!fl->f_first) {
> 0d3e622b Changwei Ge 2018-01-19 6750 dealloc->c_first_suballocator = fl->f_next_suballocator;
> 0d3e622b Changwei Ge 2018-01-19 6751 kfree(fl);
> 0d3e622b Changwei Ge 2018-01-19 6752 }
> 0d3e622b Changwei Ge 2018-01-19 6753 kfree(bf);
> 0d3e622b Changwei Ge 2018-01-19 6754 }
> 0d3e622b Changwei Ge 2018-01-19 6755
> 0d3e622b Changwei Ge 2018-01-19 6756 *blk_given = i;
> 0d3e622b Changwei Ge 2018-01-19 6757
> 0d3e622b Changwei Ge 2018-01-19 6758 bail:
> 0d3e622b Changwei Ge 2018-01-19 6759 if (unlikely(status)) {
> ^^^^^^
>
> 0d3e622b Changwei Ge 2018-01-19 6760 for (; i >= 0; i--) {
> 0d3e622b Changwei Ge 2018-01-19 @6761 if (new_eb_bh[i])
> 0d3e622b Changwei Ge 2018-01-19 6762 brelse(new_eb_bh[i]);
> 0d3e622b Changwei Ge 2018-01-19 6763 }
> 0d3e622b Changwei Ge 2018-01-19 6764 }
> 0d3e622b Changwei Ge 2018-01-19 6765
> 0d3e622b Changwei Ge 2018-01-19 6766 return status;
> 0d3e622b Changwei Ge 2018-01-19 6767 }
> 0d3e622b Changwei Ge 2018-01-19 6768
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
prev parent reply other threads:[~2018-01-31 13:34 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-31 10:50 Dan Carpenter
2018-01-31 13:22 ` Changwei Ge [this message]
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=63ADC13FD55D6546B7DECE290D39E373F29196AE@H3CMLB12-EX.srv.huawei-3com.com \
--to=ge.changwei@h3c.com \
--cc=akpm@linux-foundation.org \
--cc=dan.carpenter@oracle.com \
--cc=kbuild-all@01.org \
--cc=kbuild@01.org \
--cc=linux-mm@kvack.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