From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Al Viro <viro@zeniv.linux.org.uk>,
Dave Chinner <dchinner@redhat.com>,
Christian Brauner <brauner@kernel.org>,
Chen Zhongjin <chenzhongjin@huawei.com>,
Andrew Morton <akpm@linux-foundation.org>,
"Matthew Wilcox (Oracle)" <willy@infradead.org>,
Alexander Potapenko <glider@google.com>,
Andrey Konovalov <andreyknvl@gmail.com>,
Bagas Sanjaya <bagasdotme@gmail.com>,
Jiaqi Yan <jiaqiyan@google.com>, Tony Luck <tony.luck@intel.com>,
Peter Collingbourne <pcc@google.com>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [RFC PATCH] fs: Rename put_and_unmap_page() to unmap_and_put_page()
Date: Fri, 02 Jun 2023 12:51:31 +0200 [thread overview]
Message-ID: <4859757.31r3eYUQgx@suse> (raw)
In-Reply-To: <20230601132317.13606-1-fmdefrancesco@gmail.com>
On giovedì 1 giugno 2023 15:23:17 CEST Fabio M. De Francesco wrote:
> With commit 849ad04cf562a ("new helper: put_and_unmap_page()"), Al Viro
> introduced the put_and_unmap_page() to use in those many places where we
> have a common pattern consisting of calls to kunmap_local() +
> put_page().
>
> Obviously, first we unmap and then we put pages. Instead, the original
> name of this helper seems to imply that we first put and then unmap.
>
> Therefore, rename the helper and change the only known upstreamed user
> (i.e., fs/sysv) before this helper enters common use and might become
> difficult to find all call sites and break the Kernel builds.
>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
>
> This is an RFC
Please discard this RFC.
I just sent the real patch at
https://lore.kernel.org/linux-fsdevel/20230602103307.5637-1-fmdefrancesco@gmail.com/T/#u
and added linux-mm to the list of recipients.
Thanks,
Fabio
> because I'm pretty sure that Al must have been a good
> reason to use this counter intuitive name for his helper. I'm not
> probably aware of some obscure helpers names convention.
>
> Furthermore, I know that Al's VFS tree has already used this helper at
> least in fs/minix and probably in other filesystems.
>
> Therefore I didn't want to send a "real" patch.
>
> I'm looking forward to hearing from people involved with fs and
> especially from Al before sending a real patch or throwing it away.
>
> fs/sysv/dir.c | 22 +++++++++++-----------
> fs/sysv/namei.c | 8 ++++----
> include/linux/highmem.h | 2 +-
> 3 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/fs/sysv/dir.c b/fs/sysv/dir.c
> index cdb3d632c63d..d6a3bbb550c3 100644
> --- a/fs/sysv/dir.c
> +++ b/fs/sysv/dir.c
> @@ -52,7 +52,7 @@ static int sysv_handle_dirsync(struct inode *dir)
> }
>
> /*
> - * Calls to dir_get_page()/put_and_unmap_page() must be nested according to
> the + * Calls to dir_get_page()/unmap_and_put_page() must be nested
according
> to the * rules documented in mm/highmem.rst.
> *
> * NOTE: sysv_find_entry() and sysv_dotdot() act as calls to dir_get_page()
> @@ -103,11 +103,11 @@ static int sysv_readdir(struct file *file, struct
> dir_context *ctx) if (!dir_emit(ctx, name, strnlen(name,SYSV_NAMELEN),
> fs16_to_cpu(SYSV_SB(sb), de-
>inode),
> DT_UNKNOWN)) {
> - put_and_unmap_page(page, kaddr);
> + unmap_and_put_page(page, kaddr);
> return 0;
> }
> }
> - put_and_unmap_page(page, kaddr);
> + unmap_and_put_page(page, kaddr);
> }
> return 0;
> }
> @@ -131,7 +131,7 @@ static inline int namecompare(int len, int maxlen,
> * itself (as a parameter - res_dir). It does NOT read the inode of the
> * entry - you'll have to do that yourself if you want to.
> *
> - * On Success put_and_unmap_page() should be called on *res_page.
> + * On Success unmap_iand_put_page() should be called on *res_page.
> *
> * sysv_find_entry() acts as a call to dir_get_page() and must be treated
> * accordingly for nesting purposes.
> @@ -166,7 +166,7 @@ struct sysv_dir_entry *sysv_find_entry(struct dentry
> *dentry, struct page **res_ name, de->name))
> goto found;
> }
> - put_and_unmap_page(page, kaddr);
> + unmap_and_put_page(page, kaddr);
> }
>
> if (++n >= npages)
> @@ -209,7 +209,7 @@ int sysv_add_link(struct dentry *dentry, struct inode
> *inode) goto out_page;
> de++;
> }
> - put_and_unmap_page(page, kaddr);
> + unmap_and_put_page(page, kaddr);
> }
> BUG();
> return -EINVAL;
> @@ -228,7 +228,7 @@ int sysv_add_link(struct dentry *dentry, struct inode
> *inode) mark_inode_dirty(dir);
> err = sysv_handle_dirsync(dir);
> out_page:
> - put_and_unmap_page(page, kaddr);
> + unmap_and_put_page(page, kaddr);
> return err;
> out_unlock:
> unlock_page(page);
> @@ -321,12 +321,12 @@ int sysv_empty_dir(struct inode * inode)
> if (de->name[1] != '.' || de->name[2])
> goto not_empty;
> }
> - put_and_unmap_page(page, kaddr);
> + unmap_and_put_page(page, kaddr);
> }
> return 1;
>
> not_empty:
> - put_and_unmap_page(page, kaddr);
> + unmap_iand_put_page(page, kaddr);
> return 0;
> }
>
> @@ -352,7 +352,7 @@ int sysv_set_link(struct sysv_dir_entry *de, struct page
> *page, }
>
> /*
> - * Calls to dir_get_page()/put_and_unmap_page() must be nested according to
> the + * Calls to dir_get_page()/unmap_and_put_page() must be nested
according
> to the * rules documented in mm/highmem.rst.
> *
> * sysv_dotdot() acts as a call to dir_get_page() and must be treated
> @@ -376,7 +376,7 @@ ino_t sysv_inode_by_name(struct dentry *dentry)
>
> if (de) {
> res = fs16_to_cpu(SYSV_SB(dentry->d_sb), de->inode);
> - put_and_unmap_page(page, de);
> + unmap_and_put_page(page, de);
> }
> return res;
> }
> diff --git a/fs/sysv/namei.c b/fs/sysv/namei.c
> index 2b2dba4c4f56..fcf163fea3ad 100644
> --- a/fs/sysv/namei.c
> +++ b/fs/sysv/namei.c
> @@ -164,7 +164,7 @@ static int sysv_unlink(struct inode * dir, struct dentry
*
> dentry) inode->i_ctime = dir->i_ctime;
> inode_dec_link_count(inode);
> }
> - put_and_unmap_page(page, de);
> + unmap_and_put_page(page, de);
> return err;
> }
>
> @@ -227,7 +227,7 @@ static int sysv_rename(struct mnt_idmap *idmap, struct
> inode *old_dir, if (!new_de)
> goto out_dir;
> err = sysv_set_link(new_de, new_page, old_inode);
> - put_and_unmap_page(new_page, new_de);
> + unmap_and_put_page(new_page, new_de);
> if (err)
> goto out_dir;
> new_inode->i_ctime = current_time(new_inode);
> @@ -256,9 +256,9 @@ static int sysv_rename(struct mnt_idmap *idmap, struct
> inode *old_dir,
>
> out_dir:
> if (dir_de)
> - put_and_unmap_page(dir_page, dir_de);
> + unmap_and_put_page(dir_page, dir_de);
> out_old:
> - put_and_unmap_page(old_page, old_de);
> + unmap_and_put_page(old_page, old_de);
> out:
> return err;
> }
> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
> index 4de1dbcd3ef6..68da30625a6c 100644
> --- a/include/linux/highmem.h
> +++ b/include/linux/highmem.h
> @@ -507,7 +507,7 @@ static inline void folio_zero_range(struct folio *folio,
> zero_user_segments(&folio->page, start, start + length, 0, 0);
> }
>
> -static inline void put_and_unmap_page(struct page *page, void *addr)
> +static inline void unmap_and_put_page(struct page *page, void *addr)
> {
> kunmap_local(addr);
> put_page(page);
> --
> 2.40.1
prev parent reply other threads:[~2023-06-02 10:51 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-01 13:23 [RFC PATCH] fs: Rename put_and_unmap_page() to unmap_and_put_page() Fabio M. De Francesco
2023-06-02 10:51 ` Fabio M. De Francesco [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4859757.31r3eYUQgx@suse \
--to=fmdefrancesco@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@gmail.com \
--cc=bagasdotme@gmail.com \
--cc=brauner@kernel.org \
--cc=chenzhongjin@huawei.com \
--cc=dchinner@redhat.com \
--cc=glider@google.com \
--cc=jiaqiyan@google.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pcc@google.com \
--cc=tony.luck@intel.com \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).