Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1598098 > unrolled thread
| Started by | Alexey Khoroshilov <khoroshilov@ispras.ru> |
|---|---|
| First post | 2017-03-10 22:30 +0100 |
| Last post | 2017-03-10 23:10 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
z3fold: suspicious return with spinlock held Alexey Khoroshilov <khoroshilov@ispras.ru> - 2017-03-10 22:30 +0100
Re: z3fold: suspicious return with spinlock held Matthew Wilcox <willy@infradead.org> - 2017-03-10 22:40 +0100
Re: z3fold: suspicious return with spinlock held Alexey Khoroshilov <khoroshilov@ispras.ru> - 2017-03-10 23:00 +0100
Re: z3fold: suspicious return with spinlock held vitalywool@gmail.com - 2017-03-10 23:10 +0100
| From | Alexey Khoroshilov <khoroshilov@ispras.ru> |
|---|---|
| Date | 2017-03-10 22:30 +0100 |
| Subject | z3fold: suspicious return with spinlock held |
| Message-ID | <tjAgO-1eI-9@gated-at.bofh.it> |
Hello!
z3fold_reclaim_page() contains the only return that may
leave the function with pool->lock spinlock held.
669 spin_lock(&pool->lock);
670 if (kref_put(&zhdr->refcount, release_z3fold_page)) {
671 atomic64_dec(&pool->pages_nr);
672 return 0;
673 }
May be we need spin_unlock(&pool->lock); just before return?
Found by Linux Driver Verification project (linuxtesting.org).
--
Thank you,
Alexey Khoroshilov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
[toc] | [next] | [standalone]
| From | Matthew Wilcox <willy@infradead.org> |
|---|---|
| Date | 2017-03-10 22:40 +0100 |
| Message-ID | <tjAqt-1kb-9@gated-at.bofh.it> |
| In reply to | #1598098 |
On Sat, Mar 11, 2017 at 12:22:12AM +0300, Alexey Khoroshilov wrote:
> Hello!
>
> z3fold_reclaim_page() contains the only return that may
> leave the function with pool->lock spinlock held.
>
> 669 spin_lock(&pool->lock);
> 670 if (kref_put(&zhdr->refcount, release_z3fold_page)) {
> 671 atomic64_dec(&pool->pages_nr);
> 672 return 0;
> 673 }
>
> May be we need spin_unlock(&pool->lock); just before return?
I would tend to agree. sparse warns about this, and also about two
other locking problems ... which I'm not sure are really problems so
much as missing annotations?
mm/z3fold.c:467:35: warning: context imbalance in 'z3fold_alloc' - unexpected unlock
mm/z3fold.c:519:26: warning: context imbalance in 'z3fold_free' - different lock contexts for basic block
mm/z3fold.c:581:12: warning: context imbalance in 'z3fold_reclaim_page' - different lock contexts for basic block
[toc] | [prev] | [next] | [standalone]
| From | Alexey Khoroshilov <khoroshilov@ispras.ru> |
|---|---|
| Date | 2017-03-10 23:00 +0100 |
| Message-ID | <tjAJP-1r3-5@gated-at.bofh.it> |
| In reply to | #1598117 |
On 11.03.2017 00:34, Matthew Wilcox wrote:
> On Sat, Mar 11, 2017 at 12:22:12AM +0300, Alexey Khoroshilov wrote:
>> Hello!
>>
>> z3fold_reclaim_page() contains the only return that may
>> leave the function with pool->lock spinlock held.
>>
>> 669 spin_lock(&pool->lock);
>> 670 if (kref_put(&zhdr->refcount, release_z3fold_page)) {
>> 671 atomic64_dec(&pool->pages_nr);
>> 672 return 0;
>> 673 }
>>
>> May be we need spin_unlock(&pool->lock); just before return?
>
> I would tend to agree. sparse warns about this, and also about two
> other locking problems ... which I'm not sure are really problems so
> much as missing annotations?
>
> mm/z3fold.c:467:35: warning: context imbalance in 'z3fold_alloc' - unexpected unlock
> mm/z3fold.c:519:26: warning: context imbalance in 'z3fold_free' - different lock contexts for basic block
> mm/z3fold.c:581:12: warning: context imbalance in 'z3fold_reclaim_page' - different lock contexts for basic block
>
I also do not see problems in z3fold_alloc() and z3fold_free().
But I am unaware of sparse annotations that can help here.
--
Alexey
[toc] | [prev] | [next] | [standalone]
| From | vitalywool@gmail.com |
|---|---|
| Date | 2017-03-10 23:10 +0100 |
| Message-ID | <tjATw-1Jy-15@gated-at.bofh.it> |
| In reply to | #1598124 |
Hi Alexey,
> 10 mars 2017 kl. 22:54 skrev Alexey Khoroshilov <khoroshilov@ispras.ru>:
>
>> On 11.03.2017 00:34, Matthew Wilcox wrote:
>>> On Sat, Mar 11, 2017 at 12:22:12AM +0300, Alexey Khoroshilov wrote:
>>> Hello!
>>>
>>> z3fold_reclaim_page() contains the only return that may
>>> leave the function with pool->lock spinlock held.
>>>
>>> 669 spin_lock(&pool->lock);
>>> 670 if (kref_put(&zhdr->refcount, release_z3fold_page)) {
>>> 671 atomic64_dec(&pool->pages_nr);
>>> 672 return 0;
>>> 673 }
>>>
>>> May be we need spin_unlock(&pool->lock); just before return?
Looks so, thanks for the pointer. I'm currently commuting but will check it thoroughly tomorrow for sure.
~vitaly
>>
>> I would tend to agree. sparse warns about this, and also about two
>> other locking problems ... which I'm not sure are really problems so
>> much as missing annotations?
>>
>> mm/z3fold.c:467:35: warning: context imbalance in 'z3fold_alloc' - unexpected unlock
>> mm/z3fold.c:519:26: warning: context imbalance in 'z3fold_free' - different lock contexts for basic block
>> mm/z3fold.c:581:12: warning: context imbalance in 'z3fold_reclaim_page' - different lock contexts for basic block
>>
>
> I also do not see problems in z3fold_alloc() and z3fold_free().
> But I am unaware of sparse annotations that can help here.
>
> --
> Alexey
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web