| Message ID | 20230310204431.GW3390869@ZenIV (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | [git,pull] common helper for kmap_local_page() users in local filesystems | expand |
The pull request you sent on Fri, 10 Mar 2023 20:44:31 +0000:
> git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git tags/pull-highmem
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/d33d4c9e0888e9ee7666813b3f9ecc244a64d127
Thank you!
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(+)
On sabato 11 marzo 2023 18:11:01 CEST Fabio M. De Francesco wrote: > 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 :-) [snip] Hi Al, > 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? Can you please explain what I'm missing behind your motivation? Thanks, Fabio P.S.: Adding Ira to the Cc list, since he's been doing kmap() and kmap_atomic() conversions long time before I too started with them.
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. 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(+)