| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140924075742.GB3181@bbox>
Date: Wed, 24 Sep 2014 16:57:42 +0900
From: Minchan Kim <minchan@...nel.org>
To: Dan Streetman <ddstreet@...e.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
Linux-MM <linux-mm@...ck.org>, Hugh Dickins <hughd@...gle.com>,
Shaohua Li <shli@...nel.org>,
Jerome Marchand <jmarchan@...hat.com>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Nitin Gupta <ngupta@...are.org>,
Luigi Semenzato <semenzato@...gle.com>, juno.choi@....com
Subject: Re: [PATCH v1 2/5] mm: add full variable in swap_info_struct
On Tue, Sep 23, 2014 at 10:53:05PM -0400, Dan Streetman wrote:
> On Sun, Sep 21, 2014 at 8:03 PM, Minchan Kim <minchan@...nel.org> wrote:
> > Now, swap leans on !p->highest_bit to indicate a swap is full.
> > It works well for normal swap because every slot on swap device
> > is used up when the swap is full but in case of zram, swap sees
> > still many empty slot although backed device(ie, zram) is full
> > since zram's limit is over so that it could make trouble when
> > swap use highest_bit to select new slot via free_cluster.
> >
> > This patch introduces full varaiable in swap_info_struct
> > to solve the problem.
> >
> > Suggested-by: Dan Streetman <ddstreet@...e.org>
> > Signed-off-by: Minchan Kim <minchan@...nel.org>
> > ---
> > include/linux/swap.h | 1 +
> > mm/swapfile.c | 33 +++++++++++++++++++--------------
> > 2 files changed, 20 insertions(+), 14 deletions(-)
> >
> > diff --git a/include/linux/swap.h b/include/linux/swap.h
> > index ea4f926e6b9b..a3c11c051495 100644
> > --- a/include/linux/swap.h
> > +++ b/include/linux/swap.h
> > @@ -224,6 +224,7 @@ struct swap_info_struct {
> > struct swap_cluster_info free_cluster_tail; /* free cluster list tail */
> > unsigned int lowest_bit; /* index of first free in swap_map */
> > unsigned int highest_bit; /* index of last free in swap_map */
> > + bool full; /* whether swap is full or not */
> > unsigned int pages; /* total of usable pages of swap */
> > unsigned int inuse_pages; /* number of those currently in use */
> > unsigned int cluster_next; /* likely index for next allocation */
> > diff --git a/mm/swapfile.c b/mm/swapfile.c
> > index c07f7f4912e9..209112cf8b83 100644
> > --- a/mm/swapfile.c
> > +++ b/mm/swapfile.c
> > @@ -558,7 +558,7 @@ checks:
> > }
> > if (!(si->flags & SWP_WRITEOK))
> > goto no_page;
> > - if (!si->highest_bit)
> > + if (si->full)
> > goto no_page;
> > if (offset > si->highest_bit)
> > scan_base = offset = si->lowest_bit;
> > @@ -589,6 +589,7 @@ checks:
> > spin_lock(&swap_avail_lock);
> > plist_del(&si->avail_list, &swap_avail_head);
> > spin_unlock(&swap_avail_lock);
> > + si->full = true;
> > }
> > si->swap_map[offset] = usage;
> > inc_cluster_info_page(si, si->cluster_info, offset);
> > @@ -653,14 +654,14 @@ start_over:
> > plist_requeue(&si->avail_list, &swap_avail_head);
> > spin_unlock(&swap_avail_lock);
> > spin_lock(&si->lock);
> > - if (!si->highest_bit || !(si->flags & SWP_WRITEOK)) {
> > + if (si->full || !(si->flags & SWP_WRITEOK)) {
> > spin_lock(&swap_avail_lock);
> > if (plist_node_empty(&si->avail_list)) {
> > spin_unlock(&si->lock);
> > goto nextsi;
> > }
> > - WARN(!si->highest_bit,
> > - "swap_info %d in list but !highest_bit\n",
> > + WARN(si->full,
> > + "swap_info %d in list but swap is full\n",
> > si->type);
> > WARN(!(si->flags & SWP_WRITEOK),
> > "swap_info %d in list but !SWP_WRITEOK\n",
> > @@ -796,21 +797,25 @@ static unsigned char swap_entry_free(struct swap_info_struct *p,
> >
> > /* free if no reference */
> > if (!usage) {
> > + bool was_full;
> > +
> > dec_cluster_info_page(p, p->cluster_info, offset);
> > if (offset < p->lowest_bit)
> > p->lowest_bit = offset;
> > - if (offset > p->highest_bit) {
> > - bool was_full = !p->highest_bit;
> > + if (offset > p->highest_bit)
> > p->highest_bit = offset;
> > - if (was_full && (p->flags & SWP_WRITEOK)) {
> > - spin_lock(&swap_avail_lock);
> > - WARN_ON(!plist_node_empty(&p->avail_list));
> > - if (plist_node_empty(&p->avail_list))
> > - plist_add(&p->avail_list,
> > - &swap_avail_head);
> > - spin_unlock(&swap_avail_lock);
> > - }
> > + was_full = p->full;
> > +
> > + if (was_full && (p->flags & SWP_WRITEOK)) {
>
> was_full was only needed because highest_bit was reset to offset right
> before checking for fullness, so now that ->full is used instead of
> !highest_bit, was_full isn't needed anymore, you can just check
> p->full.
Okay.
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists