* [PATCH -V2] mm, swap: Remove unnecessary smp_rmb() in swap_type_to_swap_info()
@ 2021-05-20 7:33 Huang Ying
2021-06-02 16:03 ` Will Deacon
0 siblings, 1 reply; 4+ messages in thread
From: Huang Ying @ 2021-05-20 7:33 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-mm, linux-kernel, Huang Ying, Daniel Jordan, Dan Carpenter,
Andrea Parri, Peter Zijlstra, Andi Kleen, Dave Hansen,
Omar Sandoval, Paul McKenney, Tejun Heo, Will Deacon, Miaohe Lin
Before commit c10d38cc8d3e ("mm, swap: bounds check swap_info array
accesses to avoid NULL derefs"), the typical code to reference the
swap_info[] is as follows,
type = swp_type(swp_entry);
if (type >= nr_swapfiles)
/* handle invalid swp_entry */;
p = swap_info[type];
/* access fields of *p. OOPS! p may be NULL! */
Because the ordering isn't guaranteed, it's possible that
swap_info[type] is read before "nr_swapfiles". And that may result
in NULL pointer dereference.
So after commit c10d38cc8d3e, the code becomes,
struct swap_info_struct *swap_type_to_swap_info(int type)
{
if (type >= READ_ONCE(nr_swapfiles))
return NULL;
smp_rmb();
return READ_ONCE(swap_info[type]);
}
/* users */
type = swp_type(swp_entry);
p = swap_type_to_swap_info(type);
if (!p)
/* handle invalid swp_entry */;
/* dereference p */
Where the value of swap_info[type] (that is, "p") is checked to be
non-zero before being dereferenced. So, the NULL deferencing
becomes impossible even if "nr_swapfiles" is read after
swap_info[type]. Therefore, the "smp_rmb()" becomes unnecessary.
And, we don't even need to read "nr_swapfiles" here. Because the
non-zero checking for "p" is sufficient. We just need to make sure we
will not access out of the boundary of the array. With the change,
nr_swapfiles will only be accessed with swap_lock held, except in
swapcache_free_entries(). Where the absolute correctness of the value
isn't needed, as described in the comments.
We still need to guarantee swap_info[type] is read before being
dereferenced. That can be satisfied via the data dependency ordering
enforced by READ_ONCE(swap_info[type]). This needs to be paired with
proper write barriers. So smp_store_release() is used in
alloc_swap_info() to guarantee the fields of *swap_info[type] is
initialized before swap_info[type] itself being written. Note that
the fields of *swap_info[type] is initialized to be 0 via kvzalloc()
firstly. The assignment and deferencing of swap_info[type] is like
rcu_assign_pointer() and rcu_dereference().
Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
Cc: Daniel Jordan <daniel.m.jordan@oracle.com>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Andrea Parri <andrea.parri@amarulasolutions.com>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Paul McKenney <paulmck@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>
v2:
- Revise the patch description and comments per Peter's comments.
---
mm/swapfile.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/mm/swapfile.c b/mm/swapfile.c
index 2aad85751991..65dd979a0f94 100644
--- a/mm/swapfile.c
+++ b/mm/swapfile.c
@@ -100,11 +100,10 @@ atomic_t nr_rotate_swap = ATOMIC_INIT(0);
static struct swap_info_struct *swap_type_to_swap_info(int type)
{
- if (type >= READ_ONCE(nr_swapfiles))
+ if (type >= MAX_SWAPFILES)
return NULL;
- smp_rmb(); /* Pairs with smp_wmb in alloc_swap_info. */
- return READ_ONCE(swap_info[type]);
+ return READ_ONCE(swap_info[type]); /* rcu_dereference() */
}
static inline unsigned char swap_count(unsigned char ent)
@@ -2884,14 +2883,12 @@ static struct swap_info_struct *alloc_swap_info(void)
}
if (type >= nr_swapfiles) {
p->type = type;
- WRITE_ONCE(swap_info[type], p);
/*
- * Write swap_info[type] before nr_swapfiles, in case a
- * racing procfs swap_start() or swap_next() is reading them.
- * (We never shrink nr_swapfiles, we never free this entry.)
+ * Publish the swap_info_struct after initializing it.
+ * Note that kvzalloc() above zeroes all its fields.
*/
- smp_wmb();
- WRITE_ONCE(nr_swapfiles, nr_swapfiles + 1);
+ smp_store_release(&swap_info[type], p); /* rcu_assign_pointer() */
+ nr_swapfiles++;
} else {
defer = p;
p = swap_info[type];
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH -V2] mm, swap: Remove unnecessary smp_rmb() in swap_type_to_swap_info()
2021-05-20 7:33 [PATCH -V2] mm, swap: Remove unnecessary smp_rmb() in swap_type_to_swap_info() Huang Ying
@ 2021-06-02 16:03 ` Will Deacon
2021-06-07 1:49 ` Huang, Ying
0 siblings, 1 reply; 4+ messages in thread
From: Will Deacon @ 2021-06-02 16:03 UTC (permalink / raw)
To: Huang Ying
Cc: Andrew Morton, linux-mm, linux-kernel, Daniel Jordan,
Dan Carpenter, Andrea Parri, Peter Zijlstra, Andi Kleen,
Dave Hansen, Omar Sandoval, Paul McKenney, Tejun Heo,
Will Deacon, Miaohe Lin
On Thu, May 20, 2021 at 03:33:01PM +0800, Huang Ying wrote:
> Before commit c10d38cc8d3e ("mm, swap: bounds check swap_info array
> accesses to avoid NULL derefs"), the typical code to reference the
> swap_info[] is as follows,
>
> type = swp_type(swp_entry);
> if (type >= nr_swapfiles)
> /* handle invalid swp_entry */;
> p = swap_info[type];
> /* access fields of *p. OOPS! p may be NULL! */
>
> Because the ordering isn't guaranteed, it's possible that
> swap_info[type] is read before "nr_swapfiles". And that may result
> in NULL pointer dereference.
>
> So after commit c10d38cc8d3e, the code becomes,
>
> struct swap_info_struct *swap_type_to_swap_info(int type)
> {
> if (type >= READ_ONCE(nr_swapfiles))
> return NULL;
> smp_rmb();
> return READ_ONCE(swap_info[type]);
> }
>
> /* users */
> type = swp_type(swp_entry);
> p = swap_type_to_swap_info(type);
> if (!p)
> /* handle invalid swp_entry */;
> /* dereference p */
>
> Where the value of swap_info[type] (that is, "p") is checked to be
> non-zero before being dereferenced. So, the NULL deferencing
> becomes impossible even if "nr_swapfiles" is read after
> swap_info[type]. Therefore, the "smp_rmb()" becomes unnecessary.
>
> And, we don't even need to read "nr_swapfiles" here. Because the
> non-zero checking for "p" is sufficient. We just need to make sure we
> will not access out of the boundary of the array. With the change,
> nr_swapfiles will only be accessed with swap_lock held, except in
> swapcache_free_entries(). Where the absolute correctness of the value
> isn't needed, as described in the comments.
>
> We still need to guarantee swap_info[type] is read before being
> dereferenced. That can be satisfied via the data dependency ordering
> enforced by READ_ONCE(swap_info[type]). This needs to be paired with
> proper write barriers. So smp_store_release() is used in
> alloc_swap_info() to guarantee the fields of *swap_info[type] is
> initialized before swap_info[type] itself being written. Note that
> the fields of *swap_info[type] is initialized to be 0 via kvzalloc()
> firstly. The assignment and deferencing of swap_info[type] is like
> rcu_assign_pointer() and rcu_dereference().
>
> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> Cc: Daniel Jordan <daniel.m.jordan@oracle.com>
> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Andrea Parri <andrea.parri@amarulasolutions.com>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Paul McKenney <paulmck@kernel.org>
> Cc: Tejun Heo <tj@kernel.org>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Miaohe Lin <linmiaohe@huawei.com>
>
> v2:
>
> - Revise the patch description and comments per Peter's comments.
>
> ---
> mm/swapfile.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/mm/swapfile.c b/mm/swapfile.c
> index 2aad85751991..65dd979a0f94 100644
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -100,11 +100,10 @@ atomic_t nr_rotate_swap = ATOMIC_INIT(0);
>
> static struct swap_info_struct *swap_type_to_swap_info(int type)
> {
> - if (type >= READ_ONCE(nr_swapfiles))
> + if (type >= MAX_SWAPFILES)
> return NULL;
>
> - smp_rmb(); /* Pairs with smp_wmb in alloc_swap_info. */
> - return READ_ONCE(swap_info[type]);
> + return READ_ONCE(swap_info[type]); /* rcu_dereference() */
> }
>
> static inline unsigned char swap_count(unsigned char ent)
> @@ -2884,14 +2883,12 @@ static struct swap_info_struct *alloc_swap_info(void)
> }
> if (type >= nr_swapfiles) {
> p->type = type;
> - WRITE_ONCE(swap_info[type], p);
> /*
> - * Write swap_info[type] before nr_swapfiles, in case a
> - * racing procfs swap_start() or swap_next() is reading them.
> - * (We never shrink nr_swapfiles, we never free this entry.)
> + * Publish the swap_info_struct after initializing it.
> + * Note that kvzalloc() above zeroes all its fields.
> */
> - smp_wmb();
> - WRITE_ONCE(nr_swapfiles, nr_swapfiles + 1);
> + smp_store_release(&swap_info[type], p); /* rcu_assign_pointer() */
> + nr_swapfiles++;
Although I like this change, I comment you are removing refers to some
dodgy-looking code. For example, swap_start() has this loop:
for (type = 0; (si = swap_type_to_swap_info(type)); type++) {
if (!(si->flags & SWP_USED) || !si->swap_map)
continue;
so won't this just end up dereferencing NULL if nr_swapfiles < MAX_SWAPFILES?
I think you need to check all callers of swap_type_to_swap_info() are
either validating the 'type' they pass in or check the returned pointer.
Will
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH -V2] mm, swap: Remove unnecessary smp_rmb() in swap_type_to_swap_info()
2021-06-02 16:03 ` Will Deacon
@ 2021-06-07 1:49 ` Huang, Ying
2021-06-07 10:38 ` Will Deacon
0 siblings, 1 reply; 4+ messages in thread
From: Huang, Ying @ 2021-06-07 1:49 UTC (permalink / raw)
To: Will Deacon
Cc: Andrew Morton, linux-mm, linux-kernel, Daniel Jordan,
Dan Carpenter, Andrea Parri, Peter Zijlstra, Andi Kleen,
Dave Hansen, Omar Sandoval, Paul McKenney, Tejun Heo,
Will Deacon, Miaohe Lin
Hi, Will,
Will Deacon <will@kernel.org> writes:
> On Thu, May 20, 2021 at 03:33:01PM +0800, Huang Ying wrote:
>> Before commit c10d38cc8d3e ("mm, swap: bounds check swap_info array
>> accesses to avoid NULL derefs"), the typical code to reference the
>> swap_info[] is as follows,
>>
>> type = swp_type(swp_entry);
>> if (type >= nr_swapfiles)
>> /* handle invalid swp_entry */;
>> p = swap_info[type];
>> /* access fields of *p. OOPS! p may be NULL! */
>>
>> Because the ordering isn't guaranteed, it's possible that
>> swap_info[type] is read before "nr_swapfiles". And that may result
>> in NULL pointer dereference.
>>
>> So after commit c10d38cc8d3e, the code becomes,
>>
>> struct swap_info_struct *swap_type_to_swap_info(int type)
>> {
>> if (type >= READ_ONCE(nr_swapfiles))
>> return NULL;
>> smp_rmb();
>> return READ_ONCE(swap_info[type]);
>> }
>>
>> /* users */
>> type = swp_type(swp_entry);
>> p = swap_type_to_swap_info(type);
>> if (!p)
>> /* handle invalid swp_entry */;
>> /* dereference p */
>>
>> Where the value of swap_info[type] (that is, "p") is checked to be
>> non-zero before being dereferenced. So, the NULL deferencing
>> becomes impossible even if "nr_swapfiles" is read after
>> swap_info[type]. Therefore, the "smp_rmb()" becomes unnecessary.
>>
>> And, we don't even need to read "nr_swapfiles" here. Because the
>> non-zero checking for "p" is sufficient. We just need to make sure we
>> will not access out of the boundary of the array. With the change,
>> nr_swapfiles will only be accessed with swap_lock held, except in
>> swapcache_free_entries(). Where the absolute correctness of the value
>> isn't needed, as described in the comments.
>>
>> We still need to guarantee swap_info[type] is read before being
>> dereferenced. That can be satisfied via the data dependency ordering
>> enforced by READ_ONCE(swap_info[type]). This needs to be paired with
>> proper write barriers. So smp_store_release() is used in
>> alloc_swap_info() to guarantee the fields of *swap_info[type] is
>> initialized before swap_info[type] itself being written. Note that
>> the fields of *swap_info[type] is initialized to be 0 via kvzalloc()
>> firstly. The assignment and deferencing of swap_info[type] is like
>> rcu_assign_pointer() and rcu_dereference().
>>
>> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
>> Cc: Daniel Jordan <daniel.m.jordan@oracle.com>
>> Cc: Dan Carpenter <dan.carpenter@oracle.com>
>> Cc: Andrea Parri <andrea.parri@amarulasolutions.com>
>> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Cc: Andi Kleen <ak@linux.intel.com>
>> Cc: Dave Hansen <dave.hansen@linux.intel.com>
>> Cc: Omar Sandoval <osandov@fb.com>
>> Cc: Paul McKenney <paulmck@kernel.org>
>> Cc: Tejun Heo <tj@kernel.org>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Miaohe Lin <linmiaohe@huawei.com>
>>
>> v2:
>>
>> - Revise the patch description and comments per Peter's comments.
>>
>> ---
>> mm/swapfile.c | 15 ++++++---------
>> 1 file changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index 2aad85751991..65dd979a0f94 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -100,11 +100,10 @@ atomic_t nr_rotate_swap = ATOMIC_INIT(0);
>>
>> static struct swap_info_struct *swap_type_to_swap_info(int type)
>> {
>> - if (type >= READ_ONCE(nr_swapfiles))
>> + if (type >= MAX_SWAPFILES)
>> return NULL;
>>
>> - smp_rmb(); /* Pairs with smp_wmb in alloc_swap_info. */
>> - return READ_ONCE(swap_info[type]);
>> + return READ_ONCE(swap_info[type]); /* rcu_dereference() */
>> }
>>
>> static inline unsigned char swap_count(unsigned char ent)
>> @@ -2884,14 +2883,12 @@ static struct swap_info_struct *alloc_swap_info(void)
>> }
>> if (type >= nr_swapfiles) {
>> p->type = type;
>> - WRITE_ONCE(swap_info[type], p);
>> /*
>> - * Write swap_info[type] before nr_swapfiles, in case a
>> - * racing procfs swap_start() or swap_next() is reading them.
>> - * (We never shrink nr_swapfiles, we never free this entry.)
>> + * Publish the swap_info_struct after initializing it.
>> + * Note that kvzalloc() above zeroes all its fields.
>> */
>> - smp_wmb();
>> - WRITE_ONCE(nr_swapfiles, nr_swapfiles + 1);
>> + smp_store_release(&swap_info[type], p); /* rcu_assign_pointer() */
>> + nr_swapfiles++;
>
> Although I like this change, I comment you are removing refers to some
> dodgy-looking code. For example, swap_start() has this loop:
>
> for (type = 0; (si = swap_type_to_swap_info(type)); type++) {
> if (!(si->flags & SWP_USED) || !si->swap_map)
> continue;
>
> so won't this just end up dereferencing NULL if nr_swapfiles < MAX_SWAPFILES?
for (type = 0; (si = swap_type_to_swap_info(type)); type++) {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Because this is the second sub-statement inside "for ()", I think that "si"
will be checked to be non-NULL before executing the statements inside
"{}" follows "for ()"?
> I think you need to check all callers of swap_type_to_swap_info() are
> either validating the 'type' they pass in or check the returned pointer.
Yes. I have checked all callers of swap_type_to_swap_info(). One
suspecting caller is swap_cluster_readahead(). But after the following
patch in mmotm tree,
mm/swapfile: use percpu_ref to serialize against concurrent swapoff
get/put_swap_device() will enclose the swap_cluster_readahead() to
check the swap entry beforehand.
Best Regards,
Huang, Ying
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH -V2] mm, swap: Remove unnecessary smp_rmb() in swap_type_to_swap_info()
2021-06-07 1:49 ` Huang, Ying
@ 2021-06-07 10:38 ` Will Deacon
0 siblings, 0 replies; 4+ messages in thread
From: Will Deacon @ 2021-06-07 10:38 UTC (permalink / raw)
To: Huang, Ying
Cc: Andrew Morton, linux-mm, linux-kernel, Daniel Jordan,
Dan Carpenter, Andrea Parri, Peter Zijlstra, Andi Kleen,
Dave Hansen, Omar Sandoval, Paul McKenney, Tejun Heo,
Will Deacon, Miaohe Lin
On Mon, Jun 07, 2021 at 09:49:35AM +0800, Huang, Ying wrote:
> Hi, Will,
>
> Will Deacon <will@kernel.org> writes:
>
> > On Thu, May 20, 2021 at 03:33:01PM +0800, Huang Ying wrote:
> >> Before commit c10d38cc8d3e ("mm, swap: bounds check swap_info array
> >> accesses to avoid NULL derefs"), the typical code to reference the
> >> swap_info[] is as follows,
> >>
> >> type = swp_type(swp_entry);
> >> if (type >= nr_swapfiles)
> >> /* handle invalid swp_entry */;
> >> p = swap_info[type];
> >> /* access fields of *p. OOPS! p may be NULL! */
> >>
> >> Because the ordering isn't guaranteed, it's possible that
> >> swap_info[type] is read before "nr_swapfiles". And that may result
> >> in NULL pointer dereference.
> >>
> >> So after commit c10d38cc8d3e, the code becomes,
> >>
> >> struct swap_info_struct *swap_type_to_swap_info(int type)
> >> {
> >> if (type >= READ_ONCE(nr_swapfiles))
> >> return NULL;
> >> smp_rmb();
> >> return READ_ONCE(swap_info[type]);
> >> }
> >>
> >> /* users */
> >> type = swp_type(swp_entry);
> >> p = swap_type_to_swap_info(type);
> >> if (!p)
> >> /* handle invalid swp_entry */;
> >> /* dereference p */
> >>
> >> Where the value of swap_info[type] (that is, "p") is checked to be
> >> non-zero before being dereferenced. So, the NULL deferencing
> >> becomes impossible even if "nr_swapfiles" is read after
> >> swap_info[type]. Therefore, the "smp_rmb()" becomes unnecessary.
> >>
> >> And, we don't even need to read "nr_swapfiles" here. Because the
> >> non-zero checking for "p" is sufficient. We just need to make sure we
> >> will not access out of the boundary of the array. With the change,
> >> nr_swapfiles will only be accessed with swap_lock held, except in
> >> swapcache_free_entries(). Where the absolute correctness of the value
> >> isn't needed, as described in the comments.
> >>
> >> We still need to guarantee swap_info[type] is read before being
> >> dereferenced. That can be satisfied via the data dependency ordering
> >> enforced by READ_ONCE(swap_info[type]). This needs to be paired with
> >> proper write barriers. So smp_store_release() is used in
> >> alloc_swap_info() to guarantee the fields of *swap_info[type] is
> >> initialized before swap_info[type] itself being written. Note that
> >> the fields of *swap_info[type] is initialized to be 0 via kvzalloc()
> >> firstly. The assignment and deferencing of swap_info[type] is like
> >> rcu_assign_pointer() and rcu_dereference().
> >>
> >> Signed-off-by: "Huang, Ying" <ying.huang@intel.com>
> >> Cc: Daniel Jordan <daniel.m.jordan@oracle.com>
> >> Cc: Dan Carpenter <dan.carpenter@oracle.com>
> >> Cc: Andrea Parri <andrea.parri@amarulasolutions.com>
> >> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> >> Cc: Andi Kleen <ak@linux.intel.com>
> >> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> >> Cc: Omar Sandoval <osandov@fb.com>
> >> Cc: Paul McKenney <paulmck@kernel.org>
> >> Cc: Tejun Heo <tj@kernel.org>
> >> Cc: Will Deacon <will.deacon@arm.com>
> >> Cc: Miaohe Lin <linmiaohe@huawei.com>
> >>
> >> v2:
> >>
> >> - Revise the patch description and comments per Peter's comments.
> >>
> >> ---
> >> mm/swapfile.c | 15 ++++++---------
> >> 1 file changed, 6 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/mm/swapfile.c b/mm/swapfile.c
> >> index 2aad85751991..65dd979a0f94 100644
> >> --- a/mm/swapfile.c
> >> +++ b/mm/swapfile.c
> >> @@ -100,11 +100,10 @@ atomic_t nr_rotate_swap = ATOMIC_INIT(0);
> >>
> >> static struct swap_info_struct *swap_type_to_swap_info(int type)
> >> {
> >> - if (type >= READ_ONCE(nr_swapfiles))
> >> + if (type >= MAX_SWAPFILES)
> >> return NULL;
> >>
> >> - smp_rmb(); /* Pairs with smp_wmb in alloc_swap_info. */
> >> - return READ_ONCE(swap_info[type]);
> >> + return READ_ONCE(swap_info[type]); /* rcu_dereference() */
> >> }
> >>
> >> static inline unsigned char swap_count(unsigned char ent)
> >> @@ -2884,14 +2883,12 @@ static struct swap_info_struct *alloc_swap_info(void)
> >> }
> >> if (type >= nr_swapfiles) {
> >> p->type = type;
> >> - WRITE_ONCE(swap_info[type], p);
> >> /*
> >> - * Write swap_info[type] before nr_swapfiles, in case a
> >> - * racing procfs swap_start() or swap_next() is reading them.
> >> - * (We never shrink nr_swapfiles, we never free this entry.)
> >> + * Publish the swap_info_struct after initializing it.
> >> + * Note that kvzalloc() above zeroes all its fields.
> >> */
> >> - smp_wmb();
> >> - WRITE_ONCE(nr_swapfiles, nr_swapfiles + 1);
> >> + smp_store_release(&swap_info[type], p); /* rcu_assign_pointer() */
> >> + nr_swapfiles++;
> >
> > Although I like this change, I comment you are removing refers to some
> > dodgy-looking code. For example, swap_start() has this loop:
> >
> > for (type = 0; (si = swap_type_to_swap_info(type)); type++) {
> > if (!(si->flags & SWP_USED) || !si->swap_map)
> > continue;
> >
> > so won't this just end up dereferencing NULL if nr_swapfiles < MAX_SWAPFILES?
>
> for (type = 0; (si = swap_type_to_swap_info(type)); type++) {
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Because this is the second sub-statement inside "for ()", I think that "si"
> will be checked to be non-NULL before executing the statements inside
> "{}" follows "for ()"?
Sorry, yes, you're right. I misread the loop condition.
Will
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-07 10:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-20 7:33 [PATCH -V2] mm, swap: Remove unnecessary smp_rmb() in swap_type_to_swap_info() Huang Ying
2021-06-02 16:03 ` Will Deacon
2021-06-07 1:49 ` Huang, Ying
2021-06-07 10:38 ` Will Deacon
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).