linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@infradead.org>
To: NeilBrown <neilb@suse.de>
Cc: Trond Myklebust <trond.myklebust@hammerspace.com>,
	Anna Schumaker <anna.schumaker@netapp.com>,
	Chuck Lever <chuck.lever@oracle.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mel Gorman <mgorman@suse.de>,
	Christoph Hellwig <hch@infradead.org>,
	David Howells <dhowells@redhat.com>,
	linux-nfs@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 03/18] MM: use ->swap_rw for reads from SWP_FS_OPS swap-space
Date: Tue, 21 Dec 2021 00:40:16 -0800	[thread overview]
Message-ID: <YcGS8IMk4li9qYHC@infradead.org> (raw)
In-Reply-To: <163969850289.20885.1044395970457169316.stgit@noble.brown>

> +int sio_pool_init(void)
> +{
> +	if (!sio_pool)
> +		sio_pool = mempool_create_kmalloc_pool(
> +			SWAP_CLUSTER_MAX, sizeof(struct swap_iocb));

I can't see anything serializing access here, so we'll need a lock or
cmpxchg dance.

> +	if (sio_pool)
> +		return 0;
> +	else
> +		return -ENOMEM;

Nit: This would flow much nicer as:

	if (!sio_pool)
		return -ENOMEM;
	return 0;

>  int swap_readpage(struct page *page, bool synchronous)
>  {
>  	struct bio *bio;
> @@ -378,13 +412,25 @@ int swap_readpage(struct page *page, bool synchronous)
>  	}
>  
>  	if (data_race(sis->flags & SWP_FS_OPS)) {
> -		//struct file *swap_file = sis->swap_file;
> -		//struct address_space *mapping = swap_file->f_mapping;

This should not be left by the previous patch.  In fact I suspect the
part of the previous patch that adds ->swap_rw should probably be folded
into this patch.

> +		struct file *swap_file = sis->swap_file;
> +		struct address_space *mapping = swap_file->f_mapping;
> +		struct iov_iter from;
> +		struct swap_iocb *sio;
> +		loff_t pos = page_file_offset(page);
> +
> +		sio = mempool_alloc(sio_pool, GFP_KERNEL);
> +		init_sync_kiocb(&sio->iocb, swap_file);
> +		sio->iocb.ki_pos = pos;
> +		sio->iocb.ki_complete = sio_read_complete;
> +		sio->bvec.bv_page = page;
> +		sio->bvec.bv_len = PAGE_SIZE;
> +		sio->bvec.bv_offset = 0;
> +
> +		iov_iter_bvec(&from, READ, &sio->bvec, 1, PAGE_SIZE);
> +		ret = mapping->a_ops->swap_rw(&sio->iocb, &from);
> +		if (ret != -EIOCBQUEUED)
> +			sio_read_complete(&sio->iocb, ret);
>  
>  		goto out;

I'd be tempted to split the SWP_FS_OPS into a helper to keep the
code tidy.


  parent reply	other threads:[~2021-12-21  8:40 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-16 23:48 [PATCH 00/18 V2] Repair SWAP-over-NFS NeilBrown
2021-12-16 23:48 ` [PATCH 03/18] MM: use ->swap_rw for reads from SWP_FS_OPS swap-space NeilBrown
2021-12-20 12:16   ` Mark Hemment
2021-12-21  8:40   ` Christoph Hellwig [this message]
2021-12-16 23:48 ` [PATCH 04/18] MM: perform async writes to " NeilBrown
2021-12-21  8:41   ` Christoph Hellwig
2021-12-16 23:48 ` [PATCH 02/18] MM: create new mm/swap.h header file NeilBrown
2021-12-17 10:03   ` kernel test robot
2021-12-21  8:36   ` Christoph Hellwig
2021-12-16 23:48 ` [PATCH 05/18] MM: reclaim mustn't enter FS for SWP_FS_OPS swap-space NeilBrown
2021-12-17  8:51   ` kernel test robot
2021-12-21  8:43   ` Christoph Hellwig
2021-12-16 23:48 ` [PATCH 06/18] MM: submit multipage reads " NeilBrown
2021-12-17  7:09   ` kernel test robot
2021-12-21  8:44   ` Christoph Hellwig
2021-12-16 23:48 ` [PATCH 01/18] Structural cleanup for filesystem-based swap NeilBrown
2021-12-17 10:33   ` kernel test robot
2021-12-21  8:34   ` Christoph Hellwig
2021-12-16 23:48 ` [PATCH 11/18] SUNRPC/call_alloc: async tasks mustn't block waiting for memory NeilBrown
2021-12-16 23:48 ` [PATCH 18/18] NFS: swap-out must always use STABLE writes NeilBrown
2021-12-16 23:48 ` [PATCH 15/18] NFS: discard NFS_RPC_SWAPFLAGS and RPC_TASK_ROOTCREDS NeilBrown
2021-12-16 23:48 ` [PATCH 10/18] NFS: swap IO handling is slightly different for O_DIRECT IO NeilBrown
2021-12-20 15:02   ` Mark Hemment
2021-12-16 23:48 ` [PATCH 08/18] MM: Add AS_CAN_DIO mapping flag NeilBrown
2021-12-19 13:38   ` Mark Hemment
2021-12-19 20:59     ` NeilBrown
2021-12-21  8:46   ` Christoph Hellwig
2022-01-19  3:54     ` NeilBrown
2021-12-16 23:48 ` [PATCH 14/18] SUNRPC: remove scheduling boost for "SWAPPER" tasks NeilBrown
2021-12-16 23:48 ` [PATCH 12/18] SUNRPC/auth: async tasks mustn't block waiting for memory NeilBrown
2021-12-16 23:48 ` [PATCH 09/18] NFS: rename nfs_direct_IO and use as ->swap_rw NeilBrown
2021-12-16 23:48 ` [PATCH 13/18] SUNRPC/xprt: async tasks mustn't block waiting for memory NeilBrown
2021-12-16 23:48 ` [PATCH 07/18] MM: submit multipage write for SWP_FS_OPS swap-space NeilBrown
2021-12-20 12:21   ` Mark Hemment
2021-12-16 23:48 ` [PATCH 17/18] NFSv4: keep state manager thread active if swap is enabled NeilBrown
2021-12-16 23:48 ` [PATCH 16/18] SUNRPC: improve 'swap' handling: scheduling and PF_MEMALLOC NeilBrown
2021-12-17 21:29 ` [PATCH 00/18 V2] Repair SWAP-over-NFS Anna Schumaker
2021-12-19 21:07   ` NeilBrown
2021-12-21  8:48     ` Christoph Hellwig

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=YcGS8IMk4li9qYHC@infradead.org \
    --to=hch@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=anna.schumaker@netapp.com \
    --cc=chuck.lever@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=neilb@suse.de \
    --cc=trond.myklebust@hammerspace.com \
    /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 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).