Hi John. Thanks for the patch, and the testing, and the reference to the lwn article. I have also applied your patch to 5.7-rc7 and have run xfstests. I applied your patch to the orangefs for-next tree. -Mike On Sat, May 30, 2020 at 3:39 AM John Hubbard <jhubbard@xxxxxxxxxx> wrote: > > On 2020-05-22 20:59, John Hubbard wrote: > > This code was using get_user_pages*(), in a "Case 1" scenario > > (Direct IO), using the categorization from [1]. That means that it's > > time to convert the get_user_pages*() + put_page() calls to > > pin_user_pages*() + unpin_user_pages() calls. > > > > There is some helpful background in [2]: basically, this is a small > > part of fixing a long-standing disconnect between pinning pages, and > > file systems' use of those pages. > > > > [1] Documentation/core-api/pin_user_pages.rst > > > > [2] "Explicit pinning of user-space pages": > > https://lwn.net/Articles/807108/ > > > > Cc: Mike Marshall <hubcap@xxxxxxxxxxxx> > > Cc: Martin Brandenburg <martin@xxxxxxxxxxxx> > > Cc: devel@xxxxxxxxxxxxxxxxxx > > Cc: linux-fsdevel@xxxxxxxxxxxxxxx > > Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx> > > --- > > > > Hi, > > > > Note that I have only compile-tested this patch, although that does > > also include cross-compiling for a few other arches. > > An update on the run-time testing: Just now, I got basic orangefs tests > running in xfstests, with this patch applied, and it all looks normal. > > thanks, > -- > John Hubbard > NVIDIA > > > > > Changes since v1 [3]: correct the commit description, so that > > it refers to "Case 1" instead of "Case 2". > > > > > > [3] https://lore.kernel.org/r/20200518060139.2828423-1-jhubbard@xxxxxxxxxx > > > > thanks, > > John Hubbard > > NVIDIA > > > > fs/orangefs/orangefs-bufmap.c | 9 +++------ > > 1 file changed, 3 insertions(+), 6 deletions(-) > > > > diff --git a/fs/orangefs/orangefs-bufmap.c b/fs/orangefs/orangefs-bufmap.c > > index 2bb916d68576..538e839590ef 100644 > > --- a/fs/orangefs/orangefs-bufmap.c > > +++ b/fs/orangefs/orangefs-bufmap.c > > @@ -168,10 +168,7 @@ static DEFINE_SPINLOCK(orangefs_bufmap_lock); > > static void > > orangefs_bufmap_unmap(struct orangefs_bufmap *bufmap) > > { > > - int i; > > - > > - for (i = 0; i < bufmap->page_count; i++) > > - put_page(bufmap->page_array[i]); > > + unpin_user_pages(bufmap->page_array, bufmap->page_count); > > } > > > > static void > > @@ -268,7 +265,7 @@ orangefs_bufmap_map(struct orangefs_bufmap *bufmap, > > int offset = 0, ret, i; > > > > /* map the pages */ > > - ret = get_user_pages_fast((unsigned long)user_desc->ptr, > > + ret = pin_user_pages_fast((unsigned long)user_desc->ptr, > > bufmap->page_count, FOLL_WRITE, bufmap->page_array); > > > > if (ret < 0) > > @@ -280,7 +277,7 @@ orangefs_bufmap_map(struct orangefs_bufmap *bufmap, > > > > for (i = 0; i < ret; i++) { > > SetPageError(bufmap->page_array[i]); > > - put_page(bufmap->page_array[i]); > > + unpin_user_page(bufmap->page_array[i]); > > } > > return -ENOMEM; > > } > > >
