linux-kernel - Re: [PATCH] aio: add aio_migratepage_helper to get rid of duplicate code

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140210162402.GB26657@kvack.org>
Date:	Mon, 10 Feb 2014 11:24:02 -0500
From:	Benjamin LaHaise <bcrl@...ck.org>
To:	Rakesh Pandit <rakesh@...era.com>
Cc:	viro@...iv.linux.org.uk, linux-fsdevel@...r.kernel.org,
	linux-aio@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] aio: add aio_migratepage_helper to get rid of duplicate code

On Mon, Feb 10, 2014 at 01:52:56PM +0200, Rakesh Pandit wrote:

You're missing a commit message here providing justification for the changes 
submitted.  Why is this change a good thing?  Did it fix any bugs?

		-ben

> Signed-off-by: Rakesh Pandit <rakesh@...era.com>
> ---
>  fs/aio.c | 55 +++++++++++++++++++++++--------------------------------
>  1 file changed, 23 insertions(+), 32 deletions(-)
> 
> diff --git a/fs/aio.c b/fs/aio.c
> index d3d55dc..20d690a 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -278,32 +278,46 @@ static int aio_set_page_dirty(struct page *page)
>  }
>  
>  #if IS_ENABLED(CONFIG_MIGRATION)
> -static int aio_migratepage(struct address_space *mapping, struct page *new,
> -			struct page *old, enum migrate_mode mode)
> +static int aio_migratepage_helper(struct address_space *mapping,
> +			struct page *new, struct page *old, bool move)
>  {
>  	struct kioctx *ctx;
>  	unsigned long flags;
> -	int rc;
> -
> -	rc = 0;
> +	int rc = 0;
>  
> -	/* Make sure the old page hasn't already been changed */
> +	/* We can potentially race against kioctx teardown here.  Use the
> +	 * address_space's private data lock to protect the mapping's
> +	 * private_data.
> +	 */
>  	spin_lock(&mapping->private_lock);
>  	ctx = mapping->private_data;
>  	if (ctx) {
>  		pgoff_t idx;
>  		spin_lock_irqsave(&ctx->completion_lock, flags);
> +		if (move)
> +			migrate_page_copy(new, old);
>  		idx = old->index;
>  		if (idx < (pgoff_t)ctx->nr_pages) {
>  			if (ctx->ring_pages[idx] != old)
>  				rc = -EAGAIN;
> +			else if (move)
> +				ctx->ring_pages[idx] = new;
>  		} else
>  			rc = -EINVAL;
>  		spin_unlock_irqrestore(&ctx->completion_lock, flags);
>  	} else
> -		rc = -EINVAL;
> +		rc = move ? -EBUSY : -EINVAL;
>  	spin_unlock(&mapping->private_lock);
> +	return rc;
> +}
>  
> +static int aio_migratepage(struct address_space *mapping, struct page *new,
> +			struct page *old, enum migrate_mode mode)
> +{
> +	int rc = 0;
> +
> +	/* Make sure the old page hasn't already been changed */
> +	rc = aio_migratepage_helper(mapping, new, old, false);
>  	if (rc != 0)
>  		return rc;
>  
> @@ -317,31 +331,8 @@ static int aio_migratepage(struct address_space *mapping, struct page *new,
>  		return rc;
>  	}
>  
> -	/* We can potentially race against kioctx teardown here.  Use the
> -	 * address_space's private data lock to protect the mapping's
> -	 * private_data.
> -	 */
> -	spin_lock(&mapping->private_lock);
> -	ctx = mapping->private_data;
> -	if (ctx) {
> -		pgoff_t idx;
> -		spin_lock_irqsave(&ctx->completion_lock, flags);
> -		migrate_page_copy(new, old);
> -		idx = old->index;
> -		if (idx < (pgoff_t)ctx->nr_pages) {
> -			/* And only do the move if things haven't changed */
> -			if (ctx->ring_pages[idx] == old)
> -				ctx->ring_pages[idx] = new;
> -			else
> -				rc = -EAGAIN;
> -		} else
> -			rc = -EINVAL;
> -		spin_unlock_irqrestore(&ctx->completion_lock, flags);
> -	} else
> -		rc = -EBUSY;
> -	spin_unlock(&mapping->private_lock);
> -
> -	if (rc == MIGRATEPAGE_SUCCESS)
> +	rc = aio_migratepage_helper(mapping, new, old, true);
> +	if (rc == 0)
>  		put_page(old);
>  	else
>  		put_page(new);
> -- 
> 1.8.3.1

-- 
"Thought is the essence of where you are now."
--
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/