linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Hugh Dickins <hughd@google.com>, Sasha Levin <sasha.levin@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Konstantin Khlebnikov <koct9i@gmail.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michel Lespinasse <walken@google.com>,
	Lukas Czerner <lczerner@redhat.com>,
	Dave Jones <davej@redhat.com>,
	linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, Michal Hocko <mhocko@suse.cz>
Subject: Re: [PATCH 0/2] shmem: fix faulting into a hole while it's punched, take 3
Date: Tue, 22 Jul 2014 14:09:56 +0200	[thread overview]
Message-ID: <53CE5494.3030708@suse.cz> (raw)
In-Reply-To: <53CE37A6.2060000@suse.cz>

On 07/22/2014 12:06 PM, Vlastimil Babka wrote:
> So if this is true, the change to TASK_UNINTERRUPTIBLE will avoid the
> problem, but it would be nicer to keep the KILLABLE state.
> I think it could be done by testing if the wait queue still exists and
> is the same, before attempting finish wait. If it doesn't exist, that
> means the faulter can skip finish_wait altogether because it must be
> already TASK_RUNNING.
>
> shmem_falloc = inode->i_private;
> if (shmem_falloc && shmem_falloc->waitq == shmem_falloc_waitq)
> 	finish_wait(shmem_falloc_waitq, &shmem_fault_wait);
>
> It might still be theoretically possible that although it has the same
> address, it's not the same wait queue, but that doesn't hurt
> correctness. I might be uselessly locking some other waitq's lock, but
> the inode->i_lock still protects me from other faulters that are in the
> same situation. The puncher is already gone.

Actually, I was wrong and deleting from a different queue could corrupt 
the queue head. I don't know if trinity would be able to trigger this, 
but I wouldn't be comfortable knowing it's possible. Calling fallocate 
twice in quick succession from the same process could easily end up at 
the same address on the stack, no?

Another also somewhat ugly possibility is to make sure that the wait 
queue is empty before the puncher quits, regardless of the running state 
of the processes in the queue. I think the conditions here 
(serialization by i_lock) might allow us to do that without risking that 
we e.g. leave anyone sleeping. But it's bending the wait queue design...

> However it's quite ugly and if there is some wait queue debugging mode
> (I hadn't checked) that e.g. checks if wait queues and wait objects are
> empty before destruction, it wouldn't like this at all...

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2014-07-22 12:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-15 10:28 [PATCH 0/2] shmem: fix faulting into a hole while it's punched, take 3 Hugh Dickins
2014-07-15 10:31 ` [PATCH 1/2] shmem: fix faulting into a hole, not taking i_mutex Hugh Dickins
2014-07-15 16:07   ` Vlastimil Babka
2014-07-15 19:26     ` Hugh Dickins
2014-07-16  7:18       ` Vlastimil Babka
2014-07-25 14:25   ` Michal Hocko
2014-07-15 10:33 ` [PATCH 2/2] shmem: fix splicing from a hole while it's punched Hugh Dickins
2014-07-25 14:33   ` Michal Hocko
2014-07-17 16:10 ` [PATCH 0/2] shmem: fix faulting into a hole while it's punched, take 3 Vlastimil Babka
2014-07-17 16:12   ` Sasha Levin
2014-07-18 10:44     ` Sasha Levin
2014-07-19 23:44       ` Hugh Dickins
2014-07-22  3:24         ` Sasha Levin
2014-07-22  8:07           ` Hugh Dickins
2014-07-22 10:06             ` Vlastimil Babka
2014-07-22 12:09               ` Vlastimil Babka [this message]
2014-07-22 18:42                 ` Hugh Dickins
2014-07-22 23:19             ` Sasha Levin
2014-07-22 23:58               ` Hugh Dickins
2014-07-17 23:34   ` Hugh Dickins
2014-07-18  8:05     ` Vlastimil Babka

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=53CE5494.3030708@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=davej@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=koct9i@gmail.com \
    --cc=lczerner@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.cz \
    --cc=sasha.levin@oracle.com \
    --cc=walken@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).