From: Matthew Wilcox <willy@infradead.org> To: Michal Hocko <mhocko@kernel.org> Cc: Mike Rapoport <rppt@linux.ibm.com>, Rong Chen <rong.a.chen@intel.com>, Pavel Tatashin <pasha.tatashin@soleen.com>, linux-kernel@vger.kernel.org, Linux Memory Management List <linux-mm@kvack.org>, Andrew Morton <akpm@linux-foundation.org>, LKP <lkp@01.org>, Oscar Salvador <osalvador@suse.de> Subject: Re: [LKP] efad4e475c [ 40.308255] Oops: 0000 [#1] PREEMPT SMP PTI Date: Mon, 18 Feb 2019 11:05:19 -0800 [thread overview] Message-ID: <20190218190519.GV12668@bombadil.infradead.org> (raw) In-Reply-To: <20190218181155.GC4525@dhcp22.suse.cz> On Mon, Feb 18, 2019 at 07:11:55PM +0100, Michal Hocko wrote: > On Mon 18-02-19 09:57:26, Matthew Wilcox wrote: > > On Mon, Feb 18, 2019 at 06:05:58PM +0100, Michal Hocko wrote: > > > + end_pfn = min(start_pfn + nr_pages, > > > + zone_end_pfn(page_zone(pfn_to_page(start_pfn)))); > > > > > > /* Check the starting page of each pageblock within the range */ > > > - for (; page < end_page; page = next_active_pageblock(page)) { > > > - if (!is_pageblock_removable_nolock(page)) > > > + for (; start_pfn < end_pfn; start_pfn = next_active_pageblock(start_pfn)) { > > > + if (!is_pageblock_removable_nolock(start_pfn)) > > > > If you have a zone which contains pfns that run from ULONG_MAX-n to ULONG_MAX, > > end_pfn is going to wrap around to 0 and this loop won't execute. > > Is this a realistic situation to bother? How insane do you think hardware manufacturers are ... ? I don't know of one today, but I wouldn't bet on something like that never existing. > > I think > > you should use: > > > > max_pfn = min(start_pfn + nr_pages, > > zone_end_pfn(page_zone(pfn_to_page(start_pfn)))) - 1; > > > > for (; start_pfn <= max_pfn; ...) > > I do not really care strongly, but we have more places were we do > start_pfn + nr_pages and then use it as pfn < end_pfn construct. I > suspect we would need to make a larger audit and make the code > consistent so unless there are major concerns I would stick with what > I have for now and leave the rest for the cleanup. Does that sound > reasonable? Yes, I think so. There are a number of other places where we can wrap around from ULONG_MAX to 0 fairly easily (eg page offsets in a file on 32-bit machines). I started thinking about this with the XArray and rapidly convinced myself we have a problem throughout Linux.
WARNING: multiple messages have this Message-ID (diff)
From: Matthew Wilcox <willy@infradead.org> To: lkp@lists.01.org Subject: Re: efad4e475c [ 40.308255] Oops: 0000 [#1] PREEMPT SMP PTI Date: Mon, 18 Feb 2019 11:05:19 -0800 [thread overview] Message-ID: <20190218190519.GV12668@bombadil.infradead.org> (raw) In-Reply-To: <20190218181155.GC4525@dhcp22.suse.cz> [-- Attachment #1: Type: text/plain, Size: 1828 bytes --] On Mon, Feb 18, 2019 at 07:11:55PM +0100, Michal Hocko wrote: > On Mon 18-02-19 09:57:26, Matthew Wilcox wrote: > > On Mon, Feb 18, 2019 at 06:05:58PM +0100, Michal Hocko wrote: > > > + end_pfn = min(start_pfn + nr_pages, > > > + zone_end_pfn(page_zone(pfn_to_page(start_pfn)))); > > > > > > /* Check the starting page of each pageblock within the range */ > > > - for (; page < end_page; page = next_active_pageblock(page)) { > > > - if (!is_pageblock_removable_nolock(page)) > > > + for (; start_pfn < end_pfn; start_pfn = next_active_pageblock(start_pfn)) { > > > + if (!is_pageblock_removable_nolock(start_pfn)) > > > > If you have a zone which contains pfns that run from ULONG_MAX-n to ULONG_MAX, > > end_pfn is going to wrap around to 0 and this loop won't execute. > > Is this a realistic situation to bother? How insane do you think hardware manufacturers are ... ? I don't know of one today, but I wouldn't bet on something like that never existing. > > I think > > you should use: > > > > max_pfn = min(start_pfn + nr_pages, > > zone_end_pfn(page_zone(pfn_to_page(start_pfn)))) - 1; > > > > for (; start_pfn <= max_pfn; ...) > > I do not really care strongly, but we have more places were we do > start_pfn + nr_pages and then use it as pfn < end_pfn construct. I > suspect we would need to make a larger audit and make the code > consistent so unless there are major concerns I would stick with what > I have for now and leave the rest for the cleanup. Does that sound > reasonable? Yes, I think so. There are a number of other places where we can wrap around from ULONG_MAX to 0 fairly easily (eg page offsets in a file on 32-bit machines). I started thinking about this with the XArray and rapidly convinced myself we have a problem throughout Linux.
next prev parent reply other threads:[~2019-02-18 19:05 UTC|newest] Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-02-18 5:28 [LKP] efad4e475c [ 40.308255] Oops: 0000 [#1] PREEMPT SMP PTI kernel test robot 2019-02-18 5:28 ` kernel test robot 2019-02-18 7:08 ` [LKP] " Michal Hocko 2019-02-18 7:08 ` Michal Hocko 2019-02-18 8:47 ` [LKP] " Rong Chen 2019-02-18 8:47 ` Rong Chen 2019-02-18 9:03 ` [LKP] " Michal Hocko 2019-02-18 9:03 ` Michal Hocko 2019-02-18 9:11 ` [LKP] " Rong Chen 2019-02-18 9:11 ` Rong Chen 2019-02-18 9:29 ` [LKP] " Michal Hocko 2019-02-18 9:29 ` Michal Hocko 2019-02-18 8:55 ` [LKP] " Michal Hocko 2019-02-18 8:55 ` Michal Hocko 2019-02-18 10:01 ` [LKP] " Rong Chen 2019-02-18 10:01 ` Rong Chen 2019-02-18 10:30 ` [LKP] " Michal Hocko 2019-02-18 10:30 ` Michal Hocko 2019-02-18 14:05 ` [LKP] " Mike Rapoport 2019-02-18 15:20 ` Michal Hocko 2019-02-18 15:20 ` Michal Hocko 2019-02-18 15:22 ` [LKP] " Michal Hocko 2019-02-18 15:22 ` Michal Hocko 2019-02-18 16:48 ` [LKP] " Mike Rapoport 2019-02-18 17:05 ` Michal Hocko 2019-02-18 17:05 ` Michal Hocko 2019-02-18 17:48 ` [LKP] " Mike Rapoport 2019-02-18 17:57 ` Matthew Wilcox 2019-02-18 17:57 ` Matthew Wilcox 2019-02-18 18:11 ` [LKP] " Michal Hocko 2019-02-18 18:11 ` Michal Hocko 2019-02-18 19:05 ` Matthew Wilcox [this message] 2019-02-18 19:05 ` Matthew Wilcox 2019-02-18 18:15 ` [RFC PATCH] mm, memory_hotplug: fix off-by-one in is_pageblock_removable Michal Hocko 2019-02-18 18:15 ` Michal Hocko 2019-02-18 18:31 ` Mike Rapoport 2019-02-20 8:33 ` Oscar Salvador 2019-02-20 8:33 ` Oscar Salvador 2019-02-20 12:57 ` Michal Hocko 2019-02-20 12:57 ` Michal Hocko 2019-02-21 3:18 ` [LKP] " Rong Chen 2019-02-21 3:18 ` Rong Chen 2019-02-21 7:25 ` [LKP] " Michal Hocko 2019-02-21 7:25 ` Michal Hocko
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=20190218190519.GV12668@bombadil.infradead.org \ --to=willy@infradead.org \ --cc=akpm@linux-foundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=lkp@01.org \ --cc=mhocko@kernel.org \ --cc=osalvador@suse.de \ --cc=pasha.tatashin@soleen.com \ --cc=rong.a.chen@intel.com \ --cc=rppt@linux.ibm.com \ /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: linkBe 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.