Messages in this thread | | | From | Souptick Joarder <> | Date | Mon, 9 Nov 2020 09:06:54 +0530 | Subject | Re: [PATCH 1/2] tomoyo: Convert get_user_pages*() to pin_user_pages*() |
| |
On Sun, Nov 8, 2020 at 7:47 AM John Hubbard <jhubbard@nvidia.com> wrote: > > On 11/7/20 5:13 PM, Tetsuo Handa wrote: > > On 2020/11/08 4:17, John Hubbard wrote: > >> On 11/7/20 1:04 AM, John Hubbard wrote: > >>> On 11/7/20 12:24 AM, Souptick Joarder wrote: > >>>> In 2019, we introduced pin_user_pages*() and now we are converting > >>>> get_user_pages*() to the new API as appropriate. [1] & [2] could > >>>> be referred for more information. This is case 5 as per document [1]. > >>> > >>> It turns out that Case 5 can be implemented via a better pattern, as long > >>> as we're just dealing with a page at a time, briefly: > >>> > >>> lock_page() > >>> write to page's data > >>> unlock_page() > >>> > >>> ...which neatly synchronizes with writeback and other fs activities. > >> > >> Ahem, I left out a key step: set_page_dirty()! > >> > >> lock_page() > >> write to page's data > >> set_page_dirty() > >> unlock_page() > >> > > > > Excuse me, but Documentation/core-api/pin_user_pages.rst says > > "CASE 5: Pinning in order to _write_ to the data within the page" > > while tomoyo_dump_page() is for "_read_ the data within the page". > > Do we want to convert to pin_user_pages_remote() or lock_page() ? > > > > Sorry, I missed the direction here, was too focused on the Case 5 > aspect. Yes. Case 5 (which, again, I think we're about to re-document) > is only about *writing* to data within the page. > > So in this case, where it is just reading from the page, I think it's > already from a gup vs pup point of view. > > btw, it's not clear to me whether the current code is susceptible to any > sort of problem involving something writing to the page while it > is being dumped (I am curious). But changing from gup to pup wouldn't > fix that, if it were a problem. It a separate question from this patch. > > (Souptick, if you're interested, the Case 5 documentation change and > callsite retrofit is all yours if you want it. Otherwise it's on > my list.)
Sure John, I will take it.
| |