Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
From:   "Fabio M. De Francesco" <fabio.maria.de.francesco@linux.intel.com>
To:     linux-fsdevel@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>
Cc:     linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] minixfs: change the signature of dir_get_page()
Date:   Thu, 14 Dec 2023 12:58:24 +0100
Message-ID: <4222956.ElGaqSPkdT@fdefranc-mobl3>
Organization: intel
In-Reply-To: <20231213000849.2748576-2-viro@zeniv.linux.org.uk>
References: <20231213000656.GI1674809@ZenIV>
 <20231213000849.2748576-1-viro@zeniv.linux.org.uk>
 <20231213000849.2748576-2-viro@zeniv.linux.org.uk>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Wednesday, 13 December 2023 01:08:47 CET Al Viro wrote:
> Change the signature of dir_get_page() in order to prepare this function
> to the conversion to the use of kmap_local_page(). Change also those call
> sites which are required to adjust to the new signature.
> 
> Essentially a copy of the corresponding fs/sysv commit by
> Fabio M. De Francesco <fmdefrancesco@gmail.com>
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/minix/dir.c | 46 ++++++++++++++++++++--------------------------
>  1 file changed, 20 insertions(+), 26 deletions(-)

Reviewed-by: Fabio M. De Francesco <fabio.maria.de.francesco@linux.intel.com>

> diff --git a/fs/minix/dir.c b/fs/minix/dir.c
> index 34a5d17f0796..4e5483adea40 100644
> --- a/fs/minix/dir.c
> +++ b/fs/minix/dir.c
> @@ -70,13 +70,15 @@ static int minix_handle_dirsync(struct inode *dir)
>  	return err;
>  }
> 
> -static struct page * dir_get_page(struct inode *dir, unsigned long n)
> +static void *dir_get_page(struct inode *dir, unsigned long n, struct page
> **p) {
>  	struct address_space *mapping = dir->i_mapping;
>  	struct page *page = read_mapping_page(mapping, n, NULL);
> -	if (!IS_ERR(page))
> -		kmap(page);
> -	return page;
> +	if (IS_ERR(page))
> +		return ERR_CAST(page);
> +	kmap(page);
> +	*p = page;
> +	return page_address(page);
>  }
> 
>  static inline void *minix_next_entry(void *de, struct minix_sb_info *sbi)
> @@ -104,11 +106,11 @@ static int minix_readdir(struct file *file, struct
> dir_context *ctx)
> 
>  	for ( ; n < npages; n++, offset = 0) {
>  		char *p, *kaddr, *limit;
> -		struct page *page = dir_get_page(inode, n);
> +		struct page *page;
> 
> -		if (IS_ERR(page))
> +		kaddr = dir_get_page(inode, n, &page);
> +		if (IS_ERR(kaddr))
>  			continue;
> -		kaddr = (char *)page_address(page);
>  		p = kaddr+offset;
>  		limit = kaddr + minix_last_byte(inode, n) - chunk_size;
>  		for ( ; p <= limit; p = minix_next_entry(p, sbi)) {
> @@ -173,11 +175,10 @@ minix_dirent *minix_find_entry(struct dentry *dentry,
> struct page **res_page) for (n = 0; n < npages; n++) {
>  		char *kaddr, *limit;
> 
> -		page = dir_get_page(dir, n);
> -		if (IS_ERR(page))
> +		kaddr = dir_get_page(dir, n, &page);
> +		if (IS_ERR(kaddr))
>  			continue;
> 
> -		kaddr = (char*)page_address(page);
>  		limit = kaddr + minix_last_byte(dir, n) - sbi->s_dirsize;
>  		for (p = kaddr; p <= limit; p = minix_next_entry(p, sbi)) {
>  			if (sbi->s_version == MINIX_V3) {
> @@ -229,12 +230,10 @@ int minix_add_link(struct dentry *dentry, struct inode
> *inode) for (n = 0; n <= npages; n++) {
>  		char *limit, *dir_end;
> 
> -		page = dir_get_page(dir, n);
> -		err = PTR_ERR(page);
> -		if (IS_ERR(page))
> -			goto out;
> +		kaddr = dir_get_page(dir, n, &page);
> +		if (IS_ERR(kaddr))
> +			return PTR_ERR(kaddr);
>  		lock_page(page);
> -		kaddr = (char*)page_address(page);
>  		dir_end = kaddr + minix_last_byte(dir, n);
>  		limit = kaddr + PAGE_SIZE - sbi->s_dirsize;
>  		for (p = kaddr; p <= limit; p = minix_next_entry(p, sbi)) {
> @@ -286,7 +285,6 @@ int minix_add_link(struct dentry *dentry, struct inode
> *inode) err = minix_handle_dirsync(dir);
>  out_put:
>  	dir_put_page(page);
> -out:
>  	return err;
>  out_unlock:
>  	unlock_page(page);
> @@ -375,11 +373,10 @@ int minix_empty_dir(struct inode * inode)
>  	for (i = 0; i < npages; i++) {
>  		char *p, *kaddr, *limit;
> 
> -		page = dir_get_page(inode, i);
> -		if (IS_ERR(page))
> +		kaddr = dir_get_page(inode, i, &page);
> +		if (IS_ERR(kaddr))
>  			continue;
> 
> -		kaddr = (char *)page_address(page);
>  		limit = kaddr + minix_last_byte(inode, i) - sbi->s_dirsize;
>  		for (p = kaddr; p <= limit; p = minix_next_entry(p, sbi)) {
>  			if (sbi->s_version == MINIX_V3) {
> @@ -441,15 +438,12 @@ int minix_set_link(struct minix_dir_entry *de, struct
> page *page,
> 
>  struct minix_dir_entry * minix_dotdot (struct inode *dir, struct page **p)
>  {
> -	struct page *page = dir_get_page(dir, 0);
>  	struct minix_sb_info *sbi = minix_sb(dir->i_sb);
> -	struct minix_dir_entry *de = NULL;
> +	struct minix_dir_entry *de = dir_get_page(dir, 0, p);
> 
> -	if (!IS_ERR(page)) {
> -		de = minix_next_entry(page_address(page), sbi);
> -		*p = page;
> -	}
> -	return de;
> +	if (!IS_ERR(de))
> +		return minix_next_entry(de, sbi);
> +	return NULL;
>  }
> 
>  ino_t minix_inode_by_name(struct dentry *dentry)