public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Michal Hocko <mhocko@kernel.org>,
	Jia-Ju Bai <baijiaju1990@163.com>,
	torbjorn.lindh@gopta.se, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [BUG] fs/super: a possible sleep-in-atomic bug in put_super
Date: Sun, 8 Oct 2017 01:56:08 +0100	[thread overview]
Message-ID: <20171008005602.GT21978@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20171007211444.GS21978@ZenIV.linux.org.uk>

On Sat, Oct 07, 2017 at 10:14:44PM +0100, Al Viro wrote:

> 	1) coallocate struct list_lru and array of struct list_lru_node
> hanging off it.  Turn all existing variables and struct members of that
> type into pointers.  init would allocate and return a pointer, destroy
> would free (and leave it for callers to clear their pointers, of course).

Better yet, keep list_lru containing just the pointer to list_lru_node
array.  And put that array into the tail of struct list_lru_nodes.  That
way normal accesses are kept exactly as-is and we don't need to update
the users of that thing at all.

> 	4) have lru_list_destroy() check (under list_lru_mutex) whether it's
> being asked to kill the currently resized one.  If it is, do
> 	victim->list.prev->next = victim->list.next;
> 	victim->list.next->prev = victim->list.prev;
> 	victim->list.prev = NULL;

Doesn't work, unfortunately - it needs to stay on the list and be marked
in some other way.

> and bugger off, otherwise act as now.  Turn the loop in
> memcg_update_all_list_lrus() into
> 	mutex_lock(&list_lrus_mutex);
> 	lru = list_lrus.next;
> 	while (lru != &list_lrus) {
> 		currently_resized = list_entry(lru, struct list_lru, list);
> 		mutex_unlock(&list_lrus_mutex);
> 		ret = memcg_update_list_lru(lru, old_size, new_size);
> 		mutex_lock(&list_lrus_mutex);
> 		if (unlikely(!lru->prev)) {
> 			lru = lru->next;

... because this might very well be pointing to already freed object.

> 			free currently_resized as list_lru_destroy() would have
> 			continue;

What's more, we need to be careful about resize vs. drain.  Right now it's
on list_lrus_mutex, but if we drop that around actual resize of an individual
list_lru, we'll need something else.  Would there be any problem if we
took memcg_cache_ids_sem shared in memcg_offline_kmem()?

The first problem is not fatal - we can e.g. use the sign of the field used
to store the number of ->memcg_lrus elements (i.e. stashed value of
memcg_nr_cache_ids at allocation or last resize) to indicate that actual
freeing is left for resizer...

  reply	other threads:[~2017-10-08  0:56 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-06  8:59 [BUG] fs/super: a possible sleep-in-atomic bug in put_super Jia-Ju Bai
2017-10-06  9:06 ` Michal Hocko
2017-10-07 11:56   ` Vladimir Davydov
2017-10-07 17:06     ` Al Viro
2017-10-07 21:14       ` Al Viro
2017-10-08  0:56         ` Al Viro [this message]
2017-10-08  2:03           ` Al Viro
2017-10-08 15:47             ` Vladimir Davydov
2017-10-08 21:13               ` Al Viro
2017-10-09  8:43                 ` Vladimir Davydov
2017-10-06 12:19 ` Al Viro

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=20171008005602.GT21978@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=baijiaju1990@163.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=torbjorn.lindh@gopta.se \
    --cc=vdavydov.dev@gmail.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