All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Evgeniy Dushistov <dushistov@mail.ru>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Ira Weiny <ira.weiny@intel.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3] fs/ufs: Replace kmap() with kmap_local_page()
Date: Mon, 16 May 2022 23:59:15 +0200	[thread overview]
Message-ID: <2117615.Icojqenx9y@leap> (raw)
In-Reply-To: <YoJl+lh0QELbv/TL@casper.infradead.org>

On lunedì 16 maggio 2022 16:55:54 CEST Matthew Wilcox wrote:
> On Mon, May 16, 2022 at 12:19:25PM +0200, Fabio M. De Francesco wrote:
> > The use of kmap() is being deprecated in favor of kmap_local_page(). 
With
> > kmap_local_page(), the mapping is per thread, CPU local and not 
globally
> > visible.
> > 
> > The usage of kmap_local_page() in fs/ufs is pre-thread, therefore 
replace
> > kmap() / kunmap() calls with kmap_local_page() / kunmap_local().
> > 
> > kunmap_local() requires the mapping address, so return that address 
from
> > ufs_get_page() to be used in ufs_put_page().
> > 
> > These changes are essentially ported from fs/ext2 and are largely based 
on
> > commit 782b76d7abdf ("fs/ext2: Replace kmap() with kmap_local_page()").
> > 
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> 
> Have you done more than compile-tested this?  I'd like to know that it's
> been tested on a machine with HIGHMEM enabled (in a VM, presumably).
> UFS doesn't get a lot of testing, and it'd be annoying to put out a
> patch that breaks the kmap_local() rules.
> 
No, I have not done more than compile-testing. 

However, I understand your concerns regarding these changes. I can only say
that, while they may seem like mechanical replacements, I have carefully
checked the code to be sure enough not to break the logic of the UFS and /
or the rules of local mapping.

I have nothing against testing, but I think they are not needed here unless
you see something that is potentially harmful or suspiciously broken and 
you explicitly request it. If so, I'll be testing the code by the end of
this week (I cannot before).

Thanks,

Fabio




  parent reply	other threads:[~2022-05-16 21:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-16 10:19 [PATCH v3] fs/ufs: Replace kmap() with kmap_local_page() Fabio M. De Francesco
2022-05-16 14:55 ` Matthew Wilcox
2022-05-16 19:53   ` Ira Weiny
2022-05-16 21:59   ` Fabio M. De Francesco [this message]
2022-05-25 15:12   ` Ira Weiny
2022-08-02  7:06   ` Fabio M. De Francesco
2022-08-03 19:04     ` 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=2117615.Icojqenx9y@leap \
    --to=fmdefrancesco@gmail.com \
    --cc=dushistov@mail.ru \
    --cc=ira.weiny@intel.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.