All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Matthew Wilcox <willy@infradead.org>,
	linux-mm@kvack.org, Matthew Wilcox <mawilcox@microsoft.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/4] s390: Use _refcount for pgtables
Date: Thu, 1 Mar 2018 15:39:52 +0100	[thread overview]
Message-ID: <20180301153952.668bfdc7@mschwideX1> (raw)
In-Reply-To: <20180301142855.emaa5x65oj2hkwsm@node.shutemov.name>

On Thu, 1 Mar 2018 17:28:55 +0300
"Kirill A. Shutemov" <kirill@shutemov.name> wrote:

> On Thu, Mar 01, 2018 at 03:04:20PM +0100, Martin Schwidefsky wrote:
> > On Thu, 1 Mar 2018 15:53:10 +0300
> > "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> >   
> > > On Wed, Feb 28, 2018 at 02:31:54PM -0800, Matthew Wilcox wrote:  
> > > > From: Matthew Wilcox <mawilcox@microsoft.com>
> > > > 
> > > > s390 borrows the storage used for _mapcount in struct page in order to
> > > > account whether the bottom or top half is being used for 2kB page
> > > > tables.  I want to use that for something else, so use the top byte of
> > > > _refcount instead of the bottom byte of _mapcount.  _refcount may
> > > > temporarily be incremented by other CPUs that see a stale pointer to
> > > > this page in the page cache, but each CPU can only increment it by one,
> > > > and there are no systems with 2^24 CPUs today, so they will not change
> > > > the upper byte of _refcount.  We do have to be a little careful not to
> > > > lose any of their writes (as they will subsequently decrement the
> > > > counter).    
> > > 
> > > Hm. I'm more worried about false-negative put_page_testzero().
> > > Are you sure it won't lead to leaks. I cannot say from the code changes.
> > > 
> > > And for page-table pages should have planty space in other fields.
> > > IIRC page->mapping is unused there.  
> >  
> > 2^^24 put_page_testzero calls for page table pages? I don't think so.  
> 
> No, I mean oposite: we don't free the page when we should. 2^24 is not
> zero and page won't be freed if the acctual refcount (without the flag in
> upper bits) drops to zero.

Ah, ok. But this is not a problem as the page is freed after both bits for
the two 2K pieces havbe been set to zero.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

WARNING: multiple messages have this Message-ID (diff)
From: Martin Schwidefsky <schwidefsky@de.ibm.com>
To: "Kirill A. Shutemov" <kirill@shutemov.name>
Cc: Matthew Wilcox <willy@infradead.org>,
	linux-mm@kvack.org, Matthew Wilcox <mawilcox@microsoft.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 1/4] s390: Use _refcount for pgtables
Date: Thu, 1 Mar 2018 15:39:52 +0100	[thread overview]
Message-ID: <20180301153952.668bfdc7@mschwideX1> (raw)
In-Reply-To: <20180301142855.emaa5x65oj2hkwsm@node.shutemov.name>

On Thu, 1 Mar 2018 17:28:55 +0300
"Kirill A. Shutemov" <kirill@shutemov.name> wrote:

> On Thu, Mar 01, 2018 at 03:04:20PM +0100, Martin Schwidefsky wrote:
> > On Thu, 1 Mar 2018 15:53:10 +0300
> > "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> >   
> > > On Wed, Feb 28, 2018 at 02:31:54PM -0800, Matthew Wilcox wrote:  
> > > > From: Matthew Wilcox <mawilcox@microsoft.com>
> > > > 
> > > > s390 borrows the storage used for _mapcount in struct page in order to
> > > > account whether the bottom or top half is being used for 2kB page
> > > > tables.  I want to use that for something else, so use the top byte of
> > > > _refcount instead of the bottom byte of _mapcount.  _refcount may
> > > > temporarily be incremented by other CPUs that see a stale pointer to
> > > > this page in the page cache, but each CPU can only increment it by one,
> > > > and there are no systems with 2^24 CPUs today, so they will not change
> > > > the upper byte of _refcount.  We do have to be a little careful not to
> > > > lose any of their writes (as they will subsequently decrement the
> > > > counter).    
> > > 
> > > Hm. I'm more worried about false-negative put_page_testzero().
> > > Are you sure it won't lead to leaks. I cannot say from the code changes.
> > > 
> > > And for page-table pages should have planty space in other fields.
> > > IIRC page->mapping is unused there.  
> >  
> > 2^^24 put_page_testzero calls for page table pages? I don't think so.  
> 
> No, I mean oposite: we don't free the page when we should. 2^24 is not
> zero and page won't be freed if the acctual refcount (without the flag in
> upper bits) drops to zero.

Ah, ok. But this is not a problem as the page is freed after both bits for
the two 2K pieces havbe been set to zero.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

--
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:[~2018-03-01 14:40 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-28 22:31 [PATCH v3 0/4] Split page_type out from mapcount Matthew Wilcox
2018-02-28 22:31 ` Matthew Wilcox
2018-02-28 22:31 ` [PATCH v3 1/4] s390: Use _refcount for pgtables Matthew Wilcox
2018-02-28 22:31   ` Matthew Wilcox
2018-03-01 12:53   ` Kirill A. Shutemov
2018-03-01 12:53     ` Kirill A. Shutemov
2018-03-01 14:04     ` Martin Schwidefsky
2018-03-01 14:04       ` Martin Schwidefsky
2018-03-01 14:28       ` Kirill A. Shutemov
2018-03-01 14:28         ` Kirill A. Shutemov
2018-03-01 14:39         ` Martin Schwidefsky [this message]
2018-03-01 14:39           ` Martin Schwidefsky
2018-02-28 22:31 ` [PATCH v3 2/4] mm: Split page_type out from _map_count Matthew Wilcox
2018-02-28 22:31   ` Matthew Wilcox
2018-02-28 22:31 ` [PATCH v3 3/4] mm: Mark pages allocated through vmalloc Matthew Wilcox
2018-02-28 22:31   ` Matthew Wilcox
2018-02-28 22:31 ` [PATCH v3 4/4] mm: Mark pages in use for page tables Matthew Wilcox
2018-02-28 22:31   ` Matthew Wilcox
2018-02-28 23:22 ` [PATCH v3 0/4] Split page_type out from mapcount Randy Dunlap
2018-02-28 23:22   ` Randy Dunlap
2018-03-01  1:31   ` Matthew Wilcox
2018-03-01  1:31     ` Matthew Wilcox
2018-03-01  7:17 ` Martin Schwidefsky
2018-03-01  7:17   ` Martin Schwidefsky
2018-03-01  8:00   ` Martin Schwidefsky
2018-03-01  8:00     ` Martin Schwidefsky
2018-03-01 12:44   ` Kirill A. Shutemov
2018-03-01 12:44     ` Kirill A. Shutemov
2018-03-01 14:47     ` Martin Schwidefsky
2018-03-01 14:47       ` Martin Schwidefsky
2018-03-01 14:50     ` Matthew Wilcox
2018-03-01 14:50       ` Matthew Wilcox
2018-03-01 15:02       ` Martin Schwidefsky
2018-03-01 15:02         ` Martin Schwidefsky

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=20180301153952.668bfdc7@mschwideX1 \
    --to=schwidefsky@de.ibm.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mawilcox@microsoft.com \
    --cc=willy@infradead.org \
    /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.