From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Todd Kjos" <tkjos@google.com>,
syzbot+a76129f18c89f3e2ddd4@syzkaller.appspotmail.com,
ak@linux.intel.com, "Johannes Weiner" <hannes@cmpxchg.org>,
jack@suse.cz, jrdr.linux@gmail.com,
LKML <linux-kernel@vger.kernel.org>,
linux-mm@kvack.org, mawilcox@microsoft.com,
mgorman@techsingularity.net, syzkaller-bugs@googlegroups.com,
"Arve Hjønnevåg" <arve@android.com>,
"Todd Kjos" <tkjos@android.com>,
"Martijn Coenen" <maco@android.com>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>
Subject: Re: possible deadlock in __do_page_fault
Date: Tue, 29 Jan 2019 19:44:02 +0900 [thread overview]
Message-ID: <17f26aab-4a25-dba9-7d39-40df80d1eadb@i-love.sakura.ne.jp> (raw)
In-Reply-To: <20190128164502.GA260885@google.com>
On 2019/01/29 1:45, Joel Fernandes wrote:
>> freed += range_size(range);
>> + mutex_unlock(&ashmem_mutex);
>> + f->f_op->fallocate(f,
>> + FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
>> + start, end - start);
>> + fput(f);
>> + if (atomic_dec_and_test(&ashmem_shrink_inflight))
>> + wake_up_all(&ashmem_shrink_wait);
>> + mutex_lock(&ashmem_mutex);
>
> Let us replace mutex_lock with mutex_trylock, as done before the loop? Here
> is there is an opportunity to not block other ashmem operations. Otherwise
> LGTM. Also, CC stable.
If shrinker succeeded to grab ashmem_mutex using mutex_trylock(), it is
guaranteed that that thread is not inside
mutex_lock(&ashmem_mutex);
kmalloc(GFP_KERNEL);
mutex_unlock(&ashmem_mutex);
block. Therefore, I think that it is safe to use mutex_lock() here.
Nonetheless, although syzbot did not find other dependency, I can update this
patch to use mutex_trylock() if you worry about not-yet-discovered dependency.
From fd850fecd248951ad1ad26b37ec5bf84afe41cbb Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Tue, 29 Jan 2019 10:56:47 +0900
Subject: [PATCH v2] staging: android: ashmem: Don't call fallocate() with ashmem_mutex held.
syzbot is hitting lockdep warnings [1][2][3]. This patch tries to fix
the warning by eliminating ashmem_shrink_scan() => {shmem|vfs}_fallocate()
sequence.
[1] https://syzkaller.appspot.com/bug?id=87c399f6fa6955006080b24142e2ce7680295ad4
[2] https://syzkaller.appspot.com/bug?id=7ebea492de7521048355fc84210220e1038a7908
[3] https://syzkaller.appspot.com/bug?id=e02419c12131c24e2a957ea050c2ab6dcbbc3270
Reported-by: syzbot <syzbot+a76129f18c89f3e2ddd4@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+148c2885d71194f18d28@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+4b8b031b89e6b96c4b2e@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Cc: stable@vger.kernel.org
---
drivers/staging/android/ashmem.c | 25 ++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 90a8a9f1ac7d..ade8438a827a 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -75,6 +75,9 @@ struct ashmem_range {
/* LRU list of unpinned pages, protected by ashmem_mutex */
static LIST_HEAD(ashmem_lru_list);
+static atomic_t ashmem_shrink_inflight = ATOMIC_INIT(0);
+static DECLARE_WAIT_QUEUE_HEAD(ashmem_shrink_wait);
+
/*
* long lru_count - The count of pages on our LRU list.
*
@@ -438,7 +441,6 @@ static int ashmem_mmap(struct file *file, struct vm_area_struct *vma)
static unsigned long
ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
{
- struct ashmem_range *range, *next;
unsigned long freed = 0;
/* We might recurse into filesystem code, so bail out if necessary */
@@ -448,21 +450,33 @@ ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
if (!mutex_trylock(&ashmem_mutex))
return -1;
- list_for_each_entry_safe(range, next, &ashmem_lru_list, lru) {
+ while (!list_empty(&ashmem_lru_list)) {
+ struct ashmem_range *range =
+ list_first_entry(&ashmem_lru_list, typeof(*range), lru);
loff_t start = range->pgstart * PAGE_SIZE;
loff_t end = (range->pgend + 1) * PAGE_SIZE;
+ struct file *f = range->asma->file;
- range->asma->file->f_op->fallocate(range->asma->file,
- FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
- start, end - start);
+ get_file(f);
+ atomic_inc(&ashmem_shrink_inflight);
range->purged = ASHMEM_WAS_PURGED;
lru_del(range);
freed += range_size(range);
+ mutex_unlock(&ashmem_mutex);
+ f->f_op->fallocate(f,
+ FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
+ start, end - start);
+ fput(f);
+ if (atomic_dec_and_test(&ashmem_shrink_inflight))
+ wake_up_all(&ashmem_shrink_wait);
+ if (!mutex_trylock(&ashmem_mutex))
+ goto out;
if (--sc->nr_to_scan <= 0)
break;
}
mutex_unlock(&ashmem_mutex);
+out:
return freed;
}
@@ -713,6 +727,7 @@ static int ashmem_pin_unpin(struct ashmem_area *asma, unsigned long cmd,
return -EFAULT;
mutex_lock(&ashmem_mutex);
+ wait_event(ashmem_shrink_wait, !atomic_read(&ashmem_shrink_inflight));
if (!asma->file)
goto out_unlock;
--
2.17.1
next prev parent reply other threads:[~2019-01-29 10:44 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-20 21:04 possible deadlock in __do_page_fault syzbot
2018-09-20 21:10 ` Andrew Morton
2018-09-20 21:12 ` Todd Kjos
2018-09-20 23:33 ` Joel Fernandes
2018-09-21 6:37 ` Dmitry Vyukov
2018-09-21 23:21 ` Andrew Morton
2019-01-22 10:02 ` Tetsuo Handa
2019-01-22 10:12 ` Dmitry Vyukov
2019-01-22 10:32 ` Tetsuo Handa
2019-01-22 13:52 ` Dmitry Vyukov
2019-01-22 13:54 ` Dmitry Vyukov
2019-01-22 14:08 ` syzbot
2019-01-22 15:32 ` Joel Fernandes
2019-01-23 2:01 ` Tetsuo Handa
2019-01-23 15:57 ` Joel Fernandes
2019-01-24 1:52 ` Tetsuo Handa
2019-01-24 13:46 ` Joel Fernandes
2019-01-25 16:02 ` Tetsuo Handa
2019-01-28 16:45 ` Joel Fernandes
2019-01-29 10:44 ` Tetsuo Handa [this message]
2019-01-26 1:57 ` Tetsuo Handa
2018-10-01 5:23 ` syzbot
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=17f26aab-4a25-dba9-7d39-40df80d1eadb@i-love.sakura.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=arve@android.com \
--cc=gregkh@linuxfoundation.org \
--cc=hannes@cmpxchg.org \
--cc=jack@suse.cz \
--cc=joel@joelfernandes.org \
--cc=jrdr.linux@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=maco@android.com \
--cc=mawilcox@microsoft.com \
--cc=mgorman@techsingularity.net \
--cc=syzbot+a76129f18c89f3e2ddd4@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=tkjos@android.com \
--cc=tkjos@google.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).