Re: [PATCH] zram: move comp allocation out of init_lock
Hello Minchan,
On (03/06/14 17:26), Minchan Kim wrote:
> Hello Sergey,
>
> On Tue, Mar 04, 2014 at 01:10:56PM +0300, Sergey Senozhatsky wrote:
> > While fixing lockdep spew of ->init_lock reported by Sasha Levin [1],
> > Minchan
> > Kim noted [2] that it's better to move compression backend allocation (using
> > GPF_KERNEL) out of the ->init_lock lock, same way as with zram_meta_alloc(),
> > in order to prevent the same lockdep spew.
> >
> > [1] https://lkml.org/lkml/2014/2/27/337
> > [2] https://lkml.org/lkml/2014/3/3/32
> >
> > Cc: Sasha Levin
> > Reported-by: Minchan Kim
> > Signed-off-by: Sergey Senozhatsky
>
> Thanks for looking.
>
thanks for noting.
> > ---
> > drivers/block/zram/zram_drv.c | 27 +++
> > 1 file changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index 15d46f2..e4d536b 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
> > struct device_attribute *attr, const char *buf, size_t len)
> > {
> > u64 disksize;
> > + struct zcomp *comp;
> > struct zram_meta *meta;
> > struct zram *zram = dev_to_zram(dev);
> > - int err;
> > + int err = -EINVAL;
> >
> > disksize = memparse(buf, NULL);
> > if (!disksize)
> > @@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
> > if (!meta)
> > return -ENOMEM;
> >
> > + comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> > + if (!comp) {
> > + pr_info("Cannot initialise %s compressing backend\n",
> > + zram->compressor);
>
> -ENOMEM?
>
yes. patch 'zram: return error-valued pointer from zcomp_create()'
changed zcomp_create() and it returns ERR_PTR() (-ENOMEM or -EINVAL)
now.
-ss
> Otherwise,
> Acked-by: Minchan Kim
>
>
> > + goto out_cleanup;
> > + }
> > +
> > down_write(>init_lock);
> > if (init_done(zram)) {
> > + up_write(>init_lock);
> > pr_info("Cannot change disksize for initialized device\n");
> > err = -EBUSY;
> > - goto out_free_meta;
> > - }
> > -
> > - zram->comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> > - if (!zram->comp) {
> > - pr_info("Cannot initialise %s compressing backend\n",
> > - zram->compressor);
> > - err = -EINVAL;
> > - goto out_free_meta;
> > + goto out_cleanup;
> > }
> >
> > zram->meta = meta;
> > + zram->comp = comp;
> > zram->disksize = disksize;
> > set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
> > up_write(>init_lock);
> >
> > return len;
> >
> > -out_free_meta:
> > - up_write(>init_lock);
> > +out_cleanup:
> > + if (comp)
> > + zcomp_destroy(comp);
> > zram_meta_free(meta);
> > return err;
> > }
> > --
> > 1.9.0.382.g7f3562c
> >
> > --
> > 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/
>
> --
> 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] zram: move comp allocation out of init_lock
Hello Sergey,
On Tue, Mar 04, 2014 at 01:10:56PM +0300, Sergey Senozhatsky wrote:
> While fixing lockdep spew of ->init_lock reported by Sasha Levin [1], Minchan
> Kim noted [2] that it's better to move compression backend allocation (using
> GPF_KERNEL) out of the ->init_lock lock, same way as with zram_meta_alloc(),
> in order to prevent the same lockdep spew.
>
> [1] https://lkml.org/lkml/2014/2/27/337
> [2] https://lkml.org/lkml/2014/3/3/32
>
> Cc: Sasha Levin
> Reported-by: Minchan Kim
> Signed-off-by: Sergey Senozhatsky
Thanks for looking.
> ---
> drivers/block/zram/zram_drv.c | 27 +++
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 15d46f2..e4d536b 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
> struct device_attribute *attr, const char *buf, size_t len)
> {
> u64 disksize;
> + struct zcomp *comp;
> struct zram_meta *meta;
> struct zram *zram = dev_to_zram(dev);
> - int err;
> + int err = -EINVAL;
>
> disksize = memparse(buf, NULL);
> if (!disksize)
> @@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
> if (!meta)
> return -ENOMEM;
>
> + comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> + if (!comp) {
> + pr_info("Cannot initialise %s compressing backend\n",
> + zram->compressor);
-ENOMEM?
Otherwise,
Acked-by: Minchan Kim
> + goto out_cleanup;
> + }
> +
> down_write(>init_lock);
> if (init_done(zram)) {
> + up_write(>init_lock);
> pr_info("Cannot change disksize for initialized device\n");
> err = -EBUSY;
> - goto out_free_meta;
> - }
> -
> - zram->comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> - if (!zram->comp) {
> - pr_info("Cannot initialise %s compressing backend\n",
> - zram->compressor);
> - err = -EINVAL;
> - goto out_free_meta;
> + goto out_cleanup;
> }
>
> zram->meta = meta;
> + zram->comp = comp;
> zram->disksize = disksize;
> set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
> up_write(>init_lock);
>
> return len;
>
> -out_free_meta:
> - up_write(>init_lock);
> +out_cleanup:
> + if (comp)
> + zcomp_destroy(comp);
> zram_meta_free(meta);
> return err;
> }
> --
> 1.9.0.382.g7f3562c
>
> --
> 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/
--
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] zram: move comp allocation out of init_lock
Hello Sergey,
On Tue, Mar 04, 2014 at 01:10:56PM +0300, Sergey Senozhatsky wrote:
While fixing lockdep spew of -init_lock reported by Sasha Levin [1], Minchan
Kim noted [2] that it's better to move compression backend allocation (using
GPF_KERNEL) out of the -init_lock lock, same way as with zram_meta_alloc(),
in order to prevent the same lockdep spew.
[1] https://lkml.org/lkml/2014/2/27/337
[2] https://lkml.org/lkml/2014/3/3/32
Cc: Sasha Levin sasha.le...@oracle.com
Reported-by: Minchan Kim minc...@kernel.org
Signed-off-by: Sergey Senozhatsky sergey.senozhat...@gmail.com
Thanks for looking.
---
drivers/block/zram/zram_drv.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15d46f2..e4d536b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
u64 disksize;
+ struct zcomp *comp;
struct zram_meta *meta;
struct zram *zram = dev_to_zram(dev);
- int err;
+ int err = -EINVAL;
disksize = memparse(buf, NULL);
if (!disksize)
@@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
if (!meta)
return -ENOMEM;
+ comp = zcomp_create(zram-compressor, zram-max_comp_streams);
+ if (!comp) {
+ pr_info(Cannot initialise %s compressing backend\n,
+ zram-compressor);
-ENOMEM?
Otherwise,
Acked-by: Minchan Kim minc...@kernel.org
+ goto out_cleanup;
+ }
+
down_write(zram-init_lock);
if (init_done(zram)) {
+ up_write(zram-init_lock);
pr_info(Cannot change disksize for initialized device\n);
err = -EBUSY;
- goto out_free_meta;
- }
-
- zram-comp = zcomp_create(zram-compressor, zram-max_comp_streams);
- if (!zram-comp) {
- pr_info(Cannot initialise %s compressing backend\n,
- zram-compressor);
- err = -EINVAL;
- goto out_free_meta;
+ goto out_cleanup;
}
zram-meta = meta;
+ zram-comp = comp;
zram-disksize = disksize;
set_capacity(zram-disk, zram-disksize SECTOR_SHIFT);
up_write(zram-init_lock);
return len;
-out_free_meta:
- up_write(zram-init_lock);
+out_cleanup:
+ if (comp)
+ zcomp_destroy(comp);
zram_meta_free(meta);
return err;
}
--
1.9.0.382.g7f3562c
--
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/
--
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] zram: move comp allocation out of init_lock
Hello Minchan,
On (03/06/14 17:26), Minchan Kim wrote:
Hello Sergey,
On Tue, Mar 04, 2014 at 01:10:56PM +0300, Sergey Senozhatsky wrote:
While fixing lockdep spew of -init_lock reported by Sasha Levin [1],
Minchan
Kim noted [2] that it's better to move compression backend allocation (using
GPF_KERNEL) out of the -init_lock lock, same way as with zram_meta_alloc(),
in order to prevent the same lockdep spew.
[1] https://lkml.org/lkml/2014/2/27/337
[2] https://lkml.org/lkml/2014/3/3/32
Cc: Sasha Levin sasha.le...@oracle.com
Reported-by: Minchan Kim minc...@kernel.org
Signed-off-by: Sergey Senozhatsky sergey.senozhat...@gmail.com
Thanks for looking.
thanks for noting.
---
drivers/block/zram/zram_drv.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15d46f2..e4d536b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
u64 disksize;
+ struct zcomp *comp;
struct zram_meta *meta;
struct zram *zram = dev_to_zram(dev);
- int err;
+ int err = -EINVAL;
disksize = memparse(buf, NULL);
if (!disksize)
@@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
if (!meta)
return -ENOMEM;
+ comp = zcomp_create(zram-compressor, zram-max_comp_streams);
+ if (!comp) {
+ pr_info(Cannot initialise %s compressing backend\n,
+ zram-compressor);
-ENOMEM?
yes. patch 'zram: return error-valued pointer from zcomp_create()'
changed zcomp_create() and it returns ERR_PTR() (-ENOMEM or -EINVAL)
now.
-ss
Otherwise,
Acked-by: Minchan Kim minc...@kernel.org
+ goto out_cleanup;
+ }
+
down_write(zram-init_lock);
if (init_done(zram)) {
+ up_write(zram-init_lock);
pr_info(Cannot change disksize for initialized device\n);
err = -EBUSY;
- goto out_free_meta;
- }
-
- zram-comp = zcomp_create(zram-compressor, zram-max_comp_streams);
- if (!zram-comp) {
- pr_info(Cannot initialise %s compressing backend\n,
- zram-compressor);
- err = -EINVAL;
- goto out_free_meta;
+ goto out_cleanup;
}
zram-meta = meta;
+ zram-comp = comp;
zram-disksize = disksize;
set_capacity(zram-disk, zram-disksize SECTOR_SHIFT);
up_write(zram-init_lock);
return len;
-out_free_meta:
- up_write(zram-init_lock);
+out_cleanup:
+ if (comp)
+ zcomp_destroy(comp);
zram_meta_free(meta);
return err;
}
--
1.9.0.382.g7f3562c
--
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/
--
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] zram: move comp allocation out of init_lock
On (03/05/14 15:57), Jerome Marchand wrote:
> On 03/04/2014 11:10 AM, Sergey Senozhatsky wrote:
> > While fixing lockdep spew of ->init_lock reported by Sasha Levin [1],
> > Minchan
> > Kim noted [2] that it's better to move compression backend allocation (using
> > GPF_KERNEL) out of the ->init_lock lock, same way as with zram_meta_alloc(),
> > in order to prevent the same lockdep spew.
> >
> > [1] https://lkml.org/lkml/2014/2/27/337
> > [2] https://lkml.org/lkml/2014/3/3/32
> >
> > Cc: Sasha Levin
> > Reported-by: Minchan Kim
> > Signed-off-by: Sergey Senozhatsky
> > ---
> > drivers/block/zram/zram_drv.c | 27 +++
> > 1 file changed, 15 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> > index 15d46f2..e4d536b 100644
> > --- a/drivers/block/zram/zram_drv.c
> > +++ b/drivers/block/zram/zram_drv.c
> > @@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
> > struct device_attribute *attr, const char *buf, size_t len)
> > {
> > u64 disksize;
> > + struct zcomp *comp;
> > struct zram_meta *meta;
> > struct zram *zram = dev_to_zram(dev);
> > - int err;
> > + int err = -EINVAL;
> >
> > disksize = memparse(buf, NULL);
> > if (!disksize)
> > @@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
> > if (!meta)
> > return -ENOMEM;
> >
> > + comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> > + if (!comp) {
> > + pr_info("Cannot initialise %s compressing backend\n",
> > + zram->compressor);
> > + goto out_cleanup;
> > + }
> > +
>
> zcomp_create() could fail because of a failed allocation, in which case
> ENOMEM would be more appropriate. I guess zcomp_create() should return
> an errno in case of failure.
> But that's an other problem than the one addressed in this patch:
>
There are two possible cases of failed zcomp_create(). One is,
you're right, -ENOMEM. The second one is request of unsupported
compression algorithm - -EINVAL. agree, it makes sense to return
ERR_PTR() from zcomp_create(): -EINVAL or -ENOMEM.
-ss
> Acked-by: Jerome Marchand
>
> > down_write(>init_lock);
> > if (init_done(zram)) {
> > + up_write(>init_lock);
> > pr_info("Cannot change disksize for initialized device\n");
> > err = -EBUSY;
> > - goto out_free_meta;
> > - }
> > -
> > - zram->comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> > - if (!zram->comp) {
> > - pr_info("Cannot initialise %s compressing backend\n",
> > - zram->compressor);
> > - err = -EINVAL;
> > - goto out_free_meta;
> > + goto out_cleanup;
> > }
> >
> > zram->meta = meta;
> > + zram->comp = comp;
> > zram->disksize = disksize;
> > set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
> > up_write(>init_lock);
> >
> > return len;
> >
> > -out_free_meta:
> > - up_write(>init_lock);
> > +out_cleanup:
> > + if (comp)
> > + zcomp_destroy(comp);
> > zram_meta_free(meta);
> > return err;
> > }
> >
>
--
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] zram: move comp allocation out of init_lock
On 03/04/2014 11:10 AM, Sergey Senozhatsky wrote:
> While fixing lockdep spew of ->init_lock reported by Sasha Levin [1], Minchan
> Kim noted [2] that it's better to move compression backend allocation (using
> GPF_KERNEL) out of the ->init_lock lock, same way as with zram_meta_alloc(),
> in order to prevent the same lockdep spew.
>
> [1] https://lkml.org/lkml/2014/2/27/337
> [2] https://lkml.org/lkml/2014/3/3/32
>
> Cc: Sasha Levin
> Reported-by: Minchan Kim
> Signed-off-by: Sergey Senozhatsky
> ---
> drivers/block/zram/zram_drv.c | 27 +++
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 15d46f2..e4d536b 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
> struct device_attribute *attr, const char *buf, size_t len)
> {
> u64 disksize;
> + struct zcomp *comp;
> struct zram_meta *meta;
> struct zram *zram = dev_to_zram(dev);
> - int err;
> + int err = -EINVAL;
>
> disksize = memparse(buf, NULL);
> if (!disksize)
> @@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
> if (!meta)
> return -ENOMEM;
>
> + comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> + if (!comp) {
> + pr_info("Cannot initialise %s compressing backend\n",
> + zram->compressor);
> + goto out_cleanup;
> + }
> +
zcomp_create() could fail because of a failed allocation, in which case
ENOMEM would be more appropriate. I guess zcomp_create() should return
an errno in case of failure.
But that's an other problem than the one addressed in this patch:
Acked-by: Jerome Marchand
> down_write(>init_lock);
> if (init_done(zram)) {
> + up_write(>init_lock);
> pr_info("Cannot change disksize for initialized device\n");
> err = -EBUSY;
> - goto out_free_meta;
> - }
> -
> - zram->comp = zcomp_create(zram->compressor, zram->max_comp_streams);
> - if (!zram->comp) {
> - pr_info("Cannot initialise %s compressing backend\n",
> - zram->compressor);
> - err = -EINVAL;
> - goto out_free_meta;
> + goto out_cleanup;
> }
>
> zram->meta = meta;
> + zram->comp = comp;
> zram->disksize = disksize;
> set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
> up_write(>init_lock);
>
> return len;
>
> -out_free_meta:
> - up_write(>init_lock);
> +out_cleanup:
> + if (comp)
> + zcomp_destroy(comp);
> zram_meta_free(meta);
> return err;
> }
>
--
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] zram: move comp allocation out of init_lock
On 03/04/2014 11:10 AM, Sergey Senozhatsky wrote:
While fixing lockdep spew of -init_lock reported by Sasha Levin [1], Minchan
Kim noted [2] that it's better to move compression backend allocation (using
GPF_KERNEL) out of the -init_lock lock, same way as with zram_meta_alloc(),
in order to prevent the same lockdep spew.
[1] https://lkml.org/lkml/2014/2/27/337
[2] https://lkml.org/lkml/2014/3/3/32
Cc: Sasha Levin sasha.le...@oracle.com
Reported-by: Minchan Kim minc...@kernel.org
Signed-off-by: Sergey Senozhatsky sergey.senozhat...@gmail.com
---
drivers/block/zram/zram_drv.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15d46f2..e4d536b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
u64 disksize;
+ struct zcomp *comp;
struct zram_meta *meta;
struct zram *zram = dev_to_zram(dev);
- int err;
+ int err = -EINVAL;
disksize = memparse(buf, NULL);
if (!disksize)
@@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
if (!meta)
return -ENOMEM;
+ comp = zcomp_create(zram-compressor, zram-max_comp_streams);
+ if (!comp) {
+ pr_info(Cannot initialise %s compressing backend\n,
+ zram-compressor);
+ goto out_cleanup;
+ }
+
zcomp_create() could fail because of a failed allocation, in which case
ENOMEM would be more appropriate. I guess zcomp_create() should return
an errno in case of failure.
But that's an other problem than the one addressed in this patch:
Acked-by: Jerome Marchand jmarc...@redhat.com
down_write(zram-init_lock);
if (init_done(zram)) {
+ up_write(zram-init_lock);
pr_info(Cannot change disksize for initialized device\n);
err = -EBUSY;
- goto out_free_meta;
- }
-
- zram-comp = zcomp_create(zram-compressor, zram-max_comp_streams);
- if (!zram-comp) {
- pr_info(Cannot initialise %s compressing backend\n,
- zram-compressor);
- err = -EINVAL;
- goto out_free_meta;
+ goto out_cleanup;
}
zram-meta = meta;
+ zram-comp = comp;
zram-disksize = disksize;
set_capacity(zram-disk, zram-disksize SECTOR_SHIFT);
up_write(zram-init_lock);
return len;
-out_free_meta:
- up_write(zram-init_lock);
+out_cleanup:
+ if (comp)
+ zcomp_destroy(comp);
zram_meta_free(meta);
return err;
}
--
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] zram: move comp allocation out of init_lock
On (03/05/14 15:57), Jerome Marchand wrote:
On 03/04/2014 11:10 AM, Sergey Senozhatsky wrote:
While fixing lockdep spew of -init_lock reported by Sasha Levin [1],
Minchan
Kim noted [2] that it's better to move compression backend allocation (using
GPF_KERNEL) out of the -init_lock lock, same way as with zram_meta_alloc(),
in order to prevent the same lockdep spew.
[1] https://lkml.org/lkml/2014/2/27/337
[2] https://lkml.org/lkml/2014/3/3/32
Cc: Sasha Levin sasha.le...@oracle.com
Reported-by: Minchan Kim minc...@kernel.org
Signed-off-by: Sergey Senozhatsky sergey.senozhat...@gmail.com
---
drivers/block/zram/zram_drv.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15d46f2..e4d536b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
u64 disksize;
+ struct zcomp *comp;
struct zram_meta *meta;
struct zram *zram = dev_to_zram(dev);
- int err;
+ int err = -EINVAL;
disksize = memparse(buf, NULL);
if (!disksize)
@@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
if (!meta)
return -ENOMEM;
+ comp = zcomp_create(zram-compressor, zram-max_comp_streams);
+ if (!comp) {
+ pr_info(Cannot initialise %s compressing backend\n,
+ zram-compressor);
+ goto out_cleanup;
+ }
+
zcomp_create() could fail because of a failed allocation, in which case
ENOMEM would be more appropriate. I guess zcomp_create() should return
an errno in case of failure.
But that's an other problem than the one addressed in this patch:
There are two possible cases of failed zcomp_create(). One is,
you're right, -ENOMEM. The second one is request of unsupported
compression algorithm - -EINVAL. agree, it makes sense to return
ERR_PTR() from zcomp_create(): -EINVAL or -ENOMEM.
-ss
Acked-by: Jerome Marchand jmarc...@redhat.com
down_write(zram-init_lock);
if (init_done(zram)) {
+ up_write(zram-init_lock);
pr_info(Cannot change disksize for initialized device\n);
err = -EBUSY;
- goto out_free_meta;
- }
-
- zram-comp = zcomp_create(zram-compressor, zram-max_comp_streams);
- if (!zram-comp) {
- pr_info(Cannot initialise %s compressing backend\n,
- zram-compressor);
- err = -EINVAL;
- goto out_free_meta;
+ goto out_cleanup;
}
zram-meta = meta;
+ zram-comp = comp;
zram-disksize = disksize;
set_capacity(zram-disk, zram-disksize SECTOR_SHIFT);
up_write(zram-init_lock);
return len;
-out_free_meta:
- up_write(zram-init_lock);
+out_cleanup:
+ if (comp)
+ zcomp_destroy(comp);
zram_meta_free(meta);
return err;
}
--
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] zram: move comp allocation out of init_lock
While fixing lockdep spew of ->init_lock reported by Sasha Levin [1], Minchan
Kim noted [2] that it's better to move compression backend allocation (using
GPF_KERNEL) out of the ->init_lock lock, same way as with zram_meta_alloc(),
in order to prevent the same lockdep spew.
[1] https://lkml.org/lkml/2014/2/27/337
[2] https://lkml.org/lkml/2014/3/3/32
Cc: Sasha Levin
Reported-by: Minchan Kim
Signed-off-by: Sergey Senozhatsky
---
drivers/block/zram/zram_drv.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15d46f2..e4d536b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
u64 disksize;
+ struct zcomp *comp;
struct zram_meta *meta;
struct zram *zram = dev_to_zram(dev);
- int err;
+ int err = -EINVAL;
disksize = memparse(buf, NULL);
if (!disksize)
@@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
if (!meta)
return -ENOMEM;
+ comp = zcomp_create(zram->compressor, zram->max_comp_streams);
+ if (!comp) {
+ pr_info("Cannot initialise %s compressing backend\n",
+ zram->compressor);
+ goto out_cleanup;
+ }
+
down_write(>init_lock);
if (init_done(zram)) {
+ up_write(>init_lock);
pr_info("Cannot change disksize for initialized device\n");
err = -EBUSY;
- goto out_free_meta;
- }
-
- zram->comp = zcomp_create(zram->compressor, zram->max_comp_streams);
- if (!zram->comp) {
- pr_info("Cannot initialise %s compressing backend\n",
- zram->compressor);
- err = -EINVAL;
- goto out_free_meta;
+ goto out_cleanup;
}
zram->meta = meta;
+ zram->comp = comp;
zram->disksize = disksize;
set_capacity(zram->disk, zram->disksize >> SECTOR_SHIFT);
up_write(>init_lock);
return len;
-out_free_meta:
- up_write(>init_lock);
+out_cleanup:
+ if (comp)
+ zcomp_destroy(comp);
zram_meta_free(meta);
return err;
}
--
1.9.0.382.g7f3562c
--
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] zram: move comp allocation out of init_lock
While fixing lockdep spew of -init_lock reported by Sasha Levin [1], Minchan
Kim noted [2] that it's better to move compression backend allocation (using
GPF_KERNEL) out of the -init_lock lock, same way as with zram_meta_alloc(),
in order to prevent the same lockdep spew.
[1] https://lkml.org/lkml/2014/2/27/337
[2] https://lkml.org/lkml/2014/3/3/32
Cc: Sasha Levin sasha.le...@oracle.com
Reported-by: Minchan Kim minc...@kernel.org
Signed-off-by: Sergey Senozhatsky sergey.senozhat...@gmail.com
---
drivers/block/zram/zram_drv.c | 27 +++
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
index 15d46f2..e4d536b 100644
--- a/drivers/block/zram/zram_drv.c
+++ b/drivers/block/zram/zram_drv.c
@@ -580,9 +580,10 @@ static ssize_t disksize_store(struct device *dev,
struct device_attribute *attr, const char *buf, size_t len)
{
u64 disksize;
+ struct zcomp *comp;
struct zram_meta *meta;
struct zram *zram = dev_to_zram(dev);
- int err;
+ int err = -EINVAL;
disksize = memparse(buf, NULL);
if (!disksize)
@@ -593,30 +594,32 @@ static ssize_t disksize_store(struct device *dev,
if (!meta)
return -ENOMEM;
+ comp = zcomp_create(zram-compressor, zram-max_comp_streams);
+ if (!comp) {
+ pr_info(Cannot initialise %s compressing backend\n,
+ zram-compressor);
+ goto out_cleanup;
+ }
+
down_write(zram-init_lock);
if (init_done(zram)) {
+ up_write(zram-init_lock);
pr_info(Cannot change disksize for initialized device\n);
err = -EBUSY;
- goto out_free_meta;
- }
-
- zram-comp = zcomp_create(zram-compressor, zram-max_comp_streams);
- if (!zram-comp) {
- pr_info(Cannot initialise %s compressing backend\n,
- zram-compressor);
- err = -EINVAL;
- goto out_free_meta;
+ goto out_cleanup;
}
zram-meta = meta;
+ zram-comp = comp;
zram-disksize = disksize;
set_capacity(zram-disk, zram-disksize SECTOR_SHIFT);
up_write(zram-init_lock);
return len;
-out_free_meta:
- up_write(zram-init_lock);
+out_cleanup:
+ if (comp)
+ zcomp_destroy(comp);
zram_meta_free(meta);
return err;
}
--
1.9.0.382.g7f3562c
--
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/
