| 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
| ||
|
Date: Tue, 7 Dec 2010 18:08:27 +1100
From: Nick Piggin <npiggin@...nel.dk>
To: Trond Myklebust <Trond.Myklebust@...app.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Nick Bowler <nbowler@...iptictech.com>,
Simon Kirby <sim@...tway.ca>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
linux-nfs@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
Hugh Dickins <hughd@...gle.com>,
Nick Piggin <npiggin@...nel.dk>,
Rik van Riel <riel@...hat.com>,
Christoph Hellwig <hch@....de>,
Al Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH v4 2/3] Call the filesystem back whenever a page is
removed from the page cache
On Mon, Dec 06, 2010 at 11:59:07AM -0500, Trond Myklebust wrote:
> From: Linus Torvalds <torvalds@...ux-foundation.org>
>
> NFS needs to be able to release objects that are stored in the page
> cache once the page itself is no longer visible from the page cache.
>
> This patch adds a callback to the address space operations that allows
> filesystems to perform page cleanups once the page has been removed
> from the page cache.
>
> Original patch by: Linus Torvalds <torvalds@...ux-foundation.org>
> [trondmy: cover the cases of invalidate_inode_pages2() and
> truncate_inode_pages()]
> Signed-off-by: Trond Myklebust <Trond.Myklebust@...app.com>
> ---
> Documentation/filesystems/Locking | 7 ++++++-
> Documentation/filesystems/vfs.txt | 7 +++++++
> include/linux/fs.h | 1 +
> mm/filemap.c | 5 +++++
> mm/truncate.c | 4 ++++
> mm/vmscan.c | 7 +++++++
> 6 files changed, 30 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/filesystems/Locking b/Documentation/filesystems/Locking
> index a91f308..b6426f1 100644
> --- a/Documentation/filesystems/Locking
> +++ b/Documentation/filesystems/Locking
> @@ -173,12 +173,13 @@ prototypes:
> sector_t (*bmap)(struct address_space *, sector_t);
> int (*invalidatepage) (struct page *, unsigned long);
> int (*releasepage) (struct page *, int);
> + void (*freepage)(struct page *);
> int (*direct_IO)(int, struct kiocb *, const struct iovec *iov,
> loff_t offset, unsigned long nr_segs);
> int (*launder_page) (struct page *);
>
> locking rules:
> - All except set_page_dirty may block
> + All except set_page_dirty and freepage may block
>
> BKL PageLocked(page) i_mutex
> writepage: no yes, unlocks (see below)
> @@ -193,6 +194,7 @@ perform_write: no n/a yes
> bmap: no
> invalidatepage: no yes
> releasepage: no yes
> +freepage: no yes
> direct_IO: no
> launder_page: no yes
>
> @@ -288,6 +290,9 @@ buffers from the page in preparation for freeing it. It returns zero to
> indicate that the buffers are (or may be) freeable. If ->releasepage is zero,
> the kernel assumes that the fs has no private interest in the buffers.
>
> + ->freepage() is called when the kernel is done dropping the page
> +from the page cache.
> +
> ->launder_page() may be called prior to releasing a page if
> it is still found to be dirty. It returns zero if the page was successfully
> cleaned, or an error value if not. Note that in order to prevent the page
> diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
> index ed7e5ef..3b14a55 100644
> --- a/Documentation/filesystems/vfs.txt
> +++ b/Documentation/filesystems/vfs.txt
> @@ -534,6 +534,7 @@ struct address_space_operations {
> sector_t (*bmap)(struct address_space *, sector_t);
> int (*invalidatepage) (struct page *, unsigned long);
> int (*releasepage) (struct page *, int);
> + void (*freepage)(struct page *);
> ssize_t (*direct_IO)(int, struct kiocb *, const struct iovec *iov,
> loff_t offset, unsigned long nr_segs);
> struct page* (*get_xip_page)(struct address_space *, sector_t,
> @@ -679,6 +680,12 @@ struct address_space_operations {
> need to ensure this. Possibly it can clear the PageUptodate
> bit if it cannot free private data yet.
>
> + freepage: freepage is called once the page is no longer visible in
> + the page cache in order to allow the cleanup of any private
> + data. Since it may be called by the memory reclaimer, it
> + should not assume that the original address_space mapping still
> + exists, and it should not block.
Of course we still have bugs in this regard, without inode RCU and
filesystem deregistration RCU, but when those things are implemented
for RCU path-walk, this section should be updated somewhat, and we'll
have to look at RCU protecting the final mapping manipulations after
a page is removed from pagecache.
But I'll help work on that after RCU inodes / filesystems is merged.
> +
> direct_IO: called by the generic read/write routines to perform
> direct_IO - that is IO requests which bypass the page cache
> and transfer data directly between the storage and the
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index c9e06cc..090f0ea 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -602,6 +602,7 @@ struct address_space_operations {
> sector_t (*bmap)(struct address_space *, sector_t);
> void (*invalidatepage) (struct page *, unsigned long);
> int (*releasepage) (struct page *, gfp_t);
> + void (*freepage)(struct page *);
> ssize_t (*direct_IO)(int, struct kiocb *, const struct iovec *iov,
> loff_t offset, unsigned long nr_segs);
> int (*get_xip_mem)(struct address_space *, pgoff_t, int,
> diff --git a/mm/filemap.c b/mm/filemap.c
> index ea89840..6b9aee2 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -143,13 +143,18 @@ void __remove_from_page_cache(struct page *page)
> void remove_from_page_cache(struct page *page)
> {
> struct address_space *mapping = page->mapping;
> + void (*freepage)(struct page *);
>
> BUG_ON(!PageLocked(page));
>
> + freepage = mapping->a_ops->freepage;
> spin_lock_irq(&mapping->tree_lock);
> __remove_from_page_cache(page);
> spin_unlock_irq(&mapping->tree_lock);
> mem_cgroup_uncharge_cache_page(page);
> +
> + if (freepage)
> + freepage(page);
> }
> EXPORT_SYMBOL(remove_from_page_cache);
>
> diff --git a/mm/truncate.c b/mm/truncate.c
> index ba887bf..3c2d5dd 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -390,6 +390,10 @@ invalidate_complete_page2(struct address_space *mapping, struct page *page)
> __remove_from_page_cache(page);
> spin_unlock_irq(&mapping->tree_lock);
> mem_cgroup_uncharge_cache_page(page);
> +
> + if (mapping->a_ops->freepage)
> + mapping->a_ops->freepage(page);
> +
> page_cache_release(page); /* pagecache ref */
> return 1;
> failed:
The generic parts of the code look OK to me, but why is there a
difference in your sequences of loading the freepage function pointer
here?
--
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