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@xxxxxxxxxx>
> ---
> 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
>
