Messages in this thread | | | Date | Tue, 28 Jun 2022 17:31:43 -0700 | Subject | Re: [PATCH 1/4] mm/gup: Add FOLL_INTERRUPTIBLE | From | John Hubbard <> |
| |
On 6/28/22 15:33, Peter Xu wrote: >> The key point is the connection between "locked" and killable. If the comment >> explained why "locked" means "killable", that would help clear this up. The >> NOWAIT sentence is also confusing to me, and adding "mostly NOWAIT" does not >> clear it up either... :) > > Sorry to have a comment that makes it feels confusing. I tried to > explicitly put the comment to be after setting FAULT_FLAG_KILLABLE but > obviously I didn't do my job well.. > > Maybe that NOWAIT thing adds more complexity but not even necessary. > > Would below one more acceptable? > > /* > * We'll only be able to respond to signals when "locked != > * NULL". When with it, we'll always respond to SIGKILL > * (as implied by FAULT_FLAG_KILLABLE above), and we'll > * respond to non-fatal signals only if the GUP user has > * specified FOLL_INTERRUPTIBLE. > */
It looks like part of this comment is trying to document a pre-existing concept, which is that faultin_page() only ever sets FAULT_FLAG_KILLABLE if locked != NULL. The problem I am (personally) having is that I don't yet understand why or how those are connected: what is it about having locked non-NULL that means the process is killable? (Can you explain why that is?)
If that were clear, I think I could suggest a good comment wording.
thanks, -- John Hubbard NVIDIA
| |