Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 04:28:19PM +0900, Minchan Kim wrote:
> On Wed, Aug 27, 2014 at 02:04:38PM +0900, Joonsoo Kim wrote:
> > On Wed, Aug 27, 2014 at 11:51:32AM +0900, Minchan Kim wrote:
> > > Hey Joonsoo,
> > >
> > > On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> > > > Hello, Minchan and David.
> > > >
> > > > On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> > > > > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim
> > > > > wrote:
> > > > > > Hey Joonsoo,
> > > > > >
> > > > > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> > > > > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> > > > > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
> > > > > >> > *zram, struct bio_vec *bvec, u32 index,
> > > > > >> > ret = -ENOMEM;
> > > > > >> > goto out;
> > > > > >> > }
> > > > > >> > +
> > > > > >> > + if (zram->limit_pages &&
> > > > > >> > + zs_get_total_pages(meta->mem_pool) >
> > > > > >> > zram->limit_pages) {
> > > > > >> > + zs_free(meta->mem_pool, handle);
> > > > > >> > + ret = -ENOMEM;
> > > > > >> > + goto out;
> > > > > >> > + }
> > > > > >> > +
> > > > > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> > > > > >>
> > > > > >> Hello,
> > > > > >>
> > > > > >> I don't follow up previous discussion, so I could be wrong.
> > > > > >> Why this enforcement should be here?
> > > > > >>
> > > > > >> I think that this has two problems.
> > > > > >> 1) alloc/free happens unnecessarilly if we have used memory over
> > > > > >> the
> > > > > >> limitation.
> > > > > >
> > > > > > True but firstly, I implemented the logic in zsmalloc, not zram but
> > > > > > as I described in cover-letter, it's not a requirement of zsmalloc
> > > > > > but zram so it should be in there. If every user want it in future,
> > > > > > then we could move the function into zsmalloc. That's what we
> > > > > > concluded in previous discussion.
> > > >
> > > > Hmm...
> > > > Problem is that we can't avoid these unnecessary overhead in this
> > > > implementation. If we can implement this feature in zram efficiently,
> > > > it's okay. But, I think that current form isn't.
> > >
> > >
> > > If we can add it in zsmalloc, it would be more clean and efficient
> > > for zram but as I said, at the moment, I didn't want to put zram's
> > > requirement into zsmalloc because to me, it's weird to enforce max
> > > limit to allocator. It's client's role, I think.
> >
> > AFAIK, many kinds of pools such as thread-pool or memory-pool have
> > their own limit. It's not weird for me.
>
> Actually I don't know what is pool allocator but things you mentioned
> is basically used to gaurantee *new* thread/memory, not limit although
> it would implement limit.
>
> Another question, why do you think zsmalloc is pool allocator?
> IOW, What logic makes you think it's pool allocator?
In fact, it is not pool allocator for now. But, it looks like pool
allocator because it is used only for one zram device. If there are
many zram devices, there are many zs_pool and their memory cannot be
shared. It is totally isolated each other. We can easily make it
actual pool allocator or impose memory usage limit on it with this
property. This make me think that zsmalloc is better place to limit
memory usage.
Anyway, I don't have strong objection to current implementation. You
can fix it later when it turn out to be real problem.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 04:28:19PM +0900, Minchan Kim wrote:
On Wed, Aug 27, 2014 at 02:04:38PM +0900, Joonsoo Kim wrote:
On Wed, Aug 27, 2014 at 11:51:32AM +0900, Minchan Kim wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org
wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
*zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over
the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
AFAIK, many kinds of pools such as thread-pool or memory-pool have
their own limit. It's not weird for me.
Actually I don't know what is pool allocator but things you mentioned
is basically used to gaurantee *new* thread/memory, not limit although
it would implement limit.
Another question, why do you think zsmalloc is pool allocator?
IOW, What logic makes you think it's pool allocator?
In fact, it is not pool allocator for now. But, it looks like pool
allocator because it is used only for one zram device. If there are
many zram devices, there are many zs_pool and their memory cannot be
shared. It is totally isolated each other. We can easily make it
actual pool allocator or impose memory usage limit on it with this
property. This make me think that zsmalloc is better place to limit
memory usage.
Anyway, I don't have strong objection to current implementation. You
can fix it later when it turn out to be real problem.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 03:04:32PM -0400, Dan Streetman wrote:
> On Wed, Aug 27, 2014 at 12:59 PM, David Horner wrote:
> > On Wed, Aug 27, 2014 at 12:29 PM, Dan Streetman wrote:
> >> On Wed, Aug 27, 2014 at 11:35 AM, David Horner wrote:
> >>> On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman wrote:
> On Wed, Aug 27, 2014 at 10:44 AM, David Horner
> wrote:
> > On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman
> > wrote:
> >> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim
> >> wrote:
> >>> Hey Joonsoo,
> >>>
> >>> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> Hello, Minchan and David.
>
> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim
> > wrote:
> > > Hey Joonsoo,
> > >
> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
> > >> > *zram, struct bio_vec *bvec, u32 index,
> > >> > ret = -ENOMEM;
> > >> > goto out;
> > >> > }
> > >> > +
> > >> > + if (zram->limit_pages &&
> > >> > + zs_get_total_pages(meta->mem_pool) >
> > >> > zram->limit_pages) {
> > >> > + zs_free(meta->mem_pool, handle);
> > >> > + ret = -ENOMEM;
> > >> > + goto out;
> > >> > + }
> > >> > +
> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> > >>
> > >> Hello,
> > >>
> > >> I don't follow up previous discussion, so I could be wrong.
> > >> Why this enforcement should be here?
> > >>
> > >> I think that this has two problems.
> > >> 1) alloc/free happens unnecessarilly if we have used memory
> > >> over the
> > >> limitation.
> > >
> > > True but firstly, I implemented the logic in zsmalloc, not zram
> > > but
> > > as I described in cover-letter, it's not a requirement of
> > > zsmalloc
> > > but zram so it should be in there. If every user want it in
> > > future,
> > > then we could move the function into zsmalloc. That's what we
> > > concluded in previous discussion.
>
> Hmm...
> Problem is that we can't avoid these unnecessary overhead in this
> implementation. If we can implement this feature in zram efficiently,
> it's okay. But, I think that current form isn't.
> >>>
> >>>
> >>> If we can add it in zsmalloc, it would be more clean and efficient
> >>> for zram but as I said, at the moment, I didn't want to put zram's
> >>> requirement into zsmalloc because to me, it's weird to enforce max
> >>> limit to allocator. It's client's role, I think.
> >>>
> >>> If current implementation is expensive and rather hard to follow,
> >>> It would be one reason to move the feature into zsmalloc but
> >>> I don't think it makes critical trobule in zram usecase.
> >>> See below.
> >>>
> >>> But I still open and will wait others's opinion.
> >>> If other guys think zsmalloc is better place, I am willing to move
> >>> it into zsmalloc.
> >>
> >> Moving it into zsmalloc would allow rejecting new zsmallocs before
> >> actually crossing the limit, since it can calculate that internally.
> >> However, with the current patches the limit will only be briefly
> >> crossed, and it should not be crossed by a large amount. Now, if this
> >> is happening repeatedly and quickly during extreme memory pressure,
> >> the constant alloc/free will clearly be worse than a simple internal
> >> calculation and failure. But would it ever happen repeatedly once the
> >> zram limit is reached?
> >>
> >> Now that I'm thinking about the limit from the perspective of the zram
> >> user, I wonder what really will happen. If zram is being used for
> >> swap space, then when swap starts getting errors trying to write
> >> pages, how damaging will that be to the system? I haven't checked
> >> what swap does when it encounters disk errors. Of course, with no
> >> zram limit, continually writing to zram until memory is totally
> >> consumed isn't good either. But in any case, I would hope that swap
> >> would not repeatedly hammer on a disk when it's getting write failures
> >> from it.
> >>
> >> Alternately, if zram was being used as a compressed ram disk for
> >> regular file storage, it's entirely up to the application to handle
> >> write failures, so it may continue to try to write to a full zram
> >> disk.
> >>
> >> As
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 12:29:22PM -0400, Dan Streetman wrote:
> On Wed, Aug 27, 2014 at 11:35 AM, David Horner wrote:
> > On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman wrote:
> >> On Wed, Aug 27, 2014 at 10:44 AM, David Horner wrote:
> >>> On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman wrote:
> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
> > Hey Joonsoo,
> >
> > On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> >> Hello, Minchan and David.
> >>
> >> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> >> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim
> >> > wrote:
> >> > > Hey Joonsoo,
> >> > >
> >> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> >> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> >> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
> >> > >> > *zram, struct bio_vec *bvec, u32 index,
> >> > >> > ret = -ENOMEM;
> >> > >> > goto out;
> >> > >> > }
> >> > >> > +
> >> > >> > + if (zram->limit_pages &&
> >> > >> > + zs_get_total_pages(meta->mem_pool) >
> >> > >> > zram->limit_pages) {
> >> > >> > + zs_free(meta->mem_pool, handle);
> >> > >> > + ret = -ENOMEM;
> >> > >> > + goto out;
> >> > >> > + }
> >> > >> > +
> >> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> >> > >>
> >> > >> Hello,
> >> > >>
> >> > >> I don't follow up previous discussion, so I could be wrong.
> >> > >> Why this enforcement should be here?
> >> > >>
> >> > >> I think that this has two problems.
> >> > >> 1) alloc/free happens unnecessarilly if we have used memory over
> >> > >> the
> >> > >> limitation.
> >> > >
> >> > > True but firstly, I implemented the logic in zsmalloc, not zram but
> >> > > as I described in cover-letter, it's not a requirement of zsmalloc
> >> > > but zram so it should be in there. If every user want it in future,
> >> > > then we could move the function into zsmalloc. That's what we
> >> > > concluded in previous discussion.
> >>
> >> Hmm...
> >> Problem is that we can't avoid these unnecessary overhead in this
> >> implementation. If we can implement this feature in zram efficiently,
> >> it's okay. But, I think that current form isn't.
> >
> >
> > If we can add it in zsmalloc, it would be more clean and efficient
> > for zram but as I said, at the moment, I didn't want to put zram's
> > requirement into zsmalloc because to me, it's weird to enforce max
> > limit to allocator. It's client's role, I think.
> >
> > If current implementation is expensive and rather hard to follow,
> > It would be one reason to move the feature into zsmalloc but
> > I don't think it makes critical trobule in zram usecase.
> > See below.
> >
> > But I still open and will wait others's opinion.
> > If other guys think zsmalloc is better place, I am willing to move
> > it into zsmalloc.
>
> Moving it into zsmalloc would allow rejecting new zsmallocs before
> actually crossing the limit, since it can calculate that internally.
> However, with the current patches the limit will only be briefly
> crossed, and it should not be crossed by a large amount. Now, if this
> is happening repeatedly and quickly during extreme memory pressure,
> the constant alloc/free will clearly be worse than a simple internal
> calculation and failure. But would it ever happen repeatedly once the
> zram limit is reached?
>
> Now that I'm thinking about the limit from the perspective of the zram
> user, I wonder what really will happen. If zram is being used for
> swap space, then when swap starts getting errors trying to write
> pages, how damaging will that be to the system? I haven't checked
> what swap does when it encounters disk errors. Of course, with no
> zram limit, continually writing to zram until memory is totally
> consumed isn't good either. But in any case, I would hope that swap
> would not repeatedly hammer on a disk when it's getting write failures
> from it.
>
> Alternately, if zram was being used as a compressed ram disk for
> regular file storage, it's entirely up to the application to handle
> write failures, so it may continue to try to write to a full zram
> disk.
>
> As far as what the zsmalloc api would look like with the limit added,
> it would need a setter and getter function (adding it as a param to
> the create function would be optional i think). But more importantly,
> it would need to handle multiple ways of specifying the limit. In our
> specific current use cases, zram and zswap, each handles their
>
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hello,
On Wed, Aug 27, 2014 at 10:03:45AM -0400, Dan Streetman wrote:
> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
> > Hey Joonsoo,
> >
> > On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> >> Hello, Minchan and David.
> >>
> >> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> >> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> >> > > Hey Joonsoo,
> >> > >
> >> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> >> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> >> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
> >> > >> > struct bio_vec *bvec, u32 index,
> >> > >> > ret = -ENOMEM;
> >> > >> > goto out;
> >> > >> > }
> >> > >> > +
> >> > >> > + if (zram->limit_pages &&
> >> > >> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages)
> >> > >> > {
> >> > >> > + zs_free(meta->mem_pool, handle);
> >> > >> > + ret = -ENOMEM;
> >> > >> > + goto out;
> >> > >> > + }
> >> > >> > +
> >> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> >> > >>
> >> > >> Hello,
> >> > >>
> >> > >> I don't follow up previous discussion, so I could be wrong.
> >> > >> Why this enforcement should be here?
> >> > >>
> >> > >> I think that this has two problems.
> >> > >> 1) alloc/free happens unnecessarilly if we have used memory over the
> >> > >> limitation.
> >> > >
> >> > > True but firstly, I implemented the logic in zsmalloc, not zram but
> >> > > as I described in cover-letter, it's not a requirement of zsmalloc
> >> > > but zram so it should be in there. If every user want it in future,
> >> > > then we could move the function into zsmalloc. That's what we
> >> > > concluded in previous discussion.
> >>
> >> Hmm...
> >> Problem is that we can't avoid these unnecessary overhead in this
> >> implementation. If we can implement this feature in zram efficiently,
> >> it's okay. But, I think that current form isn't.
> >
> >
> > If we can add it in zsmalloc, it would be more clean and efficient
> > for zram but as I said, at the moment, I didn't want to put zram's
> > requirement into zsmalloc because to me, it's weird to enforce max
> > limit to allocator. It's client's role, I think.
> >
> > If current implementation is expensive and rather hard to follow,
> > It would be one reason to move the feature into zsmalloc but
> > I don't think it makes critical trobule in zram usecase.
> > See below.
> >
> > But I still open and will wait others's opinion.
> > If other guys think zsmalloc is better place, I am willing to move
> > it into zsmalloc.
>
> Moving it into zsmalloc would allow rejecting new zsmallocs before
> actually crossing the limit, since it can calculate that internally.
> However, with the current patches the limit will only be briefly
> crossed, and it should not be crossed by a large amount. Now, if this
> is happening repeatedly and quickly during extreme memory pressure,
> the constant alloc/free will clearly be worse than a simple internal
> calculation and failure. But would it ever happen repeatedly once the
> zram limit is reached?
Right. it depends on user.
If user writes continuously without any action to cover the situation
once the limit is over, it would be terrible but what I meant *terrible*
doesn't mean alloc/free cost. Actually, zsmalloc/zsfree cost is really
cheaper comparing to other mm reclaim, swap, vfs, fs, block layer's one.
What I meant *terrible* is slower system caused by swapout failure
on system but it try to reclaim anonymous pages continuously.
>
> Now that I'm thinking about the limit from the perspective of the zram
> user, I wonder what really will happen. If zram is being used for
> swap space, then when swap starts getting errors trying to write
> pages, how damaging will that be to the system? I haven't checked
> what swap does when it encounters disk errors. Of course, with no
> zram limit, continually writing to zram until memory is totally
> consumed isn't good either. But in any case, I would hope that swap
> would not repeatedly hammer on a disk when it's getting write failures
> from it.
Good point. Actually, it's my next step.
Curretly, VM doesn't handle congestion for anonymous page while it is
doing something for file-backed pages(but actually, it's really basic
stuff at this moment) so we could improve it with several ways.
I'm looking at it now.
>
> Alternately, if zram was being used as a compressed ram disk for
> regular file storage, it's entirely up to the application to handle
> write failures, so it may continue to try to write to a full zram
> disk.
>
> As far as what the zsmalloc api would look like with the limit added,
> it would need a setter and getter function (adding it as a param to
> the create function would be optional i think). But more importantly,
> it would need to handle multiple ways of specifying the limit. In our
> specific
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 12:59 PM, David Horner wrote:
> On Wed, Aug 27, 2014 at 12:29 PM, Dan Streetman wrote:
>> On Wed, Aug 27, 2014 at 11:35 AM, David Horner wrote:
>>> On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman wrote:
On Wed, Aug 27, 2014 at 10:44 AM, David Horner wrote:
> On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman wrote:
>> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
>>> Hey Joonsoo,
>>>
>>> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim
> wrote:
> > Hey Joonsoo,
> >
> > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
> >> > *zram, struct bio_vec *bvec, u32 index,
> >> > ret = -ENOMEM;
> >> > goto out;
> >> > }
> >> > +
> >> > + if (zram->limit_pages &&
> >> > + zs_get_total_pages(meta->mem_pool) >
> >> > zram->limit_pages) {
> >> > + zs_free(meta->mem_pool, handle);
> >> > + ret = -ENOMEM;
> >> > + goto out;
> >> > + }
> >> > +
> >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> >>
> >> Hello,
> >>
> >> I don't follow up previous discussion, so I could be wrong.
> >> Why this enforcement should be here?
> >>
> >> I think that this has two problems.
> >> 1) alloc/free happens unnecessarilly if we have used memory over
> >> the
> >> limitation.
> >
> > True but firstly, I implemented the logic in zsmalloc, not zram but
> > as I described in cover-letter, it's not a requirement of zsmalloc
> > but zram so it should be in there. If every user want it in future,
> > then we could move the function into zsmalloc. That's what we
> > concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
>>>
>>>
>>> If we can add it in zsmalloc, it would be more clean and efficient
>>> for zram but as I said, at the moment, I didn't want to put zram's
>>> requirement into zsmalloc because to me, it's weird to enforce max
>>> limit to allocator. It's client's role, I think.
>>>
>>> If current implementation is expensive and rather hard to follow,
>>> It would be one reason to move the feature into zsmalloc but
>>> I don't think it makes critical trobule in zram usecase.
>>> See below.
>>>
>>> But I still open and will wait others's opinion.
>>> If other guys think zsmalloc is better place, I am willing to move
>>> it into zsmalloc.
>>
>> Moving it into zsmalloc would allow rejecting new zsmallocs before
>> actually crossing the limit, since it can calculate that internally.
>> However, with the current patches the limit will only be briefly
>> crossed, and it should not be crossed by a large amount. Now, if this
>> is happening repeatedly and quickly during extreme memory pressure,
>> the constant alloc/free will clearly be worse than a simple internal
>> calculation and failure. But would it ever happen repeatedly once the
>> zram limit is reached?
>>
>> Now that I'm thinking about the limit from the perspective of the zram
>> user, I wonder what really will happen. If zram is being used for
>> swap space, then when swap starts getting errors trying to write
>> pages, how damaging will that be to the system? I haven't checked
>> what swap does when it encounters disk errors. Of course, with no
>> zram limit, continually writing to zram until memory is totally
>> consumed isn't good either. But in any case, I would hope that swap
>> would not repeatedly hammer on a disk when it's getting write failures
>> from it.
>>
>> Alternately, if zram was being used as a compressed ram disk for
>> regular file storage, it's entirely up to the application to handle
>> write failures, so it may continue to try to write to a full zram
>> disk.
>>
>> As far as what the zsmalloc api would look like with the limit added,
>> it would need a setter and getter function (adding it as a param to
>> the create function would be optional i think). But more importantly,
>> it would need to handle multiple ways of specifying the limit. In our
>> specific current use
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 12:29 PM, Dan Streetman wrote:
> On Wed, Aug 27, 2014 at 11:35 AM, David Horner wrote:
>> On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman wrote:
>>> On Wed, Aug 27, 2014 at 10:44 AM, David Horner wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman wrote:
> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
>> Hey Joonsoo,
>>
>> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
>>> Hello, Minchan and David.
>>>
>>> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
>>> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim
>>> > wrote:
>>> > > Hey Joonsoo,
>>> > >
>>> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
>>> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
>>> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
>>> > >> > *zram, struct bio_vec *bvec, u32 index,
>>> > >> > ret = -ENOMEM;
>>> > >> > goto out;
>>> > >> > }
>>> > >> > +
>>> > >> > + if (zram->limit_pages &&
>>> > >> > + zs_get_total_pages(meta->mem_pool) >
>>> > >> > zram->limit_pages) {
>>> > >> > + zs_free(meta->mem_pool, handle);
>>> > >> > + ret = -ENOMEM;
>>> > >> > + goto out;
>>> > >> > + }
>>> > >> > +
>>> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
>>> > >>
>>> > >> Hello,
>>> > >>
>>> > >> I don't follow up previous discussion, so I could be wrong.
>>> > >> Why this enforcement should be here?
>>> > >>
>>> > >> I think that this has two problems.
>>> > >> 1) alloc/free happens unnecessarilly if we have used memory over
>>> > >> the
>>> > >> limitation.
>>> > >
>>> > > True but firstly, I implemented the logic in zsmalloc, not zram but
>>> > > as I described in cover-letter, it's not a requirement of zsmalloc
>>> > > but zram so it should be in there. If every user want it in future,
>>> > > then we could move the function into zsmalloc. That's what we
>>> > > concluded in previous discussion.
>>>
>>> Hmm...
>>> Problem is that we can't avoid these unnecessary overhead in this
>>> implementation. If we can implement this feature in zram efficiently,
>>> it's okay. But, I think that current form isn't.
>>
>>
>> If we can add it in zsmalloc, it would be more clean and efficient
>> for zram but as I said, at the moment, I didn't want to put zram's
>> requirement into zsmalloc because to me, it's weird to enforce max
>> limit to allocator. It's client's role, I think.
>>
>> If current implementation is expensive and rather hard to follow,
>> It would be one reason to move the feature into zsmalloc but
>> I don't think it makes critical trobule in zram usecase.
>> See below.
>>
>> But I still open and will wait others's opinion.
>> If other guys think zsmalloc is better place, I am willing to move
>> it into zsmalloc.
>
> Moving it into zsmalloc would allow rejecting new zsmallocs before
> actually crossing the limit, since it can calculate that internally.
> However, with the current patches the limit will only be briefly
> crossed, and it should not be crossed by a large amount. Now, if this
> is happening repeatedly and quickly during extreme memory pressure,
> the constant alloc/free will clearly be worse than a simple internal
> calculation and failure. But would it ever happen repeatedly once the
> zram limit is reached?
>
> Now that I'm thinking about the limit from the perspective of the zram
> user, I wonder what really will happen. If zram is being used for
> swap space, then when swap starts getting errors trying to write
> pages, how damaging will that be to the system? I haven't checked
> what swap does when it encounters disk errors. Of course, with no
> zram limit, continually writing to zram until memory is totally
> consumed isn't good either. But in any case, I would hope that swap
> would not repeatedly hammer on a disk when it's getting write failures
> from it.
>
> Alternately, if zram was being used as a compressed ram disk for
> regular file storage, it's entirely up to the application to handle
> write failures, so it may continue to try to write to a full zram
> disk.
>
> As far as what the zsmalloc api would look like with the limit added,
> it would need a setter and getter function (adding it as a param to
> the create function would be optional i think). But more importantly,
> it would need to handle multiple ways of specifying the limit. In our
> specific current use cases, zram and zswap, each handles their
> internal limit differently - zswap currently uses a % of total ram as
> its limit (defaulting to 20),
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 11:35 AM, David Horner wrote:
> On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman wrote:
>> On Wed, Aug 27, 2014 at 10:44 AM, David Horner wrote:
>>> On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
> Hey Joonsoo,
>
> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
>> Hello, Minchan and David.
>>
>> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
>> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim
>> > wrote:
>> > > Hey Joonsoo,
>> > >
>> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
>> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
>> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
>> > >> > *zram, struct bio_vec *bvec, u32 index,
>> > >> > ret = -ENOMEM;
>> > >> > goto out;
>> > >> > }
>> > >> > +
>> > >> > + if (zram->limit_pages &&
>> > >> > + zs_get_total_pages(meta->mem_pool) >
>> > >> > zram->limit_pages) {
>> > >> > + zs_free(meta->mem_pool, handle);
>> > >> > + ret = -ENOMEM;
>> > >> > + goto out;
>> > >> > + }
>> > >> > +
>> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
>> > >>
>> > >> Hello,
>> > >>
>> > >> I don't follow up previous discussion, so I could be wrong.
>> > >> Why this enforcement should be here?
>> > >>
>> > >> I think that this has two problems.
>> > >> 1) alloc/free happens unnecessarilly if we have used memory over the
>> > >> limitation.
>> > >
>> > > True but firstly, I implemented the logic in zsmalloc, not zram but
>> > > as I described in cover-letter, it's not a requirement of zsmalloc
>> > > but zram so it should be in there. If every user want it in future,
>> > > then we could move the function into zsmalloc. That's what we
>> > > concluded in previous discussion.
>>
>> Hmm...
>> Problem is that we can't avoid these unnecessary overhead in this
>> implementation. If we can implement this feature in zram efficiently,
>> it's okay. But, I think that current form isn't.
>
>
> If we can add it in zsmalloc, it would be more clean and efficient
> for zram but as I said, at the moment, I didn't want to put zram's
> requirement into zsmalloc because to me, it's weird to enforce max
> limit to allocator. It's client's role, I think.
>
> If current implementation is expensive and rather hard to follow,
> It would be one reason to move the feature into zsmalloc but
> I don't think it makes critical trobule in zram usecase.
> See below.
>
> But I still open and will wait others's opinion.
> If other guys think zsmalloc is better place, I am willing to move
> it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman wrote:
> On Wed, Aug 27, 2014 at 10:44 AM, David Horner wrote:
>> On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman wrote:
>>> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> Hello, Minchan and David.
>
> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> > > Hey Joonsoo,
> > >
> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
> > >> > struct bio_vec *bvec, u32 index,
> > >> > ret = -ENOMEM;
> > >> > goto out;
> > >> > }
> > >> > +
> > >> > + if (zram->limit_pages &&
> > >> > + zs_get_total_pages(meta->mem_pool) >
> > >> > zram->limit_pages) {
> > >> > + zs_free(meta->mem_pool, handle);
> > >> > + ret = -ENOMEM;
> > >> > + goto out;
> > >> > + }
> > >> > +
> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> > >>
> > >> Hello,
> > >>
> > >> I don't follow up previous discussion, so I could be wrong.
> > >> Why this enforcement should be here?
> > >>
> > >> I think that this has two problems.
> > >> 1) alloc/free happens unnecessarilly if we have used memory over the
> > >> limitation.
> > >
> > > True but firstly, I implemented the logic in zsmalloc, not zram but
> > > as I described in cover-letter, it's not a requirement of zsmalloc
> > > but zram so it should be in there. If every user want it in future,
> > > then we could move the function into zsmalloc. That's what we
> > > concluded in previous discussion.
>
> Hmm...
> Problem is that we can't avoid these unnecessary overhead in this
> implementation. If we can implement this feature in zram efficiently,
> it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
>>>
>>> Moving it into zsmalloc would allow rejecting new zsmallocs before
>>> actually crossing the limit, since it can calculate that internally.
>>> However, with the current patches the limit will only be briefly
>>> crossed, and it should not be crossed by a large amount. Now, if this
>>> is happening repeatedly and quickly during extreme memory pressure,
>>> the constant alloc/free will clearly be worse than a simple internal
>>> calculation and failure. But would it ever happen repeatedly once the
>>> zram limit is reached?
>>>
>>> Now that I'm thinking about the limit from the perspective of the zram
>>> user, I wonder what really will happen. If zram is being used for
>>> swap space, then when swap starts getting errors trying to write
>>> pages, how damaging will that be to the system? I haven't checked
>>> what swap does when it encounters disk errors. Of course, with no
>>> zram limit, continually writing to zram until memory is totally
>>> consumed isn't good either. But in any case, I would hope that swap
>>> would not repeatedly hammer on a disk when it's getting write failures
>>> from it.
>>>
>>> Alternately, if zram was being used as a compressed ram disk for
>>> regular file storage, it's entirely up to the application to handle
>>> write failures, so it may continue to try to write to a full zram
>>> disk.
>>>
>>> As far as what the zsmalloc api would look like with the limit added,
>>> it would need a setter and getter function (adding it as a param to
>>> the create function would be optional i think). But more importantly,
>>> it would need to handle multiple ways of specifying the limit. In our
>>> specific current use cases, zram and zswap, each handles their
>>> internal limit differently - zswap currently uses a % of total ram as
>>> its limit (defaulting to 20), while with these patches zram will use a
>>> specific number of bytes as its limit (defaulting to no limit). If
>>> the limiting mechanism is moved into zsmalloc (and possibly zbud),
>>> then either both users need to use the same units (bytes or %ram), or
>>> zsmalloc/zbud need to be able to set their limit in either units. It
>>>
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 10:44 AM, David Horner wrote:
> On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman wrote:
>> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
>>> Hey Joonsoo,
>>>
>>> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> > Hey Joonsoo,
> >
> > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
> >> > struct bio_vec *bvec, u32 index,
> >> > ret = -ENOMEM;
> >> > goto out;
> >> > }
> >> > +
> >> > + if (zram->limit_pages &&
> >> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages)
> >> > {
> >> > + zs_free(meta->mem_pool, handle);
> >> > + ret = -ENOMEM;
> >> > + goto out;
> >> > + }
> >> > +
> >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> >>
> >> Hello,
> >>
> >> I don't follow up previous discussion, so I could be wrong.
> >> Why this enforcement should be here?
> >>
> >> I think that this has two problems.
> >> 1) alloc/free happens unnecessarilly if we have used memory over the
> >> limitation.
> >
> > True but firstly, I implemented the logic in zsmalloc, not zram but
> > as I described in cover-letter, it's not a requirement of zsmalloc
> > but zram so it should be in there. If every user want it in future,
> > then we could move the function into zsmalloc. That's what we
> > concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
>>>
>>>
>>> If we can add it in zsmalloc, it would be more clean and efficient
>>> for zram but as I said, at the moment, I didn't want to put zram's
>>> requirement into zsmalloc because to me, it's weird to enforce max
>>> limit to allocator. It's client's role, I think.
>>>
>>> If current implementation is expensive and rather hard to follow,
>>> It would be one reason to move the feature into zsmalloc but
>>> I don't think it makes critical trobule in zram usecase.
>>> See below.
>>>
>>> But I still open and will wait others's opinion.
>>> If other guys think zsmalloc is better place, I am willing to move
>>> it into zsmalloc.
>>
>> Moving it into zsmalloc would allow rejecting new zsmallocs before
>> actually crossing the limit, since it can calculate that internally.
>> However, with the current patches the limit will only be briefly
>> crossed, and it should not be crossed by a large amount. Now, if this
>> is happening repeatedly and quickly during extreme memory pressure,
>> the constant alloc/free will clearly be worse than a simple internal
>> calculation and failure. But would it ever happen repeatedly once the
>> zram limit is reached?
>>
>> Now that I'm thinking about the limit from the perspective of the zram
>> user, I wonder what really will happen. If zram is being used for
>> swap space, then when swap starts getting errors trying to write
>> pages, how damaging will that be to the system? I haven't checked
>> what swap does when it encounters disk errors. Of course, with no
>> zram limit, continually writing to zram until memory is totally
>> consumed isn't good either. But in any case, I would hope that swap
>> would not repeatedly hammer on a disk when it's getting write failures
>> from it.
>>
>> Alternately, if zram was being used as a compressed ram disk for
>> regular file storage, it's entirely up to the application to handle
>> write failures, so it may continue to try to write to a full zram
>> disk.
>>
>> As far as what the zsmalloc api would look like with the limit added,
>> it would need a setter and getter function (adding it as a param to
>> the create function would be optional i think). But more importantly,
>> it would need to handle multiple ways of specifying the limit. In our
>> specific current use cases, zram and zswap, each handles their
>> internal limit differently - zswap currently uses a % of total ram as
>> its limit (defaulting to 20), while with these patches zram will use a
>> specific number of bytes as its limit (defaulting to no limit). If
>> the limiting mechanism is moved into zsmalloc (and possibly zbud),
>> then either both users need to use the same units (bytes or %ram), or
>> zsmalloc/zbud need to be able to set their limit in either units. It
>> seems to me like keeping the limit in zram/zswap is currently
>> preferable, at least without both using the same limit units.
>>
>
> zswap knows what 20% (or
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman wrote:
> On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
>> Hey Joonsoo,
>>
>> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
>>> Hello, Minchan and David.
>>>
>>> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
>>> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
>>> > > Hey Joonsoo,
>>> > >
>>> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
>>> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
>>> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
>>> > >> > struct bio_vec *bvec, u32 index,
>>> > >> > ret = -ENOMEM;
>>> > >> > goto out;
>>> > >> > }
>>> > >> > +
>>> > >> > + if (zram->limit_pages &&
>>> > >> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
>>> > >> > + zs_free(meta->mem_pool, handle);
>>> > >> > + ret = -ENOMEM;
>>> > >> > + goto out;
>>> > >> > + }
>>> > >> > +
>>> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
>>> > >>
>>> > >> Hello,
>>> > >>
>>> > >> I don't follow up previous discussion, so I could be wrong.
>>> > >> Why this enforcement should be here?
>>> > >>
>>> > >> I think that this has two problems.
>>> > >> 1) alloc/free happens unnecessarilly if we have used memory over the
>>> > >> limitation.
>>> > >
>>> > > True but firstly, I implemented the logic in zsmalloc, not zram but
>>> > > as I described in cover-letter, it's not a requirement of zsmalloc
>>> > > but zram so it should be in there. If every user want it in future,
>>> > > then we could move the function into zsmalloc. That's what we
>>> > > concluded in previous discussion.
>>>
>>> Hmm...
>>> Problem is that we can't avoid these unnecessary overhead in this
>>> implementation. If we can implement this feature in zram efficiently,
>>> it's okay. But, I think that current form isn't.
>>
>>
>> If we can add it in zsmalloc, it would be more clean and efficient
>> for zram but as I said, at the moment, I didn't want to put zram's
>> requirement into zsmalloc because to me, it's weird to enforce max
>> limit to allocator. It's client's role, I think.
>>
>> If current implementation is expensive and rather hard to follow,
>> It would be one reason to move the feature into zsmalloc but
>> I don't think it makes critical trobule in zram usecase.
>> See below.
>>
>> But I still open and will wait others's opinion.
>> If other guys think zsmalloc is better place, I am willing to move
>> it into zsmalloc.
>
> Moving it into zsmalloc would allow rejecting new zsmallocs before
> actually crossing the limit, since it can calculate that internally.
> However, with the current patches the limit will only be briefly
> crossed, and it should not be crossed by a large amount. Now, if this
> is happening repeatedly and quickly during extreme memory pressure,
> the constant alloc/free will clearly be worse than a simple internal
> calculation and failure. But would it ever happen repeatedly once the
> zram limit is reached?
>
> Now that I'm thinking about the limit from the perspective of the zram
> user, I wonder what really will happen. If zram is being used for
> swap space, then when swap starts getting errors trying to write
> pages, how damaging will that be to the system? I haven't checked
> what swap does when it encounters disk errors. Of course, with no
> zram limit, continually writing to zram until memory is totally
> consumed isn't good either. But in any case, I would hope that swap
> would not repeatedly hammer on a disk when it's getting write failures
> from it.
>
> Alternately, if zram was being used as a compressed ram disk for
> regular file storage, it's entirely up to the application to handle
> write failures, so it may continue to try to write to a full zram
> disk.
>
> As far as what the zsmalloc api would look like with the limit added,
> it would need a setter and getter function (adding it as a param to
> the create function would be optional i think). But more importantly,
> it would need to handle multiple ways of specifying the limit. In our
> specific current use cases, zram and zswap, each handles their
> internal limit differently - zswap currently uses a % of total ram as
> its limit (defaulting to 20), while with these patches zram will use a
> specific number of bytes as its limit (defaulting to no limit). If
> the limiting mechanism is moved into zsmalloc (and possibly zbud),
> then either both users need to use the same units (bytes or %ram), or
> zsmalloc/zbud need to be able to set their limit in either units. It
> seems to me like keeping the limit in zram/zswap is currently
> preferable, at least without both using the same limit units.
>
zswap knows what 20% (or whatever % it currently uses , and perhaps it too
will become a tuning knob) of memory is in bytes.
So, if the interface to establish a limit for a pool (or pool set, or
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim wrote:
> Hey Joonsoo,
>
> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
>> Hello, Minchan and David.
>>
>> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
>> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
>> > > Hey Joonsoo,
>> > >
>> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
>> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
>> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
>> > >> > struct bio_vec *bvec, u32 index,
>> > >> > ret = -ENOMEM;
>> > >> > goto out;
>> > >> > }
>> > >> > +
>> > >> > + if (zram->limit_pages &&
>> > >> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
>> > >> > + zs_free(meta->mem_pool, handle);
>> > >> > + ret = -ENOMEM;
>> > >> > + goto out;
>> > >> > + }
>> > >> > +
>> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
>> > >>
>> > >> Hello,
>> > >>
>> > >> I don't follow up previous discussion, so I could be wrong.
>> > >> Why this enforcement should be here?
>> > >>
>> > >> I think that this has two problems.
>> > >> 1) alloc/free happens unnecessarilly if we have used memory over the
>> > >> limitation.
>> > >
>> > > True but firstly, I implemented the logic in zsmalloc, not zram but
>> > > as I described in cover-letter, it's not a requirement of zsmalloc
>> > > but zram so it should be in there. If every user want it in future,
>> > > then we could move the function into zsmalloc. That's what we
>> > > concluded in previous discussion.
>>
>> Hmm...
>> Problem is that we can't avoid these unnecessary overhead in this
>> implementation. If we can implement this feature in zram efficiently,
>> it's okay. But, I think that current form isn't.
>
>
> If we can add it in zsmalloc, it would be more clean and efficient
> for zram but as I said, at the moment, I didn't want to put zram's
> requirement into zsmalloc because to me, it's weird to enforce max
> limit to allocator. It's client's role, I think.
>
> If current implementation is expensive and rather hard to follow,
> It would be one reason to move the feature into zsmalloc but
> I don't think it makes critical trobule in zram usecase.
> See below.
>
> But I still open and will wait others's opinion.
> If other guys think zsmalloc is better place, I am willing to move
> it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
>
>>
>> > >
>> > > Another idea is we could call zs_get_total_pages right before zs_malloc
>> > > but the problem is we cannot know how many of pages are allocated
>> > > by zsmalloc in advance.
>> > > IOW, zram should be blind on zsmalloc's internal.
>> > >
>> >
>> > We did however suggest that we could check before hand to see if
>> > max was already exceeded as an optimization.
>> >
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 02:04:38PM +0900, Joonsoo Kim wrote:
> On Wed, Aug 27, 2014 at 11:51:32AM +0900, Minchan Kim wrote:
> > Hey Joonsoo,
> >
> > On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> > > Hello, Minchan and David.
> > >
> > > On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> > > > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> > > > > Hey Joonsoo,
> > > > >
> > > > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> > > > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> > > > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
> > > > >> > struct bio_vec *bvec, u32 index,
> > > > >> > ret = -ENOMEM;
> > > > >> > goto out;
> > > > >> > }
> > > > >> > +
> > > > >> > + if (zram->limit_pages &&
> > > > >> > + zs_get_total_pages(meta->mem_pool) >
> > > > >> > zram->limit_pages) {
> > > > >> > + zs_free(meta->mem_pool, handle);
> > > > >> > + ret = -ENOMEM;
> > > > >> > + goto out;
> > > > >> > + }
> > > > >> > +
> > > > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> > > > >>
> > > > >> Hello,
> > > > >>
> > > > >> I don't follow up previous discussion, so I could be wrong.
> > > > >> Why this enforcement should be here?
> > > > >>
> > > > >> I think that this has two problems.
> > > > >> 1) alloc/free happens unnecessarilly if we have used memory over the
> > > > >> limitation.
> > > > >
> > > > > True but firstly, I implemented the logic in zsmalloc, not zram but
> > > > > as I described in cover-letter, it's not a requirement of zsmalloc
> > > > > but zram so it should be in there. If every user want it in future,
> > > > > then we could move the function into zsmalloc. That's what we
> > > > > concluded in previous discussion.
> > >
> > > Hmm...
> > > Problem is that we can't avoid these unnecessary overhead in this
> > > implementation. If we can implement this feature in zram efficiently,
> > > it's okay. But, I think that current form isn't.
> >
> >
> > If we can add it in zsmalloc, it would be more clean and efficient
> > for zram but as I said, at the moment, I didn't want to put zram's
> > requirement into zsmalloc because to me, it's weird to enforce max
> > limit to allocator. It's client's role, I think.
>
> AFAIK, many kinds of pools such as thread-pool or memory-pool have
> their own limit. It's not weird for me.
Actually I don't know what is pool allocator but things you mentioned
is basically used to gaurantee *new* thread/memory, not limit although
it would implement limit.
Another question, why do you think zsmalloc is pool allocator?
IOW, What logic makes you think it's pool allocator?
>
> > If current implementation is expensive and rather hard to follow,
> > It would be one reason to move the feature into zsmalloc but
> > I don't think it makes critical trobule in zram usecase.
> > See below.
> >
> > But I still open and will wait others's opinion.
> > If other guys think zsmalloc is better place, I am willing to move
> > it into zsmalloc.
> >
> > >
> > > > >
> > > > > Another idea is we could call zs_get_total_pages right before
> > > > > zs_malloc
> > > > > but the problem is we cannot know how many of pages are allocated
> > > > > by zsmalloc in advance.
> > > > > IOW, zram should be blind on zsmalloc's internal.
> > > > >
> > > >
> > > > We did however suggest that we could check before hand to see if
> > > > max was already exceeded as an optimization.
> > > > (possibly with a guess on usage but at least using the minimum of 1
> > > > page)
> > > > In the contested case, the max may already be exceeded transiently and
> > > > therefore we know this one _could_ fail (it could also pass, but odds
> > > > aren't good).
> > > > As Minchan mentions this was discussed before - but not into great
> > > > detail.
> > > > Testing should be done to determine possible benefit. And as he also
> > > > mentions, the better place for it may be in zsmalloc, but that
> > > > requires an ABI change.
> > >
> > > Why we hesitate to change zsmalloc API? It is in-kernel API and there
> > > are just two users now, zswap and zram. We can change it easily.
> > > I think that we just need following simple API change in zsmalloc.c.
> > >
> > > zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
> > > =>
> > > zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
> > > *zpool_op)
> > >
> > > It's pool allocator so there is no obstacle for us to limit maximum
> > > memory usage in zsmalloc. It's a natural idea to limit memory usage
> > > for pool allocator.
> > >
> > > > Certainly a detailed suggestion could happen on this thread and I'm
> > > > also interested
> > > > in your thoughts, but this patchset should be able to go in as is.
> > > > Memory exhaustion avoidance probably trumps the possible thrashing at
> > > > threshold.
> > > >
> > > > > About alloc/free cost once if it is over
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 02:04:38PM +0900, Joonsoo Kim wrote:
On Wed, Aug 27, 2014 at 11:51:32AM +0900, Minchan Kim wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
AFAIK, many kinds of pools such as thread-pool or memory-pool have
their own limit. It's not weird for me.
Actually I don't know what is pool allocator but things you mentioned
is basically used to gaurantee *new* thread/memory, not limit although
it would implement limit.
Another question, why do you think zsmalloc is pool allocator?
IOW, What logic makes you think it's pool allocator?
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Another idea is we could call zs_get_total_pages right before
zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
We did however suggest that we could check before hand to see if
max was already exceeded as an optimization.
(possibly with a guess on usage but at least using the minimum of 1
page)
In the contested case, the max may already be exceeded transiently and
therefore we know this one _could_ fail (it could also pass, but odds
aren't good).
As Minchan mentions this was discussed before - but not into great
detail.
Testing should be done to determine possible benefit. And as he also
mentions, the better place for it may be in zsmalloc, but that
requires an ABI change.
Why we hesitate to change zsmalloc API? It is in-kernel API and there
are just two users now, zswap and zram. We can change it easily.
I think that we just need following simple API change in zsmalloc.c.
zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
=
zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
*zpool_op)
It's pool allocator so there is no obstacle for us to limit maximum
memory usage in zsmalloc. It's a natural idea to limit memory usage
for pool allocator.
Certainly a detailed suggestion could happen on this thread and I'm
also interested
in your thoughts, but this patchset should be able to go in as is.
Memory exhaustion avoidance probably trumps the possible thrashing at
threshold.
About alloc/free cost once if it is over the limit,
I don't think it's important to consider.
Do you have any scenario in your mind to consider alloc/free cost
when the limit is over?
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
Another idea is we could call zs_get_total_pages right before zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
We did however suggest that we could check before hand to see if
max was already exceeded as an optimization.
(possibly with a guess on usage but at least using the minimum of 1 page)
In the contested case, the max may already be exceeded transiently and
therefore we know this one _could_ fail (it could also pass, but odds
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
zswap knows what 20% (or whatever % it currently uses , and perhaps it too
will become a tuning knob) of memory is in bytes.
So, if the interface to establish a limit for a pool (or pool set, or whatever
zsmalloc sets up for its allocation mechanism) is stipulated in bytes
(to actually use pages internally, of visa-versa) , then both can use
that interface.
zram with its native page stipulation, and zswap with calculated % of memory).
Both would need a
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 10:44 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages)
{
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
zswap knows what 20% (or whatever % it currently uses , and perhaps it too
will become a tuning knob) of memory is in bytes.
So, if the interface to establish a limit for a pool (or pool set, or whatever
zsmalloc sets up for its allocation mechanism) is stipulated in bytes
(to actually use pages internally, of visa-versa) , then both can use
that interface.
zram with
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 10:44 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
zswap knows what 20% (or whatever % it currently uses , and perhaps it too
will become a tuning knob) of memory is in bytes.
So, if the interface to establish a limit for a pool (or pool set, or
whatever
zsmalloc sets up for its allocation mechanism) is stipulated in bytes
(to actually use pages
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 11:35 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 10:44 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org
wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
*zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
zswap knows what 20% (or whatever % it currently uses , and perhaps it too
will become a tuning knob) of memory is in bytes.
So, if the interface to establish a limit for a pool (or pool set, or
whatever
zsmalloc sets up
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 12:29 PM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 11:35 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 10:44 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org
wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
*zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over
the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
zswap knows what 20% (or whatever % it currently uses , and perhaps it too
will become a tuning knob) of memory is in bytes.
So, if the interface
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 12:59 PM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 12:29 PM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 11:35 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 10:44 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org
wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
*zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over
the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
zswap knows what 20% (or whatever % it currently uses , and perhaps it too
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hello,
On Wed, Aug 27, 2014 at 10:03:45AM -0400, Dan Streetman wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages)
{
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Right. it depends on user.
If user writes continuously without any action to cover the situation
once the limit is over, it would be terrible but what I meant *terrible*
doesn't mean alloc/free cost. Actually, zsmalloc/zsfree cost is really
cheaper comparing to other mm reclaim, swap, vfs, fs, block layer's one.
What I meant *terrible* is slower system caused by swapout failure
on system but it try to reclaim anonymous pages continuously.
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Good point. Actually, it's my next step.
Curretly, VM doesn't handle congestion for anonymous page while it is
doing something for file-backed pages(but actually, it's really basic
stuff at this moment) so we could improve it with several ways.
I'm looking at it now.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 12:29:22PM -0400, Dan Streetman wrote:
On Wed, Aug 27, 2014 at 11:35 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 10:44 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org
wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
*zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over
the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to me like keeping the limit in zram/zswap is currently
preferable, at least without both using the same limit units.
zswap knows what 20% (or whatever % it currently
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 03:04:32PM -0400, Dan Streetman wrote:
On Wed, Aug 27, 2014 at 12:59 PM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 12:29 PM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 11:35 AM, David Horner ds2hor...@gmail.com wrote:
On Wed, Aug 27, 2014 at 11:14 AM, Dan Streetman ddstr...@ieee.org wrote:
On Wed, Aug 27, 2014 at 10:44 AM, David Horner ds2hor...@gmail.com
wrote:
On Wed, Aug 27, 2014 at 10:03 AM, Dan Streetman ddstr...@ieee.org
wrote:
On Tue, Aug 26, 2014 at 10:51 PM, Minchan Kim minc...@kernel.org
wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org
wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram
*zram, struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool)
zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory
over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram
but
as I described in cover-letter, it's not a requirement of
zsmalloc
but zram so it should be in there. If every user want it in
future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Moving it into zsmalloc would allow rejecting new zsmallocs before
actually crossing the limit, since it can calculate that internally.
However, with the current patches the limit will only be briefly
crossed, and it should not be crossed by a large amount. Now, if this
is happening repeatedly and quickly during extreme memory pressure,
the constant alloc/free will clearly be worse than a simple internal
calculation and failure. But would it ever happen repeatedly once the
zram limit is reached?
Now that I'm thinking about the limit from the perspective of the zram
user, I wonder what really will happen. If zram is being used for
swap space, then when swap starts getting errors trying to write
pages, how damaging will that be to the system? I haven't checked
what swap does when it encounters disk errors. Of course, with no
zram limit, continually writing to zram until memory is totally
consumed isn't good either. But in any case, I would hope that swap
would not repeatedly hammer on a disk when it's getting write failures
from it.
Alternately, if zram was being used as a compressed ram disk for
regular file storage, it's entirely up to the application to handle
write failures, so it may continue to try to write to a full zram
disk.
As far as what the zsmalloc api would look like with the limit added,
it would need a setter and getter function (adding it as a param to
the create function would be optional i think). But more importantly,
it would need to handle multiple ways of specifying the limit. In our
specific current use cases, zram and zswap, each handles their
internal limit differently - zswap currently uses a % of total ram as
its limit (defaulting to 20), while with these patches zram will use a
specific number of bytes as its limit (defaulting to no limit). If
the limiting mechanism is moved into zsmalloc (and possibly zbud),
then either both users need to use the same units (bytes or %ram), or
zsmalloc/zbud need to be able to set their limit in either units. It
seems to
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 11:51:32AM +0900, Minchan Kim wrote:
> Hey Joonsoo,
>
> On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> > Hello, Minchan and David.
> >
> > On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> > > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> > > > Hey Joonsoo,
> > > >
> > > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> > > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> > > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
> > > >> > struct bio_vec *bvec, u32 index,
> > > >> > ret = -ENOMEM;
> > > >> > goto out;
> > > >> > }
> > > >> > +
> > > >> > + if (zram->limit_pages &&
> > > >> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
> > > >> > + zs_free(meta->mem_pool, handle);
> > > >> > + ret = -ENOMEM;
> > > >> > + goto out;
> > > >> > + }
> > > >> > +
> > > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> > > >>
> > > >> Hello,
> > > >>
> > > >> I don't follow up previous discussion, so I could be wrong.
> > > >> Why this enforcement should be here?
> > > >>
> > > >> I think that this has two problems.
> > > >> 1) alloc/free happens unnecessarilly if we have used memory over the
> > > >> limitation.
> > > >
> > > > True but firstly, I implemented the logic in zsmalloc, not zram but
> > > > as I described in cover-letter, it's not a requirement of zsmalloc
> > > > but zram so it should be in there. If every user want it in future,
> > > > then we could move the function into zsmalloc. That's what we
> > > > concluded in previous discussion.
> >
> > Hmm...
> > Problem is that we can't avoid these unnecessary overhead in this
> > implementation. If we can implement this feature in zram efficiently,
> > it's okay. But, I think that current form isn't.
>
>
> If we can add it in zsmalloc, it would be more clean and efficient
> for zram but as I said, at the moment, I didn't want to put zram's
> requirement into zsmalloc because to me, it's weird to enforce max
> limit to allocator. It's client's role, I think.
AFAIK, many kinds of pools such as thread-pool or memory-pool have
their own limit. It's not weird for me.
> If current implementation is expensive and rather hard to follow,
> It would be one reason to move the feature into zsmalloc but
> I don't think it makes critical trobule in zram usecase.
> See below.
>
> But I still open and will wait others's opinion.
> If other guys think zsmalloc is better place, I am willing to move
> it into zsmalloc.
>
> >
> > > >
> > > > Another idea is we could call zs_get_total_pages right before zs_malloc
> > > > but the problem is we cannot know how many of pages are allocated
> > > > by zsmalloc in advance.
> > > > IOW, zram should be blind on zsmalloc's internal.
> > > >
> > >
> > > We did however suggest that we could check before hand to see if
> > > max was already exceeded as an optimization.
> > > (possibly with a guess on usage but at least using the minimum of 1 page)
> > > In the contested case, the max may already be exceeded transiently and
> > > therefore we know this one _could_ fail (it could also pass, but odds
> > > aren't good).
> > > As Minchan mentions this was discussed before - but not into great detail.
> > > Testing should be done to determine possible benefit. And as he also
> > > mentions, the better place for it may be in zsmalloc, but that
> > > requires an ABI change.
> >
> > Why we hesitate to change zsmalloc API? It is in-kernel API and there
> > are just two users now, zswap and zram. We can change it easily.
> > I think that we just need following simple API change in zsmalloc.c.
> >
> > zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
> > =>
> > zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
> > *zpool_op)
> >
> > It's pool allocator so there is no obstacle for us to limit maximum
> > memory usage in zsmalloc. It's a natural idea to limit memory usage
> > for pool allocator.
> >
> > > Certainly a detailed suggestion could happen on this thread and I'm
> > > also interested
> > > in your thoughts, but this patchset should be able to go in as is.
> > > Memory exhaustion avoidance probably trumps the possible thrashing at
> > > threshold.
> > >
> > > > About alloc/free cost once if it is over the limit,
> > > > I don't think it's important to consider.
> > > > Do you have any scenario in your mind to consider alloc/free cost
> > > > when the limit is over?
> > > >
> > > >> 2) Even if this request doesn't do new allocation, it could be failed
> > > >> due to other's allocation. There is time gap between allocation and
> > > >> free, so legimate user who want to use preallocated zsmalloc memory
> > > >> could also see this condition true and then he will be failed.
> > > >
> > > > Yeb, we already discussed that. :)
> > > > Such false positive shouldn't be a severe problem if we
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
> Hello, Minchan and David.
>
> On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> > On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> > > Hey Joonsoo,
> > >
> > > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> > >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> > >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
> > >> > struct bio_vec *bvec, u32 index,
> > >> > ret = -ENOMEM;
> > >> > goto out;
> > >> > }
> > >> > +
> > >> > + if (zram->limit_pages &&
> > >> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
> > >> > + zs_free(meta->mem_pool, handle);
> > >> > + ret = -ENOMEM;
> > >> > + goto out;
> > >> > + }
> > >> > +
> > >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> > >>
> > >> Hello,
> > >>
> > >> I don't follow up previous discussion, so I could be wrong.
> > >> Why this enforcement should be here?
> > >>
> > >> I think that this has two problems.
> > >> 1) alloc/free happens unnecessarilly if we have used memory over the
> > >> limitation.
> > >
> > > True but firstly, I implemented the logic in zsmalloc, not zram but
> > > as I described in cover-letter, it's not a requirement of zsmalloc
> > > but zram so it should be in there. If every user want it in future,
> > > then we could move the function into zsmalloc. That's what we
> > > concluded in previous discussion.
>
> Hmm...
> Problem is that we can't avoid these unnecessary overhead in this
> implementation. If we can implement this feature in zram efficiently,
> it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
>
> > >
> > > Another idea is we could call zs_get_total_pages right before zs_malloc
> > > but the problem is we cannot know how many of pages are allocated
> > > by zsmalloc in advance.
> > > IOW, zram should be blind on zsmalloc's internal.
> > >
> >
> > We did however suggest that we could check before hand to see if
> > max was already exceeded as an optimization.
> > (possibly with a guess on usage but at least using the minimum of 1 page)
> > In the contested case, the max may already be exceeded transiently and
> > therefore we know this one _could_ fail (it could also pass, but odds
> > aren't good).
> > As Minchan mentions this was discussed before - but not into great detail.
> > Testing should be done to determine possible benefit. And as he also
> > mentions, the better place for it may be in zsmalloc, but that
> > requires an ABI change.
>
> Why we hesitate to change zsmalloc API? It is in-kernel API and there
> are just two users now, zswap and zram. We can change it easily.
> I think that we just need following simple API change in zsmalloc.c.
>
> zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
> =>
> zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
> *zpool_op)
>
> It's pool allocator so there is no obstacle for us to limit maximum
> memory usage in zsmalloc. It's a natural idea to limit memory usage
> for pool allocator.
>
> > Certainly a detailed suggestion could happen on this thread and I'm
> > also interested
> > in your thoughts, but this patchset should be able to go in as is.
> > Memory exhaustion avoidance probably trumps the possible thrashing at
> > threshold.
> >
> > > About alloc/free cost once if it is over the limit,
> > > I don't think it's important to consider.
> > > Do you have any scenario in your mind to consider alloc/free cost
> > > when the limit is over?
> > >
> > >> 2) Even if this request doesn't do new allocation, it could be failed
> > >> due to other's allocation. There is time gap between allocation and
> > >> free, so legimate user who want to use preallocated zsmalloc memory
> > >> could also see this condition true and then he will be failed.
> > >
> > > Yeb, we already discussed that. :)
> > > Such false positive shouldn't be a severe problem if we can keep a
> > > promise that zram user cannot exceed mem_limit.
> > >
>
> If we can keep such a promise, why we need to limit memory usage?
> I guess that this limit feature is useful for user who can't keep such
> promise.
> So, we should assume that this false positive happens frequently.
The goal is to limit memory usage within some threshold.
so false positive shouldn't be
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
> On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> > Hey Joonsoo,
> >
> > On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> >> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> >> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
> >> > struct bio_vec *bvec, u32 index,
> >> > ret = -ENOMEM;
> >> > goto out;
> >> > }
> >> > +
> >> > + if (zram->limit_pages &&
> >> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
> >> > + zs_free(meta->mem_pool, handle);
> >> > + ret = -ENOMEM;
> >> > + goto out;
> >> > + }
> >> > +
> >> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
> >>
> >> Hello,
> >>
> >> I don't follow up previous discussion, so I could be wrong.
> >> Why this enforcement should be here?
> >>
> >> I think that this has two problems.
> >> 1) alloc/free happens unnecessarilly if we have used memory over the
> >> limitation.
> >
> > True but firstly, I implemented the logic in zsmalloc, not zram but
> > as I described in cover-letter, it's not a requirement of zsmalloc
> > but zram so it should be in there. If every user want it in future,
> > then we could move the function into zsmalloc. That's what we
> > concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
> >
> > Another idea is we could call zs_get_total_pages right before zs_malloc
> > but the problem is we cannot know how many of pages are allocated
> > by zsmalloc in advance.
> > IOW, zram should be blind on zsmalloc's internal.
> >
>
> We did however suggest that we could check before hand to see if
> max was already exceeded as an optimization.
> (possibly with a guess on usage but at least using the minimum of 1 page)
> In the contested case, the max may already be exceeded transiently and
> therefore we know this one _could_ fail (it could also pass, but odds
> aren't good).
> As Minchan mentions this was discussed before - but not into great detail.
> Testing should be done to determine possible benefit. And as he also
> mentions, the better place for it may be in zsmalloc, but that
> requires an ABI change.
Why we hesitate to change zsmalloc API? It is in-kernel API and there
are just two users now, zswap and zram. We can change it easily.
I think that we just need following simple API change in zsmalloc.c.
zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
=>
zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
*zpool_op)
It's pool allocator so there is no obstacle for us to limit maximum
memory usage in zsmalloc. It's a natural idea to limit memory usage
for pool allocator.
> Certainly a detailed suggestion could happen on this thread and I'm
> also interested
> in your thoughts, but this patchset should be able to go in as is.
> Memory exhaustion avoidance probably trumps the possible thrashing at
> threshold.
>
> > About alloc/free cost once if it is over the limit,
> > I don't think it's important to consider.
> > Do you have any scenario in your mind to consider alloc/free cost
> > when the limit is over?
> >
> >> 2) Even if this request doesn't do new allocation, it could be failed
> >> due to other's allocation. There is time gap between allocation and
> >> free, so legimate user who want to use preallocated zsmalloc memory
> >> could also see this condition true and then he will be failed.
> >
> > Yeb, we already discussed that. :)
> > Such false positive shouldn't be a severe problem if we can keep a
> > promise that zram user cannot exceed mem_limit.
> >
If we can keep such a promise, why we need to limit memory usage?
I guess that this limit feature is useful for user who can't keep such promise.
So, we should assume that this false positive happens frequently.
> And we cannot avoid the race, nor can we avoid in a low overhead competitive
> concurrent process transient inconsistent states.
> Different views for different observers.
> They are a consequence of the theory of "Special Computational Relativity".
> I am working on a String Unification Theory of Quantum and General CR in
> LISP.
> ;-)
If we move limit logic to zsmalloc, we can avoid the race by commiting
needed memory size before actual allocation attempt. This commiting makes
concurrent process serialized so there is no race here. There is
possibilty to fail to allocate, but I think this is better than alloc
and free blindlessly depending on inconsistent states.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim wrote:
> Hey Joonsoo,
>
> On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
>> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
>> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
>> > bio_vec *bvec, u32 index,
>> > ret = -ENOMEM;
>> > goto out;
>> > }
>> > +
>> > + if (zram->limit_pages &&
>> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
>> > + zs_free(meta->mem_pool, handle);
>> > + ret = -ENOMEM;
>> > + goto out;
>> > + }
>> > +
>> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
>>
>> Hello,
>>
>> I don't follow up previous discussion, so I could be wrong.
>> Why this enforcement should be here?
>>
>> I think that this has two problems.
>> 1) alloc/free happens unnecessarilly if we have used memory over the
>> limitation.
>
> True but firstly, I implemented the logic in zsmalloc, not zram but
> as I described in cover-letter, it's not a requirement of zsmalloc
> but zram so it should be in there. If every user want it in future,
> then we could move the function into zsmalloc. That's what we
> concluded in previous discussion.
>
> Another idea is we could call zs_get_total_pages right before zs_malloc
> but the problem is we cannot know how many of pages are allocated
> by zsmalloc in advance.
> IOW, zram should be blind on zsmalloc's internal.
>
We did however suggest that we could check before hand to see if
max was already exceeded as an optimization.
(possibly with a guess on usage but at least using the minimum of 1 page)
In the contested case, the max may already be exceeded transiently and
therefore we know this one _could_ fail (it could also pass, but odds
aren't good).
As Minchan mentions this was discussed before - but not into great detail.
Testing should be done to determine possible benefit. And as he also
mentions, the better place for it may be in zsmalloc, but that
requires an ABI change.
Certainly a detailed suggestion could happen on this thread and I'm
also interested
in your thoughts, but this patchset should be able to go in as is.
Memory exhaustion avoidance probably trumps the possible thrashing at
threshold.
> About alloc/free cost once if it is over the limit,
> I don't think it's important to consider.
> Do you have any scenario in your mind to consider alloc/free cost
> when the limit is over?
>
>> 2) Even if this request doesn't do new allocation, it could be failed
>> due to other's allocation. There is time gap between allocation and
>> free, so legimate user who want to use preallocated zsmalloc memory
>> could also see this condition true and then he will be failed.
>
> Yeb, we already discussed that. :)
> Such false positive shouldn't be a severe problem if we can keep a
> promise that zram user cannot exceed mem_limit.
>
And we cannot avoid the race, nor can we avoid in a low overhead competitive
concurrent process transient inconsistent states.
Different views for different observers.
They are a consequence of the theory of "Special Computational Relativity".
I am working on a String Unification Theory of Quantum and General CR in LISP.
;-)
>>
>> Thanks.
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majord...@kvack.org. For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Don't email: mailto:"d...@kvack.org;> em...@kvack.org
>
> --
> Kind regards,
> Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
> On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> > @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
> > bio_vec *bvec, u32 index,
> > ret = -ENOMEM;
> > goto out;
> > }
> > +
> > + if (zram->limit_pages &&
> > + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
> > + zs_free(meta->mem_pool, handle);
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
>
> Hello,
>
> I don't follow up previous discussion, so I could be wrong.
> Why this enforcement should be here?
>
> I think that this has two problems.
> 1) alloc/free happens unnecessarilly if we have used memory over the
> limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Another idea is we could call zs_get_total_pages right before zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
About alloc/free cost once if it is over the limit,
I don't think it's important to consider.
Do you have any scenario in your mind to consider alloc/free cost
when the limit is over?
> 2) Even if this request doesn't do new allocation, it could be failed
> due to other's allocation. There is time gap between allocation and
> free, so legimate user who want to use preallocated zsmalloc memory
> could also see this condition true and then he will be failed.
Yeb, we already discussed that. :)
Such false positive shouldn't be a severe problem if we can keep a
promise that zram user cannot exceed mem_limit.
>
> Thanks.
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
> @@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
> bio_vec *bvec, u32 index,
> ret = -ENOMEM;
> goto out;
> }
> +
> + if (zram->limit_pages &&
> + zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
> + zs_free(meta->mem_pool, handle);
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and then he will be failed.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and then he will be failed.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Another idea is we could call zs_get_total_pages right before zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
About alloc/free cost once if it is over the limit,
I don't think it's important to consider.
Do you have any scenario in your mind to consider alloc/free cost
when the limit is over?
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and then he will be failed.
Yeb, we already discussed that. :)
Such false positive shouldn't be a severe problem if we can keep a
promise that zram user cannot exceed mem_limit.
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Another idea is we could call zs_get_total_pages right before zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
We did however suggest that we could check before hand to see if
max was already exceeded as an optimization.
(possibly with a guess on usage but at least using the minimum of 1 page)
In the contested case, the max may already be exceeded transiently and
therefore we know this one _could_ fail (it could also pass, but odds
aren't good).
As Minchan mentions this was discussed before - but not into great detail.
Testing should be done to determine possible benefit. And as he also
mentions, the better place for it may be in zsmalloc, but that
requires an ABI change.
Certainly a detailed suggestion could happen on this thread and I'm
also interested
in your thoughts, but this patchset should be able to go in as is.
Memory exhaustion avoidance probably trumps the possible thrashing at
threshold.
About alloc/free cost once if it is over the limit,
I don't think it's important to consider.
Do you have any scenario in your mind to consider alloc/free cost
when the limit is over?
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and then he will be failed.
Yeb, we already discussed that. :)
Such false positive shouldn't be a severe problem if we can keep a
promise that zram user cannot exceed mem_limit.
And we cannot avoid the race, nor can we avoid in a low overhead competitive
concurrent process transient inconsistent states.
Different views for different observers.
They are a consequence of the theory of Special Computational Relativity.
I am working on a String Unification Theory of Quantum and General CR in LISP.
;-)
Thanks.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majord...@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: a href=mailto:d...@kvack.org; em...@kvack.org /a
--
Kind regards,
Minchan Kim
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
Another idea is we could call zs_get_total_pages right before zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
We did however suggest that we could check before hand to see if
max was already exceeded as an optimization.
(possibly with a guess on usage but at least using the minimum of 1 page)
In the contested case, the max may already be exceeded transiently and
therefore we know this one _could_ fail (it could also pass, but odds
aren't good).
As Minchan mentions this was discussed before - but not into great detail.
Testing should be done to determine possible benefit. And as he also
mentions, the better place for it may be in zsmalloc, but that
requires an ABI change.
Why we hesitate to change zsmalloc API? It is in-kernel API and there
are just two users now, zswap and zram. We can change it easily.
I think that we just need following simple API change in zsmalloc.c.
zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
=
zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
*zpool_op)
It's pool allocator so there is no obstacle for us to limit maximum
memory usage in zsmalloc. It's a natural idea to limit memory usage
for pool allocator.
Certainly a detailed suggestion could happen on this thread and I'm
also interested
in your thoughts, but this patchset should be able to go in as is.
Memory exhaustion avoidance probably trumps the possible thrashing at
threshold.
About alloc/free cost once if it is over the limit,
I don't think it's important to consider.
Do you have any scenario in your mind to consider alloc/free cost
when the limit is over?
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and then he will be failed.
Yeb, we already discussed that. :)
Such false positive shouldn't be a severe problem if we can keep a
promise that zram user cannot exceed mem_limit.
If we can keep such a promise, why we need to limit memory usage?
I guess that this limit feature is useful for user who can't keep such promise.
So, we should assume that this false positive happens frequently.
And we cannot avoid the race, nor can we avoid in a low overhead competitive
concurrent process transient inconsistent states.
Different views for different observers.
They are a consequence of the theory of Special Computational Relativity.
I am working on a String Unification Theory of Quantum and General CR in
LISP.
;-)
If we move limit logic to zsmalloc, we can avoid the race by commiting
needed memory size before actual allocation attempt. This commiting makes
concurrent process serialized so there is no race here. There is
possibilty to fail to allocate, but I think this is better than alloc
and free blindlessly depending on inconsistent states.
Thanks.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Another idea is we could call zs_get_total_pages right before zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
We did however suggest that we could check before hand to see if
max was already exceeded as an optimization.
(possibly with a guess on usage but at least using the minimum of 1 page)
In the contested case, the max may already be exceeded transiently and
therefore we know this one _could_ fail (it could also pass, but odds
aren't good).
As Minchan mentions this was discussed before - but not into great detail.
Testing should be done to determine possible benefit. And as he also
mentions, the better place for it may be in zsmalloc, but that
requires an ABI change.
Why we hesitate to change zsmalloc API? It is in-kernel API and there
are just two users now, zswap and zram. We can change it easily.
I think that we just need following simple API change in zsmalloc.c.
zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
=
zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
*zpool_op)
It's pool allocator so there is no obstacle for us to limit maximum
memory usage in zsmalloc. It's a natural idea to limit memory usage
for pool allocator.
Certainly a detailed suggestion could happen on this thread and I'm
also interested
in your thoughts, but this patchset should be able to go in as is.
Memory exhaustion avoidance probably trumps the possible thrashing at
threshold.
About alloc/free cost once if it is over the limit,
I don't think it's important to consider.
Do you have any scenario in your mind to consider alloc/free cost
when the limit is over?
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and then he will be failed.
Yeb, we already discussed that. :)
Such false positive shouldn't be a severe problem if we can keep a
promise that zram user cannot exceed mem_limit.
If we can keep such a promise, why we need to limit memory usage?
I guess that this limit feature is useful for user who can't keep such
promise.
So, we should assume that this false positive happens frequently.
The goal is to limit memory usage within some threshold.
so false positive shouldn't be harmful unless it exceeds the threshold.
In addition, If such false positive happens frequently, it means
zram is very trobule so that user would see lots of write fail
message, sometime really slow system if zram is used for swap.
If we protect just one write
Re: [PATCH v5 3/4] zram: zram memory size limitation
On Wed, Aug 27, 2014 at 11:51:32AM +0900, Minchan Kim wrote:
Hey Joonsoo,
On Wed, Aug 27, 2014 at 10:26:11AM +0900, Joonsoo Kim wrote:
Hello, Minchan and David.
On Tue, Aug 26, 2014 at 08:22:29AM -0400, David Horner wrote:
On Tue, Aug 26, 2014 at 3:55 AM, Minchan Kim minc...@kernel.org wrote:
Hey Joonsoo,
On Tue, Aug 26, 2014 at 04:37:30PM +0900, Joonsoo Kim wrote:
On Mon, Aug 25, 2014 at 09:05:55AM +0900, Minchan Kim wrote:
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram,
struct bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool, handle, ZS_MM_WO);
Hello,
I don't follow up previous discussion, so I could be wrong.
Why this enforcement should be here?
I think that this has two problems.
1) alloc/free happens unnecessarilly if we have used memory over the
limitation.
True but firstly, I implemented the logic in zsmalloc, not zram but
as I described in cover-letter, it's not a requirement of zsmalloc
but zram so it should be in there. If every user want it in future,
then we could move the function into zsmalloc. That's what we
concluded in previous discussion.
Hmm...
Problem is that we can't avoid these unnecessary overhead in this
implementation. If we can implement this feature in zram efficiently,
it's okay. But, I think that current form isn't.
If we can add it in zsmalloc, it would be more clean and efficient
for zram but as I said, at the moment, I didn't want to put zram's
requirement into zsmalloc because to me, it's weird to enforce max
limit to allocator. It's client's role, I think.
AFAIK, many kinds of pools such as thread-pool or memory-pool have
their own limit. It's not weird for me.
If current implementation is expensive and rather hard to follow,
It would be one reason to move the feature into zsmalloc but
I don't think it makes critical trobule in zram usecase.
See below.
But I still open and will wait others's opinion.
If other guys think zsmalloc is better place, I am willing to move
it into zsmalloc.
Another idea is we could call zs_get_total_pages right before zs_malloc
but the problem is we cannot know how many of pages are allocated
by zsmalloc in advance.
IOW, zram should be blind on zsmalloc's internal.
We did however suggest that we could check before hand to see if
max was already exceeded as an optimization.
(possibly with a guess on usage but at least using the minimum of 1 page)
In the contested case, the max may already be exceeded transiently and
therefore we know this one _could_ fail (it could also pass, but odds
aren't good).
As Minchan mentions this was discussed before - but not into great detail.
Testing should be done to determine possible benefit. And as he also
mentions, the better place for it may be in zsmalloc, but that
requires an ABI change.
Why we hesitate to change zsmalloc API? It is in-kernel API and there
are just two users now, zswap and zram. We can change it easily.
I think that we just need following simple API change in zsmalloc.c.
zs_zpool_create(gfp_t gfp, struct zpool_ops *zpool_op)
=
zs_zpool_create(unsigned long limit, gfp_t gfp, struct zpool_ops
*zpool_op)
It's pool allocator so there is no obstacle for us to limit maximum
memory usage in zsmalloc. It's a natural idea to limit memory usage
for pool allocator.
Certainly a detailed suggestion could happen on this thread and I'm
also interested
in your thoughts, but this patchset should be able to go in as is.
Memory exhaustion avoidance probably trumps the possible thrashing at
threshold.
About alloc/free cost once if it is over the limit,
I don't think it's important to consider.
Do you have any scenario in your mind to consider alloc/free cost
when the limit is over?
2) Even if this request doesn't do new allocation, it could be failed
due to other's allocation. There is time gap between allocation and
free, so legimate user who want to use preallocated zsmalloc memory
could also see this condition true and then he will be failed.
Yeb, we already discussed that. :)
Such false positive shouldn't be a severe problem if we can keep a
promise that zram user cannot exceed mem_limit.
If we can keep such a promise, why we need to limit memory usage?
I guess that this limit feature is useful for user who can't keep such
promise.
So, we should assume that this false positive happens frequently.
The goal is to limit memory usage within some threshold.
so
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hey Sergey, On Mon, Aug 25, 2014 at 08:09:27PM +0900, Sergey Senozhatsky wrote: > On (08/25/14 09:05), Minchan Kim wrote: > > Since zram has no control feature to limit memory usage, > > it makes hard to manage system memrory. > > > > This patch adds new knob "mem_limit" via sysfs to set up the > > a limit so that zram could fail allocation once it reaches > > the limit. > > > > In addition, user could change the limit in runtime so that > > he could manage the memory more dynamically. > > > > Initial state is no limit so it doesn't break old behavior. > > > > Signed-off-by: Minchan Kim > > --- > > Documentation/ABI/testing/sysfs-block-zram | 10 > > Documentation/blockdev/zram.txt| 24 ++--- > > drivers/block/zram/zram_drv.c | 41 > > ++ > > drivers/block/zram/zram_drv.h | 5 > > 4 files changed, 76 insertions(+), 4 deletions(-) > > > > diff --git a/Documentation/ABI/testing/sysfs-block-zram > > b/Documentation/ABI/testing/sysfs-block-zram > > index 70ec992514d0..dbe643775ec1 100644 > > --- a/Documentation/ABI/testing/sysfs-block-zram > > +++ b/Documentation/ABI/testing/sysfs-block-zram > > @@ -119,3 +119,13 @@ Description: > > efficiency can be calculated using compr_data_size and this > > statistic. > > Unit: bytes > > + > > +What: /sys/block/zram/mem_limit > > +Date: August 2014 > > +Contact: Minchan Kim > > +Description: > > + The mem_limit file is read/write and specifies the amount > > + of memory to be able to consume memory to store store > > + compressed data. The limit could be changed in run time > > + and "0" means disable the limit. No limit is the initial state. > > just a nitpick, sorry. > "the amount of memory to be able to consume memory to store store compressed > data" > ^^^ > > "the maximum amount of memory ZRAM can use to store the compressed data"? Will fix. Thanks. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 3/4] zram: zram memory size limitation
On (08/25/14 09:05), Minchan Kim wrote:
> Since zram has no control feature to limit memory usage,
> it makes hard to manage system memrory.
>
> This patch adds new knob "mem_limit" via sysfs to set up the
> a limit so that zram could fail allocation once it reaches
> the limit.
>
> In addition, user could change the limit in runtime so that
> he could manage the memory more dynamically.
>
> Initial state is no limit so it doesn't break old behavior.
>
> Signed-off-by: Minchan Kim
> ---
> Documentation/ABI/testing/sysfs-block-zram | 10
> Documentation/blockdev/zram.txt| 24 ++---
> drivers/block/zram/zram_drv.c | 41
> ++
> drivers/block/zram/zram_drv.h | 5
> 4 files changed, 76 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-block-zram
> b/Documentation/ABI/testing/sysfs-block-zram
> index 70ec992514d0..dbe643775ec1 100644
> --- a/Documentation/ABI/testing/sysfs-block-zram
> +++ b/Documentation/ABI/testing/sysfs-block-zram
> @@ -119,3 +119,13 @@ Description:
> efficiency can be calculated using compr_data_size and this
> statistic.
> Unit: bytes
> +
> +What:/sys/block/zram/mem_limit
> +Date:August 2014
> +Contact: Minchan Kim
> +Description:
> + The mem_limit file is read/write and specifies the amount
> + of memory to be able to consume memory to store store
> + compressed data. The limit could be changed in run time
> + and "0" means disable the limit. No limit is the initial state.
just a nitpick, sorry.
"the amount of memory to be able to consume memory to store store compressed
data"
^^^
"the maximum amount of memory ZRAM can use to store the compressed data"?
-ss
> + Unit: bytes
> diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
> index 0595c3f56ccf..82c6a41116db 100644
> --- a/Documentation/blockdev/zram.txt
> +++ b/Documentation/blockdev/zram.txt
> @@ -74,14 +74,30 @@ There is little point creating a zram of greater than
> twice the size of memory
> since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of
> the
> size of the disk when not in use so a huge zram is wasteful.
>
> -5) Activate:
> +5) Set memory limit: Optional
> + Set memory limit by writing the value to sysfs node 'mem_limit'.
> + The value can be either in bytes or you can use mem suffixes.
> + In addition, you could change the value in runtime.
> + Examples:
> + # limit /dev/zram0 with 50MB memory
> + echo $((50*1024*1024)) > /sys/block/zram0/mem_limit
> +
> + # Using mem suffixes
> + echo 256K > /sys/block/zram0/mem_limit
> + echo 512M > /sys/block/zram0/mem_limit
> + echo 1G > /sys/block/zram0/mem_limit
> +
> + # To disable memory limit
> + echo 0 > /sys/block/zram0/mem_limit
> +
> +6) Activate:
> mkswap /dev/zram0
> swapon /dev/zram0
>
> mkfs.ext4 /dev/zram1
> mount /dev/zram1 /tmp
>
> -6) Stats:
> +7) Stats:
> Per-device statistics are exported as various nodes under
> /sys/block/zram/
> disksize
> @@ -96,11 +112,11 @@ size of the disk when not in use so a huge zram is
> wasteful.
> compr_data_size
> mem_used_total
>
> -7) Deactivate:
> +8) Deactivate:
> swapoff /dev/zram0
> umount /dev/zram1
>
> -8) Reset:
> +9) Reset:
> Write any positive value to 'reset' sysfs node
> echo 1 > /sys/block/zram0/reset
> echo 1 > /sys/block/zram1/reset
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index f0b8b30a7128..370c355eb127 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -122,6 +122,33 @@ static ssize_t max_comp_streams_show(struct device *dev,
> return scnprintf(buf, PAGE_SIZE, "%d\n", val);
> }
>
> +static ssize_t mem_limit_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + u64 val;
> + struct zram *zram = dev_to_zram(dev);
> +
> + down_read(>init_lock);
> + val = zram->limit_pages;
> + up_read(>init_lock);
> +
> + return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
> +}
> +
> +static ssize_t mem_limit_store(struct device *dev,
> + struct device_attribute *attr, const char *buf, size_t len)
> +{
> + u64 limit;
> + struct zram *zram = dev_to_zram(dev);
> +
> + limit = memparse(buf, NULL);
> + down_write(>init_lock);
> + zram->limit_pages = PAGE_ALIGN(limit) >> PAGE_SHIFT;
> + up_write(>init_lock);
> +
> + return len;
> +}
> +
> static ssize_t max_comp_streams_store(struct device *dev,
> struct device_attribute *attr, const char *buf, size_t
Re: [PATCH v5 3/4] zram: zram memory size limitation
On (08/25/14 09:05), Minchan Kim wrote:
Since zram has no control feature to limit memory usage,
it makes hard to manage system memrory.
This patch adds new knob mem_limit via sysfs to set up the
a limit so that zram could fail allocation once it reaches
the limit.
In addition, user could change the limit in runtime so that
he could manage the memory more dynamically.
Initial state is no limit so it doesn't break old behavior.
Signed-off-by: Minchan Kim minc...@kernel.org
---
Documentation/ABI/testing/sysfs-block-zram | 10
Documentation/blockdev/zram.txt| 24 ++---
drivers/block/zram/zram_drv.c | 41
++
drivers/block/zram/zram_drv.h | 5
4 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-block-zram
b/Documentation/ABI/testing/sysfs-block-zram
index 70ec992514d0..dbe643775ec1 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -119,3 +119,13 @@ Description:
efficiency can be calculated using compr_data_size and this
statistic.
Unit: bytes
+
+What:/sys/block/zramid/mem_limit
+Date:August 2014
+Contact: Minchan Kim minc...@kernel.org
+Description:
+ The mem_limit file is read/write and specifies the amount
+ of memory to be able to consume memory to store store
+ compressed data. The limit could be changed in run time
+ and 0 means disable the limit. No limit is the initial state.
just a nitpick, sorry.
the amount of memory to be able to consume memory to store store compressed
data
^^^
the maximum amount of memory ZRAM can use to store the compressed data?
-ss
+ Unit: bytes
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 0595c3f56ccf..82c6a41116db 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -74,14 +74,30 @@ There is little point creating a zram of greater than
twice the size of memory
since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of
the
size of the disk when not in use so a huge zram is wasteful.
-5) Activate:
+5) Set memory limit: Optional
+ Set memory limit by writing the value to sysfs node 'mem_limit'.
+ The value can be either in bytes or you can use mem suffixes.
+ In addition, you could change the value in runtime.
+ Examples:
+ # limit /dev/zram0 with 50MB memory
+ echo $((50*1024*1024)) /sys/block/zram0/mem_limit
+
+ # Using mem suffixes
+ echo 256K /sys/block/zram0/mem_limit
+ echo 512M /sys/block/zram0/mem_limit
+ echo 1G /sys/block/zram0/mem_limit
+
+ # To disable memory limit
+ echo 0 /sys/block/zram0/mem_limit
+
+6) Activate:
mkswap /dev/zram0
swapon /dev/zram0
mkfs.ext4 /dev/zram1
mount /dev/zram1 /tmp
-6) Stats:
+7) Stats:
Per-device statistics are exported as various nodes under
/sys/block/zramid/
disksize
@@ -96,11 +112,11 @@ size of the disk when not in use so a huge zram is
wasteful.
compr_data_size
mem_used_total
-7) Deactivate:
+8) Deactivate:
swapoff /dev/zram0
umount /dev/zram1
-8) Reset:
+9) Reset:
Write any positive value to 'reset' sysfs node
echo 1 /sys/block/zram0/reset
echo 1 /sys/block/zram1/reset
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f0b8b30a7128..370c355eb127 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -122,6 +122,33 @@ static ssize_t max_comp_streams_show(struct device *dev,
return scnprintf(buf, PAGE_SIZE, %d\n, val);
}
+static ssize_t mem_limit_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ u64 val;
+ struct zram *zram = dev_to_zram(dev);
+
+ down_read(zram-init_lock);
+ val = zram-limit_pages;
+ up_read(zram-init_lock);
+
+ return scnprintf(buf, PAGE_SIZE, %llu\n, val PAGE_SHIFT);
+}
+
+static ssize_t mem_limit_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ u64 limit;
+ struct zram *zram = dev_to_zram(dev);
+
+ limit = memparse(buf, NULL);
+ down_write(zram-init_lock);
+ zram-limit_pages = PAGE_ALIGN(limit) PAGE_SHIFT;
+ up_write(zram-init_lock);
+
+ return len;
+}
+
static ssize_t max_comp_streams_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
bio_vec *bvec,
Re: [PATCH v5 3/4] zram: zram memory size limitation
Hey Sergey, On Mon, Aug 25, 2014 at 08:09:27PM +0900, Sergey Senozhatsky wrote: On (08/25/14 09:05), Minchan Kim wrote: Since zram has no control feature to limit memory usage, it makes hard to manage system memrory. This patch adds new knob mem_limit via sysfs to set up the a limit so that zram could fail allocation once it reaches the limit. In addition, user could change the limit in runtime so that he could manage the memory more dynamically. Initial state is no limit so it doesn't break old behavior. Signed-off-by: Minchan Kim minc...@kernel.org --- Documentation/ABI/testing/sysfs-block-zram | 10 Documentation/blockdev/zram.txt| 24 ++--- drivers/block/zram/zram_drv.c | 41 ++ drivers/block/zram/zram_drv.h | 5 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-block-zram b/Documentation/ABI/testing/sysfs-block-zram index 70ec992514d0..dbe643775ec1 100644 --- a/Documentation/ABI/testing/sysfs-block-zram +++ b/Documentation/ABI/testing/sysfs-block-zram @@ -119,3 +119,13 @@ Description: efficiency can be calculated using compr_data_size and this statistic. Unit: bytes + +What: /sys/block/zramid/mem_limit +Date: August 2014 +Contact: Minchan Kim minc...@kernel.org +Description: + The mem_limit file is read/write and specifies the amount + of memory to be able to consume memory to store store + compressed data. The limit could be changed in run time + and 0 means disable the limit. No limit is the initial state. just a nitpick, sorry. the amount of memory to be able to consume memory to store store compressed data ^^^ the maximum amount of memory ZRAM can use to store the compressed data? Will fix. Thanks. -- Kind regards, Minchan Kim -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v5 3/4] zram: zram memory size limitation
Since zram has no control feature to limit memory usage,
it makes hard to manage system memrory.
This patch adds new knob "mem_limit" via sysfs to set up the
a limit so that zram could fail allocation once it reaches
the limit.
In addition, user could change the limit in runtime so that
he could manage the memory more dynamically.
Initial state is no limit so it doesn't break old behavior.
Signed-off-by: Minchan Kim
---
Documentation/ABI/testing/sysfs-block-zram | 10
Documentation/blockdev/zram.txt| 24 ++---
drivers/block/zram/zram_drv.c | 41 ++
drivers/block/zram/zram_drv.h | 5
4 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-block-zram
b/Documentation/ABI/testing/sysfs-block-zram
index 70ec992514d0..dbe643775ec1 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -119,3 +119,13 @@ Description:
efficiency can be calculated using compr_data_size and this
statistic.
Unit: bytes
+
+What: /sys/block/zram/mem_limit
+Date: August 2014
+Contact: Minchan Kim
+Description:
+ The mem_limit file is read/write and specifies the amount
+ of memory to be able to consume memory to store store
+ compressed data. The limit could be changed in run time
+ and "0" means disable the limit. No limit is the initial state.
+ Unit: bytes
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 0595c3f56ccf..82c6a41116db 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -74,14 +74,30 @@ There is little point creating a zram of greater than twice
the size of memory
since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of the
size of the disk when not in use so a huge zram is wasteful.
-5) Activate:
+5) Set memory limit: Optional
+ Set memory limit by writing the value to sysfs node 'mem_limit'.
+ The value can be either in bytes or you can use mem suffixes.
+ In addition, you could change the value in runtime.
+ Examples:
+ # limit /dev/zram0 with 50MB memory
+ echo $((50*1024*1024)) > /sys/block/zram0/mem_limit
+
+ # Using mem suffixes
+ echo 256K > /sys/block/zram0/mem_limit
+ echo 512M > /sys/block/zram0/mem_limit
+ echo 1G > /sys/block/zram0/mem_limit
+
+ # To disable memory limit
+ echo 0 > /sys/block/zram0/mem_limit
+
+6) Activate:
mkswap /dev/zram0
swapon /dev/zram0
mkfs.ext4 /dev/zram1
mount /dev/zram1 /tmp
-6) Stats:
+7) Stats:
Per-device statistics are exported as various nodes under
/sys/block/zram/
disksize
@@ -96,11 +112,11 @@ size of the disk when not in use so a huge zram is
wasteful.
compr_data_size
mem_used_total
-7) Deactivate:
+8) Deactivate:
swapoff /dev/zram0
umount /dev/zram1
-8) Reset:
+9) Reset:
Write any positive value to 'reset' sysfs node
echo 1 > /sys/block/zram0/reset
echo 1 > /sys/block/zram1/reset
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f0b8b30a7128..370c355eb127 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -122,6 +122,33 @@ static ssize_t max_comp_streams_show(struct device *dev,
return scnprintf(buf, PAGE_SIZE, "%d\n", val);
}
+static ssize_t mem_limit_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ u64 val;
+ struct zram *zram = dev_to_zram(dev);
+
+ down_read(>init_lock);
+ val = zram->limit_pages;
+ up_read(>init_lock);
+
+ return scnprintf(buf, PAGE_SIZE, "%llu\n", val << PAGE_SHIFT);
+}
+
+static ssize_t mem_limit_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ u64 limit;
+ struct zram *zram = dev_to_zram(dev);
+
+ limit = memparse(buf, NULL);
+ down_write(>init_lock);
+ zram->limit_pages = PAGE_ALIGN(limit) >> PAGE_SHIFT;
+ up_write(>init_lock);
+
+ return len;
+}
+
static ssize_t max_comp_streams_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram->limit_pages &&
+ zs_get_total_pages(meta->mem_pool) > zram->limit_pages) {
+ zs_free(meta->mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta->mem_pool, handle, ZS_MM_WO);
[PATCH v5 3/4] zram: zram memory size limitation
Since zram has no control feature to limit memory usage,
it makes hard to manage system memrory.
This patch adds new knob mem_limit via sysfs to set up the
a limit so that zram could fail allocation once it reaches
the limit.
In addition, user could change the limit in runtime so that
he could manage the memory more dynamically.
Initial state is no limit so it doesn't break old behavior.
Signed-off-by: Minchan Kim minc...@kernel.org
---
Documentation/ABI/testing/sysfs-block-zram | 10
Documentation/blockdev/zram.txt| 24 ++---
drivers/block/zram/zram_drv.c | 41 ++
drivers/block/zram/zram_drv.h | 5
4 files changed, 76 insertions(+), 4 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-block-zram
b/Documentation/ABI/testing/sysfs-block-zram
index 70ec992514d0..dbe643775ec1 100644
--- a/Documentation/ABI/testing/sysfs-block-zram
+++ b/Documentation/ABI/testing/sysfs-block-zram
@@ -119,3 +119,13 @@ Description:
efficiency can be calculated using compr_data_size and this
statistic.
Unit: bytes
+
+What: /sys/block/zramid/mem_limit
+Date: August 2014
+Contact: Minchan Kim minc...@kernel.org
+Description:
+ The mem_limit file is read/write and specifies the amount
+ of memory to be able to consume memory to store store
+ compressed data. The limit could be changed in run time
+ and 0 means disable the limit. No limit is the initial state.
+ Unit: bytes
diff --git a/Documentation/blockdev/zram.txt b/Documentation/blockdev/zram.txt
index 0595c3f56ccf..82c6a41116db 100644
--- a/Documentation/blockdev/zram.txt
+++ b/Documentation/blockdev/zram.txt
@@ -74,14 +74,30 @@ There is little point creating a zram of greater than twice
the size of memory
since we expect a 2:1 compression ratio. Note that zram uses about 0.1% of the
size of the disk when not in use so a huge zram is wasteful.
-5) Activate:
+5) Set memory limit: Optional
+ Set memory limit by writing the value to sysfs node 'mem_limit'.
+ The value can be either in bytes or you can use mem suffixes.
+ In addition, you could change the value in runtime.
+ Examples:
+ # limit /dev/zram0 with 50MB memory
+ echo $((50*1024*1024)) /sys/block/zram0/mem_limit
+
+ # Using mem suffixes
+ echo 256K /sys/block/zram0/mem_limit
+ echo 512M /sys/block/zram0/mem_limit
+ echo 1G /sys/block/zram0/mem_limit
+
+ # To disable memory limit
+ echo 0 /sys/block/zram0/mem_limit
+
+6) Activate:
mkswap /dev/zram0
swapon /dev/zram0
mkfs.ext4 /dev/zram1
mount /dev/zram1 /tmp
-6) Stats:
+7) Stats:
Per-device statistics are exported as various nodes under
/sys/block/zramid/
disksize
@@ -96,11 +112,11 @@ size of the disk when not in use so a huge zram is
wasteful.
compr_data_size
mem_used_total
-7) Deactivate:
+8) Deactivate:
swapoff /dev/zram0
umount /dev/zram1
-8) Reset:
+9) Reset:
Write any positive value to 'reset' sysfs node
echo 1 /sys/block/zram0/reset
echo 1 /sys/block/zram1/reset
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index f0b8b30a7128..370c355eb127 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -122,6 +122,33 @@ static ssize_t max_comp_streams_show(struct device *dev,
return scnprintf(buf, PAGE_SIZE, %d\n, val);
}
+static ssize_t mem_limit_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ u64 val;
+ struct zram *zram = dev_to_zram(dev);
+
+ down_read(zram-init_lock);
+ val = zram-limit_pages;
+ up_read(zram-init_lock);
+
+ return scnprintf(buf, PAGE_SIZE, %llu\n, val PAGE_SHIFT);
+}
+
+static ssize_t mem_limit_store(struct device *dev,
+ struct device_attribute *attr, const char *buf, size_t len)
+{
+ u64 limit;
+ struct zram *zram = dev_to_zram(dev);
+
+ limit = memparse(buf, NULL);
+ down_write(zram-init_lock);
+ zram-limit_pages = PAGE_ALIGN(limit) PAGE_SHIFT;
+ up_write(zram-init_lock);
+
+ return len;
+}
+
static ssize_t max_comp_streams_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
@@ -513,6 +540,14 @@ static int zram_bvec_write(struct zram *zram, struct
bio_vec *bvec, u32 index,
ret = -ENOMEM;
goto out;
}
+
+ if (zram-limit_pages
+ zs_get_total_pages(meta-mem_pool) zram-limit_pages) {
+ zs_free(meta-mem_pool, handle);
+ ret = -ENOMEM;
+ goto out;
+ }
+
cmem = zs_map_object(meta-mem_pool,
