From: John Hubbard <jhubbard@nvidia.com>
To: Ira Weiny <ira.weiny@intel.com>, <john.hubbard@gmail.com>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Christoph Hellwig" <hch@infradead.org>,
"Dan Williams" <dan.j.williams@intel.com>,
"Dave Chinner" <david@fromorbit.com>,
"Dave Hansen" <dave.hansen@linux.intel.com>,
"Jan Kara" <jack@suse.cz>, "Jason Gunthorpe" <jgg@ziepe.ca>,
"Jérôme Glisse" <jglisse@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
amd-gfx@lists.freedesktop.org, ceph-devel@vger.kernel.org,
devel@driverdev.osuosl.org, devel@lists.orangefs.org,
dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-block@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-media@vger.kernel.org, linux-mm@kvack.org,
linux-nfs@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-rpi-kernel@lists.infradead.org, linux-xfs@vger.kernel.org,
netdev@vger.kernel.org, rds-devel@oss.oracle.com,
sparclinux@vger.kernel.org, x86@kernel.org,
xen-devel@lists.xenproject.org, "Christoph Hellwig" <hch@lst.de>,
"Matthew Wilcox" <willy@infradead.org>
Subject: Re: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
Date: Tue, 6 Aug 2019 13:39:16 -0700 [thread overview]
Message-ID: <0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com> (raw)
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
WARNING: multiple messages have this Message-ID (diff)
From: John Hubbard <jhubbard@nvidia.com>
To: Ira Weiny <ira.weiny@intel.com>, john.hubbard@gmail.com
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Christoph Hellwig" <hch@infradead.org>,
"Dan Williams" <dan.j.williams@intel.com>,
"Dave Chinner" <david@fromorbit.com>,
"Dave Hansen" <dave.hansen@linux.intel.com>,
"Jan Kara" <jack@suse.cz>, "Jason Gunthorpe" <jgg@ziepe.ca>,
"Jérôme Glisse" <jglisse@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
amd-gfx@lists.freedesktop.org, ceph-devel@vger.kernel.org,
devel@driverdev.osuosl.org, devel@lists.orangefs.org,
dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-block@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-media@vger.kernel.org, linux-mm@kvack.org,
linux-nfs@vger.kernel.org, linux-rdma@vger.kernel.org,
linux-rpi-kernel@lists.infradead.org, linux-xfs@vger.kernel.org,
netdev@vger.kernel.org, rds-devel@oss.oracle.com,
sparclinux@vger.kernel.org, x86@kernel.org,
xen-devel@lists.xenproject.org, "Christoph Hellwig" <hch@lst.de>,
"Matthew Wilcox" <willy@infradead.org>
Subject: Re: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
Date: Tue, 6 Aug 2019 13:39:16 -0700 [thread overview]
Message-ID: <0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com> (raw)
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
WARNING: multiple messages have this Message-ID (diff)
From: John Hubbard <jhubbard@nvidia.com>
To: Ira Weiny <ira.weiny@intel.com>, john.hubbard@gmail.com
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Christoph Hellwig" <hch@infradead.org>,
"Dan Williams" <dan.j.williams@intel.com>,
"Dave Chinner" <david@fromorbit.com>,
"Dave Hansen" <dave.hansen@linux.intel.com>,
"Jan Kara" <jack@suse.cz>, "Jason Gunthorpe" <jgg@ziepe.ca>,
"Jérôme Glisse" <jglisse@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
amd-gfx@lists.freedesktop.org, ceph-devel@vger.kernel.org,
devel@driverdev.osuosl.org, devel@lists.orangefs.org,
dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-block@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-media@vger.kernel.org, linux-mm@kva
Subject: Re: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
Date: Tue, 06 Aug 2019 20:39:16 +0000 [thread overview]
Message-ID: <0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com> (raw)
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
WARNING: multiple messages have this Message-ID (diff)
From: John Hubbard <jhubbard@nvidia.com>
To: Ira Weiny <ira.weiny@intel.com>, john.hubbard@gmail.com
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
"Christoph Hellwig" <hch@infradead.org>,
"Dan Williams" <dan.j.williams@intel.com>,
"Dave Chinner" <david@fromorbit.com>,
"Dave Hansen" <dave.hansen@linux.intel.com>,
"Jan Kara" <jack@suse.cz>, "Jason Gunthorpe" <jgg@ziepe.ca>,
"Jérôme Glisse" <jglisse@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
amd-gfx@lists.freedesktop.org, ceph-devel@vger.kernel.org,
devel@driverdev.osuosl.org, devel@lists.orangefs.org,
dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-block@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-media@vger.kernel.org, linux-mm@kva
Subject: Re: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
Date: Tue, 6 Aug 2019 13:39:16 -0700 [thread overview]
Message-ID: <0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com> (raw)
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
WARNING: multiple messages have this Message-ID (diff)
From: jhubbard@nvidia.com (John Hubbard)
Subject: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
Date: Tue, 6 Aug 2019 13:39:16 -0700 [thread overview]
Message-ID: <0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com> (raw)
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019@03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard at nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny at intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
WARNING: multiple messages have this Message-ID (diff)
From: John Hubbard <jhubbard@nvidia.com>
To: Ira Weiny <ira.weiny@intel.com>, <john.hubbard@gmail.com>
Cc: linux-fbdev@vger.kernel.org, "Jan Kara" <jack@suse.cz>,
kvm@vger.kernel.org, "Dave Hansen" <dave.hansen@linux.intel.com>,
"Dave Chinner" <david@fromorbit.com>,
dri-devel@lists.freedesktop.org, linux-mm@kvack.org,
"Matthew Wilcox" <willy@infradead.org>,
sparclinux@vger.kernel.org,
"Dan Williams" <dan.j.williams@intel.com>,
devel@driverdev.osuosl.org, rds-devel@oss.oracle.com,
linux-rdma@vger.kernel.org, x86@kernel.org,
amd-gfx@lists.freedesktop.org, "Christoph Hellwig" <hch@lst.de>,
"Christoph Hellwig" <hch@infradead.org>,
"Jason Gunthorpe" <jgg@ziepe.ca>,
xen-devel@lists.xenproject.org, devel@lists.orangefs.org,
linux-media@vger.kernel.org, intel-gfx@lists.freedesktop.org,
linux-block@vger.kernel.org, "Jérôme Glisse" <jglisse@redhat.com>,
linux-rpi-kernel@lists.infradead.org, ceph-devel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-nfs@vger.kernel.org,
netdev@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
linux-xfs@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-fsdevel@vger.kernel.org,
"Andrew Morton" <akpm@linux-foundation.org>
Subject: Re: [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
Date: Tue, 6 Aug 2019 13:39:16 -0700 [thread overview]
Message-ID: <0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com> (raw)
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
WARNING: multiple messages have this Message-ID (diff)
From: John Hubbard <jhubbard@nvidia.com>
To: Ira Weiny <ira.weiny@intel.com>, <john.hubbard@gmail.com>
Cc: linux-fbdev@vger.kernel.org, "Jan Kara" <jack@suse.cz>,
kvm@vger.kernel.org, "Dave Hansen" <dave.hansen@linux.intel.com>,
"Dave Chinner" <david@fromorbit.com>,
dri-devel@lists.freedesktop.org, linux-mm@kvack.org,
"Matthew Wilcox" <willy@infradead.org>,
sparclinux@vger.kernel.org,
"Dan Williams" <dan.j.williams@intel.com>,
devel@driverdev.osuosl.org, rds-devel@oss.oracle.com,
linux-rdma@vger.kernel.org, x86@kernel.org,
amd-gfx@lists.freedesktop.org, "Christoph Hellwig" <hch@lst.de>,
"Christoph Hellwig" <hch@infradead.org>,
"Jason Gunthorpe" <jgg@ziepe.ca>,
xen-devel@lists.xenproject.org, devel@lists.orangefs.org,
linux-media@vger.kernel.org, intel-gfx@lists.freedesktop.org,
linux-block@vger.kernel.org, "Jérôme Glisse" <jglisse@redhat.com>,
linux-rpi-kernel@lists.infradead.org, ceph-devel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-nfs@vger.kernel.org,
netdev@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
linux-xfs@vger.kernel.org, linux-crypto@vger.kernel.org,
linux-fsdevel@vger.kernel.org,
"Andrew Morton" <akpm@linux-foundation.org>
Subject: Re: [Xen-devel] [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock()
Date: Tue, 6 Aug 2019 13:39:16 -0700 [thread overview]
Message-ID: <0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com> (raw)
In-Reply-To: <20190806173945.GA4748@iweiny-DESK2.sc.intel.com>
On 8/6/19 10:39 AM, Ira Weiny wrote:
> On Sun, Aug 04, 2019 at 03:48:42PM -0700, john.hubbard@gmail.com wrote:
>> From: John Hubbard <jhubbard@nvidia.com>
...
>> -
>> /**
>> - * put_user_pages_dirty() - release and dirty an array of gup-pinned pages
>> - * @pages: array of pages to be marked dirty and released.
>> + * put_user_pages_dirty_lock() - release and optionally dirty gup-pinned pages
>> + * @pages: array of pages to be maybe marked dirty, and definitely released.
>
> Better would be.
>
> @pages: array of pages to be put
OK, I'll change to that wording.
>
>> * @npages: number of pages in the @pages array.
>> + * @make_dirty: whether to mark the pages dirty
>> *
>> * "gup-pinned page" refers to a page that has had one of the get_user_pages()
>> * variants called on that page.
>> *
>> * For each page in the @pages array, make that page (or its head page, if a
>> - * compound page) dirty, if it was previously listed as clean. Then, release
>> - * the page using put_user_page().
>> + * compound page) dirty, if @make_dirty is true, and if the page was previously
>> + * listed as clean. In any case, releases all pages using put_user_page(),
>> + * possibly via put_user_pages(), for the non-dirty case.
>
> I don't think users of this interface need this level of detail. I think
> something like.
>
> * For each page in the @pages array, release the page. If @make_dirty is
> * true, mark the page dirty prior to release.
Yes, it is too wordy, I'll change to that.
>
...
>> -void put_user_pages_dirty_lock(struct page **pages, unsigned long npages)
>> -{
>> - __put_user_pages_dirty(pages, npages, set_page_dirty_lock);
>> + /*
>> + * TODO: this can be optimized for huge pages: if a series of pages is
>> + * physically contiguous and part of the same compound page, then a
>> + * single operation to the head page should suffice.
>> + */
>
> I think this comment belongs to the for loop below... or just something about
> how to make this and put_user_pages() more efficient. It is odd, that this is
> the same comment as in put_user_pages()...
Actually I think I'll just delete the comment entirely, it's just noise really.
>
> The code is good. So... Other than the comments.
>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Thanks for the review!
thanks,
--
John Hubbard
NVIDIA
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-08-06 20:39 UTC|newest]
Thread overview: 250+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-04 22:48 [PATCH v2 00/34] put_user_pages(): miscellaneous call sites john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 01/34] mm/gup: add make_dirty arg to put_user_pages_dirty_lock() john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-06 17:39 ` Ira Weiny
2019-08-06 17:39 ` [Xen-devel] " Ira Weiny
2019-08-06 17:39 ` Ira Weiny
2019-08-06 17:39 ` Ira Weiny
2019-08-06 17:39 ` Ira Weiny
2019-08-06 17:39 ` Ira Weiny
2019-08-06 20:39 ` John Hubbard [this message]
2019-08-06 20:39 ` [Xen-devel] " John Hubbard
2019-08-06 20:39 ` John Hubbard
2019-08-06 20:39 ` John Hubbard
2019-08-06 20:39 ` John Hubbard
2019-08-06 20:39 ` John Hubbard
2019-08-06 20:39 ` John Hubbard
2019-08-04 22:48 ` [PATCH v2 02/34] net/rds: convert put_page() to put_user_page*() john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 03/34] net/ceph: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 04/34] x86/kvm: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 05/34] drm/etnaviv: convert release_pages() to put_user_pages() john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 06/34] drm/i915: convert put_page() to put_user_page*() john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-05 16:53 ` Rodrigo Vivi
2019-08-05 16:53 ` [Xen-devel] " Rodrigo Vivi
2019-08-05 16:53 ` Rodrigo Vivi
2019-08-05 16:53 ` Rodrigo Vivi
2019-08-05 16:53 ` Rodrigo Vivi
2019-08-05 16:53 ` Rodrigo Vivi
2019-08-04 22:48 ` [PATCH v2 07/34] drm/radeon: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 08/34] media/ivtv: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 09/34] media/v4l2-core/mm: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 10/34] genwqe: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 11/34] scif: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 12/34] vmci: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 13/34] rapidio: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 14/34] oradax: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 15/34] staging/vc04_services: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-13 5:23 ` Stefan Wahren
2019-08-13 5:23 ` [Xen-devel] " Stefan Wahren
2019-08-13 5:23 ` Stefan Wahren
2019-08-13 5:23 ` Stefan Wahren
2019-08-13 5:23 ` Stefan Wahren
2019-08-13 5:23 ` Stefan Wahren
2019-08-04 22:48 ` [PATCH v2 16/34] drivers/tee: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 17/34] vfio: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` [PATCH v2 18/34] fbdev/pvr2fb: " john.hubbard
2019-08-04 22:48 ` [Xen-devel] " john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:48 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 19/34] fsl_hypervisor: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 20/34] xen: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-05 4:15 ` Juergen Gross
2019-08-05 4:15 ` [Xen-devel] " Juergen Gross
2019-08-05 4:15 ` Juergen Gross
2019-08-05 4:15 ` Juergen Gross
2019-08-05 4:15 ` Juergen Gross
2019-08-05 4:15 ` Juergen Gross
2019-08-04 22:49 ` [PATCH v2 21/34] fs/exec.c: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 22/34] orangefs: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 23/34] uprobes: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 24/34] futex: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 25/34] mm/frame_vector.c: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 26/34] mm/gup_benchmark.c: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 27/34] mm/memory.c: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 28/34] mm/madvise.c: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 29/34] mm/process_vm_access.c: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 30/34] crypt: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 31/34] fs/nfs: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-05 0:26 ` Calum Mackay
2019-08-05 0:26 ` [Xen-devel] " Calum Mackay
2019-08-05 0:26 ` Calum Mackay
2019-08-05 0:26 ` Calum Mackay
2019-08-05 0:26 ` Calum Mackay
2019-08-05 0:26 ` Calum Mackay
2019-08-04 22:49 ` [PATCH v2 32/34] goldfish_pipe: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 33/34] kernel/events/core.c: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` [PATCH v2 34/34] fs/binfmt_elf: " john.hubbard
2019-08-04 22:49 ` [Xen-devel] " john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-04 22:49 ` john.hubbard
2019-08-09 12:42 ` ✗ Fi.CI.CHECKPATCH: warning for put_user_pages(): miscellaneous call sites Patchwork
2019-08-09 13:14 ` ✓ Fi.CI.BAT: success " Patchwork
2019-08-10 5:41 ` ✓ Fi.CI.IGT: " Patchwork
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=0e232d84-e6ea-159e-91d4-77e938377161@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=akpm@linux-foundation.org \
--cc=amd-gfx@lists.freedesktop.org \
--cc=ceph-devel@vger.kernel.org \
--cc=dan.j.williams@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=david@fromorbit.com \
--cc=devel@driverdev.osuosl.org \
--cc=devel@lists.orangefs.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=hch@infradead.org \
--cc=hch@lst.de \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ira.weiny@intel.com \
--cc=jack@suse.cz \
--cc=jgg@ziepe.ca \
--cc=jglisse@redhat.com \
--cc=john.hubbard@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nfs@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=linux-rpi-kernel@lists.infradead.org \
--cc=linux-xfs@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=rds-devel@oss.oracle.com \
--cc=sparclinux@vger.kernel.org \
--cc=willy@infradead.org \
--cc=x86@kernel.org \
--cc=xen-devel@lists.xenproject.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.