Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S964800AbcJ0NwL (ORCPT ); Thu, 27 Oct 2016 09:52:11 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:15776 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S936381AbcJ0NvC (ORCPT ); Thu, 27 Oct 2016 09:51:02 -0400 Subject: Re: [f2fs-dev] [PATCH 04/28] f2fs: replace a build-time warning with runtime WARN_ON To: Arnd Bergmann , Chao Yu References: <20161017220342.1627073-1-arnd@arndb.de> <20161017220557.1688282-4-arnd@arndb.de> <3766259.9ZsupLsmdk@wuerfel> CC: Jaegeuk Kim , , , Weichao Guo , Linus Torvalds From: Chao Yu Message-ID: Date: Thu, 27 Oct 2016 19:41:05 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: <3766259.9ZsupLsmdk@wuerfel> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.134.22.195] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1967 Lines: 53 On 2016/10/26 22:57, Arnd Bergmann wrote: > On Wednesday, October 26, 2016 10:05:00 PM CEST Chao Yu wrote: >> On 2016/10/18 6:05, Arnd Bergmann wrote: >>> gcc is unsure about the use of last_ofs_in_node, which might happen >>> without a prior initialization: >>> >>> fs/f2fs//git/arm-soc/fs/f2fs/data.c: In function ‘f2fs_map_blocks’: >>> fs/f2fs/data.c:799:54: warning: ‘last_ofs_in_node’ may be used uninitialized in this function [-Wmaybe-uninitialized] >>> if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) { >> >> In each round of dnode block traverse, we will init 'prealloc' and then update >> 'prealloc' and 'last_ofs_in_node' together in below lines of f2fs_map_blocks: >> if (flag == F2FS_GET_BLOCK_PRE_AIO) { >> if (blkaddr == NULL_ADDR) { >> prealloc++; >> last_ofs_in_node = dn.ofs_in_node; >> } >> } >> >> Then in below codes, it is safe to use 'last_ofs_in_node' since we will check >> 'prealloc' firstly, so if 'prealloc' is non-zero, 'last_ofs_in_node' must be valid. >> if (prealloc && dn.ofs_in_node != last_ofs_in_node + 1) { >> >> So I think we should not add WARN_ON there. > > Ok, that make sense. Thanks for taking a closer look! > > Should we just set last_ofs_in_node to the same value as ofs_in_node > before the loop? I think it's OK as it can remove warning compiler reports. :) Thanks, > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 9ae194f..14db4b7 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -716,7 +716,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, > } > > prealloc = 0; > - ofs_in_node = dn.ofs_in_node; > + last_ofs_in_node = ofs_in_node = dn.ofs_in_node; > end_offset = ADDRS_PER_PAGE(dn.node_page, inode); > > next_block: > > Arnd > > . >