[rh7] NFS: Don't call COMMIT in ->releasepage()

Submitted by Andrey Ryabinin on Dec. 1, 2017, 1:15 p.m.

Details

Message ID 20171201131500.9437-1-aryabinin@virtuozzo.com
State New
Series "NFS: Don't call COMMIT in ->releasepage()"
Headers show

Commit Message

Andrey Ryabinin Dec. 1, 2017, 1:15 p.m.
From: Trond Myklebust <trond.myklebust@primarydata.com>

While COMMIT has the potential to free up a lot of memory that is being
taken by unstable writes, it isn't guaranteed to free up this particular
page. Also, calling fsync() on the server is expensive and so we want to
do it in a more controlled fashion, rather than have it triggered at
random by the VM.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>

https://jira.sw.ru/browse/PSBM-77949
(cherry picked from commit 4f52b6bb8c57b9accafad526a429d6c0851cc62f)
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 fs/nfs/file.c | 23 -----------------------
 1 file changed, 23 deletions(-)

Patch hide | download patch | download mbox

diff --git a/fs/nfs/file.c b/fs/nfs/file.c
index 7ad044976fd1..24d3d0c44bc4 100644
--- a/fs/nfs/file.c
+++ b/fs/nfs/file.c
@@ -470,31 +470,8 @@  static void nfs_invalidate_page(struct page *page, unsigned int offset,
  */
 static int nfs_release_page(struct page *page, gfp_t gfp)
 {
-	struct address_space *mapping = page->mapping;
-
 	dfprintk(PAGECACHE, "NFS: release_page(%p)\n", page);
 
-	/* Always try to initiate a 'commit' if relevant, but only
-	 * wait for it if __GFP_WAIT is set.  Even then, only wait 1
-	 * second and only if the 'bdi' is not congested.
-	 * Waiting indefinitely can cause deadlocks when the NFS
-	 * server is on this machine, when a new TCP connection is
-	 * needed and in other rare cases.  There is no particular
-	 * need to wait extensively here.  A short wait has the
-	 * benefit that someone else can worry about the freezer.
-	 */
-	if (mapping) {
-		struct nfs_server *nfss = NFS_SERVER(mapping->host);
-		nfs_commit_inode(mapping->host, 0);
-		if ((gfp & __GFP_WAIT) &&
-		    !bdi_write_congested(&nfss->backing_dev_info)) {
-			wait_on_page_bit_killable_timeout(page, PG_private,
-							  HZ);
-			if (PagePrivate(page))
-				set_bdi_congested(&nfss->backing_dev_info,
-						  BLK_RW_ASYNC);
-		}
-	}
 	/* If PagePrivate() is set, then the page is not freeable */
 	if (PagePrivate(page))
 		return 0;

Comments

Konstantin Khorenko Dec. 7, 2017, 11:09 a.m.
Please consider to RK this.

https://readykernel.com/

--
Best regards,

Konstantin Khorenko,
Virtuozzo Linux Kernel Team

On 12/01/2017 04:15 PM, Andrey Ryabinin wrote:
> From: Trond Myklebust <trond.myklebust@primarydata.com>
>
> While COMMIT has the potential to free up a lot of memory that is being
> taken by unstable writes, it isn't guaranteed to free up this particular
> page. Also, calling fsync() on the server is expensive and so we want to
> do it in a more controlled fashion, rather than have it triggered at
> random by the VM.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
>
> https://jira.sw.ru/browse/PSBM-77949
> (cherry picked from commit 4f52b6bb8c57b9accafad526a429d6c0851cc62f)
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  fs/nfs/file.c | 23 -----------------------
>  1 file changed, 23 deletions(-)
>
> diff --git a/fs/nfs/file.c b/fs/nfs/file.c
> index 7ad044976fd1..24d3d0c44bc4 100644
> --- a/fs/nfs/file.c
> +++ b/fs/nfs/file.c
> @@ -470,31 +470,8 @@ static void nfs_invalidate_page(struct page *page, unsigned int offset,
>   */
>  static int nfs_release_page(struct page *page, gfp_t gfp)
>  {
> -	struct address_space *mapping = page->mapping;
> -
>  	dfprintk(PAGECACHE, "NFS: release_page(%p)\n", page);
>
> -	/* Always try to initiate a 'commit' if relevant, but only
> -	 * wait for it if __GFP_WAIT is set.  Even then, only wait 1
> -	 * second and only if the 'bdi' is not congested.
> -	 * Waiting indefinitely can cause deadlocks when the NFS
> -	 * server is on this machine, when a new TCP connection is
> -	 * needed and in other rare cases.  There is no particular
> -	 * need to wait extensively here.  A short wait has the
> -	 * benefit that someone else can worry about the freezer.
> -	 */
> -	if (mapping) {
> -		struct nfs_server *nfss = NFS_SERVER(mapping->host);
> -		nfs_commit_inode(mapping->host, 0);
> -		if ((gfp & __GFP_WAIT) &&
> -		    !bdi_write_congested(&nfss->backing_dev_info)) {
> -			wait_on_page_bit_killable_timeout(page, PG_private,
> -							  HZ);
> -			if (PagePrivate(page))
> -				set_bdi_congested(&nfss->backing_dev_info,
> -						  BLK_RW_ASYNC);
> -		}
> -	}
>  	/* If PagePrivate() is set, then the page is not freeable */
>  	if (PagePrivate(page))
>  		return 0;
>