* [PATCH] swap: Check nrexceptional of swap cache before being freed
@ 2021-01-20 7:27 Huang Ying
2021-01-20 7:46 ` Michal Hocko
2021-01-21 18:44 ` Matthew Wilcox
0 siblings, 2 replies; 6+ messages in thread
From: Huang Ying @ 2021-01-20 7:27 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, Huang Ying, Minchan Kim, Joonsoo Kim,
Johannes Weiner, Vlastimil Babka, Mel Gorman, Michal Hocko,
Dan Williams, Christoph Hellwig
To catch the error in updating the swap cache shadow entries or their count.
Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Cc: Johannes Weiner <hannes@cmpxchg.org>,
Cc: Vlastimil Babka <vbabka@suse.cz>, Hugh Dickins <hughd@google.com>,
Cc: Mel Gorman <mgorman@techsingularity.net>,
Cc: Michal Hocko <mhocko@kernel.org>,
Cc: Dan Williams <dan.j.williams@intel.com>,
Cc: Christoph Hellwig <hch@lst.de>, Ilya Dryomov <idryomov@gmail.com>,
---
mm/swap_state.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/mm/swap_state.c b/mm/swap_state.c
index d0d417efeecc..240a4f97594a 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -703,7 +703,12 @@ int init_swap_address_space(unsigned int type, unsigned long nr_pages)
void exit_swap_address_space(unsigned int type)
{
- kvfree(swapper_spaces[type]);
+ int i;
+ struct address_space *spaces = swapper_spaces[type];
+
+ for (i = 0; i < nr_swapper_spaces[type]; i++)
+ VM_BUG_ON(spaces[i].nrexceptional);
+ kvfree(spaces);
nr_swapper_spaces[type] = 0;
swapper_spaces[type] = NULL;
}
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] swap: Check nrexceptional of swap cache before being freed
2021-01-20 7:27 [PATCH] swap: Check nrexceptional of swap cache before being freed Huang Ying
@ 2021-01-20 7:46 ` Michal Hocko
2021-01-20 7:54 ` Huang, Ying
2021-01-21 18:44 ` Matthew Wilcox
1 sibling, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2021-01-20 7:46 UTC (permalink / raw)
To: Huang Ying
Cc: Andrew Morton, linux-mm, linux-kernel, Minchan Kim, Joonsoo Kim,
Johannes Weiner, Vlastimil Babka, Mel Gorman, Dan Williams,
Christoph Hellwig
On Wed 20-01-21 15:27:11, Huang Ying wrote:
> To catch the error in updating the swap cache shadow entries or their count.
What is the error? Can it happens in the real life? Why do we need this
patch? Is crashing the kernel the right way to handle the situation?
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Cc: Minchan Kim <minchan@kernel.org>
> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
> Cc: Johannes Weiner <hannes@cmpxchg.org>,
> Cc: Vlastimil Babka <vbabka@suse.cz>, Hugh Dickins <hughd@google.com>,
> Cc: Mel Gorman <mgorman@techsingularity.net>,
> Cc: Michal Hocko <mhocko@kernel.org>,
> Cc: Dan Williams <dan.j.williams@intel.com>,
> Cc: Christoph Hellwig <hch@lst.de>, Ilya Dryomov <idryomov@gmail.com>,
> ---
> mm/swap_state.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index d0d417efeecc..240a4f97594a 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -703,7 +703,12 @@ int init_swap_address_space(unsigned int type, unsigned long nr_pages)
>
> void exit_swap_address_space(unsigned int type)
> {
> - kvfree(swapper_spaces[type]);
> + int i;
> + struct address_space *spaces = swapper_spaces[type];
> +
> + for (i = 0; i < nr_swapper_spaces[type]; i++)
> + VM_BUG_ON(spaces[i].nrexceptional);
> + kvfree(spaces);
> nr_swapper_spaces[type] = 0;
> swapper_spaces[type] = NULL;
> }
> --
> 2.29.2
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] swap: Check nrexceptional of swap cache before being freed
2021-01-20 7:46 ` Michal Hocko
@ 2021-01-20 7:54 ` Huang, Ying
2021-01-20 7:59 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: Huang, Ying @ 2021-01-20 7:54 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-mm, linux-kernel, Minchan Kim, Joonsoo Kim,
Johannes Weiner, Vlastimil Babka, Mel Gorman, Dan Williams,
Christoph Hellwig
Michal Hocko <mhocko@suse.com> writes:
> On Wed 20-01-21 15:27:11, Huang Ying wrote:
>> To catch the error in updating the swap cache shadow entries or their count.
>
> What is the error?
There's no error in the current code. But we will change the related
code in the future. So this checking will help us to prevent error in
the future. I will change the patch description to make it more clear.
> Can it happens in the real life? Why do we need this
> patch? Is crashing the kernel the right way to handle the situation?
Emm... The mistake to update swap shadow entries will hurt performance,
but will not trigger functionality bug. So it may be better to use
VM_WARN_ON_ONCE().
Best Regards,
Huang, Ying
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Cc: Minchan Kim <minchan@kernel.org>
>> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
>> Cc: Johannes Weiner <hannes@cmpxchg.org>,
>> Cc: Vlastimil Babka <vbabka@suse.cz>, Hugh Dickins <hughd@google.com>,
>> Cc: Mel Gorman <mgorman@techsingularity.net>,
>> Cc: Michal Hocko <mhocko@kernel.org>,
>> Cc: Dan Williams <dan.j.williams@intel.com>,
>> Cc: Christoph Hellwig <hch@lst.de>, Ilya Dryomov <idryomov@gmail.com>,
>> ---
>> mm/swap_state.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/swap_state.c b/mm/swap_state.c
>> index d0d417efeecc..240a4f97594a 100644
>> --- a/mm/swap_state.c
>> +++ b/mm/swap_state.c
>> @@ -703,7 +703,12 @@ int init_swap_address_space(unsigned int type, unsigned long nr_pages)
>>
>> void exit_swap_address_space(unsigned int type)
>> {
>> - kvfree(swapper_spaces[type]);
>> + int i;
>> + struct address_space *spaces = swapper_spaces[type];
>> +
>> + for (i = 0; i < nr_swapper_spaces[type]; i++)
>> + VM_BUG_ON(spaces[i].nrexceptional);
>> + kvfree(spaces);
>> nr_swapper_spaces[type] = 0;
>> swapper_spaces[type] = NULL;
>> }
>> --
>> 2.29.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] swap: Check nrexceptional of swap cache before being freed
2021-01-20 7:54 ` Huang, Ying
@ 2021-01-20 7:59 ` Michal Hocko
0 siblings, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2021-01-20 7:59 UTC (permalink / raw)
To: Huang, Ying
Cc: Andrew Morton, linux-mm, linux-kernel, Minchan Kim, Joonsoo Kim,
Johannes Weiner, Vlastimil Babka, Mel Gorman, Dan Williams,
Christoph Hellwig
On Wed 20-01-21 15:54:04, Huang, Ying wrote:
> Michal Hocko <mhocko@suse.com> writes:
>
> > On Wed 20-01-21 15:27:11, Huang Ying wrote:
> >> To catch the error in updating the swap cache shadow entries or their count.
> >
> > What is the error?
>
> There's no error in the current code. But we will change the related
> code in the future. So this checking will help us to prevent error in
> the future. I will change the patch description to make it more clear.
>
> > Can it happens in the real life? Why do we need this
> > patch? Is crashing the kernel the right way to handle the situation?
>
> Emm... The mistake to update swap shadow entries will hurt performance,
> but will not trigger functionality bug. So it may be better to use
> VM_WARN_ON_ONCE().
Yes a warning is much more appropriate approach. The question is whether
a test like this is really necessary. But I will leave that to others to
decide. It was really the bug on that hit my eyes.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] swap: Check nrexceptional of swap cache before being freed
2021-01-20 7:27 [PATCH] swap: Check nrexceptional of swap cache before being freed Huang Ying
2021-01-20 7:46 ` Michal Hocko
@ 2021-01-21 18:44 ` Matthew Wilcox
2021-01-22 0:02 ` Huang, Ying
1 sibling, 1 reply; 6+ messages in thread
From: Matthew Wilcox @ 2021-01-21 18:44 UTC (permalink / raw)
To: Huang Ying
Cc: Andrew Morton, linux-mm, linux-kernel, Minchan Kim, Joonsoo Kim,
Johannes Weiner, Vlastimil Babka, Mel Gorman, Michal Hocko,
Dan Williams, Christoph Hellwig
On Wed, Jan 20, 2021 at 03:27:11PM +0800, Huang Ying wrote:
> To catch the error in updating the swap cache shadow entries or their count.
I just resent a patch that removes nrexceptional tracking.
Can you use !mapping_empty() instead?
> void exit_swap_address_space(unsigned int type)
> {
> - kvfree(swapper_spaces[type]);
> + int i;
> + struct address_space *spaces = swapper_spaces[type];
> +
> + for (i = 0; i < nr_swapper_spaces[type]; i++)
> + VM_BUG_ON(spaces[i].nrexceptional);
> + kvfree(spaces);
> nr_swapper_spaces[type] = 0;
> swapper_spaces[type] = NULL;
> }
> --
> 2.29.2
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] swap: Check nrexceptional of swap cache before being freed
2021-01-21 18:44 ` Matthew Wilcox
@ 2021-01-22 0:02 ` Huang, Ying
0 siblings, 0 replies; 6+ messages in thread
From: Huang, Ying @ 2021-01-22 0:02 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Andrew Morton, linux-mm, linux-kernel, Minchan Kim, Joonsoo Kim,
Johannes Weiner, Vlastimil Babka, Mel Gorman, Michal Hocko,
Dan Williams, Christoph Hellwig
Matthew Wilcox <willy@infradead.org> writes:
> On Wed, Jan 20, 2021 at 03:27:11PM +0800, Huang Ying wrote:
>> To catch the error in updating the swap cache shadow entries or their count.
>
> I just resent a patch that removes nrexceptional tracking.
>
> Can you use !mapping_empty() instead?
Sure. Will use that in the next version.
Best Regards,
Huang, Ying
>> void exit_swap_address_space(unsigned int type)
>> {
>> - kvfree(swapper_spaces[type]);
>> + int i;
>> + struct address_space *spaces = swapper_spaces[type];
>> +
>> + for (i = 0; i < nr_swapper_spaces[type]; i++)
>> + VM_BUG_ON(spaces[i].nrexceptional);
>> + kvfree(spaces);
>> nr_swapper_spaces[type] = 0;
>> swapper_spaces[type] = NULL;
>> }
>> --
>> 2.29.2
>>
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-01-22 0:03 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-20 7:27 [PATCH] swap: Check nrexceptional of swap cache before being freed Huang Ying
2021-01-20 7:46 ` Michal Hocko
2021-01-20 7:54 ` Huang, Ying
2021-01-20 7:59 ` Michal Hocko
2021-01-21 18:44 ` Matthew Wilcox
2021-01-22 0:02 ` Huang, Ying
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).