Re: [PATCH] fs: reduce pointers while using file_ra_state_init()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 27 Jul 2021, Goldwyn Rodrigues wrote:
> Simplification.
> 
> file_ra_state_init() take struct address_space *, just to use inode
> pointer by dereferencing from mapping->host.
> 
> The callers also derive mapping either by file->f_mapping, or
> even file->f_mapping->host->i_mapping.
> 
> Change file_ra_state_init() to accept struct inode * to reduce pointer
> dereferencing, both in the callee and the caller.
> 
> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>
> ---
....

> diff --git a/mm/readahead.c b/mm/readahead.c
> index d589f147f4c2..3541941df5e7 100644
> --- a/mm/readahead.c
> +++ b/mm/readahead.c
> @@ -31,9 +31,9 @@
>   * memset *ra to zero.
>   */
>  void
> -file_ra_state_init(struct file_ra_state *ra, struct address_space *mapping)
> +file_ra_state_init(struct file_ra_state *ra, struct inode *inode)
>  {
> -	ra->ra_pages = inode_to_bdi(mapping->host)->ra_pages;
> +	ra->ra_pages = inode_to_bdi(inode)->ra_pages;
>  	ra->prev_pos = -1;

I think this patch can be made OK by adding:

  if (unlikely(inode->i_mapping != &inode->i_data))
	inode = inode->i_mapping->host;

The "unlikely" is mostly for documentation.
Loading "inode->i_mapping" is nearly free as that cache line needs to be
loaded to get i_sb, which inode_to_bdi() needs.  Calculating &->i_data
is trivial.  So this adds minimal cost, and preserves correctness.

NeilBrown


>  }
>  EXPORT_SYMBOL_GPL(file_ra_state_init);
> -- 
> 2.32.0
> 
> 
> -- 
> Goldwyn
> 
> 



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux