linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [git pull] common helper for kmap_local_page() users in local filesystems
Date: Sat, 11 Mar 2023 18:11:01 +0100	[thread overview]
Message-ID: <8232398.NyiUUSuA9g@suse> (raw)
In-Reply-To: <20230310204431.GW3390869@ZenIV>

On venerdì 10 marzo 2023 21:44:31 CET Al Viro wrote:
> 	kmap_local_page() conversions in local filesystems keep running into
> kunmap_local_page()+put_page() combinations; we can keep inventing names
> for identical inline helpers, but it's getting rather inconvenient.  I've
> added a trivial helper to linux/highmem.h instead.

Yeah, "put_and_unmap_page()". Nice helper :-)

Today I decided to prepare a series to convert all the functions of all the 
filesystems where I had found the above-mentioned pattern but I stopped 
immediately after converting dir_put_page() in fs/sysv.

Why? Just because I realized that I do not understand the reasons behind the 
choice of the name of the helper...  

Why did you name it "put_and_unmap_page()" instead of "unmap_and_put_page()", 
for we always unmap first _and_ put the page immediately the unmapping?

It seems it want to imply that instead we put first and unmap later (which 
would be wrong). That name sounds misleading to me and not sound (logically 
speaking).

Am I missing some obscure convention behind your choice of that name for the 
helper?

If not, can you please change it from "put_and_unmap_page()" to 
"unmap_and_put_page()"? 

Thanks,

Fabio

> 
> 	I would've held that back until the merge window, if not for the
> mess it causes in tree topology - I've several branches merging from that
> one, and it's only going to get worse if e.g. ext2 stuff gets picked by
> Jan.
> 
> 	The helper is trivial and it's early in the cycle.  It would 
simplify
> the things if you could pull it - then I'd simply rebase the affected 
branches
> to -rc2...
> 
> The following changes since commit fe15c26ee26efa11741a7b632e9f23b01aca4cc6:
> 
>   Linux 6.3-rc1 (2023-03-05 14:52:03 -0800)
> 
> are available in the Git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git tags/pull-
highmem
> 
> for you to fetch changes up to 849ad04cf562ac63b0371a825eed473d84de9c6d:
> 
>   new helper: put_and_unmap_page() (2023-03-07 01:50:53 -0500)
> 
> ----------------------------------------------------------------
> put_and_unmap_page() helper
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> 
> ----------------------------------------------------------------
> Al Viro (1):
>       new helper: put_and_unmap_page()
> 
>  include/linux/highmem.h | 6 ++++++
>  1 file changed, 6 insertions(+)





  parent reply	other threads:[~2023-03-11 17:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-10 20:44 [git pull] common helper for kmap_local_page() users in local filesystems Al Viro
2023-03-11  4:52 ` pr-tracker-bot
2023-03-11 17:11 ` Fabio M. De Francesco [this message]
2023-03-27 10:20   ` Fabio M. De Francesco

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=8232398.NyiUUSuA9g@suse \
    --to=fmdefrancesco@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /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).