* [PATCH v2 0/2] kill add_to_page_cache_locked()
@ 2020-10-21 19:57 Kent Overstreet
2020-10-21 19:57 ` [PATCH v2 1/2] cifs: convert to add_to_page_cache() Kent Overstreet
2020-10-21 19:57 ` [PATCH v2 2/2] fs: kill add_to_page_cache_locked() Kent Overstreet
0 siblings, 2 replies; 6+ messages in thread
From: Kent Overstreet @ 2020-10-21 19:57 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, akpm, sfrench, linux-cifs; +Cc: Kent Overstreet
since v1
- kill a faulty assertion found by kernel test robot
- drop an unneeded line break
Andrew, can this go through your tree?
Kent Overstreet (2):
cifs: convert to add_to_page_cache()
fs: kill add_to_page_cache_locked()
fs/cifs/file.c | 20 +++----------
include/linux/pagemap.h | 20 ++-----------
mm/filemap.c | 64 ++++++++++++++++++++---------------------
3 files changed, 37 insertions(+), 67 deletions(-)
--
2.28.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] cifs: convert to add_to_page_cache()
2020-10-21 19:57 [PATCH v2 0/2] kill add_to_page_cache_locked() Kent Overstreet
@ 2020-10-21 19:57 ` Kent Overstreet
2020-10-22 8:36 ` Steve French
2020-11-11 0:52 ` Matthew Wilcox
2020-10-21 19:57 ` [PATCH v2 2/2] fs: kill add_to_page_cache_locked() Kent Overstreet
1 sibling, 2 replies; 6+ messages in thread
From: Kent Overstreet @ 2020-10-21 19:57 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, akpm, sfrench, linux-cifs; +Cc: Kent Overstreet
This is just open coding add_to_page_cache(), and the next patch will
delete add_to_page_cache_locked().
Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
---
fs/cifs/file.c | 20 ++++----------------
1 file changed, 4 insertions(+), 16 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index be46fab4c9..b3ee790532 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -4296,20 +4296,11 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
page = lru_to_page(page_list);
- /*
- * Lock the page and put it in the cache. Since no one else
- * should have access to this page, we're safe to simply set
- * PG_locked without checking it first.
- */
- __SetPageLocked(page);
- rc = add_to_page_cache_locked(page, mapping,
- page->index, gfp);
+ rc = add_to_page_cache(page, mapping, page->index, gfp);
/* give up if we can't stick it in the cache */
- if (rc) {
- __ClearPageLocked(page);
+ if (rc)
return rc;
- }
/* move first page to the tmplist */
*offset = (loff_t)page->index << PAGE_SHIFT;
@@ -4328,12 +4319,9 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
if (*bytes + PAGE_SIZE > rsize)
break;
- __SetPageLocked(page);
- rc = add_to_page_cache_locked(page, mapping, page->index, gfp);
- if (rc) {
- __ClearPageLocked(page);
+ rc = add_to_page_cache(page, mapping, page->index, gfp);
+ if (rc)
break;
- }
list_move_tail(&page->lru, tmplist);
(*bytes) += PAGE_SIZE;
expected_index++;
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] fs: kill add_to_page_cache_locked()
2020-10-21 19:57 [PATCH v2 0/2] kill add_to_page_cache_locked() Kent Overstreet
2020-10-21 19:57 ` [PATCH v2 1/2] cifs: convert to add_to_page_cache() Kent Overstreet
@ 2020-10-21 19:57 ` Kent Overstreet
2020-10-21 20:51 ` Matthew Wilcox
1 sibling, 1 reply; 6+ messages in thread
From: Kent Overstreet @ 2020-10-21 19:57 UTC (permalink / raw)
To: linux-kernel, linux-fsdevel, akpm, sfrench, linux-cifs; +Cc: Kent Overstreet
No longer has any users, so remove it.
Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
---
include/linux/pagemap.h | 20 ++-----------
mm/filemap.c | 64 ++++++++++++++++++++---------------------
2 files changed, 33 insertions(+), 51 deletions(-)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 434c9c34ae..aceaebfaab 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -689,8 +689,8 @@ static inline int fault_in_pages_readable(const char __user *uaddr, int size)
return 0;
}
-int add_to_page_cache_locked(struct page *page, struct address_space *mapping,
- pgoff_t index, gfp_t gfp_mask);
+int add_to_page_cache(struct page *page, struct address_space *mapping,
+ pgoff_t index, gfp_t gfp_mask);
int add_to_page_cache_lru(struct page *page, struct address_space *mapping,
pgoff_t index, gfp_t gfp_mask);
extern void delete_from_page_cache(struct page *page);
@@ -710,22 +710,6 @@ void page_cache_readahead_unbounded(struct address_space *, struct file *,
pgoff_t index, unsigned long nr_to_read,
unsigned long lookahead_count);
-/*
- * Like add_to_page_cache_locked, but used to add newly allocated pages:
- * the page is new, so we can just run __SetPageLocked() against it.
- */
-static inline int add_to_page_cache(struct page *page,
- struct address_space *mapping, pgoff_t offset, gfp_t gfp_mask)
-{
- int error;
-
- __SetPageLocked(page);
- error = add_to_page_cache_locked(page, mapping, offset, gfp_mask);
- if (unlikely(error))
- __ClearPageLocked(page);
- return error;
-}
-
/**
* struct readahead_control - Describes a readahead request.
*
diff --git a/mm/filemap.c b/mm/filemap.c
index bb71334fdd..b92ca48b90 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -827,20 +827,20 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
}
EXPORT_SYMBOL_GPL(replace_page_cache_page);
-static int __add_to_page_cache_locked(struct page *page,
- struct address_space *mapping,
- pgoff_t offset, gfp_t gfp_mask,
- void **shadowp)
+static int __add_to_page_cache(struct page *page,
+ struct address_space *mapping,
+ pgoff_t offset, gfp_t gfp_mask,
+ void **shadowp)
{
XA_STATE(xas, &mapping->i_pages, offset);
int huge = PageHuge(page);
int error;
void *old;
- VM_BUG_ON_PAGE(!PageLocked(page), page);
VM_BUG_ON_PAGE(PageSwapBacked(page), page);
mapping_set_update(&xas, mapping);
+ __SetPageLocked(page);
get_page(page);
page->mapping = mapping;
page->index = offset;
@@ -885,29 +885,31 @@ static int __add_to_page_cache_locked(struct page *page,
page->mapping = NULL;
/* Leave page->index set: truncation relies upon it */
put_page(page);
+ __ClearPageLocked(page);
return error;
}
-ALLOW_ERROR_INJECTION(__add_to_page_cache_locked, ERRNO);
+ALLOW_ERROR_INJECTION(__add_to_page_cache, ERRNO);
/**
- * add_to_page_cache_locked - add a locked page to the pagecache
+ * add_to_page_cache - add a newly allocated page to the pagecache
* @page: page to add
* @mapping: the page's address_space
* @offset: page index
* @gfp_mask: page allocation mode
*
- * This function is used to add a page to the pagecache. It must be locked.
- * This function does not add the page to the LRU. The caller must do that.
+ * This function is used to add a page to the pagecache. It must be newly
+ * allocated. This function does not add the page to the LRU. The caller must
+ * do that.
*
* Return: %0 on success, negative error code otherwise.
*/
-int add_to_page_cache_locked(struct page *page, struct address_space *mapping,
- pgoff_t offset, gfp_t gfp_mask)
+int add_to_page_cache(struct page *page, struct address_space *mapping,
+ pgoff_t offset, gfp_t gfp_mask)
{
- return __add_to_page_cache_locked(page, mapping, offset,
- gfp_mask, NULL);
+ return __add_to_page_cache(page, mapping, offset, gfp_mask, NULL);
}
-EXPORT_SYMBOL(add_to_page_cache_locked);
+EXPORT_SYMBOL(add_to_page_cache);
+ALLOW_ERROR_INJECTION(add_to_page_cache, ERRNO);
int add_to_page_cache_lru(struct page *page, struct address_space *mapping,
pgoff_t offset, gfp_t gfp_mask)
@@ -915,26 +917,22 @@ int add_to_page_cache_lru(struct page *page, struct address_space *mapping,
void *shadow = NULL;
int ret;
- __SetPageLocked(page);
- ret = __add_to_page_cache_locked(page, mapping, offset,
- gfp_mask, &shadow);
+ ret = __add_to_page_cache(page, mapping, offset, gfp_mask, &shadow);
if (unlikely(ret))
- __ClearPageLocked(page);
- else {
- /*
- * The page might have been evicted from cache only
- * recently, in which case it should be activated like
- * any other repeatedly accessed page.
- * The exception is pages getting rewritten; evicting other
- * data from the working set, only to cache data that will
- * get overwritten with something else, is a waste of memory.
- */
- WARN_ON_ONCE(PageActive(page));
- if (!(gfp_mask & __GFP_WRITE) && shadow)
- workingset_refault(page, shadow);
- lru_cache_add(page);
- }
- return ret;
+ return ret;
+
+ /*
+ * The page might have been evicted from cache only recently, in which
+ * case it should be activated like any other repeatedly accessed page.
+ * The exception is pages getting rewritten; evicting other data from
+ * the working set, only to cache data that will get overwritten with
+ * something else, is a waste of memory.
+ */
+ WARN_ON_ONCE(PageActive(page));
+ if (!(gfp_mask & __GFP_WRITE) && shadow)
+ workingset_refault(page, shadow);
+ lru_cache_add(page);
+ return 0;
}
EXPORT_SYMBOL_GPL(add_to_page_cache_lru);
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] fs: kill add_to_page_cache_locked()
2020-10-21 19:57 ` [PATCH v2 2/2] fs: kill add_to_page_cache_locked() Kent Overstreet
@ 2020-10-21 20:51 ` Matthew Wilcox
0 siblings, 0 replies; 6+ messages in thread
From: Matthew Wilcox @ 2020-10-21 20:51 UTC (permalink / raw)
To: Kent Overstreet; +Cc: linux-kernel, linux-fsdevel, akpm, sfrench, linux-cifs
On Wed, Oct 21, 2020 at 03:57:45PM -0400, Kent Overstreet wrote:
> }
> -ALLOW_ERROR_INJECTION(__add_to_page_cache_locked, ERRNO);
> +ALLOW_ERROR_INJECTION(__add_to_page_cache, ERRNO);
[..]
> +int add_to_page_cache(struct page *page, struct address_space *mapping,
> + pgoff_t offset, gfp_t gfp_mask)
> {
> - return __add_to_page_cache_locked(page, mapping, offset,
> - gfp_mask, NULL);
> + return __add_to_page_cache(page, mapping, offset, gfp_mask, NULL);
> }
> -EXPORT_SYMBOL(add_to_page_cache_locked);
> +EXPORT_SYMBOL(add_to_page_cache);
> +ALLOW_ERROR_INJECTION(add_to_page_cache, ERRNO);
I don't think you need this second one since __add_to_page_cache can be
used for the same purpose.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] cifs: convert to add_to_page_cache()
2020-10-21 19:57 ` [PATCH v2 1/2] cifs: convert to add_to_page_cache() Kent Overstreet
@ 2020-10-22 8:36 ` Steve French
2020-11-11 0:52 ` Matthew Wilcox
1 sibling, 0 replies; 6+ messages in thread
From: Steve French @ 2020-10-22 8:36 UTC (permalink / raw)
To: Kent Overstreet; +Cc: LKML, linux-fsdevel, Andrew Morton, Steve French, CIFS
you can add my reviewed-by if you would like
On Thu, Oct 22, 2020 at 1:48 AM Kent Overstreet
<kent.overstreet@gmail.com> wrote:
>
> This is just open coding add_to_page_cache(), and the next patch will
> delete add_to_page_cache_locked().
>
> Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
> ---
> fs/cifs/file.c | 20 ++++----------------
> 1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index be46fab4c9..b3ee790532 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -4296,20 +4296,11 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
>
> page = lru_to_page(page_list);
>
> - /*
> - * Lock the page and put it in the cache. Since no one else
> - * should have access to this page, we're safe to simply set
> - * PG_locked without checking it first.
> - */
> - __SetPageLocked(page);
> - rc = add_to_page_cache_locked(page, mapping,
> - page->index, gfp);
> + rc = add_to_page_cache(page, mapping, page->index, gfp);
>
> /* give up if we can't stick it in the cache */
> - if (rc) {
> - __ClearPageLocked(page);
> + if (rc)
> return rc;
> - }
>
> /* move first page to the tmplist */
> *offset = (loff_t)page->index << PAGE_SHIFT;
> @@ -4328,12 +4319,9 @@ readpages_get_pages(struct address_space *mapping, struct list_head *page_list,
> if (*bytes + PAGE_SIZE > rsize)
> break;
>
> - __SetPageLocked(page);
> - rc = add_to_page_cache_locked(page, mapping, page->index, gfp);
> - if (rc) {
> - __ClearPageLocked(page);
> + rc = add_to_page_cache(page, mapping, page->index, gfp);
> + if (rc)
> break;
> - }
> list_move_tail(&page->lru, tmplist);
> (*bytes) += PAGE_SIZE;
> expected_index++;
> --
> 2.28.0
>
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] cifs: convert to add_to_page_cache()
2020-10-21 19:57 ` [PATCH v2 1/2] cifs: convert to add_to_page_cache() Kent Overstreet
2020-10-22 8:36 ` Steve French
@ 2020-11-11 0:52 ` Matthew Wilcox
1 sibling, 0 replies; 6+ messages in thread
From: Matthew Wilcox @ 2020-11-11 0:52 UTC (permalink / raw)
To: Kent Overstreet; +Cc: linux-kernel, linux-fsdevel, akpm, sfrench, linux-cifs
On Wed, Oct 21, 2020 at 03:57:44PM -0400, Kent Overstreet wrote:
> This is just open coding add_to_page_cache(), and the next patch will
> delete add_to_page_cache_locked().
>
> Signed-off-by: Kent Overstreet <kent.overstreet@gmail.com>
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-11 0:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 19:57 [PATCH v2 0/2] kill add_to_page_cache_locked() Kent Overstreet
2020-10-21 19:57 ` [PATCH v2 1/2] cifs: convert to add_to_page_cache() Kent Overstreet
2020-10-22 8:36 ` Steve French
2020-11-11 0:52 ` Matthew Wilcox
2020-10-21 19:57 ` [PATCH v2 2/2] fs: kill add_to_page_cache_locked() Kent Overstreet
2020-10-21 20:51 ` Matthew Wilcox
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).