All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@linux.intel.com>
To: "Huang, Ying" <ying.huang@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Michal Hocko <mhocko@suse.com>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Shaohua Li <shli@kernel.org>, Hugh Dickins <hughd@google.com>,
	Minchan Kim <minchan@kernel.org>, Rik van Riel <riel@redhat.com>,
	Daniel Jordan <daniel.m.jordan@oracle.com>,
	Dan Williams <dan.j.williams@intel.com>
Subject: Re: [PATCH 1/6] swap: Add comments to lock_cluster_or_swap_info()
Date: Fri, 13 Jul 2018 03:48:28 -0700	[thread overview]
Message-ID: <3c3a4dce-980d-0405-d269-1da9e62b1344@linux.intel.com> (raw)
In-Reply-To: <20180712233636.20629-2-ying.huang@intel.com>

> +/*
> + * At most times, fine grained cluster lock is sufficient to protect

Can we call out those times, please?

> + * the operations on sis->swap_map.  

Please be careful with the naming.  You can call it 'si' because that's
what the function argument is named.  Or, swap_info_struct because
that's the struct name.  Calling it 'sis' is a bit sloppy, no?

> 					No need to acquire gross grained

"coarse" is a conventional antonym for "fine".

> + * sis->lock.  But cluster and cluster lock isn't available for HDD,
> + * so sis->lock will be instead for them.
> + */
>  static inline struct swap_cluster_info *lock_cluster_or_swap_info(
>  	struct swap_info_struct *si,
>  	unsigned long offset)

What I already knew was: there are two locks.  We use one sometimes and
the other at other times.

What I don't know is why there are two locks, and the heuristics why we
choose between them.  This comment doesn't help explain the things I
don't know.


  reply	other threads:[~2018-07-13 10:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-12 23:36 [PATCH 0/6] swap: THP optimizing refactoring Huang, Ying
2018-07-12 23:36 ` [PATCH 1/6] swap: Add comments to lock_cluster_or_swap_info() Huang, Ying
2018-07-13 10:48   ` Dave Hansen [this message]
2018-07-14  4:07     ` Huang, Ying
2018-07-14  4:07       ` Huang, Ying
2018-07-12 23:36 ` [PATCH 2/6] mm/swapfile.c: Replace some #ifdef with IS_ENABLED() Huang, Ying
2018-07-13 18:38   ` Daniel Jordan
2018-07-13 18:55     ` Daniel Jordan
2018-07-12 23:36 ` [PATCH 3/6] swap: Unify normal/huge code path in swap_page_trans_huge_swapped() Huang, Ying
2018-07-13 20:15   ` Daniel Jordan
2018-07-14 12:53     ` Huang, Ying
2018-07-14 12:53       ` Huang, Ying
2018-07-12 23:36 ` [PATCH 4/6] swap: Unify normal/huge code path in put_swap_page() Huang, Ying
2018-07-12 23:36 ` [PATCH 5/6] swap: Add __swap_entry_free_locked() Huang, Ying
2018-07-12 23:36 ` [PATCH 6/6] swap, put_swap_page: Share more between huge/normal code path Huang, Ying
2018-07-13 20:18   ` Daniel Jordan
2018-07-14 12:57     ` Huang, Ying
2018-07-14 12:57       ` 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:
  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=3c3a4dce-980d-0405-d269-1da9e62b1344@linux.intel.com \
    --to=dave.hansen@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=dan.j.williams@intel.com \
    --cc=daniel.m.jordan@oracle.com \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@suse.com \
    --cc=minchan@kernel.org \
    --cc=riel@redhat.com \
    --cc=shli@kernel.org \
    --cc=ying.huang@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.