From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xishi Qiu Subject: Re: [PATCH] mm/fs: don't keep pages when receiving a pending SIGKILL in __get_user_pages() Date: Thu, 16 Jan 2014 20:59:26 +0800 Message-ID: <52D7D7AE.8070108@huawei.com> References: <52D65568.6080106@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Cc: Li Zefan , , Andrew Morton , Mel Gorman , Rik van Riel , , , To: David Rientjes Return-path: In-Reply-To: Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org On 2014/1/16 7:15, David Rientjes wrote: > On Wed, 15 Jan 2014, Xishi Qiu wrote: >=20 >> In the process IO direction, dio_refill_pages will call get_user_pages= _fast=20 >> to map the page from user space. If ret is less than 0 and IO is write= , the=20 >> function will create a zero page to fill data. This may work for some = file=20 >> system, but in some device operate we prefer whole write or fail, not = half=20 >> data half zero, e.g. fs metadata, like inode, identy. >> This happens often when kill a process which is doing direct IO. Consi= der=20 >> the following cases, the process A is doing IO process, may enter __ge= t_user_pages=20 >> function, if other processes send process A SIG_KILL, A will enter the= =20 >> following branches=20 >> /* >> * If we have a pending SIGKILL, don't keep faulting >> * pages and potentially allocating memory. >> */ >> if (unlikely(fatal_signal_pending(current))) >> return i ? i : -ERESTARTSYS; >> Return current pages. direct IO will write the pages, the subsequent p= ages=20 >> which can=92t get will use zero page instead.=20 >> This patch will modify this judgment, if receive SIG_KILL, release pag= es and=20 >> return an error. Direct IO will find no blocks_available and return er= ror=20 >> direct, rather than half IO data and half zero page. >> >> Signed-off-by: Xishi Qiu >> Signed-off-by: Bin Yang >=20 > It's scary to change the behavior of gup when some callers may want the= =20 > exact opposite of what you're intending here, which is sane fallback by= =20 > mapping the zero page. In fact, gup never does put_page() itself and=20 > __get_user_pages() always returns the number of pages pinned and may no= t=20 > equal what is passed. >=20 > So, this definitely isn't the right solution for a special-case direct = IO. =20 > Instead, it would be better to code this directly in the caller and=20 > compare the return value with nr_pages in dio_refill_pages() and then d= o=20 > the put_page() itself before falling back to ZERO_PAGE(). Hi Rientjes, You are right, we should not change the behavior of gup. I have a question, if we only get a part of the pages from get_user_pages= _fast(), shall we write them to the disk? or add a check before write? I'm not familiar with fs. dio_refill_pages() get_user_pages_fast() Thanks, Xishi Qiu -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org