Re: [PATCH v7 3/7] mm: check fatal signal pending of target process

On Fri, Mar 06, 2020 at 11:22:07AM +0100, Vlastimil Babka wrote:

On 3/2/20 8:36 PM, Minchan Kim wrote:

quoted

Bail out to prevent unnecessary CPU overhead if target process has
pending fatal signal during (MADV_COLD|MADV_PAGEOUT) operation.

Reviewed-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Minchan Kim <minchan@kernel.org>

Reviewed-by: Vlastimil Babka <redacted>

Nit below:

quoted

---
 mm/madvise.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/mm/madvise.c b/mm/madvise.c
index 349473fc6683..6543f2bfc3d8 100644
--- a/mm/madvise.c
+++ b/mm/madvise.c

@@ -36,6 +36,7 @@
 struct madvise_walk_private {
 	struct mmu_gather *tlb;
 	bool pageout;
+	struct task_struct *target_task;
 };
 
 /*

@@ -316,6 +317,10 @@ static int madvise_cold_or_pageout_pte_range(pmd_t *pmd,
 	if (fatal_signal_pending(current))
 		return -EINTR;
 
+	if (private->target_task &&
+			fatal_signal_pending(private->target_task))
+		return -EINTR;

With madvise(2) private->target_task will be current, thus current will be
tested twice. Not wrong, but maybe add a "private->target_task != current"
condition?

It was in old series but removed because reviewer(IIRC, suren) wanted it.
I am not strong preference either way. Since you said it's nit and
considering other reviewer wanted to remove it, I will not change
further.

Thanks!