From: "Huang, Ying" <ying.huang@intel.com>
To: Douglas Anderson <dianders@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Mel Gorman <mgorman@techsingularity.net>,
Vlastimil Babka <vbabka@suse.cz>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Christian Brauner <brauner@kernel.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Yu Zhao <yuzhao@google.com>,
linux-fsdevel@vger.kernel.org,
Matthew Wilcox <willy@infradead.org>
Subject: Re: [PATCH v2 1/4] mm/filemap: Add folio_lock_timeout()
Date: Sun, 23 Apr 2023 15:50:01 +0800 [thread overview]
Message-ID: <87leijkpg6.fsf@yhuang6-desk2.ccr.corp.intel.com> (raw)
In-Reply-To: <20230421151135.v2.1.I2b71e11264c5c214bc59744b9e13e4c353bc5714@changeid> (Douglas Anderson's message of "Fri, 21 Apr 2023 15:12:45 -0700")
Douglas Anderson <dianders@chromium.org> writes:
> Add a variant of folio_lock() that can timeout. This is useful to
> avoid unbounded waits for the page lock in kcompactd.
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - "Add folio_lock_timeout()" new for v2.
>
> include/linux/pagemap.h | 16 ++++++++++++++
> mm/filemap.c | 47 +++++++++++++++++++++++++++++------------
> 2 files changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 0acb8e1fb7af..0f3ef9f79300 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -892,6 +892,7 @@ static inline bool wake_page_match(struct wait_page_queue *wait_page,
> }
>
> void __folio_lock(struct folio *folio);
> +int __folio_lock_timeout(struct folio *folio, long timeout);
> int __folio_lock_killable(struct folio *folio);
> bool __folio_lock_or_retry(struct folio *folio, struct mm_struct *mm,
> unsigned int flags);
> @@ -952,6 +953,21 @@ static inline void folio_lock(struct folio *folio)
> __folio_lock(folio);
> }
>
> +/**
> + * folio_lock_timeout() - Lock this folio, with a timeout.
> + * @folio: The folio to lock.
> + * @timeout: The timeout in jiffies; %MAX_SCHEDULE_TIMEOUT means wait forever.
> + *
> + * Return: 0 upon success; -ETIMEDOUT upon failure.
IIUC, the funtion may return -EINTR too.
Otherwise looks good to me. Thanks!
Best Regards,
Huang, Ying
> + */
> +static inline int folio_lock_timeout(struct folio *folio, long timeout)
> +{
> + might_sleep();
> + if (!folio_trylock(folio))
> + return __folio_lock_timeout(folio, timeout);
> + return 0;
> +}
> +
> /**
> * lock_page() - Lock the folio containing this page.
> * @page: The page to lock.
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 2723104cc06a..c6056ec41284 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1220,7 +1220,7 @@ static inline bool folio_trylock_flag(struct folio *folio, int bit_nr,
> int sysctl_page_lock_unfairness = 5;
>
> static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,
> - int state, enum behavior behavior)
> + int state, enum behavior behavior, long timeout)
> {
> wait_queue_head_t *q = folio_waitqueue(folio);
> int unfairness = sysctl_page_lock_unfairness;
> @@ -1229,6 +1229,7 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,
> bool thrashing = false;
> unsigned long pflags;
> bool in_thrashing;
> + int err;
>
> if (bit_nr == PG_locked &&
> !folio_test_uptodate(folio) && folio_test_workingset(folio)) {
> @@ -1295,10 +1296,13 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,
> /* Loop until we've been woken or interrupted */
> flags = smp_load_acquire(&wait->flags);
> if (!(flags & WQ_FLAG_WOKEN)) {
> + if (!timeout)
> + break;
> +
> if (signal_pending_state(state, current))
> break;
>
> - io_schedule();
> + timeout = io_schedule_timeout(timeout);
> continue;
> }
>
> @@ -1324,10 +1328,10 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,
> }
>
> /*
> - * If a signal happened, this 'finish_wait()' may remove the last
> - * waiter from the wait-queues, but the folio waiters bit will remain
> - * set. That's ok. The next wakeup will take care of it, and trying
> - * to do it here would be difficult and prone to races.
> + * If a signal/timeout happened, this 'finish_wait()' may remove the
> + * last waiter from the wait-queues, but the folio waiters bit will
> + * remain set. That's ok. The next wakeup will take care of it, and
> + * trying to do it here would be difficult and prone to races.
> */
> finish_wait(q, wait);
>
> @@ -1336,6 +1340,13 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,
> psi_memstall_leave(&pflags);
> }
>
> + /*
> + * If we don't meet the success criteria below then we've got an error
> + * of some sort. Differentiate between the two error cases. If there's
> + * no time left it must have been a timeout.
> + */
> + err = !timeout ? -ETIMEDOUT : -EINTR;
> +
> /*
> * NOTE! The wait->flags weren't stable until we've done the
> * 'finish_wait()', and we could have exited the loop above due
> @@ -1350,9 +1361,9 @@ static inline int folio_wait_bit_common(struct folio *folio, int bit_nr,
> * waiter, but an exclusive one requires WQ_FLAG_DONE.
> */
> if (behavior == EXCLUSIVE)
> - return wait->flags & WQ_FLAG_DONE ? 0 : -EINTR;
> + return wait->flags & WQ_FLAG_DONE ? 0 : err;
>
> - return wait->flags & WQ_FLAG_WOKEN ? 0 : -EINTR;
> + return wait->flags & WQ_FLAG_WOKEN ? 0 : err;
> }
>
> #ifdef CONFIG_MIGRATION
> @@ -1442,13 +1453,15 @@ void migration_entry_wait_on_locked(swp_entry_t entry, pte_t *ptep,
>
> void folio_wait_bit(struct folio *folio, int bit_nr)
> {
> - folio_wait_bit_common(folio, bit_nr, TASK_UNINTERRUPTIBLE, SHARED);
> + folio_wait_bit_common(folio, bit_nr, TASK_UNINTERRUPTIBLE, SHARED,
> + MAX_SCHEDULE_TIMEOUT);
> }
> EXPORT_SYMBOL(folio_wait_bit);
>
> int folio_wait_bit_killable(struct folio *folio, int bit_nr)
> {
> - return folio_wait_bit_common(folio, bit_nr, TASK_KILLABLE, SHARED);
> + return folio_wait_bit_common(folio, bit_nr, TASK_KILLABLE, SHARED,
> + MAX_SCHEDULE_TIMEOUT);
> }
> EXPORT_SYMBOL(folio_wait_bit_killable);
>
> @@ -1467,7 +1480,8 @@ EXPORT_SYMBOL(folio_wait_bit_killable);
> */
> static int folio_put_wait_locked(struct folio *folio, int state)
> {
> - return folio_wait_bit_common(folio, PG_locked, state, DROP);
> + return folio_wait_bit_common(folio, PG_locked, state, DROP,
> + MAX_SCHEDULE_TIMEOUT);
> }
>
> /**
> @@ -1662,17 +1676,24 @@ EXPORT_SYMBOL_GPL(page_endio);
> void __folio_lock(struct folio *folio)
> {
> folio_wait_bit_common(folio, PG_locked, TASK_UNINTERRUPTIBLE,
> - EXCLUSIVE);
> + EXCLUSIVE, MAX_SCHEDULE_TIMEOUT);
> }
> EXPORT_SYMBOL(__folio_lock);
>
> int __folio_lock_killable(struct folio *folio)
> {
> return folio_wait_bit_common(folio, PG_locked, TASK_KILLABLE,
> - EXCLUSIVE);
> + EXCLUSIVE, MAX_SCHEDULE_TIMEOUT);
> }
> EXPORT_SYMBOL_GPL(__folio_lock_killable);
>
> +int __folio_lock_timeout(struct folio *folio, long timeout)
> +{
> + return folio_wait_bit_common(folio, PG_locked, TASK_KILLABLE,
> + EXCLUSIVE, timeout);
> +}
> +EXPORT_SYMBOL_GPL(__folio_lock_timeout);
> +
> static int __folio_lock_async(struct folio *folio, struct wait_page_queue *wait)
> {
> struct wait_queue_head *q = folio_waitqueue(folio);
next prev parent reply other threads:[~2023-04-23 7:51 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-21 22:12 [PATCH v2 0/4] migrate: Avoid unbounded blocks in MIGRATE_SYNC_LIGHT Douglas Anderson
2023-04-21 22:12 ` [PATCH v2 1/4] mm/filemap: Add folio_lock_timeout() Douglas Anderson
2023-04-22 5:18 ` Hillf Danton
[not found] ` <20230423081203.1812-1-hdanton@sina.com>
2023-04-23 8:35 ` Gao Xiang
2023-04-23 9:49 ` Hillf Danton
2023-04-23 10:45 ` Gao Xiang
2023-04-24 16:56 ` Doug Anderson
2023-04-25 1:09 ` Hillf Danton
2023-04-25 14:19 ` Doug Anderson
2023-04-26 4:42 ` Hillf Danton
2023-04-26 4:55 ` Doug Anderson
2023-04-26 10:09 ` Mel Gorman
2023-04-26 15:14 ` Matthew Wilcox
2023-04-26 20:46 ` Doug Anderson
2023-04-26 21:26 ` Matthew Wilcox
2023-04-26 21:39 ` Doug Anderson
2023-04-27 2:16 ` Matthew Wilcox
2023-04-27 9:48 ` Mel Gorman
2023-04-28 8:17 ` Hillf Danton
2023-04-26 15:24 ` Linus Torvalds
2023-04-23 7:50 ` Huang, Ying [this message]
2023-04-24 8:22 ` Mel Gorman
2023-04-24 16:22 ` Doug Anderson
2023-04-25 8:00 ` Mel Gorman
2023-04-21 22:12 ` [PATCH v2 2/4] buffer: Add lock_buffer_timeout() Douglas Anderson
2023-04-23 8:47 ` Huang, Ying
2023-04-21 22:12 ` [PATCH v2 3/4] migrate_pages: Don't wait forever locking pages in MIGRATE_SYNC_LIGHT Douglas Anderson
2023-04-23 7:59 ` Huang, Ying
2023-04-24 9:38 ` Mel Gorman
2023-04-21 22:12 ` [PATCH v2 4/4] migrate_pages: Don't wait forever locking buffers " Douglas Anderson
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=87leijkpg6.fsf@yhuang6-desk2.ccr.corp.intel.com \
--to=ying.huang@intel.com \
--cc=akpm@linux-foundation.org \
--cc=brauner@kernel.org \
--cc=dianders@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=vbabka@suse.cz \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
--cc=yuzhao@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).