LKML Archive on
help / color / mirror / Atom feed
From: Hugh Dickins <>
To: "Huang, Ying" <>
Cc: Andrew Morton <>,
	Hugh Dickins <>,
	David Hildenbrand <>,
	Yang Shi <>, Linux-MM <>,
	LKML <>,
	Miaohe Lin <>,
	Johannes Weiner <>,
	Michal Hocko <>,
	Joonsoo Kim <>,
	Matthew Wilcox <>,
	Minchan Kim <>,
	huang ying <>
Subject: Re: [PATCH] mm,shmem: Fix a typo in shmem_swapin_page()
Date: Tue, 3 Aug 2021 21:28:13 -0700 (PDT)	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Tue, 3 Aug 2021, Huang, Ying wrote:
> As Hugh pointed out, EINVAL isn't an appropriate error code for race
> condition.  After checking the code, I found that EEXIST is the error
> code used for race condition.  So I revise the patch as below.  If Hugh
> doesn't object, can you help to replace the patch with the below one?

(I'm sorry that it's so hard to extract responses from me...)

Yes, I'll go along with this version, or Matthew's better commented
version, which Andrew has now taken into his tree.

I won't go so far as to Ack this, because I still want to revert the
original commit; but this will not do actual harm, and I'm too slow
to mess you around further for 5.14.  I'll just have to work through
it and argue it later when/if I have time.

I'll say more on that in answering your earlier mail in this thread.

But should admit right now that I think have somewhat misled us all.
Neither the EINVAL nor the -EINVAL were as dangerous as they looked:
because they were followed immediately by "goto failed", and

	if (!shmem_confirm_swap(mapping, index, swap))
		error = -EEXIST;

and in the case that get_swap_device() fails, all the swapping off
has been done, so shmem_confirm_swap() will return false, and the
error then be set to -EEXIST anyway.

But let's pretend that I hadn't realized that: what's in Andrew's
tree is better than what was there before.

(And let's pretend that in writing those paragraphs, I did not
realize that get_swap_device() could also fail if entry had got
corrupted - should never happen, of course - and shmem then get
stuck in a repeating -EEXIST loop.  Maybe I'll want to do better
for that case too, but not this time.)


  parent reply	other threads:[~2021-08-04  4:32 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-23  8:00 Huang Ying
2021-07-23 11:10 ` David Hildenbrand
2021-07-23 20:23 ` Hugh Dickins
2021-07-23 21:53   ` Matthew Wilcox
2021-08-03  8:14     ` Huang, Ying
2021-08-03 12:01       ` Matthew Wilcox
2021-08-04  5:34         ` Hugh Dickins
2021-08-04  9:04           ` Huang, Ying
2021-08-05 23:08           ` Yang Shi
2021-08-06  6:01             ` Hugh Dickins
2021-08-06 20:37               ` Yang Shi
2021-08-09 21:26                 ` Yang Shi
2021-08-09 23:43                   ` Huang, Ying
2021-08-10  1:11                     ` Yang Shi
2021-08-04  9:01         ` Huang, Ying
2021-07-28 13:03   ` huang ying
2021-08-03  8:06     ` Huang, Ying
2021-08-03 11:59       ` Matthew Wilcox
2021-08-04  4:28       ` Hugh Dickins [this message]
2021-08-04  6:42     ` Hugh Dickins
2021-08-04  8:59       ` Huang, Ying

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH] mm,shmem: Fix a typo in shmem_swapin_page()' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).