linux-kernel - Re: [RFC 1/2] shmem: set shmem

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: <Y+HLFWaHq11MQ91U@casper.infradead.org>
Date:   Tue, 7 Feb 2023 03:52:53 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     Luis Chamberlain <mcgrof@...nel.org>
Cc:     hughd@...gle.com, akpm@...ux-foundation.org, linux-mm@...ck.org,
        p.raghav@...sung.com, dave@...olabs.net, a.manzanares@...sung.com,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC 1/2] shmem: set shmem_writepage() variables early

On Mon, Feb 06, 2023 at 06:52:58PM -0800, Luis Chamberlain wrote:
> shmem_writepage() sets up variables typically used *after* a possible
> huge page split. However even if that does happen the address space
> mapping should not change. So it should be safe to set that from
> the beginning.

Yes, we can get mapping from the folio early.  It doesn't change
on split.

> The folio should always be locked from the start as well. It however
> was not clear if the folio address can / should change, as well as
> the first inode.
> 
> This commit makes no functional changes other a double check on the
> folio locking which might be superflous. This change should help make
> the subsequent patch easier to review.

You don't need to check that the folio's locked, and you don't
need to reassign inode after the split.

> Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
> ---
>  mm/shmem.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 28f3c699c8ce..a2c6aa11aab8 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1332,11 +1332,13 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc)
>  {
>  	struct folio *folio = page_folio(page);
>  	struct shmem_inode_info *info;
> -	struct address_space *mapping;
> -	struct inode *inode;
> +	struct address_space *mapping = folio->mapping;
> +	struct inode *inode = mapping->host;
>  	swp_entry_t swap;
>  	pgoff_t index;
>  
> +	BUG_ON(!folio_test_locked(folio));
> +
>  	/*
>  	 * If /sys/kernel/mm/transparent_hugepage/shmem_enabled is "always" or
>  	 * "force", drivers/gpu/drm/i915/gem/i915_gem_shmem.c gets huge pages,
> @@ -1351,8 +1353,8 @@ static int shmem_writepage(struct page *page, struct writeback_control *wbc)
>  		folio_clear_dirty(folio);
>  	}
>  
> +	/* Can the folio or first inode change on after a split? */
>  	BUG_ON(!folio_test_locked(folio));
> -	mapping = folio->mapping;
>  	index = folio->index;
>  	inode = mapping->host;
>  	info = SHMEM_I(inode);
> -- 
> 2.39.0
>