diff mbox

[12/17] md: raid1: avoid direct access to bvec table in process_checks()

Message ID 1487245547-24384-13-git-send-email-tom.leiming@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ming Lei Feb. 16, 2017, 11:45 a.m. UTC
Given process_checks is only called in resync path, it should be
ok to allocate three stack variable(total 320byteds) to store
pages from bios.

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
---
 drivers/md/raid1.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Comments

kernel test robot Feb. 17, 2017, 8:33 a.m. UTC | #1
Hi Ming,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.10-rc8]
[cannot apply to next-20170216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Ming-Lei/md-cleanup-on-direct-access-to-bvec-table/20170216-210357
config: powerpc-cell_defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from arch/powerpc/include/asm/page.h:331:0,
                    from arch/powerpc/include/asm/thread_info.h:34,
                    from include/linux/thread_info.h:25,
                    from include/asm-generic/preempt.h:4,
                    from ./arch/powerpc/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/mmzone.h:7,
                    from include/linux/gfp.h:5,
                    from include/linux/slab.h:14,
                    from drivers/md/raid1.c:34:
   drivers/md/raid1.c: In function 'raid1d':
>> include/asm-generic/memory_model.h:54:52: warning: 'sbio_pages$' may be used uninitialized in this function [-Wmaybe-uninitialized]
    #define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
                                                       ^
   drivers/md/raid1.c:2008:42: note: 'sbio_pages$' was declared here
     struct page *pbio_pages[RESYNC_PAGES], *sbio_pages[RESYNC_PAGES];
                                             ^~~~~~~~~~
   drivers/md/raid1.c:2075:9: warning: 'page_len$' may be used uninitialized in this function [-Wmaybe-uninitialized]
        if (memcmp(page_address(p),
            ^~~~~~~~~~~~~~~~~~~~~~~
            page_address(s),
            ~~~~~~~~~~~~~~~~
            page_len[j]))
            ~~~~~~~~~~~~
   drivers/md/raid1.c:2007:6: note: 'page_len$' was declared here
     int page_len[RESYNC_PAGES];
         ^~~~~~~~
   In file included from arch/powerpc/include/asm/page.h:331:0,
                    from arch/powerpc/include/asm/thread_info.h:34,
                    from include/linux/thread_info.h:25,
                    from include/asm-generic/preempt.h:4,
                    from ./arch/powerpc/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:59,
                    from include/linux/spinlock.h:50,
                    from include/linux/mmzone.h:7,
                    from include/linux/gfp.h:5,
                    from include/linux/slab.h:14,
                    from drivers/md/raid1.c:34:
>> include/asm-generic/memory_model.h:54:52: warning: 'pbio_pages$' may be used uninitialized in this function [-Wmaybe-uninitialized]
    #define __page_to_pfn(page) (unsigned long)((page) - vmemmap)
                                                       ^
   drivers/md/raid1.c:2008:15: note: 'pbio_pages$' was declared here
     struct page *pbio_pages[RESYNC_PAGES], *sbio_pages[RESYNC_PAGES];
                  ^~~~~~~~~~
   drivers/md/raid1.c:1978:8: warning: 'pages$' may be used uninitialized in this function [-Wmaybe-uninitialized]
       if (r1_sync_page_io(rdev, sect, s,
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
             pages[idx],
             ~~~~~~~~~~~
             READ) != 0)
             ~~~~~
   drivers/md/raid1.c:1872:15: note: 'pages$' was declared here
     struct page *pages[RESYNC_PAGES];
                  ^~~~~

vim +54 include/asm-generic/memory_model.h

a117e66e KAMEZAWA Hiroyuki  2006-03-27  38  ({	unsigned long __pfn = (pfn);		\
c5d71243 Rafael J. Wysocki  2008-11-08  39  	unsigned long __nid = arch_pfn_to_nid(__pfn);  \
a117e66e KAMEZAWA Hiroyuki  2006-03-27  40  	NODE_DATA(__nid)->node_mem_map + arch_local_page_offset(__pfn, __nid);\
a117e66e KAMEZAWA Hiroyuki  2006-03-27  41  })
a117e66e KAMEZAWA Hiroyuki  2006-03-27  42  
67de6482 Andy Whitcroft     2006-06-23  43  #define __page_to_pfn(pg)						\
aa462abe Ian Campbell       2011-08-17  44  ({	const struct page *__pg = (pg);					\
a0140c1d KAMEZAWA Hiroyuki  2006-03-27  45  	struct pglist_data *__pgdat = NODE_DATA(page_to_nid(__pg));	\
a0140c1d KAMEZAWA Hiroyuki  2006-03-27  46  	(unsigned long)(__pg - __pgdat->node_mem_map) +			\
a0140c1d KAMEZAWA Hiroyuki  2006-03-27  47  	 __pgdat->node_start_pfn;					\
a117e66e KAMEZAWA Hiroyuki  2006-03-27  48  })
a117e66e KAMEZAWA Hiroyuki  2006-03-27  49  
8f6aac41 Christoph Lameter  2007-10-16  50  #elif defined(CONFIG_SPARSEMEM_VMEMMAP)
8f6aac41 Christoph Lameter  2007-10-16  51  
af901ca1 André Goddard Rosa 2009-11-14  52  /* memmap is virtually contiguous.  */
8f6aac41 Christoph Lameter  2007-10-16  53  #define __pfn_to_page(pfn)	(vmemmap + (pfn))
32272a26 Martin Schwidefsky 2008-12-25 @54  #define __page_to_pfn(page)	(unsigned long)((page) - vmemmap)
8f6aac41 Christoph Lameter  2007-10-16  55  
a117e66e KAMEZAWA Hiroyuki  2006-03-27  56  #elif defined(CONFIG_SPARSEMEM)
a117e66e KAMEZAWA Hiroyuki  2006-03-27  57  /*
1a49123b Zhang Yanfei       2013-10-03  58   * Note: section's mem_map is encoded to reflect its start_pfn.
a117e66e KAMEZAWA Hiroyuki  2006-03-27  59   * section[i].section_mem_map == mem_map's address - start_pfn;
a117e66e KAMEZAWA Hiroyuki  2006-03-27  60   */
67de6482 Andy Whitcroft     2006-06-23  61  #define __page_to_pfn(pg)					\
aa462abe Ian Campbell       2011-08-17  62  ({	const struct page *__pg = (pg);				\

:::::: The code at line 54 was first introduced by commit
:::::: 32272a26974d2027384fd4010cd1780fca425d94 [S390] __page_to_pfn warnings

:::::: TO: Martin Schwidefsky <schwidefsky@de.ibm.com>
:::::: CC: Martin Schwidefsky <schwidefsky@de.ibm.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox

Patch

diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 4400fbe7ce8c..54ec32be3277 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -2010,6 +2010,9 @@  static void process_checks(struct r1bio *r1_bio)
 	int primary;
 	int i;
 	int vcnt;
+	struct bio_vec *bi;
+	int page_len[RESYNC_PAGES];
+	struct page *pbio_pages[RESYNC_PAGES], *sbio_pages[RESYNC_PAGES];
 
 	/* Fix variable parts of all bios */
 	vcnt = (r1_bio->sectors + PAGE_SIZE / 512 - 1) >> (PAGE_SHIFT - 9);
@@ -2017,7 +2020,6 @@  static void process_checks(struct r1bio *r1_bio)
 		int j;
 		int size;
 		int error;
-		struct bio_vec *bi;
 		struct bio *b = r1_bio->bios[i];
 		if (b->bi_end_io != end_sync_read)
 			continue;
@@ -2051,6 +2053,11 @@  static void process_checks(struct r1bio *r1_bio)
 			break;
 		}
 	r1_bio->read_disk = primary;
+
+	/* .bi_vcnt has been set for all read bios */
+	bio_for_each_segment_all(bi, r1_bio->bios[primary], i)
+		pbio_pages[i] = bi->bv_page;
+
 	for (i = 0; i < conf->raid_disks * 2; i++) {
 		int j;
 		struct bio *pbio = r1_bio->bios[primary];
@@ -2062,14 +2069,19 @@  static void process_checks(struct r1bio *r1_bio)
 		/* Now we can 'fixup' the error value */
 		sbio->bi_error = 0;
 
+		bio_for_each_segment_all(bi, sbio, j) {
+			sbio_pages[j] = bi->bv_page;
+			page_len[j] = bi->bv_len;
+		}
+
 		if (!error) {
 			for (j = vcnt; j-- ; ) {
 				struct page *p, *s;
-				p = pbio->bi_io_vec[j].bv_page;
-				s = sbio->bi_io_vec[j].bv_page;
+				p = pbio_pages[j];
+				s = sbio_pages[j];
 				if (memcmp(page_address(p),
 					   page_address(s),
-					   sbio->bi_io_vec[j].bv_len))
+					   page_len[j]))
 					break;
 			}
 		} else