* [PATCH] mm: shmem: unlock valid page
@ 2012-03-04 5:17 Hillf Danton
2012-03-04 17:51 ` Hugh Dickins
0 siblings, 1 reply; 3+ messages in thread
From: Hillf Danton @ 2012-03-04 5:17 UTC (permalink / raw
To: Linux-MM; +Cc: Hugh Dickins, Andrew Morton, LKML, Hillf Danton
In shmem_read_mapping_page_gfp() page is unlocked if no error returned,
so the unlocked page has to valid.
To guarantee that validity, when getting page, success result is feed
back to caller only when page is valid.
Signed-off-by: Hillf Danton <dhillf@gmail.com>
---
--- a/mm/shmem.c Sun Mar 4 12:17:42 2012
+++ b/mm/shmem.c Sun Mar 4 12:26:56 2012
@@ -889,13 +889,13 @@ repeat:
goto failed;
}
- if (page || (sgp == SGP_READ && !swap.val)) {
+ if (page) {
/*
* Once we can get the page lock, it must be uptodate:
* if there were an error in reading back from swap,
* the page would not be inserted into the filecache.
*/
- BUG_ON(page && !PageUptodate(page));
+ BUG_ON(!PageUptodate(page));
*pagep = page;
return 0;
}
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm: shmem: unlock valid page
2012-03-04 5:17 [PATCH] mm: shmem: unlock valid page Hillf Danton
@ 2012-03-04 17:51 ` Hugh Dickins
2012-03-05 12:15 ` Hillf Danton
0 siblings, 1 reply; 3+ messages in thread
From: Hugh Dickins @ 2012-03-04 17:51 UTC (permalink / raw
To: Hillf Danton; +Cc: Linux-MM, Andrew Morton, LKML
On Sun, 4 Mar 2012, Hillf Danton wrote:
> In shmem_read_mapping_page_gfp() page is unlocked if no error returned,
> so the unlocked page has to valid.
>
> To guarantee that validity, when getting page, success result is feed
> back to caller only when page is valid.
>
> Signed-off-by: Hillf Danton <dhillf@gmail.com>
I don't understand your description, nor its relation to the patch.
NAK to the patch: when no page has previously been allocated, the
SGP_READ case avoids allocation and returns NULL - do_shmem_file_read
then copies the ZERO_PAGE instead, avoiding lots of unnecessary memory
allocation when reading a large sparse file.
Hugh
> ---
>
> --- a/mm/shmem.c Sun Mar 4 12:17:42 2012
> +++ b/mm/shmem.c Sun Mar 4 12:26:56 2012
> @@ -889,13 +889,13 @@ repeat:
> goto failed;
> }
>
> - if (page || (sgp == SGP_READ && !swap.val)) {
> + if (page) {
> /*
> * Once we can get the page lock, it must be uptodate:
> * if there were an error in reading back from swap,
> * the page would not be inserted into the filecache.
> */
> - BUG_ON(page && !PageUptodate(page));
> + BUG_ON(!PageUptodate(page));
> *pagep = page;
> return 0;
> }
> --
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] mm: shmem: unlock valid page
2012-03-04 17:51 ` Hugh Dickins
@ 2012-03-05 12:15 ` Hillf Danton
0 siblings, 0 replies; 3+ messages in thread
From: Hillf Danton @ 2012-03-05 12:15 UTC (permalink / raw
To: Hugh Dickins; +Cc: Linux-MM, Andrew Morton, LKML
On Mon, Mar 5, 2012 at 1:51 AM, Hugh Dickins <hughd@google.com> wrote:
> On Sun, 4 Mar 2012, Hillf Danton wrote:
>> In shmem_read_mapping_page_gfp() page is unlocked if no error returned,
>> so the unlocked page has to valid.
>>
>> To guarantee that validity, when getting page, success result is feed
>> back to caller only when page is valid.
>>
>> Signed-off-by: Hillf Danton <dhillf@gmail.com>
>
> I don't understand your description, nor its relation to the patch.
>
> NAK to the patch: when no page has previously been allocated, the
> SGP_READ case avoids allocation and returns NULL - do_shmem_file_read
> then copies the ZERO_PAGE instead, avoiding lots of unnecessary memory
> allocation when reading a large sparse file.
>
Hi Hugh
Thanks for your review.
It was not well prepared as I missed SGP_CACHE.
-hd
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-03-05 12:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-04 5:17 [PATCH] mm: shmem: unlock valid page Hillf Danton
2012-03-04 17:51 ` Hugh Dickins
2012-03-05 12:15 ` Hillf Danton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).