From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kernel@vger.kernel.org,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
kernel-team <kernel-team@lge.com>
Subject: Re: [PATCH 2/2] zram: do not count duplicated pages as compressed
Date: Tue, 16 May 2017 16:36:17 +0900 [thread overview]
Message-ID: <20170516073616.GB767@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <20170516071644.GA6224@bbox>
On (05/16/17 16:16), Minchan Kim wrote:
> > but would this be correct? the data is not valid - we failed to store
> > the valid one. but instead we assure application that read()/swapin/etc.,
> > depending on the usage scenario, is successful (even though the data is
> > not what application really expects to see), application tries to use the
> > data from that page and probably crashes (dunno, for example page contained
> > hash tables with pointers that are not valid anymore, etc. etc.).
> >
> > I'm not optimistic about stale data reads; it basically will look like
> > data corruption to the application.
>
> Hmm, I don't understand what you say.
> My point is zram_free_page should be done only if whoe write operation
> is successful.
> With you change, following situation can happens.
>
> write block 4, 'all A' -> success
> read block 4, 'all A' verified -> Good
> write block 4, 'all B' -> but failed with ENOMEM
> read block 4 expected 'all A' but 'all 0' -> Oops
yes. 'all A' in #4 can be incorrect. zram can be used as a block device
with a file system, and pid that does write op not necessarily does read
op later. it can be a completely different application. e.g. compilation,
or anything else.
suppose PID A does
wr block 1 all a
wr block 2 all a + 1
wr block 3 all a + 2
wr block 4 all a + 3
now PID A does
wr block 1 all m
wr block 2 all m + 1
wr block 3 all m + 2
wr block 4 failed. block still has 'all a + 3'.
exit
another application, PID C, reads in the file and tries to do
something sane with it
rd block 1 all m
rd block 2 all m + 1
rd block 3 all m + 3
rd block 4 all a + 3 << this is dangerous. we should return
error from read() here; not stale data.
what we can return now is a `partially updated' data, with some new
and some stale pages. this is quite unlikely to end up anywhere good.
am I wrong?
why does `rd block 4' in your case causes Oops? as a worst case scenario?
application does not expect page to be 'all A' at this point. pages are
likely to belong to some mappings/files/etc., and there is likely a data
dependency between them, dunno C++ objects that span across pages or
JPEG images, etc. so returning "new data new data stale data" is a bit
fishy.
-ss
next prev parent reply other threads:[~2017-05-16 7:36 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-15 7:41 [PATCH 1/2] zram: count same page write as page_stored Minchan Kim
2017-05-15 7:41 ` [PATCH 2/2] zram: do not count duplicated pages as compressed Minchan Kim
2017-05-16 1:30 ` Sergey Senozhatsky
2017-05-16 1:59 ` Minchan Kim
2017-05-16 2:36 ` Sergey Senozhatsky
2017-05-16 5:26 ` Minchan Kim
2017-05-16 5:45 ` Sergey Senozhatsky
2017-05-16 7:16 ` Minchan Kim
2017-05-16 7:36 ` Sergey Senozhatsky [this message]
2017-05-17 8:32 ` Minchan Kim
2017-05-17 9:14 ` Sergey Senozhatsky
2017-05-18 4:53 ` Minchan Kim
2017-05-21 7:04 ` Christoph Hellwig
2017-05-21 7:15 ` Minchan Kim
2017-05-16 1:11 ` [PATCH 1/2] zram: count same page write as page_stored Sergey Senozhatsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170516073616.GB767@jagdpanzerIV.localdomain \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=kernel-team@lge.com \
--cc=linux-kernel@vger.kernel.org \
--cc=minchan@kernel.org \
--cc=sergey.senozhatsky@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.